fix(ssh): preserve unconfirmed stop verdicts across consumers

This commit is contained in:
Brennan Benson
2026-08-16 18:37:27 -07:00
parent f48ccaf61b
commit 09fe2aae49
24 changed files with 562 additions and 61 deletions
+52 -2
View File
@@ -4,7 +4,9 @@ import {
registerPtyHandlers,
deletePtyOwnership,
setPtyOwnership,
getLocalPtyProvider
getLocalPtyProvider,
registerSshPtyProvider,
unregisterSshPtyProvider
} from './pty'
vi.mock('electron', () => import('./pty-ipc-mock-registry').then((m) => m.electronModuleMock()))
@@ -65,12 +67,14 @@ describe('stopping a PTY whose SSH provider is unregistered', () => {
setPtyController: ReturnType<typeof vi.fn>
onPtyExit: ReturnType<typeof vi.fn>
markPtyLivenessUnverifiable: ReturnType<typeof vi.fn>
markPtyLivenessLive: ReturnType<typeof vi.fn>
}
} {
const runtime = {
setPtyController: vi.fn(),
onPtyExit: vi.fn(),
markPtyLivenessUnverifiable: vi.fn()
markPtyLivenessUnverifiable: vi.fn(),
markPtyLivenessLive: vi.fn()
}
handlers.clear()
registerPtyHandlers(mainWindow as never, runtime as never)
@@ -119,4 +123,50 @@ describe('stopping a PTY whose SSH provider is unregistered', () => {
)
expect(runtime.markPtyLivenessUnverifiable).not.toHaveBeenCalled()
})
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('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)
}
})
})
+13
View File
@@ -5672,6 +5672,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)}`
)
@@ -5680,9 +5686,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)
@@ -218,6 +218,49 @@ describe('ClaudeAgentTeamsService', () => {
expect(api.closeTerminal).toHaveBeenCalledWith('teammate-1')
})
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('keeps the replacement addressable when the retired placeholder 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}', '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')
await request(['kill-pane', '-t', '%2'])
expect(api.closeTerminal).toHaveBeenLastCalledWith('teammate-2')
})
it('refuses to respawn the leader pane', async () => {
const { service, teamId, token, leaderPane, api } = createServiceWithLeader()
@@ -4,6 +4,7 @@ import {
tmuxSendKeysText,
tmuxValue
} from '../../shared/claude-agent-teams-tmux-compat'
import { describeUnconfirmedAgentStop } from '../../shared/pty-liveness-verdict'
import {
formatContext,
paneEnv,
@@ -172,9 +173,15 @@ export class ClaudeAgentTeamsTmuxDispatcher {
activate: false
})
try {
await api.closeTerminal(previousHandle)
const close = await api.closeTerminal(previousHandle)
if (!close.ptyKilled) {
pane.handle = split.handle
throw new Error(describeUnconfirmedAgentStop(close))
}
} catch (error) {
await api.closeTerminal(split.handle).catch(() => {})
if (pane.handle === previousHandle) {
await api.closeTerminal(split.handle).catch(() => {})
}
throw error
}
pane.handle = split.handle
@@ -266,7 +273,10 @@ 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))
}
team.panes.delete(pane.fakePaneId)
team.paneOrder = team.paneOrder.filter((id) => id !== pane.fakePaneId)
if (team.mainVertical?.lastColumnPane === pane.fakePaneId) {
@@ -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)
})
@@ -386,9 +386,29 @@ describe('terminal close and handle incarnation continuity', () => {
})
})
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())
@@ -401,7 +421,7 @@ describe('terminal close and handle incarnation continuity', () => {
expect(close.ptyStopReason).toBeUndefined()
})
it('falls back to kill when verified teardown rejects after retirement', async () => {
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'))
@@ -411,7 +431,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)
})
@@ -434,7 +458,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)
+65 -15
View File
@@ -1557,9 +1557,14 @@ function isAgentSessionOperationOutcomeUnknown(error: unknown): boolean {
)
}
// Bounded because verdicts outlive their pty records; oldest-first eviction.
// 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
@@ -3136,7 +3141,8 @@ export class OrcaRuntimeService {
// 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 ptyLivenessVerdictByPtyId = new Map<string, TrackedPtyLivenessVerdict>()
private ptyLivenessObservationSequence = 0
private readonly pairedRendererSessionOwnedPtyIds = new Set<string>()
private wslDistroByPtyId = new Map<string, string>()
private titleObservationSequence = 0
@@ -10458,6 +10464,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)
@@ -10491,6 +10498,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.
@@ -17544,9 +17552,13 @@ export class OrcaRuntimeService {
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) ?? null
return this.ptyLivenessVerdictByPtyId.get(ptyId)?.verdict ?? null
}
getTerminalLivenessVerdict(handle: string): PtyLivenessVerdict | null {
@@ -17561,17 +17573,35 @@ export class OrcaRuntimeService {
return
}
this.ptyLivenessVerdictByPtyId.delete(ptyId)
this.ptyLivenessVerdictByPtyId.set(ptyId, verdict)
this.ptyLivenessObservationSequence += 1
this.ptyLivenessVerdictByPtyId.set(ptyId, {
verdict,
observedAt: this.ptyLivenessObservationSequence
})
while (this.ptyLivenessVerdictByPtyId.size > MAX_TRACKED_PTY_LIVENESS_VERDICTS) {
const oldest = this.ptyLivenessVerdictByPtyId.keys().next()
if (oldest.done) {
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(oldest.value)
this.ptyLivenessVerdictByPtyId.delete(oldestOrphaned)
}
}
private forgetPtyLivenessVerdict(ptyId: string): void {
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)
}
@@ -28491,13 +28521,29 @@ 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) {
this.ptyController.kill(ptyId)
const verdict = this.getPtyLivenessVerdict(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
}
@@ -28707,7 +28753,7 @@ export class OrcaRuntimeService {
const ptyKilled = await this.stopExplicitlyClosedTabPtys(ptyIdsToKill, pty.pty.ptyId)
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.
@@ -31325,6 +31371,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
@@ -31409,6 +31456,8 @@ export class OrcaRuntimeService {
const allLivePtyIds = new Set(sessions.map((session) => session.id))
const selectedLivePtyIds = new Set<string>()
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)
@@ -31616,6 +31665,7 @@ export class OrcaRuntimeService {
if (pty) {
pty.connected = true
pty.disconnectedAt = null
this.forgetPtyLivenessVerdict(ptyId)
this.refreshPtyForegroundAgent(ptyId)
}
} else if (pty && !this.leafExistsForPty(ptyId)) {
@@ -9,6 +9,14 @@ import { getDefaultWorkspaceSession } from '../../shared/constants'
const WORKTREE_ID = 'repo-1::/tmp/inventory-verdict'
const REMOTE_PTY_ID = 'ssh:conn-1@@relay-9'
function deferred<T>() {
let resolve!: (value: T) => void
const promise = new Promise<T>((settle) => {
resolve = settle
})
return { promise, resolve }
}
function makeStore() {
const session = getDefaultWorkspaceSession()
return {
@@ -32,13 +40,16 @@ function makeStore() {
}
}
function makeRuntimeMissingFromInventory(hasPty: () => boolean | null): OrcaRuntimeService {
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: vi.fn(async () => []),
listProcesses,
getForegroundProcess: async () => null
} as never)
runtime.attachWindow(1)
@@ -67,4 +78,67 @@ describe('inventory sweep liveness verdicts', () => {
// 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('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'
})
})
})
@@ -51,7 +51,7 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [
handler: async (params, { runtime, authenticatedCallerFingerprint }) => {
requireHomeAttachment(runtime, params.dispatchId, authenticatedCallerFingerprint)
const observation = await inspectRemoteAttachment(runtime, params.dispatchId)
// Why `=== 'exited'` rather than `!== 'running'`: the other non-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') {
@@ -142,7 +142,7 @@ export const ORCHESTRATION_FEDERATION_CONTROL_METHODS: RpcMethod[] = [
}
try {
const close = await runtime.closeTerminal(observation.terminal.handle)
if (close.ptyStopVerdict) {
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(
@@ -202,7 +202,7 @@ async function inspectRemoteAttachment(
): Promise<{
terminal: Awaited<ReturnType<OrcaRuntimeService['showTerminal']>> | null
exact: boolean
status: 'unattached' | 'missing' | 'identity_changed' | 'running' | 'exited' | 'unverifiable'
status: 'unattached' | 'missing' | 'identity_changed' | 'live' | 'exited' | 'unverifiable'
/** Set with `unverifiable`; names what we lost contact with. */
reason?: string
}> {
@@ -233,7 +233,7 @@ async function inspectRemoteAttachment(
return {
terminal,
exact,
status: verdict?.status !== 'live' && terminal.connected === false ? 'exited' : 'running'
status: verdict?.status !== 'live' && terminal.connected === false ? 'exited' : 'live'
}
}
@@ -82,6 +82,23 @@ describe('federation host liveness verdicts', () => {
})
})
it('uses the canonical live verdict for an observed running 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 })
@@ -89,7 +106,7 @@ describe('federation host liveness verdicts', () => {
})
it('still serves output for a terminal we merely lost stop-contact with', async () => {
// Why this matters: the read gate used to be `status !== 'running'`, which
// 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,
@@ -132,6 +149,22 @@ describe('federation host liveness verdicts', () => {
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,
@@ -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' }
}
})
@@ -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',
@@ -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(
@@ -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()) {
@@ -10,7 +10,7 @@ export async function inspectWorkerTerminal(
): Promise<{
terminal: Awaited<ReturnType<OrcaRuntimeService['showTerminal']>> | null
exact: boolean
status: 'unattached' | 'missing' | 'identity_changed' | 'running' | 'exited' | 'unverifiable'
status: 'unattached' | 'missing' | 'identity_changed' | 'live' | 'exited' | 'unverifiable'
/** Set with `unverifiable`; names what we lost contact with. */
reason?: string
}> {
@@ -38,12 +38,12 @@ export async function inspectWorkerTerminal(
return { terminal, exact, status: 'unverifiable', reason: verdict.reason }
}
if (verdict?.status === 'live') {
return { terminal, exact, status: 'running' }
return { terminal, exact, status: 'live' }
}
return {
terminal,
exact,
status: terminal.connected === false ? 'exited' : 'running'
status: terminal.connected === false ? 'exited' : 'live'
}
}
@@ -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)) {
@@ -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}.`
)
})
})
@@ -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({
@@ -98,6 +98,49 @@ describe('worker-stop against a terminal we lost contact with', () => {
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 running 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()
@@ -106,7 +106,7 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [
// the outcome honestly, never a reason to stop trying to stop the worker.
if (
!observation.exact ||
(observation.status !== 'running' && observation.status !== 'unverifiable')
(observation.status !== 'live' && observation.status !== 'unverifiable')
) {
return unknownReceipt(
params.dispatch,
@@ -119,7 +119,7 @@ export const ORCHESTRATION_WORKER_STOP_METHODS: RpcMethod[] = [
}
try {
const close = await runtime.closeTerminal(handle)
if (close.ptyStopVerdict) {
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(
@@ -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(
@@ -73,6 +73,20 @@ export function unverifiableStopVerdict(
return null
}
export async function resolveUnstoppedPtyVerdict(
failedPtyIds: readonly string[],
provider: IPtyProvider,
sweepBudgetMs: number,
providerObservesOwningHost: boolean,
runtime?: OrcaRuntimeService
): Promise<UnstoppedPtyVerdict> {
if (failedPtyIds.length === 0) {
return { status: 'exited' }
}
const cached = providerObservesOwningHost ? null : unverifiableStopVerdict(failedPtyIds, runtime)
return cached ?? verifyUnstoppedPtys(failedPtyIds, provider, sweepBudgetMs)
}
/** Names the blocking PTYs so a wedged removal is diagnosable, not just refused. */
export function describeUnstoppedPtys(
worktreeId: string,
+45 -2
View File
@@ -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<typeof killAllProcessesForWorktree>[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<typeof killAllProcessesForWorktree>[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<boolean>((resolve) => {
+11 -12
View File
@@ -15,9 +15,7 @@ import {
describeError,
describeFailedPtySweep,
describeUnstoppedPtys,
unverifiableStopVerdict,
verifyUnstoppedPtys,
type UnstoppedPtyVerdict
resolveUnstoppedPtyVerdict
} from './unstopped-pty-verification'
// Why: normal inventories still coalesce into one process scan, while a stale
@@ -232,11 +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' }
: (unverifiableStopVerdict(failedPtyIds, deps.runtime) ??
(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)
@@ -365,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 */
}
+1 -1
View File
@@ -28,6 +28,6 @@ export function describeUnconfirmedAgentStop(close: {
const detail =
close.ptyStopVerdict === 'live'
? 'it is still running'
: (close.ptyStopReason ?? 'its host could not be reached')
: (close.ptyStopReason ?? 'the stop outcome could not be verified')
return `The agent terminal was closed but its process could not be confirmed stopped: ${detail}.`
}