fix(desktop): serve the last connection snapshot while a refresh is in flight (#4611)

apps/desktop/src/main/__tests__/use-shell-connections.test.ts

@@ -168,7 +168,7 @@ test('reports connection refresh failures against their owning Host', async () =
   ]);
 });
 
-test('invalidates stale connection choices until the current refresh succeeds', async () => {
+test('keeps serving the last ready snapshot while a refresh is in flight (#4611)', async () => {
   const { root } = installReactRenderer();
   const sessionId = desktopSessionKey({ hostId: 'host-a', sessionId: 'session-a' });
   const refreshes: Array<ReturnType<typeof deferred<DesktopConnectionSnapshot>>> = [];
@@ -210,22 +210,32 @@ test('invalidates stale connection choices until the current refresh succeeds',
     failedRefresh = current.refreshConnections();
     await Promise.resolve();
   });
-  assert.equal(projectionStatus(), 'refreshing', 'a pending refresh must hide stale choices');
-  assert.deepEqual(current.snapshot, EMPTY);
+  assert.equal(projectionStatus(), 'refreshing');
+  assert.equal(
+    current.snapshot.defaultConnection,
+    'stale-connection',
+    'a pending refresh must keep serving the last ready snapshot (#4611): emptying ' +
+      'it flashed the composer "no model connection" banner and blocked send',
+  );
   await act(async () => {
     current.seedSnapshot(snapshot('stale-onboarding-seed'));
     await Promise.resolve();
   });
   assert.equal(
     projectionStatus(),
     'refreshing',
-    'an onboarding seed must not repopulate a projection invalidated by refresh',
+    'an onboarding seed must not repopulate a projection mid-refresh',
   );
   await act(async () => {
     refreshes[1]?.reject(new Error('refresh failed'));
     await failedRefresh;
   });
   assert.equal(projectionStatus(), 'refreshing', 'a failed refresh must remain unsettled');
+  assert.equal(
+    current.snapshot.defaultConnection,
+    'stale-connection',
+    'a failed refresh must not retract the last ready snapshot either (#4611)',
+  );
 
   let recoveredRefresh!: Promise<void>;
   await act(async () => {
@@ -238,6 +248,196 @@ test('invalidates stale connection choices until the current refresh succeeds',
   assert.equal(current.snapshot.defaultConnection, 'replacement-connection');
 });
 
+test('a first load with no prior snapshot still reads empty until ready (#4611)', async () => {
+  const { root } = installReactRenderer();
+  const sessionId = desktopSessionKey({ hostId: 'host-a', sessionId: 'session-a' });
+  const pending = deferred<DesktopConnectionSnapshot>();
+  (globalThis.window as unknown as { maka: unknown }).maka = {
+    connections: { getSnapshot: async () => pending.promise },
+  };
+  let current!: ReturnType<typeof useShellConnections>;
+
+  function Probe() {
+    current = useShellConnections({
+      toastApi: { error: () => undefined },
+      uiLocale: 'en',
+      target: { kind: 'session', sessionId },
+    });
+    return null;
+  }
+
+  await act(async () => {
+    root.render(createElement(Probe));
+    await Promise.resolve();
+  });
+  assert.equal(current.projection.status, 'refreshing');
+  assert.deepEqual(
+    current.snapshot,
+    EMPTY,
+    'nothing has been read yet, so there is no stale snapshot to serve',
+  );
+  await act(async () => {
+    pending.resolve(snapshot('first-connection'));
+    await pending.promise;
+  });
+  assert.equal(current.snapshot.defaultConnection, 'first-connection');
+});
+
+test('the startup seed fills an in-flight first refresh (#4611)', async () => {
+  const { root } = installReactRenderer();
+  const sessionId = desktopSessionKey({ hostId: 'host-a', sessionId: 'session-a' });
+  const pending = deferred<DesktopConnectionSnapshot>();
+  (globalThis.window as unknown as { maka: unknown }).maka = {
+    connections: { getSnapshot: async () => pending.promise },
+  };
+  let current!: ReturnType<typeof useShellConnections>;
+
+  function Probe() {
+    current = useShellConnections({
+      toastApi: { error: () => undefined },
+      uiLocale: 'en',
+      target: { kind: 'session', sessionId },
+    });
+    return null;
+  }
+
+  await act(async () => {
+    root.render(createElement(Probe));
+    await Promise.resolve();
+  });
+  assert.equal(current.projection.status, 'refreshing');
+  // The mount layout effect beat the shell's passive seed effect to the key;
+  // the seed must still land or the cold-start composer reads EMPTY (#4611).
+  await act(async () => {
+    current.seedSnapshot(snapshot('startup-seed-connection'));
+  });
+  assert.equal(current.snapshot.defaultConnection, 'startup-seed-connection');
+  // The in-flight read stays authoritative once it lands.
+  await act(async () => {
+    pending.resolve(snapshot('read-connection'));
+    await pending.promise;
+  });
+  assert.equal(current.projection.status, 'ready');
+  assert.equal(current.snapshot.defaultConnection, 'read-connection');
+});
+
+test('a refresh that resolves to a genuinely empty catalog shows empty (#4611)', async () => {
+  const { root } = installReactRenderer();
+  const sessionId = desktopSessionKey({ hostId: 'host-a', sessionId: 'session-a' });
+  const results: DesktopConnectionSnapshot[] = [snapshot('doomed-connection'), EMPTY];
+  (globalThis.window as unknown as { maka: unknown }).maka = {
+    connections: { getSnapshot: async () => results.shift() ?? EMPTY },
+  };
+  let current!: ReturnType<typeof useShellConnections>;
+
+  function Probe() {
+    current = useShellConnections({
+      toastApi: { error: () => undefined },
+      uiLocale: 'en',
+      target: { kind: 'session', sessionId },
+    });
+    return null;
+  }
+
+  await act(async () => {
+    root.render(createElement(Probe));
+    await Promise.resolve();
+  });
+  assert.equal(current.snapshot.defaultConnection, 'doomed-connection');
+  // The connection was deleted upstream: after the refresh lands the banner
+  // must appear — stale-while-revalidate covers the flight, not the outcome.
+  await act(async () => {
+    await current.refreshConnections();
+  });
+  assert.equal(current.projection.status, 'ready');
+  assert.deepEqual(current.snapshot, EMPTY);
+});
+
+test('a default-Host identity change never serves the previous Host catalog (#4611 review)', async () => {
+  const { root } = installReactRenderer();
+  let currentHost = { profileId: 'profile-a', hostId: 'host-a' };
+  const reads: Array<{ hostId: string; pending: ReturnType<typeof deferred<DesktopConnectionSnapshot>> }> = [];
+  (globalThis.window as unknown as { maka: unknown }).maka = {
+    runtimeHostProfiles: { getDefaultHost: async () => currentHost },
+    connections: {
+      getSnapshot: (_sessionId?: string, host?: { hostId: string }) => {
+        const pending = deferred<DesktopConnectionSnapshot>();
+        reads.push({ hostId: host?.hostId ?? 'unknown', pending });
+        return pending.promise;
+      },
+    },
+  };
+  let current!: ReturnType<typeof useShellConnections>;
+
+  function Probe() {
+    current = useShellConnections({
+      toastApi: { error: () => undefined },
+      uiLocale: 'en',
+      target: { kind: 'default' },
+    });
+    return null;
+  }
+
+  const flush = () => new Promise((resolve) => setTimeout(resolve, 0));
+
+  await act(async () => {
+    root.render(createElement(Probe));
+  });
+  await act(async () => {
+    reads[0]?.pending.resolve(snapshot('connection-a'));
+    await reads[0]?.pending.promise;
+  });
+  assert.equal(current.snapshot.defaultConnection, 'connection-a');
+
+  // Same Host: a refresh keeps serving the last ready snapshot mid-flight.
+  await act(async () => {
+    void current.refreshConnections();
+    await flush();
+  });
+  assert.equal(current.projection.status, 'refreshing');
+  assert.equal(current.snapshot.defaultConnection, 'connection-a');
+  await act(async () => {
+    reads[1]?.pending.resolve(snapshot('connection-a2'));
+    await reads[1]?.pending.promise;
+  });
+  assert.equal(current.snapshot.defaultConnection, 'connection-a2');
+
+  // The default Host switches A -> B: Host A's catalog must not survive into
+  // Host B's read window, nor be resurrected when Host B's read fails.
+  currentHost = { profileId: 'profile-b', hostId: 'host-b' };
+  let failedRead!: Promise<void>;
+  await act(async () => {
+    failedRead = current.refreshConnections();
+    await flush();
+  });
+  assert.equal(reads.at(-1)?.hostId, 'host-b');
+  assert.equal(current.projection.status, 'refreshing');
+  assert.deepEqual(
+    current.snapshot,
+    EMPTY,
+    "Host A connections must not stay visible while Host B's catalog is read",
+  );
+  await act(async () => {
+    reads.at(-1)?.pending.reject(new Error('host b read failed'));
+    await failedRead;
+  });
+  assert.deepEqual(
+    current.snapshot,
+    EMPTY,
+    "a failed Host B read must not resurrect Host A's catalog",
+  );
+
+  // Recovery reads Host B for real.
+  await act(async () => {
+    void current.refreshConnections();
+    await flush();
+    reads.at(-1)?.pending.resolve(snapshot('connection-b'));
+    await reads.at(-1)?.pending.promise;
+  });
+  assert.equal(current.projection.status, 'ready');
+  assert.equal(current.snapshot.defaultConnection, 'connection-b');
+});
+
 test('an older failed refresh cannot erase a newer successful snapshot', async () => {
   const { root } = installReactRenderer();
   const sessionId = desktopSessionKey({ hostId: 'host-a', sessionId: 'session-a' });

apps/desktop/src/renderer/use-shell-connections.ts

@@ -53,9 +53,32 @@ type ShellConnectionTarget =
   | { readonly kind: 'new-task'; readonly host?: DesktopNewTaskHostRef }
   | { readonly kind: 'session'; readonly sessionId?: string };
 
+/**
+ * Stale-while-revalidate (#4611): a refresh keeps serving the last ready
+ * snapshot until the new read lands. Emptying the projection mid-refresh made
+ * the composer flash "no model connection" and block send on every
+ * `connection_list_changed` — and a failed refresh left it there permanently.
+ * The staleness window is one IPC round-trip; anything the stale catalog still
+ * names is re-validated by the Host on use (`NO_REAL_CONNECTION` path).
+ *
+ * `hostId` records which Host produced the snapshot. Session and new-task
+ * targets key by hostId, so their identity is stable per key — but the
+ * 'default' target uses one constant key while the Host behind it can change
+ * (profile switch). Carrying Host A's catalog into Host B's read window would
+ * let users act on connections that belong to the previous Host, so a default
+ * refresh whose resolved identity differs drops the old snapshot instead.
+ */
 type StoredShellConnectionProjection =
-  | { readonly status: 'refreshing' }
-  | { readonly status: 'ready'; readonly snapshot: DesktopConnectionSnapshot };
+  | {
+      readonly status: 'refreshing';
+      readonly snapshot?: DesktopConnectionSnapshot;
+      readonly hostId?: string;
+    }
+  | {
+      readonly status: 'ready';
+      readonly snapshot: DesktopConnectionSnapshot;
+      readonly hostId?: string;
+    };
 
 export type ShellConnectionProjection =
   | { readonly status: 'unrequested' }
@@ -92,8 +115,15 @@ export function useShellConnections(options: {
   }, [snapshotKey]);
 
   function seedSnapshot(next: DesktopConnectionSnapshot) {
+    // Seed into any entry that has no snapshot yet — including an in-flight
+    // first refresh. The mount layout effect writes `{status:'refreshing'}`
+    // before the shell's passive seed effect can run, so guarding on key
+    // existence alone made the startup snapshot dead code and left the
+    // first-paint / failed-first-refresh windows reading EMPTY (#4611). A
+    // successful refresh still overwrites the seed; an entry that already has
+    // a snapshot (ready or stale-while-revalidate) keeps it.
     setProjections((previous) =>
-      previous.has(snapshotKey)
+      previous.get(snapshotKey)?.snapshot !== undefined
         ? previous
         : new Map(previous).set(snapshotKey, { status: 'ready', snapshot: next }),
     );
@@ -105,26 +135,57 @@ export function useShellConnections(options: {
     if (key === NO_HOST_KEY) return;
     const sequence = (refreshSequence.current.get(key) ?? 0) + 1;
     refreshSequence.current.set(key, sequence);
-    // A refresh means the Host catalog may already have changed. Stop exposing
-    // its previous mutation targets immediately; only a successful current read
-    // may make this key settled again.
-    setProjections((previous) => new Map(previous).set(key, { status: 'refreshing' }));
+    // A refresh means the Host catalog may already have changed, so the read
+    // must land before its result is trusted — but the previous snapshot stays
+    // visible while it flies (#4611, see the type above). Only a successful
+    // current read replaces what consumers see. Session/new-task targets key
+    // by hostId, so the carry is always same-Host there; the default target
+    // gets its identity inside the read below.
+    const markRefreshing = (hostId?: string): void => {
+      setProjections((previous) => {
+        const prior = previous.get(key);
+        const carrySnapshot =
+          prior?.snapshot !== undefined &&
+          (hostId === undefined || prior.hostId === undefined || prior.hostId === hostId);
+        return new Map(previous).set(key, {
+          status: 'refreshing',
+          ...(carrySnapshot ? { snapshot: prior.snapshot } : {}),
+          ...(hostId === undefined ? {} : { hostId }),
+        });
+      });
+    };
     try {
       let next: DesktopConnectionSnapshot;
+      let nextHostId: string | undefined;
       if (target.kind === 'session' && target.sessionId) {
+        markRefreshing();
         next = await window.maka.connections.getSnapshot(target.sessionId);
       } else if (target.kind === 'new-task' && target.host) {
+        markRefreshing();
         next = await window.maka.newTasks.getConnections(target.host);
       } else if (target.kind === 'default') {
-        next = (
-          await runOnDefaultRuntimeHost((host) =>
-            window.maka.connections.getSnapshot(undefined, host),
-          )
-        ).value;
+        // The constant 'default' key outlives the Host behind it (profile
+        // switch), so the refreshing write waits for the resolved identity:
+        // a snapshot the previous default Host produced must not survive into
+        // the new one's read window (#4611 review). If the resolve itself
+        // fails, no refreshing write happens and the last snapshot stays put;
+        // the catch below toasts either way. A snapshot with unknown
+        // provenance (the startup seed) is still carried: it is the boot-time
+        // default catalog, and dropping it would reopen the first-paint flash.
+        const result = await runOnDefaultRuntimeHost(async (host) => {
+          markRefreshing(host.hostId);
+          return window.maka.connections.getSnapshot(undefined, host);
+        });
+        next = result.value;
+        nextHostId = result.host.hostId;
       } else return;
       if (refreshSequence.current.get(key) !== sequence) return;
       setProjections((previous) =>
-        new Map(previous).set(key, { status: 'ready', snapshot: next }),
+        new Map(previous).set(key, {
+          status: 'ready',
+          snapshot: next,
+          ...(nextHostId === undefined ? {} : { hostId: nextHostId }),
+        }),
       );
     } catch (error) {
       if (
@@ -155,7 +216,10 @@ export function useShellConnections(options: {
 
   const projection = projections.get(snapshotKey) ?? { status: 'unrequested' as const };
   return {
-    snapshot: projection.status === 'ready' ? projection.snapshot : EMPTY_SNAPSHOT,
+    snapshot:
+      projection.status === 'unrequested'
+        ? EMPTY_SNAPSHOT
+        : (projection.snapshot ?? EMPTY_SNAPSHOT),
     projection,
     seedSnapshot,
     refreshConnections,