fix: surface SwiftData save failures instead of swallowing them

Forji/Forji/App/ContentView.swift

@@ -81,7 +81,9 @@ struct ContentView: View {
         do {
             try await authService.restoreSession(instance: instance)
             instance.lastUsed = Date()
-            try? modelContext.save()
+            // Only the last-used timestamp rides on this save: a failure costs list
+            // ordering, not account data, and automatic restore has no error surface.
+            _ = modelContext.saveOrRollback()
         } catch {
             if Self.shouldLogout(after: error) {
                 await authService.logout(modelContext: modelContext)
@@ -139,7 +141,7 @@ struct ContentView: View {
             do {
                 try await authService.restoreSession(instance: defaultInstance)
                 defaultInstance.lastUsed = Date()
-                try? modelContext.save()
+                _ = modelContext.saveOrRollback()
                 sessionFullyRestored = true
             } catch {
                 // A transient/network failure leaves the bootstrapped session in place so the
@@ -154,7 +156,7 @@ struct ContentView: View {
         do {
             try await authService.restoreSession(instance: defaultInstance)
             defaultInstance.lastUsed = Date()
-            try? modelContext.save()
+            _ = modelContext.saveOrRollback()
         } catch {
             // Auto-login failed, fall through to instance list
         }

Forji/Forji/Helpers/PersistenceSave.swift

@@ -0,0 +1,36 @@
+import Foundation
+import SwiftData
+
+/// Turns a SwiftData save failure into a message a view can show.
+///
+/// `assertionFailure` is compiled out under `-O`, so a `catch` that only asserts
+/// discards the error entirely in Release builds and lets the caller continue as
+/// if the write had succeeded. These helpers hand the failure back instead.
+enum PersistenceSave {
+    static func message(for error: Error) -> String {
+        "Your change could not be saved: \(error.localizedDescription)"
+    }
+}
+
+extension ModelContext {
+    /// Saves pending changes and returns a user-facing message when the save fails.
+    ///
+    /// A failed save rolls the context back so the in-memory state matches what is
+    /// actually on disk. Without it a view keeps showing an inserted or edited
+    /// object that would be gone on the next launch.
+    func saveOrRollback() -> String? {
+        saveOrRollback { try save() }
+    }
+
+    /// Testing seam: `performSave` lets tests drive the failure path, which a real
+    /// save on an in-memory store cannot reach.
+    func saveOrRollback(performSave: () throws -> Void) -> String? {
+        do {
+            try performSave()
+            return nil
+        } catch {
+            rollback()
+            return PersistenceSave.message(for: error)
+        }
+    }
+}

Forji/Forji/Services/AuthenticationService.swift

@@ -103,10 +103,7 @@ class AuthenticationService {
             // unauthenticatable orphan, and bail out without clearing the session.
             instance.isDefault = false
             modelContext.delete(instance)
-            do {
-                try modelContext.save()
-            } catch {
-                assertionFailure("SwiftData save failed during logout: \(error)")
+            if modelContext.saveOrRollback() != nil {
                 return
             }
             // Delete credentials only after the instance is gone from SwiftData.

Forji/Forji/Views/InstanceFormView.swift

@@ -4,23 +4,6 @@ import SwiftUI
 
 // swiftlint:disable:next type_body_length
 struct InstanceFormView: View {
-    enum Mode: Identifiable {
-        case add
-        case edit(ForgejoInstance)
-
-        var id: String {
-            switch self {
-            case .add: "add"
-            case let .edit(instance): instance.serverURL + instance.username
-            }
-        }
-    }
-
-    enum AuthMode: String, CaseIterable {
-        case credentials = "Password"
-        case token = "API Token"
-    }
-
     @Environment(\.modelContext) private var modelContext
     @Environment(\.dismiss) private var dismiss
     @Query(sort: \ForgejoInstance.lastUsed, order: .reverse) private var instances: [ForgejoInstance]
@@ -49,6 +32,10 @@ struct InstanceFormView: View {
     @State private var isLoading = false
     @State private var errorMessage: String?
     @State private var showError = false
+    @State private var errorTitle = Self.loginFailedTitle
+
+    private static let loginFailedTitle = "Login Failed"
+    private static let saveFailedTitle = "Save Failed"
 
     init(authService: AuthenticationService, mode: Mode) {
         self.authService = authService
@@ -175,7 +162,7 @@ struct InstanceFormView: View {
                     Button("Cancel") { dismiss() }
                 }
             }
-            .errorAlert("Login Failed", message: $errorMessage, isPresented: $showError)
+            .errorAlert(errorTitle, message: $errorMessage, isPresented: $showError)
             .onAppear {
                 populateFields()
             }
@@ -248,6 +235,7 @@ struct InstanceFormView: View {
                 return inst.name == name && instId != editingInstanceId
             }
             if conflict != nil {
+                errorTitle = Self.loginFailedTitle
                 errorMessage = "An instance named \"\(name)\" already exists. Please choose a different name."
                 showError = true
                 return
@@ -290,10 +278,12 @@ struct InstanceFormView: View {
                         useTokenAuth: authMode == .token,
                     )
                     modelContext.insert(instance)
-                    do {
-                        try modelContext.save()
-                    } catch {
-                        assertionFailure("SwiftData save failed: \(error)")
+                    // The rollback drops the insert, so a store that refused the write
+                    // cannot leave the app reporting a connection to an account that
+                    // would be missing on the next launch.
+                    if let failure = modelContext.saveOrRollback() {
+                        reportSaveFailure(failure)
+                        return
                     }
                     authService.currentInstance = instance
 
@@ -332,10 +322,11 @@ struct InstanceFormView: View {
                     }
                     instance.isDefault = isDefault
 
-                    do {
-                        try modelContext.save()
-                    } catch {
-                        assertionFailure("SwiftData save failed: \(error)")
+                    // The rollback restores the instance's previous field values so the
+                    // form and list keep showing what is actually stored.
+                    if let failure = modelContext.saveOrRollback() {
+                        reportSaveFailure(failure)
+                        return
                     }
                     authService.currentInstance = instance
                 }
@@ -344,6 +335,7 @@ struct InstanceFormView: View {
             } catch AuthenticationError.otpRequired {
                 needsOTP = true
             } catch {
+                errorTitle = Self.loginFailedTitle
                 errorMessage = error.localizedDescription
                 showError = true
             }
@@ -352,6 +344,15 @@ struct InstanceFormView: View {
         }
     }
 
+    /// Surfaces a failed persist and leaves the form open so the account is never
+    /// presented as saved when it is not.
+    private func reportSaveFailure(_ message: String) {
+        errorTitle = Self.saveFailedTitle
+        errorMessage = message
+        showError = true
+        isLoading = false
+    }
+
     /// Sets the duplicate-account error and returns true if another stored account
     /// already uses this server+username. `excluding` skips the instance being edited.
     private func rejectDuplicateAccount(
@@ -362,25 +363,12 @@ struct InstanceFormView: View {
         guard Self.accountAlreadyExists(
             serverURL: normalizedURL, username: username, in: instances, excluding: excluding,
         ) else { return false }
+        errorTitle = Self.loginFailedTitle
         errorMessage = "An account for \"\(username)\" on \(normalizedURL) already exists."
         showError = true
         isLoading = false
         return true
     }
-
-    /// True if `instances` already contains an account with this server+username,
-    /// ignoring `excluding` (the instance being edited). Accounts that share a
-    /// server+username collide on the same sourceKey, which crashes merged overviews.
-    static func accountAlreadyExists(
-        serverURL normalizedURL: String,
-        username: String,
-        in instances: [ForgejoInstance],
-        excluding: ForgejoInstance? = nil,
-    ) -> Bool {
-        instances.contains { inst in
-            inst !== excluding && inst.serverURL == normalizedURL && inst.username == username
-        }
-    }
 }
 
 #if DEBUG

Forji/Forji/Views/InstanceFormViewSupport.swift

@@ -0,0 +1,36 @@
+import Foundation
+
+/// Supporting types and pure checks for `InstanceFormView`, kept out of the view file
+/// so it stays within the file length limit.
+extension InstanceFormView {
+    enum Mode: Identifiable {
+        case add
+        case edit(ForgejoInstance)
+
+        var id: String {
+            switch self {
+            case .add: "add"
+            case let .edit(instance): instance.serverURL + instance.username
+            }
+        }
+    }
+
+    enum AuthMode: String, CaseIterable {
+        case credentials = "Password"
+        case token = "API Token"
+    }
+
+    /// True if `instances` already contains an account with this server+username,
+    /// ignoring `excluding` (the instance being edited). Accounts that share a
+    /// server+username collide on the same sourceKey, which crashes merged overviews.
+    static func accountAlreadyExists(
+        serverURL normalizedURL: String,
+        username: String,
+        in instances: [ForgejoInstance],
+        excluding: ForgejoInstance? = nil,
+    ) -> Bool {
+        instances.contains { inst in
+            inst !== excluding && inst.serverURL == normalizedURL && inst.username == username
+        }
+    }
+}

Forji/Forji/Views/InstanceListView.swift

@@ -14,6 +14,10 @@ struct InstanceListView: View {
     @State private var connectingAll = false
     @State private var errorMessage: String?
     @State private var showError = false
+    @State private var errorTitle = Self.connectionFailedTitle
+
+    private static let connectionFailedTitle = "Connection Failed"
+    private static let saveFailedTitle = "Save Failed"
 
     init(authService: AuthenticationService, multiInstanceManager: Binding<MultiInstanceManager?>) {
         self.authService = authService
@@ -89,7 +93,7 @@ struct InstanceListView: View {
             .sheet(item: $editingInstance) { instance in
                 InstanceFormView(authService: authService, mode: .edit(instance))
             }
-            .errorAlert("Connection Failed", message: $errorMessage, isPresented: $showError)
+            .errorAlert(errorTitle, message: $errorMessage, isPresented: $showError)
         }
     }
 
@@ -196,14 +200,11 @@ struct InstanceListView: View {
             do {
                 try await authService.restoreSession(instance: instance)
                 instance.lastUsed = Date()
-                do {
-                    try modelContext.save()
-                } catch {
-                    assertionFailure("SwiftData save failed: \(error)")
-                }
+                persist()
             } catch is KeychainError {
                 editingInstance = instance
             } catch {
+                errorTitle = Self.connectionFailedTitle
                 errorMessage = error.localizedDescription
                 showError = true
             }
@@ -212,17 +213,18 @@ struct InstanceListView: View {
     }
 
     private func deleteInstance(_ instance: ForgejoInstance) {
-        // Disconnect if deleting the currently active instance
-        if authService.currentInstance?.id == instance.id {
-            authService.disconnect()
-        }
+        let wasActive = authService.currentInstance?.id == instance.id
         let normalizedURL = ForgejoClient.normalizeServerURL(instance.serverURL)
         let username = instance.username
+
+        // Remove the instance from SwiftData first. A failed save rolls the delete
+        // back, so the credentials have to stay put or the restored account would be
+        // left unauthenticatable.
         modelContext.delete(instance)
-        do {
-            try modelContext.save()
-        } catch {
-            assertionFailure("SwiftData save failed: \(error)")
+        guard persist() else { return }
+
+        if wasActive {
+            authService.disconnect()
         }
         Task {
             try? await KeychainManager.shared.deleteCredentials(for: normalizedURL, username: username)
@@ -233,15 +235,16 @@ struct InstanceListView: View {
         defaultAllInstances.toggle()
         if defaultAllInstances {
             ForgejoInstance.clearDefaults(in: instances)
-            do {
-                try modelContext.save()
-            } catch {
-                assertionFailure("SwiftData save failed: \(error)")
+            if !persist() {
+                // The stored per-instance defaults survived the rollback, so the
+                // @AppStorage flag must go back to match them.
+                defaultAllInstances = false
             }
         }
     }
 
     private func toggleDefault(_ instance: ForgejoInstance) {
+        let previousDefaultAll = defaultAllInstances
         if instance.isDefault {
             instance.isDefault = false
         } else {
@@ -249,12 +252,21 @@ struct InstanceListView: View {
             instance.isDefault = true
             defaultAllInstances = false
         }
-        do {
-            try modelContext.save()
-        } catch {
-            assertionFailure("SwiftData save failed: \(error)")
+        if !persist() {
+            defaultAllInstances = previousDefaultAll
         }
     }
+
+    /// Saves pending changes, surfacing a failure to the user. Returns false when the
+    /// save failed and the context was rolled back.
+    @discardableResult
+    private func persist() -> Bool {
+        guard let failure = modelContext.saveOrRollback() else { return true }
+        errorTitle = Self.saveFailedTitle
+        errorMessage = failure
+        showError = true
+        return false
+    }
 }
 
 #if DEBUG

Forji/Forji/Views/SettingsTabView.swift

@@ -149,6 +149,8 @@ struct MergedSettingsTabView: View {
     @AppStorage("defaultAllInstances") private var defaultAllInstances = false
     @State private var showClearCacheConfirmation = false
     @State private var showClearCacheSuccess = false
+    @State private var saveErrorMessage: String?
+    @State private var showSaveError = false
     @Environment(\.modelContext) private var modelContext
     @Query private var allInstances: [ForgejoInstance]
     let manager: MultiInstanceManager
@@ -215,7 +217,13 @@ struct MergedSettingsTabView: View {
                     .onChange(of: defaultAllInstances) { _, isOn in
                         if isOn {
                             ForgejoInstance.clearDefaults(in: allInstances)
-                            try? modelContext.save()
+                            if let failure = modelContext.saveOrRollback() {
+                                // The stored per-instance defaults survived the rollback,
+                                // so the toggle has to go back to match them.
+                                saveErrorMessage = failure
+                                showSaveError = true
+                                defaultAllInstances = false
+                            }
                         }
                     }
             } footer: {
@@ -256,6 +264,7 @@ struct MergedSettingsTabView: View {
             AboutSection()
         }
         .navigationTitle("Settings")
+        .errorAlert("Save Failed", message: $saveErrorMessage, isPresented: $showSaveError)
     }
 
     private func clearCache() {

Forji/ForjiTests/PersistenceSaveTests.swift

@@ -0,0 +1,92 @@
+import Foundation
+import SwiftData
+import Testing
+@testable import Forji
+
+@MainActor
+struct PersistenceSaveTests {
+    private static let saveFailure = NSError(
+        domain: "PersistenceSaveTests",
+        code: 1,
+        userInfo: [NSLocalizedDescriptionKey: "the disk is full"],
+    )
+
+    private func makeContext() throws -> ModelContext {
+        let container = try ModelContainer(
+            for: ForgejoInstance.self,
+            configurations: ModelConfiguration(isStoredInMemoryOnly: true),
+        )
+        return ModelContext(container)
+    }
+
+    private func storedInstances(in context: ModelContext) throws -> [ForgejoInstance] {
+        try context.fetch(FetchDescriptor<ForgejoInstance>())
+    }
+
+    // MARK: - message
+
+    @Test func messageIncludesTheUnderlyingError() {
+        let message = PersistenceSave.message(for: Self.saveFailure)
+        #expect(message.contains("the disk is full"))
+    }
+
+    // MARK: - success path
+
+    @Test func successfulSaveReportsNoFailureAndPersists() throws {
+        let context = try makeContext()
+        context.insert(ForgejoInstance(serverURL: "https://a.com", username: "user", name: "Work"))
+
+        #expect(context.saveOrRollback() == nil)
+        #expect(try storedInstances(in: context).count == 1)
+    }
+
+    // MARK: - failure path
+
+    @Test func failedSaveReportsTheFailure() throws {
+        let context = try makeContext()
+
+        let failure = context.saveOrRollback { throw Self.saveFailure }
+
+        #expect(failure?.contains("the disk is full") == true)
+    }
+
+    @Test func failedSaveDiscardsAPendingInsert() throws {
+        // The bug this guards: a failed save used to leave the inserted account in the
+        // context, so the app showed it as connected until the next launch lost it.
+        let context = try makeContext()
+        context.insert(ForgejoInstance(serverURL: "https://a.com", username: "user", name: "Work"))
+
+        #expect(context.saveOrRollback { throw Self.saveFailure } != nil)
+
+        #expect(try storedInstances(in: context).isEmpty)
+    }
+
+    @Test func failedSaveRestoresAPendingDelete() throws {
+        let context = try makeContext()
+        context.insert(ForgejoInstance(serverURL: "https://a.com", username: "user", name: "Work"))
+        try context.save()
+
+        let instance = try #require(try storedInstances(in: context).first)
+        context.delete(instance)
+
+        #expect(context.saveOrRollback { throw Self.saveFailure } != nil)
+
+        #expect(try storedInstances(in: context).count == 1)
+    }
+
+    @Test func failedSaveRestoresEditedValues() throws {
+        let context = try makeContext()
+        context.insert(ForgejoInstance(serverURL: "https://a.com", username: "user", name: "Work"))
+        try context.save()
+
+        let instance = try #require(try storedInstances(in: context).first)
+        instance.name = "Renamed"
+        instance.isDefault = true
+
+        #expect(context.saveOrRollback { throw Self.saveFailure } != nil)
+
+        let restored = try #require(try storedInstances(in: context).first)
+        #expect(restored.name == "Work")
+        #expect(!restored.isDefault)
+    }
+}