fix: prevent supabase org credentials from being lost due to stale settings reads (#2821)

rules/electron-ipc.md

@@ -51,6 +51,23 @@ ipc.chatStream.start(params, { onChunk, onEnd, onError });
 - `createStreamClient(...).start()` returns `void`, not a cleanup/unsubscribe function. You cannot capture a handle to abort or clean up an active stream from the caller side.
 - To guard against duplicate streams, use a module-level `Set` (like `pendingStreamChatIds` in `useStreamChat.ts`) or a React state/ref-based lock, not the return value.
 
+## Settings write safety (`writeSettings`)
+
+`writeSettings(partial)` does a **shallow top-level merge**: `{ ...currentSettings, ...partial }`. This means passing `{ supabase: { organizations: { ... } } }` replaces the entire `supabase` key, losing sibling fields like legacy tokens. Callers must spread the existing parent object:
+
+```ts
+// WRONG — destroys supabase.organizations and other fields
+writeSettings({ supabase: { accessToken: { value: newToken } } });
+
+// RIGHT — preserves sibling fields
+const settings = readSettings();
+writeSettings({
+  supabase: { ...settings.supabase, accessToken: { value: newToken } },
+});
+```
+
+**Stale-read race condition:** If you call `readSettings()` before an async operation (network call, file I/O), then use the snapshot to construct the write, any concurrent settings changes during the async gap will be silently overwritten. Always call `readSettings()` immediately before `writeSettings()` — never across an `await` boundary.
+
 ## Handler expectations
 
 - Handlers should `throw new Error("...")` on failure instead of returning `{ success: false }` style payloads.

src/supabase_admin/supabase_management_client.ts

@@ -144,9 +144,12 @@ export async function refreshSupabaseToken(): Promise<void> {
       expiresIn,
     } = await response.json();
 
-    // Update settings with new tokens
+    // Re-read settings right before writing to get latest state
+    const freshSettings = readSettings();
+    // Update settings with new tokens, preserving existing fields (e.g. organizations map)
     writeSettings({
       supabase: {
+        ...freshSettings.supabase,
         accessToken: {
           value: accessToken,
         },
@@ -273,15 +276,18 @@ async function refreshSupabaseTokenForOrganization(
       expiresIn,
     } = await response.json();
 
-    // Update the specific organization in settings
-    const existingOrgs = settings.supabase?.organizations ?? {};
+    // Re-read settings right before writing to avoid stale-read race conditions.
+    // The async fetch above may take time, during which other org credentials
+    // could be written. Reading here ensures we merge into the latest state.
+    const freshSettings = readSettings();
+    const existingOrgs = freshSettings.supabase?.organizations ?? {};
     writeSettings({
       supabase: {
-        ...settings.supabase,
+        ...freshSettings.supabase,
         organizations: {
           ...existingOrgs,
           [organizationSlug]: {
-            ...org,
+            ...existingOrgs[organizationSlug],
             accessToken: {
               value: accessToken,
             },

src/supabase_admin/supabase_return_handler.ts

@@ -20,7 +20,6 @@ export async function handleSupabaseOAuthReturn({
   refreshToken,
   expiresIn,
 }: SupabaseOAuthReturnParams) {
-  const settings = readSettings();
   let orgs: any[] = [];
   let errorOccurred = false;
 
@@ -31,6 +30,12 @@ export async function handleSupabaseOAuthReturn({
     errorOccurred = true;
   }
 
+  // Re-read settings right before writing to avoid stale-read race conditions.
+  // The async listSupabaseOrganizations call above may take time, during which
+  // other org credentials could be written (e.g. token refreshes). Reading here
+  // ensures we merge into the latest state.
+  const settings = readSettings();
+
   if (!errorOccurred && orgs.length > 0) {
     if (orgs.length > 1) {
       logger.warn(