From 7afce2ea41af48702c1642e5fb1052fa7bd9c756 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Mon, 17 Aug 2026 00:11:19 -0700 Subject: [PATCH] fix(ssh): stop reporting a confirmed kill when the SSH provider is gone (#14977) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(ssh): stop reporting a confirmed kill when the SSH provider is gone A detached relay PTY is designed to outlive the provider that addressed it (it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider is no longer registered" is lost contact, never evidence the remote process stopped. Both stop primitives in the PTY controller returned `true` from that branch, and every caller downstream reported the fabricated success: the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped, and — because the stop "succeeded" — the unstopped-PTY gate never ran, so worktree removal walked straight past a live remote agent. `kill`/`stopAndWait` now still tombstone the local lease but report an unconfirmed stop and record why, using the three-verdict vocabulary the worktree teardown gate already spoke (`live` / `unverifiable` / `exited`), promoted out of that module into `src/shared/pty-liveness-verdict.ts`. The close receipt, the CLI wording, worker-stop and the removal gate all read that verdict instead of inferring an exit from silence. The same rule fixes the mirror-image defect: the aggregate inventory only enumerates registered providers, so a dropped relay clears `connected` for every remote PTY at once. The sweep now separates the provider answering "absent" (an exit) from no provider being able to answer (lost contact), so worker-stop stops claiming `exited` from a disconnect. The `connected` wire field is unchanged in meaning and shape. * fix(orchestration): apply the same honesty to the federation stop path The federation host runs its own copy of the worker observation and stop logic, with the same two defects: `inspectRemoteAttachment` read a dropped relay's `connected: false` as `exited`, and `federationStop` settled the dispatch as stopped from a close it never confirmed — relaying a fabricated success all the way home to the coordinator. Two guards also had to move so the honest verdict does not become a new refusal. `federationRead` gated on `status !== 'running'`, which would have rejected a connected terminal the moment a stop lost contact with it; it now gates on `status === 'exited'`, which is equivalent for every pre-existing status given the two guards beside it. Local `workerStop` likewise still attempts the close when the verdict is `unverifiable` — losing contact is a reason to report the outcome honestly, never a reason to stop trying. The show observations now carry the reason alongside the status, so a bare `unverifiable` is actionable. Both are new optional fields. * fix(ssh): preserve unconfirmed stop verdicts across consumers * fix(ssh): use canonical live verdict wording * fix(ssh): refuse wrong-host teardown verification * test(orchestration): confirm worker release teardown * fix(orchestration): negotiate honest worker stop receipts * fix(agent-teams): fence uncertain teammate respawns * fix(ssh): avoid duplicate missing-provider teardown * fix(orchestration): preserve archives across release retries * fix(ssh): preserve verdicts across synthetic kill exits * fix(ssh): preserve liveness evidence across teardown * fix(agent-teams): replace panes only after confirmed stop * fix(ssh): distinguish host exits from relay loss * fix(ssh): narrow concurrent inventory verdicts * fix(orchestration): serve archives after uncertain release * fix(orchestration): expose unverifiable read liveness * test(ssh): align liveness assertions with verdicts * fix(ssh): preserve host scope across inventory failures --- src/cli/terminal-format.test.ts | 39 ++- src/cli/terminal-format.ts | 18 +- .../process-gone-killed-one-ordering.test.ts | 7 +- .../process-gone-recorder.test.ts | 4 +- .../pty-daemon-controller-teardown.test.ts | 27 +- .../pty-daemon-ssh-lease-lifecycle.test.ts | 2 +- .../ipc/pty-runtime-kill-and-exit.test.ts | 25 +- src/main/ipc/pty-ssh-stop-verdict.test.ts | 297 ++++++++++++++++++ src/main/ipc/pty.ts | 119 +++++-- .../claude-agent-teams-service.test.ts | 57 +++- .../claude-agent-teams-tmux-dispatcher.ts | 42 ++- src/main/runtime/claude-agent-teams-types.ts | 1 + ...ntime-process-incarnation-liveness.test.ts | 16 +- ...-runtime-terminal-close-continuity.test.ts | 73 ++++- ...a-runtime-terminal-split-authority.test.ts | 6 +- src/main/runtime/orca-runtime.ts | 212 +++++++++++-- .../worker-terminal-release.ts | 5 +- .../worker-terminal-process-liveness.ts | 6 +- .../pty-inventory-liveness-verdict.test.ts | 194 ++++++++++++ .../orchestration-federation-control.ts | 72 ++++- ...ration-federation-liveness-verdict.test.ts | 183 +++++++++++ .../orchestration-federation-output.test.ts | 2 +- .../orchestration-federation-setup.test.ts | 1 + .../methods/orchestration-federation.test.ts | 5 +- .../methods/orchestration-worker-control.ts | 28 +- .../orchestration-worker-observation.ts | 21 +- .../orchestration-worker-output.test.ts | 13 + .../methods/orchestration-worker-output.ts | 13 +- ...orchestration-worker-release-completion.ts | 15 +- ...on-worker-release-liveness-verdict.test.ts | 84 +++++ ...hestration-worker-release-recovery.test.ts | 35 ++- .../orchestration-worker-release.test.ts | 15 +- .../methods/orchestration-worker-release.ts | 2 +- ...chestration-worker-stop-capability.test.ts | 57 ++++ ...ation-worker-stop-liveness-verdict.test.ts | 158 ++++++++++ .../rpc/methods/orchestration-worker-stop.ts | 37 ++- .../orchestration-workers-recovery.test.ts | 5 +- .../runtime/unstopped-pty-verification.ts | 49 ++- .../worktree-teardown-unstopped-pty.test.ts | 74 +++++ src/main/runtime/worktree-teardown.test.ts | 47 ++- src/main/runtime/worktree-teardown.ts | 21 +- .../commit-message-text-generation.ts | 4 +- src/shared/orchestration-worker-output.ts | 3 + src/shared/protocol-version.ts | 3 + src/shared/pty-liveness-verdict.ts | 34 ++ src/shared/runtime-types.ts | 8 + 46 files changed, 1978 insertions(+), 161 deletions(-) create mode 100644 src/main/ipc/pty-ssh-stop-verdict.test.ts create mode 100644 src/main/runtime/pty-inventory-liveness-verdict.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration-worker-stop-capability.test.ts create mode 100644 src/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts create mode 100644 src/shared/pty-liveness-verdict.ts diff --git a/src/cli/terminal-format.test.ts b/src/cli/terminal-format.test.ts index 014f6339823..0656234e292 100644 --- a/src/cli/terminal-format.test.ts +++ b/src/cli/terminal-format.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { formatTerminalFocus } from './terminal-format' +import { formatTerminalClose, formatTerminalFocus } from './terminal-format' describe('formatTerminalFocus', () => { it('distinguishes superseded navigation from a winning focus', () => { @@ -22,3 +22,40 @@ describe('formatTerminalFocus', () => { ).toBe('Focused terminal term_winner (tab tab-winner).') }) }) + +describe('formatTerminalClose', () => { + it('prints "PTY killed." only for a confirmed kill', () => { + expect( + formatTerminalClose({ close: { handle: 'term_local', tabId: 'tab-1', ptyKilled: true } }) + ).toBe('Closed terminal term_local. PTY killed.') + }) + + it('says the remote process was not confirmed stopped instead of claiming a kill', () => { + expect( + formatTerminalClose({ + close: { + handle: 'term_remote', + tabId: 'tab-1', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH provider is no longer registered' + } + }) + ).toBe( + 'Closed terminal term_remote. The PTY was not confirmed stopped: its SSH provider is no longer registered.' + ) + }) + + it('names a PTY known to be live', () => { + expect( + formatTerminalClose({ + close: { + handle: 'term_live', + tabId: 'tab-1', + ptyKilled: false, + ptyStopVerdict: 'live' + } + }) + ).toBe('Closed terminal term_live. The PTY is live.') + }) +}) diff --git a/src/cli/terminal-format.ts b/src/cli/terminal-format.ts index a4df4ba6951..717f1d33701 100644 --- a/src/cli/terminal-format.ts +++ b/src/cli/terminal-format.ts @@ -1,3 +1,4 @@ +import { PTY_LIVE_NOTE, describeUnconfirmedStop } from '../shared/pty-liveness-verdict' import type { RuntimeTerminalClose, RuntimeTerminalCreate, @@ -185,12 +186,25 @@ export function formatTerminalFocus(result: { focus: RuntimeTerminalFocus }): st return `Focused terminal ${result.focus.handle} (tab ${result.focus.tabId}).` } +/** "PTY killed." is a claim of observed death, so only a confirmed kill earns it. */ +function describePtyStop(close: RuntimeTerminalClose): string { + if (close.ptyKilled) { + return ' PTY killed.' + } + if (close.ptyStopVerdict === 'live') { + return ` ${PTY_LIVE_NOTE}` + } + if (close.ptyStopVerdict === 'unverifiable') { + return ` ${describeUnconfirmedStop(close.ptyStopReason ?? 'its host could not be reached')}` + } + return '' +} + export function formatTerminalClose(result: { close: RuntimeTerminalClose }): string { if (result.close.closeMode === 'tab') { return `Closed terminal tab ${result.close.tabId} (${result.close.handle}).` } - const ptyNote = result.close.ptyKilled ? ' PTY killed.' : '' - return `Closed terminal ${result.close.handle}.${ptyNote}` + return `Closed terminal ${result.close.handle}.${describePtyStop(result.close)}` } export function formatTerminalWait(result: { wait: RuntimeTerminalWait }): string { diff --git a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts index 72c897088f5..2a4ded3c2a5 100644 --- a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts +++ b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts @@ -85,12 +85,7 @@ describe('recordProcessGoneCrash killed/1 ordering', () => { recordProcessGoneCrash({ record } as never, gpuKill, dedupe) recordProcessGoneCrash({ record } as never, networkServiceKill, dedupe) - recordProcessGoneCrash( - { record, attachDetails } as never, - rendererKill, - dedupe, - noMinidump - ) + recordProcessGoneCrash({ record, attachDetails } as never, rendererKill, dedupe, noMinidump) expect(record).toHaveBeenCalledOnce() // Timing proximity is evidence, not authority to discard an ambiguous report. diff --git a/src/main/crash-reporting/process-gone-recorder.test.ts b/src/main/crash-reporting/process-gone-recorder.test.ts index 70e6ca93f04..24749b1bacf 100644 --- a/src/main/crash-reporting/process-gone-recorder.test.ts +++ b/src/main/crash-reporting/process-gone-recorder.test.ts @@ -439,7 +439,9 @@ describe('recordProcessGoneCrash', () => { // Why child kills: the decode gate is source-agnostic and reads // process.platform synchronously at record time, so the platform stub is // still in force when it runs. - const nonRecoverableChildKill = (overrides: Partial): ProcessGoneCrashEvent => + const nonRecoverableChildKill = ( + overrides: Partial + ): ProcessGoneCrashEvent => event({ source: 'child', processType: 'Utility', diff --git a/src/main/ipc/pty-daemon-controller-teardown.test.ts b/src/main/ipc/pty-daemon-controller-teardown.test.ts index 25d0aa48277..73ae897b8b3 100644 --- a/src/main/ipc/pty-daemon-controller-teardown.test.ts +++ b/src/main/ipc/pty-daemon-controller-teardown.test.ts @@ -153,7 +153,7 @@ describe('registerPtyHandlers', () => { }) expect(runtime.onPtyExit).not.toHaveBeenCalled() }) - it('does not accept an incarnation-less exit as proof that the current PTY stopped', async () => { + it('uses inventory, not an incarnation-less exit, to confirm the current PTY stopped', async () => { const exitListeners = new Set< (payload: { id: string; code: number; incarnationId?: string }) => void >() @@ -203,7 +203,7 @@ describe('registerPtyHandlers', () => { await controller.spawn({ cols: 80, rows: 24 }) await expect(controller.stopAndWait('local-incarnated')).resolves.toBe(true) - expect(runtime.onPtyExit).toHaveBeenCalledWith('local-incarnated', -1, 'incarnation-live') + expect(runtime.onPtyExit).toHaveBeenCalledWith('local-incarnated', 0, 'incarnation-live') }) it('runtime controller kill routes app-scoped SSH ids through the parsed provider when ownership is absent', async () => { const localShutdown = vi.fn() @@ -319,7 +319,9 @@ describe('registerPtyHandlers', () => { kill: (ptyId: string) => boolean } - expect(controller.kill('ssh:ssh-1@@relay-pty')).toBe(true) + // The lease is tombstoned, but nothing observed the detached relay PTY + // stop, so the stop itself is reported as unconfirmed. + expect(controller.kill('ssh:ssh-1@@relay-pty')).toBe(false) expect(localShutdown).not.toHaveBeenCalled() expect(store.markSshRemotePtyLease).toHaveBeenCalledWith('ssh-1', 'relay-pty', 'terminated') @@ -347,7 +349,7 @@ describe('registerPtyHandlers', () => { kill: (ptyId: string) => boolean } - expect(controller.kill('remote-pty')).toBe(true) + expect(controller.kill('remote-pty')).toBe(false) expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( 'ssh-1', @@ -356,14 +358,15 @@ describe('registerPtyHandlers', () => { ) expect(runtime.onPtyExit).toHaveBeenCalledWith('remote-pty', -1, undefined) }) - it('retires a rejected SSH PTY after generic kill shutdown fails transiently', async () => { + it('keeps a rejected SSH PTY unverifiable after kill shutdown fails transiently', async () => { const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) const store = { markSshRemotePtyLease: vi.fn() } const runtime = { setPtyController: vi.fn(), - onPtyExit: vi.fn() + onPtyExit: vi.fn(), + markPtyLivenessUnverifiable: vi.fn() } registerSshPtyProvider('ssh-1', { spawn: vi.fn(), @@ -399,7 +402,7 @@ describe('registerPtyHandlers', () => { ) const controller = runtime.setPtyController.mock.calls[0]?.[0] as { kill: (ptyId: string) => boolean - retireRejectedPty: (ptyId: string) => void + retireRejectedPty: (ptyId: string, stopConfirmed: boolean) => void } try { @@ -410,19 +413,23 @@ describe('registerPtyHandlers', () => { 'remote-pty', 'terminated' ) - controller.retireRejectedPty('remote-pty') + controller.retireRejectedPty('remote-pty', false) } finally { warnSpy.mockRestore() deletePtyOwnership('remote-pty') } - expect(store.markSshRemotePtyLease).toHaveBeenCalledWith( + expect(store.markSshRemotePtyLease).not.toHaveBeenCalledWith( 'ssh-1', 'remote-pty', 'terminated' ) + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + 'remote-pty', + 'a follow-up stop was issued but its outcome could not be verified' + ) expect(runtime.onPtyExit).toHaveBeenCalledWith('remote-pty', -1, undefined) - expect(runtime.onPtyExit).toHaveBeenCalledWith('remote-pty', 0, undefined) + expect(runtime.onPtyExit).not.toHaveBeenCalledWith('remote-pty', 0, undefined) }) it('strips ORCA_PANE_KEY/TAB_ID/WORKTREE_ID from SSH spawn env when remote agent hooks are disabled', async () => { const sshSpawn = vi.fn(async (_opts: { env: Record }) => ({ diff --git a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts index 92c4b7f5d35..d2e07b8458c 100644 --- a/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts +++ b/src/main/ipc/pty-daemon-ssh-lease-lifecycle.test.ts @@ -459,7 +459,7 @@ describe('registerPtyHandlers', () => { 'remote-pty', 'terminated' ) - expect(runtime.onPtyExit).toHaveBeenCalledWith('remote-pty', -1, undefined) + expect(runtime.onPtyExit).toHaveBeenCalledWith('remote-pty', 0, undefined) }) it('splits the teardown budget so the liveness RPC gets only what shutdown left', async () => { // Why: sequential RPCs must share one absolute deadline; otherwise both get diff --git a/src/main/ipc/pty-runtime-kill-and-exit.test.ts b/src/main/ipc/pty-runtime-kill-and-exit.test.ts index bcef15a2492..2da8b80d4b8 100644 --- a/src/main/ipc/pty-runtime-kill-and-exit.test.ts +++ b/src/main/ipc/pty-runtime-kill-and-exit.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { spawnMock } from './pty-ipc-mock-registry' import { makeDeferred } from './pty-ipc-test-constants' import { setupPtyIpcSuite } from './pty-ipc-test-harness' +import { LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId } from '../../shared/execution-host' import { registerPtyHandlers, registerSshPtyProvider, @@ -102,11 +103,16 @@ describe('registerPtyHandlers', () => { }) registerSshPtyProvider('ssh-a', { listProcesses: sshAList } as never) registerSshPtyProvider('ssh-b', { listProcesses: sshBList } as never) - const runtime = { setPtyController: vi.fn() } + setPtyOwnership('ssh-b-pty', 'ssh-b') + const runtime = { + setPtyController: vi.fn(), + markPtyLivenessUnverifiable: vi.fn() + } handlers.clear() registerPtyHandlers(mainWindow as never, runtime as never) const controller = runtime.setPtyController.mock.calls[0]?.[0] as { listProcesses(connectionId?: string | null): Promise<{ id: string }[]> + listProcessesWithHostScope(): Promise<{ processes: { id: string }[]; hostIds: string[] }> } await expect(controller.listProcesses(null)).resolves.toEqual([ @@ -120,7 +126,22 @@ describe('registerPtyHandlers', () => { expect(sshAList).toHaveBeenCalledOnce() expect(sshBList).not.toHaveBeenCalled() - await expect(controller.listProcesses()).rejects.toThrow('ssh-b unavailable') + await expect(controller.listProcesses()).resolves.toEqual([ + { id: 'local-pty', title: 'Local', cwd: '/local' }, + { id: 'ssh-a-pty' } + ]) + expect(sshBList).toHaveBeenCalledOnce() + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + 'ssh-b-pty', + 'ssh-b unavailable' + ) + + await expect(controller.listProcessesWithHostScope()).resolves.toEqual({ + processes: [{ id: 'local-pty', title: 'Local', cwd: '/local' }, { id: 'ssh-a-pty' }], + hostIds: [LOCAL_EXECUTION_HOST_ID, toSshExecutionHostId('ssh-a')] + }) + expect(sshBList).toHaveBeenCalledTimes(2) + deletePtyOwnership('ssh-b-pty') }) it('returns unavailable runtime confirmation for unsupported or missing providers', async () => { registerSshPtyProvider('ssh-1', {} as never) diff --git a/src/main/ipc/pty-ssh-stop-verdict.test.ts b/src/main/ipc/pty-ssh-stop-verdict.test.ts new file mode 100644 index 00000000000..8001028ea1d --- /dev/null +++ b/src/main/ipc/pty-ssh-stop-verdict.test.ts @@ -0,0 +1,297 @@ +import { describe, expect, it, vi } from 'vitest' +import { SSH_PROVIDER_UNREGISTERED_REASON } from '../../shared/pty-liveness-verdict' +import { setupPtyIpcSuite } from './pty-ipc-test-harness' +import { + registerPtyHandlers, + deletePtyOwnership, + setPtyOwnership, + getLocalPtyProvider, + registerSshPtyProvider, + unregisterSshPtyProvider +} from './pty' + +vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock())) +vi.mock('fs', () => import('./pty-ipc-mock-registry').then((m) => m.fsModuleMock())) +vi.mock('node-pty', () => import('./pty-ipc-mock-registry').then((m) => m.nodePtyModuleMock())) +vi.mock('node:child_process', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).childProcessModuleMock(await importOriginal()) +) +vi.mock('../opencode/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.openCodeHookServiceModuleMock()) +) +vi.mock('../mimo/hook-service', () => + import('./pty-ipc-mock-registry').then((m) => m.mimoHookServiceModuleMock()) +) +vi.mock('../agent-hooks/server', () => + import('./pty-ipc-mock-registry').then((m) => m.agentHookServerModuleMock()) +) +vi.mock('../pi/titlebar-extension-service', () => + import('./pty-ipc-mock-registry').then((m) => m.piTitlebarExtensionModuleMock()) +) +vi.mock('../pwsh', () => import('./pty-ipc-mock-registry').then((m) => m.pwshModuleMock())) +vi.mock('../wsl', async (importOriginal) => + (await import('./pty-ipc-mock-registry')).wslModuleMock(await importOriginal()) +) +vi.mock('../telemetry/client', () => + import('./pty-ipc-mock-registry').then((m) => m.telemetryClientModuleMock()) +) +vi.mock('../telemetry/classify-error', () => + import('./pty-ipc-mock-registry').then((m) => m.classifyErrorModuleMock()) +) +vi.mock('../cli/linux-terminal-orca-cli-shim', () => + import('./pty-ipc-mock-registry').then((m) => m.linuxCliShimModuleMock()) +) +vi.mock('../memory/pty-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.ptyRegistryModuleMock()) +) +vi.mock('../agent-hooks/migration-unsupported-pty-state', () => + import('./pty-ipc-mock-registry').then((m) => m.migrationUnsupportedPtyModuleMock()) +) +vi.mock('../codex/codex-pane-account-registry', () => + import('./pty-ipc-mock-registry').then((m) => m.codexPaneAccountRegistryModuleMock()) +) +vi.mock('../codex/codex-state-db-backfill-recovery', () => + import('./pty-ipc-mock-registry').then((m) => m.codexBackfillRecoveryModuleMock()) +) + +// A detached relay PTY is designed to outlive the provider that addressed it, so +// "the SSH provider is gone" is never evidence that the remote process stopped. +describe('stopping a PTY whose SSH provider is unregistered', () => { + const { handlers, mainWindow, installObservableDaemonTestProvider } = setupPtyIpcSuite() + + function installController(): { + controller: { + kill: (ptyId: string) => boolean + listProcesses: (connectionId?: string | null) => Promise<{ id: string }[]> + retireRejectedPty: (ptyId: string, stopConfirmed: boolean) => void + stopAndWait: (ptyId: string, opts?: { deadlineMs?: number }) => Promise + } + runtime: { + setPtyController: ReturnType + onPtyExit: ReturnType + markPtyLivenessUnverifiable: ReturnType + markPtyLivenessLive: ReturnType + } + } { + const runtime = { + setPtyController: vi.fn(), + onPtyExit: vi.fn(), + markPtyLivenessUnverifiable: vi.fn(), + markPtyLivenessLive: vi.fn() + } + handlers.clear() + registerPtyHandlers(mainWindow as never, runtime as never) + return { + controller: runtime.setPtyController.mock.calls[0]?.[0] as never, + runtime + } + } + + it('reports an unconfirmed stop instead of a fabricated kill', () => { + setPtyOwnership('ssh-detached-pty', 'ssh-dropped') + const { controller, runtime } = installController() + + expect(controller.kill('ssh-detached-pty')).toBe(false) + // The local lease is still tombstoned so reconnect cannot revive the pane. + expect(runtime.onPtyExit).toHaveBeenCalledWith('ssh-detached-pty', -1, undefined) + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + 'ssh-detached-pty', + expect.stringContaining('SSH') + ) + deletePtyOwnership('ssh-detached-pty') + }) + + it('reports an unconfirmed exact stop instead of a fabricated teardown', async () => { + setPtyOwnership('ssh-detached-stop', 'ssh-dropped') + const { controller, runtime } = installController() + + await expect(controller.stopAndWait('ssh-detached-stop')).resolves.toBe(false) + expect(runtime.onPtyExit).toHaveBeenCalledWith('ssh-detached-stop', -1, undefined) + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + 'ssh-detached-stop', + expect.stringContaining('SSH') + ) + deletePtyOwnership('ssh-detached-stop') + }) + + it('preserves lost-contact evidence for renderer IPC teardown', async () => { + const ptyId = 'ssh-renderer-detached' + setPtyOwnership(ptyId, 'ssh-dropped') + const { runtime } = installController() + try { + await handlers.get('pty:kill')!(null, { id: ptyId }) + + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + ptyId, + SSH_PROVIDER_UNREGISTERED_REASON + ) + expect(runtime.onPtyExit).toHaveBeenCalledWith(ptyId, -1, undefined) + } finally { + deletePtyOwnership(ptyId) + } + }) + + it('retires a rejected split without asserting an unconfirmed exit', () => { + const ptyId = 'ssh-rejected-split' + setPtyOwnership(ptyId, 'ssh-dropped') + const { controller, runtime } = installController() + try { + controller.retireRejectedPty(ptyId, false) + + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + ptyId, + 'a follow-up stop was issued but its outcome could not be verified' + ) + expect(runtime.onPtyExit).toHaveBeenCalledWith(ptyId, -1, undefined) + expect(runtime.onPtyExit).not.toHaveBeenCalledWith(ptyId, 0, expect.anything()) + } finally { + deletePtyOwnership(ptyId) + } + }) + + it('still confirms a stop the owning provider actually performed', async () => { + const daemon = installObservableDaemonTestProvider() + vi.spyOn(getLocalPtyProvider(), 'listProcesses').mockResolvedValue([]) + const { controller, runtime } = installController() + + await expect(controller.stopAndWait('wt-1@@local-session')).resolves.toBe(true) + expect(daemon.shutdown).toHaveBeenCalledWith( + 'wt-1@@local-session', + expect.objectContaining({ immediate: true }) + ) + expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalled() + }) + + it('records provider-confirmed absence when the exit event was missed', async () => { + const connectionId = 'ssh-confirmed-absent' + const ptyId = 'ssh-confirmed-absent-pty' + const provider = { + onExit: vi.fn(() => () => {}), + shutdown: vi.fn(async () => {}), + listProcesses: vi.fn(async () => []) + } + registerSshPtyProvider(connectionId, provider as never) + setPtyOwnership(ptyId, connectionId) + try { + const { controller, runtime } = installController() + + await expect(controller.stopAndWait(ptyId)).resolves.toBe(true) + expect(runtime.onPtyExit).toHaveBeenCalledWith(ptyId, 0, undefined) + expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalled() + } finally { + deletePtyOwnership(ptyId) + unregisterSshPtyProvider(connectionId) + } + }) + + it('reports lost contact when a registered provider drops during the stop', async () => { + const connectionId = 'ssh-mid-stop-drop' + const ptyId = 'ssh-mid-stop-pty' + const provider = { + onExit: vi.fn(() => () => {}), + shutdown: vi.fn(async () => { + throw new Error('relay disconnected during stop') + }) + } + registerSshPtyProvider(connectionId, provider as never) + setPtyOwnership(ptyId, connectionId) + try { + const { controller, runtime } = installController() + + await expect(controller.stopAndWait(ptyId)).resolves.toBe(false) + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + ptyId, + 'relay disconnected during stop' + ) + } finally { + deletePtyOwnership(ptyId) + unregisterSshPtyProvider(connectionId) + } + }) + + it('preserves lost-contact evidence when fire-and-forget shutdown rejects', async () => { + const connectionId = 'ssh-async-kill-drop' + const ptyId = 'ssh-async-kill-pty' + const provider = { + onExit: vi.fn(() => () => {}), + shutdown: vi.fn(async () => { + throw new Error('relay disconnected during kill') + }) + } + registerSshPtyProvider(connectionId, provider as never) + setPtyOwnership(ptyId, connectionId) + try { + const { controller, runtime } = installController() + + expect(controller.kill(ptyId)).toBe(true) + await vi.waitFor(() => + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + ptyId, + 'relay disconnected during kill' + ) + ) + expect(runtime.onPtyExit).toHaveBeenCalledWith(ptyId, -1, undefined) + } finally { + deletePtyOwnership(ptyId) + unregisterSshPtyProvider(connectionId) + } + }) + + it('isolates a failed SSH inventory from healthy providers', async () => { + const failedConnectionId = 'ssh-inventory-failed' + const healthyConnectionId = 'ssh-inventory-healthy' + const failedPtyId = 'ssh-inventory-failed-pty' + const healthyPtyId = 'ssh-inventory-healthy-pty' + registerSshPtyProvider(failedConnectionId, { + listProcesses: vi.fn(async () => { + throw new Error('inventory transport failed') + }) + } as never) + registerSshPtyProvider(healthyConnectionId, { + listProcesses: vi.fn(async () => [{ id: healthyPtyId }]) + } as never) + setPtyOwnership(failedPtyId, failedConnectionId) + setPtyOwnership(healthyPtyId, healthyConnectionId) + try { + const { controller, runtime } = installController() + + await expect(controller.listProcesses()).resolves.toEqual( + expect.arrayContaining([{ id: healthyPtyId }]) + ) + expect(runtime.markPtyLivenessUnverifiable).toHaveBeenCalledWith( + failedPtyId, + 'inventory transport failed' + ) + expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalledWith( + healthyPtyId, + expect.anything() + ) + } finally { + deletePtyOwnership(failedPtyId) + deletePtyOwnership(healthyPtyId) + unregisterSshPtyProvider(failedConnectionId) + unregisterSshPtyProvider(healthyConnectionId) + } + }) + + it('reports a provider-observed survivor as live', async () => { + const connectionId = 'ssh-still-live' + const ptyId = 'ssh-still-live-pty' + const provider = { + onExit: vi.fn(() => () => {}), + shutdown: vi.fn(async () => {}), + listProcesses: vi.fn(async () => [{ id: ptyId }]) + } + registerSshPtyProvider(connectionId, provider as never) + setPtyOwnership(ptyId, connectionId) + try { + const { controller, runtime } = installController() + + await expect(controller.stopAndWait(ptyId)).resolves.toBe(false) + expect(runtime.markPtyLivenessLive).toHaveBeenCalledWith(ptyId) + } finally { + deletePtyOwnership(ptyId) + unregisterSshPtyProvider(connectionId) + } + }) +}) diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 376b705ef05..10ee17200db 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -15,6 +15,7 @@ export { getBashShellReadyRcfileContent } from '../providers/local-pty-shell-rea import type { OrcaRuntimeService } from '../runtime/orca-runtime' import type { PtyBindingSourceExpectation, Store } from '../persistence' import { retireTerminalSurfaceFromPersistence } from '../runtime/mobile-session-terminal-persistence-retirement' +import { SSH_PROVIDER_UNREGISTERED_REASON } from '../../shared/pty-liveness-verdict' import type { GlobalSettings } from '../../shared/global-settings-types' import type { TuiAgent } from '../../shared/tui-agent' import { @@ -264,19 +265,36 @@ function registeredPtyProviders(): RegisteredPtyProvider[] { ] } -async function listRegisteredPtyProcessesWithHostScope(): Promise<{ +async function listRegisteredPtyProcessesWithHostScope( + onSshInventoryUnavailable?: (connectionId: string, error: unknown) => void +): Promise<{ processes: PtyProcessInfo[] hostIds: ExecutionHostId[] }> { const providers = registeredPtyProviders() const providerSessions = await Promise.all( - providers.map(({ provider }) => provider.listProcesses()) + providers.map(async ({ provider, connectionId }) => { + try { + const hostId: ExecutionHostId = connectionId + ? toSshExecutionHostId(connectionId) + : LOCAL_EXECUTION_HOST_ID + return { + processes: await provider.listProcesses(), + hostId + } + } catch (error) { + if (!connectionId) { + throw error + } + onSshInventoryUnavailable?.(connectionId, error) + return null + } + }) ) + const respondingSessions = providerSessions.filter((session) => session !== null) return { - processes: providerSessions.flat(), - hostIds: providers.map(({ connectionId }) => - connectionId ? toSshExecutionHostId(connectionId) : LOCAL_EXECUTION_HOST_ID - ) + processes: respondingSessions.flatMap((session) => session.processes), + hostIds: respondingSessions.map((session) => session.hostId) } } @@ -3930,7 +3948,9 @@ export function registerPtyHandlers( return release }, finalizeExit: (event) => { - runtime?.onPtyExit(event.id, event.code, event.ptyIncarnation) + runtime?.onPtyExit(event.id, event.code, event.ptyIncarnation, { + hostExitConfirmed: true + }) finalizePtyExitForRenderer(event) }, pauseProvider: (generation, id) => { @@ -4465,6 +4485,15 @@ export function registerPtyHandlers( } } + const markSshInventoryUnverifiable = (connectionId: string, error: unknown): void => { + const reason = error instanceof Error ? error.message : String(error) + for (const [ptyId, ownerConnectionId] of ptyOwnership) { + if (ownerConnectionId === connectionId) { + runtime?.markPtyLivenessUnverifiable?.(ptyId, reason) + } + } + } + // Why: route through getProviderForPty() so CLI commands work for remote PTYs too; localProvider would silently fail for them. runtime?.setPtyController({ claimStablePaneCreate: (args) => { @@ -5539,12 +5568,15 @@ export function registerPtyHandlers( if (connectionId) { // Why: runtime/CLI close can target a detached SSH PTY after its // provider was unregistered. Tombstone the lease so reconnect does - // not revive a terminal the user explicitly closed. + // not revive a terminal the user explicitly closed — but a detached + // relay PTY outlives its provider, so report an unconfirmed stop + // rather than a kill nobody performed. const incarnationId = finishPtyShutdown(ptyId, connectionId, store) runtime?.onPtyExit(ptyId, -1, incarnationId) rememberSyntheticKillExit(ptyId) sendPtyExitToRenderer({ id: ptyId, code: -1 }) - return true + runtime?.markPtyLivenessUnverifiable?.(ptyId, SSH_PROVIDER_UNREGISTERED_REASON) + return false } return false } @@ -5576,6 +5608,12 @@ export function registerPtyHandlers( // Why: close runtime tails without clearing provider ownership, so // a retry can still target a PTY that survived the failed shutdown. if (!retired) { + if (connectionId) { + runtime?.markPtyLivenessUnverifiable?.( + ptyId, + err instanceof Error ? err.message : String(err) + ) + } runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId)) } }) @@ -5589,6 +5627,12 @@ export function registerPtyHandlers( `[pty] Failed to stop PTY ${ptyId}: ${err instanceof Error ? err.message : String(err)}` ) if (!retiredRejectedPtyIds.has(ptyId)) { + if (connectionId) { + runtime?.markPtyLivenessUnverifiable?.( + ptyId, + err instanceof Error ? err.message : String(err) + ) + } runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId)) } }) @@ -5596,11 +5640,23 @@ export function registerPtyHandlers( } return killWithCurrentProvider() }, - retireRejectedPty: (ptyId) => { + retireRejectedPty: (ptyId, stopConfirmed) => { rememberRetiredRejectedPty(ptyId) + if (!stopConfirmed) { + runtime?.markPtyLivenessUnverifiable?.( + ptyId, + 'a follow-up stop was issued but its outcome could not be verified' + ) + if (!ptyOwnership.has(ptyId)) { + return + } + runtime?.onPtyExit(ptyId, -1, ptyIncarnationById.get(ptyId)) + rememberSyntheticKillExit(ptyId) + sendPtyExitToRenderer({ id: ptyId, code: -1 }) + return + } // Why: a completed stop already cleared provider state, tombstoned the lease and told the - // renderer; repeating that double-fires the exit IPC. The runtime still needs code 0 so an - // SSH pane retires for good instead of staying preserved by the stop's negative exit. + // renderer; repeating that double-fires the exit IPC. if (!ptyOwnership.has(ptyId)) { runtime?.onPtyExit(ptyId, 0, ptyIncarnationById.get(ptyId)) return @@ -5669,13 +5725,14 @@ export function registerPtyHandlers( provider = connectionId ? getProvider(connectionId) : getProviderForPty(ptyId) } catch { if (connectionId) { - // Why: an absent SSH provider means there is no live target left to - // await, but the relay lease must still be tombstoned. + // Why: the relay lease must still be tombstoned, but an absent SSH + // provider is lost contact — the remote PTY is designed to survive it, + // so nothing here observed an exit to report as a confirmed stop. const incarnationId = finishPtyShutdown(ptyId, connectionId, store) runtime?.onPtyExit(ptyId, -1, incarnationId) rememberSyntheticKillExit(ptyId) sendPtyExitToRenderer({ id: ptyId, code: -1 }) - return true + runtime?.markPtyLivenessUnverifiable?.(ptyId, SSH_PROVIDER_UNREGISTERED_REASON) } return false } @@ -5688,6 +5745,12 @@ export function registerPtyHandlers( }) } catch (err) { if (!isPtyAlreadyGoneError(err)) { + if (connectionId) { + runtime?.markPtyLivenessUnverifiable?.( + ptyId, + err instanceof Error ? err.message : String(err) + ) + } console.warn( `[pty] Failed to stop PTY ${ptyId}: ${err instanceof Error ? err.message : String(err)}` ) @@ -5696,9 +5759,16 @@ export function registerPtyHandlers( } try { if (!(await verifyPtyStopped(provider, ptyId, opts))) { + runtime?.markPtyLivenessLive?.(ptyId) return false } } catch (err) { + if (connectionId) { + runtime?.markPtyLivenessUnverifiable?.( + ptyId, + err instanceof Error ? err.message : String(err) + ) + } console.warn( `[pty] Failed to verify PTY ${ptyId} stopped: ${ err instanceof Error ? err.message : String(err) @@ -5708,9 +5778,11 @@ export function registerPtyHandlers( } const incarnationId = finishPtyShutdown(ptyId, connectionId, store) if (!providerExitObserved) { - runtime?.onPtyExit(ptyId, -1, incarnationId) + // The owning provider's fresh inventory observed absence, so this is a + // death certificate even when its exit event was missed. + runtime?.onPtyExit(ptyId, 0, incarnationId) rememberSyntheticKillExit(ptyId) - sendPtyExitToRenderer({ id: ptyId, code: -1 }) + sendPtyExitToRenderer({ id: ptyId, code: 0 }) } return true }, @@ -5767,11 +5839,17 @@ export function registerPtyHandlers( return localProvider.listProcesses() } if (connectionId !== undefined) { - return getProvider(connectionId).listProcesses() + try { + return await getProvider(connectionId).listProcesses() + } catch (error) { + markSshInventoryUnverifiable(connectionId, error) + throw error + } } - return (await listRegisteredPtyProcessesWithHostScope()).processes + return (await listRegisteredPtyProcessesWithHostScope(markSshInventoryUnverifiable)).processes }, - listProcessesWithHostScope: listRegisteredPtyProcessesWithHostScope, + listProcessesWithHostScope: () => + listRegisteredPtyProcessesWithHostScope(markSshInventoryUnverifiable), serializeBuffer: (ptyId, opts) => { // Why: mobile xterm must start from the desktop's exact screen state/dimensions before live TUI chunks render correctly. return requestSerializedBuffer(ptyId, opts) @@ -7604,6 +7682,7 @@ export function registerPtyHandlers( // provider is unregistered; hydrated app-scoped ids can also arrive // before ownership is rebuilt. Tombstone instead of falling back local. const incarnationId = finishPtyShutdown(args.id, connectionId, store) + runtime?.markPtyLivenessUnverifiable?.(args.id, SSH_PROVIDER_UNREGISTERED_REASON) runtime?.onPtyExit(args.id, -1, incarnationId) rememberSyntheticKillExit(args.id) sendPtyExitToRenderer({ id: args.id, code: -1 }) diff --git a/src/main/runtime/claude-agent-teams-service.test.ts b/src/main/runtime/claude-agent-teams-service.test.ts index a41668932f0..57f867cb1c2 100644 --- a/src/main/runtime/claude-agent-teams-service.test.ts +++ b/src/main/runtime/claude-agent-teams-service.test.ts @@ -189,7 +189,7 @@ describe('ClaudeAgentTeamsService', () => { expect(api.closeTerminal).toHaveBeenLastCalledWith('teammate-2') }) - it('keeps the placeholder handle when the respawn split fails', async () => { + it('removes the pane when replacement split fails after a confirmed placeholder stop', async () => { const { service, teamId, token, leaderPane, api } = createServiceWithLeader() const request = (argv: string[], envPane = leaderPane) => service.handleTmuxCompat({ teamId, token, envPane, argv }, api) @@ -212,10 +212,59 @@ describe('ClaudeAgentTeamsService', () => { request(['respawn-pane', '-k', '-t', '%2', '--', 'claude --agent-id a']) ).resolves.toMatchObject({ ok: false, exitCode: 1 }) - // the placeholder terminal is left intact and the fake pane id still resolves. - expect(api.closeTerminal).not.toHaveBeenCalled() - await request(['kill-pane', '-t', '%2']) expect(api.closeTerminal).toHaveBeenCalledWith('teammate-1') + await expect( + request(['list-panes', '-t', 'orca:0', '-F', '#{pane_id}']) + ).resolves.toMatchObject({ stdout: '%1\n' }) + }) + + it('keeps a pane registered when its process stop is unconfirmed', async () => { + const { service, teamId, token, leaderPane, api } = createServiceWithLeader() + const request = (argv: string[], envPane = leaderPane) => + service.handleTmuxCompat({ teamId, token, envPane, argv }, api) + + await request(['split-window', '-t', leaderPane, '-h', '-P', '-F', '#{pane_id}']) + vi.mocked(api.closeTerminal).mockResolvedValueOnce({ + handle: 'teammate-1', + tabId: 'tab-1', + ptyKilled: false + }) + + await expect(request(['kill-pane', '-t', '%2'])).resolves.toMatchObject({ + ok: false, + exitCode: 1 + }) + await expect( + request(['list-panes', '-t', 'orca:0', '-F', '#{pane_id}']) + ).resolves.toMatchObject({ stdout: '%1\n%2\n' }) + }) + + it('does not launch a replacement when the placeholder stop is unconfirmed', async () => { + const { service, teamId, token, leaderPane, api, splitCalls } = createServiceWithLeader() + const request = (argv: string[], envPane = leaderPane) => + service.handleTmuxCompat({ teamId, token, envPane, argv }, api) + + await request(['split-window', '-t', leaderPane, '-h', '-P', '-F', '#{pane_id}', 'cat']) + vi.mocked(api.closeTerminal).mockResolvedValueOnce({ + handle: 'teammate-1', + tabId: 'tab-1', + ptyKilled: false, + ptyStopVerdict: 'live' + }) + + await expect( + request(['respawn-pane', '-k', '-t', '%2', '--', 'claude --agent-id a']) + ).resolves.toMatchObject({ ok: false, exitCode: 1 }) + expect(api.closeTerminal).toHaveBeenNthCalledWith(1, 'teammate-1') + expect(splitCalls).toHaveLength(1) + + await expect( + request(['respawn-pane', '-k', '-t', '%2', '--', 'claude --agent-id a']) + ).resolves.toMatchObject({ ok: false, exitCode: 1 }) + expect(splitCalls).toHaveLength(1) + + await request(['kill-pane', '-t', '%2']) + expect(api.closeTerminal).toHaveBeenLastCalledWith('teammate-1') }) it('refuses to respawn the leader pane', async () => { diff --git a/src/main/runtime/claude-agent-teams-tmux-dispatcher.ts b/src/main/runtime/claude-agent-teams-tmux-dispatcher.ts index 98159e8725e..a9a3b4484a8 100644 --- a/src/main/runtime/claude-agent-teams-tmux-dispatcher.ts +++ b/src/main/runtime/claude-agent-teams-tmux-dispatcher.ts @@ -4,6 +4,7 @@ import { tmuxSendKeysText, tmuxValue } from '../../shared/claude-agent-teams-tmux-compat' +import { describeUnconfirmedAgentStop } from '../../shared/pty-liveness-verdict' import { formatContext, paneEnv, @@ -157,27 +158,31 @@ export class ClaudeAgentTeamsTmuxDispatcher { if (!command) { return '' } + if (pane.respawnBlockedReason) { + throw new Error(pane.respawnBlockedReason) + } const origin = (pane.splitFromPane ? team.panes.get(pane.splitFromPane) : undefined) ?? team.panes.get(team.leaderPane)! - // Why: create the replacement before destroying the placeholder so a failed - // split leaves the fake pane id pointing at a still-live terminal; on cleanup - // failure, discard the new split and keep the placeholder registered. const previousHandle = pane.handle - const split = await api.splitTerminal(origin.handle, { - direction: pane.splitDirection ?? 'horizontal', - command, - env: paneEnv(team, pane.fakePaneId), - envToDelete: ['TERM_PROGRAM'], - activate: false - }) + const close = await api.closeTerminal(previousHandle) + if (!close.ptyKilled) { + pane.respawnBlockedReason = describeUnconfirmedAgentStop(close) + throw new Error(pane.respawnBlockedReason) + } try { - await api.closeTerminal(previousHandle) + const split = await api.splitTerminal(origin.handle, { + direction: pane.splitDirection ?? 'horizontal', + command, + env: paneEnv(team, pane.fakePaneId), + envToDelete: ['TERM_PROGRAM'], + activate: false + }) + pane.handle = split.handle } catch (error) { - await api.closeTerminal(split.handle).catch(() => {}) + this.removePane(team, pane) throw error } - pane.handle = split.handle return '' } @@ -266,14 +271,21 @@ export class ClaudeAgentTeamsTmuxDispatcher { if (pane.fakePaneId === team.leaderPane) { throw new Error('refusing to kill leader pane') } - await api.closeTerminal(pane.handle) + const close = await api.closeTerminal(pane.handle) + if (!close.ptyKilled) { + throw new Error(describeUnconfirmedAgentStop(close)) + } + this.removePane(team, pane) + return '' + } + + private removePane(team: AgentTeam, pane: TeamPane): void { team.panes.delete(pane.fakePaneId) team.paneOrder = team.paneOrder.filter((id) => id !== pane.fakePaneId) if (team.mainVertical?.lastColumnPane === pane.fakePaneId) { team.mainVertical.lastColumnPane = [...team.paneOrder].toReversed().find((id) => id !== team.leaderPane) ?? null } - return '' } private async lastPane( diff --git a/src/main/runtime/claude-agent-teams-types.ts b/src/main/runtime/claude-agent-teams-types.ts index 981c9eee024..10234f5226d 100644 --- a/src/main/runtime/claude-agent-teams-types.ts +++ b/src/main/runtime/claude-agent-teams-types.ts @@ -59,6 +59,7 @@ export type TeamPane = { // respawn can recreate it in the same slot while preserving its fake pane id. splitFromPane?: string splitDirection?: 'horizontal' | 'vertical' + respawnBlockedReason?: string } export type AgentTeam = { diff --git a/src/main/runtime/orca-runtime-process-incarnation-liveness.test.ts b/src/main/runtime/orca-runtime-process-incarnation-liveness.test.ts index 06a5f1cfcd0..3066367d7e8 100644 --- a/src/main/runtime/orca-runtime-process-incarnation-liveness.test.ts +++ b/src/main/runtime/orca-runtime-process-incarnation-liveness.test.ts @@ -35,12 +35,12 @@ describe('terminal process incarnation liveness', () => { ).resolves.toBe('live') await expect( runtime.inspectTerminalProcessIncarnationLiveness('remote:ssh-1:pty-1:inc-old', SSH_SCOPE) - ).resolves.toBe('dead') + ).resolves.toBe('exited') expect(listProcesses).toHaveBeenNthCalledWith(1, 'ssh-1') expect(listProcesses).toHaveBeenNthCalledWith(2, 'ssh-1') }) - it('keeps missing or malformed identity and unavailable inventory unknown', async () => { + it('keeps missing or malformed identity and unavailable inventory unverifiable', async () => { const listProcesses = vi .fn() .mockResolvedValueOnce([{ id: 'remote:ssh-1:pty-1', cwd: '', title: 'worker' }]) @@ -52,13 +52,13 @@ describe('terminal process incarnation liveness', () => { await expect( runtime.inspectTerminalProcessIncarnationLiveness(PROCESS_INCARNATION, SSH_SCOPE) - ).resolves.toBe('unknown') + ).resolves.toBe('unverifiable') await expect( runtime.inspectTerminalProcessIncarnationLiveness(PROCESS_INCARNATION, SSH_SCOPE) - ).resolves.toBe('unknown') + ).resolves.toBe('unverifiable') await expect( runtime.inspectTerminalProcessIncarnationLiveness(PROCESS_INCARNATION, SSH_SCOPE) - ).resolves.toBe('unknown') + ).resolves.toBe('unverifiable') }) it('does not inspect an unproven host scope', async () => { @@ -67,10 +67,10 @@ describe('terminal process incarnation liveness', () => { await expect( runtime.inspectTerminalProcessIncarnationLiveness(PROCESS_INCARNATION, null) - ).resolves.toBe('unknown') + ).resolves.toBe('unverifiable') await expect( runtime.inspectTerminalProcessIncarnationLiveness(PROCESS_INCARNATION, '{"kind":"ssh"}') - ).resolves.toBe('unknown') + ).resolves.toBe('unverifiable') expect(listProcesses).not.toHaveBeenCalled() }) @@ -83,7 +83,7 @@ describe('terminal process incarnation liveness', () => { await expect( runtime.inspectTerminalProcessIncarnationLiveness('local-pty:inc-1', JSON.stringify(scope)) - ).resolves.toBe('dead') + ).resolves.toBe('exited') expect(listProcesses).toHaveBeenCalledWith(connectionId) }) }) diff --git a/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts b/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts index 6d22718dcae..c3a3e747b01 100644 --- a/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts +++ b/src/main/runtime/orca-runtime-terminal-close-continuity.test.ts @@ -311,7 +311,7 @@ describe('terminal close and handle incarnation continuity', () => { harness.retirePersistedTab() harness.acknowledged.resolve() - await expect(closing).resolves.toMatchObject({ handle, tabId: TAB_ID, ptyKilled: true }) + await expect(closing).resolves.toMatchObject({ handle, tabId: TAB_ID, ptyKilled: false }) expect(harness.kill).toHaveBeenCalledWith(PTY_ID) expect(harness.closeTerminal).not.toHaveBeenCalled() expect(harness.getSession().tabsByWorktree[WORKTREE_ID]).toEqual([]) @@ -328,7 +328,7 @@ describe('terminal close and handle incarnation continuity', () => { expect(harness.getSession().tabsByWorktree[WORKTREE_ID]).toHaveLength(1) }) - it('kills every live tab PTY after retirement when the renderer graph is stale', async () => { + it('requests a stop for every live tab PTY after retirement when the renderer graph is stale', async () => { const harness = createHarness() harness.syncSplitFixtureGraph() const terminal = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals.find( @@ -342,7 +342,7 @@ describe('terminal close and handle incarnation continuity', () => { harness.retirePersistedTab() harness.acknowledged.resolve() - await expect(closing).resolves.toMatchObject({ ptyKilled: true }) + await expect(closing).resolves.toMatchObject({ ptyKilled: false }) expect(harness.kill).toHaveBeenCalledWith(PTY_ID) expect(harness.kill).toHaveBeenCalledWith(SIBLING_PTY_ID) }) @@ -365,7 +365,64 @@ describe('terminal close and handle incarnation continuity', () => { expect(harness.kill).not.toHaveBeenCalled() }) - it('falls back to kill when verified teardown rejects after retirement', async () => { + it('reports an unconfirmed stop on the close receipt rather than a bare uncertain false', async () => { + const harness = createHarness() + const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals + // Mirrors pty.ts when the SSH provider is gone: the lease is tombstoned, the + // stop reports false, and the PTY is marked as contact we lost — not a kill. + harness.kill.mockReturnValue(false) + harness.setVerifiedStopResult(false) + harness.runtime.markPtyLivenessUnverifiable(PTY_ID, 'its SSH provider is no longer registered') + + const closing = harness.runtime.closeTerminal(handle) + await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalled()) + harness.retirePersistedTab() + harness.acknowledged.resolve() + + await expect(closing).resolves.toMatchObject({ + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH provider is no longer registered' + }) + expect(harness.kill).not.toHaveBeenCalled() + }) + + it('downgrades a live verdict after issuing an unverified follow-up stop', async () => { + const harness = createHarness() + const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals + harness.setVerifiedStopResult(false) + harness.runtime.markPtyLivenessLive(PTY_ID) + + const closing = harness.runtime.closeTerminal(handle) + await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalled()) + harness.retirePersistedTab() + harness.acknowledged.resolve() + + await expect(closing).resolves.toMatchObject({ + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'a follow-up stop was issued but its outcome could not be verified' + }) + expect(harness.kill).toHaveBeenCalledWith(PTY_ID) + }) + + it('leaves a confirmed kill receipt free of any stop verdict', async () => { + const harness = createHarness() + const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals + harness.setVerifiedStopResult(true) + + const closing = harness.runtime.closeTerminal(handle) + await vi.waitFor(() => expect(harness.closeTerminalTab).toHaveBeenCalled()) + harness.retirePersistedTab() + harness.acknowledged.resolve() + + const close = await closing + expect(close.ptyKilled).toBe(true) + expect(close.ptyStopVerdict).toBeUndefined() + expect(close.ptyStopReason).toBeUndefined() + }) + + it('reports an unconfirmed stop when verified teardown rejects after retirement', async () => { const harness = createHarness() const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals harness.setVerifiedStopResult(new Error('provider_unavailable')) @@ -375,7 +432,11 @@ describe('terminal close and handle incarnation continuity', () => { harness.retirePersistedTab() harness.acknowledged.resolve() - await expect(closing).resolves.toMatchObject({ ptyKilled: true }) + await expect(closing).resolves.toMatchObject({ + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'provider_unavailable' + }) expect(harness.stopAndWait).toHaveBeenCalledWith(PTY_ID, { deadlineMs: expect.any(Number) }) @@ -398,7 +459,7 @@ describe('terminal close and handle incarnation continuity', () => { harness.makeSessionUnavailable() harness.acknowledged.resolve() - await expect(closing).resolves.toMatchObject({ handle, tabId: TAB_ID, ptyKilled: true }) + await expect(closing).resolves.toMatchObject({ handle, tabId: TAB_ID, ptyKilled: false }) expect(harness.closeTerminal).toHaveBeenCalledWith(TAB_ID) expect(harness.stopAndWait).toHaveBeenCalledWith(RUNTIME_OWNED_PTY_ID, { deadlineMs: expect.any(Number) diff --git a/src/main/runtime/orca-runtime-terminal-split-authority.test.ts b/src/main/runtime/orca-runtime-terminal-split-authority.test.ts index ca79f9e55a8..a5adbedd269 100644 --- a/src/main/runtime/orca-runtime-terminal-split-authority.test.ts +++ b/src/main/runtime/orca-runtime-terminal-split-authority.test.ts @@ -348,7 +348,7 @@ describe('remote runtime terminal split authority', () => { expect.objectContaining({ deadlineMs: expect.any(Number) }) ) expect(harness.kill).not.toHaveBeenCalled() - expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID) + expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID, true) }) it('revalidates a projected paired-runtime source after renderer adoption', async () => { @@ -374,7 +374,7 @@ describe('remote runtime terminal split authority', () => { expect.objectContaining({ deadlineMs: expect.any(Number) }) ) expect(harness.kill).toHaveBeenCalledWith(SPLIT_PTY_ID) - expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID) + expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID, false) }) it('preserves the split error when kill and retirement throw', async () => { @@ -398,6 +398,6 @@ describe('remote runtime terminal split authority', () => { await expect(split).rejects.toThrow('terminal_split_source_not_found') expect(harness.kill).toHaveBeenCalledWith(SPLIT_PTY_ID) - expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID) + expect(harness.retireRejectedPty).toHaveBeenCalledWith(SPLIT_PTY_ID, false) }) }) diff --git a/src/main/runtime/orca-runtime.ts b/src/main/runtime/orca-runtime.ts index fb490109950..f5af4fb0786 100644 --- a/src/main/runtime/orca-runtime.ts +++ b/src/main/runtime/orca-runtime.ts @@ -664,6 +664,12 @@ import { type RetiredTerminalSurface } from './mobile-session-terminal-retirement' import { retireTerminalSurfaceFromPersistence } from './mobile-session-terminal-persistence-retirement' +import { + NO_OBSERVING_PROVIDER_REASON, + SSH_EXIT_UNCONFIRMED_REASON, + SSH_PROVIDER_UNREGISTERED_REASON, + type PtyLivenessVerdict +} from '../../shared/pty-liveness-verdict' import { advanceTerminalTopologyRevision, hasHostAuthoritativeTerminalMembership @@ -1568,6 +1574,14 @@ function isAgentSessionOperationOutcomeUnknown(error: unknown): boolean { ) } +// Orphaned verdicts are bounded; active PTYs retain theirs until new evidence resolves them. +const MAX_TRACKED_PTY_LIVENESS_VERDICTS = 256 + +type TrackedPtyLivenessVerdict = { + verdict: PtyLivenessVerdict + observedAt: number +} + const AGENT_SESSION_OPERATION_PER_CLIENT_LIMIT = 512 const AGENT_SESSION_OPERATION_GLOBAL_LIMIT = 4_096 @@ -1865,7 +1879,7 @@ type RuntimePtyController = { * False on doubt (absent session, SSH-scoped id, non-daemon provider). */ attach?(ptyId: string): Promise kill(ptyId: string): boolean - retireRejectedPty?(ptyId: string): void + retireRejectedPty?(ptyId: string, stopConfirmed: boolean): void stopAndWait?( ptyId: string, opts?: { keepHistory?: boolean; deadlineMs?: number } @@ -3206,6 +3220,12 @@ export class OrcaRuntimeService { // iterates them all. Listeners are cleaned up via subscriptionCleanups. private notificationListeners = new Set<(event: MobileNotificationEvent) => void>() private ptysById = new Map() + // Why a separate map: `connected` is a wire field that any inventory gap + // clears, so it cannot distinguish an observed exit from lost contact. This + // records the last liveness verdict we actually earned, and outlives the pty + // record so a close/stop receipt can still say the stop was unconfirmed. + private ptyLivenessVerdictByPtyId = new Map() + private ptyLivenessObservationSequence = 0 private readonly pairedRendererSessionOwnedPtyIds = new Set() private wslDistroByPtyId = new Map() private titleObservationSequence = 0 @@ -10579,6 +10599,7 @@ export class OrcaRuntimeService { incarnationId?: PtyIncarnationId, options: { awaitsRegistration?: boolean } = {} ): void { + this.forgetPtyLivenessVerdict(ptyId) if (options.awaitsRegistration !== false) { // Why: surface absence cannot distinguish an in-flight admission from a completed headless lifecycle. this.pendingPtyRegistrationIncarnations.set(ptyId, incarnationId ?? null) @@ -10612,6 +10633,7 @@ export class OrcaRuntimeService { isWsl?: boolean ): void { this.assertPtyDidNotExitBeforeRegistration(ptyId, binding?.incarnationId) + this.forgetPtyLivenessVerdict(ptyId) this.spawnPublishedPtys.add(ptyId) // Why: record the renderer pane identity at spawn time so a stalled graph // sync can't hide that a live PTY already backs a pending mobile create. @@ -14906,14 +14928,25 @@ export class OrcaRuntimeService { } } - onPtyExit(ptyId: string, exitCode: number, exitIncarnationId?: PtyIncarnationId): void { + onPtyExit( + ptyId: string, + exitCode: number, + exitIncarnationId?: PtyIncarnationId, + options?: { hostExitConfirmed?: boolean } + ): void { const pty = this.ptysById.get(ptyId) if (exitIncarnationId && pty?.incarnationId && exitIncarnationId !== pty.incarnationId) { return } const preservesAbnormalSshSurface = - this.isSshOwnedPtyId(ptyId) && pty?.connectionId != null && exitCode < 0 + this.isSshOwnedPtyId(ptyId) && + pty?.connectionId != null && + exitCode < 0 && + options?.hostExitConfirmed !== true if (preservesAbnormalSshSurface) { + if (this.getPtyLivenessVerdict(ptyId)?.status !== 'unverifiable') { + this.markPtyLivenessUnverifiable(ptyId, SSH_EXIT_UNCONFIRMED_REASON) + } this.restoredOrchestrationAuthorityByPtyId.delete(ptyId) } else { this.retirePtyAgentLaunchAuthority(ptyId) @@ -15047,6 +15080,11 @@ export class OrcaRuntimeService { this.setPairedRendererSessionOwnership(pty.ptyId, false) pty.disconnectedAt = Date.now() pty.lastExitCode = exitCode + if (exitCode >= 0 || options?.hostExitConfirmed === true) { + // A real wait status from the owning host is the death certificate; the + // synthetic -1 we emit on a failed/unroutable stop is not. + this.forgetPtyLivenessVerdict(ptyId) + } // Why: the exited process's live frames say nothing about a replacement. // A same-id respawn makes the leaf writable again before any new title, // so leaving this true would let push delivery type into the new process @@ -16929,17 +16967,17 @@ export class OrcaRuntimeService { async inspectTerminalProcessIncarnationLiveness( processIncarnation: string, serializedHostScope: string | null - ): Promise<'live' | 'dead' | 'unknown'> { + ): Promise<'live' | 'exited' | 'unverifiable'> { const hostScope = parseWorkerTerminalHostScope(serializedHostScope) if (!hostScope || !this.ptyController?.listProcesses) { - return 'unknown' + return 'unverifiable' } const listed = await withTimeoutResult( this.ptyController.listProcesses(hostScope.kind === 'ssh' ? hostScope.targetId : null), PTY_CONTROLLER_LIST_TIMEOUT_MS ) if (!listed.ok) { - return 'unknown' + return 'unverifiable' } return classifyWorkerTerminalProcessIncarnation(processIncarnation, listed.value) } @@ -17787,6 +17825,68 @@ export class OrcaRuntimeService { return `${this.runtimeId}:${record.ptyId}:${record.ptyGeneration}` } + /** + * Records that we lost contact with a PTY's owning host. Callers must never + * read this as an exit: a detached relay PTY is designed to outlive the + * provider that addressed it. + */ + markPtyLivenessUnverifiable(ptyId: string, reason: string): void { + this.rememberPtyLivenessVerdict(ptyId, { status: 'unverifiable', reason }) + } + + markPtyLivenessLive(ptyId: string): void { + this.rememberPtyLivenessVerdict(ptyId, { status: 'live', ptyIds: [ptyId] }) + } + + /** Null when nothing has been observed either way, so callers keep their own default. */ + getPtyLivenessVerdict(ptyId: string): PtyLivenessVerdict | null { + return this.ptyLivenessVerdictByPtyId.get(ptyId)?.verdict ?? null + } + + getTerminalLivenessVerdict(handle: string): PtyLivenessVerdict | null { + const record = this.getLivePtyForHandle(handle)?.record ?? this.handles.get(handle) + return record?.ptyId ? this.getPtyLivenessVerdict(record.ptyId) : null + } + + private rememberPtyLivenessVerdict(ptyId: string, verdict: PtyLivenessVerdict): void { + if (verdict.status === 'exited') { + // An earned death certificate ends the question; nothing left to remember. + this.ptyLivenessVerdictByPtyId.delete(ptyId) + return + } + this.ptyLivenessVerdictByPtyId.delete(ptyId) + this.ptyLivenessObservationSequence += 1 + this.ptyLivenessVerdictByPtyId.set(ptyId, { + verdict, + observedAt: this.ptyLivenessObservationSequence + }) + while (this.ptyLivenessVerdictByPtyId.size > MAX_TRACKED_PTY_LIVENESS_VERDICTS) { + let oldestOrphaned: string | null = null + for (const candidate of this.ptyLivenessVerdictByPtyId.keys()) { + if ( + !this.ptysById.has(candidate) && + !this.handleByPtyId.has(candidate) && + !this.leafExistsForPty(candidate) + ) { + oldestOrphaned = candidate + break + } + } + if (!oldestOrphaned) { + return + } + this.ptyLivenessVerdictByPtyId.delete(oldestOrphaned) + } + } + + private forgetPtyLivenessVerdict(ptyId: string, observedNoLaterThan?: number): void { + const tracked = this.ptyLivenessVerdictByPtyId.get(ptyId) + if (observedNoLaterThan !== undefined && tracked && tracked.observedAt > observedNoLaterThan) { + return + } + this.ptyLivenessVerdictByPtyId.delete(ptyId) + } + getExactWorkerProviderSession( handle: string, observedAfter: number @@ -28845,13 +28945,34 @@ export class OrcaRuntimeService { let addressedPtyStopped = false const deadlineMs = Date.now() + EXPLICIT_TERMINAL_CLOSE_STOP_TIMEOUT_MS for (const ptyId of ptyIds) { - let verifiedStopped = false - try { - verifiedStopped = (await this.ptyController?.stopAndWait?.(ptyId, { deadlineMs })) ?? false - } catch { - // Why: verified teardown is preferred, but its transport failure must not suppress the legacy provider kill. + let stopped = false + if (this.ptyController?.stopAndWait) { + try { + stopped = await this.ptyController.stopAndWait(ptyId, { deadlineMs }) + } catch (error) { + this.markPtyLivenessUnverifiable( + ptyId, + error instanceof Error ? error.message : String(error) + ) + } + if (!stopped) { + const verdict = this.getPtyLivenessVerdict(ptyId) + const providerAlreadyRetiredPty = + verdict?.status === 'unverifiable' && + verdict.reason === SSH_PROVIDER_UNREGISTERED_REASON + if (!providerAlreadyRetiredPty) { + this.ptyController.kill(ptyId) + if (!verdict || verdict.status === 'live') { + this.markPtyLivenessUnverifiable( + ptyId, + 'a follow-up stop was issued but its outcome could not be verified' + ) + } + } + } + } else { + stopped = this.ptyController?.kill(ptyId) ?? false } - const stopped = verifiedStopped || (this.ptyController?.kill(ptyId) ?? false) if (ptyId === addressedPtyId) { addressedPtyStopped = stopped } @@ -29053,15 +29174,15 @@ export class OrcaRuntimeService { this.notifier.closeTerminal?.(tabId) } const ptyKilled = await this.stopExplicitlyClosedTabPtys(ptyIdsToKill, pty.pty.ptyId) - return { handle, tabId, ptyKilled } + return this.describeTerminalClose(handle, tabId, pty.pty.ptyId, ptyKilled) } if (siblingCount <= 1 && !surface && pty.pty.tabId && this.notifier?.closeTerminalTab) { const ptyIdsToKill = this.getPtyIdsForExplicitTabClose(pty.pty.worktreeId, tabId) await this.notifier.closeTerminalTab(tabId, { localPtyTeardownOwnedExternally: true }) const ptyKilled = await this.stopExplicitlyClosedTabPtys(ptyIdsToKill, pty.pty.ptyId) - return { handle, tabId, ptyKilled } + return this.describeTerminalClose(handle, tabId, pty.pty.ptyId, ptyKilled) } - const ptyKilled = this.ptyController?.kill(pty.pty.ptyId) ?? false + const ptyKilled = await this.stopExplicitlyClosedTabPtys([pty.pty.ptyId], pty.pty.ptyId) if (!ptyKilled || siblingCount <= 1) { if (surface) { // Why: paired viewers keep ended streams mounted until the HUB publishes removal, so explicit close uses the durable host-tab transaction instead of viewer-local exit handling. @@ -29077,7 +29198,7 @@ export class OrcaRuntimeService { this.notifier?.closeTerminal(tabId) } } - return { handle, tabId, ptyKilled } + return this.describeTerminalClose(handle, tabId, pty.pty.ptyId, ptyKilled) } this.assertGraphReady() const { leaf } = this.getLiveLeafForHandle(handle) @@ -29100,7 +29221,36 @@ export class OrcaRuntimeService { if (siblingCount > 1 ? !ptyKilled : !this.notifier?.closeTerminalTab) { this.notifier?.closeTerminal(leaf.tabId, leaf.paneRuntimeId) } - return { handle, tabId: leaf.tabId, ptyKilled } + return this.describeTerminalClose(handle, leaf.tabId, leaf.ptyId ?? null, ptyKilled) + } + + /** + * A close receipt must not read as a kill nobody performed: when the stop was + * not confirmed, the receipt carries why so the CLI and callers can say so. + */ + private describeTerminalClose( + handle: string, + tabId: string, + ptyId: string | null, + ptyKilled: boolean + ): RuntimeTerminalClose { + if (ptyKilled || !ptyId) { + return { handle, tabId, ptyKilled } + } + const verdict = this.getPtyLivenessVerdict(ptyId) + if (verdict?.status === 'unverifiable') { + return { + handle, + tabId, + ptyKilled, + ptyStopVerdict: 'unverifiable', + ptyStopReason: verdict.reason + } + } + if (verdict?.status === 'live') { + return { handle, tabId, ptyKilled, ptyStopVerdict: 'live' } + } + return { handle, tabId, ptyKilled } } async closeTerminalTab(handle: string): Promise { @@ -29331,7 +29481,7 @@ export class OrcaRuntimeService { } } try { - this.ptyController.retireRejectedPty?.(result.id) + this.ptyController.retireRejectedPty?.(result.id, stopped) } catch { // Best-effort cleanup; preserve the original split authority error. } @@ -31651,6 +31801,7 @@ export class OrcaRuntimeService { } const inventoryGeneration = this.ptyControllerInventorySequence + 1 this.ptyControllerInventorySequence = inventoryGeneration + const livenessObservationAtStart = this.ptyLivenessObservationSequence const providerKey = typeof connectionId === 'string' ? `ssh:${connectionId}` : 'local' if (connectionId === undefined) { this.ptyControllerAggregateInventoryGeneration = inventoryGeneration @@ -31760,6 +31911,8 @@ export class OrcaRuntimeService { const allLivePtyIds = new Set(sessions.map((session) => session.id)) const selectedLivePtyIds = new Set() for (const session of sessions) { + // The owning inventory positively observed this PTY again; prior lost-contact doubt is stale. + this.forgetPtyLivenessVerdict(session.id, livenessObservationAtStart) const sessionConnectionId = parseAppSshPtyId(session.id)?.connectionId ?? (typeof connectionId === 'string' ? connectionId : null) @@ -31862,7 +32015,18 @@ export class OrcaRuntimeService { continue } if (!allLivePtyIds.has(pty.ptyId) && !this.leafExistsForPty(pty.ptyId)) { - if (this.ptyController.hasPty?.(pty.ptyId) === true) { + const currentVerdict = this.ptyLivenessVerdictByPtyId.get(pty.ptyId) + if ( + currentVerdict && + currentVerdict.observedAt > livenessObservationAtStart && + currentVerdict.verdict.status === 'unverifiable' + ) { + pty.connected = false + pty.disconnectedAt ??= Date.now() + continue + } + const observed = this.ptyController.hasPty?.(pty.ptyId) + if (observed === true) { // Why: an SSH spawn can become addressable before an overlapping relay list includes it. allLivePtyIds.add(pty.ptyId) if ( @@ -31873,10 +32037,19 @@ export class OrcaRuntimeService { } pty.connected = true pty.disconnectedAt = null + this.forgetPtyLivenessVerdict(pty.ptyId) continue } pty.connected = false pty.disconnectedAt ??= Date.now() + // Why: this list only enumerates registered providers, so a dropped relay + // clears `connected` for every one of its PTYs at once. Only `false` here + // is an observed absence; `null` means no provider could be asked. + if (observed === false) { + this.forgetPtyLivenessVerdict(pty.ptyId) + } else if (observed === null) { + this.markPtyLivenessUnverifiable(pty.ptyId, NO_OBSERVING_PROVIDER_REASON) + } } } this.pruneDisconnectedPtyRecords() @@ -31958,6 +32131,7 @@ export class OrcaRuntimeService { if (pty) { pty.connected = true pty.disconnectedAt = null + this.forgetPtyLivenessVerdict(ptyId) this.refreshPtyForegroundAgent(ptyId) } } else if (pty && !this.leafExistsForPty(ptyId)) { diff --git a/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-release.ts b/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-release.ts index 01a20948a8d..03035aa2eec 100644 --- a/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-release.ts +++ b/src/main/runtime/orchestration/db/worker-terminal/worker-terminal-release.ts @@ -64,10 +64,7 @@ export function requestWorkerTerminalRelease( this.db.exec('COMMIT') return { disposition: 'retained', resource, reason: 'ownership_transferred' } } - if ( - resource.release_state === 'unknown' || - (resource.release_state === 'retained' && resource.retained_reason === 'user_requested') - ) { + if (resource.release_state === 'retained' && resource.retained_reason === 'user_requested') { this.db.prepare('DELETE FROM worker_terminal_archives WHERE dispatch_id = ?').run(dispatchId) } this.db diff --git a/src/main/runtime/orchestration/worker-terminal-process-liveness.ts b/src/main/runtime/orchestration/worker-terminal-process-liveness.ts index 5a6e92c7878..67277644bfc 100644 --- a/src/main/runtime/orchestration/worker-terminal-process-liveness.ts +++ b/src/main/runtime/orchestration/worker-terminal-process-liveness.ts @@ -39,7 +39,7 @@ export function parseWorkerTerminalHostScope(value: string | null): WorkerTermin export function classifyWorkerTerminalProcessIncarnation( processIncarnation: string, sessions: readonly PtyProcessInfo[] -): 'live' | 'dead' | 'unknown' { +): 'live' | 'exited' | 'unverifiable' { const possibleMatches = sessions.filter((session) => processIncarnation.startsWith(`${session.id}:`) ) @@ -57,6 +57,6 @@ export function classifyWorkerTerminalProcessIncarnation( return possibleMatches.some( (session) => !session.incarnationId || session.incarnationId !== session.incarnationId.trim() ) - ? 'unknown' - : 'dead' + ? 'unverifiable' + : 'exited' } diff --git a/src/main/runtime/pty-inventory-liveness-verdict.test.ts b/src/main/runtime/pty-inventory-liveness-verdict.test.ts new file mode 100644 index 00000000000..38e3dda20eb --- /dev/null +++ b/src/main/runtime/pty-inventory-liveness-verdict.test.ts @@ -0,0 +1,194 @@ +import { describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from './orca-runtime' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import { SSH_EXIT_UNCONFIRMED_REASON } from '../../shared/pty-liveness-verdict' + +// The aggregate inventory only enumerates registered providers, so a dropped +// relay clears `connected` for every one of its PTYs at once. Only the +// provider's own answer separates an observed exit from lost contact. + +const WORKTREE_ID = 'repo-1::/tmp/inventory-verdict' +const REMOTE_PTY_ID = 'ssh:conn-1@@relay-9' + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((settle) => { + resolve = settle + }) + return { promise, resolve } +} + +function makeStore() { + const session = getDefaultWorkspaceSession() + return { + getWorkspaceSession: vi.fn(() => session), + setWorkspaceSession: vi.fn(), + getRepos: vi.fn(() => [ + { + id: 'repo-1', + path: '/tmp/inventory-verdict', + displayName: 'inventory-verdict', + badgeColor: '#000000', + addedAt: 0 + } + ]), + getAllWorktreeMeta: vi.fn(() => ({})), + getWorktreeMeta: vi.fn(() => undefined), + setWorktreeMeta: vi.fn(), + removeWorktreeMeta: vi.fn(), + getSettings: vi.fn(() => ({ workspaceDir: '/tmp/workspaces' })), + getProjects: vi.fn(() => []) + } +} + +function makeRuntimeMissingFromInventory( + hasPty: () => boolean | null, + listProcesses: () => Promise<{ id: string; worktreeId: string }[]> = vi.fn(async () => []) +): OrcaRuntimeService { + const runtime = new OrcaRuntimeService(makeStore() as never) + runtime.setPtyController({ + write: () => true, + kill: () => true, + hasPty, + listProcesses, + getForegroundProcess: async () => null + } as never) + runtime.attachWindow(1) + runtime.syncWindowGraph(1, { tabs: [], leaves: [] }) + runtime.registerPty(REMOTE_PTY_ID, WORKTREE_ID, 'conn-1') + return runtime +} + +describe('inventory sweep liveness verdicts', () => { + it('records an abnormal SSH exit as unverifiable at the runtime boundary', () => { + const runtime = makeRuntimeMissingFromInventory(() => null) + + runtime.onPtyExit(REMOTE_PTY_ID, -1) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ + status: 'unverifiable', + reason: SSH_EXIT_UNCONFIRMED_REASON + }) + }) + + it('preserves a more specific lost-contact reason across an abnormal SSH exit', () => { + const runtime = makeRuntimeMissingFromInventory(() => null) + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'inventory transport failed') + + runtime.onPtyExit(REMOTE_PTY_ID, -1) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ + status: 'unverifiable', + reason: 'inventory transport failed' + }) + }) + + it('accepts a current owning-host exit even when its numeric code is negative', () => { + const runtime = makeRuntimeMissingFromInventory(() => null) + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'inventory transport failed') + + runtime.onPtyExit(REMOTE_PTY_ID, -1, undefined, { hostExitConfirmed: true }) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + }) + + it('records lost contact when no provider can answer for the PTY', async () => { + const runtime = makeRuntimeMissingFromInventory(() => null) + + await runtime.listTerminals(`id:${WORKTREE_ID}`) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ + status: 'unverifiable', + reason: 'no registered provider can observe its host' + }) + }) + + it('records no doubt when the owning provider reports the PTY absent', async () => { + const runtime = makeRuntimeMissingFromInventory(() => false) + + await runtime.listTerminals(`id:${WORKTREE_ID}`) + + // An observed absence is the death certificate callers already act on. + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + }) + + it('clears lost-contact doubt when reconnect inventory observes the PTY live', async () => { + let reconnected = false + const runtime = makeRuntimeMissingFromInventory( + () => null, + vi.fn(async () => (reconnected ? [{ id: REMOTE_PTY_ID, worktreeId: WORKTREE_ID }] : [])) + ) + + await runtime.listTerminals(`id:${WORKTREE_ID}`) + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)?.status).toBe('unverifiable') + + reconnected = true + await runtime.listTerminals(`id:${WORKTREE_ID}`) + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + }) + + it('does not let a pre-drop inventory clear a newer lost-contact verdict', async () => { + const inventory = deferred<{ id: string; worktreeId: string }[]>() + const listProcesses = vi.fn(() => inventory.promise) + const runtime = makeRuntimeMissingFromInventory(() => null, listProcesses) + + const listing = runtime.listTerminals(`id:${WORKTREE_ID}`) + await vi.waitFor(() => expect(listProcesses).toHaveBeenCalled()) + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'relay disconnected during stop') + inventory.resolve([{ id: REMOTE_PTY_ID, worktreeId: WORKTREE_ID }]) + await listing + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ + status: 'unverifiable', + reason: 'relay disconnected during stop' + }) + }) + + it('does not let a partial inventory overwrite a concurrent provider failure', async () => { + const inventory = deferred<{ id: string; worktreeId: string }[]>() + const listProcesses = vi.fn(() => inventory.promise) + const runtime = makeRuntimeMissingFromInventory(() => false, listProcesses) + + const listing = runtime.listTerminals(`id:${WORKTREE_ID}`) + await vi.waitFor(() => expect(listProcesses).toHaveBeenCalled()) + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'inventory transport failed') + inventory.resolve([]) + await listing + + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toEqual({ + status: 'unverifiable', + reason: 'inventory transport failed' + }) + }) + + it('clears stale doubt when a new PTY lifecycle is positively registered', () => { + const runtime = makeRuntimeMissingFromInventory(() => null) + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'old incarnation lost contact') + + runtime.onPtySpawned(REMOTE_PTY_ID, 'incarnation-2') + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + + runtime.markPtyLivenessUnverifiable(REMOTE_PTY_ID, 'registration raced reconnect') + runtime.registerPty(REMOTE_PTY_ID, WORKTREE_ID, 'conn-1', { + tabId: 'tab-new', + leafId: '00000000-0000-4000-8000-000000000001', + incarnationId: 'incarnation-2' + }) + expect(runtime.getPtyLivenessVerdict(REMOTE_PTY_ID)).toBeNull() + }) + + it('retains unresolved verdicts for every still-addressable PTY', () => { + const runtime = new OrcaRuntimeService(makeStore() as never) + for (let index = 0; index < 257; index += 1) { + const ptyId = `ssh:conn-1@@relay-${index}` + runtime.registerPty(ptyId, WORKTREE_ID, 'conn-1') + runtime.markPtyLivenessUnverifiable(ptyId, 'provider disconnected') + } + + expect(runtime.getPtyLivenessVerdict('ssh:conn-1@@relay-0')).toEqual({ + status: 'unverifiable', + reason: 'provider disconnected' + }) + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-federation-control.ts b/src/main/runtime/rpc/methods/orchestration-federation-control.ts index 215a45c5676..eb8a29bce74 100644 --- a/src/main/runtime/rpc/methods/orchestration-federation-control.ts +++ b/src/main/runtime/rpc/methods/orchestration-federation-control.ts @@ -6,6 +6,7 @@ import type { RemoteDispatchAttachmentRow } from '../../orchestration/types' import { defineMethod, type RpcMethod } from '../core' import { OptionalFiniteNumber, requiredString } from '../schemas' import { readExactWorkerOutput } from './orchestration-worker-output' +import { describeUnconfirmedAgentStop } from '../../../../shared/pty-liveness-verdict' const FederationDispatchParams = z.object({ dispatchId: requiredString('Missing Dispatch ID') @@ -36,7 +37,11 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [ runtimeEpoch: runtime.getRuntimeId(), attachment: exposeRemoteAttachment(attachment), terminal: observation.exact ? observation.terminal : null, - observation: { status: observation.status, exactWorker: observation.exact } + observation: { + status: observation.status, + exactWorker: observation.exact, + ...(observation.reason ? { reason: observation.reason } : {}) + } } } }), @@ -46,7 +51,10 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [ handler: async (params, { runtime, authenticatedCallerFingerprint }) => { requireHomeAttachment(runtime, params.dispatchId, authenticatedCallerFingerprint) const observation = await inspectRemoteAttachment(runtime, params.dispatchId) - if (!observation.exact || !observation.terminal || observation.status !== 'running') { + // Why `=== 'exited'` rather than `!== 'live'`: the other non-live + // statuses are already covered by the two guards, and an unverifiable + // terminal is still readable — losing stop-contact is not an exit. + if (!observation.exact || !observation.terminal || observation.status === 'exited') { throw new OrchestrationError( 'worker_identity_changed', `Remote Dispatch ${params.dispatchId} no longer resolves to its exact process.` @@ -83,7 +91,18 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [ dispatchId: params.dispatchId, terminalHandle: observation.terminal.handle, workerState: attachment.state, - terminalStatus: observation.status === 'exited' ? 'exited' : 'running', + terminalStatus: + observation.status === 'exited' + ? 'exited' + : observation.status === 'unverifiable' + ? 'unknown' + : 'running', + terminalLiveness: + observation.status === 'unverifiable' + ? 'unverifiable' + : observation.status === 'exited' + ? 'exited' + : 'live', attachedAt: attachment.created_at, source: params.source, cursor: params.cursor, @@ -134,6 +153,22 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [ } try { const close = await runtime.closeTerminal(observation.terminal.handle) + if (!close.ptyKilled) { + // The tab is retired but the process was never confirmed stopped, so + // the coordinator must not be told this dispatch reached 'stopped'. + const attachment = db.markRemoteAttachmentStopUnknown( + params.dispatchId, + describeUnconfirmedAgentStop(close) + ) + return { + dispatchId: params.dispatchId, + state: attachment.state, + alreadySettled: false, + processAction: 'closed_agent_terminal', + lastError: attachment.last_error, + close + } + } const attachment = db.settleRemoteAttachmentStop(params.dispatchId) return { dispatchId: params.dispatchId, @@ -172,29 +207,44 @@ function requireHomeAttachment( return attachment } -async function inspectRemoteAttachment(runtime: OrcaRuntimeService, dispatchId: string) { +async function inspectRemoteAttachment( + runtime: OrcaRuntimeService, + dispatchId: string +): Promise<{ + terminal: Awaited> | null + exact: boolean + status: 'unattached' | 'missing' | 'identity_changed' | 'live' | 'exited' | 'unverifiable' + /** Set with `unverifiable`; names what we lost contact with. */ + reason?: string +}> { const db = runtime.getOrchestrationDb() const attachment = db.getRemoteDispatchAttachment(dispatchId) if (!attachment?.terminal_handle) { - return { terminal: null, exact: false, status: 'unattached' as const } + return { terminal: null, exact: false, status: 'unattached' } } const terminal = await runtime.showTerminal(attachment.terminal_handle).catch(() => null) if (!terminal) { - return { terminal: null, exact: false, status: 'missing' as const } + return { terminal: null, exact: false, status: 'missing' } } const exact = db.isRemoteAttachmentProcessCurrent({ dispatchId, paneKey: runtime.getTerminalPaneKey(attachment.terminal_handle), processIncarnation: runtime.getTerminalProcessIncarnation(attachment.terminal_handle) }) + if (!exact) { + return { terminal, exact, status: 'identity_changed' } + } + // Why: the same rule as the local worker observation — the inventory only + // iterates registered providers, so a dropped relay clears `connected` for + // every remote PTY at once. Lost contact is not a death certificate. + const verdict = runtime.getTerminalLivenessVerdict?.(attachment.terminal_handle) ?? null + if (verdict?.status === 'unverifiable') { + return { terminal, exact, status: 'unverifiable', reason: verdict.reason } + } return { terminal, exact, - status: exact - ? terminal.connected === false - ? ('exited' as const) - : ('running' as const) - : ('identity_changed' as const) + status: verdict?.status !== 'live' && terminal.connected === false ? 'exited' : 'live' } } diff --git a/src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.ts b/src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.ts new file mode 100644 index 00000000000..9f8436f96ef --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration-federation-liveness-verdict.test.ts @@ -0,0 +1,183 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { ORCHESTRATION_CONTRACT_VERSION } from '../../../../shared/protocol-version' +import { OrcaRuntimeService } from '../../orca-runtime' +import { OrchestrationDb } from '../../orchestration/db' +import { ORCHESTRATION_METHODS } from './orchestration' + +// The federation host runs its own copy of the observation and stop logic, so +// it needs the same rule: lost contact with a worker's host is not an exit, and +// a close it could not confirm must not be relayed home as a settled stop. + +const HOME_FINGERPRINT = 'home-peer-fingerprint' +const DISPATCH_ID = 'ctx_federation_verdict' +const HANDLE = 'term_remote_worker' +const PANE_KEY = 'tab_remote:bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb' +const INCARNATION = 'runtime:pty:7' +const SSH_PROVIDER_GONE = 'its SSH provider is no longer registered' + +describe('federation host liveness verdicts', () => { + let db: OrchestrationDb + let runtime: OrcaRuntimeService + + beforeEach(() => { + db = new OrchestrationDb(':memory:') + runtime = new OrcaRuntimeService() + runtime.setOrchestrationDb(db) + vi.spyOn(runtime, 'getTerminalPaneKey').mockReturnValue(PANE_KEY) + vi.spyOn(runtime, 'getTerminalProcessIncarnation').mockReturnValue(INCARNATION) + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: HANDLE, + worktreeId: 'repo::remote-worktree', + connected: false, + status: 'exited' + } as never) + db.createRemoteDispatchAttachment({ + dispatchId: DISPATCH_ID, + taskId: 'task_remote', + homePeerFingerprint: HOME_FINGERPRINT, + protocolVersion: ORCHESTRATION_CONTRACT_VERSION, + runtimeEpoch: runtime.getRuntimeId(), + mutationReceipt: { + callerFingerprint: HOME_FINGERPRINT, + requestId: 'rpc_attach', + method: 'orchestration.federationStart', + payloadHash: 'hash' + } + }) + db.prepareRemoteAttachmentAuthority({ + dispatchId: DISPATCH_ID, + paneKey: PANE_KEY, + processIncarnation: INCARNATION, + worktreeId: 'repo::remote-worktree', + terminalHandle: HANDLE, + setupState: 'not_applicable', + effects: [{ kind: 'terminal', action: 'created', id: HANDLE }] + }) + db.markRemoteAttachmentReady(DISPATCH_ID) + }) + + afterEach(() => db.close()) + + async function call(name: string, params: Record) { + const method = ORCHESTRATION_METHODS.find((candidate) => candidate.name === name) + if (!method) { + throw new Error(`Method not found: ${name}`) + } + return method.handler(method.params!.parse(params), { + runtime, + authenticatedCallerFingerprint: HOME_FINGERPRINT + } as never) + } + + it('reports lost contact as unverifiable rather than an observed exit', async () => { + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'unverifiable', + reason: SSH_PROVIDER_GONE + }) + + await expect( + call('orchestration.federationShow', { dispatchId: DISPATCH_ID }) + ).resolves.toMatchObject({ + observation: { status: 'unverifiable', exactWorker: true, reason: SSH_PROVIDER_GONE } + }) + }) + + it('uses the canonical live verdict for an observed process', async () => { + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: HANDLE, + worktreeId: 'repo::remote-worktree', + connected: true, + status: 'running' + } as never) + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'live', + ptyIds: [HANDLE] + }) + + await expect( + call('orchestration.federationShow', { dispatchId: DISPATCH_ID }) + ).resolves.toMatchObject({ observation: { status: 'live', exactWorker: true } }) + }) + + it('still reports a locally observed exit as exited', async () => { + await expect( + call('orchestration.federationShow', { dispatchId: DISPATCH_ID }) + ).resolves.toMatchObject({ observation: { status: 'exited', exactWorker: true } }) + }) + + it('still serves output for a terminal we merely lost stop-contact with', async () => { + // Why this matters: the read gate used to reject every status except live, which + // would refuse a connected terminal the moment a stop lost contact with it. + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: HANDLE, + worktreeId: 'repo::remote-worktree', + connected: true + } as never) + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'unverifiable', + reason: SSH_PROVIDER_GONE + }) + + const outcome = await call('orchestration.federationRead', { + dispatchId: DISPATCH_ID + }).catch((error: unknown) => error) + + expect(outcome).not.toMatchObject({ code: 'worker_identity_changed' }) + }) + + it('does not relay an unconfirmed close home as a settled stop', async () => { + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'unverifiable', + reason: SSH_PROVIDER_GONE + }) + const closeTerminal = vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ + handle: HANDLE, + tabId: 'tab_remote', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: SSH_PROVIDER_GONE + }) + + const stopped = (await call('orchestration.federationStop', { dispatchId: DISPATCH_ID })) as { + state: string + lastError?: string + } + + // Losing contact is a reason to report honestly, never to stop trying. + expect(closeTerminal).toHaveBeenCalledWith(HANDLE) + expect(stopped.state).not.toBe('stopped') + expect(stopped.lastError).toContain('could not be confirmed stopped') + }) + + it('does not settle a bare false close as a stop', async () => { + vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ + handle: HANDLE, + tabId: 'tab_remote', + ptyKilled: false + }) + + const stopped = (await call('orchestration.federationStop', { dispatchId: DISPATCH_ID })) as { + state: string + lastError?: string + } + + expect(stopped.state).not.toBe('stopped') + expect(stopped.lastError).toContain('could not be confirmed stopped') + }) + + it('still settles a confirmed close as a stop', async () => { + vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ + handle: HANDLE, + tabId: 'tab_remote', + ptyKilled: true + }) + + const stopped = (await call('orchestration.federationStop', { dispatchId: DISPATCH_ID })) as { + state: string + processAction: string + } + + expect(stopped.state).toBe('stopped') + expect(stopped.processAction).toBe('closed_agent_terminal') + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-federation-output.test.ts b/src/main/runtime/rpc/methods/orchestration-federation-output.test.ts index 9617c05c125..adb2cc5459a 100644 --- a/src/main/runtime/rpc/methods/orchestration-federation-output.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-federation-output.test.ts @@ -192,7 +192,7 @@ describe('orchestration federated worker output', () => { ok: true, result: { server: { environmentId: 'environment_windows', name: 'windows' }, - observation: { status: 'running', exactWorker: true }, + observation: { status: 'live', exactWorker: true }, terminal: { handle: 'term_windows_worker' } } }) diff --git a/src/main/runtime/rpc/methods/orchestration-federation-setup.test.ts b/src/main/runtime/rpc/methods/orchestration-federation-setup.test.ts index 70c08e2ce95..6eec52cd93f 100644 --- a/src/main/runtime/rpc/methods/orchestration-federation-setup.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-federation-setup.test.ts @@ -181,6 +181,7 @@ describe('orchestration federated setup evidence', () => { await expect( workerShow.handler(workerShow.params!.parse({ dispatch: started.dispatch.id }), { runtime }) ).resolves.toMatchObject({ + observation: { status: 'live', exactWorker: true }, worker: { state: 'ready', stage: 'input_accepted', diff --git a/src/main/runtime/rpc/methods/orchestration-federation.test.ts b/src/main/runtime/rpc/methods/orchestration-federation.test.ts index 18db8750ff0..59dd8a7a5f8 100644 --- a/src/main/runtime/rpc/methods/orchestration-federation.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-federation.test.ts @@ -152,7 +152,8 @@ describe('orchestration federation', () => { } as never) vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ handle: 'term_windows_worker', - closed: true + tabId: 'tab-windows-worker', + ptyKilled: true } as never) } @@ -664,7 +665,7 @@ describe('orchestration federation', () => { expect(shown).toMatchObject({ ok: true, - result: { observation: { status: 'running', exactWorker: true } } + result: { observation: { status: 'live', exactWorker: true } } }) expect(homeDb.getFederatedDispatch(dispatch.id)?.remote_runtime_epoch).not.toBe(oldEpoch) expect(homeDb.getFederatedDispatch(dispatch.id)?.peer_fingerprint).toBe( diff --git a/src/main/runtime/rpc/methods/orchestration-worker-control.ts b/src/main/runtime/rpc/methods/orchestration-worker-control.ts index 87133ede7b2..480e7d3db88 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-control.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-control.ts @@ -99,7 +99,11 @@ export const ORCHESTRATION_WORKER_CONTROL_METHODS: RpcMethod[] = [ server: { environmentId: server.environmentId, name: server.name }, remoteRuntimeEpoch: remote.runtimeEpoch, terminal: remote.terminal, - observation: remote.observation + observation: { + ...remote.observation, + // Legacy servers published `running`; normalize at the compatibility boundary. + status: remote.observation.status === 'running' ? 'live' : remote.observation.status + } } } if (worker.runtime_epoch && worker.runtime_epoch !== runtime.getRuntimeId()) { @@ -122,7 +126,12 @@ export const ORCHESTRATION_WORKER_CONTROL_METHODS: RpcMethod[] = [ dispatch, worker: exposeWorker(worker), terminal: observation.exact ? observation.terminal : null, - observation: { status: observation.status, exactWorker: observation.exact }, + observation: { + status: observation.status, + exactWorker: observation.exact, + // Why: a bare `unverifiable` is not actionable without naming what we lost. + ...(observation.reason ? { reason: observation.reason } : {}) + }, terminalResource: resource ? exposeWorkerTerminalResource(resource) : null } } @@ -176,7 +185,7 @@ export const ORCHESTRATION_WORKER_CONTROL_METHODS: RpcMethod[] = [ ) } const resource = db.getWorkerTerminalResourceByOwner(params.dispatch) - if (resource && ['releasing', 'released'].includes(resource.release_state)) { + if (resource && ['releasing', 'unknown', 'released'].includes(resource.release_state)) { return readArchivedWorkerOutput({ db, dispatchId: params.dispatch, @@ -199,7 +208,18 @@ export const ORCHESTRATION_WORKER_CONTROL_METHODS: RpcMethod[] = [ dispatchId: params.dispatch, terminalHandle: worker.agent_terminal_handle, workerState: worker.state, - terminalStatus: observation.status === 'exited' ? 'exited' : 'running', + terminalStatus: + observation.status === 'exited' + ? 'exited' + : observation.status === 'unverifiable' + ? 'unknown' + : 'running', + terminalLiveness: + observation.status === 'unverifiable' + ? 'unverifiable' + : observation.status === 'exited' + ? 'exited' + : 'live', attachedAt: worker.created_at, source: params.source, cursor: params.cursor, diff --git a/src/main/runtime/rpc/methods/orchestration-worker-observation.ts b/src/main/runtime/rpc/methods/orchestration-worker-observation.ts index 7148fe32dcc..fe6ce087b7f 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-observation.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-observation.ts @@ -10,7 +10,9 @@ export async function inspectWorkerTerminal( ): Promise<{ terminal: Awaited> | null exact: boolean - status: 'unattached' | 'missing' | 'identity_changed' | 'running' | 'exited' + status: 'unattached' | 'missing' | 'identity_changed' | 'live' | 'exited' | 'unverifiable' + /** Set with `unverifiable`; names what we lost contact with. */ + reason?: string }> { const worker = db.getWorkerDispatch(dispatchId) if (!worker?.agent_terminal_handle) { @@ -25,10 +27,23 @@ export async function inspectWorkerTerminal( paneKey: runtime.getTerminalPaneKey(worker.agent_terminal_handle), processIncarnation: runtime.getTerminalProcessIncarnation(worker.agent_terminal_handle) }) + if (!exact) { + return { terminal, exact, status: 'identity_changed' } + } + // Why: the aggregate inventory only iterates registered providers, so a dropped + // relay clears `connected` for every remote PTY at once. Lost contact is not a + // death certificate, and the verdict is the only field that can tell them apart. + const verdict = runtime.getTerminalLivenessVerdict?.(worker.agent_terminal_handle) ?? null + if (verdict?.status === 'unverifiable') { + return { terminal, exact, status: 'unverifiable', reason: verdict.reason } + } + if (verdict?.status === 'live') { + return { terminal, exact, status: 'live' } + } return { terminal, exact, - status: exact ? (terminal.connected === false ? 'exited' : 'running') : 'identity_changed' + status: terminal.connected === false ? 'exited' : 'live' } } @@ -71,7 +86,7 @@ export async function callFederatedWorkerShow( residualResources: unknown[] } terminal: unknown - observation: { status: string; exactWorker: boolean } + observation: { status: string; exactWorker: boolean; reason?: string } }> { return (await runtime.callOrchestrationWorkerServer( federated.environment_id, diff --git a/src/main/runtime/rpc/methods/orchestration-worker-output.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-output.test.ts index 6db96efe33d..0e24a6bced8 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-output.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-output.test.ts @@ -83,6 +83,19 @@ describe('exact orchestration worker output', () => { expect(readTerminal).not.toHaveBeenCalled() }) + it('reports unverifiable liveness without claiming the terminal is running', async () => { + const result = await read({ + terminalStatus: 'unknown', + terminalLiveness: 'unverifiable' + }) + + expect(result.status).toEqual({ + worker: 'ready', + terminal: 'unknown', + liveness: 'unverifiable' + }) + }) + it('reads Grok through the shared Native Chat transcript decoder', async () => { await writeFile( transcriptA, diff --git a/src/main/runtime/rpc/methods/orchestration-worker-output.ts b/src/main/runtime/rpc/methods/orchestration-worker-output.ts index 2b222f01586..bd63da46546 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-output.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-output.ts @@ -20,6 +20,7 @@ export async function readExactWorkerOutput(args: { terminalHandle: string workerState: string terminalStatus: RuntimeTerminalState + terminalLiveness?: 'live' | 'unverifiable' | 'exited' attachedAt: string source?: OrchestrationWorkerReadSource cursor?: string | number @@ -100,7 +101,11 @@ export async function readExactWorkerOutput(args: { returnedMessageCount: transcript.messages.length }, cursor: nextCursor, - status: { worker: args.workerState, terminal: args.terminalStatus }, + status: { + worker: args.workerState, + terminal: args.terminalStatus, + ...(args.terminalLiveness ? { liveness: args.terminalLiveness } : {}) + }, fallbackReason: null, warnings: transcript.warnings } @@ -145,7 +150,11 @@ async function readTerminalOutput( sourceIdentity, terminal: { ...terminal, tail: redactedTerminal.lines }, cursor: nextCursor, - status: { worker: args.workerState, terminal: terminal.status }, + status: { + worker: args.workerState, + terminal: args.terminalLiveness === 'unverifiable' ? args.terminalStatus : terminal.status, + ...(args.terminalLiveness ? { liveness: args.terminalLiveness } : {}) + }, fallbackReason: null, warnings: redactedTerminal.warnings } diff --git a/src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts b/src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts index 4beb0323fd1..ecac17b8cbb 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-release-completion.ts @@ -10,6 +10,7 @@ import { type WorkerTerminalTailArchive } from '../../orchestration/worker-output-archive' import type { OrcaRuntimeService } from '../../orca-runtime' +import { describeUnconfirmedAgentStop } from '../../../../shared/pty-liveness-verdict' import { inspectWorkerTerminal } from './orchestration-worker-observation' import { orchestrationTimestampToMs } from './orchestration-worker-output' @@ -212,7 +213,19 @@ async function completeWorkerTerminalReleaseOnce( } try { - await runtime.closeTerminal(resource.terminal_handle) + const close = await runtime.closeTerminal(resource.terminal_handle) + if (!close.ptyKilled) { + const reason = describeUnconfirmedAgentStop(close) + const unknown = db.markWorkerTerminalReleaseUnknown(resource.id, reason) + return { + dispatchId, + state: 'release_unknown', + processAction: 'closed_agent_terminal', + archive: { source: archiveSource, status: archiveStatus }, + lastError: unknown.release_error ?? reason, + recovery: `Inspect with: orca orchestration worker-show --dispatch ${dispatchId} --json — then repeat worker-release with the same --retry-request. Never substitute a broad terminal close.` + } + } } catch (error) { const reason = error instanceof Error ? error.message : String(error) if (/disposed|not connected|unavailable/i.test(reason)) { diff --git a/src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts new file mode 100644 index 00000000000..6124b8b6cae --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration-worker-release-liveness-verdict.test.ts @@ -0,0 +1,84 @@ +import { describe, expect, it, vi } from 'vitest' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { OrchestrationDb } from '../../orchestration/db' +import type { WorkerTerminalResourceRow } from '../../orchestration/worker-terminal-ownership' +import { completeWorkerTerminalRelease } from './orchestration-worker-release-completion' + +describe('orchestration worker release liveness verdict', () => { + it.each([ + { + name: 'an explicit unverifiable verdict', + close: { + handle: 'term_worker', + tabId: 'tab-worker', + ptyKilled: false, + ptyStopVerdict: 'unverifiable' as const, + ptyStopReason: 'its SSH provider is no longer registered' + }, + detail: 'its SSH provider is no longer registered' + }, + { + name: 'a bare unconfirmed close', + close: { handle: 'term_worker', tabId: 'tab-worker', ptyKilled: false }, + detail: 'the stop outcome could not be verified' + } + ])('does not release a worker after $name', async ({ close, detail }) => { + const reason = 'its SSH provider is no longer registered' + const resource = { + id: 'resource-1', + terminal_handle: 'term_worker', + host_scope: JSON.stringify({ kind: 'ssh', targetId: 'target-1' }), + archive_source: 'terminal', + archive_status: 'captured', + ownership_state: 'owned', + release_state: 'requested' + } as WorkerTerminalResourceRow + const runtime = { + showTerminal: vi.fn(async () => ({ handle: 'term_worker', connected: false })), + getTerminalPaneKey: vi.fn(() => 'tab-worker:leaf-worker'), + getTerminalProcessIncarnation: vi.fn(() => 'pty-worker:incarnation-1'), + getTerminalLivenessVerdict: vi.fn(() => ({ status: 'unverifiable', reason })), + getOrchestrationDispatchAuthority: vi.fn(() => ({ + hostScope: { kind: 'ssh', targetId: 'target-1' } + })), + closeTerminal: vi.fn(async () => close), + notifyMessageArrived: vi.fn() + } as unknown as OrcaRuntimeService + const markWorkerTerminalReleaseUnknown = vi.fn((_resourceId: string, releaseError: string) => ({ + ...resource, + release_state: 'unknown', + release_error: releaseError + })) + const db = { + getWorkerDispatch: vi.fn(() => ({ + agent_terminal_handle: 'term_worker', + created_at: '2026-08-16T00:00:00.000Z' + })), + isDispatchProcessCurrent: vi.fn(() => true), + workerTerminalResourceHasIdentityConflict: vi.fn(() => false), + getWorkerTerminalArchive: vi.fn(() => ({ kind: 'transcript_pin' })), + commitWorkerTerminalArchiveForRelease: vi.fn(() => ({ + ...resource, + release_state: 'releasing' + })), + markWorkerTerminalReleaseUnknown + } as unknown as OrchestrationDb + + await expect( + completeWorkerTerminalRelease({ + runtime, + db, + dispatchId: 'ctx-worker', + resource + }) + ).resolves.toMatchObject({ + state: 'release_unknown', + processAction: 'closed_agent_terminal', + lastError: `The agent terminal was closed but its process could not be confirmed stopped: ${detail}.` + }) + expect(markWorkerTerminalReleaseUnknown).toHaveBeenCalledWith( + 'resource-1', + `The agent terminal was closed but its process could not be confirmed stopped: ${detail}.` + ) + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.ts index d090e15103a..b29d765dcad 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-release-recovery.test.ts @@ -80,8 +80,9 @@ describe('orchestration worker release recovery', () => { }) vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ handle: 'term_worker', - closed: true - } as never) + tabId: 'tab-worker', + ptyKilled: true + }) vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {}) activeRunId = db.createRun({ objective: 'Release recovery test Run', @@ -162,6 +163,36 @@ describe('orchestration worker release recovery', () => { expect(db.getWorkerTerminalResourceByOwner(dispatchId)?.release_state).toBe('releasing') }) + it('preserves archived output when an unconfirmed release retry cannot find the terminal', async () => { + setup() + const { dispatchId } = await startSettledWorker() + vi.mocked(runtime.closeTerminal).mockResolvedValueOnce({ + handle: 'term_worker', + tabId: 'tab-worker', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH provider is no longer registered' + }) + + await expect( + call('orchestration.workerRelease', { dispatch: dispatchId }) + ).resolves.toMatchObject({ state: 'release_unknown' }) + expect(db.getWorkerTerminalArchive(dispatchId)).toBeDefined() + + vi.mocked(runtime.showTerminal).mockRejectedValue(new Error('terminal_handle_stale')) + await expect( + call('orchestration.workerRelease', { dispatch: dispatchId }) + ).resolves.toMatchObject({ state: 'release_unknown' }) + const read = (await call('orchestration.workerRead', { dispatch: dispatchId })) as { + archived?: boolean + terminal: { tail: string[] } + } + expect(read).toMatchObject({ + archived: true, + terminal: { tail: ['worker output line 1', 'worker output line 2'] } + }) + }) + it('never touches resources without requested releases', async () => { setup() await startSettledWorker() diff --git a/src/main/runtime/rpc/methods/orchestration-worker-release.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-release.test.ts index b5ba5b0000a..5bdac829942 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-release.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-release.test.ts @@ -93,7 +93,8 @@ describe('orchestration worker release', () => { }) vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ handle: 'term_worker', - closed: true + tabId: 'tab-worker', + ptyKilled: true } as never) vi.spyOn(runtime, 'notifyMessageArrived').mockImplementation(() => {}) activeRunId = db.createRun({ @@ -242,7 +243,7 @@ describe('orchestration worker release', () => { it('reconciles a dead external terminal without closing a process', async () => { setup() const { dispatchId } = await startSettledWorker('succeeded', { terminal: 'term_worker' }) - inspectProcessLiveness.mockResolvedValue('dead') + inspectProcessLiveness.mockResolvedValue('exited') await expect( call('orchestration.workerRelease', { dispatch: dispatchId }) @@ -268,7 +269,7 @@ describe('orchestration worker release', () => { raw .prepare('UPDATE worker_terminal_resources SET prior_owner_dispatch_ids = ? WHERE id = ?') .run('{invalid', resource?.id) - inspectProcessLiveness.mockResolvedValue('dead') + inspectProcessLiveness.mockResolvedValue('exited') await expect( call('orchestration.workerRelease', { dispatch: dispatchId }) @@ -297,7 +298,7 @@ describe('orchestration worker release', () => { setup() const { dispatchId } = await startSettledWorker() await call('orchestration.workerTerminalUserInput', { paneKey: workerPaneKey }) - inspectProcessLiveness.mockResolvedValue('dead') + inspectProcessLiveness.mockResolvedValue('exited') await expect( call('orchestration.workerRelease', { dispatch: dispatchId }) @@ -324,7 +325,7 @@ describe('orchestration worker release', () => { } else { db.abandonWorkerDispatch(dispatchId) } - inspectProcessLiveness.mockResolvedValue('dead') + inspectProcessLiveness.mockResolvedValue('exited') await expect( call('orchestration.workerRelease', { dispatch: dispatchId }) @@ -706,7 +707,7 @@ describe('orchestration worker release', () => { expect(transferred?.terminal_handle).toBe('term_reminted') expect(db.getWorkerTerminalResourceByOwner(first.dispatchId)).toBeUndefined() - inspectProcessLiveness.mockResolvedValueOnce('dead') + inspectProcessLiveness.mockResolvedValueOnce('exited') const oldRelease = (await call('orchestration.workerRelease', { dispatch: first.dispatchId })) as { state: string; reason?: string } @@ -727,7 +728,7 @@ describe('orchestration worker release', () => { const first = await startSettledWorker('succeeded') const second = await startWorker({ terminal: 'term_reminted' }) settle(second.taskId, second.dispatchId, 'succeeded') - inspectProcessLiveness.mockResolvedValue('dead') + inspectProcessLiveness.mockResolvedValue('exited') await expect( call('orchestration.workerRelease', { dispatch: first.dispatchId }) diff --git a/src/main/runtime/rpc/methods/orchestration-worker-release.ts b/src/main/runtime/rpc/methods/orchestration-worker-release.ts index 446887f581a..dd5b5ed4aa3 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-release.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-release.ts @@ -61,7 +61,7 @@ export const ORCHESTRATION_WORKER_RELEASE_METHODS: RpcMethod[] = [ (await runtime.inspectTerminalProcessIncarnationLiveness( processIncarnation, resource.host_scope - )) === 'dead' + )) === 'exited' ) { const reconciled = db.settleDeadWorkerTerminalRelease({ requestingDispatchId: params.dispatch, diff --git a/src/main/runtime/rpc/methods/orchestration-worker-stop-capability.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-stop-capability.test.ts new file mode 100644 index 00000000000..15d9ef62612 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration-worker-stop-capability.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it, vi } from 'vitest' +import { ORCHESTRATION_CONTRACT_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { OrcaRuntimeService } from '../../orca-runtime' +import type { OrchestrationDb } from '../../orchestration/db' +import { ORCHESTRATION_WORKER_STOP_METHODS } from './orchestration-worker-stop' + +describe('federated worker stop capability', () => { + it('does not trust a legacy server stop receipt', async () => { + const markWorkerStopUnknown = vi.fn((_dispatchId: string, reason: string) => ({ + state: 'stop_unknown', + last_error: reason + })) + const db = { + getFederatedDispatch: vi.fn(() => ({ + dispatch_id: 'ctx_remote', + environment_id: 'environment_linux', + environment_name: 'linux', + peer_fingerprint: 'peer-linux' + })), + beginWorkerStop: vi.fn(() => ({ disposition: 'started' })), + markWorkerStopUnknown + } as unknown as OrchestrationDb + const callOrchestrationWorkerServer = vi.fn(async (_environmentId, method) => { + if (method === 'status.get') { + return { capabilities: [ORCHESTRATION_CONTRACT_RUNTIME_CAPABILITY] } + } + return { state: 'stopped', processAction: 'closed_agent_terminal' } + }) + const runtime = { + getOrchestrationDb: () => db, + resolveOrchestrationWorkerServer: () => ({ + environmentId: 'environment_linux', + name: 'linux', + peerFingerprint: 'peer-linux' + }), + callOrchestrationWorkerServer + } as unknown as OrcaRuntimeService + const method = ORCHESTRATION_WORKER_STOP_METHODS[0]! + + await expect( + method.handler(method.params!.parse({ dispatch: 'ctx_remote' }), { + runtime, + orchestrationMutation: { + callerFingerprint: 'coordinator', + requestId: 'request_stop', + method: 'orchestration.workerStop', + payloadHash: 'hash' + } + }) + ).resolves.toMatchObject({ state: 'stop_unknown', processAction: 'none' }) + expect(markWorkerStopUnknown).toHaveBeenCalledWith( + 'ctx_remote', + 'Connected server linux cannot prove the worker stop outcome.' + ) + expect(callOrchestrationWorkerServer).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts b/src/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts new file mode 100644 index 00000000000..60ff3f66575 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration-worker-stop-liveness-verdict.test.ts @@ -0,0 +1,158 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { OrcaRuntimeService } from '../../orca-runtime' +import { OrchestrationDb } from '../../orchestration/db' +import { ORCHESTRATION_METHODS } from './orchestration' + +// The aggregate terminal inventory only iterates registered providers, so a +// dropped relay clears `connected` for every remote PTY at once. That is lost +// contact, not a death certificate. +describe('worker-stop against a terminal we lost contact with', () => { + let db: OrchestrationDb + let runtime: OrcaRuntimeService + + beforeEach(() => { + db = new OrchestrationDb(':memory:') + runtime = new OrcaRuntimeService() + runtime.setOrchestrationDb(db) + vi.spyOn(runtime, 'getTerminalPaneKey').mockReturnValue( + 'tab_worker:bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb' + ) + vi.spyOn(runtime, 'getTerminalProcessIncarnation').mockReturnValue('runtime:pty:1') + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: 'term_worker', + worktreeId: 'repo::worktree', + connected: false, + status: 'exited' + } as never) + }) + + afterEach(() => db.close()) + + async function call(name: string, params: Record) { + const method = ORCHESTRATION_METHODS.find((candidate) => candidate.name === name) + if (!method) { + throw new Error(`Method not found: ${name}`) + } + return method.handler(method.params!.parse(params), { runtime }) + } + + function createWorker() { + const run = db.createRun({ + objective: 'Verdict', + coordinatorHandle: 'term_coord', + coordinatorPaneKey: 'tab_coord:aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' + }) + const task = db.createTask({ spec: 'stop worker', runId: run.id }) + const started = db.createStartingWorkerDispatch({ + taskId: task.id, + startOptions: {}, + runtimeEpoch: runtime.getRuntimeId() + }) + db.prepareStartingWorkerAuthority({ + dispatchId: started.dispatch.id, + handle: 'term_worker', + paneKey: 'tab_worker:bbbbbbbb-bbbb-4bbb-8bbb-bbbbbbbbbbbb', + processIncarnation: 'runtime:pty:1', + worktreeId: 'repo::worktree', + setupState: 'not_applicable', + effects: [{ kind: 'terminal', action: 'created', id: 'term_worker' }] + }) + db.markWorkerDispatchReady(started.dispatch.id) + return started.dispatch + } + + it('reports the process as unverifiable, not exited, when the link dropped', async () => { + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'unverifiable', + reason: 'its SSH provider is no longer registered' + }) + const closeTerminal = vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ + handle: 'term_worker', + tabId: 'tab_worker', + ptyKilled: false, + ptyStopVerdict: 'unverifiable', + ptyStopReason: 'its SSH provider is no longer registered' + }) + const dispatch = createWorker() + + await expect( + call('orchestration.workerShow', { dispatch: dispatch.id }) + ).resolves.toMatchObject({ + observation: { + status: 'unverifiable', + exactWorker: true, + reason: 'its SSH provider is no longer registered' + } + }) + + const stopped = (await call('orchestration.workerStop', { dispatch: dispatch.id })) as { + state: string + processAction: string + lastError: string + } + // Losing contact is a reason to report honestly, never to stop trying. + expect(closeTerminal).toHaveBeenCalledWith('term_worker') + expect(stopped.processAction).toBe('closed_agent_terminal') + expect(stopped.state).toBe('stop_unknown') + expect(stopped.lastError).toContain('could not be confirmed stopped') + expect(stopped.lastError).not.toContain('exited') + }) + + it('does not settle a bare false close as stopped', async () => { + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: 'term_worker', + worktreeId: 'repo::worktree', + connected: true, + status: 'running' + } as never) + vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ + handle: 'term_worker', + tabId: 'tab_worker', + ptyKilled: false + }) + const dispatch = createWorker() + + const stopped = (await call('orchestration.workerStop', { dispatch: dispatch.id })) as { + state: string + lastError: string + } + + expect(stopped.state).toBe('stop_unknown') + expect(stopped.lastError).toContain('could not be confirmed stopped') + }) + + it('uses the canonical live verdict for an observed process', async () => { + vi.spyOn(runtime, 'showTerminal').mockResolvedValue({ + handle: 'term_worker', + worktreeId: 'repo::worktree', + connected: true, + status: 'running' + } as never) + vi.spyOn(runtime, 'getTerminalLivenessVerdict').mockReturnValue({ + status: 'live', + ptyIds: ['runtime:pty:1'] + }) + const dispatch = createWorker() + + await expect( + call('orchestration.workerShow', { dispatch: dispatch.id }) + ).resolves.toMatchObject({ + observation: { status: 'live', exactWorker: true } + }) + }) + + it('still reports a locally observed exit as exited', async () => { + const dispatch = createWorker() + + await expect( + call('orchestration.workerShow', { dispatch: dispatch.id }) + ).resolves.toMatchObject({ + observation: { status: 'exited', exactWorker: true } + }) + + const stopped = (await call('orchestration.workerStop', { dispatch: dispatch.id })) as { + lastError: string + } + expect(stopped.lastError).toBe('The recorded worker process is exited; no terminal was closed.') + }) +}) diff --git a/src/main/runtime/rpc/methods/orchestration-worker-stop.ts b/src/main/runtime/rpc/methods/orchestration-worker-stop.ts index 0802fac3954..c02b43d5dc2 100644 --- a/src/main/runtime/rpc/methods/orchestration-worker-stop.ts +++ b/src/main/runtime/rpc/methods/orchestration-worker-stop.ts @@ -2,6 +2,9 @@ import { z } from 'zod' import { OrchestrationError } from '../../orchestration/orchestration-error' import { defineMethod, type RpcMethod } from '../core' import { requiredString } from '../schemas' +import { describeUnconfirmedAgentStop } from '../../../../shared/pty-liveness-verdict' +import { ORCHESTRATION_WORKER_STOP_VERDICT_RUNTIME_CAPABILITY } from '../../../../shared/protocol-version' +import type { RuntimeStatus } from '../../../../shared/runtime-types' import { inspectWorkerTerminal, resolvePinnedFederatedServer @@ -29,6 +32,24 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [ return settledReceipt(params.dispatch, begun.worker.state) } try { + const status = (await runtime.callOrchestrationWorkerServer( + server.environmentId, + 'status.get', + undefined, + 30_000 + )) as RuntimeStatus + if ( + !status.capabilities?.includes(ORCHESTRATION_WORKER_STOP_VERDICT_RUNTIME_CAPABILITY) + ) { + return unknownReceipt( + params.dispatch, + db.markWorkerStopUnknown( + params.dispatch, + `Connected server ${server.name} cannot prove the worker stop outcome.` + ), + 'none' + ) + } const remote = (await runtime.callOrchestrationWorkerServer( server.environmentId, 'orchestration.federationStop', @@ -101,7 +122,12 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [ ) } const observation = await inspectWorkerTerminal(runtime, db, params.dispatch) - if (!observation.exact || observation.status !== 'running') { + // Why `unverifiable` still proceeds: losing contact is a reason to report + // the outcome honestly, never a reason to stop trying to stop the worker. + if ( + !observation.exact || + (observation.status !== 'live' && observation.status !== 'unverifiable') + ) { return unknownReceipt( params.dispatch, db.markWorkerStopUnknown( @@ -113,6 +139,15 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [ } try { const close = await runtime.closeTerminal(handle) + if (!close.ptyKilled) { + // The tab is retired, but the agent process was never confirmed stopped — + // settling here is the false success this receipt exists to prevent. + return unknownReceipt( + params.dispatch, + db.markWorkerStopUnknown(params.dispatch, describeUnconfirmedAgentStop(close)), + 'closed_agent_terminal' + ) + } const worker = db.settleWorkerStop(params.dispatch) runtime.notifyMessageArrived(`dispatch:${params.dispatch}`, 'status') return { diff --git a/src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts b/src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts index 880620eee43..3f4679353bf 100644 --- a/src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts +++ b/src/main/runtime/rpc/methods/orchestration-workers-recovery.test.ts @@ -30,7 +30,8 @@ describe('orchestration worker recovery', () => { }) vi.spyOn(runtime, 'closeTerminal').mockResolvedValue({ handle: 'term_worker', - closed: true + tabId: 'tab-worker', + ptyKilled: true } as never) }) @@ -80,7 +81,7 @@ describe('orchestration worker recovery', () => { call('orchestration.workerShow', { dispatch: dispatch.id }) ).resolves.toMatchObject({ worker: { state: 'ready' }, - observation: { status: 'running', exactWorker: true }, + observation: { status: 'live', exactWorker: true }, terminal: { handle: 'term_worker' } }) await expect( diff --git a/src/main/runtime/unstopped-pty-verification.ts b/src/main/runtime/unstopped-pty-verification.ts index 2dc9aac5960..a2cd83b8eec 100644 --- a/src/main/runtime/unstopped-pty-verification.ts +++ b/src/main/runtime/unstopped-pty-verification.ts @@ -1,18 +1,20 @@ import type { IPtyProvider } from '../providers/types' +import type { OrcaRuntimeService } from './orca-runtime' import { UNSTOPPED_PTY_DETAIL_SEPARATOR, UNSTOPPED_PTY_LIVE_DETAIL_PREFIX, UNSTOPPED_PTY_REMOVAL_PREFIX } from '../../shared/worktree/removal' +import { + NO_OBSERVING_PROVIDER_REASON, + type PtyLivenessVerdict +} from '../../shared/pty-liveness-verdict' import { settleBeforeDeadline } from './settle-before-deadline' // Floor for the verification window when the sweep ran on a very short budget. export const WORKTREE_TEARDOWN_VERIFY_GRACE_MS = 2_000 -export type UnstoppedPtyVerdict = - | { status: 'exited' } - | { status: 'live'; ptyIds: string[] } - | { status: 'unverifiable'; reason: string } +export type UnstoppedPtyVerdict = PtyLivenessVerdict /** * Re-lists the provider's processes to decide what a failed stop RPC actually @@ -56,6 +58,45 @@ export async function verifyUnstoppedPtys( return stillLive.length > 0 ? { status: 'live', ptyIds: stillLive } : { status: 'exited' } } +/** + * A stop that lost contact with the PTY's own host stays unverifiable: the + * surviving provider's inventory is silent about a host it cannot reach, and + * silence is not evidence of an exit. Force Delete is still the escape hatch. + */ +export function unverifiableStopVerdict( + failedPtyIds: readonly string[], + runtime: OrcaRuntimeService | undefined +): UnstoppedPtyVerdict | null { + for (const ptyId of failedPtyIds) { + const verdict = runtime?.getPtyLivenessVerdict?.(ptyId) + if (verdict?.status === 'unverifiable') { + return verdict + } + } + return null +} + +export async function resolveUnstoppedPtyVerdict( + failedPtyIds: readonly string[], + provider: IPtyProvider, + sweepBudgetMs: number, + providerObservesOwningHost: boolean, + runtime?: OrcaRuntimeService +): Promise { + if (failedPtyIds.length === 0) { + return { status: 'exited' } + } + if (!providerObservesOwningHost) { + return ( + unverifiableStopVerdict(failedPtyIds, runtime) ?? { + status: 'unverifiable', + reason: NO_OBSERVING_PROVIDER_REASON + } + ) + } + return verifyUnstoppedPtys(failedPtyIds, provider, sweepBudgetMs) +} + /** Names the blocking PTYs so a wedged removal is diagnosable, not just refused. */ export function describeUnstoppedPtys( worktreeId: string, diff --git a/src/main/runtime/worktree-teardown-unstopped-pty.test.ts b/src/main/runtime/worktree-teardown-unstopped-pty.test.ts index 31ba390c3ec..c89c5e93747 100644 --- a/src/main/runtime/worktree-teardown-unstopped-pty.test.ts +++ b/src/main/runtime/worktree-teardown-unstopped-pty.test.ts @@ -175,6 +175,80 @@ describe('destructive teardown when a PTY stop cannot be proven', () => { expect(error.message).not.toContain('w1@@gone-2') }) + // Why: the client's own provider list is silent about an SSH host, so a stop + // it could not confirm must stay unverifiable instead of reading as exited — + // otherwise removal walks straight past a live remote agent. + it("blocks removal when the stop lost contact with the PTY's own host", async () => { + // The surviving provider lists nothing for a host it cannot reach, so without + // the recorded verdict this empty list reads as "exited" and removal walks + // straight past a live remote agent. + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + const runtime = { + stopTerminalsForWorktree: async ( + _worktreeId: string, + opts: { + stopPty?: ( + ptyId: string, + stop: () => boolean | Promise + ) => Promise<{ stopped: boolean; owner: boolean }> + } + ) => { + await opts.stopPty?.('ssh:conn-1@@relay-9', () => false) + return { stopped: 0 } + }, + getPtyLivenessVerdict: (ptyId: string) => + ptyId === 'ssh:conn-1@@relay-9' + ? { status: 'unverifiable', reason: 'its SSH provider is no longer registered' } + : null + } + + await expect( + killAllProcessesForWorktree('w1', { + runtime: runtime as never, + localProvider, + resolvedConnectionId: 'conn-1', + includeProviderInventory: false, + includeLocalRegistry: false, + requirePhysicalStop: true + }) + ).rejects.toThrow( + /could not verify[\s\S]*ssh:conn-1@@relay-9[\s\S]*SSH provider is no longer registered[\s\S]*--force/ + ) + }) + + it('does not infer a remote exit from fallback local inventory without a cached verdict', async () => { + const localProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + const runtime = { + stopTerminalsForWorktree: async ( + _worktreeId: string, + opts: { + stopPty?: ( + ptyId: string, + stop: () => boolean | Promise + ) => Promise<{ stopped: boolean; owner: boolean }> + } + ) => { + await opts.stopPty?.('ssh:conn-1@@relay-10', () => false) + return { stopped: 0 } + }, + getPtyLivenessVerdict: () => null + } + + await expect( + killAllProcessesForWorktree('w1', { + runtime: runtime as never, + localProvider, + resolvedConnectionId: 'conn-1', + includeProviderInventory: false, + includeLocalRegistry: false, + requirePhysicalStop: true + }) + ).rejects.toThrow(/could not verify[\s\S]*no registered provider can observe its host/) + expect(localProvider.listProcesses).not.toHaveBeenCalled() + }) + it('reports unverifiable separately from live when the process list fails', async () => { const localProvider = createProviderStub(async () => { throw new Error('daemon socket closed') diff --git a/src/main/runtime/worktree-teardown.test.ts b/src/main/runtime/worktree-teardown.test.ts index b51f292c29c..ed29bc26e2f 100644 --- a/src/main/runtime/worktree-teardown.test.ts +++ b/src/main/runtime/worktree-teardown.test.ts @@ -521,7 +521,7 @@ describe('killAllProcessesForWorktree', () => { expect(localProvider.shutdown).toHaveBeenCalledTimes(1) }) - it('accepts a failed Windows stop when a fresh inventory proves the PTY exited', async () => { + it('accepts a failed stop when exact-owner inventory supersedes cached uncertainty', async () => { const worktreeId = 'repo-1::C:/Users/User/orca/workspaces/repo/feature' const ptyId = `${worktreeId}@@windows-pty` const stopTerminalsForWorktree = vi.fn( @@ -538,7 +538,11 @@ describe('killAllProcessesForWorktree', () => { }) ) const runtime = { - stopTerminalsForWorktree + stopTerminalsForWorktree, + getPtyLivenessVerdict: vi.fn(() => ({ + status: 'unverifiable', + reason: 'the provider disconnected during stop' + })) } as unknown as Parameters[1]['runtime'] let inventoryCount = 0 const localProvider = createProviderStub(async () => { @@ -568,6 +572,45 @@ describe('killAllProcessesForWorktree', () => { expect(localProvider.listProcesses).toHaveBeenCalledTimes(2) }) + it('keeps cached uncertainty when no provider can observe the owning host', async () => { + const worktreeId = 'repo-1::/remote/worktree' + const ptyId = `${worktreeId}@@remote-pty` + const stopTerminalsForWorktree = vi.fn( + async ( + _worktreeId: string, + options: { + stopPty: ( + ptyId: string, + stop: () => boolean + ) => Promise<{ stopped: boolean; owner: boolean }> + } + ) => ({ + stopped: (await options.stopPty(ptyId, () => false)).owner ? 1 : 0 + }) + ) + const runtime = { + stopTerminalsForWorktree, + getPtyLivenessVerdict: vi.fn(() => ({ + status: 'unverifiable', + reason: 'no registered provider can observe its host' + })) + } as unknown as Parameters[1]['runtime'] + const fallbackProvider = createProviderStub(async () => []) + listRegisteredPtysMock.mockReturnValue([]) + + await expect( + killAllProcessesForWorktree(worktreeId, { + runtime, + resolvedConnectionId: 'missing-connection', + localProvider: fallbackProvider, + includeProviderInventory: false, + includeLocalRegistry: false, + requirePhysicalStop: true + }) + ).rejects.toThrow('no registered provider can observe its host') + expect(fallbackProvider.listProcesses).not.toHaveBeenCalled() + }) + it('keeps duplicate sweeps behind the runtime physical-stop promise', async () => { let releasePhysicalStop: () => void = () => undefined const physicalStop = new Promise((resolve) => { diff --git a/src/main/runtime/worktree-teardown.ts b/src/main/runtime/worktree-teardown.ts index ed0df69e8f7..a28875dff24 100644 --- a/src/main/runtime/worktree-teardown.ts +++ b/src/main/runtime/worktree-teardown.ts @@ -15,8 +15,7 @@ import { describeError, describeFailedPtySweep, describeUnstoppedPtys, - verifyUnstoppedPtys, - type UnstoppedPtyVerdict + resolveUnstoppedPtyVerdict } from './unstopped-pty-verification' // Why: normal inventories still coalesce into one process scan, while a stale @@ -231,10 +230,15 @@ export async function killAllProcessesForWorktree( [...stopAttempts].map(async ([ptyId, stopped]) => [ptyId, await stopped] as const) ) const failedPtyIds = stopResults.filter(([, stopped]) => !stopped).map(([ptyId]) => ptyId) - const verdict: UnstoppedPtyVerdict = - failedPtyIds.length === 0 - ? { status: 'exited' } - : await verifyUnstoppedPtys(failedPtyIds, deps.localProvider, sweepBudgetMs) + const verdict = await resolveUnstoppedPtyVerdict( + failedPtyIds, + deps.localProvider, + sweepBudgetMs, + deps.includeProviderInventory !== false || + (deps.resolvedConnectionId === undefined && + deps.resolvedRuntimeEnvironmentId === undefined), + deps.runtime + ) if (verdict.status === 'exited') { for (const ptyId of failedPtyIds) { clearStoppedPtyState(ptyId, deps.onPtyStopped) @@ -363,13 +367,10 @@ async function sweepRegistryForWorktree( } function clearStoppedPtyState(ptyId: string, onPtyStopped?: (ptyId: string) => void): void { - if (!onPtyStopped) { - return - } try { // Why: daemon shutdown does not always fan a local pty:exit event back // through pty.ts, but removed worktrees must immediately drop memory rows. - onPtyStopped(ptyId) + onPtyStopped?.(ptyId) } catch { /* cleanup is best-effort and must not block git-level removal */ } diff --git a/src/main/text-generation/commit-message-text-generation.ts b/src/main/text-generation/commit-message-text-generation.ts index 6370117066c..e4077b3bddf 100644 --- a/src/main/text-generation/commit-message-text-generation.ts +++ b/src/main/text-generation/commit-message-text-generation.ts @@ -845,9 +845,7 @@ export function commandBackslashMode( target: CommitMessageGenerationTarget, platform: NodeJS.Platform = process.platform ): CommandTemplateBackslash { - return platform === 'win32' && target.kind === 'local' && !target.wslDistro - ? 'literal' - : 'escape' + return platform === 'win32' && target.kind === 'local' && !target.wslDistro ? 'literal' : 'escape' } type LocalGenerationTarget = Extract diff --git a/src/shared/orchestration-worker-output.ts b/src/shared/orchestration-worker-output.ts index 8e61d1d3b97..767623f96f8 100644 --- a/src/shared/orchestration-worker-output.ts +++ b/src/shared/orchestration-worker-output.ts @@ -1,6 +1,7 @@ import type { AgentProviderSessionMetadata } from './agent-session-resume' import type { AgentType, NativeChatMessage } from './native-chat-types' import type { RuntimeTerminalRead, RuntimeTerminalState } from './runtime-types' +import type { PtyLivenessVerdict } from './pty-liveness-verdict' export const ORCHESTRATION_WORKER_READ_SOURCES = ['auto', 'transcript', 'terminal'] as const export type OrchestrationWorkerReadSource = (typeof ORCHESTRATION_WORKER_READ_SOURCES)[number] @@ -41,6 +42,7 @@ export type OrchestrationWorkerReadTranscriptResult = { status: { worker: string terminal: RuntimeTerminalState + liveness?: PtyLivenessVerdict['status'] } fallbackReason: null warnings: string[] @@ -57,6 +59,7 @@ export type OrchestrationWorkerReadTerminalResult = { status: { worker: string terminal: RuntimeTerminalState + liveness?: PtyLivenessVerdict['status'] } fallbackReason: OrchestrationWorkerReadFallbackReason | null warnings: string[] diff --git a/src/shared/protocol-version.ts b/src/shared/protocol-version.ts index ddaa9a6fc53..15d376192e1 100644 --- a/src/shared/protocol-version.ts +++ b/src/shared/protocol-version.ts @@ -45,6 +45,8 @@ export const ORCHESTRATION_FEDERATION_CONTROL_MAIL_RUNTIME_CAPABILITY = 'orchestration.federation-control-mail.v1' as const export const ORCHESTRATION_FEDERATION_LIFECYCLE_SETTLEMENT_RUNTIME_CAPABILITY = 'orchestration.federation-lifecycle-settlement.v1' as const +export const ORCHESTRATION_WORKER_STOP_VERDICT_RUNTIME_CAPABILITY = + 'orchestration.worker-stop-verdict.v1' as const export const ORCHESTRATION_WORKER_LAUNCH_PREFERENCES_RUNTIME_CAPABILITY = 'orchestration.worker-launch-preferences.v1' as const export const ORCHESTRATION_FEDERATION_CONTROL_MAIL_PROTOCOL_VERSION = 2 as const @@ -115,6 +117,7 @@ export const RUNTIME_CAPABILITIES = [ ORCHESTRATION_FEDERATION_RUNTIME_CAPABILITY, ORCHESTRATION_FEDERATION_CONTROL_MAIL_RUNTIME_CAPABILITY, ORCHESTRATION_FEDERATION_LIFECYCLE_SETTLEMENT_RUNTIME_CAPABILITY, + ORCHESTRATION_WORKER_STOP_VERDICT_RUNTIME_CAPABILITY, ORCHESTRATION_WORKER_LAUNCH_PREFERENCES_RUNTIME_CAPABILITY, ORCHESTRATION_CONTRACT_RUNTIME_CAPABILITY, BROWSER_SCREENCAST_RUNTIME_CAPABILITY, diff --git a/src/shared/pty-liveness-verdict.ts b/src/shared/pty-liveness-verdict.ts new file mode 100644 index 00000000000..f16e756ca9a --- /dev/null +++ b/src/shared/pty-liveness-verdict.ts @@ -0,0 +1,34 @@ +/** + * The one vocabulary Orca uses to talk about whether a PTY is live. + * + * `exited` requires positive evidence of absence from the owning host. Losing + * contact with that host — an unregistered SSH provider, a dropped relay, an + * inventory that only enumerates registered providers — is `unverifiable`, never + * a death certificate and never a successful stop. + */ +export type PtyLivenessVerdict = + | { status: 'exited' } + | { status: 'live'; ptyIds: string[] } + | { status: 'unverifiable'; reason: string } + +export const SSH_PROVIDER_UNREGISTERED_REASON = 'its SSH provider is no longer registered' +export const NO_OBSERVING_PROVIDER_REASON = 'no registered provider can observe its host' +export const SSH_EXIT_UNCONFIRMED_REASON = 'the owning SSH host did not confirm the PTY exit' +export const PTY_LIVE_NOTE = 'The PTY is live.' + +/** The one sentence every surface uses to admit a stop was not confirmed. */ +export function describeUnconfirmedStop(reason: string): string { + return `The PTY was not confirmed stopped: ${reason}.` +} + +/** Words a close whose PTY teardown was never confirmed, for a stop receipt. */ +export function describeUnconfirmedAgentStop(close: { + ptyStopVerdict?: 'live' | 'unverifiable' + ptyStopReason?: string +}): string { + const detail = + close.ptyStopVerdict === 'live' + ? 'it is live' + : (close.ptyStopReason ?? 'the stop outcome could not be verified') + return `The agent terminal was closed but its process could not be confirmed stopped: ${detail}.` +} diff --git a/src/shared/runtime-types.ts b/src/shared/runtime-types.ts index fa2e28bc5bc..a2fa36ecadb 100644 --- a/src/shared/runtime-types.ts +++ b/src/shared/runtime-types.ts @@ -734,6 +734,14 @@ export type RuntimeTerminalClose = { /** Present for the durable whole-tab lifecycle without changing legacy receipts. */ closeMode?: 'tab' ptyKilled: boolean + /** + * Why the PTY was not killed, when we know. Absent means today's answer — + * nothing observed either way — so older clients reading only `ptyKilled` are + * unaffected. `exited` never appears here: that is what `ptyKilled` reports. + */ + ptyStopVerdict?: 'live' | 'unverifiable' + /** Set with `ptyStopVerdict: 'unverifiable'`; names what we lost contact with. */ + ptyStopReason?: string } export type RuntimeTerminalWaitCondition = 'exit' | 'tui-idle'