mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
fix(orchestration): stop two surfaces lying about a worker with no terminal
`orca terminal <verb>` answered `terminal_handle_stale` for a structured worker's handle. Nothing went stale: the session is live and simply has no terminal, and it never had one — so callers acted on a false claim and went hunting for a remint that cannot exist. The refusal now carries its own code and names the structured equivalents (`orca terminal read`, `worker-read --source transcript`, `orca orchestration send`), so an agent that lands there learns what to run rather than what failed. A PTY handle that really did go stale keeps the old error, and so does a session this runtime no longer owns — that handle IS dead. `terminal.show` stays non-resolving: synthesising a ptyId/leafId/paneRuntimeId would hand every public terminal verb something that looks writable and is not. `orchestration-worker-specs.ts` promised "the same verbs, the same handle, and the same worker-read sources", and all three clauses were false for a worker with no terminal. A spec agents read must not carry a false promise, so it now states the limitation and the alternative that always works. Note this had to be reconciled with an invariant this branch already holds: the worker MODE must stay opaque, or a coordinator starts branching on something no verb it runs behaves differently for. So the note says "not every worker has a terminal" and points at `--source auto`/`--source transcript` WITHOUT naming a kind — the same mode-neutral wording `readStructuredWorkerOutput` already uses when it refuses `--source terminal`. Both properties are now pinned by tests, so neither can be restored by breaking the other.
This commit is contained in:
@@ -34,7 +34,8 @@ export const ORCHESTRATION_WORKER_COMMAND_SPECS: CommandSpec[] = [
|
||||
'New worktrees use agent-first creation and default --setup to run. Repository start-immediately runs setup beside the agent; wait-for-setup gates agent readiness and task input.',
|
||||
'Creation flags (--name, --repo, --base-branch, --display-name, --comment, --setup) are rejected for current/existing worktrees. Use exact --repo on the selected server; project/host convenience routing remains on worktree create.',
|
||||
"How the worker runs follows the user's own setting for new agent tabs; there is no flag for it and no caller needs to ask. A dispatch the setting cannot apply to still starts, so the placement, agent, and launch options passed here are always the ones honoured.",
|
||||
'Drive every worker the same way whichever way it was started: the same verbs, the same handle, and the same worker-read sources. The start receipt records which one ran, for operators and telemetry.',
|
||||
'Drive every worker the same way whichever way it was started: the same orchestration verbs, the same handle. Mail, dispatch, worker-show, worker-read and the whole lifecycle behave identically. The start receipt records which one ran, for operators and telemetry.',
|
||||
'Not every worker has a terminal. Read output with worker-read --source auto or --source transcript, which always work; --source terminal is refused when there is none, and orca terminal verbs do not accept every worker handle. Nothing above needs you to know which kind you have — the orchestration verbs cover all of them.',
|
||||
'--on selects only the worker server; the Run and this command remain on the current Orca server.',
|
||||
'Remote current and new-child are invalid; discover an exact remote selector or use new-top-level.',
|
||||
'--retry-of links the replacement attempt but does not inherit placement; repeat the intended --on/worktree and --agent/terminal choices.',
|
||||
|
||||
@@ -41,4 +41,17 @@ describe('orchestration worker-start command spec', () => {
|
||||
expect(notes).not.toMatch(/structured chat session/)
|
||||
expect(notes).toContain('Drive every worker the same way')
|
||||
})
|
||||
|
||||
it('does not promise uniformity it cannot deliver', () => {
|
||||
// The note used to promise "the same verbs, the same handle, and the same worker-read
|
||||
// sources". All three clauses were false for a worker with no terminal: `orca terminal` verbs
|
||||
// refuse its handle and `--source terminal` has nothing to serve. A spec agents read must not
|
||||
// carry a false promise — but it also must not name the worker kind, or a coordinator starts
|
||||
// branching on something no verb it runs behaves differently for. So it states the limitation
|
||||
// and the always-working alternative, without naming a mode.
|
||||
const notes = startSpec?.notes?.join('\n') ?? ''
|
||||
expect(notes).not.toContain('the same worker-read sources')
|
||||
expect(notes).toContain('Not every worker has a terminal')
|
||||
expect(notes).toContain('--source transcript')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -7,6 +7,7 @@ import { getLatestPtyTitle } from './runtime-worktree-status-projection'
|
||||
import { parsePaneKey } from '../../shared/stable-pane-id'
|
||||
import type { TerminalHandleRecord } from './runtime-terminal-contracts'
|
||||
import { readTerminalTail } from './terminal-tail-read'
|
||||
import { structuredWorkerTerminalRefusal } from './structured-worker-terminal-refusal'
|
||||
import { randomUUID } from 'node:crypto'
|
||||
|
||||
export class OrcaRuntimeWithBuildPtyTerminalSummary extends OrcaRuntimeWithGetPtyRecordForPaneKey {
|
||||
@@ -52,7 +53,11 @@ export class OrcaRuntimeWithBuildPtyTerminalSummary extends OrcaRuntimeWithGetPt
|
||||
this.assertGraphReady()
|
||||
const record = this.handles.get(handle)
|
||||
if (!record || record.runtimeId !== this.runtimeId) {
|
||||
throw new Error('terminal_handle_stale')
|
||||
// A structured worker's handle is not stale — nothing went dead. It names a live agent
|
||||
// session that simply has no terminal, and saying `terminal_handle_stale` sent callers
|
||||
// hunting for a remint that will never exist. Read paths (`terminal read`,
|
||||
// `isTerminalRunningAgent`, the identity probe) answer for it BEFORE reaching here.
|
||||
throw structuredWorkerTerminalRefusal(handle, this._orchestrationDb)
|
||||
}
|
||||
if (record.rendererGraphEpoch !== this.rendererGraphEpoch) {
|
||||
throw new Error('terminal_handle_stale')
|
||||
|
||||
@@ -89,6 +89,9 @@ const STRUCTURED_RUNTIME_PASSTHROUGH_CODES: ReadonlySet<string> = new Set([
|
||||
'dispatch_not_found',
|
||||
'dispatch_run_mismatch',
|
||||
'terminal_not_found',
|
||||
// A handle that names a live agent session with no terminal. Distinct from
|
||||
// `terminal_handle_stale`, which claims the handle went dead — nothing went stale here.
|
||||
'terminal_unsupported_for_agent_session',
|
||||
'recipient_ambiguous',
|
||||
'recipient_run_mismatch',
|
||||
'dispatch_inactive',
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { AgentSessionRecord } from '../../shared/agent-session-record'
|
||||
|
||||
const hostRef: { current: unknown } = { current: null }
|
||||
|
||||
vi.mock('../native-chat/agent-session-wire/structured-agent-session-registry', () => ({
|
||||
getStructuredAgentSessionHost: () => hostRef.current
|
||||
}))
|
||||
|
||||
const { structuredWorkerTerminalRefusal } = await import('./structured-worker-terminal-refusal')
|
||||
const {
|
||||
mintStructuredWorkerHandle,
|
||||
mintStructuredWorkerPaneKey,
|
||||
structuredWorkerIdentities,
|
||||
structuredWorkerProcessIncarnation
|
||||
} = await import('./structured-worker-identity')
|
||||
|
||||
const SESSION_ID = 'a1b2c3d4-e5f6-4a7b-8c9d-0e1f2a3b4c5d'
|
||||
|
||||
function installRecord(): void {
|
||||
hostRef.current = {
|
||||
deps: {
|
||||
store: {
|
||||
getRecord: (sessionId: string) =>
|
||||
({
|
||||
sessionId,
|
||||
location: { executionHostId: 'local', wslDistro: null },
|
||||
lease: {
|
||||
runtimeKind: 'native',
|
||||
claimStatus: 'live',
|
||||
runtimeFence: 1,
|
||||
deathEvidence: null
|
||||
}
|
||||
}) as unknown as AgentSessionRecord
|
||||
}
|
||||
},
|
||||
hasSession: () => true
|
||||
}
|
||||
}
|
||||
|
||||
function registerWorker(): string {
|
||||
const handle = mintStructuredWorkerHandle()
|
||||
structuredWorkerIdentities.register({
|
||||
handle,
|
||||
sessionId: SESSION_ID,
|
||||
agent: 'claude',
|
||||
paneKey: mintStructuredWorkerPaneKey(SESSION_ID),
|
||||
processIncarnation: structuredWorkerProcessIncarnation(SESSION_ID),
|
||||
worktreeId: 'wt_1',
|
||||
hostScope: { kind: 'local', hostId: 'local' }
|
||||
})
|
||||
return handle
|
||||
}
|
||||
|
||||
describe('the refusal a terminal verb gives a structured worker handle', () => {
|
||||
beforeEach(() => {
|
||||
structuredWorkerIdentities.clear()
|
||||
hostRef.current = null
|
||||
})
|
||||
|
||||
it('says the handle is an agent session, not that it went stale', () => {
|
||||
// `terminal_handle_stale` is a claim the handle died. It never did — the session is live and
|
||||
// has no terminal — so callers went looking for a remint that cannot exist.
|
||||
const handle = registerWorker()
|
||||
installRecord()
|
||||
const error = structuredWorkerTerminalRefusal(handle, null)
|
||||
expect(error.message).not.toContain('terminal_handle_stale')
|
||||
expect((error as { code?: string }).code).toBe('terminal_unsupported_for_agent_session')
|
||||
})
|
||||
|
||||
it('points at the structured equivalents rather than just failing', () => {
|
||||
const handle = registerWorker()
|
||||
installRecord()
|
||||
const message = structuredWorkerTerminalRefusal(handle, null).message
|
||||
expect(message).toContain('orca terminal read')
|
||||
expect(message).toContain('worker-read --source transcript')
|
||||
expect(message).toContain('orca orchestration send')
|
||||
})
|
||||
|
||||
it('keeps the stale error for a PTY handle, which really can go stale', () => {
|
||||
expect(structuredWorkerTerminalRefusal('term_gone', null).message).toBe('terminal_handle_stale')
|
||||
})
|
||||
|
||||
it('keeps the stale error for a session this runtime no longer owns', () => {
|
||||
// Once the lease moves or the session is released the handle IS dead, and saying so is right.
|
||||
registerWorker()
|
||||
hostRef.current = null
|
||||
expect(structuredWorkerTerminalRefusal('term_gone', null).message).toBe('terminal_handle_stale')
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* What a terminal verb should say when handed a structured worker's handle.
|
||||
*
|
||||
* `terminal_handle_stale` is a claim that the handle went dead, and for a structured worker it is
|
||||
* simply false: the session is live, it has no terminal, and it never had one. Callers acting on
|
||||
* that claim went looking for a remint that cannot exist. The refusal names the structured
|
||||
* equivalent instead, so an agent that lands here knows what to run rather than what failed.
|
||||
*
|
||||
* `terminal.show` stays non-resolving on purpose: synthesising a `ptyId`/`leafId`/`paneRuntimeId`
|
||||
* would hand every public terminal verb something that looks writable and is not.
|
||||
*/
|
||||
|
||||
import type { OrchestrationDb } from './orchestration/db'
|
||||
import { resolveStructuredWorkerAuthority } from './structured-worker-authority'
|
||||
|
||||
const TERMINAL_HANDLE_STALE = 'terminal_handle_stale'
|
||||
const AGENT_SESSION_HAS_NO_TERMINAL = 'terminal_unsupported_for_agent_session'
|
||||
|
||||
export function structuredWorkerTerminalRefusal(
|
||||
handle: string,
|
||||
db: OrchestrationDb | null | undefined
|
||||
): Error {
|
||||
if (!resolveStructuredWorkerAuthority(handle, db)) {
|
||||
return new Error(TERMINAL_HANDLE_STALE)
|
||||
}
|
||||
const error = new Error(
|
||||
`${handle} is an agent session, not a terminal, so terminal commands cannot address it. ` +
|
||||
'Read its output with `orca terminal read` or `orca orchestration worker-read --source transcript`, ' +
|
||||
'send it work with `orca orchestration send`, and open it from its chat tab.'
|
||||
)
|
||||
Object.assign(error, { code: AGENT_SESSION_HAS_NO_TERMINAL })
|
||||
return error
|
||||
}
|
||||
Reference in New Issue
Block a user