From cf69e2bcca69b3231d03188d5f49044e360207ef Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 16 Mar 2026 04:12:20 +0000
Subject: [PATCH 1/2] Initial plan


From 237baf85e4e392edc55172dbf16f7b7eb2d041e2 Mon Sep 17 00:00:00 2001
From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com>
Date: Mon, 16 Mar 2026 04:18:42 +0000
Subject: [PATCH 2/2] fix: ConnectionSettings secret migration hardening
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Save() now returns bool (true on success, false on failure)
- RecoverSecretsFromSecureStorage uses Save() return value instead of
  File.Exists() to guard Keychain cleanup — eliminates data loss if
  Save() fails but an old settings file exists
- Added explanatory comment on ReadSecureStorage's intentional
  sync-over-async Task.Run pattern
- Outer catch in RecoverSecretsFromSecureStorage now logs to
  Debug.WriteLine and ~/.polypilot/crash.log instead of swallowing silently

Co-authored-by: PureWeen <5375137+PureWeen@users.noreply.github.com>
---
 PolyPilot/Models/ConnectionSettings.cs | 27 +++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/PolyPilot/Models/ConnectionSettings.cs b/PolyPilot/Models/ConnectionSettings.cs
index 1fd653b6..214b2f50 100644
--- a/PolyPilot/Models/ConnectionSettings.cs
+++ b/PolyPilot/Models/ConnectionSettings.cs
@@ -259,7 +259,7 @@ private static ConnectionSettings DefaultSettings()
 #endif
     }
 
-    public void Save()
+    public bool Save()
     {
         try
         {
@@ -272,8 +272,9 @@ public void Save()
             var json = JsonSerializer.Serialize(this, new JsonSerializerOptions { WriteIndented = true });
 #endif
             File.WriteAllText(SettingsPath, json);
+            return true;
         }
-        catch { }
+        catch { return false; }
     }
 
 #if MACCATALYST
@@ -308,12 +309,13 @@ private static void RecoverSecretsFromSecureStorage(ConnectionSettings settings)
 
             if (needsSave)
             {
-                settings.Save();
+                bool saved = settings.Save();
 
                 // Per-key cleanup: only remove a Keychain entry if that specific value was recovered
-                // and Save() wrote the file. Prevents data loss if Keychain read fails transiently
-                // for one secret but succeeds for another.
-                if (File.Exists(SettingsPath))
+                // and Save() succeeded. Using the bool return value is stronger than File.Exists():
+                // if Save() fails (e.g., disk full) but a prior file exists, File.Exists would return
+                // true and we'd delete Keychain entries without having persisted the recovered values.
+                if (saved)
                 {
                     if (recoveredRemote)
                         try { SecureStorage.Default.Remove("polypilot.connection.remoteToken"); } catch { }
@@ -324,11 +326,22 @@ private static void RecoverSecretsFromSecureStorage(ConnectionSettings settings)
                 }
             }
         }
-        catch { }
+        catch (Exception ex)
+        {
+            System.Diagnostics.Debug.WriteLine($"[ConnectionSettings] RecoverSecretsFromSecureStorage failed: {ex.Message}");
+            try
+            {
+                var logPath = Path.Combine(GetPolyPilotDir(), "crash.log");
+                File.AppendAllText(logPath, $"\n=== {DateTime.Now:yyyy-MM-dd HH:mm:ss} [RecoverSecretsFromSecureStorage] ===\n{ex}\n");
+            }
+            catch { /* Don't throw in exception handler */ }
+        }
     }
 
     private static string? ReadSecureStorage(string key)
     {
+        // Intentional sync-over-async: Task.Run avoids SynchronizationContext deadlock
+        // on the UI thread while keeping this one-time migration path synchronous.
         try { return Task.Run(() => SecureStorage.Default.GetAsync(key)).GetAwaiter().GetResult(); }
         catch { return null; }
     }