From 745b7af45b14bcb9eadd75a1b63824a85643804b Mon Sep 17 00:00:00 2001
From: Stefan Hausotte <stefan.hausotte@gmx.de>
Date: Thu, 4 Jun 2026 14:08:07 +0200
Subject: [PATCH] fix: persist instance removal before deleting keychain on
 logout (#48)

logout deleted keychain credentials first, then removed the SwiftData instance
with a swallowed `try? modelContext.save()`. If the save failed, the instance
survived with its credentials gone, leaving an account that could not
authenticate. A `nil` default modelContext also let the instance removal be
skipped entirely while credentials were still wiped.

Reorder to delete + save the SwiftData instance first (matching
InstanceListView.deleteInstance) and only delete credentials once that succeeds;
bail out on save failure so the account stays usable. Make modelContext
required to remove the latent orphan path (all call sites already pass one).

Reviewed-on: https://codeberg.org/secana/Forji/pulls/65
---
 .../Services/AuthenticationService.swift      | 20 ++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/Forji/Forji/Services/AuthenticationService.swift b/Forji/Forji/Services/AuthenticationService.swift
index b506ada..99f180c 100644
--- a/Forji/Forji/Services/AuthenticationService.swift
+++ b/Forji/Forji/Services/AuthenticationService.swift
@@ -95,9 +95,21 @@ class AuthenticationService {
         return true
     }
 
-    func logout(modelContext: ModelContext? = nil) async {
+    func logout(modelContext: ModelContext) async {
         if let instance = currentInstance {
             let normalizedURL = ForgejoClient.normalizeServerURL(instance.serverURL)
+            // Remove the instance from SwiftData first. If the save fails, keep the
+            // credentials so the account stays usable instead of becoming an
+            // 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)")
+                return
+            }
+            // Delete credentials only after the instance is gone from SwiftData.
             do {
                 try await KeychainManager.shared.deleteCredentials(
                     for: normalizedURL,
@@ -109,12 +121,6 @@ class AuthenticationService {
                     print("Keychain delete failed during logout: \(error)")
                 #endif
             }
-            // Remove the instance from SwiftData and unset default
-            if let modelContext {
-                instance.isDefault = false
-                modelContext.delete(instance)
-                try? modelContext.save()
-            }
         }
         // Clears cache for all instances since keys are hashed and cannot be scoped per-instance
         DiskCache.removeAll()