From 86f9fa032afcdacd282d59c2d5eab354dc9ed636 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:24:41 -0700 Subject: [PATCH 01/10] fix(runtime): void a handle-gap verdict the reconnect made stale The per-pane park bounds itself with one deadline per connection, but the waiter never recorded WHICH connection it was armed on. A wait armed on generation 0 that fires after a reconnect stamps its expiry against the current generation, so hasHostMirrorHandleWaitExpired agrees, the mirror lookup returns null, and the pane is resumed after 1ms on a connection that has had no chance to publish the handle. That is #19735's fork with an extra step, reached through the guard that exists to prevent it. The module's own doc comment claims the opposite -- "a reconnect bumps the connection generation and arms a fresh wait" -- and that is true only for a wait which had ALREADY expired, which is precisely the case the existing test covered. The test and the comment agreed with each other and both were wrong about the live case. The waiter now carries the generation it was armed on and records no verdict when the generation has moved; the replay re-parks through the existing machinery and the new connection gets its own full budget. Still bounded per connection generation, which is what was documented all along. Also pins the three sibling attacks on the same window: two panes in one environment where only one handle lands, a handle published by a foreign environment, and an environment tearing its rows down mid-park (which leaves no waiter and no scheduled timer). The test file now leads with how to assert on this module at all, because the obvious shape cannot fail. "Did the waiter release" is not an observable here -- a waiter released for the wrong reason is re-parked by the replayed sweep, so the store reads identically one tick later, and a mutation releasing every waiter on any tab's handle survived twelve assertions written that way. What a spurious release costs is the deadline, so the assertions advance the clock and require the pane to decide on the ORIGINAL schedule. --- .../lib/host-mirror-handle-gap-resume.test.ts | 141 +++++++++++++++++- .../src/lib/host-mirror-handle-gap-wait.ts | 17 ++- 2 files changed, 152 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts index dcebb3953c4..3d5e2625944 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -22,11 +22,21 @@ import { // separate frames, so there is a frame where the row exists and `ptyIdsByTabId` is still empty. // An empty handle map for a row the host is still publishing is `unverifiable`, never `exited` // (docs/reference/ssh-execution-boundary.md), so nothing may be resumed off it. +// +// HOW TO ASSERT ON THIS MODULE, because the obvious way cannot fail. "Did the waiter release" is +// NOT an observable here: a waiter released for the wrong reason is immediately re-parked by the +// replayed sweep, so the store, the record and the parked count all read identically one tick +// later. A mutation that released every waiter on any tab's handle survived twelve tests written +// that way. What a spurious release actually costs is the deadline — the re-park starts a fresh +// budget — so the assertion has to advance the clock: park, advance part of the budget, do the +// thing, then advance to the ORIGINAL deadline and require the pane to decide on schedule. const initialAppStoreState = useAppStore.getState() const LEAF_ID = '22222222-2222-4222-8222-222222222222' const WEB_TAB_ID = 'web-terminal-host-tab-1' +const SECOND_LEAF_ID = '33333333-3333-4333-8333-333333333333' +const SECOND_TAB_ID = 'web-terminal-host-tab-2' const RUNTIME_ENV_ID = 'env-handle-gap' function makeRuntimeOwnedWorktree(): ReturnType { @@ -74,17 +84,45 @@ function seedMirroredWorkspace(worktree: ReturnType { beforeEach(() => { vi.useFakeTimers() @@ -264,4 +306,95 @@ describe('resume across the mirror handle gap', () => { expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() }) + + // Why this is not the test above: there the wait had already expired before the reconnect, so + // the stale verdict was a map entry. Here the wait is still armed when the generation moves, and + // its deadline then fires on a connection that has had no chance at all to publish the handle. + it('does not let a wait armed on the previous connection decide the new one', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + // The host reconnects one millisecond before the wait's own deadline. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS - 1) + setRuntimeEnvironmentConnectionGenerationForTests(RUNTIME_ENV_ID, 1) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + vi.advanceTimersByTime(1) + + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(0) + // Re-armed, not held: the new connection gets its own budget and then decides. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) + }) + + it('releases only the pane whose handle landed when two panes share the environment', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedSecondMirroredPane(worktree.id) + const firstPaneKey = seedActiveSleepingRecordFor(worktree.id, WEB_TAB_ID, LEAF_ID, 'session-1') + const secondPaneKey = seedActiveSleepingRecordFor( + worktree.id, + SECOND_TAB_ID, + SECOND_LEAF_ID, + 'session-2' + ) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(2) + + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + useAppStore.setState({ ptyIdsByTabId: { [WEB_TAB_ID]: ['remote:env-handle-gap@@term_1'] } }) + + // The first pane owns its live PTY; the second is still undecided, not resumed. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + const after = useAppStore.getState() + expect(after.sleepingAgentSessionsByPaneKey[firstPaneKey]).toBeDefined() + expect(after.sleepingAgentSessionsByPaneKey[secondPaneKey]).toBeDefined() + expect(Object.keys(after.automaticAgentResumeClaimsByTabId)).toHaveLength(0) + + // Why the clock matters: releasing the second pane here and letting the replay re-park it + // would look identical right now and silently restart its budget. Its own deadline still has + // to land on the original schedule. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[secondPaneKey]).toBeUndefined() + }) + + it('does not release or reschedule a park because another environment published a handle', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + useAppStore.setState({ + ptyIdsByTabId: { 'web-terminal-other-env-tab': ['remote:env-other@@term_1'] } + }) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + // The unrelated handle must not have restarted this pane's budget. + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + }) + + it('leaves no waiter or timer behind when the environment tears its rows down mid-park', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + // Teardown drops every row the environment owned. + useAppStore.setState({ tabsByWorktree: {}, terminalLayoutsByTabId: {} } as never) + + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(0) + // Nothing may still be scheduled against the torn-down environment. + expect(vi.getTimerCount()).toBe(0) + }) }) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts index bc33f91689b..9582dcc9d6b 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -26,6 +26,8 @@ export const HOST_MIRROR_HANDLE_GAP_DEADLINE_MS = WEB_SESSION_TAB_RPC_TIMEOUT_MS type HandleGapWaiter = { worktreeId: string tabId: string + /** Connection generation the wait was armed on; its verdict is void on any other. */ + generation: number deadline: ReturnType run: () => void } @@ -141,11 +143,22 @@ export function parkUntilHostMirrorHandleLands( existing.run = run return } + const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) const deadline = setTimeout(() => { - recordExpiredWait(environmentId, key) + // Why the generation is re-read: a reconnect mid-park makes this wait's silence + // evidence about a connection that is gone. Recording it would let a wait armed + // milliseconds before the reconnect authorize a resume on the new one — the #19735 + // fork with an extra step. Release without a verdict instead; the replay re-parks + // and the new connection gets its own full budget. + if ( + waitersByPane.get(key)?.generation === + getRuntimeEnvironmentConnectionGeneration(environmentId) + ) { + recordExpiredWait(environmentId, key) + } releaseWaiter(key) }, HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) - waitersByPane.set(key, { worktreeId, tabId, deadline, run }) + waitersByPane.set(key, { worktreeId, tabId, generation, deadline, run }) startStoreSubscription() } From 3ec1bfc970e71aa4539b136cfc002337f1d1838d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:24:58 -0700 Subject: [PATCH 02/10] fix(terminal): a live pane owns its transcript in any workspace The resume dedup was scoped to the record's own workspace on both terms -- the entry's tab had to be in worktreeTabIds AND entry.worktreeId had to match -- while the completed-turn widening only relaxed the status term. A cross-workspace record whose peer pane is `done` and still holds a live PTY therefore matched nothing, and the sweep launched a second agent onto a transcript the peer is still writing. The two ids really do drift. canonicalizeTerminalSessionWorktreeId re-keys tabsByWorktree, tabGroups, tabGroupLayouts, activeTabIdByWorktree and activeGroupIdByWorktree onto the canonical worktree id, and does NOT re-key sleepingAgentSessionsByPaneKey, whose records carry worktreeId inside them. So adopting an orphaned terminal is a direct producer of a record naming one workspace while its pane and status row name another. Split into two arms rather than widening the existing condition. The new arm carries no workspace scope but demands hard evidence: a provider session id names one transcript, so a pane whose exact PTY is live right now already owns it wherever that pane sits, and no workspace boundary makes a live PTY less live. The scoped arm keeps its scope and its state !== 'done' term, because a status row with no live PTY is a claim about the past and must not reach across workspaces. Pins both directions: the live peer is not forked, and the same peer without a live PTY still resumes. --- ...eping-agent-session-provider-claim.test.ts | 71 +++++++++++++++++++ .../src/lib/resume-sleeping-agent-session.ts | 27 ++++--- 2 files changed, 90 insertions(+), 8 deletions(-) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts b/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts index 7a79e239eb0..1339403c118 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session-provider-claim.test.ts @@ -141,4 +141,75 @@ describe('resume sleeping agent provider claims', () => { expect(state.tabsByWorktree['wt-1']).toHaveLength(1) expect(state.sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() }) + + // Why a peer in another workspace is reachable at all: adopting an orphaned terminal re-keys + // `tabsByWorktree` onto the canonical worktree id and leaves the sleeping records that named the + // old one untouched (workspace-session-worktree-id.ts). A provider session id names one + // transcript, so the live pane owns it wherever it sits; resuming here forks the agent the user + // is watching. `done` is the cell that had no cover: a finished turn on a still-live pane. + it('does not fork a provider session a live pane in another workspace is running', () => { + const paneKey = makePaneKey('tab-1', LEAF_ID) + const peerPaneKey = makePaneKey('tab-peer', OTHER_LEAF_ID) + const record = makeRecord(paneKey) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + // The record's own pane is gone, so nothing local can own its recovery. + tabsByWorktree: { 'wt-1': [], 'wt-2': [makeTerminalTab('tab-peer')] }, + terminalLayoutsByTabId: { + 'tab-peer': { + root: { type: 'leaf', leafId: OTHER_LEAF_ID }, + activeLeafId: OTHER_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF_ID]: 'pty-peer' } + } + }, + ptyIdsByTabId: { 'tab-peer': ['pty-peer'] }, + sleepingAgentSessionsByPaneKey: { [paneKey]: record }, + agentStatusByPaneKey: { + [peerPaneKey]: { + ...makeWorkingStatus(peerPaneKey, 'tab-peer', record), + worktreeId: 'wt-2', + state: 'done' + } + } + } as never) + + expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(0) + + const state = useAppStore.getState() + expect(state.tabsByWorktree['wt-1']).toHaveLength(0) + expect(state.sleepingAgentSessionsByPaneKey[record.paneKey]).toBeUndefined() + }) + + // The same peer without a live PTY is history, not a claim: the session must still come back. + it('still resumes when the other workspace peer finished and holds no live PTY', () => { + const paneKey = makePaneKey('tab-1', LEAF_ID) + const peerPaneKey = makePaneKey('tab-peer', OTHER_LEAF_ID) + const record = makeRecord(paneKey) + useAppStore.setState({ + activeWorktreeId: 'wt-1', + activeTabType: 'terminal', + tabsByWorktree: { 'wt-1': [], 'wt-2': [makeTerminalTab('tab-peer')] }, + terminalLayoutsByTabId: { + 'tab-peer': { + root: { type: 'leaf', leafId: OTHER_LEAF_ID }, + activeLeafId: OTHER_LEAF_ID, + expandedLeafId: null, + ptyIdsByLeafId: { [OTHER_LEAF_ID]: 'pty-peer' } + } + }, + ptyIdsByTabId: {}, + sleepingAgentSessionsByPaneKey: { [paneKey]: record }, + agentStatusByPaneKey: { + [peerPaneKey]: { + ...makeWorkingStatus(peerPaneKey, 'tab-peer', record), + worktreeId: 'wt-2', + state: 'done' + } + } + } as never) + + expect(resumeSleepingAgentSessionsForWorktree('wt-1')).toBe(1) + }) }) diff --git a/src/renderer/src/lib/resume-sleeping-agent-session.ts b/src/renderer/src/lib/resume-sleeping-agent-session.ts index cb2a0311873..9b9279b4f0b 100644 --- a/src/renderer/src/lib/resume-sleeping-agent-session.ts +++ b/src/renderer/src/lib/resume-sleeping-agent-session.ts @@ -104,9 +104,20 @@ function activeOrQueuedResumeClaimsProviderSession( } const tabId = getAgentStatusTabId(entry) const pane = parsePaneKey(entry.paneKey) - // A completed turn still owns its transcript while its exact PTY is live. - const completedPaneIsLive = Boolean( - entry.state === 'done' && + if ( + entry.agentType !== record.agent || + !agentProviderSessionsEqual(record.agent, entry.providerSession, record.providerSession) + ) { + continue + } + // Why this arm carries no workspace scope: a provider session id names one transcript, so a + // pane whose exact PTY is live right now already owns it wherever that pane happens to sit, and + // resuming forks the agent the user is watching. The scoped arm below still needs its scope — + // a status row with no live PTY is a claim about the past. The two ids do drift: adopting an + // orphaned terminal re-keys `tabsByWorktree` without re-keying the sleeping records that name + // the old id (workspace-session-worktree-id.ts), and a completed turn on a live pane is exactly + // where the drift stops being caught. + if ( pane && tabId === pane.tabId && stablePaneHasLivePty( @@ -115,13 +126,13 @@ function activeOrQueuedResumeClaimsProviderSession( state.ptyIdsByTabId, state.terminalLayoutsByTabId[pane.tabId] ) - ) + ) { + return true + } if ( + entry.state !== 'done' && worktreeTabIds.has(tabId ?? '') && - entry.worktreeId === record.worktreeId && - entry.agentType === record.agent && - (entry.state !== 'done' || completedPaneIsLive) && - agentProviderSessionsEqual(record.agent, entry.providerSession, record.providerSession) + entry.worktreeId === record.worktreeId ) { return true } From 99a9c01a920f9db4a3110925580e32a2827cef10 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:25:10 -0700 Subject: [PATCH 03/10] fix(session): an emptied workspace still survives the reconnect merge localActiveWorkspaceSurvives decided whether the workspace the user is standing in still exists by counting its tabs. A workspace they had just closed the last terminal in reads as zero, so it "did not survive", the host's null activeWorktreeId was taken literally, and the reconnect dropped them onto the home screen -- for closing a tab. An explicit empty tabsByWorktree row is the record that the user closed the last terminal, not the absence of a workspace (initial-terminal.ts). hasLocalTabsRow two hundred lines up in this same function already draws that distinction with Object.hasOwn; this line was simply missed. The counterweight is pinned too: presence must not turn into "never follow the host", so a host that does name an active worktree still wins over the emptied local one. --- ...space-session-merge-local-survival.test.ts | 33 +++++++++++++++++++ .../hooks/remote-workspace-session-merge.ts | 6 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts b/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts index 361493b34ef..95b4004fa72 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge-local-survival.test.ts @@ -353,6 +353,39 @@ describe('local rows the snapshot carries no answer for', () => { expect(merged.tabsByWorktree[WORKTREE]).toEqual([]) }) + it('keeps the user standing in the workspace they emptied', () => { + // Same row, one level up. `localActiveWorkspaceSurvives` counted tabs, so the workspace the + // user had just closed the last terminal in read as "did not survive the merge" and the host's + // null active worktree was taken literally — the home screen, for closing a tab. + const current = sessionState({ tabsByWorktree: { [WORKTREE]: [] } }) + const remote = sessionState({ + activeWorktreeId: null, + activeWorkspaceKey: null, + activeRepoId: null, + tabsByWorktree: { [WORKTREE]: [] } + }) + + const merged = merge(current, remote, { [WORKTREE]: [] }) + + expect(merged.activeWorktreeId).toBe(WORKTREE) + expect(merged.activeWorkspaceKey).toBe(worktreeWorkspaceKey(WORKTREE)) + expect(merged.activeRepoId).toBe('repo-1') + }) + + it('still lets the host move the user off a workspace it does name one for', () => { + // The counterweight: presence must not turn into "never follow the host". A host that names an + // active worktree still wins over the emptied local one. + const current = sessionState({ tabsByWorktree: { [WORKTREE]: [] } }) + const remote = sessionState({ + activeWorktreeId: OTHER_WORKTREE, + tabsByWorktree: { [WORKTREE]: [] } + }) + + const merged = merge(current, remote, { [WORKTREE]: [] }) + + expect(merged.activeWorktreeId).toBe(OTHER_WORKTREE) + }) + it('invents no row for a worktree neither side has one for', () => { // The counterweight: presence has to come from a real local row, not from membership in the // replace set, or a never-initialized workspace gets a tombstone it never earned. diff --git a/src/renderer/src/hooks/remote-workspace-session-merge.ts b/src/renderer/src/hooks/remote-workspace-session-merge.ts index fa247d8bbbd..71b4e7d1884 100644 --- a/src/renderer/src/hooks/remote-workspace-session-merge.ts +++ b/src/renderer/src/hooks/remote-workspace-session-merge.ts @@ -258,10 +258,14 @@ export function mergeDirectSshRemoteWorkspaceSession( // the workspace or its path did not resolve to a local id, and taking that literally drops the // user onto the home screen while their terminals keep running. So it is only overridden when the // workspace they are standing in demonstrably still exists in the merged result. + // Why presence and not length, the same reading `hasLocalTabsRow` above already gives: an + // explicit empty row is the record that the user closed the last terminal, and a workspace they + // emptied still exists in the merged result. Counting rows sent them to the home screen for + // having closed their last tab. const localActiveWorkspaceSurvives = current.activeWorktreeId != null && replaceWorktreeIds.has(current.activeWorktreeId) && - (tabsByWorktree[current.activeWorktreeId]?.length ?? 0) > 0 + Object.hasOwn(tabsByWorktree, current.activeWorktreeId) const preservedActiveWorktreeId = localActiveWorkspaceSurvives ? current.activeWorktreeId : null // The three active-* fields have to describe ONE workspace, so they are all derived from whichever // worktree wins rather than each choosing a source. Taking the repo from the host while the From 8a061c468aa1d79b434104ab76cee2c24e59ae52 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:37:44 -0700 Subject: [PATCH 04/10] fix(relay): stop an unreadable session file reporting as a sign-out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coordinator's own comment states the contract: readContext "throws on transient failures and returns null solely when the cloud session is gone (absent, or cleared by a 401)". The implementation did not honour it. readFreshOrcaCloudSession collapsed readOrcaCloudSession's `unreadable` status into `reconnect-required`, so readRelayAuthContext returned null and the coordinator published RELAY_HOST_CLOSE_REASON.SIGNED_OUT, closed the broker with that wire reason, and armed no retry because a terminal cause arms none. `unreadable` fires on EPERM/EACCES/EBUSY/EMFILE/ENFILE/EIO — descriptor exhaustion on a busy machine, a Windows AV file lock. The phone latches setHostSignedOut and shows "Desktop signed out — sign in to Orca on your desktop to reconnect" for a failure the next read would have cleared. The contrast is the argument: `unreadable` is the one status the session store's own doc says "licenses nothing", and clearCloudSessionIfUnchanged already refuses to delete a session because of it — "a session we were denied is not a session we may delete". The relay taxonomy was the single place treating it as evidence the user signed out. Give it its own arm on FreshCloudSessionResult and throw for it in readRelayAuthContext, which lands it on the retryable auth_unavailable path the coordinator already has. Every other caller tests `status !== 'found'`, so their behaviour is unchanged. Correct the coordinator comment to say what it actually depends on: null-means-gone is a contract readRelayAuthContext owes it, not something that branch can verify. Measured before the fix: offlineReason "signed-out", mint code relay_signed_out. After: auth_unavailable. Not fixed here, and worth a separate look: `decrypt-failed` when safeStorage.isEncryptionAvailable() is false (a Linux keyring still locked at login) takes the same route to SIGNED_OUT, but reclassifying it changes sign-in semantics well beyond the relay. --- .../profile-cloud-session-refresh.ts | 9 +++ ...ay-auth-context-unreadable-session.test.ts | 65 +++++++++++++++++++ src/main/runtime/relay/relay-auth-context.ts | 6 ++ .../runtime/relay/relay-auth-coordinator.ts | 9 ++- 4 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts diff --git a/src/main/orca-profiles/profile-cloud-session-refresh.ts b/src/main/orca-profiles/profile-cloud-session-refresh.ts index 273f71f016e..c5cea26e964 100644 --- a/src/main/orca-profiles/profile-cloud-session-refresh.ts +++ b/src/main/orca-profiles/profile-cloud-session-refresh.ts @@ -32,6 +32,12 @@ const CLOUD_SESSION_REFRESH_SKEW_MS = 60_000 export type FreshCloudSessionResult = | { status: 'found'; session: OrcaCloudSession } | { status: 'reconnect-required' } + /** + * The session file is there and this process could not read it (EACCES/EBUSY/EMFILE/…). Distinct + * from `reconnect-required`, which means the session is genuinely gone: the store refuses to + * delete an unreadable session for the same reason a caller must not report one as a sign-out. + */ + | { status: 'unreadable' } export type CloudSessionOperationResult = | { status: 'ok'; value: T } @@ -226,6 +232,9 @@ export async function readFreshOrcaCloudSession( userDataPath: string ): Promise { const session = readOrcaCloudSession(active.profile.id, userDataPath) + if (session.status === 'unreadable') { + return { status: 'unreadable' } + } if (session.status !== 'found') { return { status: 'reconnect-required' } } diff --git a/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts b/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts new file mode 100644 index 00000000000..6b2f514f816 --- /dev/null +++ b/src/main/runtime/relay/relay-auth-context-unreadable-session.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it, vi } from 'vitest' +import { RELAY_HOST_CLOSE_REASON } from '../../../shared/relay-host-close-reason' + +const fakes = vi.hoisted(() => ({ + ensureActiveOrcaProfile: vi.fn(), + readFreshOrcaCloudSession: vi.fn() +})) + +vi.mock('../../orca-profiles/profile-index-store', () => ({ + ensureActiveOrcaProfile: fakes.ensureActiveOrcaProfile +})) +vi.mock('../../orca-profiles/profile-cloud-session-refresh', () => ({ + readFreshOrcaCloudSession: fakes.readFreshOrcaCloudSession +})) + +import { readRelayAuthContext } from './relay-auth-context' +import { RelayAuthCoordinator } from './relay-auth-coordinator' + +const profile = { + profile: { + id: 'profile-1', + cloud: { userId: 'u1', cloudProfileId: 'cp1', activeOrgId: 'org-1' } + } +} + +const authConfig = {} as never + +describe('readRelayAuthContext session-read taxonomy', () => { + it('refuses to call an unreadable session file a sign-out', async () => { + // EACCES/EBUSY/EMFILE on the session file means "present, could not read it" — the store + // itself declines to delete one for that reason. Reporting it as signed-out tells every + // paired phone to sign in on the desktop for a failure a retry would have cleared. + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'unreadable' }) + + await expect(readRelayAuthContext(authConfig, '/tmp/x')).rejects.toThrow( + 'orca_cloud_session_unreadable' + ) + }) + + it('still reports a genuinely absent session as gone', async () => { + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'reconnect-required' }) + + await expect(readRelayAuthContext(authConfig, '/tmp/x')).resolves.toBeNull() + }) + + it('classifies an unreadable session as auth_unavailable, never signed_out', async () => { + fakes.ensureActiveOrcaProfile.mockReturnValue(profile) + fakes.readFreshOrcaCloudSession.mockResolvedValue({ status: 'unreadable' }) + const broker = { closeNow: vi.fn() } + const coordinator = new RelayAuthCoordinator({ + readContext: () => readRelayAuthContext(authConfig, '/tmp/x'), + openBroker: async () => broker, + onStatus: vi.fn() + }) + coordinator.reconcile() + + const result = await coordinator.waitForLiveBrokerResult(0) + expect(result).toEqual({ broker: null, offlineReason: 'auth_unavailable' }) + expect(result.broker).toBeNull() + // The wire close reason is what the phone latches on; it must not be spent here. + expect(broker.closeNow).not.toHaveBeenCalledWith(RELAY_HOST_CLOSE_REASON.SIGNED_OUT) + }) +}) diff --git a/src/main/runtime/relay/relay-auth-context.ts b/src/main/runtime/relay/relay-auth-context.ts index 117f3f78057..53f156e8915 100644 --- a/src/main/runtime/relay/relay-auth-context.ts +++ b/src/main/runtime/relay/relay-auth-context.ts @@ -12,6 +12,12 @@ export async function readRelayAuthContext( return null } const session = await readFreshOrcaCloudSession(authConfig, active, userDataPath) + // Why throw rather than return null: the coordinator reads null as "the cloud session is gone" + // and closes every paired phone's relay with signed-out, arming no retry. A session file this + // process could not read is intact, so the retryable auth_unavailable path is the honest word. + if (session.status === 'unreadable') { + throw new Error('orca_cloud_session_unreadable') + } if (session.status !== 'found') { return null } diff --git a/src/main/runtime/relay/relay-auth-coordinator.ts b/src/main/runtime/relay/relay-auth-coordinator.ts index a284618ac72..80cff8f878e 100644 --- a/src/main/runtime/relay/relay-auth-coordinator.ts +++ b/src/main/runtime/relay/relay-auth-coordinator.ts @@ -189,9 +189,12 @@ export class RelayAuthCoordinator { if (!context || !context.relayEntitled) { this.cancelLinger() this.retry.reset() - // Why only the null case: readContext throws on transient failures and - // returns null solely when the cloud session is gone (absent, or cleared - // by a 401). A present-but-unentitled context is still a signed-in + // Why only the null case: null must mean the cloud session is gone (absent, or cleared + // by a 401), and every other read outcome must throw so it lands in auth_unavailable. + // That is a contract readRelayAuthContext owes this branch, not something this branch + // can verify — it held for a refresh failure but not for a session file the process + // could not read, which spent SIGNED_OUT on transient I/O until relay-auth-context.ts + // started throwing for it. A present-but-unentitled context is still a signed-in // desktop, and "sign in to reconnect" would be wrong advice for it. this.invalidateOwnership(context ? undefined : RELAY_HOST_CLOSE_REASON.SIGNED_OUT) this.publish('offline', context ? 'not_entitled' : RELAY_HOST_CLOSE_REASON.SIGNED_OUT) From fe8c41ab786aab8f830a6ffa0c785c317935cdfd Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:38:00 -0700 Subject: [PATCH 05/10] fix(relay): name the LAN flip that lands mid-mint, not relay_control_not_active MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit withTransientDemand checks the host pairing policy once, at entry. But RelayDemandLedger.hasDemand filters the in-flight operation's OWN transient ref through the live policy, so a flip to local-only while createPairingRelay or provisionRelay is awaiting withdraws the demand that operation is holding. The coordinator then reaches its no-demand branch and publishes 'standby', publish() clears offlineReason to null for any non-offline status, and the broker wait returns with no cause at all — relay_control_not_active, the generic code this stack exists to remove. The flip is exactly the cause and relay_disabled_for_device is already its word; the entry gate just could not see a flip that had not happened yet. Re-ask the policy on the failure path rather than trusting the thrown code. Reproduced first: the added test failed with "expected ... to throw error including 'relay_disabled_for_device' but got 'relay_control_not_active'". Controls cover the two ways this could over-reach — a failure the policy had nothing to do with is not rewritten, and a grant that succeeded under a flip is left alone. Top follow-up found alongside this and deliberately not fixed here: flushRevoke swallows every error with a bare catch, no attempt cap and no item expiry, while the revoke check in hasDemand is deliberately not filtered through the policy. A revoke that fails permanently server-side therefore holds demand open forever and defeats the LAN pick entirely — the same symptom this change is part of closing, by a different route, with no test coverage. --- ...op-relay-service-policy-flip-cause.test.ts | 89 +++++++++++++++++++ .../runtime/relay/desktop-relay-service.ts | 9 ++ 2 files changed, 98 insertions(+) create mode 100644 src/main/runtime/relay/desktop-relay-service-policy-flip-cause.test.ts diff --git a/src/main/runtime/relay/desktop-relay-service-policy-flip-cause.test.ts b/src/main/runtime/relay/desktop-relay-service-policy-flip-cause.test.ts new file mode 100644 index 00000000000..ae15f70b4d4 --- /dev/null +++ b/src/main/runtime/relay/desktop-relay-service-policy-flip-cause.test.ts @@ -0,0 +1,89 @@ +import { describe, expect, it, vi } from 'vitest' +import { DesktopRelayService } from './desktop-relay-service' +import type { MobilePairingConnectionMode } from '../../../shared/mobile-pairing-connection-mode' + +/** + * A host LAN flip lands mid-mint. + * + * `hasDemand` filters the in-flight operation's OWN transient ref through the live policy + * (relay-demand-ledger.ts:50-54), so the flip withdraws the demand that operation is holding. + * The coordinator then publishes `standby`, which clears `offlineReason` to null, and the broker + * wait returns with no cause — `relay_control_not_active`, the generic code this stack exists to + * remove. The flip is the cause and `relay_disabled_for_device` is already its word. + */ +type TransientDemandHost = { + withTransientDemand: ( + kind: string, + deviceId: string, + operation: () => Promise + ) => Promise +} + +function serviceWithMode(mode: { current: MobilePairingConnectionMode }): { + run: (operation: () => Promise) => Promise + release: ReturnType +} { + const release = vi.fn() + const service = Object.create(DesktopRelayService.prototype) as DesktopRelayService + Object.assign(service, { + hostMobilePairingConnectionMode: () => mode.current, + runtimeRpc: { + getDeviceRegistry: () => ({ getMobilePairingConnectionMode: () => 'automatic' }) + }, + demandLedger: { acquireTransient: () => release }, + refreshDemand: () => {} + }) + const host = service as unknown as TransientDemandHost + return { + run: (operation) => host.withTransientDemand.call(service, 'pairing', 'device-1', operation), + release + } +} + +describe('DesktopRelayService policy flip during an in-flight grant', () => { + it('names the LAN flip rather than the generic control-not-active code', async () => { + const mode = { current: 'automatic' as MobilePairingConnectionMode } + const { run, release } = serviceWithMode(mode) + + await expect( + run(async () => { + mode.current = 'local-only' + // What requireActiveBroker throws once the flip cleared the offline reason. + throw new Error('relay_control_not_active') + }) + ).rejects.toThrow('relay_disabled_for_device') + expect(release).toHaveBeenCalled() + }) + + it('does not rewrite a failure the policy had nothing to do with', async () => { + const mode = { current: 'automatic' as MobilePairingConnectionMode } + const { run } = serviceWithMode(mode) + + await expect( + run(async () => { + throw new Error('relay_broker_unavailable') + }) + ).rejects.toThrow('relay_broker_unavailable') + }) + + it('still refuses at the entry gate when the policy already excludes the device', async () => { + const mode = { current: 'local-only' as MobilePairingConnectionMode } + const { run } = serviceWithMode(mode) + const operation = vi.fn(async () => 'unreachable') + + await expect(run(operation)).rejects.toThrow('relay_disabled_for_device') + expect(operation).not.toHaveBeenCalled() + }) + + it('leaves a successful grant alone even if the policy flips under it', async () => { + const mode = { current: 'automatic' as MobilePairingConnectionMode } + const { run } = serviceWithMode(mode) + + await expect( + run(async () => { + mode.current = 'local-only' + return 'minted' + }) + ).resolves.toBe('minted') + }) +}) diff --git a/src/main/runtime/relay/desktop-relay-service.ts b/src/main/runtime/relay/desktop-relay-service.ts index be9b22bb0c4..f108fda2968 100644 --- a/src/main/runtime/relay/desktop-relay-service.ts +++ b/src/main/runtime/relay/desktop-relay-service.ts @@ -295,6 +295,15 @@ export class DesktopRelayService { this.refreshDemand() try { return await operation() + } catch (error) { + // Why re-ask instead of trusting the thrown code: a flip lands mid-operation, and + // `hasDemand` filters this ref's own demand through the live policy, so the coordinator + // reaches `standby` and clears the offline reason. The wait then ends with no cause at all + // — the generic `relay_control_not_active` — when the flip is exactly the cause. + if (!this.isRelayAllowedForDevice(deviceId)) { + throw new Error('relay_disabled_for_device') + } + throw error } finally { release() this.refreshDemand() From 9a81146e1301fa165582961610edab104a7ee0ed Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 16:38:13 -0700 Subject: [PATCH 06/10] test(runtime): pin sleeping-agent resume on a failed SSH target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal-state floor in workspace-terminal-host-authority.ts has three consumers: initial-terminal seeding, the startup terminal watcher, and sleeping-agent resume. Seeding is covered end to end by worktree-agent-activation-seam.test.ts. Resume was covered only at the predicate, so nothing failed if the floor stopped reaching it — and the floor's own comment says the cost of losing it is a failed target left terminal-less with unresumable agents for the rest of the app session. Pins the resume half directly: an SSH git worktree on a target whose sync terminated in offline/error with an empty hydrated set resumes its sleeping agent. Two controls keep the floor from widening into "resume whenever we are unsure" — an in-flight 'pulling' sync and no sync status at all both stay unverifiable and resume nothing. Verified by mutation: emptying TERMINATED_WITHOUT_ANSWER_PHASES fails exactly the two floor assertions and leaves both controls passing. Routes independently of the two fixes on this branch: the floor predates this stack (#16750), and this only closes a coverage gap in it. --- ...ailed-target-sleeping-agent-resume.test.ts | 115 ++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts diff --git a/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts b/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts new file mode 100644 index 00000000000..a94c0438231 --- /dev/null +++ b/src/renderer/src/lib/ssh-failed-target-sleeping-agent-resume.test.ts @@ -0,0 +1,115 @@ +/** + * The resume half of the terminal-state floor. + * + * `workspace-terminal-host-authority.ts` says an SSH target whose sync terminated in + * `offline`/`error` without ever hydrating answers `none`, so this client may act. The seeding + * consumer is covered end to end (worktree-agent-activation-seam.test.ts); the sleeping-agent + * consumer (resume-sleeping-agent-session.ts) was only covered at the predicate. Without this, + * a failed target's agents stay unresumable for the rest of the app session and nothing fails. + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../shared/agent-session-resume' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import { useAppStore } from '@/store' +import { makeWorktree } from '@/store/slices/store-test-helpers' +import { resolveWorkspaceTerminalHostAuthority } from './workspace-terminal-host-authority' +import { resumeSleepingAgentSessionsForWorktree } from './resume-sleeping-agent-session' + +vi.mock('sonner', () => ({ toast: { info: vi.fn(), success: vi.fn(), error: vi.fn() } })) + +const initialAppStoreState = useAppStore.getState() +const TARGET_ID = 'ssh-target-1' +const WORKTREE_ID = 'repoSsh::/srv/proj/feature' + +afterEach(() => { + useAppStore.setState(initialAppStoreState, true) +}) + +function seedFailedSshTarget(phase?: 'offline' | 'error' | 'pulling'): void { + const tab: TerminalTab = { + id: 'tab-1', + ptyId: null, + worktreeId: WORKTREE_ID, + title: 'shell', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + const record: SleepingAgentSessionRecord = { + paneKey: 'tab-1:leaf-1', + tabId: 'tab-1', + worktreeId: WORKTREE_ID, + agent: 'pi', + providerSession: { key: 'session_id', id: 'pi-session-1', transcriptPath: '/tmp/pi-1.jsonl' }, + prompt: '', + state: 'working', + capturedAt: 1, + updatedAt: 1, + origin: 'worktree-sleep' + } + useAppStore.setState({ + repos: [ + { + id: 'repoSsh', + path: '/srv/proj', + displayName: 'repoSsh', + badgeColor: '#000', + addedAt: 0, + connectionId: TARGET_ID + } + ] as never, + worktreesByRepo: { + repoSsh: [ + makeWorktree({ + id: WORKTREE_ID, + repoId: 'repoSsh', + path: '/srv/proj/feature', + hostId: `ssh:${TARGET_ID}` + } as never) + ] + }, + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: + phase === undefined ? {} : { [TARGET_ID]: { phase, direction: 'pull' as const } }, + tabsByWorktree: { [WORKTREE_ID]: [tab] }, + sleepingAgentSessionsByPaneKey: { [record.paneKey]: record } + }) +} + +describe('sleeping-agent resume on a failed SSH target', () => { + it.each(['offline', 'error'] as const)( + 'resumes a sleeping agent once a sync terminates in %s without ever hydrating', + (phase) => { + seedFailedSshTarget(phase) + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'none' + ) + // The gate this exists for: a target that failed must not stay unresumable for the session. + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(1) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBeUndefined() + } + ) + + it('still declines to resume while the host has not answered', () => { + // Control: an in-flight sync is `unverifiable`, and resuming there forks a session the host + // may still be running. The floor must not widen into "resume whenever we are unsure". + seedFailedSshTarget('pulling') + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(0) + expect(useAppStore.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBeDefined() + }) + + it('still declines to resume when no sync status exists at all', () => { + seedFailedSshTarget(undefined) + + expect(resolveWorkspaceTerminalHostAuthority(useAppStore.getState(), WORKTREE_ID)).toBe( + 'unverifiable' + ) + expect(resumeSleepingAgentSessionsForWorktree(WORKTREE_ID)).toBe(0) + }) +}) From 2da662424ba74acdde5d4e37e1131dc655571f4d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:47:58 -0700 Subject: [PATCH 07/10] fix(runtime): an outage is not a handle-gap verdict MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-pane handle-gap wait releases at a 15s deadline and records that expiry as a verdict, which authorises the sleeping-agent resume. The connection generation was the only thing voiding that verdict, and a plain disconnect never advances it — runtime-status.ts advances on the reconnect, under a new runtime id. So a network drop mid-turn expired the wait with a generation that still matched, and the replay forked a second `--resume` onto the transcript the host was still writing: #19735 through the disconnect door. Suppress the verdict while the client positively knows it is out of contact, reusing the shared runtime-host connection derivation. The waiter still releases and re-parks, so contact returning gets a full fresh budget and the pane is still decided on real silence. --- .../lib/host-mirror-handle-gap-resume.test.ts | 51 +++++++++++++++++++ .../src/lib/host-mirror-handle-gap-wait.ts | 34 ++++++++++++- 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts index 3d5e2625944..5d77a679caf 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-resume.test.ts @@ -140,6 +140,22 @@ function seedActiveSleepingRecord(worktreeId: string): string { return seedActiveSleepingRecordFor(worktreeId, WEB_TAB_ID, LEAF_ID, 'handle-gap-session') } +/** A recorded status entry whose runtime answered nothing: the shape a dropped link leaves behind. */ +function setRuntimeEnvironmentDisconnectedForTests(environmentId: string): void { + useAppStore.setState({ + runtimeStatusByEnvironmentId: new Map(useAppStore.getState().runtimeStatusByEnvironmentId).set( + environmentId, + { status: null } as never + ) + } as never) +} + +function clearRuntimeEnvironmentStatusEntryForTests(environmentId: string): void { + const next = new Map(useAppStore.getState().runtimeStatusByEnvironmentId) + next.delete(environmentId) + useAppStore.setState({ runtimeStatusByEnvironmentId: next } as never) +} + describe('resume across the mirror handle gap', () => { beforeEach(() => { vi.useFakeTimers() @@ -332,6 +348,41 @@ describe('resume across the mirror handle gap', () => { expect(Object.keys(useAppStore.getState().automaticAgentResumeClaimsByTabId)).toHaveLength(1) }) + // The journey: the network drops mid-turn on a paired runtime. Nothing is unpaired and no + // reconnect has happened, so the connection generation has not moved — runtime-status.ts + // advances it on the *reconnect*, under a new runtime id. The deadline therefore fires with a + // generation that still matches, and its silence is about the outage, not about the host. A + // verdict recorded there resumes the agent the host is still running (#19735 through the + // disconnect door, docs/reference/ssh-execution-boundary.md). + it('does not turn an outage into a verdict when the environment dropped mid-park', () => { + const worktree = makeRuntimeOwnedWorktree() + seedMirroredWorkspace(worktree) + const paneKey = seedActiveSleepingRecord(worktree.id) + markHostSessionMirrorHydrated(RUNTIME_ENV_ID) + expect(resumeSleepingAgentSessionsForWorktree(worktree.id)).toBe(0) + + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS / 2) + setRuntimeEnvironmentDisconnectedForTests(RUNTIME_ENV_ID) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + const during = useAppStore.getState() + expect(during.sleepingAgentSessionsByPaneKey[paneKey]).toBeDefined() + expect(Object.keys(during.automaticAgentResumeClaimsByTabId)).toHaveLength(0) + expect((during.tabsByWorktree[worktree.id] ?? []).map((tab) => tab.id)).toEqual([WEB_TAB_ID]) + // Held, not abandoned: something is still armed to decide once contact returns. + expect(countParkedHostMirrorHandleGapPanesForTests()).toBe(1) + + // Contact returns and the host still publishes no handle for the pane. That silence IS + // evidence, so the next full budget decides — a hold that outlives the outage would be the + // latch-that-never-releases defect this module exists to avoid. + clearRuntimeEnvironmentStatusEntryForTests(RUNTIME_ENV_ID) + vi.advanceTimersByTime(HOST_MIRROR_HANDLE_GAP_DEADLINE_MS) + + const after = useAppStore.getState() + expect(after.sleepingAgentSessionsByPaneKey[paneKey]).toBeUndefined() + expect(Object.keys(after.automaticAgentResumeClaimsByTabId)).toHaveLength(1) + }) + it('releases only the pane whose handle landed when two panes share the environment', () => { const worktree = makeRuntimeOwnedWorktree() seedMirroredWorkspace(worktree) diff --git a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts index 9582dcc9d6b..278d5f5fddc 100644 --- a/src/renderer/src/lib/host-mirror-handle-gap-wait.ts +++ b/src/renderer/src/lib/host-mirror-handle-gap-wait.ts @@ -1,6 +1,10 @@ import { useAppStore } from '@/store' import { getRuntimeEnvironmentConnectionGeneration } from '@/store/slices/runtime-status' import { WEB_SESSION_TAB_RPC_TIMEOUT_MS } from '@/runtime/web-session-tab-rpc-timeout' +import { + isDisconnectedRuntimeHostState, + runtimeHostConnectionStateForEntry +} from '@/runtime/runtime-host-connection-state' /** * Per-pane park for the frame between a host's tab rows and its PTY handles. @@ -54,6 +58,27 @@ export function hasHostMirrorHandleWaitExpired(environmentId: string, tabId: str ) } +/** + * True only when the client positively knows it is out of contact — the link dropped, or its + * replacement is still being established. + * + * Why not `isConnectedRuntimeHostState`: that reads a host nobody has probed yet as not + * connected, and a never-probed host is not the outage this guards. Narrowing to the two states + * an outage actually produces keeps the guard to the case where silence provably means "we could + * not ask" rather than "the host had nothing to say". + * + * Why this and not the connection generation: a plain disconnect leaves the generation where it + * was — runtime-status.ts advances it on the *reconnect*, under a new runtime id — so a wait that + * expires mid-outage is indistinguishable, to the generation guard, from one that expired on a + * healthy connection. + */ +function environmentContactIsLost(environmentId: string): boolean { + const connectionState = runtimeHostConnectionStateForEntry( + useAppStore.getState().runtimeStatusByEnvironmentId.get(environmentId) + ) + return isDisconnectedRuntimeHostState(connectionState) || connectionState === 'reconnecting' +} + function recordExpiredWait(environmentId: string, key: string): void { const generation = getRuntimeEnvironmentConnectionGeneration(environmentId) // Why: a verdict from a previous connection is dead weight; drop it so the map @@ -150,9 +175,16 @@ export function parkUntilHostMirrorHandleLands( // milliseconds before the reconnect authorize a resume on the new one — the #19735 // fork with an extra step. Release without a verdict instead; the replay re-parks // and the new connection gets its own full budget. + // + // Why contact is checked too: an environment that dropped mid-park publishes nothing, + // so the deadline measures the outage rather than the host. Loss of contact is never + // evidence about a process (docs/reference/ssh-execution-boundary.md), and a verdict + // recorded here authorizes the resume that forks the agent the host is still running. + // The generation cannot stand in for it — a plain disconnect never advances it. if ( + !environmentContactIsLost(environmentId) && waitersByPane.get(key)?.generation === - getRuntimeEnvironmentConnectionGeneration(environmentId) + getRuntimeEnvironmentConnectionGeneration(environmentId) ) { recordExpiredWait(environmentId, key) } From 6594dccf4d38180cb2fda24e9c60ea5d7aa2adfb Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 17:58:53 -0700 Subject: [PATCH 08/10] test(e2e): journeys for a reopened client and two clients on one host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps this suite had no coverage for, both driven end to end against a real paired desktop client rather than at a seam. A relaunched client holding a live remote terminal: every paired restart spec here restarts around a browser pane, none around the terminal the user is actually mid-work in. The host-side fixture's on-disk sink is the oracle — one READY for the whole run proves the host never re-spawned the session, and a recorded line for input sent after the relaunch proves the restored pane is wired to that same process rather than painted with its scrollback. Two clients on one host across an emptied workspace: the tombstone is client-local on the runtime path, so a client that never held a row still seeds into a workspace another client deliberately emptied. That asymmetry is by design; a client falling out of step with the host and staying there is not. Phase 0 is the control — without it a later divergence cannot be attributed to the emptying rather than to mirroring never having worked. The input probe goes through `pane.terminal.input`, not `window.api.pty.write`: a mirrored pane's handle is a `remote:` id that no local PTY answers to, so a direct write is swallowed and the assertion passes on nothing. The pre-restart control exists to catch exactly that, and did. --- ...e-terminal-client-restart-survival.spec.ts | 370 ++++++++++++++++++ ...wo-client-emptied-workspace-reseed.spec.ts | 355 +++++++++++++++++ 2 files changed, 725 insertions(+) create mode 100644 tests/e2e/paired-remote-terminal-client-restart-survival.spec.ts create mode 100644 tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts diff --git a/tests/e2e/paired-remote-terminal-client-restart-survival.spec.ts b/tests/e2e/paired-remote-terminal-client-restart-survival.spec.ts new file mode 100644 index 00000000000..c83a3d1a58f --- /dev/null +++ b/tests/e2e/paired-remote-terminal-client-restart-survival.spec.ts @@ -0,0 +1,370 @@ +/** + * JOURNEY: quit the desktop app while remote terminals are live on the host, then reopen it. + * + * TOPOLOGY: the `orcaPage` app is the host (orca server); a separate real Orca desktop client + * pairs to it, opens a host terminal, works in it, is force-quit, and relaunched on the same + * profile — the pairing credential and the persisted session survive, as they do for a real + * force-quit reopen. + * + * Why this exists: every paired restart spec in this suite restarts around a *browser* pane + * (paired-client-hosted-browser-*.spec.ts). None of them restarts a client holding a live remote + * *terminal*, which is the thing the user is actually mid-work in. + * + * The terminal is a fixture that appends one line per event to a file on disk. That sink is the + * oracle nothing on the client can fake: + * - exactly one `READY` for the whole run means the host never re-spawned the process, so the + * user came back to their session rather than a fresh shell wearing its name; + * - a `LINE:` for input sent after the relaunch means the restored pane is wired to that same + * process, not merely painted with its scrollback. + * + * Run: + * pnpm exec playwright test \ + * tests/e2e/paired-remote-terminal-client-restart-survival.spec.ts \ + * --config tests/playwright.config.ts --project electron-headless --workers=1 + */ +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { randomUUID } from 'node:crypto' +import os from 'node:os' +import path from 'node:path' +import type { Page } from '@stablyai/playwright-test' +import { + HOST_TERMINAL_SURFACE_SEPARATOR, + toWebTerminalSurfaceTabId +} from '../../src/shared/terminal-surface-id' +import { closeElectronAppForE2E } from './helpers/electron-process-shutdown' +import { expect, test } from './helpers/orca-app' +import { + createRuntimeDesktopPairingOffer, + launchPairedElectronClient, + type PairedElectronClient +} from './helpers/paired-electron-client' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +/** What a user would accept for "my terminal is back" after reopening the app. */ +const RESTORE_BUDGET_MS = 60_000 + +const scratch = mkdtempSync(path.join(os.tmpdir(), 'orca-client-restart-survival-')) +const fixturePath = path.join(scratch, 'restart-survival-terminal.mjs') +writeFileSync( + fixturePath, + [ + "import { appendFileSync } from 'node:fs'", + 'const sink = process.argv[2]', + 'const record = (line) => appendFileSync(sink, `${line}\\n`)', + "record('READY')", + "process.stdout.write('RESTART_SURVIVAL_READY\\r\\n')", + "process.stdin.setEncoding('utf8')", + "let pending = ''", + "process.stdin.on('data', (data) => {", + ' pending += data', + ' const lines = pending.split(/\\r\\n|\\r|\\n/)', + " pending = lines.pop() ?? ''", + ' for (const line of lines) {', + ' record(`LINE:${line}`)', + ' process.stdout.write(`LINE:${line}\\r\\n`)', + ' }', + '})', + 'process.stdin.resume()' + ].join('\n') +) + +test.afterAll(() => { + rmSync(scratch, { recursive: true, force: true }) +}) + +function shellQuote(value: string): string { + return `'${value.replaceAll("'", `'\\''`)}'` +} + +function fixtureCommand(sinkPath: string): string { + const command = [process.execPath, fixturePath, sinkPath] + return process.platform === 'win32' + ? command.map((value) => `"${value.replaceAll('"', '""')}"`).join(' ') + : command.map(shellQuote).join(' ') +} + +function readSinkLines(sinkPath: string): string[] { + try { + return readFileSync(sinkPath, 'utf8').split('\n').filter(Boolean) + } catch { + return [] + } +} + +async function callEnvironment( + page: Page, + environmentId: string, + method: string, + params: unknown +): Promise { + return page.evaluate( + async ({ environmentId, method, params }) => { + const response = await window.api.runtimeEnvironments.call({ + selector: environmentId, + method, + params + }) + if (!response.ok) { + throw new Error(`${response.error.code}: ${response.error.message}`) + } + return response.result + }, + { environmentId, method, params } + ) as Promise +} + +async function focusWorkspace(page: Page, worktreeId: string): Promise { + await page.evaluate((id) => { + const state = window.__store?.getState() + state?.setActiveView('terminal') + state?.setActiveWorktree(id) + }, worktreeId) +} + +async function waitForClientWorkspace(page: Page, worktreeId: string): Promise { + await expect + .poll( + () => + page.evaluate( + (id) => (window.__store?.getState().allWorktrees() ?? []).some((w) => w.id === id), + worktreeId + ), + { timeout: 60_000, message: 'paired client never received the host workspace' } + ) + .toBe(true) +} + +/** Milliseconds until the tab is mirrored again, or null if it never was. */ +async function waitForMirroredTab( + page: Page, + worktreeId: string, + webTabId: string, + budgetMs: number +): Promise { + const startedAt = Date.now() + while (Date.now() - startedAt < budgetMs) { + const present = await page.evaluate( + ({ id, worktreeId }) => + (window.__store?.getState().tabsByWorktree[worktreeId] ?? []).some((tab) => tab.id === id), + { id: webTabId, worktreeId } + ) + if (present) { + return Date.now() - startedAt + } + await page.waitForTimeout(500) + } + return null +} + +/** Milliseconds until the restored pane paints `marker`, or null if it never did. */ +async function waitForPanePaint( + page: Page, + webTabId: string, + marker: string, + budgetMs: number +): Promise { + const startedAt = Date.now() + while (Date.now() - startedAt < budgetMs) { + const content = await page.evaluate((id) => { + const manager = window.__paneManagers?.get(id) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + return pane?.serializeAddon?.serialize?.() ?? '' + }, webTabId) + if (content.includes(marker)) { + return Date.now() - startedAt + } + await page.waitForTimeout(500) + } + return null +} + +async function selectClientTab(page: Page, worktreeId: string, webTabId: string): Promise { + await page.evaluate( + ({ webTabId, worktreeId }) => { + const state = window.__store?.getState() + state?.setActiveView('terminal') + state?.setActiveWorktree(worktreeId) + state?.setActiveTab(webTabId) + state?.setActiveTabType('terminal') + }, + { webTabId, worktreeId } + ) +} + +/** + * Types `marker` into the pane until the host-side process records it, or the budget ends. + * + * Why through `pane.terminal.input` and not `window.api.pty.write`: a mirrored pane's handle is + * a `remote:` id that no local PTY answers to, so a direct write is silently swallowed. This is + * the path a keystroke actually takes, and it is retried because a pane still reattaching can + * replay-suppress a write (helpers/restored-terminal-input-readiness.ts polls for that reason). + */ +async function driveInputUntilProcessSees( + client: PairedElectronClient, + webTabId: string, + sinkPath: string, + marker: string, + budgetMs: number +): Promise { + const startedAt = Date.now() + while (Date.now() - startedAt < budgetMs) { + await client.page.evaluate( + ({ id, text }) => { + const manager = window.__paneManagers?.get(id) + const pane = manager?.getActivePane?.() ?? manager?.getPanes?.()[0] ?? null + pane?.terminal?.input?.(text, true) + }, + { id: webTabId, text: `${marker}\r` } + ) + if (readSinkLines(sinkPath).some((line) => line.includes(marker))) { + return true + } + await client.page.waitForTimeout(1_000) + } + return false +} + +async function readTabPtyIds(client: PairedElectronClient, webTabId: string): Promise { + return client.page.evaluate((id) => window.__store?.getState().ptyIdsByTabId[id] ?? [], webTabId) +} + +test('a relaunched client gets its live remote terminal back, still attached to the same process', async ({ + orcaPage +}, testInfo) => { + test.setTimeout(900_000) + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const worktreeId = await orcaPage.evaluate(() => { + const id = window.__store?.getState().activeWorktreeId + if (!id) { + throw new Error('host has no active worktree') + } + return id + }) + + const sinkPath = path.join(scratch, `sink-${randomUUID()}.log`) + const failures: string[] = [] + let client: PairedElectronClient | null = null + const offer = await createRuntimeDesktopPairingOffer(orcaPage) + try { + client = await launchPairedElectronClient(offer, testInfo, 'remote-terminal-restart-survival') + const userDataDir = client.userDataDir + await waitForClientWorkspace(client.page, worktreeId) + await focusWorkspace(client.page, worktreeId) + + const created = await callEnvironment<{ tab: { id: string; terminal: string | null } }>( + client.page, + client.environmentId, + 'session.tabs.createTerminal', + { + worktree: `id:${worktreeId}`, + command: fixtureCommand(sinkPath), + activate: true, + select: true, + navigation: 'caller' + } + ) + const hostTabId = created.tab.id.split(HOST_TERMINAL_SURFACE_SEPARATOR)[0]! + const webTabId = toWebTerminalSurfaceTabId(hostTabId) + expect( + await waitForMirroredTab(client.page, worktreeId, webTabId, RESTORE_BUDGET_MS), + 'the client never mirrored the terminal it created' + ).not.toBeNull() + await selectClientTab(client.page, worktreeId, webTabId) + await expect + .poll(() => readSinkLines(sinkPath), { + timeout: RESTORE_BUDGET_MS, + message: 'the host terminal fixture never started' + }) + .toContain('READY') + expect( + await waitForPanePaint(client.page, webTabId, 'RESTART_SURVIVAL_READY', RESTORE_BUDGET_MS), + 'the pane never painted the live terminal before the restart' + ).not.toBeNull() + + // The control. Without it, "input did not arrive after the restart" cannot be told apart + // from "this input path never worked in this topology". + const ptyIdsBefore = await readTabPtyIds(client, webTabId) + expect(ptyIdsBefore, 'the live pane had no PTY handle before the restart').not.toHaveLength(0) + expect( + await driveInputUntilProcessSees( + client, + webTabId, + sinkPath, + 'PRE_RESTART_CONTROL', + RESTORE_BUDGET_MS + ), + 'input did not reach the host process even before the restart — the probe, not the product' + ).toBe(true) + + // ── The restart: force-quit and reopen on the same profile. ── + // Quit without disposing: the profile has to outlive the app, as it does for a real Cmd+Q. + const quitting = client.app + client = null + await closeElectronAppForE2E(quitting) + client = await launchPairedElectronClient( + offer, + testInfo, + 'remote-terminal-restart-survival-relaunch', + { reuseUserDataDir: userDataDir } + ) + await waitForClientWorkspace(client.page, worktreeId) + await focusWorkspace(client.page, worktreeId) + + const tabBackMs = await waitForMirroredTab(client.page, worktreeId, webTabId, RESTORE_BUDGET_MS) + console.error(`[client-restart] tabBackMs=${tabBackMs}`) + if (tabBackMs === null) { + failures.push('the remote terminal tab never came back after the app was reopened') + } else { + await selectClientTab(client.page, worktreeId, webTabId) + const paintedMs = await waitForPanePaint( + client.page, + webTabId, + 'RESTART_SURVIVAL_READY', + RESTORE_BUDGET_MS + ) + console.error(`[client-restart] paintedMs=${paintedMs}`) + if (paintedMs === null) { + failures.push( + 'the remote terminal came back empty — the tab is there but the transcript is not' + ) + } + } + + // Is the restored pane actually wired to the live process, or only painted with its past? + const marker = `POST_RESTART_${randomUUID().slice(0, 8)}` + const ptyIds = await readTabPtyIds(client, webTabId) + console.error(`[client-restart] ptyBefore=${ptyIdsBefore[0]} ptyAfter=${ptyIds[0] ?? 'none'}`) + if (ptyIds.length === 0) { + failures.push( + 'the restored tab has no PTY handle — nothing the user types can reach the host' + ) + } else { + const echoed = await driveInputUntilProcessSees( + client, + webTabId, + sinkPath, + marker, + RESTORE_BUDGET_MS + ) + console.error(`[client-restart] inputReachedProcess=${echoed}`) + if (!echoed) { + failures.push( + 'input typed into the restored terminal never reached the process the host is running' + ) + } + } + + // The sink is the fork oracle: a second READY means the host re-spawned the user's work. + const readyCount = readSinkLines(sinkPath).filter((line) => line === 'READY').length + console.error(`[client-restart] readyCount=${readyCount}`) + if (readyCount !== 1) { + failures.push( + `the host process was re-spawned across the client restart (READY x${readyCount}) — the user's session was replaced, not restored` + ) + } + } finally { + await client?.dispose() + } + expect(failures, failures.join('\n')).toEqual([]) +}) diff --git a/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts new file mode 100644 index 00000000000..3bf73bd928f --- /dev/null +++ b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts @@ -0,0 +1,355 @@ +/** + * JOURNEY: two desktop clients paired to one Orca server, working in the same workspace. + * + * TOPOLOGY: the `orcaPage` app is the host (orca server). Two separate real Orca desktop + * clients pair to it, exactly as two of the user's machines would. Nothing is faulted — this + * is the ordinary shape of using Orca from a laptop and a desktop at the same time. + * + * The emptied-workspace tombstone is an explicit `tabsByWorktree[worktreeId] = []` row and it + * is client-local on the runtime path: it never crosses the wire, so the second client cannot + * know the first emptied the workspace on purpose and still seeds into it. That asymmetry is + * by design. What is NOT by design is a client falling out of step with the host and staying + * there, which is what this spec measures. + * + * Phase 0 is the control: with both clients attached, does a terminal created on one reach the + * other at all? Without it a later divergence cannot be attributed to the emptying. + * + * Run: + * pnpm exec playwright test \ + * tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts \ + * --config tests/playwright.config.ts --project electron-headless --workers=1 + */ +import type { Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { + createRuntimeDesktopPairingOffer, + launchPairedElectronClient, + type PairedElectronClient +} from './helpers/paired-electron-client' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' + +/** How long a client may lag the host before the user would call it broken. */ +const MIRROR_BUDGET_MS = 30_000 +/** A retraction may be slow; what matters is whether it arrives at all. */ +const RETRACTION_BUDGET_MS = 90_000 + +type HostTabRow = { id: string; parentTabId?: string; terminal?: string | null } + +async function callEnvironment( + page: Page, + environmentId: string, + method: string, + params: unknown +): Promise { + return page.evaluate( + async ({ environmentId, method, params }) => { + const response = await window.api.runtimeEnvironments.call({ + selector: environmentId, + method, + params + }) + if (!response.ok) { + throw new Error(`${response.error.code}: ${response.error.message}`) + } + return response.result + }, + { environmentId, method, params } + ) as Promise +} + +/** The host's own tab inventory — the only oracle that is not a client re-derivation. */ +async function readHostTerminalTabIds( + client: PairedElectronClient, + worktreeId: string +): Promise { + const inventory = await callEnvironment<{ tabs: HostTabRow[] }>( + client.page, + client.environmentId, + 'session.tabs.list', + { worktree: `id:${worktreeId}` } + ) + return [ + ...new Set( + inventory.tabs + .filter((tab) => tab.terminal !== undefined && tab.terminal !== null) + .map((tab) => tab.parentTabId ?? tab.id) + ) + ].sort() +} + +async function readMirroredTabCount(page: Page, worktreeId: string): Promise { + return page.evaluate( + (id) => (window.__store?.getState().tabsByWorktree[id] ?? []).length, + worktreeId + ) +} + +/** Whether the client holds an explicit empty row (the tombstone) versus no row at all. */ +async function readWorkspaceRowState( + page: Page, + worktreeId: string +): Promise<'missing' | 'tombstoned' | 'populated'> { + return page.evaluate((id) => { + const tabs = window.__store?.getState().tabsByWorktree + if (!tabs || !Object.hasOwn(tabs, id)) { + return 'missing' as const + } + return (tabs[id] ?? []).length === 0 ? ('tombstoned' as const) : ('populated' as const) + }, worktreeId) +} + +async function focusWorkspace(page: Page, worktreeId: string): Promise { + await page.evaluate((id) => { + const state = window.__store?.getState() + state?.setActiveView('terminal') + state?.setActiveWorktree(id) + }, worktreeId) +} + +/** Milliseconds until the client's mirrored count matches the host's, or null if it never did. */ +async function waitForClientToMatchHost( + client: PairedElectronClient, + hostCount: number, + worktreeId: string, + budgetMs: number +): Promise { + const startedAt = Date.now() + while (Date.now() - startedAt < budgetMs) { + if ((await readMirroredTabCount(client.page, worktreeId)) === hostCount) { + return Date.now() - startedAt + } + await client.page.waitForTimeout(500) + } + return null +} + +async function waitForClientWorkspace(page: Page, worktreeId: string): Promise { + await expect + .poll( + () => + page.evaluate( + (id) => (window.__store?.getState().allWorktrees() ?? []).some((w) => w.id === id), + worktreeId + ), + { timeout: 60_000, message: 'paired client never received the host workspace' } + ) + .toBe(true) +} + +test('two paired clients stay in step with the host across an emptied workspace', async ({ + orcaPage +}, testInfo) => { + test.setTimeout(600_000) + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const worktreeId = await orcaPage.evaluate(() => { + const id = window.__store?.getState().activeWorktreeId + if (!id) { + throw new Error('host has no active worktree') + } + return id + }) + + let clientA: PairedElectronClient | null = null + let clientB: PairedElectronClient | null = null + const failures: string[] = [] + try { + clientA = await launchPairedElectronClient( + await createRuntimeDesktopPairingOffer(orcaPage), + testInfo, + 'emptied-workspace-client-a' + ) + clientB = await launchPairedElectronClient( + await createRuntimeDesktopPairingOffer(orcaPage), + testInfo, + 'emptied-workspace-client-b' + ) + for (const client of [clientA, clientB]) { + await waitForClientWorkspace(client.page, worktreeId) + await focusWorkspace(client.page, worktreeId) + } + + // ── Phase 0: the control. A creates a terminal; B must see it. ── + await callEnvironment(clientA.page, clientA.environmentId, 'session.tabs.createTerminal', { + worktree: `id:${worktreeId}`, + activate: true, + select: true, + navigation: 'caller' + }) + const afterCreate = (await readHostTerminalTabIds(clientA, worktreeId)).length + const controlA = await waitForClientToMatchHost( + clientA, + afterCreate, + worktreeId, + MIRROR_BUDGET_MS + ) + const controlB = await waitForClientToMatchHost( + clientB, + afterCreate, + worktreeId, + MIRROR_BUDGET_MS + ) + console.error(`[two-client] phase0 host=${afterCreate} A=${controlA}ms B=${controlB}ms`) + if (controlA === null || controlB === null) { + failures.push( + `phase0: a terminal created on one client never reached the other (host=${afterCreate}, A=${controlA}, B=${controlB})` + ) + } + + // ── Phase 1: A empties the workspace by hand. ── + for (const hostTabId of await readHostTerminalTabIds(clientA, worktreeId)) { + await callEnvironment(clientA.page, clientA.environmentId, 'session.tabs.close', { + worktree: `id:${worktreeId}`, + tabId: hostTabId, + reason: 'user', + navigation: 'caller' + }) + } + await expect + .poll(() => readHostTerminalTabIds(clientA, worktreeId).then((ids) => ids.length), { + timeout: MIRROR_BUDGET_MS, + message: 'host still held terminals after client A closed them all' + }) + .toBe(0) + // Deliberately generous: the question is whether the retraction ever arrives, not whether + // it is prompt. A client still showing a terminal the host has destroyed is a dead pane the + // user will click. + const emptyA = await waitForClientToMatchHost(clientA, 0, worktreeId, RETRACTION_BUDGET_MS) + const emptyB = await waitForClientToMatchHost(clientB, 0, worktreeId, RETRACTION_BUDGET_MS) + const hostOwnView = await readMirroredTabCount(orcaPage, worktreeId) + console.error( + `[two-client] phase1 host=0 hostOwnView=${hostOwnView}` + + ` A=${emptyA}ms(${await readWorkspaceRowState(clientA.page, worktreeId)})` + + ` B=${emptyB}ms(${await readWorkspaceRowState(clientB.page, worktreeId)})` + ) + if (emptyA === null || emptyB === null) { + failures.push( + `phase1: a client kept showing terminals the host no longer has (A=${emptyA}, B=${emptyB})` + ) + } + + // Neither client may seed a replacement into a workspace the user deliberately emptied: + // both hold a row for it, so both know it was emptied rather than never initialized. + await orcaPage.waitForTimeout(10_000) + const hostAfterSettle = (await readHostTerminalTabIds(clientA, worktreeId)).length + console.error(`[two-client] phase1-settled host=${hostAfterSettle}`) + if (hostAfterSettle !== 0) { + failures.push( + `phase1: the emptied workspace grew ${hostAfterSettle} terminal(s) back on its own` + ) + } + + // ── Phase 2: B creates a terminal again. Both clients must follow the host. ── + await callEnvironment(clientB.page, clientB.environmentId, 'session.tabs.createTerminal', { + worktree: `id:${worktreeId}`, + activate: true, + select: true, + navigation: 'caller' + }) + const hostAfterB = (await readHostTerminalTabIds(clientB, worktreeId)).length + const rejoinB = await waitForClientToMatchHost( + clientB, + hostAfterB, + worktreeId, + MIRROR_BUDGET_MS + ) + const rejoinA = await waitForClientToMatchHost( + clientA, + hostAfterB, + worktreeId, + MIRROR_BUDGET_MS + ) + console.error(`[two-client] phase2 host=${hostAfterB} A=${rejoinA}ms B=${rejoinB}ms`) + if (rejoinA === null || rejoinB === null) { + failures.push( + `phase2: a client never adopted the terminal the host holds — the user sees an empty` + + ` workspace while work runs on it (host=${hostAfterB}, A=${rejoinA}, B=${rejoinB})` + ) + } + } finally { + await clientB?.dispose() + await clientA?.dispose() + } + expect(failures, failures.join('\n')).toEqual([]) +}) + +/** + * The same workspace, driven by a client that starts working the moment it finishes pairing — + * which is what a user does on a machine they have just added. + * + * Isolated from the two-client test above because the failure it hunts is a startup race, not a + * multi-client one: the earlier form of that test drove the close seconds after the pairing + * completed and repeatedly left the client's mirror stuck — sometimes still showing the terminal + * the host had closed, sometimes stuck empty afterwards — with the link demonstrably alive. + */ +test('a client that works immediately after pairing stays in step with the host', async ({ + orcaPage +}, testInfo) => { + test.setTimeout(600_000) + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + const worktreeId = await orcaPage.evaluate(() => { + const id = window.__store?.getState().activeWorktreeId + if (!id) { + throw new Error('host has no active worktree') + } + return id + }) + + let client: PairedElectronClient | null = null + const failures: string[] = [] + try { + client = await launchPairedElectronClient( + await createRuntimeDesktopPairingOffer(orcaPage), + testInfo, + 'fresh-pairing-immediate-work' + ) + await waitForClientWorkspace(client.page, worktreeId) + await focusWorkspace(client.page, worktreeId) + + await callEnvironment(client.page, client.environmentId, 'session.tabs.createTerminal', { + worktree: `id:${worktreeId}`, + activate: true, + select: true, + navigation: 'caller' + }) + const afterCreate = (await readHostTerminalTabIds(client, worktreeId)).length + const sawCreate = await waitForClientToMatchHost( + client, + afterCreate, + worktreeId, + MIRROR_BUDGET_MS + ) + console.error(`[fresh-pairing] create host=${afterCreate} client=${sawCreate}ms`) + if (sawCreate === null) { + failures.push( + `the client never mirrored the terminal it had just created (host=${afterCreate})` + ) + } + + for (const hostTabId of await readHostTerminalTabIds(client, worktreeId)) { + await callEnvironment(client.page, client.environmentId, 'session.tabs.close', { + worktree: `id:${worktreeId}`, + tabId: hostTabId, + reason: 'user', + navigation: 'caller' + }) + } + await expect + .poll(() => readHostTerminalTabIds(client!, worktreeId).then((ids) => ids.length), { + timeout: MIRROR_BUDGET_MS, + message: 'host still held terminals after the client closed them all' + }) + .toBe(0) + const sawClose = await waitForClientToMatchHost(client, 0, worktreeId, MIRROR_BUDGET_MS) + console.error( + `[fresh-pairing] close client=${sawClose}ms row=${await readWorkspaceRowState(client.page, worktreeId)}` + ) + if (sawClose === null) { + failures.push('the client kept showing a terminal the host had already closed') + } + } finally { + await client?.dispose() + } + expect(failures, failures.join('\n')).toEqual([]) +}) From 456ed8a98b53c378ac8d29a7cfca4f73eef7ef8d Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:00:35 -0700 Subject: [PATCH 09/10] test(e2e): keep the two-client journey spec type-clean --- tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts index 3bf73bd928f..231e5cc58fb 100644 --- a/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts +++ b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts @@ -206,7 +206,7 @@ test('two paired clients stay in step with the host across an emptied workspace' }) } await expect - .poll(() => readHostTerminalTabIds(clientA, worktreeId).then((ids) => ids.length), { + .poll(() => readHostTerminalTabIds(clientA!, worktreeId).then((ids) => ids.length), { timeout: MIRROR_BUDGET_MS, message: 'host still held terminals after client A closed them all' }) From fc794784cd6a43c2ee51c088c520e2bf8acb637c Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 18:22:05 -0700 Subject: [PATCH 10/10] test(e2e): pin the close retraction a paired host does not publish --- ...wo-client-emptied-workspace-reseed.spec.ts | 54 +++++++++++++++++-- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts index 231e5cc58fb..f82ad8e7682 100644 --- a/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts +++ b/tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts @@ -14,6 +14,17 @@ * Phase 0 is the control: with both clients attached, does a terminal created on one reach the * other at all? Without it a later divergence cannot be attributed to the emptying. * + * KNOWN RED as of this commit, and the shape of the failure is the finding. Across 8 runs the + * close phases were all-or-nothing: either every retraction reached both clients in single-digit + * milliseconds, or none reached either client within 90 seconds — and phase 1a, which closes a + * terminal while others remain open, fails alongside phase 1b, so it is not about the workspace + * going empty. Creates always propagate, including the phase 2 create that lands in ~3ms on the + * very clients that just missed a close for 90s, so the subscription is demonstrably alive. Both + * clients failing together, while the host's own window shows the correct count, puts the fault + * on the host's publish-after-close rather than on any client's mirror. What the user sees: a + * terminal they closed on one machine stays in the tab bar on the other, pointing at a process + * that no longer exists, until some unrelated change to the workspace forces a republish. + * * Run: * pnpm exec playwright test \ * tests/e2e/paired-two-client-emptied-workspace-reseed.spec.ts \ @@ -196,7 +207,40 @@ test('two paired clients stay in step with the host across an emptied workspace' ) } - // ── Phase 1: A empties the workspace by hand. ── + // ── Phase 1a: A closes one terminal, but not the last one. ── + // Separated from the emptying below on purpose: it is the control that says whether a + // retraction propagates at all, so a failure in 1b can be attributed to the workspace going + // empty rather than to close retractions being broken in general. + const beforePartialClose = await readHostTerminalTabIds(clientA, worktreeId) + if (beforePartialClose.length > 1) { + await callEnvironment(clientA.page, clientA.environmentId, 'session.tabs.close', { + worktree: `id:${worktreeId}`, + tabId: beforePartialClose[0]!, + reason: 'user', + navigation: 'caller' + }) + const remaining = beforePartialClose.length - 1 + const partialA = await waitForClientToMatchHost( + clientA, + remaining, + worktreeId, + RETRACTION_BUDGET_MS + ) + const partialB = await waitForClientToMatchHost( + clientB, + remaining, + worktreeId, + RETRACTION_BUDGET_MS + ) + console.error(`[two-client] phase1a host=${remaining} A=${partialA}ms B=${partialB}ms`) + if (partialA === null || partialB === null) { + failures.push( + `phase1a: a client kept showing a terminal the host closed, with others still open (A=${partialA}, B=${partialB})` + ) + } + } + + // ── Phase 1b: A empties the workspace by hand. ── for (const hostTabId of await readHostTerminalTabIds(clientA, worktreeId)) { await callEnvironment(clientA.page, clientA.environmentId, 'session.tabs.close', { worktree: `id:${worktreeId}`, @@ -218,13 +262,13 @@ test('two paired clients stay in step with the host across an emptied workspace' const emptyB = await waitForClientToMatchHost(clientB, 0, worktreeId, RETRACTION_BUDGET_MS) const hostOwnView = await readMirroredTabCount(orcaPage, worktreeId) console.error( - `[two-client] phase1 host=0 hostOwnView=${hostOwnView}` + + `[two-client] phase1b host=0 hostOwnView=${hostOwnView}` + ` A=${emptyA}ms(${await readWorkspaceRowState(clientA.page, worktreeId)})` + ` B=${emptyB}ms(${await readWorkspaceRowState(clientB.page, worktreeId)})` ) if (emptyA === null || emptyB === null) { failures.push( - `phase1: a client kept showing terminals the host no longer has (A=${emptyA}, B=${emptyB})` + `phase1b: a client kept showing terminals the host no longer has (A=${emptyA}, B=${emptyB})` ) } @@ -232,10 +276,10 @@ test('two paired clients stay in step with the host across an emptied workspace' // both hold a row for it, so both know it was emptied rather than never initialized. await orcaPage.waitForTimeout(10_000) const hostAfterSettle = (await readHostTerminalTabIds(clientA, worktreeId)).length - console.error(`[two-client] phase1-settled host=${hostAfterSettle}`) + console.error(`[two-client] phase1b-settled host=${hostAfterSettle}`) if (hostAfterSettle !== 0) { failures.push( - `phase1: the emptied workspace grew ${hostAfterSettle} terminal(s) back on its own` + `phase1b: the emptied workspace grew ${hostAfterSettle} terminal(s) back on its own` ) }