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'