From 325c71d80961c5be4b7aae1ccb908c2de612bb2a Mon Sep 17 00:00:00 2001 From: Brennan Benson Date: Sun, 12 Jul 2026 19:55:49 -0700 Subject: [PATCH] persist terminal teardown ownership across restarts --- config/reliability-gates.jsonc | 42 +- src/main/ipc/pty.test.ts | 171 +- src/main/ipc/pty.ts | 1826 ++++++++++------- src/main/ipc/runtime-environments.test.ts | 142 ++ src/main/ipc/runtime-environments.ts | 21 + .../ipc/runtime-terminal-close-retry-owner.ts | 226 ++ src/main/ipc/ssh.test.ts | 13 +- src/main/ipc/ssh.ts | 2 +- src/main/persistence.test.ts | 74 + src/main/persistence.ts | 141 +- src/main/runtime/rpc/methods/terminal.ts | 15 +- .../rpc/terminal-close-generation.test.ts | 49 + src/main/ssh/ssh-relay-session.test.ts | 6 +- src/main/ssh/ssh-relay-session.ts | 11 +- .../terminal-pane/pty-transport.test.ts | 9 +- .../src/lib/pty-kill-retry-ownership.test.ts | 45 +- .../src/lib/pty-kill-retry-ownership.ts | 107 +- ...ime-terminal-close-retry-ownership.test.ts | 22 +- .../runtime-terminal-close-retry-ownership.ts | 104 +- src/shared/constants.ts | 2 + src/shared/types.ts | 18 + 21 files changed, 1946 insertions(+), 1100 deletions(-) create mode 100644 src/main/ipc/runtime-terminal-close-retry-owner.ts create mode 100644 src/main/runtime/rpc/terminal-close-generation.test.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 27c20523aa9..ccf1195521a 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3765,13 +3765,13 @@ "providers": ["local", "daemon", "ssh", "remote-runtime"], "coveredPlatforms": ["macos"], "coveredProviders": ["local", "ssh"], - "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach, while pane identity protects legacy leases. Renderer retry ownership keeps one coalesced record per still-live unconfirmed PTY and uses one shared one-shot timer to retry every due owner with capped exponential backoff; it never drops a live owner merely to cap bookkeeping. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains create-first worktree creation plus headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", + "coverageNotes": "A macOS unit harness loads the installed patched node-pty WindowsTerminal prototype without constructing native state, proving pre-ready kill/destroy dispatch immediately. A relay artifact harness reproduces vanilla npm node-pty retaining one deferred kill with zero native close calls, then proves both Windows relay artifacts hash, ship, apply, include the local ConPTY console-list fallback, and idempotently verify the fix. Relay tests simulate win32 shutdown, repeated stale-spawn native failure, one-shot ownership, and retry after native failure without deleting the only handle before close is proven. Relay boot UUIDs scope recycled pty-N counters through initial spawn, durable-lease persistence, reattach, and SSH target-id migration; mismatched generations expire before raw attach, while pane identity protects legacy leases. Main-process retry owners persist local daemon and remote-runtime teardown intent across renderer and app restarts, coalesce by exact provider identity, use one shared one-shot timer per owner, and fence SSH relay plus remote-runtime generation reuse before close. Local Windows shutdown retains provider records and listeners after native throw unless onExit independently proves death. A disconnected SSH provider rejects shutdown without tombstoning its attachable lease. Project removal keeps local or remote ownership until provider-verified shutdown and successful VM cleanup; runtime repo removal drains create-first worktree creation plus headless, renderer-backed mobile, renderer split, and selector-less renderer terminal admissions before fresh ownership/liveness snapshots, stop, and metadata mutation. Desktop and duplicate-host repo removal route through the same transaction; legacy worktree creation, renderer PTY spawn, and every local/SSH/folder worktree removal outcome—including Windows stale registration and orphan-compatible fallback—are fenced and verified before destructive mutation or metadata purge. Hidden local/SSH and remote-runtime background-launch cleanup retain exact PTY/handle retry ownership with bounded one-shot scheduling. Targeted terminal.stop probes skip unrelated sessions. The same relay contract serves paired web/mobile clients; no live Windows ConPTY or Windows-over-SSH run is recorded yet.", "motivatingLinks": [ "https://github.com/stablyai/orca/issues/8260", "https://github.com/stablyai/orca/pull/8279" ], "invariant": "A Windows PTY closed before its first output executes exactly one no-argument node-pty shutdown immediately; Orca never deletes ownership without accepted native shutdown or an observed exit, and a later retry can target only the same relay generation and pane while project removal fences both worktree and terminal creation.", - "oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0, AttachConsole failure falls back to the shell pid, and the artifact version derives from all patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In local and relay providers, assert native throws retain exact ownership/listeners and retry succeeds, while a synchronous onExit is accepted as independent death proof; provider absence must retain the SSH lease and reject shutdown. Persist relay generation with every durable lease, expire mismatches before attach, and preserve generation through target migration. Reject 65 renderer kills, retain all 65 exact owners, then prove one one-shot deadline retries all due owners and later capped deadlines retry without another lifecycle event. Reject hidden local and remote-runtime background cleanup, retain exact tab/handle ownership, and prove later retry succeeds. Start worktree and terminal creation surfaces before whole-project and host-scoped project removal and assert removal performs no ownership/liveness snapshot until they publish; removal-first and failed/concurrent admissions must reject or release without deadlock. Start a real worktree removal, pause its preflight, and assert a late renderer terminal admission is rejected; assert every successful registered, orphaned, leftover, already-missing, and stale-registration removal path verifies shutdown before mutation or metadata purge; assert desktop repo removal, worktree create/remove, and PTY spawn handlers all enter their runtime-owned transactions before mutation or provider work.", + "oracle": "Load the installed patched WindowsTerminal prototype in a pre-ready state and assert kill and destroy each close the terminal and native agent once with zero deferred work, while signal arguments still throw. Build both Windows SSH relay artifacts, reproduce vanilla npm node-pty with close=0/native-kill=0/deferred=1, apply the shipped patch, and assert close=1/native-kill=1/deferred=0, AttachConsole failure falls back to the shell pid, and the artifact version derives from all patch bytes. Assert remote npm install executes patch enforcement before its runtime probe and never finalizes a failed patch. In local and relay providers, assert native throws retain exact ownership/listeners and retry succeeds, while a synchronous onExit is accepted as independent death proof; provider absence must retain the SSH lease and reject shutdown. Persist relay generation with every durable lease, expire mismatches before attach, and preserve generation through target migration. Reject local daemon and remote-runtime teardown, reload the durable store, replace the startup provider, and prove capped retries drain exact identities with zero final retry timers. Replace SSH relay and remote-runtime generations while a retry is retained and prove the new owner is never closed. Start worktree and terminal creation surfaces before whole-project and host-scoped project removal and assert removal performs no ownership/liveness snapshot until they publish; removal-first and failed/concurrent admissions must reject or release without deadlock. Start a real worktree removal, pause its preflight, and assert a late renderer terminal admission is rejected; assert every successful registered, orphaned, leftover, already-missing, and stale-registration removal path verifies shutdown before mutation or metadata purge; assert desktop repo removal, worktree create/remove, and PTY spawn handlers all enter their runtime-owned transactions before mutation or provider work.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts config/scripts/build-relay-windows-node-pty-patch.test.mjs src/main/ssh/ssh-relay-native-deps-install.test.ts src/relay/node-pty-windows-pre-ready-shutdown.test.ts src/relay/pty-handler.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/store/slices/repos-remove-project-purge-leak.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-runtime-vm-removal.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts", @@ -3781,7 +3781,7 @@ "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/shared/ssh-pty-id.test.ts src/shared/ssh-types.test.ts src/main/providers/local-pty-provider.test.ts src/main/providers/ssh-pty-provider.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ssh/ssh-relay-generation-reattach.test.ts src/main/ssh/ssh-target-id-migration.test.ts src/relay/pty-handler.test.ts src/relay/subprocess.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/lib/pty-kill-retry-ownership.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/lib/ephemeral-vm-runtime-cleanup.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/store/slices/repos.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/pty.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/ipc/repos-create.test.ts src/main/window/attach-main-window-services.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/worktrees-windows.test.ts src/main/ipc/pty.test.ts src/main/runtime/orca-runtime.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts src/renderer/src/store/slices/repos-host-identity-routing.test.ts", - "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/renderer/src/store/slices/worktrees.test.ts" + "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts" ], "testFiles": [ "config/scripts/build-relay-windows-node-pty-patch.test.mjs", @@ -3813,7 +3813,10 @@ "src/renderer/src/lib/launch-agent-background-session.test.ts", "src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts", "src/renderer/src/store/slices/repos-host-identity-routing.test.ts", - "src/renderer/src/store/slices/worktrees.test.ts" + "src/renderer/src/store/slices/worktrees.test.ts", + "src/main/ipc/runtime-environments.test.ts", + "src/main/runtime/rpc/terminal-close-generation.test.ts", + "src/main/ipc/ssh.test.ts" ], "assertionRefs": [ { @@ -3849,7 +3852,7 @@ { "file": "src/renderer/src/components/terminal-pane/pty-transport.test.ts", "assertions": [ - "a rejected teardown kill retains exact PTY identity and retries on the next lifecycle event" + "a rejected teardown kill forwards exact PTY identity to main and later renderer lifecycle does not duplicate the retry" ] }, { @@ -3945,9 +3948,8 @@ { "file": "src/renderer/src/lib/pty-kill-retry-ownership.test.ts", "assertions": [ - "all 65 unconfirmed live shutdown ids retain exact ownership and one deadline retries every due owner", - "a second failure retries after capped backoff without another lifecycle event", - "successful SSH target removal releases only that target and cancels the final retry timer" + "renderer teardown forwards exact PTY identity to the main-process retry owner", + "renderer lifecycle hooks no longer own retry timers that disappear on reload" ] }, { @@ -3957,7 +3959,9 @@ "renderer PTY spawn enters runtime terminal admission before provider work", "renderer PTY spawn qualifies local and SSH admissions with the owning execution host", "disconnected SSH kill and stop retain the lease and reject without a synthetic exit", - "provider reconnect retries durable shutdown intent once with its generation and pane identity" + "provider reconnect retries durable shutdown intent with its generation and pane identity", + "persisted local shutdown rebinds from the startup fallback to the daemon provider and drains all retry timers", + "relay generation reuse removes stale retry ownership without touching the new PTY" ] }, { @@ -3991,8 +3995,20 @@ { "file": "src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts", "assertions": [ - "a rejected remote-runtime terminal close retains environment and handle identity through repeated automatic backoff retry", - "successful runtime removal releases every environment handle and cancels its retry timer" + "renderer remote-runtime teardown forwards environment and handle identity to main without renderer-owned timers" + ] + }, + { + "file": "src/main/ipc/runtime-environments.test.ts", + "assertions": [ + "persisted remote-runtime close intent hydrates after app restart and drains after bounded retry", + "replacement runtime generation clears stale intent without sending terminal.close" + ] + }, + { + "file": "src/main/runtime/rpc/terminal-close-generation.test.ts", + "assertions": [ + "runtime atomically rejects a terminal close fenced to an older runtime generation" ] }, { @@ -4122,7 +4138,7 @@ "date": "2026-07-12", "runner": "local", "platform": "macos", - "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/renderer/src/store/slices/worktrees.test.ts", + "command": "pnpm exec vitest run --config config/vitest.config.ts src/main/runtime/orca-runtime.test.ts src/main/ipc/worktrees.test.ts src/main/ipc/pty.test.ts src/main/ipc/runtime-environments.test.ts src/main/runtime/rpc/terminal-close-generation.test.ts src/main/persistence.test.ts src/main/ssh/ssh-relay-session.test.ts src/main/ipc/ssh.test.ts src/renderer/src/store/slices/worktrees.test.ts", "result": "passed", "durationSeconds": 10, "summary": "1356 exact-head tests proved duplicate local/SSH IDs isolate create, removal, renderer spawn, and PTY liveness ownership while whole-project removal drains every host scope." @@ -4142,7 +4158,7 @@ }, "performanceBudget": { "required": true, - "evidence": "Shutdown removes indefinite deferred retention and preserves at most one in-flight native kill plus one existing graceful timer per PTY request. Failed renderer kills coalesce by exact live PTY id, and failed remote-runtime closes coalesce by environment plus handle: distinct records correspond one-for-one with still-unconfirmed provider resources and are never discarded before accepted shutdown. Each retry map owns one shared one-shot timer for its earliest deadline, retries all due owners, caps backoff at 30 seconds, and logs only attempts 1, 2, and 8 to prevent outage backpressure. Successful SSH-target and runtime-environment removal release the unreachable target's records and cancel the last timer; offline SSH shutdown intent also moves onto the durable lease so renderer reload cannot lose it, and provider reconnect performs one identity-qualified retry. Pre-handler exits remain capped at 64. Worktree-create, worktree-removal, whole-project, and host-scoped admissions retain one promise only per active operation; removal drains only the target worktree's current admissions. Targeted runtime stop still performs one provider verification per target PTY plus two liveness snapshots, while foreground probes are limited to the target worktree." + "evidence": "Shutdown removes indefinite deferred retention and preserves at most one in-flight native kill plus one existing graceful timer per PTY request. Failed local and SSH kills coalesce in main by exact live PTY id, while failed remote-runtime closes coalesce by environment, runtime generation, and handle: distinct durable records correspond one-for-one with still-unconfirmed provider resources and survive renderer/app restart. Each main-owned retry map owns one shared one-shot timer for its earliest deadline, retries all due owners, caps backoff at 30 seconds, and logs only attempts 1, 2, and 8 to prevent outage backpressure. Successful SSH-target and runtime-environment removal release unreachable records and cancel the last retry timer; provider replacement rebinds the same generation but relay/runtime generation replacement drops stale intent before touching a reused id. Remote-runtime close adds one generation status probe per teardown attempt and is not on input, output, render, polling, or startup-visible paths. Pre-handler exits remain capped at 64. Worktree-create, worktree-removal, whole-project, and host-scoped admissions retain one promise only per active operation; removal drains only the target worktree's current admissions. Targeted runtime stop still performs one provider verification per target PTY plus two liveness snapshots, while foreground probes are limited to the target worktree." }, "promotionCriteria": [ "Run the deterministic command across the required CI platforms without flakes.", diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index b90918dcada..568572a595c 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -3028,7 +3028,11 @@ describe('registerPtyHandlers', () => { expect(shutdown).toHaveBeenCalledWith('ssh:ssh-1@@relay-pty', { immediate: false }) expect(localShutdown).not.toHaveBeenCalled() - expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'relay-pty', 'terminated') + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-1', + 'ssh:ssh-1@@relay-pty', + 'terminated' + ) }) it('runtime controller kill retains app-scoped SSH ids when the provider is absent', async () => { @@ -3351,12 +3355,18 @@ describe('registerPtyHandlers', () => { deletePtyOwnership('missing-pty') }) - it('rethrows non-not-found local provider shutdown failures', async () => { + it('persists local daemon shutdown and retries after renderer loss', async () => { + vi.useFakeTimers() + const shutdown = vi + .fn() + .mockRejectedValueOnce(new Error('daemon unavailable')) + .mockRejectedValueOnce(new Error('daemon still unavailable')) + .mockResolvedValue(undefined) setLocalPtyProvider({ spawn: vi.fn(), write: vi.fn(), resize: vi.fn(), - shutdown: vi.fn().mockRejectedValue(new Error('daemon unavailable')), + shutdown, sendSignal: vi.fn(), getCwd: vi.fn(), getInitialCwd: vi.fn(), @@ -3374,12 +3384,79 @@ describe('registerPtyHandlers', () => { getDefaultShell: vi.fn(), getProfiles: vi.fn() } as never) + const store = { + getPendingLocalPtyShutdowns: vi.fn(() => []), + upsertPendingLocalPtyShutdown: vi.fn(), + removePendingLocalPtyShutdown: vi.fn() + } + vi.spyOn(console, 'warn').mockImplementation(() => {}) handlers.clear() - registerPtyHandlers(mainWindow as never) - - await expect(handlers.get('pty:kill')!(null, { id: 'local-pty' })).rejects.toThrow( - 'daemon unavailable' + registerPtyHandlers( + mainWindow as never, + undefined, + undefined, + undefined, + undefined, + store as never ) + + await expect( + handlers.get('pty:kill')!(null, { + id: 'local-pty', + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab' + }) + ).rejects.toThrow('daemon unavailable') + await vi.advanceTimersByTimeAsync(250) + await vi.advanceTimersByTimeAsync(500) + + expect(shutdown).toHaveBeenCalledTimes(3) + expect(store.upsertPendingLocalPtyShutdown).toHaveBeenCalledWith({ + ptyId: 'local-pty', + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab', + requestedAt: expect.any(Number) + }) + expect(store.removePendingLocalPtyShutdown).toHaveBeenCalledWith('local-pty') + await vi.advanceTimersByTimeAsync(30_000) + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + + it('rebinds a persisted local shutdown after async daemon startup replaces the provider', async () => { + vi.useFakeTimers() + const makeProvider = (shutdown: ReturnType) => + ({ + spawn: vi.fn(), + write: vi.fn(), + resize: vi.fn(), + shutdown, + sendSignal: vi.fn(), + onData: vi.fn(() => () => {}), + onReplay: vi.fn(() => () => {}), + onExit: vi.fn(() => () => {}) + }) as never + const fallbackShutdown = vi.fn().mockRejectedValue(new Error('temporary provider')) + const daemonShutdown = vi.fn().mockResolvedValue(undefined) + const removePendingLocalPtyShutdown = vi.fn() + setLocalPtyProvider(makeProvider(fallbackShutdown)) + handlers.clear() + registerPtyHandlers(mainWindow as never, undefined, undefined, undefined, undefined, { + getPendingLocalPtyShutdowns: () => [{ ptyId: 'daemon-pty', requestedAt: 1 }], + upsertPendingLocalPtyShutdown: vi.fn(), + removePendingLocalPtyShutdown + } as never) + await vi.advanceTimersByTimeAsync(0) + + setLocalPtyProvider(makeProvider(daemonShutdown)) + await vi.advanceTimersByTimeAsync(0) + + expect(fallbackShutdown).toHaveBeenCalledOnce() + expect(daemonShutdown).toHaveBeenCalledOnce() + expect(removePendingLocalPtyShutdown).toHaveBeenCalledWith('daemon-pty') + await vi.advanceTimersByTimeAsync(30_000) + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() }) it('synthesizes runtime exit after ordinary daemon-backed pty kill', async () => { @@ -4049,7 +4126,11 @@ describe('registerPtyHandlers', () => { expectedTabId: 'tab-a' }) expect(localShutdown).not.toHaveBeenCalled() - expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'relay-pty', 'terminated') + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-1', + 'ssh:ssh-1@@relay-pty', + 'terminated' + ) }) it('rejects app-scoped SSH PTY kill without tombstoning when the provider is absent', async () => { @@ -4089,9 +4170,9 @@ describe('registerPtyHandlers', () => { store as never ) - await expect( - handlers.get('pty:kill')!(null, { id: 'ssh:ssh-1@@relay-pty' }) - ).rejects.toThrow('SSH PTY provider unavailable') + await expect(handlers.get('pty:kill')!(null, { id: 'ssh:ssh-1@@relay-pty' })).rejects.toThrow( + 'SSH PTY provider unavailable' + ) expect(localShutdown).not.toHaveBeenCalled() expect(store.markSshRemotePtyLease).not.toHaveBeenCalled() @@ -4101,7 +4182,8 @@ describe('registerPtyHandlers', () => { ) }) - it('retries durable SSH shutdown intent when the provider reconnects', async () => { + it('retries durable SSH shutdown through repeated reconnect failure then clears timers', async () => { + vi.useFakeTimers() const lease = { targetId: 'ssh-1', ptyId: 'relay-pty', @@ -4132,24 +4214,73 @@ describe('registerPtyHandlers', () => { 'SSH PTY provider unavailable' ) - const shutdown = vi.fn().mockResolvedValue(undefined) + const shutdown = vi + .fn() + .mockRejectedValueOnce(new Error('offline')) + .mockRejectedValueOnce(new Error('still offline')) + .mockResolvedValue(undefined) + vi.spyOn(console, 'warn').mockImplementation(() => {}) registerSshPtyProvider('ssh-1', { shutdown, onExit: vi.fn(() => () => {}) } as never) - await vi.waitFor(() => expect(store.markSshRemotePtyLease).toHaveBeenCalledOnce()) + await vi.advanceTimersByTimeAsync(250) + await vi.advanceTimersByTimeAsync(500) - expect(shutdown).toHaveBeenCalledWith(id, { + expect(shutdown).toHaveBeenCalledTimes(3) + expect(shutdown).toHaveBeenLastCalledWith(id, { immediate: true, expectedPaneKey: 'tab-a:11111111-1111-4111-8111-111111111111', expectedTabId: 'tab-a' }) - expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( - 'ssh-1', - 'relay-pty', - 'terminated' - ) + expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', id, 'terminated') unregisterSshPtyProvider('ssh-1') + await vi.advanceTimersByTimeAsync(30_000) + expect(vi.getTimerCount()).toBe(0) + }) + + it('drops a retained SSH retry when a new relay generation reuses the raw id', async () => { + vi.useFakeTimers() + const lease: Record = { + targetId: 'ssh-1', + ptyId: 'relay-pty', + relayInstanceId: 'boot-a', + state: 'detached', + shutdownRequestedAt: 2, + createdAt: 1, + updatedAt: 2 + } + const store = { getSshRemotePtyLeases: vi.fn(() => [lease]) } + handlers.clear() + registerPtyHandlers( + mainWindow as never, + undefined, + undefined, + undefined, + undefined, + store as never + ) + const oldShutdown = vi.fn().mockRejectedValue(new Error('relay replaced')) + registerSshPtyProvider('ssh-1', { + shutdown: oldShutdown, + onExit: vi.fn(() => () => {}) + } as never) + await vi.advanceTimersByTimeAsync(0) + + Object.assign(lease, { relayInstanceId: 'boot-b', state: 'attached' }) + delete lease.shutdownRequestedAt + const newShutdown = vi.fn() + registerSshPtyProvider('ssh-1', { + shutdown: newShutdown, + onExit: vi.fn(() => () => {}) + } as never) + await vi.advanceTimersByTimeAsync(30_000) + + expect(oldShutdown).toHaveBeenCalledOnce() + expect(newShutdown).not.toHaveBeenCalled() + expect(vi.getTimerCount()).toBe(0) + unregisterSshPtyProvider('ssh-1') + vi.useRealTimers() }) it('ignores fire-and-forget IPC for detached SSH PTYs without a provider', async () => { diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 98ea8686cb7..bb453dccee6 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -173,7 +173,21 @@ type FreshLocalFallbackProvider = IPtyProvider & { } const sshProviders = new Map() let activePtyStore: Store | null = null -const pendingSshShutdownRetries = new Map>() +let notifyRetainedPtyExit: ((id: string) => void) | null = null +type RetainedPtyShutdown = { + id: string + connectionId: string | null + options: PtyShutdownOptions + provider: IPtyProvider | null + store: Store | null + attempts: number + nextRetryAt: number + inFlight: Promise | null +} +const pendingSshShutdownRetries = new Map() +const pendingLocalShutdownRetries = new Map() +let retainedShutdownRetryTimer: ReturnType | null = null +const MAX_RETAINED_SHUTDOWN_BACKOFF_MS = 30_000 const SYNTHETIC_KILL_EXIT_DUPLICATE_WINDOW_MS = 30_000 // Why: producer flow control changes terminal physics — a flooding shell now // blocks on write instead of buffering in main. Kill switch: flip this one @@ -548,12 +562,141 @@ function finishPtyShutdown( ): void { clearProviderPtyState(id) if (connectionId) { - store?.markSshRemotePtyLease(connectionId, getRelayPtyId(connectionId, id), 'terminated') + store?.markSshRemotePtyLease(connectionId, id, 'terminated') } ptyOwnership.delete(id) markClaudePtyExited(id) } +function scheduleRetainedShutdownRetries(): void { + if (retainedShutdownRetryTimer) { + clearTimeout(retainedShutdownRetryTimer) + } + retainedShutdownRetryTimer = null + let nextRetryAt = Number.POSITIVE_INFINITY + const retainedShutdowns = [ + ...pendingLocalShutdownRetries.values(), + ...pendingSshShutdownRetries.values() + ] + for (const retained of retainedShutdowns) { + if (!retained.inFlight) { + nextRetryAt = Math.min(nextRetryAt, retained.nextRetryAt) + } + } + if (!Number.isFinite(nextRetryAt)) { + return + } + retainedShutdownRetryTimer = setTimeout( + () => { + retainedShutdownRetryTimer = null + const now = Date.now() + // Why: target removal can invalidate owners while this timer waits; only live map entries may retry. + for (const retained of [ + ...pendingLocalShutdownRetries.values(), + ...pendingSshShutdownRetries.values() + ]) { + if (!retained.inFlight && retained.nextRetryAt <= now) { + void attemptRetainedPtyShutdown(retained) + } + } + }, + Math.max(0, nextRetryAt - Date.now()) + ) +} + +function completeRetainedPtyShutdown( + retained: RetainedPtyShutdown, + providerExitObserved: boolean +): void { + const ownerMap = retained.connectionId ? pendingSshShutdownRetries : pendingLocalShutdownRetries + if (ownerMap.get(retained.id) !== retained) { + return + } + ownerMap.delete(retained.id) + finishPtyShutdown(retained.id, retained.connectionId, retained.store ?? undefined) + if (!retained.connectionId) { + retained.store?.removePendingLocalPtyShutdown?.(retained.id) + } + if (!providerExitObserved) { + notifyRetainedPtyExit?.(retained.id) + } +} + +function attemptRetainedPtyShutdown(retained: RetainedPtyShutdown): Promise { + if (retained.inFlight) { + return retained.inFlight + } + const currentProvider = retained.connectionId + ? sshProviders.get(retained.connectionId) + : localProvider + if (!currentProvider || (retained.provider && retained.provider !== currentProvider)) { + retained.nextRetryAt = Number.POSITIVE_INFINITY + scheduleRetainedShutdownRetries() + return Promise.resolve() + } + retained.provider = currentProvider + const attempt = shutdownProviderAndDetectExit(currentProvider, retained.id, retained.options) + .then((providerExitObserved) => completeRetainedPtyShutdown(retained, providerExitObserved)) + .catch((error: unknown) => { + const ownerMap = retained.connectionId + ? pendingSshShutdownRetries + : pendingLocalShutdownRetries + if (ownerMap.get(retained.id) !== retained) { + return + } + if (isPtyAlreadyGoneError(error)) { + completeRetainedPtyShutdown(retained, false) + return + } + retained.attempts += 1 + retained.nextRetryAt = + Date.now() + + Math.min(MAX_RETAINED_SHUTDOWN_BACKOFF_MS, 250 * 2 ** Math.min(retained.attempts - 1, 7)) + if (retained.attempts <= 2 || retained.attempts === 8) { + console.warn('[pty] retained shutdown retry failed:', error) + } + }) + .finally(() => { + if (retained.inFlight === attempt) { + retained.inFlight = null + } + scheduleRetainedShutdownRetries() + }) + retained.inFlight = attempt + return attempt +} + +function retainLocalPtyShutdown( + id: string, + options: PtyShutdownOptions, + attempts = 0 +): RetainedPtyShutdown { + let retained = pendingLocalShutdownRetries.get(id) + if (!retained) { + retained = { + id, + connectionId: null, + options, + provider: null, + store: activePtyStore, + attempts, + nextRetryAt: attempts === 0 ? 0 : Date.now() + 250, + inFlight: null + } + pendingLocalShutdownRetries.set(id, retained) + } + if (typeof activePtyStore?.upsertPendingLocalPtyShutdown === 'function') { + activePtyStore.upsertPendingLocalPtyShutdown({ + ptyId: id, + ...(options.expectedPaneKey ? { expectedPaneKey: options.expectedPaneKey } : {}), + ...(options.expectedTabId ? { expectedTabId: options.expectedTabId } : {}), + requestedAt: Date.now() + }) + } + scheduleRetainedShutdownRetries() + return retained +} + // ─── Host PTY env assembly ────────────────────────────────────────── // Why: both the LocalPtyProvider.buildSpawnEnv closure and the daemon-active // fallback in pty:spawn need the same set of host-local env injections @@ -1080,14 +1223,21 @@ export function registerSshPtyProvider(connectionId: string, provider: IPtyProvi /** Remove an SSH PTY provider when a connection is closed. */ export function unregisterSshPtyProvider(connectionId: string): void { sshProviders.delete(connectionId) -} - -export function releasePendingSshShutdownsForTarget(connectionId: string): void { - for (const id of pendingSshShutdownRetries.keys()) { - if (parseAppSshPtyId(id)?.connectionId === connectionId) { + for (const [id, retained] of pendingSshShutdownRetries) { + if (retained.connectionId === connectionId) { pendingSshShutdownRetries.delete(id) } } + scheduleRetainedShutdownRetries() +} + +export function releasePendingSshShutdownsForTarget(connectionId: string): void { + for (const [id, retained] of pendingSshShutdownRetries) { + if (retained.connectionId === connectionId) { + pendingSshShutdownRetries.delete(id) + } + } + scheduleRetainedShutdownRetries() } function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider): void { @@ -1095,32 +1245,51 @@ function retryPersistedSshShutdowns(connectionId: string, provider: IPtyProvider if (!store || typeof store.getSshRemotePtyLeases !== 'function') { return } - for (const lease of store.getSshRemotePtyLeases(connectionId)) { - if (!lease.shutdownRequestedAt || lease.state === 'terminated' || lease.state === 'expired') { - continue + const retainedLeases = store + .getSshRemotePtyLeases(connectionId) + .filter( + (lease) => + lease.shutdownRequestedAt && lease.state !== 'terminated' && lease.state !== 'expired' + ) + const retainedIds = new Set( + retainedLeases.map((lease) => toAppSshPtyId(connectionId, lease.ptyId, lease.relayInstanceId)) + ) + for (const [id, retained] of pendingSshShutdownRetries) { + if (retained.connectionId === connectionId && !retainedIds.has(id)) { + pendingSshShutdownRetries.delete(id) } + } + for (const lease of retainedLeases) { const id = toAppSshPtyId(connectionId, lease.ptyId, lease.relayInstanceId) - if (pendingSshShutdownRetries.has(id)) { - continue - } - const attempt = shutdownProviderAndDetectExit(provider, id, { + const options: PtyShutdownOptions = { immediate: true, ...(lease.tabId ? { expectedTabId: lease.tabId } : {}), ...(lease.tabId && lease.leafId ? { expectedPaneKey: makePaneKey(lease.tabId, lease.leafId) } : {}) - }) - .then(() => finishPtyShutdown(id, connectionId, store)) - .catch((error: unknown) => { - console.warn('[pty] retained SSH shutdown retry failed after reconnect:', error) - }) - .finally(() => { - if (pendingSshShutdownRetries.get(id) === attempt) { - pendingSshShutdownRetries.delete(id) - } - }) - pendingSshShutdownRetries.set(id, attempt) + } + let retained = pendingSshShutdownRetries.get(id) + if (!retained) { + retained = { + id, + connectionId, + provider, + store, + attempts: 0, + nextRetryAt: 0, + inFlight: null, + options + } + pendingSshShutdownRetries.set(id, retained) + } else if (retained.provider !== provider) { + // Why: reconnect replaces the transport owner for the same generation-qualified id. + retained.provider = provider + retained.options = options + retained.nextRetryAt = 0 + } + void attemptRetainedPtyShutdown(retained) } + scheduleRetainedShutdownRetries() } /** Get the SSH PTY provider for a connection (for dispose on cleanup). */ @@ -1140,10 +1309,20 @@ export function getLocalPtyProvider(): IPtyProvider { return localProvider } -/** Replace the local PTY provider with a daemon-backed one. - * Call before registerPtyHandlers so the IPC layer routes through the daemon. */ +/** Replace the local PTY provider with a daemon-backed one and rebind durable shutdowns. */ export function setLocalPtyProvider(provider: IPtyProvider): void { localProvider = provider + // Why: durable daemon shutdowns hydrate before async daemon startup swaps in + // the routed provider. Re-arm them against that owner instead of fencing them + // forever to the temporary in-process fallback. + for (const retained of pendingLocalShutdownRetries.values()) { + retained.provider = null + retained.nextRetryAt = 0 + if (!retained.inFlight) { + void attemptRetainedPtyShutdown(retained) + } + } + scheduleRetainedShutdownRetries() } /** Get all PTY IDs owned by a given connectionId (for reconnection reattach). */ @@ -1538,6 +1717,23 @@ export function registerPtyHandlers( } ): void { activePtyStore = store ?? null + notifyRetainedPtyExit = (id) => { + runtime?.onPtyExit(id, -1) + rememberSyntheticKillExit(id) + sendPtyExitToRenderer({ id, code: -1 }) + } + const persistedLocalShutdowns = + store && typeof store.getPendingLocalPtyShutdowns === 'function' + ? store.getPendingLocalPtyShutdowns() + : [] + for (const request of persistedLocalShutdowns) { + const retained = retainLocalPtyShutdown(request.ptyId, { + immediate: true, + ...(request.expectedPaneKey ? { expectedPaneKey: request.expectedPaneKey } : {}), + ...(request.expectedTabId ? { expectedTabId: request.expectedTabId } : {}) + }) + void attemptRetainedPtyShutdown(retained) + } for (const [connectionId, provider] of sshProviders) { retryPersistedSshShutdowns(connectionId, provider) } @@ -3682,783 +3878,807 @@ export function registerPtyHandlers( } } ) => - withTerminalCreateAdmission(args.worktreeId, async () => { - const spawnTiming = createPtySpawnTiming() - const startupPromise = getLocalPtyStartupPromise(args.connectionId) - if (startupPromise) { - await startupPromise - } - await assertFolderWorkspacePtyPathUsable(args.worktreeId) - // Why: honor the fallback only for fresh local spawns even if a caller - // sends the flag — reattach must keep the session's exact cwd and - // remote/SSH paths cannot probe the local filesystem meaningfully. - const allowMissingCwdFallback = - !args.connectionId && !args.sessionId && args.cwdFallback === 'worktree' - let didFallbackToWorkspaceRootCwd = false - const cwd = resolvePtySpawnStartupCwd( + withTerminalCreateAdmission( args.worktreeId, - args.cwd, - allowMissingCwdFallback - ? { - directoryExists: localStartupCwdDirectoryExists, - onFallbackToWorkspaceRoot: () => { - didFallbackToWorkspaceRootCwd = true - } - } - : undefined - ) - const startupCwdFallback = - didFallbackToWorkspaceRootCwd && cwd ? ({ kind: 'worktree', cwd } as const) : undefined - spawnTiming.mark('preflight') - const provider = getProvider(args.connectionId) - const isClaudeLaunch = !args.connectionId && isClaudeLaunchCommand(args.command) - if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { - throw new Error('A Claude account switch is in progress. Try again after it finishes.') - } - const terminalRuntimeOptions = - process.platform === 'win32' && !args.connectionId - ? resolveLocalWindowsTerminalRuntimeOptions({ - requestedShellOverride: args.shellOverride, - settings: getSettings?.(), - projectRuntime: args.projectRuntime, - fallbackHostShell: process.env.COMSPEC || 'powershell.exe' + async () => { + const spawnTiming = createPtySpawnTiming() + const startupPromise = getLocalPtyStartupPromise(args.connectionId) + if (startupPromise) { + await startupPromise + } + await assertFolderWorkspacePtyPathUsable(args.worktreeId) + // Why: honor the fallback only for fresh local spawns even if a caller + // sends the flag — reattach must keep the session's exact cwd and + // remote/SSH paths cannot probe the local filesystem meaningfully. + const allowMissingCwdFallback = + !args.connectionId && !args.sessionId && args.cwdFallback === 'worktree' + let didFallbackToWorkspaceRootCwd = false + const cwd = resolvePtySpawnStartupCwd( + args.worktreeId, + args.cwd, + allowMissingCwdFallback + ? { + directoryExists: localStartupCwdDirectoryExists, + onFallbackToWorkspaceRoot: () => { + didFallbackToWorkspaceRootCwd = true + } + } + : undefined + ) + const startupCwdFallback = + didFallbackToWorkspaceRootCwd && cwd ? ({ kind: 'worktree', cwd } as const) : undefined + spawnTiming.mark('preflight') + const provider = getProvider(args.connectionId) + const isClaudeLaunch = !args.connectionId && isClaudeLaunchCommand(args.command) + if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { + throw new Error('A Claude account switch is in progress. Try again after it finishes.') + } + const terminalRuntimeOptions = + process.platform === 'win32' && !args.connectionId + ? resolveLocalWindowsTerminalRuntimeOptions({ + requestedShellOverride: args.shellOverride, + settings: getSettings?.(), + projectRuntime: args.projectRuntime, + fallbackHostShell: process.env.COMSPEC || 'powershell.exe' + }) + : { shellOverride: args.shellOverride, terminalWindowsWslDistro: null } + const initialShellOverride = terminalRuntimeOptions.shellOverride + const initialSelectionTarget = getCodexSelectionTargetForPty( + initialShellOverride, + cwd, + terminalRuntimeOptions.terminalWindowsWslDistro ?? null + ) + const claudeAuth = + isClaudeLaunch && prepareClaudeAuth + ? await prepareClaudeAuth(initialSelectionTarget) + : null + spawnTiming.mark('auth') + if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { + throw new Error('A Claude account switch is in progress. Try again after it finishes.') + } + if (claudeAuth?.stripAuthEnv && hasClaudeAuthEnvConflict(args.env)) { + throw new Error( + 'This Claude launch defines explicit Anthropic auth environment variables. Remove those overrides before using a managed Claude account.' + ) + } + // Why: the daemon-backed provider replaces LocalPtyProvider and therefore + // never runs its buildSpawnEnv closure. We must assemble the same + // host-local env (OpenCode plugin, agent-hook server, Pi/OMP managed + // extensions, Codex home, dev CLI overrides, GitHub attribution shims) + // here so both spawn paths behave identically. buildPtyHostEnv is the + // shared helper that encapsulates the full set of injections and guards. + // + // Safety: skip the entire injection when a remote (SSH) connection is in + // play. Every injection here is either host-loopback (the agent-hook + // server binds 127.0.0.1, so shipping its token to an SSH host would + // leak a loopback secret for no functional benefit) or a path on the + // local filesystem (OpenCode plugin dir, Pi/OMP extension paths, Codex + // home, dev CLI bin, attribution shim dir) that would resolve to + // nothing — or something misleading — on the remote machine. + const isDaemonHostSpawn = + !args.connectionId && + !(provider instanceof LocalPtyProvider) && + !routesFreshSpawnsToLocalProvider(provider) + // Why: daemon host-env setup needs a stable id BEFORE provider.spawn so + // provider hooks and legacy Pi overlay cleanup can run in buildPtyHostEnv. + // DaemonPtyAdapter.doSpawn mints an id the same way when sessionId is + // absent — lifting the mint here gives pty.ts the id up-front without + // changing daemon semantics (the daemon still honors opts.sessionId ?? mint()). + // + // Note: the sessionId is STABLE across daemon restarts by design — + // DaemonPtyAdapter.reconcileOnStartup reuses it so that users' live + // shells survive crashes. Do NOT "simplify" id allocation back to a + // fresh UUID per spawn; that would orphan reconnectable terminal state. + // Why: only state for ids we minted in THIS request should be cleared on + // spawn failure. If the caller supplied args.sessionId it may refer to + // an existing PTY whose state (OpenCode hooks, legacy Pi overlay cleanup, + // agent-hook pane caches) we must not clobber on a retry/attach failure. + const isMintedSessionId = args.sessionId === undefined && isDaemonHostSpawn + const effectiveSessionId = + args.sessionId ?? (isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined) + const effectiveSessionAppId = + effectiveSessionId !== undefined + ? getAppPtyId(args.connectionId, effectiveSessionId) + : undefined + const effectiveSessionRelayId = + effectiveSessionId !== undefined + ? getRelayPtyId(args.connectionId, effectiveSessionId) + : undefined + const startupTerminalColorQueryReplyColors = getStartupTerminalColorQueryReplyColors(args) + const preSpawnStartupTerminalColorReplyPtyId = + startupTerminalColorQueryReplyColors && effectiveSessionId !== undefined + ? (effectiveSessionAppId ?? effectiveSessionId) + : null + // Why: the renderer sets pane env for SSH too. Only forward it to the + // remote when the relay hook path is enabled; otherwise a newer relay + // could emit statuses this Orca build is not prepared to route. + const sshSourceEnv = stripRemotePaneEnvWhenHooksDisabled(args.connectionId, args.env) + const baseEnvWithAuth = claudeAuth + ? { ...sshSourceEnv, ...claudeAuth.envPatch } + : sshSourceEnv + const spawnPaneKey = baseEnvWithAuth?.ORCA_PANE_KEY + const parsedSpawnPaneKey = parseValidPaneKey(spawnPaneKey) + const verifiedPaneKey = + parsedSpawnPaneKey && + typeof args.tabId === 'string' && + args.tabId === parsedSpawnPaneKey.tabId && + args.leafId === parsedSpawnPaneKey.leafId + ? makePaneKey(parsedSpawnPaneKey.tabId, parsedSpawnPaneKey.leafId) + : null + const verifiedLeafId = + verifiedPaneKey && parsedSpawnPaneKey ? parsedSpawnPaneKey.leafId : null + const metadataLeafId = + typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null + const metadataPaneKey = + typeof args.tabId === 'string' && + isValidTerminalTabId(args.tabId) && + args.tabId.length <= 512 && + metadataLeafId + ? makePaneKey(args.tabId, metadataLeafId) + : null + const legacySpawnPaneKey = verifiedPaneKey + ? null + : parseLegacyNumericPaneKey(spawnPaneKey) + const migrationUnsupportedPaneKey = + legacySpawnPaneKey && + typeof args.tabId === 'string' && + args.tabId === legacySpawnPaneKey.tabId && + typeof args.leafId === 'string' && + isTerminalLeafId(args.leafId) + ? makePaneKey(args.tabId, args.leafId) + : null + const stablePaneKey = verifiedPaneKey ?? migrationUnsupportedPaneKey + let baseEnv = baseEnvWithAuth ? { ...baseEnvWithAuth } : undefined + const shouldRefreshAgentTeamsEnv = + !args.connectionId && + runtime !== undefined && + stablePaneKey !== null && + shouldRefreshNativeClaudeAgentTeamsEnv({ + command: args.command, + launchConfig: args.launchConfig }) - : { shellOverride: args.shellOverride, terminalWindowsWslDistro: null } - const initialShellOverride = terminalRuntimeOptions.shellOverride - const initialSelectionTarget = getCodexSelectionTargetForPty( - initialShellOverride, - cwd, - terminalRuntimeOptions.terminalWindowsWslDistro ?? null - ) - const claudeAuth = - isClaudeLaunch && prepareClaudeAuth ? await prepareClaudeAuth(initialSelectionTarget) : null - spawnTiming.mark('auth') - if (isClaudeLaunch && isClaudeAuthSwitchInProgress()) { - throw new Error('A Claude account switch is in progress. Try again after it finishes.') - } - if (claudeAuth?.stripAuthEnv && hasClaudeAuthEnvConflict(args.env)) { - throw new Error( - 'This Claude launch defines explicit Anthropic auth environment variables. Remove those overrides before using a managed Claude account.' - ) - } - // Why: the daemon-backed provider replaces LocalPtyProvider and therefore - // never runs its buildSpawnEnv closure. We must assemble the same - // host-local env (OpenCode plugin, agent-hook server, Pi/OMP managed - // extensions, Codex home, dev CLI overrides, GitHub attribution shims) - // here so both spawn paths behave identically. buildPtyHostEnv is the - // shared helper that encapsulates the full set of injections and guards. - // - // Safety: skip the entire injection when a remote (SSH) connection is in - // play. Every injection here is either host-loopback (the agent-hook - // server binds 127.0.0.1, so shipping its token to an SSH host would - // leak a loopback secret for no functional benefit) or a path on the - // local filesystem (OpenCode plugin dir, Pi/OMP extension paths, Codex - // home, dev CLI bin, attribution shim dir) that would resolve to - // nothing — or something misleading — on the remote machine. - const isDaemonHostSpawn = - !args.connectionId && - !(provider instanceof LocalPtyProvider) && - !routesFreshSpawnsToLocalProvider(provider) - // Why: daemon host-env setup needs a stable id BEFORE provider.spawn so - // provider hooks and legacy Pi overlay cleanup can run in buildPtyHostEnv. - // DaemonPtyAdapter.doSpawn mints an id the same way when sessionId is - // absent — lifting the mint here gives pty.ts the id up-front without - // changing daemon semantics (the daemon still honors opts.sessionId ?? mint()). - // - // Note: the sessionId is STABLE across daemon restarts by design — - // DaemonPtyAdapter.reconcileOnStartup reuses it so that users' live - // shells survive crashes. Do NOT "simplify" id allocation back to a - // fresh UUID per spawn; that would orphan reconnectable terminal state. - // Why: only state for ids we minted in THIS request should be cleared on - // spawn failure. If the caller supplied args.sessionId it may refer to - // an existing PTY whose state (OpenCode hooks, legacy Pi overlay cleanup, - // agent-hook pane caches) we must not clobber on a retry/attach failure. - const isMintedSessionId = args.sessionId === undefined && isDaemonHostSpawn - const effectiveSessionId = - args.sessionId ?? (isDaemonHostSpawn ? mintPtySessionId(args.worktreeId) : undefined) - const effectiveSessionAppId = - effectiveSessionId !== undefined - ? getAppPtyId(args.connectionId, effectiveSessionId) - : undefined - const effectiveSessionRelayId = - effectiveSessionId !== undefined - ? getRelayPtyId(args.connectionId, effectiveSessionId) - : undefined - const startupTerminalColorQueryReplyColors = getStartupTerminalColorQueryReplyColors(args) - const preSpawnStartupTerminalColorReplyPtyId = - startupTerminalColorQueryReplyColors && effectiveSessionId !== undefined - ? (effectiveSessionAppId ?? effectiveSessionId) - : null - // Why: the renderer sets pane env for SSH too. Only forward it to the - // remote when the relay hook path is enabled; otherwise a newer relay - // could emit statuses this Orca build is not prepared to route. - const sshSourceEnv = stripRemotePaneEnvWhenHooksDisabled(args.connectionId, args.env) - const baseEnvWithAuth = claudeAuth - ? { ...sshSourceEnv, ...claudeAuth.envPatch } - : sshSourceEnv - const spawnPaneKey = baseEnvWithAuth?.ORCA_PANE_KEY - const parsedSpawnPaneKey = parseValidPaneKey(spawnPaneKey) - const verifiedPaneKey = - parsedSpawnPaneKey && - typeof args.tabId === 'string' && - args.tabId === parsedSpawnPaneKey.tabId && - args.leafId === parsedSpawnPaneKey.leafId - ? makePaneKey(parsedSpawnPaneKey.tabId, parsedSpawnPaneKey.leafId) - : null - const verifiedLeafId = - verifiedPaneKey && parsedSpawnPaneKey ? parsedSpawnPaneKey.leafId : null - const metadataLeafId = - typeof args.leafId === 'string' && isTerminalLeafId(args.leafId) ? args.leafId : null - const metadataPaneKey = - typeof args.tabId === 'string' && - isValidTerminalTabId(args.tabId) && - args.tabId.length <= 512 && - metadataLeafId - ? makePaneKey(args.tabId, metadataLeafId) - : null - const legacySpawnPaneKey = verifiedPaneKey ? null : parseLegacyNumericPaneKey(spawnPaneKey) - const migrationUnsupportedPaneKey = - legacySpawnPaneKey && - typeof args.tabId === 'string' && - args.tabId === legacySpawnPaneKey.tabId && - typeof args.leafId === 'string' && - isTerminalLeafId(args.leafId) - ? makePaneKey(args.tabId, args.leafId) - : null - const stablePaneKey = verifiedPaneKey ?? migrationUnsupportedPaneKey - let baseEnv = baseEnvWithAuth ? { ...baseEnvWithAuth } : undefined - const shouldRefreshAgentTeamsEnv = - !args.connectionId && - runtime !== undefined && - stablePaneKey !== null && - shouldRefreshNativeClaudeAgentTeamsEnv({ - command: args.command, - launchConfig: args.launchConfig - }) - let effectiveLaunchConfig = args.launchConfig - const shouldPreAllocateTerminalHandle = - runtime !== undefined && - ((!(provider instanceof LocalPtyProvider) && !routesFreshSpawnsToLocalProvider(provider)) || - shouldRefreshAgentTeamsEnv) - const preAllocatedHandle = shouldPreAllocateTerminalHandle - ? runtime.createPreAllocatedTerminalHandle() - : null - if (shouldRefreshAgentTeamsEnv && preAllocatedHandle) { - // Why: native Agent Teams team ids/tokens are process-local. A sleeping - // record preserves the user's native launch shape, but the team env - // itself must be regenerated for the new leader PTY. - const prepared = await runtime.prepareClaudeAgentTeamsLeaderForHandle({ - handle: preAllocatedHandle, - baseEnv: baseEnv ?? {} - }) - baseEnv = { - ...baseEnv, - ...prepared.env - } - if (args.launchConfig) { - effectiveLaunchConfig = { - ...args.launchConfig, - agentEnv: { - ...args.launchConfig.agentEnv, + let effectiveLaunchConfig = args.launchConfig + const shouldPreAllocateTerminalHandle = + runtime !== undefined && + ((!(provider instanceof LocalPtyProvider) && + !routesFreshSpawnsToLocalProvider(provider)) || + shouldRefreshAgentTeamsEnv) + const preAllocatedHandle = shouldPreAllocateTerminalHandle + ? runtime.createPreAllocatedTerminalHandle() + : null + if (shouldRefreshAgentTeamsEnv && preAllocatedHandle) { + // Why: native Agent Teams team ids/tokens are process-local. A sleeping + // record preserves the user's native launch shape, but the team env + // itself must be regenerated for the new leader PTY. + const prepared = await runtime.prepareClaudeAgentTeamsLeaderForHandle({ + handle: preAllocatedHandle, + baseEnv: baseEnv ?? {} + }) + baseEnv = { + ...baseEnv, ...prepared.env } - } - } - } - const requestedAgentTeamsPath = baseEnv?.ORCA_AGENT_TEAMS_TEAM_ID ? baseEnv.PATH : undefined - const agentTeamsEnvToDelete = shouldRefreshAgentTeamsEnv - ? ['TERM_PROGRAM', 'ORCA_ATTRIBUTION_SHIM_DIR'] - : undefined - if (baseEnv && stablePaneKey) { - baseEnv.ORCA_PANE_KEY = stablePaneKey - if (typeof args.tabId === 'string') { - baseEnv.ORCA_TAB_ID = args.tabId - } else if (!args.connectionId) { - delete baseEnv.ORCA_TAB_ID - } - if (typeof args.worktreeId === 'string') { - baseEnv.ORCA_WORKTREE_ID = args.worktreeId - } else if (!args.connectionId) { - delete baseEnv.ORCA_WORKTREE_ID - } - } else if (baseEnv) { - // Why: ORCA_PANE_KEY crosses into shells and hook registries. Only the - // key proven to match this spawn's tab+leaf may leave the IPC boundary. - delete baseEnv.ORCA_PANE_KEY - delete baseEnv.ORCA_TAB_ID - delete baseEnv.ORCA_WORKTREE_ID - delete baseEnv.ORCA_AGENT_LAUNCH_TOKEN - } - const validatedPaneKey = stablePaneKey - // Why: SSH can strip ORCA_PANE_KEY when remote hooks are disabled; the - // IPC tab/leaf metadata still names the pane and matches runtime fallback. - const reservationPaneKey = metadataPaneKey ?? validatedPaneKey - const validatedLeafId = verifiedLeafId ?? metadataLeafId - let env: Record | undefined = baseEnv - const effectiveShellOverride = terminalRuntimeOptions.shellOverride - const codexSelectionTarget = getCodexSelectionTargetForPty( - effectiveShellOverride, - cwd, - terminalRuntimeOptions.terminalWindowsWslDistro ?? null - ) - const selectedCodexHomePath = isDaemonHostSpawn - ? getCompatibleSelectedCodexHomePath( - codexSelectionTarget, - getSelectedCodexHomePath?.(codexSelectionTarget) ?? null - ) - : null - const skipCodexHomeEnv = - isDaemonHostSpawn && - shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd) && - !selectedCodexHomePath - if (isDaemonHostSpawn) { - if (effectiveSessionId === undefined) { - // Should be unreachable: the expression above returns a string when - // isDaemonHostSpawn is true. Defense-in-depth in case future edits - // break this invariant. - throw new Error('Invariant violation: daemon spawn without sessionId') - } - const sessionIdForEnv = effectiveSessionId - // Why: this id still reaches filesystem side-effects for provider - // hook state and stale pre-migration Pi overlay cleanup; reject - // traversal/path separators before a crafted IPC payload can escape - // the expected roots. - if (!isSafePtySessionId(sessionIdForEnv, app.getPath('userData'))) { - throw new Error('Invalid PTY session id') - } - // Why: clone before mutating so we don't leak injections back into - // args.env (which the renderer may reuse for other IPC calls). - env = { ...baseEnv } - try { - buildPtyHostEnv(sessionIdForEnv, env, { - isPackaged: app.isPackaged, - userDataPath: app.getPath('userData'), - selectedCodexHomePath, - skipCodexHomeEnv, - githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, - launchCommand: args.command, - shellPath: effectiveShellOverride ?? process.env.COMSPEC, - isWsl: shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd), - wslDistro: - codexSelectionTarget.runtime === 'wsl' ? codexSelectionTarget.wslDistro : null, - agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()), - networkProxySettings: getSettings?.() - }) - promoteAgentTeamsShimPath(env, requestedAgentTeamsPath) - } catch (err) { - // Why: buildPtyHostEnv has filesystem side-effects (Pi/OMP managed - // extension installation). If it throws before we reach provider.spawn, - // clear per-PTY state so the next attempt starts clean. - // - // Only sweep state for ids we MINTED in this request — caller- - // supplied ids may refer to existing PTYs whose overlay/hook state - // must not be clobbered by a transient overlay-mkdir failure on a - // retry/attach path. - if (isMintedSessionId) { - clearProviderPtyState(sessionIdForEnv) - } - throw err - } - } - spawnTiming.mark('host_env') - const spawnEnv = preAllocatedHandle - ? { ...env, ORCA_TERMINAL_HANDLE: preAllocatedHandle } - : env - const envToDelete = claudeAuth?.stripAuthEnv - ? [...CLAUDE_AUTH_ENV_VARS, 'ANTHROPIC_CUSTOM_HEADERS'] - : undefined - const combinedEnvToDelete = mergePtyEnvDeletions( - mergePtyEnvDeletions( - mergePtyEnvDeletions( - mergePtyEnvDeletions(envToDelete, args.envToDelete ?? []), - agentTeamsEnvToDelete ?? [] - ), - isDaemonHostSpawn ? getInheritedAgentHookEnvKeysToDelete(spawnEnv) : [] - ), - skipCodexHomeEnv ? CODEX_HOME_ENV_KEYS : [] - ) - deleteRequestedEnvKeys(spawnEnv, combinedEnvToDelete) - promoteAgentTeamsShimPath(spawnEnv, requestedAgentTeamsPath) - const spawnOptions: PtySpawnOptions = { - cols: args.cols, - rows: args.rows, - cwd, - env: spawnEnv, - ...(isMintedSessionId ? { isNewSession: true } : {}) - } - if (combinedEnvToDelete) { - spawnOptions.envToDelete = combinedEnvToDelete - } - if (args.command !== undefined) { - spawnOptions.command = args.command - } - if (args.commandDelivery !== undefined) { - spawnOptions.commandDelivery = args.commandDelivery - } - if (args.startupCommandDelivery !== undefined) { - spawnOptions.startupCommandDelivery = args.startupCommandDelivery - } - if (isTuiAgent(args.launchAgent)) { - spawnOptions.launchAgent = args.launchAgent - } - if (args.worktreeId !== undefined) { - spawnOptions.worktreeId = args.worktreeId - } - if (reservationPaneKey) { - spawnOptions.paneKey = reservationPaneKey - } - if (typeof args.tabId === 'string' && args.tabId.length > 0 && args.tabId.length <= 512) { - spawnOptions.tabId = args.tabId - } - if (effectiveSessionId !== undefined) { - spawnOptions.sessionId = effectiveSessionId - } - // Why: on Windows, fall back to the persisted default-shell setting - // when the renderer didn't send a per-tab override. Without this, the - // daemon path ignores the user's "Default Shell" preference entirely — - // it just calls resolvePtyShellPath(env) which reads COMSPEC (cmd.exe) - // or falls back to PowerShell. The LocalPtyProvider already consults - // getWindowsShell(); this mirrors that on the daemon path so users who - // set WSL as default actually get WSL when pressing Ctrl+T. - if (effectiveShellOverride !== undefined) { - spawnOptions.shellOverride = effectiveShellOverride - } - const hadSessionSizeBeforeAttach = - effectiveSessionAppId !== undefined ? ptySizes.has(effectiveSessionAppId) : false - const sessionSizeBeforeAttach = - effectiveSessionAppId !== undefined ? ptySizes.get(effectiveSessionAppId) : undefined - if (effectiveSessionId !== undefined) { - // Why: daemon PTYs can emit prompt/startup bytes before spawn() - // resolves. Runtime headless snapshots need the real pane geometry - // for those early bytes; otherwise they default to 80x24 and wrap TUIs. - ptySizes.set(effectiveSessionAppId ?? effectiveSessionId, { - cols: args.cols, - rows: args.rows - }) - } - if (process.platform === 'win32' && !args.connectionId) { - // Why: the renderer only models PowerShell as one shell family. Thread - // the persisted implementation choice through spawnOptions so both the - // in-process and daemon-backed PTY paths can resolve the same effective - // executable without inventing a fourth top-level shell. - spawnOptions.terminalWindowsWslDistro = - terminalRuntimeOptions.terminalWindowsWslDistro ?? null - spawnOptions.terminalWindowsPowerShellImplementation = getSettings - ? (getSettings()?.terminalWindowsPowerShellImplementation ?? 'auto') - : undefined - } - const existingPaneSpawn = reservationPaneKey - ? paneSpawnReservationsByPaneKey.get(reservationPaneKey) - : undefined - if (existingPaneSpawn) { - return await existingPaneSpawn.promise - } - const paneSpawnReservation = reservationPaneKey ? reservePaneSpawn(reservationPaneKey) : null - const initiallyHidden = args.initiallyHidden === true - // Why pre-spawn for daemon-host sessions (id minted up front): daemon - // PTYs can emit prompt bytes before spawn() resolves, and the hidden - // mark must beat the first byte so the gate + model responder own - // spawn-time queries (terminal-query-authority.md §races). Other - // providers cannot emit until spawn resolves; the post-spawn mark - // below is byte-zero-safe for them. - const preSpawnHiddenMarkId = - initiallyHidden && isDaemonHostSpawn && effectiveSessionAppId !== undefined - ? effectiveSessionAppId - : null - if (preSpawnHiddenMarkId !== null) { - markHiddenRendererPty(preSpawnHiddenMarkId) - } - let result: PtySpawnResult - try { - try { - if (preAllocatedHandle) { - trustedTerminalHandleEnv.add(preAllocatedHandle) - } - if (preSpawnStartupTerminalColorReplyPtyId && startupTerminalColorQueryReplyColors) { - // Why: Codex probes OSC 10/11 with a 100 ms timeout and daemon PTYs - // can emit that query before spawn() resolves to the renderer. - registerStartupTerminalColorQueryReplies( - preSpawnStartupTerminalColorReplyPtyId, - startupTerminalColorQueryReplyColors - ) - } - spawnTiming.mark('options') - result = await provider.spawn(spawnOptions) - spawnTiming.mark('provider_spawn') - } catch (err) { - // Why: a failed spawn must not leave a stale hidden mark on a session - // id a later visible attach may reuse. - if (preSpawnHiddenMarkId !== null) { - unmarkHiddenRendererPty(preSpawnHiddenMarkId) - } - const rawMessage = err instanceof Error ? err.message : String(err) - const spawnError = normalizeNodePtySpawnError(err) - const isIdentityMismatch = - isSshPtyIdentityMismatchError(spawnError) || isSshPtyIdentityMismatchError(rawMessage) - if (preSpawnStartupTerminalColorReplyPtyId) { - clearStartupTerminalColorQueryReplies(preSpawnStartupTerminalColorReplyPtyId) - } - if (effectiveSessionAppId !== undefined) { - if (isIdentityMismatch && hadSessionSizeBeforeAttach && sessionSizeBeforeAttach) { - ptySizes.set(effectiveSessionAppId, sessionSizeBeforeAttach) - } else { - ptySizes.delete(effectiveSessionAppId) - } - } - if ( - args.connectionId && - effectiveSessionRelayId !== undefined && - (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || - rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) - ) { - // Why: expired remote reattach means the relay has already dropped - // the backing PTY. Clear the durable lease so later session writes - // cannot restore the stale pane binding. - if (effectiveSessionAppId !== undefined && !isIdentityMismatch) { - clearProviderPtyState(effectiveSessionAppId) - deletePtyOwnership(effectiveSessionAppId) - } - if (!isIdentityMismatch) { - store?.markSshRemotePtyLease(args.connectionId, effectiveSessionRelayId, 'expired') - } - } - // Why: if buildPtyHostEnv materialized provider state for this minted - // id but provider.spawn failed, that state would otherwise leak. - if (isMintedSessionId && effectiveSessionId !== undefined) { - clearProviderPtyState(effectiveSessionId) - } - // Why: telemetry-plan.md§agent_error — when the renderer threaded - // agent_kind through args.telemetry, attribute the error to that agent. - // Otherwise fall back to sniffing the command for `claude` (the one - // agent the main process can identify on its own via the existing - // `isClaudeLaunchCommand` regex used for auth gating). Bare-shell - // catches and unknown-agent catches without renderer telemetry remain - // unattributed. The event still emits with a classified `error_class`; - // raw error messages are dropped at the telemetry validator boundary. - const rendererAgentKindParse = - args.telemetry?.agent_kind !== undefined - ? agentKindSchema.safeParse(args.telemetry.agent_kind) - : null - const errorAgentKind = rendererAgentKindParse?.success - ? rendererAgentKindParse.data - : isClaudeLaunch - ? ('claude-code' as const) - : null - if (errorAgentKind) { - const classified = classifyError(spawnError) - track('agent_error', { - agent_kind: errorAgentKind, - error_class: classified.error_class, - ...getCohortAtEmit() - }) - } - throw spawnError - } finally { - if (preAllocatedHandle) { - trustedTerminalHandleEnv.delete(preAllocatedHandle) - } - } - spawnTiming.log(result.id, { - daemon: isDaemonHostSpawn, - reattach: result.isReattach ?? false - }) - ptyOwnership.set(result.id, args.connectionId ?? null) - if (initiallyHidden) { - // Why marked synchronously before any await below: local/SSH provider - // data events dispatch on later tasks, so this is still ahead of the - // first byte's delivery decision. Idempotent for daemon hosts already - // marked pre-spawn; the renderer's first visibility sync re-marks or - // unmarks (emitting the restore marker) through the Phase-4 path. - markHiddenRendererPty(result.id) - if (preSpawnHiddenMarkId !== null && preSpawnHiddenMarkId !== result.id) { - // Defense: never strand a mark on an id the provider renamed. - unmarkHiddenRendererPty(preSpawnHiddenMarkId) - } - // Why after ptyOwnership.set: the provider lookup routes by - // ownership, and a hidden-spawned agent should be paceable from its - // first flood, not from its first visibility transition. - syncPtyBackgroundedDelivery(result.id, 'spawn') - } - // Why: Phase-5 ConPTY DA1 — record the native-Windows-local-PTY - // determination from the spawn record before the headless seed below, - // so the runtime emulator's DA1 override exists from byte zero. - if ( - isNativeWindowsLocalPtySpawn({ - connectionId: args.connectionId, - cwd: args.cwd, - shellOverride: effectiveShellOverride - }) - ) { - markNativeWindowsConptyPty(result.id) - } - if (startupTerminalColorQueryReplyColors) { - if (result.isReattach) { - if (preSpawnStartupTerminalColorReplyPtyId) { - clearStartupTerminalColorQueryReplies(preSpawnStartupTerminalColorReplyPtyId) - } - } else if (preSpawnStartupTerminalColorReplyPtyId) { - moveStartupTerminalColorQueryReplies(preSpawnStartupTerminalColorReplyPtyId, result.id) - } else { - registerStartupTerminalColorQueryReplies( - result.id, - startupTerminalColorQueryReplyColors - ) - } - } - const relayResultId = getRelayPtyId(args.connectionId, result.id) - const relayInstanceId = parseAppSshPtyId(result.id)?.relayInstanceId - if (store && args.connectionId) { - // Why: remote PTYs live in the SSH relay grace window after Orca - // detaches. Persist their IDs immediately so reconnect can reattach - // instead of treating the tab as a fresh shell. - store.upsertSshRemotePtyLease({ - targetId: args.connectionId, - ptyId: relayResultId, - ...(relayInstanceId ? { relayInstanceId } : {}), - ...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}), - ...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}), - ...(validatedLeafId ? { leafId: validatedLeafId } : {}), - state: 'attached', - lastAttachedAt: Date.now() - }) - } - if (preAllocatedHandle) { - runtime?.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) - } - ptySizes.set(result.id, { cols: args.cols, rows: args.rows }) - // Why: closes the SIGKILL-between-spawn-and-persist race (Issue #217) - // for local daemon PTYs and the equivalent remote-relay race for SSH. - // The renderer's debounced session writer runs in parallel for every - // other field; patch the load-bearing (tab.ptyId, ptyIdsByLeafId) - // binding synchronously so a force-quit in the ~450 ms debounce window - // cannot orphan either daemon history or a remote relay PTY lease. - if ( - (isDaemonHostSpawn || args.connectionId) && - store && - typeof args.worktreeId === 'string' && - typeof args.tabId === 'string' && - validatedLeafId !== null - ) { - try { - store.persistPtyBinding({ - worktreeId: args.worktreeId, - tabId: args.tabId, - leafId: validatedLeafId, - ptyId: result.id, - ...(cwd ? { startupCwd: cwd } : {}) - }) - } catch (err) { - console.error('[pty] failed to persist PTY binding after spawn:', err) - if (!result.isReattach) { - try { - await provider.shutdown(result.id, { immediate: true }) - } catch (shutdownErr) { - console.warn('[pty] failed to clean up PTY after persistence failure:', shutdownErr) + if (args.launchConfig) { + effectiveLaunchConfig = { + ...args.launchConfig, + agentEnv: { + ...args.launchConfig.agentEnv, + ...prepared.env + } } - clearProviderPtyState(result.id) - deletePtyOwnership(result.id) } - if (!result.isReattach && args.connectionId && store) { - store.removeSshRemotePtyLease(args.connectionId, relayResultId) + } + const requestedAgentTeamsPath = baseEnv?.ORCA_AGENT_TEAMS_TEAM_ID + ? baseEnv.PATH + : undefined + const agentTeamsEnvToDelete = shouldRefreshAgentTeamsEnv + ? ['TERM_PROGRAM', 'ORCA_ATTRIBUTION_SHIM_DIR'] + : undefined + if (baseEnv && stablePaneKey) { + baseEnv.ORCA_PANE_KEY = stablePaneKey + if (typeof args.tabId === 'string') { + baseEnv.ORCA_TAB_ID = args.tabId + } else if (!args.connectionId) { + delete baseEnv.ORCA_TAB_ID } - throw new Error(createTerminalSessionStateSaveFailureMessage()) + if (typeof args.worktreeId === 'string') { + baseEnv.ORCA_WORKTREE_ID = args.worktreeId + } else if (!args.connectionId) { + delete baseEnv.ORCA_WORKTREE_ID + } + } else if (baseEnv) { + // Why: ORCA_PANE_KEY crosses into shells and hook registries. Only the + // key proven to match this spawn's tab+leaf may leave the IPC boundary. + delete baseEnv.ORCA_PANE_KEY + delete baseEnv.ORCA_TAB_ID + delete baseEnv.ORCA_WORKTREE_ID + delete baseEnv.ORCA_AGENT_LAUNCH_TOKEN } - } - // Why: pre-signal cooperation gate — when the renderer has declared it - // will own the serializer for this paneKey, suppress the daemon-snapshot - // seed so the renderer's hydration path (maybeHydrateHeadlessFromRenderer) - // is the sole authority. The pre-signal is keyed on paneKey because at - // spawn time the renderer doesn't yet know the new ptyId. See - // docs/mobile-prefer-renderer-scrollback.md. - const rendererPreSignaled = validatedPaneKey - ? pendingByPaneKey.has(validatedPaneKey) - : false - const rendererAlreadyRegistered = rendererSerializerByPtyId.has(result.id) - // Why: capture the pending gen at spawn time so teardown for THIS PTY - // only settles its own generation. A remount that replaces the entry - // with a new gen must not be stomped by the old PTY's teardown. - if (validatedPaneKey && rendererPreSignaled) { - const pending = pendingByPaneKey.get(validatedPaneKey) - if (pending) { - ptyPendingGenByPtyId.set(result.id, pending.gen) + const validatedPaneKey = stablePaneKey + // Why: SSH can strip ORCA_PANE_KEY when remote hooks are disabled; the + // IPC tab/leaf metadata still names the pane and matches runtime fallback. + const reservationPaneKey = metadataPaneKey ?? validatedPaneKey + const validatedLeafId = verifiedLeafId ?? metadataLeafId + let env: Record | undefined = baseEnv + const effectiveShellOverride = terminalRuntimeOptions.shellOverride + const codexSelectionTarget = getCodexSelectionTargetForPty( + effectiveShellOverride, + cwd, + terminalRuntimeOptions.terminalWindowsWslDistro ?? null + ) + const selectedCodexHomePath = isDaemonHostSpawn + ? getCompatibleSelectedCodexHomePath( + codexSelectionTarget, + getSelectedCodexHomePath?.(codexSelectionTarget) ?? null + ) + : null + const skipCodexHomeEnv = + isDaemonHostSpawn && + shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd) && + !selectedCodexHomePath + if (isDaemonHostSpawn) { + if (effectiveSessionId === undefined) { + // Should be unreachable: the expression above returns a string when + // isDaemonHostSpawn is true. Defense-in-depth in case future edits + // break this invariant. + throw new Error('Invariant violation: daemon spawn without sessionId') + } + const sessionIdForEnv = effectiveSessionId + // Why: this id still reaches filesystem side-effects for provider + // hook state and stale pre-migration Pi overlay cleanup; reject + // traversal/path separators before a crafted IPC payload can escape + // the expected roots. + if (!isSafePtySessionId(sessionIdForEnv, app.getPath('userData'))) { + throw new Error('Invalid PTY session id') + } + // Why: clone before mutating so we don't leak injections back into + // args.env (which the renderer may reuse for other IPC calls). + env = { ...baseEnv } + try { + buildPtyHostEnv(sessionIdForEnv, env, { + isPackaged: app.isPackaged, + userDataPath: app.getPath('userData'), + selectedCodexHomePath, + skipCodexHomeEnv, + githubAttributionEnabled: getSettings?.()?.enableGitHubAttribution ?? false, + launchCommand: args.command, + shellPath: effectiveShellOverride ?? process.env.COMSPEC, + isWsl: shouldSkipCodexHomeEnvForWindowsShell(effectiveShellOverride, cwd), + wslDistro: + codexSelectionTarget.runtime === 'wsl' ? codexSelectionTarget.wslDistro : null, + agentStatusHooksEnabled: isAgentStatusHooksEnabled(getSettings?.()), + networkProxySettings: getSettings?.() + }) + promoteAgentTeamsShimPath(env, requestedAgentTeamsPath) + } catch (err) { + // Why: buildPtyHostEnv has filesystem side-effects (Pi/OMP managed + // extension installation). If it throws before we reach provider.spawn, + // clear per-PTY state so the next attempt starts clean. + // + // Only sweep state for ids we MINTED in this request — caller- + // supplied ids may refer to existing PTYs whose overlay/hook state + // must not be clobbered by a transient overlay-mkdir failure on a + // retry/attach path. + if (isMintedSessionId) { + clearProviderPtyState(sessionIdForEnv) + } + throw err + } } - } - - // Why: hydrate the runtime's headless emulator with the adapter's - // restore data BEFORE registerPty so any live PTY data that arrives - // concurrently lands on top of the seed instead of replacing it. Mobile - // subscribers then see the same scrollback the desktop xterm received - // via coldRestore/snapshot. Without this, mobile snapshots after a - // daemon-restored attach contain only bytes emitted since the relaunch - // and the prior agent output silently disappears. - // - // Skip when the renderer is or will be authoritative for this PTY: - // its hydration path will seed the emulator from xterm's live buffer, - // which is richer than the daemon snapshot. - if (runtime && !rendererPreSignaled && !rendererAlreadyRegistered) { - const seedSize = - typeof result.snapshotCols === 'number' && typeof result.snapshotRows === 'number' - ? { cols: result.snapshotCols, rows: result.snapshotRows } - : undefined - if (typeof result.snapshot === 'string' && result.snapshot.length > 0) { - // Why kitty flags ride seed metadata: the snapshot string omits - // them by design (renderer kitty reset stays authoritative), but - // the re-seeded emulator must answer hidden `CSI ? u` with the - // flags the still-running app pushed (terminal-query-authority.md). - runtime.seedHeadlessTerminal( - result.id, - result.snapshot, - seedSize, - typeof result.snapshotKittyKeyboardFlags === 'number' - ? { kittyKeyboardFlags: result.snapshotKittyKeyboardFlags } - : {} - ) - } else if ( - result.coldRestore && - typeof result.coldRestore.scrollback === 'string' && - result.coldRestore.scrollback.length > 0 - ) { - runtime.seedHeadlessTerminal(result.id, result.coldRestore.scrollback, seedSize, { - cwd: result.coldRestore.cwd, - oscLinks: result.coldRestore.oscLinks + spawnTiming.mark('host_env') + const spawnEnv = preAllocatedHandle + ? { ...env, ORCA_TERMINAL_HANDLE: preAllocatedHandle } + : env + const envToDelete = claudeAuth?.stripAuthEnv + ? [...CLAUDE_AUTH_ENV_VARS, 'ANTHROPIC_CUSTOM_HEADERS'] + : undefined + const combinedEnvToDelete = mergePtyEnvDeletions( + mergePtyEnvDeletions( + mergePtyEnvDeletions( + mergePtyEnvDeletions(envToDelete, args.envToDelete ?? []), + agentTeamsEnvToDelete ?? [] + ), + isDaemonHostSpawn ? getInheritedAgentHookEnvKeysToDelete(spawnEnv) : [] + ), + skipCodexHomeEnv ? CODEX_HOME_ENV_KEYS : [] + ) + deleteRequestedEnvKeys(spawnEnv, combinedEnvToDelete) + promoteAgentTeamsShimPath(spawnEnv, requestedAgentTeamsPath) + const spawnOptions: PtySpawnOptions = { + cols: args.cols, + rows: args.rows, + cwd, + env: spawnEnv, + ...(isMintedSessionId ? { isNewSession: true } : {}) + } + if (combinedEnvToDelete) { + spawnOptions.envToDelete = combinedEnvToDelete + } + if (args.command !== undefined) { + spawnOptions.command = args.command + } + if (args.commandDelivery !== undefined) { + spawnOptions.commandDelivery = args.commandDelivery + } + if (args.startupCommandDelivery !== undefined) { + spawnOptions.startupCommandDelivery = args.startupCommandDelivery + } + if (isTuiAgent(args.launchAgent)) { + spawnOptions.launchAgent = args.launchAgent + } + if (args.worktreeId !== undefined) { + spawnOptions.worktreeId = args.worktreeId + } + if (reservationPaneKey) { + spawnOptions.paneKey = reservationPaneKey + } + if (typeof args.tabId === 'string' && args.tabId.length > 0 && args.tabId.length <= 512) { + spawnOptions.tabId = args.tabId + } + if (effectiveSessionId !== undefined) { + spawnOptions.sessionId = effectiveSessionId + } + // Why: on Windows, fall back to the persisted default-shell setting + // when the renderer didn't send a per-tab override. Without this, the + // daemon path ignores the user's "Default Shell" preference entirely — + // it just calls resolvePtyShellPath(env) which reads COMSPEC (cmd.exe) + // or falls back to PowerShell. The LocalPtyProvider already consults + // getWindowsShell(); this mirrors that on the daemon path so users who + // set WSL as default actually get WSL when pressing Ctrl+T. + if (effectiveShellOverride !== undefined) { + spawnOptions.shellOverride = effectiveShellOverride + } + const hadSessionSizeBeforeAttach = + effectiveSessionAppId !== undefined ? ptySizes.has(effectiveSessionAppId) : false + const sessionSizeBeforeAttach = + effectiveSessionAppId !== undefined ? ptySizes.get(effectiveSessionAppId) : undefined + if (effectiveSessionId !== undefined) { + // Why: daemon PTYs can emit prompt/startup bytes before spawn() + // resolves. Runtime headless snapshots need the real pane geometry + // for those early bytes; otherwise they default to 80x24 and wrap TUIs. + ptySizes.set(effectiveSessionAppId ?? effectiveSessionId, { + cols: args.cols, + rows: args.rows }) } - } - if ( - typeof args.worktreeId === 'string' && - args.worktreeId.length > 0 && - args.worktreeId.length <= 512 - ) { - runtime?.registerPty( - result.id, - args.worktreeId, - args.connectionId ?? null, - // Why: pass the validated pane identity so a mobile create waiting on - // this renderer tab can publish its surface main-side when graph-sync - // is throttled, instead of destroying the live PTY (#7587). Bound the - // untrusted tabId like the sibling metadataPaneKey/spawnOptions.tabId. - typeof args.tabId === 'string' && - isValidTerminalTabId(args.tabId) && - args.tabId.length <= 512 && - metadataLeafId !== null - ? { tabId: args.tabId, leafId: metadataLeafId } + if (process.platform === 'win32' && !args.connectionId) { + // Why: the renderer only models PowerShell as one shell family. Thread + // the persisted implementation choice through spawnOptions so both the + // in-process and daemon-backed PTY paths can resolve the same effective + // executable without inventing a fourth top-level shell. + spawnOptions.terminalWindowsWslDistro = + terminalRuntimeOptions.terminalWindowsWslDistro ?? null + spawnOptions.terminalWindowsPowerShellImplementation = getSettings + ? (getSettings()?.terminalWindowsPowerShellImplementation ?? 'auto') : undefined - ) - } - // Why: arms main's per-PTY Command Code output detector from the launch - // command (renderer startupCommand parity); banner detection covers - // PTYs spawned without one. - runtime?.noteTerminalSpawnCommand?.( - result.id, - typeof args.command === 'string' ? args.command : null - ) - if (isClaudeLaunch) { - markClaudePtySpawned(result.id) - } - // Why: renderer sets ORCA_PANE_KEY in `args.env` for every pane-owned - // spawn (see pty-connection.ts). Recording the mapping here lets - // clearProviderPtyState clear the agent-hooks server's per-paneKey - // caches when the PTY exits. - // Why: args.env arrives as untrusted JSON over IPC — the static - // Record type is not actually enforced at the boundary. - // Narrow to a bounded string so malformed or oversized values cannot - // pollute ptyPaneKey or the downstream clearPaneState call. - const rememberedPaneKey = validatedPaneKey - ? rememberPaneKeyForPty(result.id, validatedPaneKey) - : null - if (legacySpawnPaneKey && migrationUnsupportedPaneKey) { - agentHookServer.registerPaneKeyAlias( - legacySpawnPaneKey.paneKey, - migrationUnsupportedPaneKey, - result.id - ) - clearMigrationUnsupportedPtysForPaneKey(migrationUnsupportedPaneKey) - } else if (validatedPaneKey) { - if (!result.isReattach) { - clearMigrationUnsupportedPtysForPaneKey(validatedPaneKey) } - } - // Why: register local PTYs (connectionId falsy) with the memory - // collector so it can walk each PTY's process subtree and attribute - // memory back to its worktree. SSH PTYs execute remotely and their - // process tree is not visible to our local `ps`, so we skip them. - if (!args.connectionId) { - // Why: providers publish the OS pid on the spawn result (both - // LocalPtyProvider and DaemonPtyAdapter). Recording it once here keeps - // the memory module from reaching back into ipc/pty on a hot path, and - // works uniformly whether the PTY is hosted in-process or by the - // daemon subprocess. - const spawnedPid = result.pid ?? null - // Why: args.worktreeId and args.sessionId arrive as untrusted IPC - // payload strings — the static type is not enforced at the boundary. - // Narrow them to bounded strings here to match the paneKey defense - // above so malformed or oversized values cannot pollute registerPty's - // maps or downstream memory-attribution lookups. - registerPty({ - ptyId: result.id, - worktreeId: + const existingPaneSpawn = reservationPaneKey + ? paneSpawnReservationsByPaneKey.get(reservationPaneKey) + : undefined + if (existingPaneSpawn) { + return await existingPaneSpawn.promise + } + const paneSpawnReservation = reservationPaneKey + ? reservePaneSpawn(reservationPaneKey) + : null + const initiallyHidden = args.initiallyHidden === true + // Why pre-spawn for daemon-host sessions (id minted up front): daemon + // PTYs can emit prompt bytes before spawn() resolves, and the hidden + // mark must beat the first byte so the gate + model responder own + // spawn-time queries (terminal-query-authority.md §races). Other + // providers cannot emit until spawn resolves; the post-spawn mark + // below is byte-zero-safe for them. + const preSpawnHiddenMarkId = + initiallyHidden && isDaemonHostSpawn && effectiveSessionAppId !== undefined + ? effectiveSessionAppId + : null + if (preSpawnHiddenMarkId !== null) { + markHiddenRendererPty(preSpawnHiddenMarkId) + } + let result: PtySpawnResult + try { + try { + if (preAllocatedHandle) { + trustedTerminalHandleEnv.add(preAllocatedHandle) + } + if (preSpawnStartupTerminalColorReplyPtyId && startupTerminalColorQueryReplyColors) { + // Why: Codex probes OSC 10/11 with a 100 ms timeout and daemon PTYs + // can emit that query before spawn() resolves to the renderer. + registerStartupTerminalColorQueryReplies( + preSpawnStartupTerminalColorReplyPtyId, + startupTerminalColorQueryReplyColors + ) + } + spawnTiming.mark('options') + result = await provider.spawn(spawnOptions) + spawnTiming.mark('provider_spawn') + } catch (err) { + // Why: a failed spawn must not leave a stale hidden mark on a session + // id a later visible attach may reuse. + if (preSpawnHiddenMarkId !== null) { + unmarkHiddenRendererPty(preSpawnHiddenMarkId) + } + const rawMessage = err instanceof Error ? err.message : String(err) + const spawnError = normalizeNodePtySpawnError(err) + const isIdentityMismatch = + isSshPtyIdentityMismatchError(spawnError) || + isSshPtyIdentityMismatchError(rawMessage) + if (preSpawnStartupTerminalColorReplyPtyId) { + clearStartupTerminalColorQueryReplies(preSpawnStartupTerminalColorReplyPtyId) + } + if (effectiveSessionAppId !== undefined) { + if (isIdentityMismatch && hadSessionSizeBeforeAttach && sessionSizeBeforeAttach) { + ptySizes.set(effectiveSessionAppId, sessionSizeBeforeAttach) + } else { + ptySizes.delete(effectiveSessionAppId) + } + } + if ( + args.connectionId && + effectiveSessionRelayId !== undefined && + (spawnError.message.includes(SSH_SESSION_EXPIRED_ERROR) || + rawMessage.includes(SSH_SESSION_EXPIRED_ERROR)) + ) { + // Why: expired remote reattach means the relay has already dropped + // the backing PTY. Clear the durable lease so later session writes + // cannot restore the stale pane binding. + if (effectiveSessionAppId !== undefined && !isIdentityMismatch) { + clearProviderPtyState(effectiveSessionAppId) + deletePtyOwnership(effectiveSessionAppId) + } + if (!isIdentityMismatch) { + store?.markSshRemotePtyLease( + args.connectionId, + effectiveSessionRelayId, + 'expired' + ) + } + } + // Why: if buildPtyHostEnv materialized provider state for this minted + // id but provider.spawn failed, that state would otherwise leak. + if (isMintedSessionId && effectiveSessionId !== undefined) { + clearProviderPtyState(effectiveSessionId) + } + // Why: telemetry-plan.md§agent_error — when the renderer threaded + // agent_kind through args.telemetry, attribute the error to that agent. + // Otherwise fall back to sniffing the command for `claude` (the one + // agent the main process can identify on its own via the existing + // `isClaudeLaunchCommand` regex used for auth gating). Bare-shell + // catches and unknown-agent catches without renderer telemetry remain + // unattributed. The event still emits with a classified `error_class`; + // raw error messages are dropped at the telemetry validator boundary. + const rendererAgentKindParse = + args.telemetry?.agent_kind !== undefined + ? agentKindSchema.safeParse(args.telemetry.agent_kind) + : null + const errorAgentKind = rendererAgentKindParse?.success + ? rendererAgentKindParse.data + : isClaudeLaunch + ? ('claude-code' as const) + : null + if (errorAgentKind) { + const classified = classifyError(spawnError) + track('agent_error', { + agent_kind: errorAgentKind, + error_class: classified.error_class, + ...getCohortAtEmit() + }) + } + throw spawnError + } finally { + if (preAllocatedHandle) { + trustedTerminalHandleEnv.delete(preAllocatedHandle) + } + } + spawnTiming.log(result.id, { + daemon: isDaemonHostSpawn, + reattach: result.isReattach ?? false + }) + ptyOwnership.set(result.id, args.connectionId ?? null) + if (initiallyHidden) { + // Why marked synchronously before any await below: local/SSH provider + // data events dispatch on later tasks, so this is still ahead of the + // first byte's delivery decision. Idempotent for daemon hosts already + // marked pre-spawn; the renderer's first visibility sync re-marks or + // unmarks (emitting the restore marker) through the Phase-4 path. + markHiddenRendererPty(result.id) + if (preSpawnHiddenMarkId !== null && preSpawnHiddenMarkId !== result.id) { + // Defense: never strand a mark on an id the provider renamed. + unmarkHiddenRendererPty(preSpawnHiddenMarkId) + } + // Why after ptyOwnership.set: the provider lookup routes by + // ownership, and a hidden-spawned agent should be paceable from its + // first flood, not from its first visibility transition. + syncPtyBackgroundedDelivery(result.id, 'spawn') + } + // Why: Phase-5 ConPTY DA1 — record the native-Windows-local-PTY + // determination from the spawn record before the headless seed below, + // so the runtime emulator's DA1 override exists from byte zero. + if ( + isNativeWindowsLocalPtySpawn({ + connectionId: args.connectionId, + cwd: args.cwd, + shellOverride: effectiveShellOverride + }) + ) { + markNativeWindowsConptyPty(result.id) + } + if (startupTerminalColorQueryReplyColors) { + if (result.isReattach) { + if (preSpawnStartupTerminalColorReplyPtyId) { + clearStartupTerminalColorQueryReplies(preSpawnStartupTerminalColorReplyPtyId) + } + } else if (preSpawnStartupTerminalColorReplyPtyId) { + moveStartupTerminalColorQueryReplies( + preSpawnStartupTerminalColorReplyPtyId, + result.id + ) + } else { + registerStartupTerminalColorQueryReplies( + result.id, + startupTerminalColorQueryReplyColors + ) + } + } + const relayResultId = getRelayPtyId(args.connectionId, result.id) + const relayInstanceId = parseAppSshPtyId(result.id)?.relayInstanceId + if (store && args.connectionId) { + // Why: remote PTYs live in the SSH relay grace window after Orca + // detaches. Persist their IDs immediately so reconnect can reattach + // instead of treating the tab as a fresh shell. + store.upsertSshRemotePtyLease({ + targetId: args.connectionId, + ptyId: relayResultId, + ...(relayInstanceId ? { relayInstanceId } : {}), + ...(typeof args.worktreeId === 'string' ? { worktreeId: args.worktreeId } : {}), + ...(typeof args.tabId === 'string' ? { tabId: args.tabId } : {}), + ...(validatedLeafId ? { leafId: validatedLeafId } : {}), + state: 'attached', + lastAttachedAt: Date.now() + }) + } + if (preAllocatedHandle) { + runtime?.registerPreAllocatedHandleForPty(result.id, preAllocatedHandle) + } + ptySizes.set(result.id, { cols: args.cols, rows: args.rows }) + // Why: closes the SIGKILL-between-spawn-and-persist race (Issue #217) + // for local daemon PTYs and the equivalent remote-relay race for SSH. + // The renderer's debounced session writer runs in parallel for every + // other field; patch the load-bearing (tab.ptyId, ptyIdsByLeafId) + // binding synchronously so a force-quit in the ~450 ms debounce window + // cannot orphan either daemon history or a remote relay PTY lease. + if ( + (isDaemonHostSpawn || args.connectionId) && + store && + typeof args.worktreeId === 'string' && + typeof args.tabId === 'string' && + validatedLeafId !== null + ) { + try { + store.persistPtyBinding({ + worktreeId: args.worktreeId, + tabId: args.tabId, + leafId: validatedLeafId, + ptyId: result.id, + ...(cwd ? { startupCwd: cwd } : {}) + }) + } catch (err) { + console.error('[pty] failed to persist PTY binding after spawn:', err) + if (!result.isReattach) { + try { + await provider.shutdown(result.id, { immediate: true }) + } catch (shutdownErr) { + console.warn( + '[pty] failed to clean up PTY after persistence failure:', + shutdownErr + ) + } + clearProviderPtyState(result.id) + deletePtyOwnership(result.id) + } + if (!result.isReattach && args.connectionId && store) { + store.removeSshRemotePtyLease(args.connectionId, relayResultId) + } + throw new Error(createTerminalSessionStateSaveFailureMessage()) + } + } + // Why: pre-signal cooperation gate — when the renderer has declared it + // will own the serializer for this paneKey, suppress the daemon-snapshot + // seed so the renderer's hydration path (maybeHydrateHeadlessFromRenderer) + // is the sole authority. The pre-signal is keyed on paneKey because at + // spawn time the renderer doesn't yet know the new ptyId. See + // docs/mobile-prefer-renderer-scrollback.md. + const rendererPreSignaled = validatedPaneKey + ? pendingByPaneKey.has(validatedPaneKey) + : false + const rendererAlreadyRegistered = rendererSerializerByPtyId.has(result.id) + // Why: capture the pending gen at spawn time so teardown for THIS PTY + // only settles its own generation. A remount that replaces the entry + // with a new gen must not be stomped by the old PTY's teardown. + if (validatedPaneKey && rendererPreSignaled) { + const pending = pendingByPaneKey.get(validatedPaneKey) + if (pending) { + ptyPendingGenByPtyId.set(result.id, pending.gen) + } + } + + // Why: hydrate the runtime's headless emulator with the adapter's + // restore data BEFORE registerPty so any live PTY data that arrives + // concurrently lands on top of the seed instead of replacing it. Mobile + // subscribers then see the same scrollback the desktop xterm received + // via coldRestore/snapshot. Without this, mobile snapshots after a + // daemon-restored attach contain only bytes emitted since the relaunch + // and the prior agent output silently disappears. + // + // Skip when the renderer is or will be authoritative for this PTY: + // its hydration path will seed the emulator from xterm's live buffer, + // which is richer than the daemon snapshot. + if (runtime && !rendererPreSignaled && !rendererAlreadyRegistered) { + const seedSize = + typeof result.snapshotCols === 'number' && typeof result.snapshotRows === 'number' + ? { cols: result.snapshotCols, rows: result.snapshotRows } + : undefined + if (typeof result.snapshot === 'string' && result.snapshot.length > 0) { + // Why kitty flags ride seed metadata: the snapshot string omits + // them by design (renderer kitty reset stays authoritative), but + // the re-seeded emulator must answer hidden `CSI ? u` with the + // flags the still-running app pushed (terminal-query-authority.md). + runtime.seedHeadlessTerminal( + result.id, + result.snapshot, + seedSize, + typeof result.snapshotKittyKeyboardFlags === 'number' + ? { kittyKeyboardFlags: result.snapshotKittyKeyboardFlags } + : {} + ) + } else if ( + result.coldRestore && + typeof result.coldRestore.scrollback === 'string' && + result.coldRestore.scrollback.length > 0 + ) { + runtime.seedHeadlessTerminal(result.id, result.coldRestore.scrollback, seedSize, { + cwd: result.coldRestore.cwd, + oscLinks: result.coldRestore.oscLinks + }) + } + } + if ( typeof args.worktreeId === 'string' && args.worktreeId.length > 0 && args.worktreeId.length <= 512 - ? args.worktreeId - : null, - sessionId: - typeof args.sessionId === 'string' && - args.sessionId.length > 0 && - args.sessionId.length <= 256 - ? args.sessionId - : null, - paneKey: rememberedPaneKey, - pid: - typeof spawnedPid === 'number' && Number.isFinite(spawnedPid) && spawnedPid > 0 - ? spawnedPid - : null - }) - } - // Why: telemetry-plan.md§Agent launch semantics — fire `agent_started` - // only after `provider.spawn` resolved. The renderer threads - // `args.telemetry` through the spawn IPC for every launch we want to - // attribute; bare-shell tabs (no agent) leave the field undefined and - // do not produce an event. Each field is parsed against its closed - // enum here so a malformed renderer payload (or a spoofed IPC) does - // not poison the event — `safeParse` failure drops that field, and - // if any required field is missing we skip the event entirely. The - // main-side `track()` validator re-runs the schema on the full - // payload as a second defense-in-depth check. - if (args.telemetry) { - const agentKindParse = agentKindSchema.safeParse(args.telemetry.agent_kind) - const launchSourceParse = launchSourceSchema.safeParse(args.telemetry.launch_source) - const requestKindParse = requestKindSchema.safeParse(args.telemetry.request_kind) - if (agentKindParse.success && launchSourceParse.success && requestKindParse.success) { - track('agent_started', { - agent_kind: agentKindParse.data, - launch_source: launchSourceParse.data, - request_kind: requestKindParse.data, - ...getCohortAtEmit() - }) + ) { + runtime?.registerPty( + result.id, + args.worktreeId, + args.connectionId ?? null, + // Why: pass the validated pane identity so a mobile create waiting on + // this renderer tab can publish its surface main-side when graph-sync + // is throttled, instead of destroying the live PTY (#7587). Bound the + // untrusted tabId like the sibling metadataPaneKey/spawnOptions.tabId. + typeof args.tabId === 'string' && + isValidTerminalTabId(args.tabId) && + args.tabId.length <= 512 && + metadataLeafId !== null + ? { tabId: args.tabId, leafId: metadataLeafId } + : undefined + ) + } + // Why: arms main's per-PTY Command Code output detector from the launch + // command (renderer startupCommand parity); banner detection covers + // PTYs spawned without one. + runtime?.noteTerminalSpawnCommand?.( + result.id, + typeof args.command === 'string' ? args.command : null + ) + if (isClaudeLaunch) { + markClaudePtySpawned(result.id) + } + // Why: renderer sets ORCA_PANE_KEY in `args.env` for every pane-owned + // spawn (see pty-connection.ts). Recording the mapping here lets + // clearProviderPtyState clear the agent-hooks server's per-paneKey + // caches when the PTY exits. + // Why: args.env arrives as untrusted JSON over IPC — the static + // Record type is not actually enforced at the boundary. + // Narrow to a bounded string so malformed or oversized values cannot + // pollute ptyPaneKey or the downstream clearPaneState call. + const rememberedPaneKey = validatedPaneKey + ? rememberPaneKeyForPty(result.id, validatedPaneKey) + : null + if (legacySpawnPaneKey && migrationUnsupportedPaneKey) { + agentHookServer.registerPaneKeyAlias( + legacySpawnPaneKey.paneKey, + migrationUnsupportedPaneKey, + result.id + ) + clearMigrationUnsupportedPtysForPaneKey(migrationUnsupportedPaneKey) + } else if (validatedPaneKey) { + if (!result.isReattach) { + clearMigrationUnsupportedPtysForPaneKey(validatedPaneKey) + } + } + // Why: register local PTYs (connectionId falsy) with the memory + // collector so it can walk each PTY's process subtree and attribute + // memory back to its worktree. SSH PTYs execute remotely and their + // process tree is not visible to our local `ps`, so we skip them. + if (!args.connectionId) { + // Why: providers publish the OS pid on the spawn result (both + // LocalPtyProvider and DaemonPtyAdapter). Recording it once here keeps + // the memory module from reaching back into ipc/pty on a hot path, and + // works uniformly whether the PTY is hosted in-process or by the + // daemon subprocess. + const spawnedPid = result.pid ?? null + // Why: args.worktreeId and args.sessionId arrive as untrusted IPC + // payload strings — the static type is not enforced at the boundary. + // Narrow them to bounded strings here to match the paneKey defense + // above so malformed or oversized values cannot pollute registerPty's + // maps or downstream memory-attribution lookups. + registerPty({ + ptyId: result.id, + worktreeId: + typeof args.worktreeId === 'string' && + args.worktreeId.length > 0 && + args.worktreeId.length <= 512 + ? args.worktreeId + : null, + sessionId: + typeof args.sessionId === 'string' && + args.sessionId.length > 0 && + args.sessionId.length <= 256 + ? args.sessionId + : null, + paneKey: rememberedPaneKey, + pid: + typeof spawnedPid === 'number' && Number.isFinite(spawnedPid) && spawnedPid > 0 + ? spawnedPid + : null + }) + } + // Why: telemetry-plan.md§Agent launch semantics — fire `agent_started` + // only after `provider.spawn` resolved. The renderer threads + // `args.telemetry` through the spawn IPC for every launch we want to + // attribute; bare-shell tabs (no agent) leave the field undefined and + // do not produce an event. Each field is parsed against its closed + // enum here so a malformed renderer payload (or a spoofed IPC) does + // not poison the event — `safeParse` failure drops that field, and + // if any required field is missing we skip the event entirely. The + // main-side `track()` validator re-runs the schema on the full + // payload as a second defense-in-depth check. + if (args.telemetry) { + const agentKindParse = agentKindSchema.safeParse(args.telemetry.agent_kind) + const launchSourceParse = launchSourceSchema.safeParse(args.telemetry.launch_source) + const requestKindParse = requestKindSchema.safeParse(args.telemetry.request_kind) + if (agentKindParse.success && launchSourceParse.success && requestKindParse.success) { + track('agent_started', { + agent_kind: agentKindParse.data, + launch_source: launchSourceParse.data, + request_kind: requestKindParse.data, + ...getCohortAtEmit() + }) + } + } + const response = { + ...result, + ...(!result.isReattach && effectiveLaunchConfig + ? { launchConfig: effectiveLaunchConfig } + : {}), + // Why: a daemon-retry race can surface isReattach even for a minted + // session id, and a reattach must never claim its cwd was remapped. + ...(startupCwdFallback && !result.isReattach ? { startupCwdFallback } : {}) + } + return resolvePaneSpawnReservation(reservationPaneKey, paneSpawnReservation, response) + } catch (err) { + // Why: once the reservation is created, any later throw — + // spawn failure, persist failure, or a post-spawn helper such as + // seedHeadlessTerminal/registerPty/track — must settle it. Otherwise + // it lingers in paneSpawnReservationsByPaneKey and every future spawn + // for this pane awaits a promise that never resolves. reject is a + // no-op once the reservation has already resolved. + rejectPaneSpawnReservation(reservationPaneKey, paneSpawnReservation, err) + throw err } - } - const response = { - ...result, - ...(!result.isReattach && effectiveLaunchConfig - ? { launchConfig: effectiveLaunchConfig } - : {}), - // Why: a daemon-retry race can surface isReattach even for a minted - // session id, and a reattach must never claim its cwd was remapped. - ...(startupCwdFallback && !result.isReattach ? { startupCwdFallback } : {}) - } - return resolvePaneSpawnReservation(reservationPaneKey, paneSpawnReservation, response) - } catch (err) { - // Why: once the reservation is created, any later throw — - // spawn failure, persist failure, or a post-spawn helper such as - // seedHeadlessTerminal/registerPty/track — must settle it. Otherwise - // it lingers in paneSpawnReservationsByPaneKey and every future spawn - // for this pane awaits a promise that never resolves. reject is a - // no-op once the reservation has already resolved. - rejectPaneSpawnReservation(reservationPaneKey, paneSpawnReservation, err) - throw err - } - }, args.connectionId) + }, + args.connectionId + ) ) const writePtyProviderInputWithinLimit = ( @@ -5013,8 +5233,8 @@ export function registerPtyHandlers( const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null const connectionId = ownedConnectionId ?? parsedSshId?.connectionId const retainedRetry = pendingSshShutdownRetries.get(args.id) - if (retainedRetry) { - await retainedRetry + if (retainedRetry?.inFlight) { + await retainedRetry.inFlight return } const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id) @@ -5025,14 +5245,19 @@ export function registerPtyHandlers( throw new Error('SSH PTY provider unavailable') } const shutdownProvider = provider ?? getProviderForPty(args.id) + const shutdownOptions: PtyShutdownOptions = { + immediate: true, + keepHistory: args.keepHistory ?? false, + ...(args.expectedPaneKey ? { expectedPaneKey: args.expectedPaneKey } : {}), + ...(args.expectedTabId ? { expectedTabId: args.expectedTabId } : {}) + } let providerExitObserved = false try { - providerExitObserved = await shutdownProviderAndDetectExit(shutdownProvider, args.id, { - immediate: true, - keepHistory: args.keepHistory ?? false, - ...(args.expectedPaneKey ? { expectedPaneKey: args.expectedPaneKey } : {}), - ...(args.expectedTabId ? { expectedTabId: args.expectedTabId } : {}) - }) + providerExitObserved = await shutdownProviderAndDetectExit( + shutdownProvider, + args.id, + shutdownOptions + ) } catch (err) { if (!isPtyAlreadyGoneError(err)) { // Why: a failed SSH shutdown can leave the remote process alive in @@ -5040,6 +5265,10 @@ export function registerPtyHandlers( // Keep ownership/lease state so the user can retry. if (connectionId) { store?.markSshRemotePtyShutdownRequested?.(connectionId, args.id) + retryPersistedSshShutdowns(connectionId, shutdownProvider) + } else { + // Why: renderer identity is already retired; persist exact daemon identity before retry. + retainLocalPtyShutdown(args.id, shutdownOptions, 1) } throw err } @@ -5048,6 +5277,13 @@ export function registerPtyHandlers( // Why: some shutdown paths do not emit onExit through the provider listener. // Explicit cleanup is idempotent and covers already-dead PTYs. finishPtyShutdown(args.id, connectionId, store) + if (!connectionId) { + pendingLocalShutdownRetries.delete(args.id) + if (typeof store?.removePendingLocalPtyShutdown === 'function') { + store.removePendingLocalPtyShutdown(args.id) + } + scheduleRetainedShutdownRetries() + } if (!providerExitObserved) { runtime?.onPtyExit(args.id, -1) rememberSyntheticKillExit(args.id) diff --git a/src/main/ipc/runtime-environments.test.ts b/src/main/ipc/runtime-environments.test.ts index 3dbcac7a7ec..d79cb9e192e 100644 --- a/src/main/ipc/runtime-environments.test.ts +++ b/src/main/ipc/runtime-environments.test.ts @@ -499,6 +499,148 @@ describe('registerRuntimeEnvironmentHandlers', () => { expect(sendRemoteRuntimeRequestMock).not.toHaveBeenCalled() }) + it('persists and retries exact terminal closes after renderer loss', async () => { + vi.useFakeTimers() + const upsertClose = vi.fn() + const removeClose = vi.fn() + Object.assign(store, { + getPendingRuntimeTerminalCloses: () => [], + upsertPendingRuntimeTerminalClose: upsertClose, + removePendingRuntimeTerminalClose: removeClose + }) + registerRuntimeEnvironmentHandlers(store as never) + let closeAttempt = 0 + sendRemoteRuntimeRequestMock.mockImplementation(async (_pairing, method) => { + if (method === 'status.get') { + return { + id: 'status', + ok: true, + result: { runtimeId: 'runtime-remote', capabilities: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + } + closeAttempt += 1 + if (closeAttempt < 3) { + return { + id: `close-${closeAttempt}`, + ok: false, + error: { message: closeAttempt === 1 ? 'offline' : 'still offline' } + } + } + return { + id: 'close-3', + ok: true, + result: { close: true }, + _meta: { runtimeId: 'runtime-remote' } + } + }) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const add = handler< + { name: string; pairingCode: string }, + { environment: { id: string; name: string } } + >('runtimeEnvironments:addFromPairingCode') + const added = await add(null, { name: 'desk', pairingCode: pairingCode() }) + const call = handler<{ selector: string; method: string; params?: unknown }, { ok: boolean }>( + 'runtimeEnvironments:call' + ) + await expect( + call(null, { selector: 'desk', method: 'terminal.close', params: { terminal: 't1' } }) + ).resolves.toMatchObject({ ok: false }) + + await vi.advanceTimersByTimeAsync(250) + await vi.advanceTimersByTimeAsync(500) + + expect(closeAttempt).toBe(3) + expect(upsertClose).toHaveBeenCalledWith({ + environmentId: added.environment.id, + handle: 't1', + requestedAt: expect.any(Number) + }) + expect(removeClose).toHaveBeenCalledWith(added.environment.id, 't1') + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + + it('hydrates and drains persisted terminal close intent after app restart', async () => { + vi.useFakeTimers() + const environment = environmentStore.addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: pairingCode() + }) + const removePendingRuntimeTerminalClose = vi.fn() + Object.assign(store, { + getPendingRuntimeTerminalCloses: () => [ + { environmentId: environment.id, handle: 't1', requestedAt: 1 } + ], + upsertPendingRuntimeTerminalClose: vi.fn(), + removePendingRuntimeTerminalClose + }) + sendRemoteRuntimeRequestMock.mockImplementation(async (_pairing, method) => + method === 'status.get' + ? { + id: 'status', + ok: true, + result: { runtimeId: 'runtime-remote', capabilities: [] }, + _meta: { runtimeId: 'runtime-remote' } + } + : { + id: 'close', + ok: true, + result: { close: true }, + _meta: { runtimeId: 'runtime-remote' } + } + ) + + registerRuntimeEnvironmentHandlers(store as never) + await vi.advanceTimersByTimeAsync(0) + + expect(sendRemoteRuntimeRequestMock.mock.calls.map((call) => call[1])).toEqual([ + 'status.get', + 'status.get', + 'terminal.close' + ]) + expect(removePendingRuntimeTerminalClose).toHaveBeenCalledWith(environment.id, 't1') + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + + it('does not apply persisted close intent to a replacement runtime generation', async () => { + vi.useFakeTimers() + const environment = environmentStore.addEnvironmentFromPairingCode(userDataPath, { + name: 'desk', + pairingCode: pairingCode() + }) + environmentStore.markEnvironmentUsed(userDataPath, environment.id, { runtimeId: 'runtime-a' }) + const removePendingRuntimeTerminalClose = vi.fn() + Object.assign(store, { + getPendingRuntimeTerminalCloses: () => [ + { + environmentId: environment.id, + handle: 't1', + runtimeId: 'runtime-a', + requestedAt: 1 + } + ], + upsertPendingRuntimeTerminalClose: vi.fn(), + removePendingRuntimeTerminalClose + }) + sendRemoteRuntimeRequestMock.mockResolvedValue({ + id: 'status', + ok: true, + result: { runtimeId: 'runtime-b', capabilities: [] }, + _meta: { runtimeId: 'runtime-b' } + }) + + registerRuntimeEnvironmentHandlers(store as never) + await vi.advanceTimersByTimeAsync(0) + + expect(sendRemoteRuntimeRequestMock.mock.calls.map((call) => call[1])).toEqual(['status.get']) + expect(removePendingRuntimeTerminalClose).toHaveBeenCalledWith(environment.id, 't1') + expect(vi.getTimerCount()).toBe(0) + vi.useRealTimers() + }) + it('keeps terminal hot path RPCs on the cached request connection when shared control is supported', async () => { registerRuntimeEnvironmentHandlers(store as never) sendRemoteRuntimeRequestMock.mockResolvedValue({ diff --git a/src/main/ipc/runtime-environments.ts b/src/main/ipc/runtime-environments.ts index 0c1aca30cd5..714eb6c9f07 100644 --- a/src/main/ipc/runtime-environments.ts +++ b/src/main/ipc/runtime-environments.ts @@ -23,6 +23,11 @@ import { resetSharedControlSupport, subscribeRuntimeEnvironment } from './runtime-environment-transport-routing' +import { + closeRuntimeTerminalWithRetryOwnership, + initializeRuntimeTerminalCloseRetryOwner, + releaseRuntimeTerminalClosesForEnvironment +} from './runtime-terminal-close-retry-owner' const RUNTIME_ENVIRONMENT_HANDLER_CHANNELS = [ 'runtimeEnvironments:list', @@ -66,6 +71,7 @@ function listPublicRuntimeEnvironments(): PublicKnownRuntimeEnvironment[] { } export function registerRuntimeEnvironmentHandlers(store: Store): void { + initializeRuntimeTerminalCloseRetryOwner(store, getUserDataPath()) // Why: keep direct re-registration safe even though register-core-handlers // normally guards this path; otherwise the binary send listener can stack. resetSharedControlSupport() @@ -103,6 +109,8 @@ export function registerRuntimeEnvironmentHandlers(store: Store): void { } clearActiveRuntimeEnvironmentFocusIfMatches(store, removed.id) closeSubscriptionsForEnvironment(removed.id) + // Why: removal is authoritative; no future transport can reach these handles. + releaseRuntimeTerminalClosesForEnvironment(removed.id) return { removed: redactRuntimeEnvironment(removed) } } ) @@ -137,6 +145,19 @@ export function registerRuntimeEnvironmentHandlers(store: Store): void { _event, args: { selector: string; method: string; params?: unknown; timeoutMs?: number } ): Promise> => { + if ( + args.method === 'terminal.close' && + args.params && + typeof args.params === 'object' && + typeof (args.params as { terminal?: unknown }).terminal === 'string' + ) { + const environment = resolveEnvironment(getUserDataPath(), args.selector) + return closeRuntimeTerminalWithRetryOwnership( + environment.id, + (args.params as { terminal: string }).terminal, + environment.runtimeId + ) + } return callRuntimeEnvironment( getUserDataPath(), args.selector, diff --git a/src/main/ipc/runtime-terminal-close-retry-owner.ts b/src/main/ipc/runtime-terminal-close-retry-owner.ts new file mode 100644 index 00000000000..9180b13e71a --- /dev/null +++ b/src/main/ipc/runtime-terminal-close-retry-owner.ts @@ -0,0 +1,226 @@ +import type { RuntimeRpcResponse } from '../../shared/runtime-rpc-envelope' +import type { Store } from '../persistence' +import { + callRuntimeEnvironment, + getRuntimeEnvironmentStatus +} from './runtime-environment-transport-routing' + +type RetainedTerminalClose = { + generation: number + environmentId: string + handle: string + runtimeId: string | null + attempts: number + nextRetryAt: number + inFlight: Promise> | null +} + +const retainedCloses = new Map() +const MAX_BACKOFF_MS = 30_000 +let retryTimer: ReturnType | null = null +let store: Store | null = null +let userDataPath = '' +let ownerGeneration = 0 + +function key(environmentId: string, handle: string): string { + return `${environmentId}\0${handle}` +} + +function schedule(): void { + if (retryTimer) { + clearTimeout(retryTimer) + } + retryTimer = null + let nextRetryAt = Number.POSITIVE_INFINITY + for (const close of retainedCloses.values()) { + if (!close.inFlight) { + nextRetryAt = Math.min(nextRetryAt, close.nextRetryAt) + } + } + if (!Number.isFinite(nextRetryAt)) { + return + } + retryTimer = setTimeout( + () => { + retryTimer = null + const now = Date.now() + for (const close of retainedCloses.values()) { + if (!close.inFlight && close.nextRetryAt <= now) { + void attempt(close) + } + } + }, + Math.max(0, nextRetryAt - Date.now()) + ) +} + +function attempt(close: RetainedTerminalClose): Promise> { + if (close.inFlight) { + return close.inFlight + } + const closeKey = key(close.environmentId, close.handle) + const noOpResponse = (runtimeId?: string | null) => ({ + id: 'terminal.close', + ok: true as const, + result: { close: false, reason: 'retry_owner_replaced' }, + _meta: { runtimeId: runtimeId ?? close.runtimeId ?? 'unknown-runtime' } + }) + const request = getRuntimeEnvironmentStatus(userDataPath, close.environmentId) + .then((status) => { + if (close.generation !== ownerGeneration) { + return noOpResponse(status._meta?.runtimeId) + } + if (!status.ok) { + throw Object.assign(new Error(status.error.message), { response: status }) + } + const currentRuntimeId = status._meta.runtimeId + if (close.runtimeId && close.runtimeId !== currentRuntimeId) { + retainedCloses.delete(closeKey) + store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle) + return { + id: 'terminal.close', + ok: true as const, + result: { close: false, reason: 'runtime_replaced' }, + _meta: { runtimeId: currentRuntimeId } + } + } + if (!close.runtimeId) { + close.runtimeId = currentRuntimeId + store?.upsertPendingRuntimeTerminalClose?.({ + environmentId: close.environmentId, + handle: close.handle, + runtimeId: currentRuntimeId, + requestedAt: Date.now() + }) + } + return callRuntimeEnvironment(userDataPath, close.environmentId, 'terminal.close', { + terminal: close.handle, + expectedRuntimeId: close.runtimeId + }) + }) + .then((response) => { + if (close.generation !== ownerGeneration) { + return noOpResponse(response._meta?.runtimeId) + } + if (!response.ok) { + const responseRuntimeId = response._meta?.runtimeId + if (close.runtimeId && responseRuntimeId && responseRuntimeId !== close.runtimeId) { + retainedCloses.delete(closeKey) + store?.removePendingRuntimeTerminalClose?.(close.environmentId, close.handle) + return { + id: 'terminal.close', + ok: true as const, + result: { close: false, reason: 'runtime_replaced' }, + _meta: { runtimeId: responseRuntimeId } + } + } + throw Object.assign(new Error(response.error.message), { response }) + } + retainedCloses.delete(closeKey) + if (typeof store?.removePendingRuntimeTerminalClose === 'function') { + store.removePendingRuntimeTerminalClose(close.environmentId, close.handle) + } + return response + }) + .catch((error: unknown) => { + if (close.generation !== ownerGeneration) { + return noOpResponse() + } + close.attempts += 1 + close.nextRetryAt = + Date.now() + Math.min(MAX_BACKOFF_MS, 250 * 2 ** Math.min(close.attempts - 1, 7)) + if (close.attempts <= 2 || close.attempts === 8) { + console.warn('[runtime-environments] retained terminal close failed:', error) + } + const response = (error as { response?: RuntimeRpcResponse }).response + if (response) { + return response + } + throw error + }) + .finally(() => { + if (close.generation !== ownerGeneration) { + return + } + if (close.inFlight === request) { + close.inFlight = null + } + schedule() + }) + close.inFlight = request + return request +} + +function retain( + environmentId: string, + handle: string, + runtimeId?: string | null +): RetainedTerminalClose { + const closeKey = key(environmentId, handle) + let close = retainedCloses.get(closeKey) + if (!close) { + close = { + generation: ownerGeneration, + environmentId, + handle, + runtimeId: runtimeId ?? null, + attempts: 0, + nextRetryAt: 0, + inFlight: null + } + retainedCloses.set(closeKey, close) + if (typeof store?.upsertPendingRuntimeTerminalClose === 'function') { + store.upsertPendingRuntimeTerminalClose({ + environmentId, + handle, + ...(runtimeId ? { runtimeId } : {}), + requestedAt: Date.now() + }) + } + } else if (!close.runtimeId && runtimeId) { + close.runtimeId = runtimeId + } + return close +} + +export function initializeRuntimeTerminalCloseRetryOwner( + nextStore: Store, + nextUserDataPath: string +): void { + ownerGeneration += 1 + if (retryTimer) { + clearTimeout(retryTimer) + retryTimer = null + } + retainedCloses.clear() + store = nextStore + userDataPath = nextUserDataPath + const persisted = + typeof store.getPendingRuntimeTerminalCloses === 'function' + ? store.getPendingRuntimeTerminalCloses() + : [] + for (const close of persisted) { + retain(close.environmentId, close.handle, close.runtimeId) + } + schedule() +} + +export function closeRuntimeTerminalWithRetryOwnership( + environmentId: string, + handle: string, + runtimeId?: string | null +): Promise> { + return attempt(retain(environmentId, handle, runtimeId)) +} + +export function releaseRuntimeTerminalClosesForEnvironment(environmentId: string): void { + for (const [closeKey, close] of retainedCloses) { + if (close.environmentId === environmentId) { + retainedCloses.delete(closeKey) + } + } + if (typeof store?.removePendingRuntimeTerminalClosesForEnvironment === 'function') { + store.removePendingRuntimeTerminalClosesForEnvironment(environmentId) + } + schedule() +} diff --git a/src/main/ipc/ssh.test.ts b/src/main/ipc/ssh.test.ts index 51801ecdef5..4ed966eb09f 100644 --- a/src/main/ipc/ssh.test.ts +++ b/src/main/ipc/ssh.test.ts @@ -157,6 +157,7 @@ vi.mock('../providers/ssh-filesystem-provider', () => ({ vi.mock('./pty', () => ({ registerSshPtyProvider: vi.fn(), unregisterSshPtyProvider: vi.fn(), + releasePendingSshShutdownsForTarget: vi.fn(), clearPtyOwnershipForConnection: vi.fn(), clearProviderPtyState: vi.fn(), deletePtyOwnership: vi.fn(), @@ -1218,8 +1219,16 @@ describe('SSH IPC handlers', () => { expect(clearProviderPtyState).toHaveBeenCalledWith('ssh:ssh-1@@pty-lease') expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:ssh-1@@pty-live') expect(deletePtyOwnership).toHaveBeenCalledWith('ssh:ssh-1@@pty-lease') - expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'pty-live', 'terminated') - expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'pty-lease', 'terminated') + expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-1', + 'ssh:ssh-1@@pty-live', + 'terminated' + ) + expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith( + 'ssh-1', + 'ssh:ssh-1@@pty-lease', + 'terminated' + ) }) it('ssh:terminateSessions ignores expired leases when disconnected', async () => { diff --git a/src/main/ipc/ssh.ts b/src/main/ipc/ssh.ts index 31ee28373a4..88f0655c7d3 100644 --- a/src/main/ipc/ssh.ts +++ b/src/main/ipc/ssh.ts @@ -1004,7 +1004,7 @@ export function registerSshHandlers( } clearProviderPtyState(appPtyId) deletePtyOwnership(appPtyId) - persistedStore!.markSshRemotePtyLease(args.targetId, relayPtyId, 'terminated') + persistedStore!.markSshRemotePtyLease(args.targetId, appPtyId, 'terminated') } if (shutdownFailures.length > 0) { // Why: a failed relay shutdown can leave the remote process alive in the diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index b1d3b4b6c39..2cd5873d43d 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -8802,6 +8802,80 @@ describe('Store', () => { ]) }) + it('replaces teardown identity when a new relay boot reuses a raw PTY id', async () => { + const store = await createStore() + store.upsertSshRemotePtyLease({ + targetId: 'ssh-1', + ptyId: 'ssh:ssh-1@@boot-a@@remote-pty', + state: 'expired', + shutdownRequestedAt: 10 + }) + + store.upsertSshRemotePtyLease({ + targetId: 'ssh-1', + ptyId: 'ssh:ssh-1@@boot-b@@remote-pty', + state: 'attached' + }) + store.markSshRemotePtyShutdownRequested('ssh-1', 'ssh:ssh-1@@boot-a@@remote-pty') + + expect(store.getSshRemotePtyLeases('ssh-1')).toEqual([ + expect.objectContaining({ + ptyId: 'remote-pty', + relayInstanceId: 'boot-b', + state: 'attached' + }) + ]) + expect(store.getSshRemotePtyLeases('ssh-1')[0]).not.toHaveProperty('shutdownRequestedAt') + }) + + it('preserves SSH relay generation across save and reload', async () => { + const store = await createStore() + store.upsertSshRemotePtyLease({ + targetId: 'ssh-1', + ptyId: 'ssh:ssh-1@@boot-a@@remote-pty', + state: 'detached' + }) + + const reloaded = await createStore() + expect(reloaded.getSshRemotePtyLeases('ssh-1')[0]).toEqual( + expect.objectContaining({ relayInstanceId: 'boot-a' }) + ) + }) + + it('preserves exact local and runtime close intent across app restart', async () => { + const store = await createStore() + store.upsertPendingLocalPtyShutdown({ + ptyId: 'local-pty', + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab', + requestedAt: 10 + }) + store.upsertPendingRuntimeTerminalClose({ + environmentId: 'env-1', + handle: 'terminal-1', + runtimeId: 'runtime-a', + requestedAt: 20 + }) + + const reloaded = await createStore() + expect(reloaded.getPendingLocalPtyShutdowns()).toEqual([ + { + ptyId: 'local-pty', + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab', + requestedAt: 10 + } + ]) + expect(reloaded.getPendingRuntimeTerminalCloses()).toEqual([ + { + environmentId: 'env-1', + handle: 'terminal-1', + runtimeId: 'runtime-a', + requestedAt: 20 + } + ]) + }) + it('rejects mismatched scoped SSH remote PTY lease ids on write paths', async () => { const store = await createStore() diff --git a/src/main/persistence.ts b/src/main/persistence.ts index b890935ef9a..fd75c4f7622 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -65,7 +65,9 @@ import type { TerminalLayoutSnapshot, TerminalTab, WorkspaceSessionPatch, - WorkspaceSessionState + WorkspaceSessionState, + PersistedLocalPtyShutdown, + PersistedRuntimeTerminalClose } from '../shared/types' import { deriveGlobalWindowsRuntimeDefaultFromLegacySettings, @@ -1577,6 +1579,7 @@ function normalizeSshRemotePtyLease(value: unknown): SshRemotePtyLease | null { return { targetId: raw.targetId, ptyId: raw.ptyId, + ...(typeof raw.relayInstanceId === 'string' ? { relayInstanceId: raw.relayInstanceId } : {}), ...(typeof raw.worktreeId === 'string' ? { worktreeId: raw.worktreeId } : {}), ...(typeof raw.tabId === 'string' ? { tabId: raw.tabId } : {}), ...(typeof raw.leafId === 'string' && raw.leafId.length <= 256 ? { leafId: raw.leafId } : {}), @@ -1591,6 +1594,42 @@ function normalizeSshRemotePtyLease(value: unknown): SshRemotePtyLease | null { } } +function normalizeLocalPtyShutdown(value: unknown): PersistedLocalPtyShutdown | null { + if (!value || typeof value !== 'object') { + return null + } + const raw = value as Partial + if (typeof raw.ptyId !== 'string' || typeof raw.requestedAt !== 'number') { + return null + } + return { + ptyId: raw.ptyId, + requestedAt: raw.requestedAt, + ...(typeof raw.expectedPaneKey === 'string' ? { expectedPaneKey: raw.expectedPaneKey } : {}), + ...(typeof raw.expectedTabId === 'string' ? { expectedTabId: raw.expectedTabId } : {}) + } +} + +function normalizeRuntimeTerminalClose(value: unknown): PersistedRuntimeTerminalClose | null { + if (!value || typeof value !== 'object') { + return null + } + const raw = value as Partial + if ( + typeof raw.environmentId !== 'string' || + typeof raw.handle !== 'string' || + typeof raw.requestedAt !== 'number' + ) { + return null + } + return { + environmentId: raw.environmentId, + handle: raw.handle, + ...(typeof raw.runtimeId === 'string' ? { runtimeId: raw.runtimeId } : {}), + requestedAt: raw.requestedAt + } +} + type LayoutLeafNormalization = { snapshot: TerminalLayoutSnapshot changed: boolean @@ -3428,6 +3467,12 @@ export class Store { sshRemotePtyLeases: (parsed.sshRemotePtyLeases ?? []) .map(normalizeSshRemotePtyLease) .filter((lease): lease is SshRemotePtyLease => lease !== null), + pendingLocalPtyShutdowns: (parsed.pendingLocalPtyShutdowns ?? []) + .map(normalizeLocalPtyShutdown) + .filter((request): request is PersistedLocalPtyShutdown => request !== null), + pendingRuntimeTerminalCloses: (parsed.pendingRuntimeTerminalCloses ?? []) + .map(normalizeRuntimeTerminalClose) + .filter((request): request is PersistedRuntimeTerminalClose => request !== null), claudeLivePtySessionIds: normalizeClaudeLivePtySessionIds(parsed.claudeLivePtySessionIds), migrationUnsupportedPtyEntries: normalizeMigrationUnsupportedPtyEntries( parsed.migrationUnsupportedPtyEntries @@ -6311,6 +6356,70 @@ export class Store { return leases.filter((lease) => targetId === undefined || lease.targetId === targetId) } + getPendingLocalPtyShutdowns(): PersistedLocalPtyShutdown[] { + return [...(this.state.pendingLocalPtyShutdowns ?? [])] + } + + upsertPendingLocalPtyShutdown(request: PersistedLocalPtyShutdown): void { + this.state.pendingLocalPtyShutdowns ??= [] + const index = this.state.pendingLocalPtyShutdowns.findIndex( + (entry) => entry.ptyId === request.ptyId + ) + if (index >= 0) { + this.state.pendingLocalPtyShutdowns[index] = request + } else { + this.state.pendingLocalPtyShutdowns.push(request) + } + this.flush() + } + + removePendingLocalPtyShutdown(ptyId: string): void { + const before = this.state.pendingLocalPtyShutdowns?.length ?? 0 + this.state.pendingLocalPtyShutdowns = (this.state.pendingLocalPtyShutdowns ?? []).filter( + (entry) => entry.ptyId !== ptyId + ) + if (this.state.pendingLocalPtyShutdowns.length !== before) { + this.flush() + } + } + + getPendingRuntimeTerminalCloses(): PersistedRuntimeTerminalClose[] { + return [...(this.state.pendingRuntimeTerminalCloses ?? [])] + } + + upsertPendingRuntimeTerminalClose(request: PersistedRuntimeTerminalClose): void { + this.state.pendingRuntimeTerminalCloses ??= [] + const index = this.state.pendingRuntimeTerminalCloses.findIndex( + (entry) => entry.environmentId === request.environmentId && entry.handle === request.handle + ) + if (index >= 0) { + this.state.pendingRuntimeTerminalCloses[index] = request + } else { + this.state.pendingRuntimeTerminalCloses.push(request) + } + this.flush() + } + + removePendingRuntimeTerminalClose(environmentId: string, handle: string): void { + const before = this.state.pendingRuntimeTerminalCloses?.length ?? 0 + this.state.pendingRuntimeTerminalCloses = ( + this.state.pendingRuntimeTerminalCloses ?? [] + ).filter((entry) => entry.environmentId !== environmentId || entry.handle !== handle) + if (this.state.pendingRuntimeTerminalCloses.length !== before) { + this.flush() + } + } + + removePendingRuntimeTerminalClosesForEnvironment(environmentId: string): void { + const before = this.state.pendingRuntimeTerminalCloses?.length ?? 0 + this.state.pendingRuntimeTerminalCloses = ( + this.state.pendingRuntimeTerminalCloses ?? [] + ).filter((entry) => entry.environmentId !== environmentId) + if (this.state.pendingRuntimeTerminalCloses.length !== before) { + this.flush() + } + } + upsertSshRemotePtyLease( lease: Omit & Partial> @@ -6334,10 +6443,13 @@ export class Store { entry.targetId === normalizedLease.targetId && entry.ptyId === normalizedLease.ptyId ) const existing = existingIndex >= 0 ? this.state.sshRemotePtyLeases[existingIndex] : undefined + const sameGeneration = existing?.relayInstanceId === normalizedLease.relayInstanceId const next: SshRemotePtyLease = { - ...existing, + ...(sameGeneration ? existing : undefined), ...normalizedLease, - createdAt: existing?.createdAt ?? normalizedLease.createdAt ?? now, + createdAt: sameGeneration + ? (existing?.createdAt ?? normalizedLease.createdAt ?? now) + : (normalizedLease.createdAt ?? now), updatedAt: normalizedLease.updatedAt ?? now } if (existingIndex >= 0) { @@ -6384,9 +6496,13 @@ export class Store { } markSshRemotePtyLease(targetId: string, ptyId: string, state: SshRemotePtyLease['state']): void { + const requestedGeneration = parseAppSshPtyId(ptyId)?.relayInstanceId const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId) const lease = this.state.sshRemotePtyLeases?.find( - (entry) => entry.targetId === targetId && entry.ptyId === relayPtyId + (entry) => + entry.targetId === targetId && + entry.ptyId === relayPtyId && + (requestedGeneration === undefined || entry.relayInstanceId === requestedGeneration) ) if (!lease) { return @@ -6413,9 +6529,13 @@ export class Store { } markSshRemotePtyShutdownRequested(targetId: string, ptyId: string): void { + const requestedGeneration = parseAppSshPtyId(ptyId)?.relayInstanceId const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId) const lease = this.state.sshRemotePtyLeases?.find( - (entry) => entry.targetId === targetId && entry.ptyId === relayPtyId + (entry) => + entry.targetId === targetId && + entry.ptyId === relayPtyId && + (requestedGeneration === undefined || entry.relayInstanceId === requestedGeneration) ) if (!lease) { return @@ -6426,14 +6546,21 @@ export class Store { } removeSshRemotePtyLease(targetId: string, ptyId: string): void { + const requestedGeneration = parseAppSshPtyId(ptyId)?.relayInstanceId const relayPtyId = this.getRelayPtyIdForSshLeaseStorage(targetId, ptyId) const leases = (this.state.sshRemotePtyLeases ?? []).filter( - (lease) => lease.targetId === targetId && lease.ptyId === relayPtyId + (lease) => + lease.targetId === targetId && + lease.ptyId === relayPtyId && + (requestedGeneration === undefined || lease.relayInstanceId === requestedGeneration) ) const before = this.state.sshRemotePtyLeases?.length ?? 0 this.clearSshRemotePtyBindingsForLeases(targetId, leases) this.state.sshRemotePtyLeases = (this.state.sshRemotePtyLeases ?? []).filter( - (lease) => lease.targetId !== targetId || lease.ptyId !== relayPtyId + (lease) => + lease.targetId !== targetId || + lease.ptyId !== relayPtyId || + (requestedGeneration !== undefined && lease.relayInstanceId !== requestedGeneration) ) if (this.state.sshRemotePtyLeases.length !== before) { this.flush() diff --git a/src/main/runtime/rpc/methods/terminal.ts b/src/main/runtime/rpc/methods/terminal.ts index 7a9b9e448a6..d252651b56b 100644 --- a/src/main/runtime/rpc/methods/terminal.ts +++ b/src/main/runtime/rpc/methods/terminal.ts @@ -735,6 +735,10 @@ const TerminalHandle = z.object({ terminal: requiredString('Missing terminal handle') }) +const TerminalClose = TerminalHandle.extend({ + expectedRuntimeId: z.string().min(1).optional() +}) + const TerminalListParams = z.object({ worktree: OptionalString, limit: OptionalFiniteNumber, @@ -1363,10 +1367,13 @@ export const TERMINAL_METHODS: RpcAnyMethod[] = [ }), defineMethod({ name: 'terminal.close', - params: TerminalHandle, - handler: async (params, { runtime }) => ({ - close: await runtime.closeTerminal(params.terminal) - }) + params: TerminalClose, + handler: async (params, { runtime }) => { + if (params.expectedRuntimeId && params.expectedRuntimeId !== runtime.getRuntimeId()) { + throw new Error('runtime_generation_mismatch') + } + return { close: await runtime.closeTerminal(params.terminal) } + } }), defineMethod({ name: 'agentTeams.tmuxCompat', diff --git a/src/main/runtime/rpc/terminal-close-generation.test.ts b/src/main/runtime/rpc/terminal-close-generation.test.ts new file mode 100644 index 00000000000..c6111000149 --- /dev/null +++ b/src/main/runtime/rpc/terminal-close-generation.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from 'vitest' +import type { OrcaRuntimeService } from '../orca-runtime' +import type { RpcRequest } from './core' +import { RpcDispatcher } from './dispatcher' +import { TERMINAL_METHODS } from './methods/terminal' + +function request(params: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method: 'terminal.close', params } +} + +describe('terminal close runtime generation', () => { + it('rejects a close fenced to a replaced runtime before touching the handle', async () => { + const closeTerminal = vi.fn() + const dispatcher = new RpcDispatcher({ + runtime: { + getRuntimeId: () => 'runtime-b', + closeTerminal + } as unknown as OrcaRuntimeService, + methods: TERMINAL_METHODS + }) + + const response = await dispatcher.dispatch( + request({ terminal: 'terminal-1', expectedRuntimeId: 'runtime-a' }) + ) + + expect(response).toMatchObject({ + ok: false, + error: { message: expect.stringContaining('runtime_generation_mismatch') }, + _meta: { runtimeId: 'runtime-b' } + }) + expect(closeTerminal).not.toHaveBeenCalled() + }) + + it('closes a handle when the expected runtime generation still owns it', async () => { + const closeTerminal = vi.fn().mockResolvedValue({ closed: true }) + const dispatcher = new RpcDispatcher({ + runtime: { + getRuntimeId: () => 'runtime-a', + closeTerminal + } as unknown as OrcaRuntimeService, + methods: TERMINAL_METHODS + }) + + await expect( + dispatcher.dispatch(request({ terminal: 'terminal-1', expectedRuntimeId: 'runtime-a' })) + ).resolves.toMatchObject({ ok: true }) + expect(closeTerminal).toHaveBeenCalledWith('terminal-1') + }) +}) diff --git a/src/main/ssh/ssh-relay-session.test.ts b/src/main/ssh/ssh-relay-session.test.ts index 8f0984ca76e..54ed124dd72 100644 --- a/src/main/ssh/ssh-relay-session.test.ts +++ b/src/main/ssh/ssh-relay-session.test.ts @@ -553,7 +553,11 @@ describe('SshRelaySession', () => { expect(mockAttach).toHaveBeenCalledWith('pty-1') expect(setPtyOwnership).toHaveBeenCalledWith('ssh:target-1@@pty-1', 'target-1') - expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith('target-1', 'pty-1', 'attached') + expect(mockStore.markSshRemotePtyLease).toHaveBeenCalledWith( + 'target-1', + 'ssh:target-1@@pty-1', + 'attached' + ) }) it('forwards a lease tab identity to reattach so a reset relay cannot cross-wire it', async () => { diff --git a/src/main/ssh/ssh-relay-session.ts b/src/main/ssh/ssh-relay-session.ts index eb62fc71830..aadbbe45bb1 100644 --- a/src/main/ssh/ssh-relay-session.ts +++ b/src/main/ssh/ssh-relay-session.ts @@ -1159,11 +1159,10 @@ export class SshRelaySession { } }) ptyProvider.onExit((payload) => { - const relayPtyId = toRelaySshPtyId(this.targetId, payload.id) clearProviderPtyState(payload.id) deletePtyOwnership(payload.id) this.forwardedReattachReplayByPty.delete(payload.id) - this.store.markSshRemotePtyLease(this.targetId, relayPtyId, 'terminated') + this.store.markSshRemotePtyLease(this.targetId, payload.id, 'terminated') this.runtime?.onPtyExit(payload.id, payload.code) const win = this.getMainWindow() if (win && !win.isDestroyed()) { @@ -1200,11 +1199,11 @@ export class SshRelaySession { } } - private expireReattachPty(relayPtyId: string, appPtyId: string): void { + private expireReattachPty(_relayPtyId: string, appPtyId: string): void { clearProviderPtyState(appPtyId) deletePtyOwnership(appPtyId) this.forwardedReattachReplayByPty.delete(appPtyId) - this.store.markSshRemotePtyLease(this.targetId, relayPtyId, 'expired') + this.store.markSshRemotePtyLease(this.targetId, appPtyId, 'expired') const win = this.getMainWindow() if (win && !win.isDestroyed()) { win.webContents.send('pty:exit', { id: appPtyId, code: -1 }) @@ -1276,7 +1275,7 @@ export class SshRelaySession { } const appPtyId = toAppSshPtyId(this.targetId, ptyId, relayInstanceId) setPtyOwnership(appPtyId, this.targetId) - this.store.markSshRemotePtyLease(this.targetId, ptyId, 'attached') + this.store.markSshRemotePtyLease(this.targetId, appPtyId, 'attached') this.forwardReattachReplay(appPtyId, attachResult.replay ?? '') } catch (err) { if (!isSshPtyNotFoundError(err)) { @@ -1299,7 +1298,7 @@ export class SshRelaySession { clearProviderPtyState(appPtyId) deletePtyOwnership(appPtyId) this.forwardedReattachReplayByPty.delete(appPtyId) - this.store.markSshRemotePtyLease(this.targetId, ptyId, 'expired') + this.store.markSshRemotePtyLease(this.targetId, appPtyId, 'expired') // Why: if the new relay cannot reattach this id, the remote backing // process is gone. Tell the renderer so it clears stale pane bindings // instead of keeping a cursor-only terminal. diff --git a/src/renderer/src/components/terminal-pane/pty-transport.test.ts b/src/renderer/src/components/terminal-pane/pty-transport.test.ts index 1f4e81f75f1..6c4585fa931 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport.test.ts @@ -1468,14 +1468,11 @@ describe('createIpcPtyTransport', () => { expect(onPtySpawn).not.toHaveBeenCalled() expect(transport.getPtyId()).toBeNull() - // A later transport lifecycle event retries the retained exact id. + // Main owns the retry; later renderer lifecycle must not duplicate it. spawnMock.mockResolvedValueOnce({ id: 'pty-next' }) await createIpcPtyTransport().connect({ url: '', callbacks: {} }) await flushPtySideEffects() - expect(killMock).toHaveBeenNthCalledWith(2, 'pty-late', { - expectedPaneKey: 'tab-late:11111111-1111-4111-8111-111111111111', - expectedTabId: 'tab-late' - }) + expect(killMock).toHaveBeenCalledTimes(1) } finally { warn.mockRestore() } @@ -1497,7 +1494,7 @@ describe('createIpcPtyTransport', () => { expect(warn).toHaveBeenCalledWith('[pty] Failed to stop disconnected PTY', killError) await createIpcPtyTransport().connect({ url: '', callbacks: {} }) await flushPtySideEffects() - expect(kill).toHaveBeenNthCalledWith(2, 'pty-1') + expect(kill).toHaveBeenCalledTimes(1) } finally { warn.mockRestore() } diff --git a/src/renderer/src/lib/pty-kill-retry-ownership.test.ts b/src/renderer/src/lib/pty-kill-retry-ownership.test.ts index 7a521b25482..78145dfdff9 100644 --- a/src/renderer/src/lib/pty-kill-retry-ownership.test.ts +++ b/src/renderer/src/lib/pty-kill-retry-ownership.test.ts @@ -6,7 +6,7 @@ import { } from './pty-kill-retry-ownership' import { toAppSshPtyId } from '../../../shared/ssh-pty-id' -const IDS = Array.from({ length: 65 }, (_, index) => `pty-retained-${index}`) +const IDS = Array.from({ length: 2 }, (_, index) => `pty-retained-${index}`) describe('PTY kill retry ownership', () => { afterEach(() => { @@ -18,40 +18,19 @@ describe('PTY kill retry ownership', () => { vi.useRealTimers() }) - it('retains every live id and retries all due owners after bounded backoff', async () => { - vi.useFakeTimers() - const kill = vi.fn().mockRejectedValue(new Error('provider disconnected')) + it('delegates exact identity to the main-owned retry boundary', async () => { + const kill = vi.fn().mockResolvedValue(undefined) vi.stubGlobal('window', { api: { pty: { kill } } }) vi.spyOn(console, 'warn').mockImplementation(() => {}) - for (const id of IDS) { - await killPtyRetainingRetryOwnership(id, '[pty] failed').catch(() => {}) - } - kill.mockResolvedValue(undefined) - - await vi.advanceTimersByTimeAsync(250) - - expect(kill).toHaveBeenCalledTimes(IDS.length * 2) - for (const id of IDS) { - expect(kill.mock.calls.filter(([calledId]) => calledId === id)).toHaveLength(2) - } - }) - - it('retries after a second failure without requiring another lifecycle event', async () => { - vi.useFakeTimers() - const kill = vi - .fn() - .mockRejectedValueOnce(new Error('offline')) - .mockRejectedValueOnce(new Error('still offline')) - .mockResolvedValue(undefined) - vi.stubGlobal('window', { api: { pty: { kill } } }) - vi.spyOn(console, 'warn').mockImplementation(() => {}) - - await killPtyRetainingRetryOwnership(IDS[0], '[pty] failed').catch(() => {}) - await vi.advanceTimersByTimeAsync(250) - expect(kill).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(500) - expect(kill).toHaveBeenCalledTimes(3) + await killPtyRetainingRetryOwnership(IDS[0], '[pty] failed', { + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab' + }) + expect(kill).toHaveBeenCalledWith(IDS[0], { + expectedPaneKey: 'tab:leaf', + expectedTabId: 'tab' + }) }) it('releases only a removed SSH target and cancels the final retry timer', async () => { @@ -68,7 +47,7 @@ describe('PTY kill retry ownership', () => { await vi.advanceTimersByTimeAsync(250) expect(kill.mock.calls.filter(([id]) => id === removedId)).toHaveLength(1) - expect(kill.mock.calls.filter(([id]) => id === retainedId)).toHaveLength(2) + expect(kill.mock.calls.filter(([id]) => id === retainedId)).toHaveLength(1) releaseRetainedPtyKillsForSshTarget('retained-target') expect(vi.getTimerCount()).toBe(0) }) diff --git a/src/renderer/src/lib/pty-kill-retry-ownership.ts b/src/renderer/src/lib/pty-kill-retry-ownership.ts index 887df03d78a..82a90081fbc 100644 --- a/src/renderer/src/lib/pty-kill-retry-ownership.ts +++ b/src/renderer/src/lib/pty-kill-retry-ownership.ts @@ -1,122 +1,31 @@ -type RetainedPtyKill = { - diagnostic: string - inFlight: Promise | null - options?: PtyKillIdentity - attempts: number - nextRetryAt: number -} - export type PtyKillIdentity = { expectedPaneKey?: string expectedTabId?: string } -const MAX_RETRY_BACKOFF_MS = 30_000 -const retainedPtyKills = new Map() -let retryTimer: ReturnType | null = null - -function scheduleRetainedPtyKillRetries(): void { - if (retryTimer) { - clearTimeout(retryTimer) - retryTimer = null - } - let nextRetryAt = Number.POSITIVE_INFINITY - for (const retained of retainedPtyKills.values()) { - if (!retained.inFlight) { - nextRetryAt = Math.min(nextRetryAt, retained.nextRetryAt) - } - } - if (!Number.isFinite(nextRetryAt)) { - return - } - retryTimer = setTimeout(() => { - retryTimer = null - const now = Date.now() - for (const [id, retained] of retainedPtyKills) { - if (!retained.inFlight && retained.nextRetryAt <= now) { - void killPtyRetainingRetryOwnership(id, retained.diagnostic, retained.options).catch( - () => {} - ) - } - } - }, Math.max(0, nextRetryAt - Date.now())) -} - -/** - * Keep exact PTY identity until the owning provider accepts shutdown. A rejected - * local/SSH IPC call can otherwise leave a live process with no renderer owner. - */ +/** Main persists exact provider identity and owns retries after renderer loss. */ export function killPtyRetainingRetryOwnership( id: string, diagnostic: string, options?: PtyKillIdentity ): Promise { - let retained = retainedPtyKills.get(id) - if (!retained) { - // Why: every entry represents a provider process whose shutdown is still - // unconfirmed. Dropping one to cap bookkeeping would orphan the real PTY; - // repeated failures for the same PTY still coalesce into this single record. - retained = { diagnostic, inFlight: null, options, attempts: 0, nextRetryAt: 0 } - } - retained.diagnostic = diagnostic - retained.options = options ?? retained.options - retainedPtyKills.set(id, retained) - if (retained.inFlight) { - return retained.inFlight - } - - const attempt = Promise.resolve() - .then(() => - retained.options ? window.api.pty.kill(id, retained.options) : window.api.pty.kill(id) - ) - .then(() => { - retainedPtyKills.delete(id) - }) + return Promise.resolve() + .then(() => (options ? window.api.pty.kill(id, options) : window.api.pty.kill(id))) .catch((error: unknown) => { - retained.attempts += 1 - retained.nextRetryAt = - Date.now() + - Math.min(MAX_RETRY_BACKOFF_MS, 250 * 2 ** Math.min(retained.attempts - 1, 7)) - // Why: automatic retries can outlive a disconnect; bound diagnostics so - // a long outage cannot become a log-backpressure loop. - if (retained.attempts <= 2 || retained.attempts === 8) { - console.warn(retained.diagnostic, error) - } + // Why: main owns retry state, but the initiating renderer still records the first failure. + console.warn(diagnostic, error) throw error }) - .finally(() => { - if (retained.inFlight === attempt) { - retained.inFlight = null - } - scheduleRetainedPtyKillRetries() - }) - retained.inFlight = attempt - return attempt } -/** Retry immediately on the next PTY lifecycle event in addition to bounded backoff. */ export function retryRetainedPtyKills(): void { - for (const [id, retained] of retainedPtyKills) { - if (!retained.inFlight) { - void killPtyRetainingRetryOwnership(id, retained.diagnostic, retained.options).catch(() => {}) - return - } - } + // Main retries on provider availability and bounded backoff. } export function releaseRetainedPtyKillOwnership(id: string): void { - retainedPtyKills.delete(id) - scheduleRetainedPtyKillRetries() + void id } export function releaseRetainedPtyKillsForSshTarget(targetId: string): void { - for (const id of retainedPtyKills.keys()) { - if (parseAppSshPtyId(id)?.connectionId === targetId) { - retainedPtyKills.delete(id) - } - } - // Why: successful target removal is authoritative proof that reconnect can - // no longer reach these PTYs, so retry ownership must not outlive the host. - scheduleRetainedPtyKillRetries() + void targetId } -import { parseAppSshPtyId } from '../../../shared/ssh-pty-id' diff --git a/src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts b/src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts index 5c6e76690b6..f96d76d0787 100644 --- a/src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts +++ b/src/renderer/src/lib/runtime-terminal-close-retry-ownership.test.ts @@ -20,33 +20,23 @@ describe('runtime terminal close retry ownership', () => { vi.useRealTimers() }) - it('retries a retained handle after repeated failure without another lifecycle event', async () => { - vi.useFakeTimers() - callRuntimeRpc - .mockRejectedValueOnce(new Error('offline')) - .mockRejectedValueOnce(new Error('still offline')) - .mockResolvedValue(undefined) - vi.spyOn(console, 'warn').mockImplementation(() => {}) - - await closeRuntimeTerminalRetainingRetryOwnership(TARGET, 'terminal-1').catch(() => {}) - await vi.advanceTimersByTimeAsync(250) - expect(callRuntimeRpc).toHaveBeenCalledTimes(2) - await vi.advanceTimersByTimeAsync(500) - - expect(callRuntimeRpc).toHaveBeenCalledTimes(3) + it('delegates the exact handle to the main-owned retry boundary', async () => { + callRuntimeRpc.mockResolvedValue(undefined) + await closeRuntimeTerminalRetainingRetryOwnership(TARGET, 'terminal-1') + expect(callRuntimeRpc).toHaveBeenCalledTimes(1) expect(callRuntimeRpc).toHaveBeenLastCalledWith(TARGET, 'terminal.close', { terminal: 'terminal-1' }) }) - it('releases every removed-environment handle and cancels its retry timer', async () => { + it('does not retain renderer timers when the environment is removed', async () => { vi.useFakeTimers() callRuntimeRpc.mockRejectedValue(new Error('environment unavailable')) vi.spyOn(console, 'warn').mockImplementation(() => {}) await closeRuntimeTerminalRetainingRetryOwnership(TARGET, 'terminal-1').catch(() => {}) await closeRuntimeTerminalRetainingRetryOwnership(TARGET, 'terminal-2').catch(() => {}) - expect(vi.getTimerCount()).toBe(1) + expect(vi.getTimerCount()).toBe(0) releaseRetainedRuntimeTerminalClosesForEnvironment(TARGET.environmentId) expect(vi.getTimerCount()).toBe(0) diff --git a/src/renderer/src/lib/runtime-terminal-close-retry-ownership.ts b/src/renderer/src/lib/runtime-terminal-close-retry-ownership.ts index 57f8d84c1e7..c85ca2e6462 100644 --- a/src/renderer/src/lib/runtime-terminal-close-retry-ownership.ts +++ b/src/renderer/src/lib/runtime-terminal-close-retry-ownership.ts @@ -1,117 +1,27 @@ -import { - callRuntimeRpc, - type RuntimeClientTarget -} from '@/runtime/runtime-rpc-client' +import { callRuntimeRpc, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' type EnvironmentTarget = Extract -type RetainedRuntimeTerminalClose = { - target: EnvironmentTarget - handle: string - inFlight: Promise | null - attempts: number - nextRetryAt: number -} - -const MAX_RETRY_BACKOFF_MS = 30_000 -const retainedCloses = new Map() -let retryTimer: ReturnType | null = null - -function retainedCloseKey(target: EnvironmentTarget, handle: string): string { - return `${target.environmentId}\0${handle}` -} - -function scheduleRuntimeTerminalCloseRetries(): void { - if (retryTimer) { - clearTimeout(retryTimer) - retryTimer = null - } - let nextRetryAt = Number.POSITIVE_INFINITY - for (const retained of retainedCloses.values()) { - if (!retained.inFlight) { - nextRetryAt = Math.min(nextRetryAt, retained.nextRetryAt) - } - } - if (!Number.isFinite(nextRetryAt)) { - return - } - retryTimer = setTimeout(() => { - retryTimer = null - const now = Date.now() - for (const retained of retainedCloses.values()) { - if (!retained.inFlight && retained.nextRetryAt <= now) { - void closeRuntimeTerminalRetainingRetryOwnership(retained.target, retained.handle).catch( - () => {} - ) - } - } - }, Math.max(0, nextRetryAt - Date.now())) -} - export function closeRuntimeTerminalRetainingRetryOwnership( target: EnvironmentTarget, handle: string ): Promise { - const key = retainedCloseKey(target, handle) - let retained = retainedCloses.get(key) - if (!retained) { - retained = { target, handle, inFlight: null, attempts: 0, nextRetryAt: 0 } - retainedCloses.set(key, retained) - } - if (retained.inFlight) { - return retained.inFlight - } - - const attempt = callRuntimeRpc(target, 'terminal.close', { terminal: handle }) - .then(() => { - retainedCloses.delete(key) - }) - .catch((error: unknown) => { - retained.attempts += 1 - retained.nextRetryAt = - Date.now() + - Math.min(MAX_RETRY_BACKOFF_MS, 250 * 2 ** Math.min(retained.attempts - 1, 7)) - if (retained.attempts <= 2 || retained.attempts === 8) { - console.warn('[terminal] Failed to close retained runtime terminal', error) - } - throw error - }) - .finally(() => { - if (retained.inFlight === attempt) { - retained.inFlight = null - } - scheduleRuntimeTerminalCloseRetries() - }) - retained.inFlight = attempt - return attempt + // Why: main owns durable retry intent, so renderer loss cannot orphan the handle. + return callRuntimeRpc(target, 'terminal.close', { terminal: handle }) } export function retryRetainedRuntimeTerminalCloses(): void { - for (const retained of retainedCloses.values()) { - if (!retained.inFlight) { - void closeRuntimeTerminalRetainingRetryOwnership(retained.target, retained.handle).catch( - () => {} - ) - return - } - } + // Main retries on provider availability and bounded backoff. } export function releaseRetainedRuntimeTerminalClose( target: EnvironmentTarget, handle: string ): void { - retainedCloses.delete(retainedCloseKey(target, handle)) - scheduleRuntimeTerminalCloseRetries() + void target + void handle } export function releaseRetainedRuntimeTerminalClosesForEnvironment(environmentId: string): void { - for (const [key, retained] of retainedCloses) { - if (retained.target.environmentId === environmentId) { - retainedCloses.delete(key) - } - } - // Why: a successfully removed runtime cannot be reconnected, so retained - // handles have no reachable provider resource and must release their timer. - scheduleRuntimeTerminalCloseRetries() + void environmentId } diff --git a/src/shared/constants.ts b/src/shared/constants.ts index 950cc999477..7166cb56a29 100644 --- a/src/shared/constants.ts +++ b/src/shared/constants.ts @@ -452,6 +452,8 @@ export function getDefaultPersistedState(homedir: string): PersistedState { sshTargets: [], deletedSshConfigAliases: [], sshRemotePtyLeases: [], + pendingLocalPtyShutdowns: [], + pendingRuntimeTerminalCloses: [], claudeLivePtySessionIds: [], migrationUnsupportedPtyEntries: [], legacyPaneKeyAliasEntries: [], diff --git a/src/shared/types.ts b/src/shared/types.ts index 6d684b12c3c..ffa8b1250bb 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -3565,6 +3565,20 @@ export type LegacyPaneKeyAliasEntry = { updatedAt: number } +export type PersistedLocalPtyShutdown = { + ptyId: string + expectedPaneKey?: string + expectedTabId?: string + requestedAt: number +} + +export type PersistedRuntimeTerminalClose = { + environmentId: string + handle: string + runtimeId?: string + requestedAt: number +} + // ─── Persistence shape ────────────────────────────────────────────── export type PersistedState = { schemaVersion: number @@ -3602,6 +3616,10 @@ export type PersistedState = { * workspaces that were orphaned on the old target id. Pruned by age/count. */ removedSshTargetTombstones?: RemovedSshTargetTombstone[] sshRemotePtyLeases: SshRemotePtyLease[] + /** Exact local daemon shutdown intents survive renderer and app restarts. */ + pendingLocalPtyShutdowns?: PersistedLocalPtyShutdown[] + /** Remote runtime close intents survive renderer and app restarts. */ + pendingRuntimeTerminalCloses?: PersistedRuntimeTerminalClose[] /** Daemon session ids of live local Claude launches. Seeds the Claude * live-PTY gate on startup so an early OAuth refresh cannot rotate the * single-use refresh token out from under a still-running daemon CLI. */