diff --git a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts index b89468c77a0..5e0f5f5b142 100644 --- a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts +++ b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-authority.ts @@ -160,12 +160,66 @@ export function prepareStartingWorkerAuthority( } } +/** + * Custody for an agent terminal this worker-start just created, recorded at creation instead of + * after the agent boot wait. Until the row exists a keystroke into the booting pane finds no + * ownership to flip, so the takeover is silently dropped and a later `worker-release` closes the + * pane under the user. + * + * Ownership of a pane only; the Dispatch capability stays behind the boot wait, because authority + * must not be handed to a process that has not come up. + */ +export function recordCreatedWorkerTerminalCustody( + this: OrchestrationDb, + params: { + dispatchId: string + handle: string + paneKey: string + processIncarnation: string + worktreeId: string + hostScope?: string | null + } +): void { + this.db.exec('BEGIN IMMEDIATE') + try { + // Same guard as prepareStartingWorkerAuthority, read inside the transaction: a dispatch stopped + // while the terminal was being created must not acquire an owner. + const dispatch = this.getDispatchContextById(params.dispatchId) + const worker = this.getWorkerDispatch(params.dispatchId) + if (!dispatch || dispatch.status !== 'pending' || worker?.state !== 'starting') { + throw new OrchestrationError( + 'dispatch_inactive', + `Dispatch ${params.dispatchId} is not starting.` + ) + } + if (!this.getWorkerTerminalResourceByOwner(params.dispatchId)) { + this.createWorkerTerminalResourceStatement({ + dispatchId: params.dispatchId, + worktreeId: params.worktreeId, + terminalHandle: params.handle, + paneKey: params.paneKey, + processIncarnation: params.processIncarnation, + endpointId: worker.runtime_epoch, + endpointIncarnation: params.processIncarnation, + hostScope: params.hostScope, + ownership: 'owned' + }) + } + this.db.exec('COMMIT') + } catch (error) { + this.db.exec('ROLLBACK') + throw error + } +} + export type WorkerDispatchAuthorityMethods = { prepareStartingWorkerAuthority: typeof prepareStartingWorkerAuthority + recordCreatedWorkerTerminalCustody: typeof recordCreatedWorkerTerminalCustody } export function attachWorkerDispatchAuthority(ctor: { prototype: object }): void { Object.assign(ctor.prototype, { - prepareStartingWorkerAuthority + prepareStartingWorkerAuthority, + recordCreatedWorkerTerminalCustody }) } diff --git a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts index 5ff97f83fd9..6544f519497 100644 --- a/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts +++ b/src/main/runtime/orchestration/db/worker-dispatch/worker-dispatch-outcome.ts @@ -2,10 +2,7 @@ import type { WorkerDispatchRow } from '../../types' import { OrchestrationError } from '../../orchestration-error' import type { OrchestrationDb } from '../orchestration-db' import { transitionLifecycleWithDb } from '../lifecycle-transition' -import { - adoptFailedStartTerminal, - type FailedStartTerminalAdoption -} from '../worker-terminal/failed-start-terminal-adoption' +import { recordFailedStartDispatchIdentity } from '../worker-terminal/failed-start-dispatch-identity' export function markWorkerDispatchReady( this: OrchestrationDb, @@ -51,11 +48,7 @@ export function failWorkerStart( // Why (#16095): revocation exists to stop a worker acting on a dispatch that never landed. A // prompt whose turn start went unobserved provably landed, so its worker keeps the authority its // own report needs. - options: { - retainCapability?: boolean - /** A start that died before authority attached still owns the terminal it created. */ - adoptResidualTerminal?: FailedStartTerminalAdoption - } = {} + options: { retainCapability?: boolean } = {} ): WorkerDispatchRow { this.db.exec('BEGIN IMMEDIATE') try { @@ -104,11 +97,7 @@ export function failWorkerStart( }) } this.closeQuestionsForDispatch(dispatchId) - adoptFailedStartTerminal( - this, - this.getWorkerDispatch(dispatchId) as WorkerDispatchRow, - options.adoptResidualTerminal - ) + recordFailedStartDispatchIdentity(this, this.getWorkerDispatch(dispatchId) as WorkerDispatchRow) this.db.exec('COMMIT') return this.getWorkerDispatch(dispatchId) as WorkerDispatchRow } catch (error) { diff --git a/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts b/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts new file mode 100644 index 00000000000..671b12c39a7 --- /dev/null +++ b/src/main/runtime/orchestration/db/worker-terminal/failed-start-dispatch-identity.ts @@ -0,0 +1,34 @@ +import type { WorkerDispatchRow } from '../../types' +import type { OrchestrationDb } from '../orchestration-db' + +/** + * A start that dies before `prepareStartingWorkerAuthority` never filled the Dispatch context in, + * and release re-proves identity through it — so the custody row written at terminal creation would + * name a pane no release path could match. Copy that identity across. + * + * `capability_hash` stays null, so this grants nothing: it records which pane the Dispatch owns. + * + * No transaction: composes inside `failWorkerStart`'s. + */ +export function recordFailedStartDispatchIdentity( + db: OrchestrationDb, + worker: WorkerDispatchRow +): void { + const resource = db.getWorkerTerminalResourceByOwner(worker.dispatch_id) + if (!resource || resource.terminal_handle !== worker.agent_terminal_handle) { + return + } + db.db + .prepare( + `UPDATE dispatch_contexts + SET assignee_handle = ?, assignee_pane_key = ?, process_incarnation = ?, host_scope = ? + WHERE id = ? AND status = 'failed' AND capability_hash IS NULL` + ) + .run( + resource.terminal_handle, + resource.pane_key, + resource.process_incarnation, + resource.host_scope, + worker.dispatch_id + ) +} diff --git a/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts b/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts deleted file mode 100644 index 607ae9f45a7..00000000000 --- a/src/main/runtime/orchestration/db/worker-terminal/failed-start-terminal-adoption.ts +++ /dev/null @@ -1,68 +0,0 @@ -import type { WorkerDispatchRow } from '../../types' -import type { OrchestrationDb } from '../orchestration-db' - -/** Identity of a terminal this worker-start created and never handed to an owner. */ -export type FailedStartTerminalAdoption = { - terminalHandle: string - worktreeId: string | null - paneKey: string - processIncarnation: string - hostScope?: string | null -} - -/** - * A start that dies before `prepareStartingWorkerAuthority` leaves the terminal it created with no - * owner, so no release path can ever close it and the fleet can only say `inspect`. Record the - * ownership the successful path would have recorded, so ordinary `worker-release` owns the cleanup. - * - * No transaction: composes inside `failWorkerStart`'s. - */ -export function adoptFailedStartTerminal( - db: OrchestrationDb, - worker: WorkerDispatchRow, - adoption: FailedStartTerminalAdoption | undefined -): void { - if (!adoption || worker.agent_terminal_handle !== adoption.terminalHandle) { - return - } - if (db.getWorkerTerminalResourceByOwner(worker.dispatch_id)) { - return - } - // A second owner for one process could close it twice, or close a terminal already handed on. - const conflict = db.db - .prepare( - `SELECT 1 FROM worker_terminal_resources - WHERE ownership_state <> 'released' - AND (terminal_handle = ? OR process_incarnation = ?) LIMIT 1` - ) - .get(adoption.terminalHandle, adoption.processIncarnation) - if (conflict) { - return - } - db.createWorkerTerminalResourceStatement({ - dispatchId: worker.dispatch_id, - worktreeId: adoption.worktreeId ?? worker.worktree_id, - terminalHandle: adoption.terminalHandle, - paneKey: adoption.paneKey, - processIncarnation: adoption.processIncarnation, - endpointId: worker.runtime_epoch ?? null, - endpointIncarnation: adoption.processIncarnation, - hostScope: adoption.hostScope ?? null, - ownership: 'owned' - }) - // Release re-proves identity through the Dispatch context, which a failed start never filled in. - // This records which pane the Dispatch owns; `capability_hash` stays null, so it grants nothing. - db.db - .prepare( - `UPDATE dispatch_contexts - SET assignee_handle = ?, assignee_pane_key = ?, process_incarnation = ?, host_scope = ? - WHERE id = ? AND status = 'failed' AND capability_hash IS NULL` - ) - .run( - adoption.terminalHandle, - adoption.paneKey, - adoption.processIncarnation, - adoption.hostScope ?? null, - worker.dispatch_id - ) -} diff --git a/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts b/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts deleted file mode 100644 index b980a7f2a25..00000000000 --- a/src/main/runtime/orchestration/failed-start-terminal-adoption.test.ts +++ /dev/null @@ -1,157 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest' -import { OrchestrationDb } from './db' - -const HANDLE = 'term_residual' -const PANE_KEY = 'tab_residual:leaf_residual' -const INCARNATION = 'runtime:pty-residual:1' - -describe('a start that fails before authority still owns the terminal it created', () => { - let db: OrchestrationDb | undefined - - afterEach(() => { - db?.close() - }) - - /** Replays the shipping order: readiness stage records the handle, then the wait fails. */ - function failStartAfterCreatingTerminal( - adoption?: Parameters[3] - ): { db: OrchestrationDb; dispatchId: string } { - const d = (db = new OrchestrationDb(':memory:')) - const task = d.createTask({ runId: 'run_legacy_local', spec: 'residual terminal' }) - const started = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: task.id, - startOptions: {} - }) - const effects = [ - { kind: 'terminal', role: 'agent', action: 'created', id: HANDLE, surface: 'visible' } - ] - d.recordWorkerStage({ - dispatchId: started.dispatch.id, - stage: 'terminal_readying', - worktreeId: 'repo::worktree', - terminalHandle: HANDLE, - effects, - residualResources: effects - }) - d.failWorkerStart( - started.dispatch.id, - 'agent_readiness', - 'Agent startup blocked: codex-interactive-prompt', - adoption - ) - return { db: d, dispatchId: started.dispatch.id } - } - - const adoption = { - adoptResidualTerminal: { - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: null - } - } - - it('leaves nothing that can close the terminal when the start is not adopted', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal() - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toBeUndefined() - expect(d.requestWorkerTerminalRelease(dispatchId)).toMatchObject({ - disposition: 'retained', - reason: 'no_owned_resource' - }) - }) - - it('records the ownership the successful path would have recorded', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toMatchObject({ - owner_dispatch_id: dispatchId, - terminal_handle: HANDLE, - pane_key: PANE_KEY, - process_incarnation: INCARNATION, - ownership_state: 'owned', - release_state: 'not_requested' - }) - }) - - it('lets worker-release proceed on the failed dispatch', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.requestWorkerTerminalRelease(dispatchId)).toMatchObject({ - disposition: 'requested', - resource: { release_state: 'requested' } - }) - }) - - it('re-proves identity through the dispatch context release reads', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect( - d.isDispatchProcessCurrent({ dispatchId, paneKey: PANE_KEY, processIncarnation: INCARNATION }) - ).toBe(true) - // Adoption records which pane the dispatch owns; it never restores authority over it. - expect(d.getDispatchContextById(dispatchId)).toMatchObject({ - status: 'failed', - capability_hash: null - }) - expect(d.getDispatchContextById(dispatchId)?.capability_revoked_at).not.toBeNull() - }) - - it('publishes the terminal as reclaimable so the fleet names release', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal(adoption) - - expect(d.listWorkerTerminalResources({ dispatchIds: [dispatchId] })[0]).toMatchObject({ - agentTerminalHandle: HANDLE, - terminalState: 'reclaimable' - }) - }) - - it('never claims a terminal the durable row does not name', () => { - const { db: d, dispatchId } = failStartAfterCreatingTerminal({ - adoptResidualTerminal: { ...adoption.adoptResidualTerminal, terminalHandle: 'term_other' } - }) - - expect(d.getWorkerTerminalResourceByOwner(dispatchId)).toBeUndefined() - }) - - it('never claims a terminal another live resource already accounts for', () => { - const d = (db = new OrchestrationDb(':memory:')) - const first = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: d.createTask({ runId: 'run_legacy_local', spec: 'owner' }).id, - startOptions: {} - }) - d.prepareStartingWorkerAuthority({ - dispatchId: first.dispatch.id, - handle: HANDLE, - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - worktreeId: 'repo::worktree', - setupState: 'not_applicable', - effects: [], - terminalOwnership: 'created' - }) - const second = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: d.createTask({ runId: 'run_legacy_local', spec: 'claimant' }).id, - startOptions: {} - }) - d.recordWorkerStage({ - dispatchId: second.dispatch.id, - stage: 'terminal_readying', - terminalHandle: HANDLE - }) - - d.failWorkerStart(second.dispatch.id, 'agent_readiness', 'blocked', adoption) - - expect(d.getWorkerTerminalResourceByOwner(second.dispatch.id)).toBeUndefined() - expect(d.getWorkerTerminalResourceByOwner(first.dispatch.id)).toMatchObject({ - ownership_state: 'owned' - }) - }) -}) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts b/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts new file mode 100644 index 00000000000..c73884b1d96 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/worker/created-worker-terminal-custody.ts @@ -0,0 +1,32 @@ +import type { OrcaRuntimeService } from '../../../../orca-runtime' +import type { OrchestrationDb } from '../../../../orchestration/db' +import { requireWorkerAuthority } from './worker-topology' + +/** + * Custody for an agent terminal this start created, recorded when the terminal exists rather than + * after the agent boot wait: a keystroke into the booting pane has to find an `owned` row to flip, + * or the takeover is dropped and a later `worker-release` closes the pane under the user. + * + * Ownership of a pane only. The Dispatch capability still waits for the agent to come up. + * + * `created` is false for an explicit `--terminal` reuse, which is the caller's own pane, and for a + * structured session, which reaches its authority in this same turn and so has no gap to close. + */ +export function recordCreatedWorkerTerminalCustody( + runtime: OrcaRuntimeService, + stage: { db: OrchestrationDb; dispatchId: string; worktreeId: string; terminalHandle: string }, + created: boolean +): void { + if (!created) { + return + } + const authority = requireWorkerAuthority(runtime, stage.terminalHandle) + stage.db.recordCreatedWorkerTerminalCustody({ + dispatchId: stage.dispatchId, + handle: stage.terminalHandle, + paneKey: authority.paneKey, + processIncarnation: authority.processIncarnation, + worktreeId: stage.worktreeId, + hostScope: authority.hostScope ?? null + }) +} diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts deleted file mode 100644 index 413a397462b..00000000000 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.test.ts +++ /dev/null @@ -1,191 +0,0 @@ -import { afterEach, describe, expect, it } from 'vitest' -import type { OrcaRuntimeService } from '../../../../orca-runtime' -import { OrchestrationDb } from '../../../../orchestration/db' -import { resolveResidualAgentTerminal } from './failed-start-residual-terminal' -import { failWorkerStartWithReceipt } from './worker-start-receipt' -import type { WorkerEffect } from './worker-topology' - -const HANDLE = 'term_residual' -const PANE_KEY = 'tab_residual:leaf_residual' -const INCARNATION = 'pty-residual:1' - -const createdAgentTerminal: WorkerEffect = { - kind: 'terminal', - role: 'agent', - action: 'created', - id: HANDLE, - surface: 'visible' -} - -function createRuntime(overrides: Partial> = {}): OrcaRuntimeService { - return { - getOrchestrationDispatchAuthority: () => ({ - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: { kind: 'local', hostId: 'local' } - }), - getTerminalPaneKey: () => PANE_KEY, - getTerminalProcessIncarnation: () => INCARNATION, - ...overrides - } as unknown as OrcaRuntimeService -} - -describe('residual agent terminal left by a failed start', () => { - it('resolves identity for a terminal this start created', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: 'repo::worktree' - }) - ).toEqual({ - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: JSON.stringify({ kind: 'local', hostId: 'local' }) - }) - }) - - it('resolves the agent-first worktree terminal the same way', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, action: 'reused_agent_terminal' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toMatchObject({ terminalHandle: HANDLE }) - }) - - it('never claims a caller-supplied terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, action: 'reused' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('never claims a setup terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [{ ...createdAgentTerminal, role: 'setup' }], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('refuses a pane whose process cannot be identified', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime({ - getOrchestrationDispatchAuthority: () => null, - getTerminalProcessIncarnation: () => null - }), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('refuses when the start never resolved a terminal', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime(), - effects: [], - terminalHandle: undefined, - worktreeId: null - }) - ).toBeUndefined() - }) - - it('stays silent when identity resolution throws', () => { - expect( - resolveResidualAgentTerminal({ - runtime: createRuntime({ - getOrchestrationDispatchAuthority: () => { - throw new Error('handle retired') - } - }), - effects: [createdAgentTerminal], - terminalHandle: HANDLE, - worktreeId: null - }) - ).toBeUndefined() - }) -}) - -describe('failed worker-start receipt for a residual terminal', () => { - let db: OrchestrationDb | undefined - - afterEach(() => { - db?.close() - }) - - function failStart(residual: boolean): { recovery?: string } { - const d = (db = new OrchestrationDb(':memory:')) - const task = d.createTask({ runId: 'run_legacy_local', spec: 'residual receipt' }) - const started = d.createStartingWorkerDispatch({ - creator: { kind: 'system' }, - maxDepth: Number.MAX_SAFE_INTEGER, - taskId: task.id, - startOptions: {} - }) - d.recordWorkerStage({ - dispatchId: started.dispatch.id, - stage: 'terminal_readying', - terminalHandle: HANDLE, - effects: [createdAgentTerminal], - residualResources: [createdAgentTerminal] - }) - return failWorkerStartWithReceipt({ - db: d, - mode: { - mode: 'terminal', - preferred: 'terminal', - reason: 'user_default', - detail: 'terminal by default' - } as const, - runId: 'run_residual', - taskId: task.id, - dispatchId: started.dispatch.id, - failedStage: 'agent_readiness', - error: new Error('Agent startup blocked: codex-interactive-prompt'), - setup: { - requested: 'not_applicable', - effective: 'not_applicable', - source: 'existing_worktree', - hookFound: false, - startupPolicy: 'start-immediately', - state: 'not_applicable' - }, - launch: { requested: { agent: 'codex' }, effective: { agent: 'codex' } } as never, - ...(residual - ? { - residualAgentTerminal: { - terminalHandle: HANDLE, - worktreeId: 'repo::worktree', - paneKey: PANE_KEY, - processIncarnation: INCARNATION, - hostScope: null - } - } - : {}) - }) as { recovery?: string } - } - - it('names worker-release for the terminal it left behind', () => { - expect(failStart(true).recovery).toContain('worker-release') - }) - - it('promises no cleanup when there is no residual terminal', () => { - expect(failStart(false).recovery).toBeUndefined() - }) -}) diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts deleted file mode 100644 index e42923e93a9..00000000000 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-start-residual-terminal.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { OrcaRuntimeService } from '../../../../orca-runtime' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' -import type { WorkerEffect } from './worker-topology' - -/** True only for an agent terminal this worker-start brought into existence. An explicit - * `--terminal` reuse records `reused` and is never residual — it is the caller's terminal. */ -function orchestrationCreatedAgentTerminal( - effects: readonly WorkerEffect[], - handle: string -): boolean { - return effects.some( - (effect) => - effect.kind === 'terminal' && - effect.role === 'agent' && - effect.id === handle && - (effect.action?.startsWith('created') === true || effect.action === 'reused_agent_terminal') - ) -} - -/** - * Identity for the terminal a failed start leaves behind, so the failed Dispatch can own it and - * `worker-release` can close it. Returns nothing unless the pane and process are both provable: - * an unprovable identity must never authorize a later close. - */ -export function resolveResidualAgentTerminal(args: { - runtime: OrcaRuntimeService - effects: readonly WorkerEffect[] - terminalHandle: string | undefined - worktreeId: string | null -}): FailedStartTerminalAdoption | undefined { - const handle = args.terminalHandle - if (!handle || !orchestrationCreatedAgentTerminal(args.effects, handle)) { - return undefined - } - try { - const authority = args.runtime.getOrchestrationDispatchAuthority(handle) - const paneKey = authority?.paneKey ?? args.runtime.getTerminalPaneKey(handle) - const processIncarnation = - authority?.processIncarnation ?? args.runtime.getTerminalProcessIncarnation(handle) - if (!paneKey || !processIncarnation) { - return undefined - } - return { - terminalHandle: handle, - worktreeId: args.worktreeId, - paneKey, - processIncarnation, - hostScope: authority?.hostScope ? JSON.stringify(authority.hostScope) : null - } - } catch { - return undefined - } -} diff --git a/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts b/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts index 32827377b54..250b639fc60 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/failed-worker-start-teardown.ts @@ -3,40 +3,27 @@ import { discardStructuredWorkerSession, releaseStructuredWorkerSession } from '../../orchestration-structured-worker-session' -import { resolveResidualAgentTerminal } from './failed-start-residual-terminal' import type { createStructuredWorkerSessionForWorktree } from './worker-topology' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' /** - * Undoes what a start created before it failed, and reports what `worker-release` still owns. + * Undoes what a start created before it failed. * * A start that never reached ready leaves no settlement to release the hold later, and its session * was already published as a chat tab — without the discard, a failed start strands a dead chat tab * that the durable restore index republishes on every app launch. Both halves are best-effort by * construction, so neither can replace the real error. + * + * A created PTY terminal is deliberately NOT torn down: its custody row was written at creation, so + * `worker-release` on the failed Dispatch owns that cleanup and the coordinator decides when. */ export async function tearDownFailedWorkerStart(args: { runtime: OrcaRuntimeService structuredSession: Awaited> | null dispatchId: string - effects: unknown[] - terminalHandle: string | undefined - worktreeId: string | null -}): Promise { +}): Promise { const { runtime, structuredSession } = args - // A structured session is torn down outright here, so it must never also be adopted as a residual - // terminal for `worker-release` to close a second time. - const residualAgentTerminal = structuredSession - ? undefined - : resolveResidualAgentTerminal({ - runtime, - effects: args.effects as never, - terminalHandle: args.terminalHandle, - worktreeId: args.worktreeId - }) releaseStructuredWorkerSession(args.dispatchId, runtime) if (structuredSession) { await discardStructuredWorkerSession(structuredSession.identity.sessionId, runtime) } - return residualAgentTerminal } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts index 48b14f9a84e..d67d09b766a 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/local-worker-start.ts @@ -19,6 +19,7 @@ import { failWorkerStartWithReceipt } from './worker-start-receipt' import { parseTaskDeps } from './task-deps-argument' import { assertExplicitWorkerTerminalUsable } from './explicit-worker-terminal-validation' import { deliverWorkerDispatchPreamble } from './deliver-worker-dispatch-preamble' +import { recordCreatedWorkerTerminalCustody } from './created-worker-terminal-custody' import { tearDownFailedWorkerStart } from './failed-worker-start-teardown' import { createExistingWorktreeWorkerTerminal, @@ -203,6 +204,7 @@ export async function startLocalWorker(args: { setup: setupReceipt, effects } + recordCreatedWorkerTerminalCustody(runtime, setupStage, !params.terminal && !structuredSession) if (persistGatedSetupSpawnFailure(setupStage)) { failedStage = 'setup_start' throw new Error('Setup terminal failed to start before the gated agent launch.') @@ -285,13 +287,10 @@ export async function startLocalWorker(args: { ...(terminalRevealWarning ? { warning: terminalRevealWarning } : {}) } } catch (error) { - const residualAgentTerminal = await tearDownFailedWorkerStart({ + await tearDownFailedWorkerStart({ runtime, structuredSession, - dispatchId: started.dispatch.id, - effects, - terminalHandle, - worktreeId: resolvedWorktree?.id ?? null + dispatchId: started.dispatch.id }) return failWorkerStartWithReceipt({ db, @@ -302,8 +301,7 @@ export async function startLocalWorker(args: { error, setup: setupReceipt, launch: launch.receipt, - mode, - ...(residualAgentTerminal ? { residualAgentTerminal } : {}) + mode }) } } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts index 9fd98dd9db3..f2dee00b44c 100644 --- a/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-start-receipt.ts @@ -4,8 +4,8 @@ import { isUnknownWorkerStartOutcome, type WorkerSetupReceipt } from './worker-t import type { OrchestrationWorkerLaunchReceipt } from './worker-launch-preferences' import type { WorkerStartModeReceipt } from '../../orchestration-worker-start-mode' import { isAgentSessionPtyWriteRefusedError } from '../../../../../../shared/agent-session-pty-write-admission' -import type { FailedStartTerminalAdoption } from '../../../../orchestration/db/worker-terminal/failed-start-terminal-adoption' import { structuredChatPtyWriteRefusalCopy } from '../../../../../../shared/agent-session-pty-write-refusal-copy' +import { isStructuredWorkerHandle } from '../../../../structured-worker-identity' export function failWorkerStartWithReceipt(args: { db: OrchestrationDb @@ -17,8 +17,6 @@ export function failWorkerStartWithReceipt(args: { setup: WorkerSetupReceipt launch: OrchestrationWorkerLaunchReceipt mode: WorkerStartModeReceipt - /** The terminal this start created and never handed to an owner. */ - residualAgentTerminal?: FailedStartTerminalAdoption }): unknown { const agentSessionRefusal = isAgentSessionPtyWriteRefusedError(args.error) ? args.error.refusal @@ -33,14 +31,14 @@ export function failWorkerStartWithReceipt(args: { : args.db.failWorkerStart(args.dispatchId, args.failedStage, reason, { // Why (#16095): the preamble is written before submission is verified, so a stalled // verdict never means the worker lacks its task — keep the authority its report needs. - retainCapability: isAgentPromptStalledError(args.error), - ...(args.residualAgentTerminal ? { adoptResidualTerminal: args.residualAgentTerminal } : {}) + retainCapability: isAgentPromptStalledError(args.error) }) - // Only claim cleanup the ownership table actually accepted; the adoption declines a terminal - // another resource already accounts for. - const adopted = - Boolean(args.residualAgentTerminal) && - Boolean(args.db.getWorkerTerminalResourceByOwner(args.dispatchId)) + // Only name cleanup this start actually left behind: a terminal it created and still owns. A + // structured session is discarded by the teardown, a pane the user typed into is theirs, and an + // unknown outcome is not settled — none of the three has anything for `worker-release` to close. + const residual = unknown ? undefined : args.db.getWorkerTerminalResourceByOwner(args.dispatchId) + const releasable = + residual?.ownership_state === 'owned' && !isStructuredWorkerHandle(residual.terminal_handle) return { runId: args.runId, taskId: args.taskId, @@ -55,7 +53,7 @@ export function failWorkerStartWithReceipt(args: { effects: JSON.parse(worker.effects) as unknown[], residualResources: JSON.parse(worker.residual_resources) as unknown[], ...(agentSessionRefusal ? { agentSessionRefusal } : {}), - ...(adopted + ...(releasable ? { recovery: `This start created a terminal that never ran the Task. Close it with: orca orchestration worker-release --dispatch ${args.dispatchId}` } diff --git a/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts b/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts new file mode 100644 index 00000000000..201201e5877 --- /dev/null +++ b/src/main/runtime/rpc/methods/orchestration/worker/worker-terminal-custody-at-creation.test.ts @@ -0,0 +1,216 @@ +/** + * Custody for an agent terminal this start created is written when the terminal is created, not + * after the agent boot wait. + * + * A worker pane is visible on desktop and phone the moment it exists. While the row was written + * only after `tui-idle` (up to 60 s later), a keystroke into the booting pane found no `owned` row, + * `markWorkerTerminalUserOwned` returned 0, and the takeover was lost — so a later `worker-release` + * closed the pane the user had claimed. + */ + +import { afterEach, describe, expect, it, vi } from 'vitest' +import { OrchestrationDb } from '../../../../orchestration/db' +import { createOrchestrationWorkerReleaseHarness } from './worker-release.test-support' + +const READY_WAIT = { + handle: 'term_worker', + condition: 'tui-idle', + satisfied: true, + status: 'running', + exitCode: null +} + +describe('worker terminal custody is recorded at terminal creation', () => { + const h = createOrchestrationWorkerReleaseHarness() + + afterEach(() => h.cleanup()) + + /** Holds the agent boot wait open so the mid-start database state can be read. */ + function holdBootWait(): { finish: (satisfied?: boolean) => void } { + const gate = h.deferred() + vi.spyOn(h.runtime, 'waitForTerminal').mockReturnValue(gate.promise as never) + return { + finish: (satisfied = true) => + gate.resolve({ ...READY_WAIT, satisfied, status: satisfied ? 'running' : 'exited' }) + } + } + + function startingDispatchId(): string { + return ( + h.db.db + .prepare("SELECT dispatch_id FROM worker_dispatches WHERE state = 'starting'") + .get() as { dispatch_id: string } + ).dispatch_id + } + + async function startHeldAtBootWait(options: { terminal?: string } = {}): Promise<{ + dispatchId: string + taskId: string + start: Promise + finish: (satisfied?: boolean) => void + }> { + const task = h.db.createTask({ spec: 'custody at creation', runId: h.activeRunId }) + const { finish } = holdBootWait() + const start = h.call('orchestration.workerStart', { + task: task.id, + from: 'term_coord', + ...(options.terminal ? { terminal: options.terminal } : { agent: 'codex' }) + }) + await vi.waitFor(() => expect(h.runtime.waitForTerminal).toHaveBeenCalled()) + return { dispatchId: startingDispatchId(), taskId: task.id, start, finish } + } + + it('owns the created terminal before the boot wait resolves', async () => { + h.setup() + const held = await startHeldAtBootWait() + + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'owned', + release_state: 'not_requested', + terminal_handle: 'term_worker', + pane_key: h.workerPaneKey, + process_incarnation: 'runtime_test:term_worker:1', + host_scope: JSON.stringify({ kind: 'local', hostId: 'local' }) + }) + // worker-list reads the same row: a booting worker now says `active`, not `retained`. + expect(h.db.listWorkerTerminalResources({ dispatchIds: [held.dispatchId] })[0]).toMatchObject({ + agentTerminalHandle: 'term_worker', + terminalState: 'active' + }) + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + }) + + it('claims nothing for an explicitly reused terminal until authority transfers it', async () => { + h.setup() + const held = await startHeldAtBootWait({ terminal: 'term_worker' }) + + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toBeUndefined() + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'external', + retained_reason: 'external_terminal' + }) + }) + + it('lets a keystroke during the boot wait take the pane, and release then retains it', async () => { + h.setup() + const held = await startHeldAtBootWait() + + await expect( + h.call('orchestration.workerTerminalUserInput', { paneKey: h.workerPaneKey }) + ).resolves.toEqual({ changed: 1 }) + + held.finish() + await expect(held.start).resolves.toMatchObject({ state: 'ready' }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'user_owned', + retained_reason: 'user_takeover' + }) + + h.settle(held.taskId, held.dispatchId, 'succeeded') + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).resolves.toMatchObject({ state: 'retained', reason: 'user_takeover', processAction: 'none' }) + expect(h.runtime.closeTerminal).not.toHaveBeenCalled() + }) + + it('still refuses to release a starting worker that already owns its terminal', async () => { + h.setup() + const held = await startHeldAtBootWait() + + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).rejects.toThrow(/only a settled worker can release/) + + held.finish() + await held.start + }) + + it('leaves a start that died on the boot wait a terminal worker-release can close', async () => { + h.setup() + const held = await startHeldAtBootWait() + held.finish(false) + + await expect(held.start).resolves.toMatchObject({ + state: 'failed', + failedStage: 'agent_readiness', + recovery: expect.stringContaining('worker-release') + }) + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toMatchObject({ + ownership_state: 'owned', + terminal_handle: 'term_worker' + }) + + await expect( + h.call('orchestration.workerRelease', { dispatch: held.dispatchId }) + ).resolves.toMatchObject({ state: 'released', processAction: 'closed_agent_terminal' }) + expect(h.runtime.closeTerminal).toHaveBeenCalledWith('term_worker') + }) + + it('promises no cleanup while the start outcome is still unknown', async () => { + h.setup() + const task = h.db.createTask({ spec: 'unknown outcome', runId: h.activeRunId }) + const unknown = Object.assign(new Error('the execution host went away'), { + code: 'operation_unknown' + }) + vi.spyOn(h.runtime, 'waitForTerminal').mockRejectedValue(unknown) + + const receipt = (await h.call('orchestration.workerStart', { + task: task.id, + from: 'term_coord', + agent: 'codex' + })) as { state: string; dispatchId: string; nextCommands?: string[] } + + expect(receipt).toMatchObject({ state: 'outcome_unknown' }) + // worker-release refuses an unsettled worker, so the receipt must not name it. + expect(receipt).not.toHaveProperty('recovery') + expect(receipt.nextCommands?.join(' ')).toContain('worker-abandon') + expect(h.db.getWorkerTerminalResourceByOwner(receipt.dispatchId)).toMatchObject({ + ownership_state: 'owned' + }) + }) + + it('promises no cleanup for a reused terminal whose start died', async () => { + h.setup() + const held = await startHeldAtBootWait({ terminal: 'term_worker' }) + held.finish(false) + + const receipt = await held.start + expect(receipt).toMatchObject({ state: 'failed' }) + expect(receipt).not.toHaveProperty('recovery') + expect(h.db.getWorkerTerminalResourceByOwner(held.dispatchId)).toBeUndefined() + }) +}) + +describe('custody refuses a dispatch that stopped while its terminal was being created', () => { + let db: OrchestrationDb | undefined + + afterEach(() => db?.close()) + + it('records no owner once the dispatch is no longer starting', () => { + const d = (db = new OrchestrationDb(':memory:')) + const started = d.createStartingWorkerDispatch({ + creator: { kind: 'system' }, + maxDepth: Number.MAX_SAFE_INTEGER, + taskId: d.createTask({ runId: 'run_legacy_local', spec: 'stopped mid-create' }).id, + startOptions: {} + }) + // Startup reconciliation abandons a `starting` worker whose terminal it cannot find. + d.reconcileMissingWorkerTerminal(started.dispatch.id, 'runtime restarted') + + expect(() => + d.recordCreatedWorkerTerminalCustody({ + dispatchId: started.dispatch.id, + handle: 'term_worker', + paneKey: 'tab_w:leaf_w', + processIncarnation: 'pty_w:1', + worktreeId: 'repo::worktree' + }) + ).toThrow(/is not starting/) + expect(d.getWorkerTerminalResourceByOwner(started.dispatch.id)).toBeUndefined() + }) +}) diff --git a/src/renderer/src/i18n/en-runtime-required.json b/src/renderer/src/i18n/en-runtime-required.json index 47024572e8c..a1c33b790bc 100644 --- a/src/renderer/src/i18n/en-runtime-required.json +++ b/src/renderer/src/i18n/en-runtime-required.json @@ -1503,16 +1503,7 @@ "ask_before_closing_running_terminals_description": "Show a confirmation before closing a terminal that has a running command or agent.", "ask_before_closing_running_terminals_title": "Ask Before Closing Running Terminals", "cc8c5ca224": "Windows default", - "d78fc4fdef": "Loading distributions", - "minimumContrast": { - "automatic": "Automatic: {{light}} on light backgrounds, {{dark}} on dark.", - "description": "Lifts terminal foreground colors that sit too close to the background. Leave blank for automatic, or set 1 to render program colors exactly as sent.", - "disabled": "Correction off. Programs that rely on low contrast, like Powerline separators, render as sent.", - "pinned": "Targets {{ratio}}:1 contrast for foreground colors, where possible.", - "placeholder": "Auto", - "suffix": "blank = automatic, 1 = off", - "title": "Minimum Contrast Ratio" - } + "d78fc4fdef": "Loading distributions" }, "TerminalSettingsPreview": { "d06664e889": "dark"