fix(ssh): stop reporting a confirmed kill when the SSH provider is gone

A detached relay PTY is designed to outlive the provider that addressed it
(it ignores SIGHUP and ships with an unlimited grace), so "the SSH provider
is no longer registered" is lost contact, never evidence the remote process
stopped. Both stop primitives in the PTY controller returned `true` from
that branch, and every caller downstream reported the fabricated success:
the CLI printed "PTY killed.", worker-stop settled the dispatch as stopped,
and — because the stop "succeeded" — the unstopped-PTY gate never ran, so
worktree removal walked straight past a live remote agent.

`kill`/`stopAndWait` now still tombstone the local lease but report an
unconfirmed stop and record why, using the three-verdict vocabulary the
worktree teardown gate already spoke (`live` / `unverifiable` / `exited`),
promoted out of that module into `src/shared/pty-liveness-verdict.ts`.
The close receipt, the CLI wording, worker-stop and the removal gate all
read that verdict instead of inferring an exit from silence.

The same rule fixes the mirror-image defect: the aggregate inventory only
enumerates registered providers, so a dropped relay clears `connected` for
every remote PTY at once. The sweep now separates the provider answering
"absent" (an exit) from no provider being able to answer (lost contact), so
worker-stop stops claiming `exited` from a disconnect.

The `connected` wire field is unchanged in meaning and shape.
This commit is contained in:
Brennan Benson
2026-08-16 16:29:10 -07:00
parent d6703552c9
commit 2f3f79c597
16 changed files with 631 additions and 23 deletions
+38 -1
View File
@@ -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 still known to be running', () => {
expect(
formatTerminalClose({
close: {
handle: 'term_live',
tabId: 'tab-1',
ptyKilled: false,
ptyStopVerdict: 'live'
}
})
).toBe('Closed terminal term_live. The PTY is still running.')
})
})
+16 -2
View File
@@ -1,3 +1,4 @@
import { PTY_STILL_RUNNING_NOTE, describeUnconfirmedStop } from '../shared/pty-liveness-verdict'
import type {
RuntimeTerminalClose,
RuntimeTerminalCreate,
@@ -170,12 +171,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_STILL_RUNNING_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 {
@@ -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',
+122
View File
@@ -0,0 +1,122 @@
import { describe, expect, it, vi } from 'vitest'
import { setupPtyIpcSuite } from './pty-ipc-test-harness'
import {
registerPtyHandlers,
deletePtyOwnership,
setPtyOwnership,
getLocalPtyProvider
} 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
stopAndWait: (ptyId: string, opts?: { deadlineMs?: number }) => Promise<boolean>
}
runtime: {
setPtyController: ReturnType<typeof vi.fn>
onPtyExit: ReturnType<typeof vi.fn>
markPtyLivenessUnverifiable: ReturnType<typeof vi.fn>
}
} {
const runtime = {
setPtyController: vi.fn(),
onPtyExit: vi.fn(),
markPtyLivenessUnverifiable: 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('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()
})
})
+10 -5
View File
@@ -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 { toSshExecutionHostId } from '../../shared/execution-host'
@@ -5518,12 +5519,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
}
@@ -5648,13 +5652,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
}
@@ -365,6 +365,42 @@ describe('terminal close and handle incarnation continuity', () => {
expect(harness.kill).not.toHaveBeenCalled()
})
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'
})
})
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
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('falls back to kill when verified teardown rejects after retirement', async () => {
const harness = createHarness()
const [{ handle }] = (await harness.runtime.listTerminals(`id:${WORKTREE_ID}`)).terminals
+101 -5
View File
@@ -657,6 +657,10 @@ import {
type RetiredTerminalSurface
} from './mobile-session-terminal-retirement'
import { retireTerminalSurfaceFromPersistence } from './mobile-session-terminal-persistence-retirement'
import {
NO_OBSERVING_PROVIDER_REASON,
type PtyLivenessVerdict
} from '../../shared/pty-liveness-verdict'
import {
advanceTerminalTopologyRevision,
hasHostAuthoritativeTerminalMembership
@@ -1553,6 +1557,9 @@ function isAgentSessionOperationOutcomeUnknown(error: unknown): boolean {
)
}
// Bounded because verdicts outlive their pty records; oldest-first eviction.
const MAX_TRACKED_PTY_LIVENESS_VERDICTS = 256
const AGENT_SESSION_OPERATION_PER_CLIENT_LIMIT = 512
const AGENT_SESSION_OPERATION_GLOBAL_LIMIT = 4_096
@@ -3125,6 +3132,11 @@ export class OrcaRuntimeService {
// iterates them all. Listeners are cleaned up via subscriptionCleanups.
private notificationListeners = new Set<(event: MobileNotificationEvent) => void>()
private ptysById = new Map<string, RuntimePtyWorktreeRecord>()
// 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<string, PtyLivenessVerdict>()
private readonly pairedRendererSessionOwnedPtyIds = new Set<string>()
private wslDistroByPtyId = new Map<string, string>()
private titleObservationSequence = 0
@@ -14830,6 +14842,11 @@ export class OrcaRuntimeService {
this.setPairedRendererSessionOwnership(pty.ptyId, false)
pty.disconnectedAt = Date.now()
pty.lastExitCode = exitCode
if (exitCode >= 0) {
// 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
@@ -17518,6 +17535,46 @@ 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 })
}
/** Null when nothing has been observed either way, so callers keep their own default. */
getPtyLivenessVerdict(ptyId: string): PtyLivenessVerdict | null {
return this.ptyLivenessVerdictByPtyId.get(ptyId) ?? 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.ptyLivenessVerdictByPtyId.set(ptyId, verdict)
while (this.ptyLivenessVerdictByPtyId.size > MAX_TRACKED_PTY_LIVENESS_VERDICTS) {
const oldest = this.ptyLivenessVerdictByPtyId.keys().next()
if (oldest.done) {
return
}
this.ptyLivenessVerdictByPtyId.delete(oldest.value)
}
}
private forgetPtyLivenessVerdict(ptyId: string): void {
this.ptyLivenessVerdictByPtyId.delete(ptyId)
}
getExactWorkerProviderSession(
handle: string,
observedAfter: number
@@ -28642,13 +28699,13 @@ 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
if (!ptyKilled || siblingCount <= 1) {
@@ -28666,7 +28723,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)
@@ -28689,7 +28746,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<RuntimeTerminalClose> {
@@ -31425,7 +31511,8 @@ export class OrcaRuntimeService {
continue
}
if (!allLivePtyIds.has(pty.ptyId) && !this.leafExistsForPty(pty.ptyId)) {
if (this.ptyController.hasPty?.(pty.ptyId) === true) {
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 (
@@ -31436,10 +31523,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()
@@ -0,0 +1,70 @@
import { describe, expect, it, vi } from 'vitest'
import { OrcaRuntimeService } from './orca-runtime'
import { getDefaultWorkspaceSession } from '../../shared/constants'
// 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 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): OrcaRuntimeService {
const runtime = new OrcaRuntimeService(makeStore() as never)
runtime.setPtyController({
write: () => true,
kill: () => true,
hasPty,
listProcesses: vi.fn(async () => []),
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 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()
})
})
@@ -10,7 +10,9 @@ export async function inspectWorkerTerminal(
): Promise<{
terminal: Awaited<ReturnType<OrcaRuntimeService['showTerminal']>> | null
exact: boolean
status: 'unattached' | 'missing' | 'identity_changed' | 'running' | 'exited'
status: 'unattached' | 'missing' | 'identity_changed' | 'running' | '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: 'running' }
}
return {
terminal,
exact,
status: exact ? (terminal.connected === false ? 'exited' : 'running') : 'identity_changed'
status: terminal.connected === false ? 'exited' : 'running'
}
}
@@ -0,0 +1,98 @@
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<string, unknown>) {
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 dispatch = createWorker()
await expect(
call('orchestration.workerShow', { dispatch: dispatch.id })
).resolves.toMatchObject({
observation: { status: 'unverifiable', exactWorker: true }
})
const stopped = (await call('orchestration.workerStop', { dispatch: dispatch.id })) as {
lastError: string
}
expect(stopped.lastError).toContain('could not be confirmed stopped')
expect(stopped.lastError).not.toContain('exited')
})
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.')
})
})
@@ -106,13 +106,35 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [
params.dispatch,
db.markWorkerStopUnknown(
params.dispatch,
`The recorded worker process is ${observation.status}; no terminal was closed.`
observation.status === 'unverifiable'
? // Why: losing the link to the worker's host is not a death certificate,
// so this must not settle as a stop or read as an observed exit.
`The recorded worker process could not be confirmed stopped: ${
observation.reason ?? 'its host could not be reached'
}; no terminal was closed.`
: `The recorded worker process is ${observation.status}; no terminal was closed.`
),
'none'
)
}
try {
const close = await runtime.closeTerminal(handle)
if (close.ptyStopVerdict) {
// 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,
`The agent terminal was closed but its process could not be confirmed stopped: ${
close.ptyStopVerdict === 'live'
? 'it is still running'
: (close.ptyStopReason ?? 'its host could not be reached')
}.`
),
'closed_agent_terminal'
)
}
const worker = db.settleWorkerStop(params.dispatch)
runtime.notifyMessageArrived(`dispatch:${params.dispatch}`, 'status')
return {
+21 -4
View File
@@ -1,18 +1,17 @@
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 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 +55,24 @@ 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
}
/** Names the blocking PTYs so a wedged removal is diagnosable, not just refused. */
export function describeUnstoppedPtys(
worktreeId: string,
@@ -175,6 +175,48 @@ 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<boolean>
) => 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('reports unverifiable separately from live when the process list fails', async () => {
const localProvider = createProviderStub(async () => {
throw new Error('daemon socket closed')
+3 -1
View File
@@ -15,6 +15,7 @@ import {
describeError,
describeFailedPtySweep,
describeUnstoppedPtys,
unverifiableStopVerdict,
verifyUnstoppedPtys,
type UnstoppedPtyVerdict
} from './unstopped-pty-verification'
@@ -234,7 +235,8 @@ export async function killAllProcessesForWorktree(
const verdict: UnstoppedPtyVerdict =
failedPtyIds.length === 0
? { status: 'exited' }
: await verifyUnstoppedPtys(failedPtyIds, deps.localProvider, sweepBudgetMs)
: (unverifiableStopVerdict(failedPtyIds, deps.runtime) ??
(await verifyUnstoppedPtys(failedPtyIds, deps.localProvider, sweepBudgetMs)))
if (verdict.status === 'exited') {
for (const ptyId of failedPtyIds) {
clearStoppedPtyState(ptyId, deps.onPtyStopped)
+22
View File
@@ -0,0 +1,22 @@
/**
* The one vocabulary Orca uses to talk about whether a PTY is still running.
*
* `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'
/** 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}.`
}
export const PTY_STILL_RUNNING_NOTE = 'The PTY is still running.'
+8
View File
@@ -717,6 +717,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'