From 17822d2fe7a6a242f5f2a2c8faf260df8a781df8 Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sat, 5 Sep 2026 19:23:21 -0700 Subject: [PATCH] fix(native-chat): bound the capability probe and stop naming the wrong machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-flight guard releases when the activation settles, so an await that never settles holds the row for the life of the process. The capability probe was the one call in the chain not raced against a deadline: on a cache hit it awaits a promise an earlier probe created, which may carry no deadline of its own. Race it like the two calls around it. A version block can name either side — evaluateRuntimeCompat reports client-too-old as well as host-too-old — so a message that blamed the host pointed half of those at the wrong machine. Name the remedy instead of the machine, which is true for every case that reaches it. --- src/renderer/src/i18n/locales/en.json | 2 +- ...ai-vault-structured-session-reveal.test.ts | 18 ++ .../activate-ai-vault-structured-session.ts | 23 ++- .../src/runtime/__scratch-absorption.test.ts | 186 ++++++++++++++++++ 4 files changed, 220 insertions(+), 9 deletions(-) create mode 100644 src/renderer/src/runtime/__scratch-absorption.test.ts diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index a2c6a4aad76..c10b5069b47 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -965,7 +965,7 @@ "activateAiVaultStructuredSession": { "unavailable": "The structured agent session is not available yet. Retry in a moment.", "gone": "This chat is no longer on this host, so it cannot be reopened here.", - "hostCannotOpen": "This host can't reopen that chat. It may need an Orca update." + "hostCannotOpen": "This chat can't be reopened until Orca is updated." }, "ephemeralVmWorktreeCreation": { "sparseCheckoutUnsupported": "Provisioned-root recipes do not support sparse checkout." diff --git a/src/renderer/src/lib/activate-ai-vault-structured-session-reveal.test.ts b/src/renderer/src/lib/activate-ai-vault-structured-session-reveal.test.ts index 066480f4159..0de7f0edb3a 100644 --- a/src/renderer/src/lib/activate-ai-vault-structured-session-reveal.test.ts +++ b/src/renderer/src/lib/activate-ai-vault-structured-session-reveal.test.ts @@ -95,6 +95,24 @@ describe('revealStructuredSession', () => { expect(mocks.call).not.toHaveBeenCalled() }) + it('gives up on a probe that never settles instead of hanging the row', async () => { + // The in-flight guard releases on settle, so a probe that never answers would otherwise hold + // this session's entry for the life of the process and leave the row permanently dead. + vi.useFakeTimers() + mocks.environmentIdFor.mockReturnValue('env-1') + mocks.supports.mockReturnValue(new Promise(() => {})) + + try { + const outcome = revealStructuredSession(target) + await vi.advanceTimersByTimeAsync(10_000) + + await expect(outcome).resolves.toBe('unreachable') + expect(mocks.call).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) + it('reports the chat gone only for the refusal that means the record is absent', async () => { mocks.call.mockResolvedValue({ ok: false, diff --git a/src/renderer/src/lib/activate-ai-vault-structured-session.ts b/src/renderer/src/lib/activate-ai-vault-structured-session.ts index 69d95eef2db..298b4ea299a 100644 --- a/src/renderer/src/lib/activate-ai-vault-structured-session.ts +++ b/src/renderer/src/lib/activate-ai-vault-structured-session.ts @@ -62,14 +62,16 @@ const defaultDeps: StructuredSessionActivationDeps = { ) ) }, - // Separate from `gone` because the chat is not gone — this host cannot open it, whether it is - // too old to know the method or its adapters do not cover that provider. Telling someone their - // work is lost when an update would bring it back is the worse of the two wrong answers. + // Separate from `gone` because the chat is not gone: something about this pairing cannot open + // it — a host too old to know the method, adapters that do not cover the provider, or a version + // block that can name EITHER side. Naming a machine here would point half of those at the wrong + // one, so the message names the remedy instead. Telling someone their work is lost when an + // update would bring it back is the worse of the two wrong answers. hostCannotOpen: () => { toast.error( translate( 'auto.lib.activateAiVaultStructuredSession.hostCannotOpen', - "This host can't reopen that chat. It may need an Orca update." + "This chat can't be reopened until Orca is updated." ) ) } @@ -179,10 +181,15 @@ export async function revealStructuredSession(target: { if (host.kind === 'environment') { let supported: boolean try { - supported = await runtimeEnvironmentSupportsCapability( - host.environmentId, - STRUCTURED_AGENT_SESSION_REVEAL_RUNTIME_CAPABILITY, - STRUCTURED_SESSION_RESTORE_TIMEOUT_MS + // Raced, not merely given a timeout argument: on a cache hit this awaits a promise created + // by an earlier probe that may carry no deadline of its own, and one that never settles + // would hold this session's in-flight entry for the life of the process. + supported = await withStructuredSessionRestoreTimeout( + runtimeEnvironmentSupportsCapability( + host.environmentId, + STRUCTURED_AGENT_SESSION_REVEAL_RUNTIME_CAPABILITY, + STRUCTURED_SESSION_RESTORE_TIMEOUT_MS + ) ) } catch (error) { // A version block is the host's age, stated outright; anything else means we never got to diff --git a/src/renderer/src/runtime/__scratch-absorption.test.ts b/src/renderer/src/runtime/__scratch-absorption.test.ts new file mode 100644 index 00000000000..b71ff5aa447 --- /dev/null +++ b/src/renderer/src/runtime/__scratch-absorption.test.ts @@ -0,0 +1,186 @@ +// @vitest-environment happy-dom +// +// Scratch repro: a host-published structured chat tab is dropped by the LIVE renderer +// when the host's stored snapshotVersion for the worktree regresses under an unchanged +// publication lineage (the `renderer:` epoch), which is exactly what happens after +// the host prunes and recreates its per-worktree entry. + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import { + applyLocalStructuredSessionTabSnapshots, + resetLocalStructuredSessionVersionForTests +} from './local-structured-session-tabs-sync' +import { + resetWebSessionTabsSnapshotFreshnessForTests, + type WebSessionTabsSyncState +} from './web-session-tabs-sync' +import { resetWebSessionFocusIntentForTests } from './web-session-focus-intent' + +const W = 'repo-1::/tmp/wt1' +// Stable for the whole renderer lifetime: src/renderer/src/runtime/sync-runtime-graph/graph-state.ts:36 +const RENDERER_EPOCH = 'renderer:11111111-2222-3333-4444-555555555555' + +beforeEach(() => { + resetLocalStructuredSessionVersionForTests() + resetWebSessionTabsSnapshotFreshnessForTests() + resetWebSessionFocusIntentForTests() +}) +afterEach(() => { + resetLocalStructuredSessionVersionForTests() + resetWebSessionFocusIntentForTests() +}) + +function emptyState(): WebSessionTabsSyncState { + return { + activeBrowserTabId: null, + activeBrowserTabIdByWorktree: {}, + activeFileId: null, + activeFileIdByWorktree: {}, + activeGroupIdByWorktree: {}, + activeTabId: null, + activeTabIdByWorktree: {}, + activeTabType: 'terminal', + activeTabTypeByWorktree: {}, + activeWorktreeId: W, + agentStatusByPaneKey: {}, + agentStatusEpoch: 0, + browserCertificateFailuresByPageId: {}, + browserPagesByWorkspace: {}, + browserTabsByWorktree: {}, + groupsByWorktree: {}, + layoutByWorktree: {}, + openFiles: [], + ptyIdsByTabId: {}, + remoteBrowserPageHandlesByPageId: {}, + tabBarOrderByWorktree: {}, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + unifiedTabsByWorktree: {}, + unreadTerminalTabs: {}, + sortEpoch: 0 + } +} + +/** What `publishStructuredAgentSessionTab` emits for a chat tab on this worktree. */ +function chatSnapshot( + publicationEpoch: string, + snapshotVersion: number +): RuntimeMobileSessionTabsResult { + return { + worktree: W, + publicationEpoch, + snapshotVersion, + // Observed in QA: the host had no `tabGroups`, so the publish defaulted to this group id. + activeGroupId: `headless-terminals:${W}`, + activeTabId: 'agent-session:codex-1', + activeTabType: 'agent-session', + tabGroups: [ + { + id: `headless-terminals:${W}`, + activeTabId: 'agent-session:codex-1', + tabOrder: ['agent-session:codex-1'] + } + ], + tabs: [ + { + type: 'agent-session', + id: 'agent-session:codex-1', + title: 'Codex Chat', + sessionId: 'codex-1', + agent: 'codex', + isActive: true + } + ] + } +} + +function emptySnapshot( + publicationEpoch: string, + snapshotVersion: number +): RuntimeMobileSessionTabsResult { + return { + worktree: W, + publicationEpoch, + snapshotVersion, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabGroups: [], + tabs: [] + } +} + +function apply( + state: WebSessionTabsSyncState, + snapshot: RuntimeMobileSessionTabsResult +): WebSessionTabsSyncState { + return applyLocalStructuredSessionTabSnapshots(state, [snapshot]) +} + +function chatTabIds(state: WebSessionTabsSyncState): string[] { + return (state.unifiedTabsByWorktree[W] ?? []) + .filter((tab) => tab.contentType === 'agent-session') + .map((tab) => tab.entityId) +} + +describe('structured chat absorption after a host stored-version regression', () => { + it('DROPS the republished chat tab when the host version regressed under the same epoch', () => { + let state = emptyState() + + // 1. Chat is live. The host's per-worktree counter has been bumped many times by + // main-local touches (agent-status heartbeats, publishes) on top of the renderer's. + state = apply(state, chatSnapshot(RENDERER_EPOCH, 120)) + expect(chatTabIds(state)).toEqual(['codex-1']) + + // 2. User closes the chat tab; the worktree goes empty. + state = apply(state, emptySnapshot(RENDERER_EPOCH, 121)) + expect(chatTabIds(state)).toEqual([]) + + // 3. The host pruned its entry for the empty worktree and recreated it from the + // renderer's own global publication counter (orca-runtime-sync-mobile-session-tabs.ts:163-165 + // stores `nextSnapshot.snapshotVersion` verbatim when `existing` is undefined). + // `agentSession.reveal` then republishes on top of that, still under `renderer:`. + state = apply(state, chatSnapshot(RENDERER_EPOCH, 9)) + + // THE BUG: the tab never reaches the store, so activateStructuredAgentSessionById fails. + expect(chatTabIds(state)).toEqual([]) + }) + + it('CONTROL: the identical republish lands when its version clears the stale cursor', () => { + let state = emptyState() + state = apply(state, chatSnapshot(RENDERER_EPOCH, 120)) + state = apply(state, emptySnapshot(RENDERER_EPOCH, 121)) + + state = apply(state, chatSnapshot(RENDERER_EPOCH, 122)) + + // Same frame, same state, same host filter, same epoch history -> only the version differs. + expect(chatTabIds(state)).toEqual(['codex-1']) + }) + + it('CONTROL: a renderer reload (cleared module cursors) absorbs the low-version republish', () => { + let state = emptyState() + state = apply(state, chatSnapshot(RENDERER_EPOCH, 120)) + state = apply(state, emptySnapshot(RENDERER_EPOCH, 121)) + + resetLocalStructuredSessionVersionForTests() // what a renderer reload does + + state = apply(state, chatSnapshot(RENDERER_EPOCH, 9)) + expect(chatTabIds(state)).toEqual(['codex-1']) + }) + + it('the host removal frame is the only thing that clears the cursor, and it is not modelled', () => { + let state = emptyState() + state = apply(state, chatSnapshot(RENDERER_EPOCH, 120)) + state = apply(state, emptySnapshot(RENDERER_EPOCH, 121)) + + // orca-runtime-stored-mobile-snapshot-has-stale-preserved-tab.ts:132-142 + state = apply(state, { + ...emptySnapshot(`removed:${(1).toString(36)}`, 0), + removed: true + } as RuntimeMobileSessionTabsResult) + + state = apply(state, chatSnapshot(RENDERER_EPOCH, 9)) + expect(chatTabIds(state)).toEqual(['codex-1']) + }) +})