From 2e4987c2f68809d0748a9f533ec53ee1787d4779 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 17:36:59 -0700 Subject: [PATCH] fix(terminal): replace stale reconnect identities atomically --- config/reliability-gates.jsonc | 46 ++++++-- .../terminal-pane/pty-connection-types.ts | 2 +- .../terminal-pane/pty-connection.test.ts | 10 +- .../terminal-pane/pty-connection.ts | 13 ++- .../terminal-pane/pty-transport-types.ts | 2 +- .../remote-runtime-pty-transport.test.ts | 109 ++++++++++++++++-- .../remote-runtime-pty-transport.ts | 68 ++++++----- .../use-terminal-pane-lifecycle.ts | 2 +- .../terminal-pty-identity-replacement.test.ts | 26 +++++ src/renderer/src/store/slices/terminals.ts | 60 +++++----- 10 files changed, 255 insertions(+), 83 deletions(-) create mode 100644 src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 448b4beac63..68f7d3c5fce 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -1,6 +1,6 @@ { "schemaVersion": 1, - "updatedAt": "2026-07-13", + "updatedAt": "2026-07-17", "policy": { "maturityLevels": [ "experimental", @@ -2176,9 +2176,10 @@ "macos" ], "coveredProviders": [ - "ssh" + "ssh", + "remote-runtime" ], - "coverageNotes": "Deterministic renderer coverage proves startup publishes the state returned by ssh.connect, remote or unresolved hosts cannot enter local-daemon cold activation deferral, and stale cleanup cannot unregister a replacement runtime terminal. A macOS Electron journey connects to a real Linux SSH relay in Docker, persists six terminals, reloads the renderer, and verifies all six original relay PTYs remount eagerly and still execute remote input. Linux and Windows desktop clients, WSL, and remote-runtime restore remain gaps.", + "coverageNotes": "Deterministic renderer coverage proves startup publishes the state returned by ssh.connect, remote or unresolved hosts cannot enter local-daemon cold activation deferral, stale cleanup cannot unregister a replacement runtime terminal, and a mounted remote-runtime web mirror replaces a stale handle without retiring its pane or accumulating obsolete PTY identities. A macOS Electron journey connects to a real Linux SSH relay in Docker, persists six terminals, reloads the renderer, and verifies all six original relay PTYs remount eagerly and still execute remote input. Linux and Windows desktop clients, WSL, and live remote-runtime restore remain gaps.", "motivatingLinks": [ "https://github.com/stablyai/orca/pull/6951", "https://github.com/stablyai/orca/pull/6955", @@ -2186,10 +2187,10 @@ "https://github.com/stablyai/orca/pull/7009", "https://github.com/stablyai/orca/pull/8597" ], - "invariant": "SSH, WSL, and remote-runtime restore paths must treat provider listing failures and unknown liveness as unknown, not dead, while still avoiding duplicate spawn and clearing expired relay leases exactly once. Snapshot-backed cold activation deferral is local-daemon-only; every restored remote terminal must mount a manager eagerly and preserve its provider PTY identity.", - "oracle": "Deterministic tests assert startup publishes ssh.connect's authoritative state before terminal reconnect, only an explicit local execution host can defer cold activation, host ownership changes clear old restrictions, and replacement runtime registrations survive stale cleanup. The live Docker SSH journey commits six terminal records to the relay, reloads Electron, requires the exact six manager ids with no parked tabs, compares all restored PTY ids to their pre-reload identities, and sends a marker through the restored terminal to a proof file on the Linux host.", + "invariant": "SSH, WSL, and remote-runtime restore paths must treat provider listing failures and unknown liveness as unknown, not dead, while still avoiding duplicate spawn and clearing expired relay leases exactly once. Snapshot-backed cold activation deferral is local-daemon-only; every restored remote terminal must mount a manager eagerly and preserve its provider PTY identity. A stale remote-runtime mirror handle must keep its pane mounted until a replacement or explicit terminal-removal fact arrives, and each replacement must atomically supersede rather than accumulate PTY identity.", + "oracle": "Deterministic tests assert startup publishes ssh.connect's authoritative state before terminal reconnect, only an explicit local execution host can defer cold activation, host ownership changes clear old restrictions, replacement runtime registrations survive stale cleanup, stale mirror polling is bounded, and replacement handles resubscribe without exit/disconnect callbacks or stale PTY index growth. The live Docker SSH journey commits six terminal records to the relay, reloads Electron, requires the exact six manager ids with no parked tabs, compares all restored PTY ids to their pre-reload identities, and sends a marker through the restored terminal to a proof file on the Linux host.", "commands": [ - "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/startup/ssh-startup-reconnect.test.ts src/renderer/src/lib/resolved-worktree-execution-host.test.ts src/renderer/src/components/terminal/background-terminal-worktree-mount.test.ts src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/startup/ssh-startup-reconnect.test.ts src/renderer/src/lib/resolved-worktree-execution-host.test.ts src/renderer/src/components/terminal/background-terminal-worktree-mount.test.ts src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts", "pnpm exec electron-vite build --mode e2e", "SKIP_BUILD=1 pnpm exec playwright test tests/e2e/terminal-cold-activation-deferral.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", "ORCA_E2E_SSH_DOCKER=1 SKIP_BUILD=1 pnpm exec playwright test tests/e2e/ssh-cold-activation-restore.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1" @@ -2201,6 +2202,8 @@ "src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts", "src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts", "src/renderer/src/components/terminal-pane/pty-connection.test.ts", + "src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts", + "src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts", "tests/e2e/terminal-cold-activation-deferral.spec.ts", "tests/e2e/ssh-cold-activation-restore.spec.ts" ], @@ -2218,6 +2221,20 @@ "SSH, remote-runtime, and unresolved owners remain eager" ] }, + { + "file": "src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts", + "assertions": [ + "a stale web-mirror handle polls until a different ready handle is published without resubscribing the stale handle", + "replacement does not emit pane exit or disconnect callbacks and explicit terminal exit still retires the mirror", + "replacement polling backs off to a 15-second bound, preserves the pane without removal evidence, and retires it when the host surface is explicitly absent" + ] + }, + { + "file": "src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts", + "assertions": [ + "repeated handle rotations retain exactly one live PTY identity and update the tab fallback atomically" + ] + }, { "file": "tests/e2e/ssh-cold-activation-restore.spec.ts", "assertions": [ @@ -2236,6 +2253,15 @@ "result": "passed", "durationSeconds": 128, "summary": "Six focused files and 475 tests passed; the local cold-activation journey passed; the Docker/Linux SSH journey restored six of six original relay PTYs, mounted six of six managers without parking, and executed remote input through the restored terminal." + }, + { + "date": "2026-07-17", + "runner": "local", + "platform": "macos", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/startup/ssh-startup-reconnect.test.ts src/renderer/src/lib/resolved-worktree-execution-host.test.ts src/renderer/src/components/terminal/background-terminal-worktree-mount.test.ts src/renderer/src/runtime/sync-runtime-graph-scheduling.test.ts src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts src/renderer/src/components/terminal-pane/pty-connection.test.ts src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts", + "result": "passed", + "durationSeconds": 15, + "summary": "Eight provider-contract files and 560 tests passed, including bounded stale-handle polling, timeout without pane retirement, in-place rebind, explicit-exit retirement, and repeated replacement without PTY index growth." } ], "runtimeBudget": { @@ -2248,11 +2274,11 @@ }, "redGreenEvidence": { "status": "partial", - "evidence": "Tests assert SSH attach uses relay pty.attach for saved sessions, expired relay attach does not fresh-spawn in the provider, a failed SSH listProcesses observation does not clear previously learned ownership, a rejected SSH hasPty probe resolves as unknown liveness, deferred SSH passphrase cancellation does not auto-reconnect, deferred attach uses saved leaf/tab session ids once connected, transient deferred reattach failure preserves the saved session id without clearing pane/tab bindings or fresh-spawning, deferred SSH no-result cleanup clears the pending serializer without clearing pane/tab bindings or consuming the saved restore id, disconnected SSH relay ids are retained as deferred reconnect metadata and sidebar wake hints rather than attached PTY proof, and expired deferred relay state clears stale pane/tab bindings before one fresh replacement spawn. Needs WSL, remote-runtime mirror polling, live SSH, and saved intentional-break artifacts before promotion." + "evidence": "Tests assert SSH attach uses relay pty.attach for saved sessions, expired relay attach does not fresh-spawn in the provider, a failed SSH listProcesses observation does not clear previously learned ownership, a rejected SSH hasPty probe resolves as unknown liveness, deferred SSH passphrase cancellation does not auto-reconnect, deferred attach uses saved leaf/tab session ids once connected, transient deferred reattach failure preserves the saved session id without clearing pane/tab bindings or fresh-spawning, deferred SSH no-result cleanup clears the pending serializer without clearing pane/tab bindings or consuming the saved restore id, disconnected SSH relay ids are retained as deferred reconnect metadata and sidebar wake hints rather than attached PTY proof, expired deferred relay state clears stale pane/tab bindings before one fresh replacement spawn, and remote-runtime stale handles rotate in place without stale resubscription, pane retirement, or PTY index growth. Needs WSL, live remote-runtime/SSH, and saved intentional-break artifacts before promotion." }, "performanceBudget": { "required": true, - "evidence": "Host eligibility is resolved from existing store metadata during activation and adds no provider listing or polling. The local journey preserves cold deferral, while SSH eagerly mounts six managers as required; no work is added to typing or terminal-output hot paths." + "evidence": "Host eligibility is resolved from existing store metadata during activation. Remote-runtime resubscribe polling runs only after a transport close, reserves replacement-only matching for typed stale-handle evidence, backs off from 150 ms to 1 s, stops after 15 s or lifecycle cancellation, and atomically replaces stale PTY state instead of growing it. The local journey preserves cold deferral, while SSH eagerly mounts six managers as required; no work is added to typing or terminal-output hot paths." }, "promotionCriteria": [ "Use deterministic fake providers for failure and unknown-liveness cases.", @@ -2262,8 +2288,8 @@ "knownGaps": [ "Current command covers store wake-hint metadata, main-process SSH provider failure semantics, provider attach/expired-attach behavior, and renderer deferred SSH reconnect/transient-failure/expired-relay fallback with mocked transports.", "The live SSH journey is environment-dependent and currently runs from a macOS Electron client against a Linux Docker host.", - "WSL and remote-runtime mirror polling contracts are not wired yet.", - "Linux and Windows desktop-client journeys are not yet collected." + "WSL restore remains inferred rather than directly covered.", + "Linux and Windows desktop-client journeys and a live remote-runtime reconnect soak are not yet collected." ], "demotionRule": "Cannot promote if provider failure can close panes or if the oracle is screenshot-only." }, diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index 67005266f7d..6805ccab3ba 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -55,7 +55,7 @@ export type PtyConnectionDeps = { updateTabTitle: (tabId: string, title: string) => void setRuntimePaneTitle: (tabId: string, paneId: number, title: string) => void clearRuntimePaneTitle: (tabId: string, paneId: number) => void - updateTabPtyId: (tabId: string, ptyId: string) => void + updateTabPtyId: (tabId: string, ptyId: string, replacedPtyId?: string) => void markWorktreeUnread: (worktreeId: string) => void markTerminalTabUnread: (tabId: string) => void markTerminalPaneUnread: (paneKey: string) => void diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index f0febe190c3..1a07cdd5dfd 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -2542,17 +2542,21 @@ describe('connectPanePty', () => { connectPanePty(createPane(1) as never, manager as never, deps as never) const onPtyRebind = createdTransportOptions[0]?.onPtyRebind as - | ((ptyId: string) => void) + | ((ptyId: string, replacedPtyId: string) => void) | undefined const onPtyExit = createdTransportOptions[0]?.onPtyExit as ((ptyId: string) => void) | undefined expect(onPtyRebind).toBeTypeOf('function') expect(onPtyExit).toBeTypeOf('function') - onPtyRebind?.('terminal-reconnected') + onPtyRebind?.('terminal-reconnected', 'terminal-old') onPtyExit?.('terminal-reconnected') expect(deps.syncPanePtyLayoutBinding).toHaveBeenCalledWith(1, 'terminal-reconnected') - expect(deps.updateTabPtyId).toHaveBeenCalledWith('tab-1', 'terminal-reconnected') + expect(deps.updateTabPtyId).toHaveBeenCalledWith( + 'tab-1', + 'terminal-reconnected', + 'terminal-old' + ) expect(deps.onPtyExitRef.current).toHaveBeenCalledWith('terminal-reconnected') expect(manager.closePane).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index f240e0cd5a1..ee99f0be997 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -2737,6 +2737,7 @@ export function connectPanePty( options: { seedInitialAgentStatus?: boolean updateTabPtyId?: 'always' | 'if-missing' + replacePtyId?: string sampleVisibleForegroundAgent?: boolean } = {} ): void => { @@ -2754,7 +2755,11 @@ export function connectPanePty( deps.syncPanePtyLayoutBinding(pane.id, ptyId) const tabPtyIds = useAppStore.getState().ptyIdsByTabId?.[deps.tabId] ?? [] if (options.updateTabPtyId !== 'if-missing' || !tabPtyIds.includes(ptyId)) { - deps.updateTabPtyId(deps.tabId, ptyId) + if (options.replacePtyId) { + deps.updateTabPtyId(deps.tabId, ptyId, options.replacePtyId) + } else { + deps.updateTabPtyId(deps.tabId, ptyId) + } } if (options.seedInitialAgentStatus) { applyInitialAgentStatus() @@ -2795,10 +2800,10 @@ export function connectPanePty( // once the PTY exists, then let real hook events refine or complete it. bindActivePanePty(ptyId, { seedInitialAgentStatus: true }) } - const onPtyRebind = (ptyId: string): void => { + const onPtyRebind = (ptyId: string, replacedPtyId: string): void => { // Why: provider handle rotation keeps the existing pane/session generation; - // treating it as a fresh spawn can strand a later real exit as a newborn. - bindActivePanePty(ptyId) + // replace its stale store identity without fresh-spawn exit semantics. + bindActivePanePty(ptyId, { replacePtyId: replacedPtyId }) } // ─── Attention signal: BEL ──────────────────────────────────────────── // diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index 0cd8570f6bf..40ffca3c65c 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -158,7 +158,7 @@ export type IpcPtyTransportOptions = { onTitleChange?: (title: string, rawTitle: string) => void onPtySpawn?: (ptyId: string) => void /** Rebind an existing pane after its provider replaces the PTY identity. */ - onPtyRebind?: (ptyId: string) => void + onPtyRebind?: (ptyId: string, replacedPtyId: string) => void onBell?: () => void onAgentBecameIdle?: (title: string) => void onAgentBecameWorking?: () => void diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 8d169e6c224..2dbd44d324b 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -367,7 +367,10 @@ describe('createRemoteRuntimePtyTransport', () => { ) expect(transport.getPtyId()).toContain('terminal-2') expect(onPtySpawn).not.toHaveBeenCalled() - expect(onPtyRebind).toHaveBeenCalledWith(expect.stringContaining('terminal-2')) + expect(onPtyRebind).toHaveBeenCalledWith( + expect.stringContaining('terminal-2'), + expect.stringContaining('terminal-1') + ) }) it('retires the mirror when the host no longer publishes the surface after a transport close', async () => { @@ -462,7 +465,7 @@ describe('createRemoteRuntimePtyTransport', () => { ) }) - it('keeps the regular TUI surface and draft through stale-handle reconnect', async () => { + it('keeps the regular TUI and draft through inventory failure and stale-handle reconnect', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const onError = vi.fn() const onPtyExit = vi.fn() @@ -501,10 +504,13 @@ describe('createRemoteRuntimePtyTransport', () => { runtimeCall.mockImplementation(async (args: { method: string }) => { if (args.method === 'session.tabs.list') { hostListCalls += 1 + if (hostListCalls === 1) { + throw new Error('runtime reconnect in progress') + } const terminal = - hostListCalls === 1 + hostListCalls === 2 ? 'terminal-stale' - : hostListCalls === 2 + : hostListCalls === 3 ? null : 'terminal-reconnected' return { @@ -542,8 +548,9 @@ describe('createRemoteRuntimePtyTransport', () => { await vi.waitFor(() => expect(hostListCalls).toBeGreaterThanOrEqual(1)) expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-stale' }) expect(onPtyExit).not.toHaveBeenCalled() - await vi.waitFor(() => - expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-reconnected' }) + await vi.waitFor( + () => expect(latestSubscribePayload()).toMatchObject({ terminal: 'terminal-reconnected' }), + { timeout: 2_000 } ) const replacementStreamId = latestSubscribePayload().streamId emitSnapshot(replacementStreamId, draft) @@ -552,13 +559,16 @@ describe('createRemoteRuntimePtyTransport', () => { expect(onPtyExit).not.toHaveBeenCalled() expect(onPtySpawn).not.toHaveBeenCalled() expect(onPtyRebind).toHaveBeenCalledOnce() - expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-reconnected') + expect(onPtyRebind).toHaveBeenCalledWith( + 'remote:env-1@@terminal-reconnected', + 'remote:env-1@@terminal-stale' + ) expect(onExit).not.toHaveBeenCalled() expect(onDisconnect).not.toHaveBeenCalled() expect(transport.getPtyId()).toBe('remote:env-1@@terminal-reconnected') expect(transport.isConnected()).toBe(true) expect(renderedScreen.at(-1)).toBe(draft) - expect(hostListCalls).toBe(3) + expect(hostListCalls).toBe(4) const subscribedTerminals = subscriptionSendBinary.mock.calls .map((call) => decodeTerminalStreamFrame(call[0])) .flatMap((frame) => { @@ -571,6 +581,89 @@ describe('createRemoteRuntimePtyTransport', () => { expect(subscribedTerminals).toEqual(['terminal-stale', 'terminal-reconnected']) }) + it('keeps the mirror mounted when bounded replacement polling has no removal evidence', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'web-terminal-tab-1', + leafId: 'pane:1', + onPtyExit + }) + + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-stale', + cols: 80, + rows: 24, + callbacks: { onError } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + + let hostListCalls = 0 + runtimeCall.mockImplementation(async (args: { method: string }) => { + if (args.method !== 'session.tabs.list') { + return { ok: true, result: {} } + } + hostListCalls += 1 + return { + ok: true, + result: { + worktree: 'wt-1', + publicationEpoch: 'epoch-1', + snapshotVersion: hostListCalls + 1, + activeGroupId: null, + activeTabId: 'tab-1::pane:1', + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: 'tab-1::pane:1', + parentTabId: 'tab-1', + leafId: 'pane:1', + title: 'Claude Code', + isActive: true, + status: 'ready', + terminal: 'terminal-stale' + } + ] + } + } + }) + + subscriptionCallbacks?.onResponse({ + ok: true, + result: { + type: 'error', + streamId: latestSubscribePayload().streamId, + message: 'terminal_handle_stale' + } + }) + await vi.advanceTimersByTimeAsync(16_000) + + expect(hostListCalls).toBeGreaterThan(1) + expect(hostListCalls).toBeLessThan(25) + expect(onError).not.toHaveBeenCalled() + expect(onPtyExit).not.toHaveBeenCalled() + expect(transport.getPtyId()).toBe('remote:env-1@@terminal-stale') + expect(transport.isConnected()).toBe(true) + const subscribedTerminals = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .flatMap((frame) => { + if (frame?.opcode !== TerminalStreamOpcode.Subscribe) { + return [] + } + const payload = decodeTerminalStreamJson<{ terminal: string }>(frame.payload) + return payload ? [payload.terminal] : [] + }) + expect(subscribedTerminals).toEqual(['terminal-stale']) + } finally { + vi.useRealTimers() + } + }) + it('coalesces concurrent stale errors for the handle that was replaced', async () => { const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') const onPtyExit = vi.fn() diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 213eb368ba0..f96f7b1434e 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -205,47 +205,51 @@ export function createRemoteRuntimePtyTransport( return null } - async function listHostSessionHandle(hostTabId: string): Promise { - if (!worktreeId) { - return null - } - const listed = await callRuntime('session.tabs.list', { - worktree: toRuntimeWorktreeSelector(worktreeId) - }) - return findReadyHostSessionHandle(listed, hostTabId) - } - - async function waitForReplacementHostSessionHandle( + async function waitForResubscribeHostSessionHandle( hostTabId: string, - previousHandle: string - ): Promise { + previousHandle: string, + requireReplacement: boolean + ): Promise { if (!worktreeId) { return null } const worktree = toRuntimeWorktreeSelector(worktreeId) const startedAt = Date.now() let pollMs = HOST_SESSION_ATTACH_POLL_MS + let lastListError: unknown = null while (!destroyed && connected && handle === previousHandle) { - const listed = await callRuntime('session.tabs.list', { - worktree - }) - const nextHandle = findReadyHostSessionHandle(listed, hostTabId) - if (nextHandle && nextHandle !== previousHandle) { - return nextHandle - } - if (!hasHostSessionTerminalSurface(listed, hostTabId)) { - return null + try { + const listed = await callRuntime('session.tabs.list', { + worktree + }) + lastListError = null + const nextHandle = findReadyHostSessionHandle(listed, hostTabId) + if (nextHandle && (!requireReplacement || nextHandle !== previousHandle)) { + return nextHandle + } + if (!hasHostSessionTerminalSurface(listed, hostTabId)) { + return null + } + } catch (error) { + // Why: the session inventory can race the same runtime reconnect that + // invalidated the handle. Unknown liveness must not retire the pane. + lastListError = error } const remainingMs = HOST_SESSION_ATTACH_TIMEOUT_MS - (Date.now() - startedAt) if (remainingMs <= 0) { - return null + if (lastListError) { + throw lastListError + } + // Why: a bounded wait without removal evidence is unknown liveness; + // keep the mounted pane for an external snapshot to reattach later. + return undefined } // Why: a stale response can precede publication of its replacement. // Bounded backoff avoids retrying the known-stale handle in a hot loop. await new Promise((resolve) => setTimeout(resolve, Math.min(pollMs, remainingMs))) pollMs = Math.min(pollMs * 2, HOST_SESSION_REPLACEMENT_POLL_MAX_MS) } - return null + return undefined } async function attachHostSessionMirror( @@ -490,12 +494,17 @@ export function createRemoteRuntimePtyTransport( ): Promise { if (tabId && isWebTerminalSurfaceTabId(tabId)) { const hostTabId = toHostSessionTabId(tabId) - const nextHandle = requireReplacement - ? await waitForReplacementHostSessionHandle(hostTabId, previousHandle) - : await listHostSessionHandle(hostTabId) + const nextHandle = await waitForResubscribeHostSessionHandle( + hostTabId, + previousHandle, + requireReplacement + ) if (destroyed || !connected || handle !== previousHandle) { return } + if (nextHandle === undefined) { + return + } if (!nextHandle) { // Why: the host no longer publishes this surface; retire quietly and // let the next session-tabs snapshot drive respawn/removal. @@ -503,11 +512,14 @@ export function createRemoteRuntimePtyTransport( return } if (nextHandle !== previousHandle) { + const replacedPtyId = remotePtyId handle = nextHandle remotePtyId = toRemoteRuntimePtyId(nextHandle, currentRuntimeEnvironmentId) // Why: the host replaced an existing mirror identity; rebinding must not // apply fresh-spawn exit/agent seeding semantics to the mounted pane. - onPtyRebind?.(remotePtyId) + if (replacedPtyId) { + onPtyRebind?.(remotePtyId, replacedPtyId) + } } } await subscribeToHandle() diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index a0b992846e0..90bdd83d304 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -275,7 +275,7 @@ type UseTerminalPaneLifecycleDeps = { updateTabTitle: (tabId: string, title: string) => void setRuntimePaneTitle: (tabId: string, paneId: number, title: string) => void clearRuntimePaneTitle: (tabId: string, paneId: number) => void - updateTabPtyId: (tabId: string, ptyId: string) => void + updateTabPtyId: (tabId: string, ptyId: string, replacedPtyId?: string) => void markWorktreeUnread: (worktreeId: string) => void markTerminalTabUnread: (tabId: string) => void markTerminalPaneUnread: (paneKey: string) => void diff --git a/src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts b/src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts new file mode 100644 index 00000000000..302ca03b57d --- /dev/null +++ b/src/renderer/src/store/slices/terminal-pty-identity-replacement.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest' +import { createTestStore, makeTab, seedStore } from './store-test-helpers' + +describe('terminal PTY identity replacement', () => { + it('keeps repeated remote handle rotations bounded to the live identity', () => { + const store = createTestStore() + const worktreeId = 'repo1::/path/wt1' + const firstPtyId = 'remote:env-1@@terminal-1' + const secondPtyId = 'remote:env-1@@terminal-2' + const thirdPtyId = 'remote:env-1@@terminal-3' + seedStore(store, { + tabsByWorktree: { + [worktreeId]: [makeTab({ id: 'tab-1', worktreeId, ptyId: firstPtyId })] + }, + ptyIdsByTabId: { 'tab-1': [firstPtyId] } + }) + + store.getState().updateTabPtyId('tab-1', secondPtyId, firstPtyId) + store.getState().updateTabPtyId('tab-1', thirdPtyId, secondPtyId) + + const state = store.getState() + expect(state.ptyIdsByTabId['tab-1']).toEqual([thirdPtyId]) + expect(state.tabsByWorktree[worktreeId][0]?.ptyId).toBe(thirdPtyId) + expect(state.lastKnownRelayPtyIdByTabId['tab-1']).toBe(thirdPtyId) + }) +}) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index a64a9a2c9e2..368cb8ad120 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -648,7 +648,7 @@ export type TerminalSlice = { opts?: { recordInteraction?: boolean } ) => void setTabColor: (tabId: string, color: string | null) => void - updateTabPtyId: (tabId: string, ptyId: string) => void + updateTabPtyId: (tabId: string, ptyId: string, replacedPtyId?: string) => void clearTabPtyId: (tabId: string, ptyId?: string) => void shutdownWorktreeTerminals: ( worktreeId: string, @@ -2000,7 +2000,7 @@ export const createTerminalSlice: StateCreator } }, - updateTabPtyId: (tabId, ptyId) => { + updateTabPtyId: (tabId, ptyId, replacedPtyId) => { // Why: async spawn owners must perform provider teardown themselves, but // this final guard prevents any late caller from recreating retired tab maps. if (!isTerminalTabPresent(get(), tabId)) { @@ -2016,8 +2016,14 @@ export const createTerminalSlice: StateCreator const hasLegacyPtyBinding = legacyRemotePtyId ? existingPtyIds.includes(legacyRemotePtyId) : false - const nextPtyIds = hasLegacyPtyBinding - ? [...new Set(existingPtyIds.map((id) => (id === legacyRemotePtyId ? ptyId : id)))] + const replacementPtyId = + replacedPtyId && existingPtyIds.includes(replacedPtyId) + ? replacedPtyId + : hasLegacyPtyBinding + ? legacyRemotePtyId + : null + const nextPtyIds = replacementPtyId + ? [...new Set(existingPtyIds.map((id) => (id === replacementPtyId ? ptyId : id)))] : existingPtyIds.includes(ptyId) ? existingPtyIds : [...existingPtyIds, ptyId] @@ -2041,7 +2047,7 @@ export const createTerminalSlice: StateCreator // paths. In split panes, later pane spawns must not steal that // primary binding from the original pane or remount/close flows can // reattach the tab to the wrong PTY and appear to "reset" panes. - const currentTabPtyId = tab.ptyId === legacyRemotePtyId ? ptyId : tab.ptyId + const currentTabPtyId = tab.ptyId === replacementPtyId ? ptyId : tab.ptyId const nextTabPtyId = currentTabPtyId ?? nextPtyIds[0] ?? null const nextPendingActivationSpawn = consumePendingActivationSpawn(tab.pendingActivationSpawn) if (tab.pendingActivationSpawn || tab.ptyId !== nextTabPtyId) { @@ -2068,44 +2074,44 @@ export const createTerminalSlice: StateCreator const shouldBumpSortEpoch = isFirstPty && isActiveWorktree && !wasActivationSpawn const nextSuppressedPtyExitIds = { ...s.suppressedPtyExitIds } delete nextSuppressedPtyExitIds[ptyId] - if (legacyRemotePtyId) { - delete nextSuppressedPtyExitIds[legacyRemotePtyId] + if (replacementPtyId) { + delete nextSuppressedPtyExitIds[replacementPtyId] } - const hasLegacyPendingRestart = legacyRemotePtyId - ? legacyRemotePtyId in s.pendingCodexPaneRestartIds + const hasReplacementPendingRestart = replacementPtyId + ? replacementPtyId in s.pendingCodexPaneRestartIds : false - const hasLegacyRestartNotice = legacyRemotePtyId - ? legacyRemotePtyId in s.codexRestartNoticeByPtyId + const hasReplacementRestartNotice = replacementPtyId + ? replacementPtyId in s.codexRestartNoticeByPtyId : false - const hasLegacyMigrationUnsupported = legacyRemotePtyId - ? legacyRemotePtyId in s.migrationUnsupportedByPtyId + const hasReplacementMigrationUnsupported = replacementPtyId + ? replacementPtyId in s.migrationUnsupportedByPtyId : false - const nextPendingCodexPaneRestartIds = hasLegacyPendingRestart + const nextPendingCodexPaneRestartIds = hasReplacementPendingRestart ? { ...s.pendingCodexPaneRestartIds } : s.pendingCodexPaneRestartIds - const nextCodexRestartNoticeByPtyId = hasLegacyRestartNotice + const nextCodexRestartNoticeByPtyId = hasReplacementRestartNotice ? { ...s.codexRestartNoticeByPtyId } : s.codexRestartNoticeByPtyId - const nextMigrationUnsupportedByPtyId = hasLegacyMigrationUnsupported + const nextMigrationUnsupportedByPtyId = hasReplacementMigrationUnsupported ? { ...s.migrationUnsupportedByPtyId } : s.migrationUnsupportedByPtyId - if (legacyRemotePtyId) { - if (hasLegacyPendingRestart) { + if (replacementPtyId) { + if (hasReplacementPendingRestart) { nextPendingCodexPaneRestartIds[ptyId] = true - delete nextPendingCodexPaneRestartIds[legacyRemotePtyId] + delete nextPendingCodexPaneRestartIds[replacementPtyId] } - if (hasLegacyRestartNotice) { - const legacyNotice = nextCodexRestartNoticeByPtyId[legacyRemotePtyId] - nextCodexRestartNoticeByPtyId[ptyId] ??= legacyNotice - delete nextCodexRestartNoticeByPtyId[legacyRemotePtyId] + if (hasReplacementRestartNotice) { + const replacedNotice = nextCodexRestartNoticeByPtyId[replacementPtyId] + nextCodexRestartNoticeByPtyId[ptyId] ??= replacedNotice + delete nextCodexRestartNoticeByPtyId[replacementPtyId] } - if (hasLegacyMigrationUnsupported) { - const legacyMigrationUnsupported = nextMigrationUnsupportedByPtyId[legacyRemotePtyId] + if (hasReplacementMigrationUnsupported) { + const replacedMigrationUnsupported = nextMigrationUnsupportedByPtyId[replacementPtyId] nextMigrationUnsupportedByPtyId[ptyId] ??= { - ...legacyMigrationUnsupported, + ...replacedMigrationUnsupported, ptyId } - delete nextMigrationUnsupportedByPtyId[legacyRemotePtyId] + delete nextMigrationUnsupportedByPtyId[replacementPtyId] } } return {