diff --git a/apps/desktop/src/main/__tests__/use-shell-connections.test.ts b/apps/desktop/src/main/__tests__/use-shell-connections.test.ts index bfd7bfef24..a38f45eb81 100644 --- a/apps/desktop/src/main/__tests__/use-shell-connections.test.ts +++ b/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>> = []; @@ -210,8 +210,13 @@ 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(); @@ -219,13 +224,18 @@ test('invalidates stale connection choices until the current refresh succeeds', 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; 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(); + (globalThis.window as unknown as { maka: unknown }).maka = { + connections: { getSnapshot: async () => pending.promise }, + }; + let current!: ReturnType; + + 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(); + (globalThis.window as unknown as { maka: unknown }).maka = { + connections: { getSnapshot: async () => pending.promise }, + }; + let current!: ReturnType; + + 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; + + 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> }> = []; + (globalThis.window as unknown as { maka: unknown }).maka = { + runtimeHostProfiles: { getDefaultHost: async () => currentHost }, + connections: { + getSnapshot: (_sessionId?: string, host?: { hostId: string }) => { + const pending = deferred(); + reads.push({ hostId: host?.hostId ?? 'unknown', pending }); + return pending.promise; + }, + }, + }; + let current!: ReturnType; + + 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; + 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' }); diff --git a/apps/desktop/src/renderer/use-shell-connections.ts b/apps/desktop/src/renderer/use-shell-connections.ts index 0bacc224c1..1715c8e282 100644 --- a/apps/desktop/src/renderer/use-shell-connections.ts +++ b/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,