From 1d04fbbc55d16f4812bdcb2e6d9eab414013e14b Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Mon, 20 Jul 2026 11:01:46 -0700 Subject: [PATCH] fix(windows): close legacy and SSH teardown gaps --- .../windows-terminal-restart-e2e.yml | 33 ++++- config/patches/node-pty@1.1.0.patch | 37 +---- config/reliability-gates.jsonc | 37 ++++- pnpm-lock.yaml | 6 +- src/main/daemon/daemon-pty-adapter.test.ts | 59 +++++++- src/main/daemon/daemon-pty-adapter.ts | 41 ++++-- src/main/daemon/daemon-pty-router.test.ts | 33 +++++ src/main/daemon/daemon-pty-router.ts | 12 ++ .../degraded-daemon-pty-provider.test.ts | 33 +++++ .../daemon/degraded-daemon-pty-provider.ts | 10 ++ src/main/ipc/pty.ts | 22 ++- src/main/providers/local-pty-provider.test.ts | 22 +++ src/main/providers/local-pty-provider.ts | 38 +---- src/main/providers/types.ts | 6 + src/main/windows-pty-job-control.ts | 29 ++++ src/preload/api-types.ts | 2 + src/preload/index.ts | 3 + src/relay/pty-handler.test.ts | 77 ++++++++++ src/relay/pty-handler.ts | 28 +++- .../windows-agent-job.integration.test.ts | 131 +++++++++++++++++ .../close-local-terminal-tab-state.ts | 4 +- .../terminal/terminal-tab-actions.ts | 35 ++++- src/renderer/src/i18n/locales/en.json | 10 ++ src/renderer/src/i18n/locales/es.json | 10 ++ src/renderer/src/i18n/locales/ja.json | 10 ++ src/renderer/src/i18n/locales/ko.json | 10 ++ src/renderer/src/i18n/locales/zh.json | 10 ++ .../store/slices/terminal-shutdown-safety.ts | 132 ++++++++++++++++++ .../terminal-tab-retirement-store.test.ts | 55 +++++++- src/renderer/src/store/slices/terminals.ts | 27 ++++ src/shared/pty-shutdown-safety.ts | 4 + 31 files changed, 866 insertions(+), 100 deletions(-) create mode 100644 src/relay/windows-agent-job.integration.test.ts create mode 100644 src/renderer/src/store/slices/terminal-shutdown-safety.ts create mode 100644 src/shared/pty-shutdown-safety.ts diff --git a/.github/workflows/windows-terminal-restart-e2e.yml b/.github/workflows/windows-terminal-restart-e2e.yml index f206518876d..6e33e57dc41 100644 --- a/.github/workflows/windows-terminal-restart-e2e.yml +++ b/.github/workflows/windows-terminal-restart-e2e.yml @@ -32,8 +32,12 @@ on: - 'src/main/providers/**' - 'src/main/pty/**' - 'src/preload/**' + - 'src/relay/**' + - 'src/renderer/src/components/terminal/**' - 'src/renderer/src/components/terminal-pane/**' - 'src/renderer/src/lib/pane-manager/**' + - 'src/renderer/src/store/slices/terminal*.ts' + - 'src/shared/pty-shutdown-safety.ts' - 'src/shared/pty-session-id-format.ts' workflow_dispatch: inputs: @@ -82,8 +86,35 @@ jobs: - name: Install dependencies run: pnpm install --frozen-lockfile + - name: Run real Windows SSH relay Job integration + shell: pwsh + run: | + node config/scripts/ensure-native-runtime.mjs --runtime=node + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + pnpm exec vitest run src/relay/windows-agent-job.integration.test.ts --config config/vitest.config.ts + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + + - name: Verify generated node-pty projects retain Spectre mitigation + shell: pwsh + run: | + $projects = Get-ChildItem node_modules/node-pty/build -Recurse -Filter *.vcxproj + foreach ($target in @('conpty', 'conpty_console_list', 'winpty-agent', 'winpty')) { + $project = $projects | Where-Object BaseName -eq $target | Select-Object -First 1 + if (-not $project) { + throw "Missing generated node-pty project: $target" + } + if (-not (Select-String -Path $project.FullName -Pattern 'Spectre' -Quiet)) { + throw "Spectre mitigation missing from generated project: $($project.FullName)" + } + } + - name: Build Electron app for Windows E2E - run: pnpm exec electron-vite build --mode e2e + shell: pwsh + run: | + node config/scripts/ensure-native-runtime.mjs --runtime=electron + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + pnpm exec electron-vite build --mode e2e + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } - name: Run patched node-pty failed-connect cleanup regressions env: diff --git a/config/patches/node-pty@1.1.0.patch b/config/patches/node-pty@1.1.0.patch index 32cc844017d..702d2b569ab 100644 --- a/config/patches/node-pty@1.1.0.patch +++ b/config/patches/node-pty@1.1.0.patch @@ -1,8 +1,8 @@ diff --git a/binding.gyp b/binding.gyp -index 5f63978b07ab50aaf7523219a2170ec737a6b5db..7fecd38719772f7b31c2a91b17c7f3700446189f 100644 +index 5f63978b07ab50aaf7523219a2170ec737a6b5db..e23988358eecc292ca769a738d5bfdef8172bb3d 100644 --- a/binding.gyp +++ b/binding.gyp -@@ -1,13 +1,13 @@ +@@ -1,4 +1,7 @@ { + 'variables': { + 'node_pty_test_faults%': 0, @@ -10,16 +10,7 @@ index 5f63978b07ab50aaf7523219a2170ec737a6b5db..7fecd38719772f7b31c2a91b17c7f370 'target_defaults': { 'dependencies': [ " agent Node process -> exited launcher -> reparented native lock-holder, invoke production store.removeWorktree(id, true), and prove the exact PTY, root, agent, lock-holder, worktree directory, Git registration, and worktree list entry are absent; separately prove a plain terminal's intentionally detached child survives close. A natural-root-exit journey records the public PTY exit callback and requires the detached Job child to be absent at that exact callback. Unit tests require agent-only spawn-time Job selection, handle-scoped local Job termination before ConPTY close, daemon root signalling before Job termination and drain, fail-closed timeout/rejection/synchronous-throw behavior, adopted-v24 destructive-shutdown refusal, provider-scoped legacy/fallback route refresh, duplicate ownership refusal and exit suppression, daemon WSL exclusion, safe failure without PID-only fallback when Job ownership is unavailable, daemon-wide routing through the same teardown, naturally exited id reservation, bounded locale-stable process-table reads, duplicate completion sharing, canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.", + "invariant": "Close permanently removes the owned provider session, agent descendants, and resume authority even when no TerminalPane is mounted; a native Windows agent PTY owns its spawn-time process family in a kill-on-close Job that must drain before destructive worktree deletion succeeds, including agents hosted by a Windows SSH relay, while plain terminals, WSL, POSIX SSH, and remote runtimes retain their existing semantics; a terminating id remains reserved through natural exit, duplicate callers await the same completion, and immediate teardown upgrades any graceful request without signalling a recycled PID or a descendant tree after root ownership is lost; process-table work is locale-stable, bounded, fresh for each post-start request, same-turn coalesced, and begins within the requesting caller's deadline, including bulk worktree cleanup; detach and park preserve ownership; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.", + "oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; an agent-marked PTY's detached-pgid child is alive before close and absent afterward. On Windows, prove the fixture is Git-registered, launch PowerShell shell -> agent Node process -> exited launcher -> reparented native lock-holder, invoke production store.removeWorktree(id, true), and prove the exact PTY, root, agent, lock-holder, worktree directory, Git registration, and worktree list entry are absent; separately prove a plain terminal's intentionally detached child survives close. A natural-root-exit journey records the public PTY exit callback and requires the detached Job child to be absent at that exact callback. A real Windows relay/node-pty integration holds a delete-denying file handle in a detached PowerShell child and requires remote-host Job drain before worktree deletion. Unit tests require agent-only spawn-time Job selection, handle-scoped local and Windows-relay Job termination before ConPTY close, daemon root signalling before Job termination and drain, fail-closed timeout/rejection/synchronous-throw behavior, adopted-v24 destructive-shutdown refusal without renderer-state loss, provider-scoped legacy/fallback route refresh, duplicate ownership refusal and exit suppression, local and relay WSL exclusion, safe failure without PID-only fallback when Job ownership is unavailable, daemon-wide routing through the same teardown, naturally exited id reservation, bounded locale-stable process-table reads, duplicate completion sharing, canonical owner dedupe, exact pane tombstones, chained detach transfer, and restart alias restoration.", "commands": [ "pnpm dlx node@24 ./node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/agent-hooks/server-pane-authority.test.ts src/main/ipc/agent-hooks.test.ts src/main/ipc/agent-pane-authority-ownership.test.ts src/main/ipc/pty-management.test.ts src/main/persistence.test.ts src/renderer/src/store/slices/agent-pane-authority.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/renderer/src/store/slices/terminal-tab-retirement.test.ts src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts", "pnpm exec vitest run --config config/vitest.config.ts src/main/pty-descendant-termination.test.ts src/main/daemon/daemon-pty-adapter.test.ts src/main/daemon/daemon-pty-router.test.ts src/main/daemon/daemon-pty-session-routing.test.ts src/main/daemon/degraded-daemon-pty-provider.test.ts src/main/daemon/session.test.ts src/main/daemon/terminal-host.test.ts src/main/daemon/terminal-session-teardown.test.ts src/main/daemon/pty-subprocess.test.ts src/main/providers/local-pty-provider.test.ts src/main/runtime/worktree-teardown.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/relay/pty-handler.test.ts", + "pnpm exec vitest run --config config/vitest.config.ts src/relay/windows-agent-job.integration.test.ts", "pnpm run test:e2e -- tests/e2e/terminal-parked-close-retirement.spec.ts --workers=1", "pnpm run test:e2e -- tests/e2e/agent-descendant-process-kill.spec.ts --workers=1", "pnpm run test:e2e -- tests/e2e/windows-agent-job-natural-exit.spec.ts --workers=1", @@ -745,6 +747,8 @@ "src/main/daemon/pty-subprocess.test.ts", "src/main/providers/local-pty-provider.test.ts", "src/main/runtime/worktree-teardown.test.ts", + "src/relay/pty-handler.test.ts", + "src/relay/windows-agent-job.integration.test.ts", "src/renderer/src/store/slices/agent-pane-authority.test.ts", "src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts", "src/renderer/src/store/slices/terminal-tab-retirement.test.ts", @@ -850,6 +854,25 @@ "inventories above 32 sessions never exceed 32 concurrent provider shutdowns" ] }, + { + "file": "src/relay/pty-handler.test.ts", + "assertions": [ + "native Windows SSH relay agents request remote-host Job ownership, drain before acknowledgement, and retain PTY ownership when drain fails", + "Windows SSH relay WSL agents do not request native Job ownership" + ] + }, + { + "file": "src/relay/windows-agent-job.integration.test.ts", + "assertions": [ + "real Windows relay/node-pty shutdown kills a detached PowerShell lock-holder and releases its delete-denying handle before acknowledgement" + ] + }, + { + "file": "src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts", + "assertions": [ + "unsafe adopted-v24 close and sleep keep the renderer tab and exact PTY mapping intact without issuing root-only kill" + ] + }, { "file": "tests/e2e/agent-descendant-process-kill.spec.ts", "assertions": [ @@ -948,16 +971,16 @@ }, "promotionCriteria": [ "Accumulate 100 clean runs or 14 days on required CI platforms.", - "Add live Linux, WSL, SSH, and ordinary runtime process-absence evidence.", + "Add live Linux, WSL, end-to-end SSH transport, and ordinary runtime process-absence evidence.", "Add restart/no-resurrection and repeated park-close soak coverage." ], "knownGaps": [ - "Live process proof covers macOS local PTYs and native Windows agent/plain PTYs, but not Linux, WSL, SSH, or remote-runtime PTYs.", + "Live process proof covers macOS local PTYs, native Windows agent/plain PTYs, and the Windows relay with a real node-pty child, but not Linux, WSL, end-to-end SSH transport, or remote-runtime PTYs.", "Disconnected SSH relay death still requires reconnect-aware provider ownership.", "Daemon owner leases and durable retry inventory remain follow-up hardening.", - "Existing adopted Windows v24 sessions (agent, plain, or WSL) lack spawn-time Job capability metadata, so deadline-bearing worktree deletion fails closed. Closing or restarting the tracked session under v25 cannot retroactively adopt descendants that detached before v25; those older orphans may still require manual termination or an OS restart. Fresh v25 plain and WSL behavior is unchanged.", + "Existing adopted Windows v24 native or unclassified sessions lack spawn-time Job capability metadata, so ordinary close, sleep, and deadline-bearing worktree deletion fail closed while the renderer keeps the tab and PTY ownership visible. Exiting the agent or shell may release them; otherwise users may need Task Manager or a Windows restart. A v24 reattach is treated as WSL only when the daemon reports WSL metadata; attached known-WSL sessions remain compatible. Fresh v25 plain and WSL behavior is unchanged.", "An agent started manually after an ordinary native Windows terminal has spawned remains a plain terminal: it is not retroactively placed in Orca's agent Job, so detached descendants are preserved and may still block deletion. The Job-backed guarantee applies to Orca-launched or spawn-recognized native Windows agent sessions.", - "SSH-hosted, WSL, and remote-runtime PTYs retain their host-specific teardown paths and are not placed in the local Windows Job.", + "Fresh native Windows SSH relay agents use a Job on the remote Windows host; POSIX SSH, WSL, and remote-runtime PTYs retain their host-specific teardown paths and are not placed in the desktop's local Windows Job.", "A process born in the capture second is SIGTERMed but not SIGKILLed because ps cannot prove its recycled-PID identity.", "A descendant orphaned before or during root ownership loss requires the separate crash-orphan sweep and is not recovered from a stale kill-time snapshot." ], diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5751bac5452..45765585f80 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -21,7 +21,7 @@ patchedDependencies: hash: 9c1de9931d86864923ff53bc9d64474a86478085ac81ea4e432648c7e23d702c path: config/patches/@xterm__xterm@6.1.0-beta.287.patch node-pty@1.1.0: - hash: dd945bcc21ad45f8a5d2e7f48cf4fd1fb3e4c9ba6862b92f8aa07bbd3dfabe2e + hash: d4382e83147336cfe41bd6174e27ae922723cab8da1da6939d16c74b71a46820 path: config/patches/node-pty@1.1.0.patch importers: @@ -63,7 +63,7 @@ importers: version: 3.3.1 node-pty: specifier: ^1.1.0 - version: 1.1.0(patch_hash=dd945bcc21ad45f8a5d2e7f48cf4fd1fb3e4c9ba6862b92f8aa07bbd3dfabe2e) + version: 1.1.0(patch_hash=d4382e83147336cfe41bd6174e27ae922723cab8da1da6939d16c74b71a46820) posthog-node: specifier: ^5.33.3 version: 5.33.3 @@ -12043,7 +12043,7 @@ snapshots: undici: 6.27.0 which: 6.0.1 - node-pty@1.1.0(patch_hash=dd945bcc21ad45f8a5d2e7f48cf4fd1fb3e4c9ba6862b92f8aa07bbd3dfabe2e): + node-pty@1.1.0(patch_hash=d4382e83147336cfe41bd6174e27ae922723cab8da1da6939d16c74b71a46820): dependencies: node-addon-api: 7.1.1 diff --git a/src/main/daemon/daemon-pty-adapter.test.ts b/src/main/daemon/daemon-pty-adapter.test.ts index ba2a37ba3f2..d7ed7e90dc2 100644 --- a/src/main/daemon/daemon-pty-adapter.test.ts +++ b/src/main/daemon/daemon-pty-adapter.test.ts @@ -567,10 +567,10 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { }) ).rejects.toThrow('requires daemon protocol 25 or newer') await expect(legacy.shutdown(id, { immediate: true })).rejects.toThrow( - 'restart Orca before closing adopted terminals' + 'end its process tree in Task Manager or restart Windows' ) await expect(legacy.shutdown(id, { immediate: true, keepHistory: true })).rejects.toThrow( - 'restart Orca before closing adopted terminals' + 'end its process tree in Task Manager or restart Windows' ) } expect(ensureConnectedSpy).not.toHaveBeenCalled() @@ -619,6 +619,61 @@ describe('DaemonPtyAdapter (IPtyProvider)', () => { } }) + it('does not trust caller WSL preference on metadata-less v22/v23 reattach', async () => { + const ensureConnectedSpy = vi + .spyOn(DaemonClient.prototype, 'ensureConnected') + .mockResolvedValue() + const requestSpy = vi + .spyOn(DaemonClient.prototype, 'request') + .mockImplementation(async (method) => + method === 'createOrAttach' + ? { + isNew: false, + pid: 123, + shellState: 'unsupported', + snapshot: null + } + : undefined + ) + const platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + const adapters: DaemonPtyAdapter[] = [] + + try { + for (const protocolVersion of [22, 23]) { + const legacy = new DaemonPtyAdapter({ socketPath, tokenPath, protocolVersion }) + adapters.push(legacy) + const id = `metadata-less-v${protocolVersion}` + await legacy.spawn({ + sessionId: id, + cols: 80, + rows: 24, + shellOverride: 'wsl.exe', + terminalWindowsWslDistro: 'Ubuntu' + }) + + await expect(legacy.shutdown(id, { immediate: true })).rejects.toThrow( + 'requires daemon protocol 25 or newer' + ) + await expect(legacy.shutdown(id, { immediate: true, keepHistory: true })).rejects.toThrow( + 'requires daemon protocol 25 or newer' + ) + await expect( + legacy.shutdown(id, { immediate: true, deadlineMs: Date.now() + 5_000 }) + ).rejects.toThrow('requires daemon protocol 25 or newer') + } + + expect(requestSpy.mock.calls.some(([method]) => method === 'kill')).toBe(false) + } finally { + adapters.forEach((adapter) => adapter.dispose()) + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + requestSpy.mockRestore() + ensureConnectedSpy.mockRestore() + } + }) + it('kills the session', async () => { const { id } = await adapter.spawn({ cols: 80, rows: 24 }) await adapter.shutdown(id, { immediate: false }) diff --git a/src/main/daemon/daemon-pty-adapter.ts b/src/main/daemon/daemon-pty-adapter.ts index a30e3787493..9433b314772 100644 --- a/src/main/daemon/daemon-pty-adapter.ts +++ b/src/main/daemon/daemon-pty-adapter.ts @@ -32,6 +32,10 @@ import type { PtySpawnOptions, PtySpawnResult } from '../providers/types' +import { + WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON, + type PtyShutdownBlockReason +} from '../../shared/pty-shutdown-safety' import { isShellProcess } from '../../shared/agent-detection' import { resolveWslSessionContext } from './wsl-session-context' import { normalizeWslColdRestoreCwd } from './wsl-cold-restore-cwd' @@ -217,12 +221,13 @@ export class DaemonPtyAdapter implements IPtyProvider { private async doSpawn(opts: PtySpawnOptions): Promise { const sessionId = opts.sessionId ?? mintPtySessionId(opts.worktreeId) - let wslDistro = resolveWslSessionContext({ + const requestedWslDistro = resolveWslSessionContext({ cwd: opts.cwd, sessionId, shellOverride: opts.shellOverride, terminalWindowsWslDistro: opts.terminalWindowsWslDistro })?.distro + let wslDistro = requestedWslDistro const detectColdRestore = (options?: { ignoreCleanEnd?: boolean }): ColdRestoreInfo | null => { const restoreInfo = this.historyReader?.detectColdRestore(sessionId, { ...options, wslDistro }) ?? null @@ -321,9 +326,14 @@ export class DaemonPtyAdapter implements IPtyProvider { let scrollback = restoreInfo ? getRecoveredHistorySeed(restoreInfo) : null let result = await createOrAttach(scrollback) - let providerWslDistro = result.wslDistro === undefined ? wslDistro : result.wslDistro - // Why: explicit null from a current daemon overrides the caller's WSL - // preference; undefined preserves compatibility with older daemons. + let providerWslDistro = + result.wslDistro === undefined + ? result.isNew + ? requestedWslDistro + : undefined + : result.wslDistro + // Why: caller WSL preference proves a fresh spawn, not an old daemon's + // reattached host. Reattach requires daemon metadata before root-only close. wslDistro = providerWslDistro ?? undefined if (wslDistro) { this.wslDistrosBySessionId.set(sessionId, wslDistro) @@ -384,7 +394,12 @@ export class DaemonPtyAdapter implements IPtyProvider { effectiveCols = restoreInfo.cols effectiveRows = restoreInfo.rows result = await createOrAttach(scrollback) - providerWslDistro = result.wslDistro === undefined ? wslDistro : result.wslDistro + providerWslDistro = + result.wslDistro === undefined + ? result.isNew + ? requestedWslDistro + : undefined + : result.wslDistro wslDistro = providerWslDistro ?? undefined if (wslDistro) { this.wslDistrosBySessionId.set(sessionId, wslDistro) @@ -586,15 +601,11 @@ export class DaemonPtyAdapter implements IPtyProvider { id: string, opts: { immediate?: boolean; keepHistory?: boolean; deadlineMs?: number } ): Promise { - if ( - process.platform === 'win32' && - this.protocolVersion < WINDOWS_AGENT_JOB_PROTOCOL_VERSION && - !this.wslDistrosBySessionId.has(id) - ) { + if (this.getShutdownBlockReason(id)) { // Why: v24 could miss native agent identity and never Job-owned descendants; // only an attached, known WSL session is safe for ordinary root close. throw new DaemonProtocolError( - `Windows PTY shutdown requires daemon protocol ${WINDOWS_AGENT_JOB_PROTOCOL_VERSION} or newer; restart Orca before closing adopted terminals` + `Windows PTY shutdown requires daemon protocol ${WINDOWS_AGENT_JOB_PROTOCOL_VERSION} or newer. Exit the agent or shell in this terminal; if it stays running, end its process tree in Task Manager or restart Windows.` ) } // Why: shutdown can be the first lazy-client operation after restart; connect @@ -676,6 +687,14 @@ export class DaemonPtyAdapter implements IPtyProvider { } } + getShutdownBlockReason(id: string): PtyShutdownBlockReason | null { + return process.platform === 'win32' && + this.protocolVersion < WINDOWS_AGENT_JOB_PROTOCOL_VERSION && + !this.wslDistrosBySessionId.has(id) + ? WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON + : null + } + ackColdRestore(sessionId: string): void { this.coldRestoreCache.delete(sessionId) this.sleepRestoreSessionIds.delete(sessionId) diff --git a/src/main/daemon/daemon-pty-router.test.ts b/src/main/daemon/daemon-pty-router.test.ts index 57a6c5b66bb..46d8146b637 100644 --- a/src/main/daemon/daemon-pty-router.test.ts +++ b/src/main/daemon/daemon-pty-router.test.ts @@ -8,6 +8,7 @@ import type { PtySpawnResult } from '../providers/types' import { GIT_CREDENTIAL_GUARD_HOST_PROTOCOL_VERSION } from './types' +import { WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON } from '../../shared/pty-shutdown-safety' type AdapterMock = DaemonPtyAdapter & { emitData: (id: string, data: string, sequenceChars?: number) => void @@ -60,6 +61,7 @@ function createAdapter( resize: vi.fn(), setPtyBackgrounded: vi.fn(), getBufferSnapshot: vi.fn(async () => null), + getShutdownBlockReason: vi.fn(() => null), shutdown: vi.fn(async (id: string) => { const idx = sessions.indexOf(id) if (idx !== -1) { @@ -143,6 +145,37 @@ describe('DaemonPtyRouter', () => { expect(current.shutdown).toHaveBeenCalledWith('current-session', { immediate: true }) }) + it('preflights against the authoritative legacy owner', async () => { + const current = createAdapter('current') + const legacy = createAdapter('legacy', ['legacy-session']) + vi.mocked(legacy.getShutdownBlockReason).mockReturnValue( + WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON + ) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + await expect(router.getShutdownBlockReason('legacy-session')).resolves.toBe( + WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON + ) + + expect(current.listProcesses).toHaveBeenCalledOnce() + expect(legacy.listProcesses).toHaveBeenCalledOnce() + expect(legacy.getShutdownBlockReason).toHaveBeenCalledWith('legacy-session') + expect(current.getShutdownBlockReason).not.toHaveBeenCalled() + }) + + it('fails shutdown preflight closed for ambiguous ownership', async () => { + const current = createAdapter('current', ['duplicate-session']) + const legacy = createAdapter('legacy', ['duplicate-session']) + const router = new DaemonPtyRouter({ current, legacy: [legacy] }) + + await expect(router.getShutdownBlockReason('duplicate-session')).rejects.toThrow( + 'Ambiguous PTY session ownership across daemons: duplicate-session' + ) + + expect(current.getShutdownBlockReason).not.toHaveBeenCalled() + expect(legacy.getShutdownBlockReason).not.toHaveBeenCalled() + }) + it('reports snapshot capability for the adapter that owns each session', async () => { const current = createAdapter('current', ['current-session'], undefined, 22) const legacy = createAdapter('legacy', ['legacy-session'], undefined, 19) diff --git a/src/main/daemon/daemon-pty-router.ts b/src/main/daemon/daemon-pty-router.ts index fa8685266f0..1a6139838ef 100644 --- a/src/main/daemon/daemon-pty-router.ts +++ b/src/main/daemon/daemon-pty-router.ts @@ -8,6 +8,7 @@ import type { PtySpawnOptions, PtySpawnResult } from '../providers/types' +import type { PtyShutdownBlockReason } from '../../shared/pty-shutdown-safety' export class DaemonPtyRouter implements IPtyProvider { private current: DaemonPtyAdapter @@ -138,6 +139,17 @@ export class DaemonPtyRouter implements IPtyProvider { } } + async getShutdownBlockReason(id: string): Promise { + if (this.legacy.length > 0) { + // Why: preflight must select the same authoritative legacy owner as shutdown. + await this.refreshInventories() + } + if (this.sessionRouting.isAmbiguous(id)) { + throw this.ambiguousOwnershipError([id]) + } + return this.adapterFor(id).getShutdownBlockReason(id) + } + async sendSignal(id: string, signal: string): Promise { await this.adapterFor(id).sendSignal(id, signal) } diff --git a/src/main/daemon/degraded-daemon-pty-provider.test.ts b/src/main/daemon/degraded-daemon-pty-provider.test.ts index 42377abaccf..2025ac90aa1 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.test.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.test.ts @@ -7,6 +7,7 @@ import type { PtySpawnOptions, PtySpawnResult } from '../providers/types' +import { WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON } from '../../shared/pty-shutdown-safety' type ProviderMock = IPtyProvider & { emitData: (id: string, data: string, sequenceChars?: number) => void @@ -29,6 +30,7 @@ function createProvider(label: string, sessions: string[] = []): ProviderMock { hasPty: vi.fn((id: string) => sessions.includes(id)), write: vi.fn(), resize: vi.fn(), + getShutdownBlockReason: vi.fn(() => null), shutdown: vi.fn(async (id: string) => { const idx = sessions.indexOf(id) if (idx !== -1) { @@ -114,6 +116,37 @@ function createDaemonAdapter( } describe('DegradedDaemonPtyProvider', () => { + it('preflights against the authoritative daemon owner', async () => { + const current = createDaemonAdapter('daemon', ['daemon-session']) + const fallback = createProvider('fallback') + vi.mocked(current.getShutdownBlockReason!).mockReturnValue( + WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON + ) + const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) + + await expect(provider.getShutdownBlockReason('daemon-session')).resolves.toBe( + WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON + ) + + expect(current.listProcesses).toHaveBeenCalledOnce() + expect(fallback.listProcesses).toHaveBeenCalledOnce() + expect(current.getShutdownBlockReason).toHaveBeenCalledWith('daemon-session') + expect(fallback.getShutdownBlockReason).not.toHaveBeenCalled() + }) + + it('fails shutdown preflight closed for ambiguous degraded ownership', async () => { + const current = createDaemonAdapter('daemon', ['duplicate-session']) + const fallback = createProvider('fallback', ['duplicate-session']) + const provider = new DegradedDaemonPtyProvider({ current, legacy: [], fallback }) + + await expect(provider.getShutdownBlockReason('duplicate-session')).rejects.toThrow( + 'Ambiguous PTY session ownership across degraded providers: duplicate-session' + ) + + expect(current.getShutdownBlockReason).not.toHaveBeenCalled() + expect(fallback.getShutdownBlockReason).not.toHaveBeenCalled() + }) + it('routes fresh foreground confirmation to the session owner', async () => { const current = createDaemonAdapter('daemon', ['daemon-session']) const fallback = createProvider('fallback') diff --git a/src/main/daemon/degraded-daemon-pty-provider.ts b/src/main/daemon/degraded-daemon-pty-provider.ts index 9e00ec78d4f..f369a66c4e2 100644 --- a/src/main/daemon/degraded-daemon-pty-provider.ts +++ b/src/main/daemon/degraded-daemon-pty-provider.ts @@ -13,6 +13,7 @@ import type { PtySpawnOptions, PtySpawnResult } from '../providers/types' +import type { PtyShutdownBlockReason } from '../../shared/pty-shutdown-safety' export class DegradedDaemonPtyProvider implements IPtyProvider { readonly routesFreshSpawnsToLocalProvider = true @@ -126,6 +127,15 @@ export class DegradedDaemonPtyProvider implements IPtyProvider { await this.sessionRouting.shutdown(id, target, opts) } + async getShutdownBlockReason(id: string): Promise { + // Why: degraded routing can hide a preserved legacy owner behind the fallback. + await this.sessionRouting.refreshInventories() + if (this.sessionRouting.isAmbiguous(id)) { + throw this.ambiguousOwnershipError([id]) + } + return (await this.sessionRouting.get(id)?.getShutdownBlockReason?.(id)) ?? null + } + async sendSignal(id: string, signal: string): Promise { await this.providerFor(id).sendSignal(id, signal) } diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index cd4242143a1..5320b056be4 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -499,9 +499,9 @@ async function isProviderPtyLive( ): Promise { // Why: bound the liveness list RPC by the teardown deadline so a wedged daemon // fails fast; undefined keeps the provider default for all other callers. - return ( - await provider.listProcesses(deadlineMs !== undefined ? { deadlineMs } : undefined) - ).some((session) => session.id === ptyId) + return (await provider.listProcesses(deadlineMs !== undefined ? { deadlineMs } : undefined)).some( + (session) => session.id === ptyId + ) } async function verifyPtyStopped( @@ -1545,6 +1545,7 @@ export function registerPtyHandlers( // Remove any previously registered handlers so we can re-register them // (e.g. when macOS re-activates the app and creates a new window). ipcMain.removeHandler('pty:spawn') + ipcMain.removeHandler('pty:getShutdownBlockReason') ipcMain.removeHandler('pty:kill') ipcMain.removeHandler('pty:listSessions') ipcMain.removeHandler('pty:hasPty') @@ -5196,6 +5197,21 @@ export function registerPtyHandlers( runtime?.clearHeadlessTerminalBuffer(args.id).catch(() => {}) }) + ipcMain.handle('pty:getShutdownBlockReason', async (_event, args: { id: string }) => { + if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) { + throw new Error('Invalid PTY provider id') + } + const ownedConnectionId = ptyOwnership.get(args.id) + const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null + const connectionId = ownedConnectionId ?? parsedSshId?.connectionId + const startupPromise = getLocalPtyProviderStartupPromise(connectionId) + if (startupPromise) { + await startupPromise + } + const provider = connectionId ? sshProviders.get(connectionId) : tryGetProviderForPty(args.id) + return (await provider?.getShutdownBlockReason?.(args.id)) ?? null + }) + ipcMain.handle('pty:kill', async (_event, args: { id: string; keepHistory?: boolean }) => { if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) { // Why: runtime terminal handles belong to terminal.close; allowing them diff --git a/src/main/providers/local-pty-provider.test.ts b/src/main/providers/local-pty-provider.test.ts index e54197ee28e..80d94fc861b 100644 --- a/src/main/providers/local-pty-provider.test.ts +++ b/src/main/providers/local-pty-provider.test.ts @@ -1170,6 +1170,28 @@ describe('LocalPtyProvider', () => { expect(order).toEqual(['job', 'conpty']) }) + it('Job-owns a native agent despite a stale saved WSL distro', async () => { + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + mockProc.terminateJobTree.mockResolvedValue(true) + mockProc.kill.mockImplementation(() => exitCb?.({ exitCode: 1 })) + + const { id } = await provider.spawn({ + cols: 80, + rows: 24, + shellOverride: 'powershell.exe', + terminalWindowsWslDistro: 'Debian', + launchAgent: 'claude' + }) + + expect(spawnMock).toHaveBeenCalledWith( + expect.stringMatching(/powershell/i), + expect.any(Array), + expect.objectContaining({ windowsAgentJob: true }) + ) + await expect(provider.shutdown(id, { immediate: true })).resolves.toBeUndefined() + expect(mockProc.terminateJobTree).toHaveBeenCalledOnce() + }) + it('rejects fail-closed after closing ConPTY when the agent Job does not drain', async () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) mockProc.terminateJobTree.mockResolvedValue(false) diff --git a/src/main/providers/local-pty-provider.ts b/src/main/providers/local-pty-provider.ts index dab84d774b1..ebdb864b04c 100644 --- a/src/main/providers/local-pty-provider.ts +++ b/src/main/providers/local-pty-provider.ts @@ -60,7 +60,7 @@ import { captureDescendantSnapshot, terminateDescendantSnapshot } from '../pty-descendant-termination' -import { WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS } from '../windows-pty-job-control' +import { terminateWindowsPtyTree } from '../windows-pty-job-control' import { readWindowsConptyProcessIds } from './windows-conpty-process-membership' import { canConfirmAgentFromConsolePresence } from './windows-console-foreground' import { forceKillPosixPtyProcessGroups } from '../pty/posix-pty-process-groups' @@ -283,34 +283,6 @@ function killLocalPtyProcess(proc: pty.IPty, immediate: boolean): void { forceKillPosixPtyProcessGroups(proc.pid, () => proc.kill('SIGKILL')) } -type WindowsPtyJobControl = pty.IPty & { - terminateJobTree?(timeoutMs: number): Promise | undefined -} - -async function terminateWindowsPtyTree(proc: pty.IPty, closeRoot: () => void): Promise { - let nativeCompletion: Promise | undefined - try { - nativeCompletion = (proc as WindowsPtyJobControl).terminateJobTree?.call( - proc, - WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS - ) - } catch (error) { - // Why: a synchronous native bridge failure must still close ConPTY while - // the rejected shutdown keeps destructive deletion fail-closed. - closeRoot() - throw error - } - if (!nativeCompletion) { - closeRoot() - throw new Error(`Windows PTY Job ownership unavailable for process ${proc.pid}`) - } - - closeRoot() - if (!(await nativeCompletion)) { - throw new Error(`Windows PTY Job did not drain for process ${proc.pid}`) - } -} - function armLocalPtyForceKill( id: string, proc: pty.IPty, @@ -878,9 +850,9 @@ export class LocalPtyProvider implements IPtyProvider { const historyEnabled = worktreeId && (this.opts.isHistoryEnabled?.() ?? true) // Resolve the effective shell kind for history injection. For WSL, the // outer executable is wsl.exe but the inner login shell is bash. - const isWslTerminal = - Boolean(wslInfo || worktreeWslContext || preferredWslContext) || - pathWin32.basename(shellPath).toLowerCase() === 'wsl.exe' + // Why: a saved WSL distro is only a preference; a native shell override must + // still get native history and agent Job ownership. + const isWslTerminal = isWslShell const effectiveShellPath = isWslTerminal ? 'bash' : shellPath let historyResult: ReturnType | null = null if (historyEnabled) { @@ -912,7 +884,7 @@ export class LocalPtyProvider implements IPtyProvider { // belongs to a separate execution host and plain terminals stay unchanged. windowsAgentJob: process.platform === 'win32' && - !isWslTerminal && + !isWslShell && Boolean(args.launchAgent || startupAgentRecognition) }) shellPath = spawnResult.shellPath diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 6523736fd2b..7cf2652a526 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -19,6 +19,7 @@ import type { } from '../../shared/types' import type { GitHistoryOptions, GitHistoryResult } from '../../shared/git-history' import type { PtyStartupIngressIntent } from '../../shared/pty-startup-ingress' +import type { PtyShutdownBlockReason } from '../../shared/pty-shutdown-safety' import type { CommitMessageDraftContext } from '../../shared/commit-message-generation' import type { WorkspaceSpaceDirectoryScanResult } from '../../shared/workspace-space-types' import type { StartupCommandDelivery } from '../../shared/codex-startup-delivery' @@ -161,6 +162,11 @@ export type IPtyProvider = { */ getAppliedSize?: (id: string) => Promise<{ cols: number; rows: number } | null> + /** A non-mutating preflight for shutdown modes that must preserve renderer ownership. */ + getShutdownBlockReason?: ( + id: string + ) => PtyShutdownBlockReason | null | Promise + // Why: deadlineMs (absolute epoch ms) bounds the underlying RPCs so destructive // teardown fails fast inside its sweep budget instead of tripping the outer sweep // deadline; each RPC leaf converts to a relative timeout when it actually issues. diff --git a/src/main/windows-pty-job-control.ts b/src/main/windows-pty-job-control.ts index 849d1bf10f6..7776ca0113a 100644 --- a/src/main/windows-pty-job-control.ts +++ b/src/main/windows-pty-job-control.ts @@ -1 +1,30 @@ export const WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS = 8_000 + +type WindowsPtyJobControl = { + pid: number + terminateJobTree?(timeoutMs: number): Promise | undefined +} + +export async function terminateWindowsPtyTree( + proc: WindowsPtyJobControl, + closeRoot: () => void +): Promise { + let nativeCompletion: Promise | undefined + try { + nativeCompletion = proc.terminateJobTree?.call(proc, WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS) + } catch (error) { + // Why: a synchronous native bridge failure must still close ConPTY while + // the rejected shutdown keeps destructive deletion fail-closed. + closeRoot() + throw error + } + if (!nativeCompletion) { + closeRoot() + throw new Error(`Windows PTY Job ownership unavailable for process ${proc.pid}`) + } + + closeRoot() + if (!(await nativeCompletion)) { + throw new Error(`Windows PTY Job did not drain for process ${proc.pid}`) + } +} diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 2e0d86ab3b2..a52c6eb69c7 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -27,6 +27,7 @@ import type { } from '../shared/terminal-render-desync-evidence' import type { MobileRelayStatus } from '../shared/mobile-relay-status' import type { MobilePairingConnectionMode } from '../shared/mobile-pairing-connection-mode' +import type { PtyShutdownBlockReason } from '../shared/pty-shutdown-safety' import type { CreateLocalOrcaProfileArgs, CreateLocalOrcaProfileResult, @@ -1350,6 +1351,7 @@ export type PreloadApi = { reportGeometry: (id: string, cols: number, rows: number) => void signal: (id: string, signal: string) => void clearBuffer: (id: string) => void + getShutdownBlockReason: (id: string) => Promise kill: (id: string, opts?: { keepHistory?: boolean }) => Promise ackColdRestore: (id: string) => void ackData: (id: string, charCount: number, processedChars?: number) => void diff --git a/src/preload/index.ts b/src/preload/index.ts index da24ff26aa7..d7aeadf14ad 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -950,6 +950,9 @@ const api = { ipcRenderer.send('pty:terminalViewAttributes', attributes) }, + getShutdownBlockReason: (id: string) => + ipcRenderer.invoke('pty:getShutdownBlockReason', { id }), + kill: (id: string, opts?: { keepHistory?: boolean }): Promise => ipcRenderer.invoke('pty:kill', { id, keepHistory: opts?.keepHistory ?? false }), diff --git a/src/relay/pty-handler.test.ts b/src/relay/pty-handler.test.ts index 0c0b33d9b94..04ea1923d26 100644 --- a/src/relay/pty-handler.test.ts +++ b/src/relay/pty-handler.test.ts @@ -11,6 +11,7 @@ import { SETUP_AGENT_SEQUENCE_STARTUP_COMMAND_ENV } from '../shared/setup-agent-sequencing' import { PTY_STARTUP_INGRESS_VERSION } from '../shared/pty-startup-ingress' +import { WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS } from '../main/windows-pty-job-control' const { mockPtySpawn, mockPtyInstance } = vi.hoisted(() => ({ mockPtySpawn: vi.fn(), @@ -1350,6 +1351,82 @@ describe('PtyHandler', () => { }) }) + it('Job-owns and drains a native Windows SSH relay agent', async () => { + await withWindowsPlatform(async () => { + let onExitCb: ((evt: { exitCode: number }) => void) | undefined + const order: string[] = [] + const terminateJobTree = vi.fn(async () => { + order.push('job') + return true + }) + const mockKill = vi.fn(() => { + order.push('conpty') + onExitCb?.({ exitCode: 137 }) + }) + mockPtySpawn.mockReturnValue({ + ...mockPtyInstance, + kill: mockKill, + terminateJobTree, + onData: vi.fn(), + onExit: vi.fn((cb: (evt: { exitCode: number }) => void) => { + onExitCb = cb + }) + }) + + await dispatcher.callRequest('pty.spawn', { + shellOverride: 'powershell.exe', + launchAgent: 'claude' + }) + expect(mockPtySpawn).toHaveBeenCalledWith( + expect.stringMatching(/powershell/i), + expect.any(Array), + expect.objectContaining({ windowsAgentJob: true }) + ) + + await dispatcher.callRequest('pty.shutdown', { id: 'pty-1', immediate: true }) + + expect(terminateJobTree).toHaveBeenCalledWith(WINDOWS_PTY_JOB_DRAIN_TIMEOUT_MS) + expect(order).toEqual(['job', 'conpty']) + }) + }) + + it('rejects and retains relay ownership when a Windows SSH agent Job does not drain', async () => { + await withWindowsPlatform(async () => { + const mockKill = vi.fn() + const terminateJobTree = vi.fn(async () => false) + mockPtySpawn.mockReturnValue({ + ...mockPtyInstance, + kill: mockKill, + terminateJobTree, + onData: vi.fn(), + onExit: vi.fn() + }) + + await dispatcher.callRequest('pty.spawn', { + shellOverride: 'powershell.exe', + launchAgent: 'claude' + }) + + await expect( + dispatcher.callRequest('pty.shutdown', { id: 'pty-1', immediate: true }) + ).rejects.toThrow('Windows PTY Job did not drain') + expectBareKills(mockKill, 1) + expect(handler.activePtyCount).toBe(1) + }) + }) + + it('does not Job-own WSL agents on a Windows SSH relay', async () => { + await withWindowsPlatform(async () => { + await dispatcher.callRequest('pty.spawn', { + shellOverride: 'wsl.exe', + launchAgent: 'claude' + }) + + const spawnOptions = mockPtySpawn.mock.calls[0]?.[2] + expect(spawnOptions).not.toHaveProperty('windowsAgentJob') + }) + }) + it('does not double-kill when immediate cleanup joins graceful shutdown', async () => { await withWindowsPlatform(async () => { let onExitCb: ((evt: { exitCode: number }) => void) | undefined diff --git a/src/relay/pty-handler.ts b/src/relay/pty-handler.ts index d06695342d0..fe5870f6f5d 100644 --- a/src/relay/pty-handler.ts +++ b/src/relay/pty-handler.ts @@ -39,6 +39,9 @@ import { } from '../shared/git-credential-prompt-env' import { isTuiAgent } from '../shared/tui-agent-config' import { forceKillPosixPtyProcessGroups } from '../main/pty/posix-pty-process-groups' +import { terminateWindowsPtyTree } from '../main/windows-pty-job-control' +import { isWslShellName } from '../shared/local-windows-terminal-runtime' +import { recognizeAgentProcessFromCommandLine } from '../shared/agent-process-recognition' import { PTY_STARTUP_INGRESS_VERSION, PtyStartupIngress, @@ -84,6 +87,7 @@ type ManagedPty = { physicalExit?: PhysicalExitTracker forceKillSent?: boolean gracefulKillSent?: boolean + windowsAgentJob?: boolean startupIngress?: PtyStartupIngress startupIngressIntent?: ReturnType } @@ -924,6 +928,13 @@ export class PtyHandler { const shouldProviderDeliverCommand = commandDelivery === 'provider' && command !== undefined const spawnEnv = this.buildSpawnEnv(env, { id, paneKey, shell, command }, envToDelete) const launchCommandHint = resolveSetupAgentSequenceLaunchCommand(spawnEnv, command) + const windowsAgentJob = + process.platform === 'win32' && + !isWslShellName(shell) && + Boolean( + isTuiAgent(params.launchAgent) || + recognizeAgentProcessFromCommandLine(launchCommandHint)?.agent + ) // Why: SSH PTYs bypass main's host-env builder. Apply the policy only // after the relay merges its authoritative process environment so indexed // Git config and remote Windows/WSL behavior remain intact. @@ -962,7 +973,8 @@ export class PtyHandler { cwd, // Why: relay shells inherit process.env; never let an ambient Orca marker // enable shell-ready behavior unless this spawn explicitly requested it. - env: { ...spawnEnv, ORCA_SHELL_READY_MARKER: '0', ...shellLaunch.env } + env: { ...spawnEnv, ORCA_SHELL_READY_MARKER: '0', ...shellLaunch.env }, + ...(windowsAgentJob ? { windowsAgentJob: true } : {}) }) } catch (error) { // Why: Windows node-pty loads conpty.node only on first spawn, after the @@ -1002,6 +1014,7 @@ export class PtyHandler { ...(explicitTerm !== undefined ? { explicitTerm } : {}), envToDelete, gitCredentialPromptGuarded, + ...(windowsAgentJob ? { windowsAgentJob: true } : {}), ...(startupIngressIntent ? { startupIngressIntent } : {}), ...(terminalHandle ? { terminalHandle } : {}), ...(shouldProviderDeliverCommand @@ -1143,7 +1156,18 @@ export class PtyHandler { return } - if (immediate) { + if (process.platform === 'win32' && managed.windowsAgentJob) { + this.releaseStartupCommand(managed) + if (immediate) { + this.flushPtyOutput(id) + } + // Why: the SSH relay is the native host on Windows; it must drain its Job + // before acknowledging close to a client that may delete the worktree. + await terminateWindowsPtyTree(managed.pty, () => this.requestForceKill(managed)) + if (immediate) { + await this.waitForPhysicalExit(managed, IMMEDIATE_PTY_EXIT_TIMEOUT_MS) + } + } else if (immediate) { this.releaseStartupCommand(managed) this.flushPtyOutput(id) this.requestForceKill(managed) diff --git a/src/relay/windows-agent-job.integration.test.ts b/src/relay/windows-agent-job.integration.test.ts new file mode 100644 index 00000000000..ad1af4eb6c4 --- /dev/null +++ b/src/relay/windows-agent-job.integration.test.ts @@ -0,0 +1,131 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { execFileSync } from 'node:child_process' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + SshChannelMultiplexer, + type MultiplexerTransport +} from '../main/ssh/ssh-channel-multiplexer' +import { RelayDispatcher } from './dispatcher' +import { PtyHandler } from './pty-handler' + +function quotePowerShell(value: string): string { + return `'${value.replaceAll("'", "''")}'` +} + +async function waitFor(predicate: () => boolean, timeoutMs = 15_000): Promise { + const deadline = Date.now() + timeoutMs + while (Date.now() < deadline) { + if (predicate()) { + return + } + await new Promise((resolve) => setTimeout(resolve, 50)) + } + throw new Error('Timed out waiting for Windows relay process evidence') +} + +describe.skipIf(process.platform !== 'win32')('Windows relay agent Job integration', () => { + let rootDir: string + let dispatcher: RelayDispatcher + let handler: PtyHandler + let mux: SshChannelMultiplexer + let lockHolderPid: number | null + + beforeEach(() => { + rootDir = mkdtempSync(join(tmpdir(), 'orca-relay-job-')) + lockHolderPid = null + let feedRelay: (data: Buffer) => void + const clientDataCallbacks: ((data: Buffer) => void)[] = [] + const clientTransport: MultiplexerTransport = { + write: (data) => setImmediate(() => feedRelay(data)), + onData: (callback) => clientDataCallbacks.push(callback), + onClose: () => {} + } + dispatcher = new RelayDispatcher((data) => { + setImmediate(() => clientDataCallbacks.forEach((callback) => callback(data))) + }) + feedRelay = (data) => dispatcher.feed(data) + handler = new PtyHandler(dispatcher) + mux = new SshChannelMultiplexer(clientTransport) + }) + + afterEach(async () => { + mux.dispose() + dispatcher.dispose() + await handler.dispose({ waitForPhysicalExit: false }).catch(() => {}) + if (lockHolderPid) { + try { + execFileSync('taskkill.exe', ['/PID', String(lockHolderPid), '/T', '/F'], { + stdio: 'ignore' + }) + } catch { + /* already gone */ + } + } + rmSync(rootDir, { recursive: true, force: true }) + }) + + it('drains a detached lock-holder before acknowledging remote shutdown', async () => { + const worktreeDir = join(rootDir, 'worktree') + const lockPath = join(worktreeDir, 'held.lock') + const pidPath = join(rootDir, 'lock-holder.pid') + const childScriptPath = join(rootDir, 'hold-lock.ps1') + const rootScriptPath = join(rootDir, 'launch-child.ps1') + mkdirSync(worktreeDir) + writeFileSync( + childScriptPath, + [ + 'param([string]$LockPath, [string]$PidPath)', + "$stream = [IO.File]::Open($LockPath, 'OpenOrCreate', 'ReadWrite', 'Read')", + '[IO.File]::WriteAllText($PidPath, [string]$PID)', + 'try { while ($true) { Start-Sleep -Milliseconds 250 } } finally { $stream.Dispose() }' + ].join('\r\n') + ) + writeFileSync( + rootScriptPath, + [ + 'param([string]$ChildScript, [string]$LockPath, [string]$PidPath)', + '$childArgs = "-NoLogo -NoProfile -ExecutionPolicy Bypass -File `"$ChildScript`" -LockPath `"$LockPath`" -PidPath `"$PidPath`""', + '$child = Start-Process powershell.exe -WindowStyle Hidden -PassThru -ArgumentList $childArgs', + 'while (-not (Test-Path -LiteralPath $PidPath)) { Start-Sleep -Milliseconds 50 }', + 'while ($true) { Start-Sleep -Milliseconds 250 }' + ].join('\r\n') + ) + writeFileSync(lockPath, '') + const command = + `& ${quotePowerShell(rootScriptPath)}` + + ` -ChildScript ${quotePowerShell(childScriptPath)}` + + ` -LockPath ${quotePowerShell(lockPath)}` + + ` -PidPath ${quotePowerShell(pidPath)}` + + const spawned = (await mux.request('pty.spawn', { + cols: 80, + rows: 24, + cwd: rootDir, + shellOverride: 'powershell.exe', + command, + commandDelivery: 'provider', + launchAgent: 'claude' + })) as { id: string } + await waitFor(() => existsSync(pidPath)) + lockHolderPid = Number(readFileSync(pidPath, 'utf8').trim()) + expect(lockHolderPid).toBeGreaterThan(0) + // Why: prove the child holds a real Windows delete-denying handle before teardown. + expect(() => rmSync(lockPath, { force: true })).toThrow() + + await mux.request('pty.shutdown', { id: spawned.id, immediate: true }) + await waitFor(() => { + try { + process.kill(lockHolderPid!, 0) + return false + } catch { + return true + } + }) + + rmSync(worktreeDir, { recursive: true, force: true }) + expect(existsSync(worktreeDir)).toBe(false) + lockHolderPid = null + }, 30_000) +}) diff --git a/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts b/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts index 3dd4aecc89e..ae680e2d0dd 100644 --- a/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts +++ b/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts @@ -12,6 +12,7 @@ export function closeLocalTerminalTabState( remoteCloseOwnedByHost?: boolean localPtyTeardownOwnedExternally?: boolean precomputedRetirementPlan?: TerminalTabRetirementPlan + shutdownSafetyChecked?: boolean } ): void { const state = useAppStore.getState() @@ -24,7 +25,8 @@ export function closeLocalTerminalTabState( options?.captureRecentlyClosed !== undefined || options?.remoteCloseOwnedByHost || options?.localPtyTeardownOwnedExternally || - options?.precomputedRetirementPlan + options?.precomputedRetirementPlan || + options?.shutdownSafetyChecked ) { state.closeTab(terminalTabId, options) } else { diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.ts b/src/renderer/src/components/terminal/terminal-tab-actions.ts index ebd861e6952..02b2c6fb69b 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.ts @@ -15,6 +15,7 @@ import type { TerminalTabCloseReason, TerminalTabRetirementPlan } from '@/store/slices/terminal-tab-retirement' +import { deferTerminalTabCloseForShutdownSafety } from '@/store/slices/terminal-shutdown-safety' import { closeLocalTerminalTabState } from './close-local-terminal-tab-state' import { getWorktreeTerminalTabIds, @@ -55,6 +56,8 @@ export function closeTerminalTab( localPtyTeardownOwnedExternally?: boolean precomputedRetirementPlan?: TerminalTabRetirementPlan precomputedCloseState?: PrecomputedTerminalCloseState + /** Internal re-entry after main has verified shutdown ownership. */ + shutdownSafetyChecked?: boolean onClosed?: () => void onCancel?: () => void } @@ -94,6 +97,29 @@ export function closeTerminalTab( return } + const retiresSession = options?.reason !== 'pty-exit' + if ( + retiresSession && + !options?.localPtyTeardownOwnedExternally && + !options?.shutdownSafetyChecked && + deferTerminalTabCloseForShutdownSafety({ + state, + tabId: terminalTabId, + ...(options?.precomputedRetirementPlan + ? { precomputedRetirementPlan: options.precomputedRetirementPlan } + : {}), + onAllowed: (retirementPlan) => + closeTerminalTab(tabId, { + ...options, + precomputedRetirementPlan: retirementPlan, + shutdownSafetyChecked: true + }), + ...(options?.onCancel ? { onBlocked: options.onCancel } : {}) + }) + ) { + return + } + const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(state, owningWorktreeId) if (runtimeEnvironmentId && isWebRuntimeSessionActive(runtimeEnvironmentId)) { // Why: a remote-owned worktree's tabs are host-authoritative, so the close @@ -124,7 +150,8 @@ export function closeTerminalTab( : {}), ...(options?.precomputedRetirementPlan ? { precomputedRetirementPlan: options.precomputedRetirementPlan } - : {}) + : {}), + ...(options?.shutdownSafetyChecked ? { shutdownSafetyChecked: true } : {}) }) void closeWebRuntimeSessionTab({ worktreeId: owningWorktreeId, @@ -151,7 +178,8 @@ export function closeTerminalTab( : {}), ...(options?.precomputedRetirementPlan ? { precomputedRetirementPlan: options.precomputedRetirementPlan } - : {}) + : {}), + ...(options?.shutdownSafetyChecked ? { shutdownSafetyChecked: true } : {}) }) if (state.activeWorktreeId === owningWorktreeId) { // Why: only deactivate the worktree when no tabs of any kind remain. @@ -193,7 +221,8 @@ export function closeTerminalTab( ...(options?.localPtyTeardownOwnedExternally ? { localPtyTeardownOwnedExternally: true } : {}), ...(options?.precomputedRetirementPlan ? { precomputedRetirementPlan: options.precomputedRetirementPlan } - : {}) + : {}), + ...(options?.shutdownSafetyChecked ? { shutdownSafetyChecked: true } : {}) }) options?.onClosed?.() } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 40f61f74600..122d541a956 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -315,6 +315,16 @@ "f518e89aa5": "Project already exists in that profile", "f03ae7f27b": "Failed to transfer project" } + }, + "terminal": { + "shutdown": { + "safety": { + "5a57c04372": "Terminal kept open for safety", + "8a31e30a75": "This older Windows session cannot be stopped safely by Orca. Exit the agent or shell in this terminal. If it stays running, end its process tree in Task Manager or restart Windows.", + "d929a6767b": "Terminal kept open because shutdown could not be verified", + "44c3706121": "Retry closing the terminal. Orca kept the tab and its process ownership intact." + } + } } } }, diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 64f47a9c258..85f3fe34a52 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -292,6 +292,16 @@ "f518e89aa5": "El proyecto ya existe en ese perfil", "f03ae7f27b": "Error al transferir proyecto" } + }, + "terminal": { + "shutdown": { + "safety": { + "5a57c04372": "Terminal mantenida abierta por seguridad", + "8a31e30a75": "Orca no puede detener de forma segura esta sesión antigua de Windows. Sal del agente o del shell en esta terminal. Si sigue en ejecución, finaliza su árbol de procesos en el Administrador de tareas o reinicia Windows.", + "d929a6767b": "Terminal mantenida abierta porque no se pudo verificar el cierre", + "44c3706121": "Vuelve a intentar cerrar la terminal. Orca mantuvo intactos la pestaña y el control de sus procesos." + } + } } } }, diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index 45b3893de81..27149416d26 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -292,6 +292,16 @@ "f518e89aa5": "プロジェクトはそのプロファイルに既に存在します", "f03ae7f27b": "プロジェクトの転送に失敗しました" } + }, + "terminal": { + "shutdown": { + "safety": { + "5a57c04372": "安全のためターミナルを開いたままにしました", + "8a31e30a75": "この古い Windows セッションは Orca から安全に停止できません。このターミナルでエージェントまたはシェルを終了してください。実行が続く場合は、タスク マネージャーでプロセス ツリーを終了するか、Windows を再起動してください。", + "d929a6767b": "シャットダウンを確認できなかったため、ターミナルを開いたままにしました", + "44c3706121": "ターミナルをもう一度閉じてください。Orca はタブとプロセスの所有状態を維持しました。" + } + } } } }, diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 106911afaad..22f002d234d 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -292,6 +292,16 @@ "f518e89aa5": "프로젝트가 해당 프로필에 이미 있습니다", "f03ae7f27b": "프로젝트를 전송하지 못했습니다" } + }, + "terminal": { + "shutdown": { + "safety": { + "5a57c04372": "안전을 위해 터미널을 열린 상태로 유지했습니다", + "8a31e30a75": "Orca는 이 이전 Windows 세션을 안전하게 중지할 수 없습니다. 이 터미널에서 에이전트나 셸을 종료하세요. 계속 실행되면 작업 관리자에서 해당 프로세스 트리를 종료하거나 Windows를 다시 시작하세요.", + "d929a6767b": "종료를 확인할 수 없어 터미널을 열린 상태로 유지했습니다", + "44c3706121": "터미널 닫기를 다시 시도하세요. Orca는 탭과 프로세스 소유권을 그대로 유지했습니다." + } + } } } }, diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index e54cec11c5b..83bcf162da9 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -292,6 +292,16 @@ "f518e89aa5": "该项目已存在于那个配置文件中", "f03ae7f27b": "转移项目失败" } + }, + "terminal": { + "shutdown": { + "safety": { + "5a57c04372": "为安全起见，终端保持打开", + "8a31e30a75": "Orca 无法安全停止这个较旧的 Windows 会话。请在此终端中退出代理或 Shell。如果它仍在运行，请在任务管理器中结束其进程树，或重启 Windows。", + "d929a6767b": "由于无法验证关闭操作，终端保持打开", + "44c3706121": "请重试关闭终端。Orca 保留了此标签页及其进程所有权。" + } + } } } }, diff --git a/src/renderer/src/store/slices/terminal-shutdown-safety.ts b/src/renderer/src/store/slices/terminal-shutdown-safety.ts new file mode 100644 index 00000000000..59959ba4921 --- /dev/null +++ b/src/renderer/src/store/slices/terminal-shutdown-safety.ts @@ -0,0 +1,132 @@ +import { toast } from 'sonner' +import { translate } from '@/i18n/i18n' +import { WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON } from '../../../../shared/pty-shutdown-safety' +import { + buildTerminalTabRetirementPlan, + type TerminalTabRetirementPlan, + type TerminalTabRetirementState +} from './terminal-tab-retirement' + +const pendingPreflights = new Set() + +function getShutdownSafetyPreflight(): + | ((id: string) => ReturnType) + | undefined { + return (globalThis.window?.api?.pty as Partial | undefined) + ?.getShutdownBlockReason +} + +export function hasTerminalShutdownSafetyPreflight(): boolean { + return typeof getShutdownSafetyPreflight() === 'function' +} + +function surfaceBlockedShutdown(surfaceId: string): void { + toast.error( + translate( + 'auto.store.slices.terminal.shutdown.safety.5a57c04372', + 'Terminal kept open for safety' + ), + { + id: `terminal-shutdown-safety-${surfaceId}`, + description: translate( + 'auto.store.slices.terminal.shutdown.safety.8a31e30a75', + 'This older Windows session cannot be stopped safely by Orca. Exit the agent or shell in this terminal. If it stays running, end its process tree in Task Manager or restart Windows.' + ), + duration: 15_000 + } + ) +} + +function surfacePreflightFailure(surfaceId: string): void { + toast.error( + translate( + 'auto.store.slices.terminal.shutdown.safety.d929a6767b', + 'Terminal kept open because shutdown could not be verified' + ), + { + id: `terminal-shutdown-safety-${surfaceId}`, + description: translate( + 'auto.store.slices.terminal.shutdown.safety.44c3706121', + 'Retry closing the terminal. Orca kept the tab and its process ownership intact.' + ), + duration: 15_000 + } + ) +} + +export async function assertTerminalShutdownSafety(args: { + surfaceId: string + ptyIds: string[] +}): Promise { + const getShutdownBlockReason = getShutdownSafetyPreflight() + if (!getShutdownBlockReason || args.ptyIds.length === 0) { + return + } + + let reasons: Awaited>[] + try { + reasons = await Promise.all(args.ptyIds.map((ptyId) => getShutdownBlockReason(ptyId))) + } catch (error) { + // Why: an unavailable ownership inventory is not permission to hide a live session. + console.warn('[terminal-retirement] shutdown safety preflight failed', { + surfaceId: args.surfaceId, + error + }) + surfacePreflightFailure(args.surfaceId) + throw error + } + + if (reasons.includes(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON)) { + surfaceBlockedShutdown(args.surfaceId) + throw new Error(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON) + } +} + +export function deferTerminalCloseForShutdownSafety(args: { + tabId: string + ptyIds: string[] + onAllowed: () => void + onBlocked?: () => void +}): boolean { + if (!hasTerminalShutdownSafetyPreflight() || args.ptyIds.length === 0) { + return false + } + if (pendingPreflights.has(args.tabId)) { + return true + } + + pendingPreflights.add(args.tabId) + void assertTerminalShutdownSafety({ surfaceId: args.tabId, ptyIds: args.ptyIds }).then( + () => { + pendingPreflights.delete(args.tabId) + args.onAllowed() + }, + () => { + pendingPreflights.delete(args.tabId) + args.onBlocked?.() + } + ) + return true +} + +export function deferTerminalTabCloseForShutdownSafety(args: { + state: TerminalTabRetirementState + tabId: string + precomputedRetirementPlan?: TerminalTabRetirementPlan + onAllowed: (plan: TerminalTabRetirementPlan) => void + onBlocked?: () => void +}): boolean { + if (!hasTerminalShutdownSafetyPreflight()) { + return false + } + const plan = + args.precomputedRetirementPlan?.tabId === args.tabId + ? args.precomputedRetirementPlan + : buildTerminalTabRetirementPlan(args.state, args.tabId) + return deferTerminalCloseForShutdownSafety({ + tabId: args.tabId, + ptyIds: plan.localOrSshPtyIds, + onAllowed: () => args.onAllowed(plan), + ...(args.onBlocked ? { onBlocked: args.onBlocked } : {}) + }) +} diff --git a/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts b/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts index 13df54d1d9f..473ce9be16b 100644 --- a/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts +++ b/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts @@ -1,7 +1,9 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' +import { WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON } from '../../../../shared/pty-shutdown-safety' const mockKill = vi.fn().mockResolvedValue(undefined) +const mockGetShutdownBlockReason = vi.fn().mockResolvedValue(null) const mockRuntimeCall = vi.fn().mockResolvedValue({ id: 'rpc-1', ok: true, @@ -11,12 +13,16 @@ const mockRuntimeCall = vi.fn().mockResolvedValue({ vi.stubGlobal('window', { api: { - pty: { kill: mockKill }, + pty: { kill: mockKill, getShutdownBlockReason: mockGetShutdownBlockReason }, runtime: { call: mockRuntimeCall }, runtimeEnvironments: { call: vi.fn() } } }) +vi.mock('sonner', () => ({ toast: { error: vi.fn() } })) + +import { toast } from 'sonner' + import { capturedPanesByTabId, parkedWatchersByTabId @@ -47,6 +53,7 @@ describe('terminal tab retirement store boundary', () => { beforeEach(() => { vi.clearAllMocks() mockKill.mockResolvedValue(undefined) + mockGetShutdownBlockReason.mockResolvedValue(null) mockRuntimeCall.mockResolvedValue({ id: 'rpc-1', ok: true, @@ -256,4 +263,50 @@ describe('terminal tab retirement store boundary', () => { expect(store.getState().tabsByWorktree['wt-1']).toEqual([]) warn.mockRestore() }) + + it('keeps a legacy Windows tab and PTY ownership visible when shutdown is unsafe', async () => { + const store = createTestStore() + mockGetShutdownBlockReason.mockResolvedValueOnce(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON) + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-v24' })] + }, + ptyIdsByTabId: { 'tab-1': ['pty-v24'] } + }) + + store.getState().closeTab('tab-1') + await vi.waitFor(() => + expect(toast.error).toHaveBeenCalledWith( + 'Terminal kept open for safety', + expect.objectContaining({ + description: expect.stringContaining('Task Manager') + }) + ) + ) + + expect(mockGetShutdownBlockReason).toHaveBeenCalledWith('pty-v24') + expect(mockKill).not.toHaveBeenCalled() + expect(store.getState().tabsByWorktree['wt-1']).toHaveLength(1) + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-v24']) + }) + + it('keeps sleep state and PTY ownership intact when legacy shutdown is unsafe', async () => { + const store = createTestStore() + mockGetShutdownBlockReason.mockResolvedValueOnce(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON) + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-v24' })] + }, + ptyIdsByTabId: { 'tab-1': ['pty-v24'] } + }) + + await expect( + store.getState().shutdownWorktreeTerminals('wt-1', { keepIdentifiers: true }) + ).rejects.toThrow(WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON) + + expect(mockGetShutdownBlockReason).toHaveBeenCalledWith('pty-v24') + expect(mockKill).not.toHaveBeenCalled() + expect(store.getState().tabsByWorktree['wt-1']).toHaveLength(1) + expect(store.getState().ptyIdsByTabId['tab-1']).toEqual(['pty-v24']) + }) }) diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index d6f8b196324..b3c91cd499a 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -77,6 +77,10 @@ import { resolvePtyBoundActiveLeafId } from '@/components/terminal-pane/terminal-layout-leaf-ids' import { shutdownBufferCaptures } from '@/components/terminal-pane/shutdown-buffer-captures' +import { + assertTerminalShutdownSafety, + deferTerminalCloseForShutdownSafety +} from './terminal-shutdown-safety' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { parseRemoteRuntimePtyId, toRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream' import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' @@ -613,6 +617,8 @@ export type TerminalSlice = { remoteCloseOwnedByHost?: boolean localPtyTeardownOwnedExternally?: boolean precomputedRetirementPlan?: TerminalTabRetirementPlan + /** Internal re-entry after main has verified shutdown ownership. */ + shutdownSafetyChecked?: boolean } ) => void reorderTabs: (worktreeId: string, tabIds: string[]) => void @@ -1224,6 +1230,23 @@ export const createTerminalSlice: StateCreator opts?.precomputedRetirementPlan?.tabId === tabId ? opts.precomputedRetirementPlan : buildTerminalTabRetirementPlan(get(), tabId) + if ( + retiresSession && + !opts?.localPtyTeardownOwnedExternally && + !opts?.shutdownSafetyChecked && + deferTerminalCloseForShutdownSafety({ + tabId, + ptyIds: retirementPlan.localOrSshPtyIds, + onAllowed: () => + get().closeTab(tabId, { + ...opts, + precomputedRetirementPlan: retirementPlan, + shutdownSafetyChecked: true + }) + }) + ) { + return + } let closingWorktreeId: string | null = null // Why: a parked tab has no mounted TerminalPane cleanup. Retirement must @@ -2493,6 +2516,10 @@ export const createTerminalSlice: StateCreator const rendererShutdownPtyIds = sortedUniquePtyIds(ptyIds) const expectedRuntimePtyIds = sortedUniquePtyIds(opts?.expectedRuntimePtyIds) const runtimeEnvironmentId = resolveTerminalStopRuntimeEnvironmentId(get(), worktreeId) + await assertTerminalShutdownSafety({ + surfaceId: worktreeId, + ptyIds: rendererShutdownPtyIds.filter((ptyId) => !ptyId.startsWith('remote:')) + }) // Why: expectedRuntimePtyIds are raw RPC handles. Only renderer-bound ids // can emit pane exit callbacks, so they are the complete guard identity set. const exitGuardPtyIds = rendererShutdownPtyIds diff --git a/src/shared/pty-shutdown-safety.ts b/src/shared/pty-shutdown-safety.ts new file mode 100644 index 00000000000..5606caaed7e --- /dev/null +++ b/src/shared/pty-shutdown-safety.ts @@ -0,0 +1,4 @@ +export const WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON = + 'windows-legacy-pty-shutdown-unsafe' as const + +export type PtyShutdownBlockReason = typeof WINDOWS_LEGACY_PTY_SHUTDOWN_BLOCK_REASON