From f981eeb8fd061bbbfa0ed7c13391a37b715b8be9 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Fri, 14 Aug 2026 23:36:25 -0600 Subject: [PATCH] fix(tui): refuse a worker cancel the runtime acknowledger cannot answer The cancel acknowledger resolves references against the root manager's direct children only. The TUI worker table lists every journal node, including nested descendants. A 'c' on a nested worker queued a request no acknowledger reads: the operation stays 'unknown' forever and the worker runs on, while the notice implied a cancel was in progress. - workerCancelRoute (top-model) resolves a worker id to its routable target: direct child, or the top-level lead of a nested descendant. - The TUI refuses a nested worker's cancel and names the lead to cancel instead of queueing a dead request. - The queued notice now says the operation awaits acknowledgement instead of printing the raw 'unknown' effect. - A kernel test pins the boundary: a request naming a live nested descendant is never acknowledged and never applied, and still reads 'unknown' after the run. - canonical-api.md names the direct-children-only boundary. --- docs/canonical-api.md | 2 +- src/tui/top-app.ts | 16 +++++- src/tui/top-model.ts | 34 ++++++++++++ tests/kernel/worker-cancellation.test.ts | 71 ++++++++++++++++++++++++ tests/tui/top-model.test.ts | 57 ++++++++++++++++++- 5 files changed, 177 insertions(+), 3 deletions(-) diff --git a/docs/canonical-api.md b/docs/canonical-api.md index 186ae9d8..d3088e93 100644 --- a/docs/canonical-api.md +++ b/docs/canonical-api.md @@ -118,7 +118,7 @@ A general "loop" primitive is the single most common modelling error in this rep | **Supervise agents to solve a graded `AgenticSurface` task** (workers `runAgentic` the surface, settle on its own check, driver self-improves from the failing tests) | `superviseSurface(profile, task, { surface, worker })`: `/kernel` | a worker-seam + a "self-improving supervisor" wrapper around `supervise()`; passing a custom `makeWorkerAgent` that runs `runAgentic` | | Run a profile through a topology shape over the keystone Supervisor, end-to-end | `runPersonified({ persona, shape, task, budget })`: `/kernel` | a hand-rolled `createSupervisor().run` + seam-wiring helper | | Address a supervisor run's durable state on disk, or steer a live worker from another process | `supervisorRunsRoot(root)` / `supervisorRunDir(root, id)` / `writeWorkerSteer(...)` / `readWorkerSteerRequests(...)`: `/kernel` — the `/.agent/supervisor/` contract `traces analyze --supervisor-run-dir` reads (`legacySupervisorRunDir` names the pre-rename `.loops` location for readers only) | inventing a run-dir layout, joining `.agent/supervisor` by hand, or writing a steer file whose shape no published reader knows | -| Cancel one worker from another process and read the acknowledged effect | `cancelWorker(eventDir, worker, operationId)` / `readWorkerCancellation(eventDir, operationId)`: `/kernel` — retry-safe by `operationId` lookup; the run's own turn loop applies the abort to exactly that worker's subtree and records `cancel_requested` → `cancelled` / `not_live` (reusing `RetainedRunEffect`), with the `terminated` set naming every node that died | writing an unread cancel file and calling it done, minting a second four-state cancellation vocabulary, killing the worker's process from outside, or treating a missing worker as a successful cancel | +| Cancel one worker from another process and read the acknowledged effect | `cancelWorker(eventDir, worker, operationId)` / `readWorkerCancellation(eventDir, operationId)`: `/kernel` — retry-safe by `operationId` lookup; the run's own turn loop applies the abort to exactly that worker's subtree and records `cancel_requested` → `cancelled` / `not_live` (reusing `RetainedRunEffect`), with the `terminated` set naming every node that died. Only the root manager's DIRECT children are addressable; a request naming a deeper descendant stays `unknown` — cancel its lead instead | writing an unread cancel file and calling it done, minting a second four-state cancellation vocabulary, killing the worker's process from outside, naming a nested descendant and waiting for an acknowledgement that cannot come, or treating a missing worker as a successful cancel | | Give a worker's clone the source workspace's untracked build artifacts | `withUntrackedArtifacts(ws, sourceDir)` wrapping the `Workspace`: `/kernel` | a post-materialize `cp -r`, a hardlink farm, or accepting that a bare `git clone` cannot build | | Expose what a settled worker shows the brain (failing verify tail + diff head + note, bounded) | `composeWorkerEvidence(...)` + `settledWorkerOut(...)` + `closingWorkerNote(...)`: `/kernel` | re-rolling truncation caps per consumer, or settling with bare counters the brain cannot act on | | Loop a worker over one evolving artifact, K rounds, stop-when-good | `loopUntil(seed, spec)` as the `shape`: `/kernel` | a `while(!done){runWorker();decide()}` hand-loop or "multi-attempt refine driver" | diff --git a/src/tui/top-app.ts b/src/tui/top-app.ts index 86d26091..eb46f262 100644 --- a/src/tui/top-app.ts +++ b/src/tui/top-app.ts @@ -26,6 +26,7 @@ import { type RenderTarget, renderTopFrameWithLayout, type TopSnapshot, + workerCancelRoute, } from './top-model' interface UiState { @@ -401,12 +402,25 @@ function requestCancel(): void { state.notice = `${worker.label} is ${worker.status}; nothing to cancel` return } + // The acknowledger resolves DIRECT children of the root manager only; a request naming a + // nested descendant would sit unanswered forever. Refuse it and name the routable lead + // instead of queueing an operation nothing will ever apply. + const route = workerCancelRoute(supervisor.workers, worker.id) + if (route.kind === 'nested') { + state.notice = + `${worker.label} is nested under lead ${route.lead.label}; ` + + `cancel ${route.lead.label} to cancel its whole subtree` + return + } try { const record = cancelWorker(supervisor.stateDir, worker.id, randomUUID(), { reason: 'operator requested cancel from TUI', source: 'agent-runtime-top', }) - state.notice = `cancel ${record.effect} for ${supervisor.id}/${worker.label} (op ${record.operationId})` + state.notice = + record.effect === 'unknown' + ? `cancel queued for ${supervisor.id}/${worker.label} (op ${record.operationId}); awaiting runtime acknowledgement` + : `cancel ${record.effect} for ${supervisor.id}/${worker.label} (op ${record.operationId})` } catch (err) { state.notice = `cancel request failed: ${err instanceof Error ? err.message : String(err)}` } diff --git a/src/tui/top-model.ts b/src/tui/top-model.ts index c06a6b74..78a89fad 100644 --- a/src/tui/top-model.ts +++ b/src/tui/top-model.ts @@ -419,6 +419,40 @@ function buildSupervisorView( } } +/** + * Where a worker-scoped cancel for `workerId` must go. + * + * The runtime's cancel acknowledger runs in the ROOT manager's turn loop and resolves references + * against its DIRECT children only; a request naming a deeper descendant stays pending forever + * (`cancelWorker` keeps answering `unknown`). See `DriverAgentOptions.controlDir`. So a cancel is + * routable only for a top-level worker; for a nested one the operator must cancel its top-level + * LEAD, which cancels that lead's whole subtree. + * + * The worker list never contains the root itself, so the top-level ancestor is the highest node + * in the parent chain that is still a listed worker. A `parent` that matches no listed worker is + * the root (or missing journal data) — both mean the worker is treated as top-level. + */ +export function workerCancelRoute( + workers: ReadonlyArray, + workerId: string, +): + | { readonly kind: 'direct'; readonly worker: WorkerView } + | { readonly kind: 'nested'; readonly worker: WorkerView; readonly lead: WorkerView } + | { readonly kind: 'unknown' } { + const byId = new Map(workers.map((worker) => [worker.id, worker])) + const worker = byId.get(workerId) + if (worker === undefined) return { kind: 'unknown' } + let lead = worker + const seen = new Set([lead.id]) + while (lead.parent !== undefined) { + const parent = byId.get(lead.parent) + if (parent === undefined || seen.has(parent.id)) break + seen.add(parent.id) + lead = parent + } + return lead.id === worker.id ? { kind: 'direct', worker } : { kind: 'nested', worker, lead } +} + /** Render one snapshot to an ANSI frame. Use this when nothing needs to be clickable. */ export function renderTopFrame(snapshot: TopSnapshot, options: RenderOptions = {}): string { return renderTopFrameWithLayout(snapshot, options).frame diff --git a/tests/kernel/worker-cancellation.test.ts b/tests/kernel/worker-cancellation.test.ts index 8719cf28..b0fb2d66 100644 --- a/tests/kernel/worker-cancellation.test.ts +++ b/tests/kernel/worker-cancellation.test.ts @@ -455,6 +455,77 @@ describe('acknowledged worker cancellation (#758)', () => { expect(cancelWorker(dir, 'ghost', 'op-ghost').effect).toBe('unknown') }) + it('a request naming a nested descendant stays unanswered — cancel its lead instead', async () => { + // The acknowledger resolves references against the root manager's DIRECT children only + // (`DriverAgentOptions.controlDir`). This pins the boundary: an operation naming a live + // nested descendant is neither acknowledged nor applied, across every acknowledger pass + // including the post-drain one, and still reads `unknown` after the run — never a success. + const dir = runDir() + const blobs = new InMemoryResultBlobStore() + const journal = new FileSpawnJournal(join(dir, 'spawn-journal.jsonl')) + + let markStarted: (() => void) | undefined + const descendantLive = new Promise((resolveGate) => { + markStarted = resolveGate + }) + const makeAgent = (p: AgentProfile): Agent => { + if (p.metadata?.kind === 'lead') { + const leadBrain = scriptedBrain([ + { toolCalls: spawnCall('d1') }, + awaitTurn, // repeats forever — the lead never stops on its own + ]) + return driverChild( + testAgentProfile('lead'), + driverAgent(driverOpts('lead', leadBrain, makeAgent, blobs)), + journal, + ) + } + return hangingLeaf('d1', { onStart: () => markStarted?.() }) + } + + const rootScript = scriptedBrain([ + { + toolCalls: [ + { + name: 'spawn_agent', + arguments: { + profile: { metadata: { kind: 'lead' } }, + task: 'go', + label: 'lead', + budget: { maxTokens: 10_000, maxIterations: 20 }, + }, + }, + ], + }, + { toolCalls: [{ name: 'list_questions', arguments: {} }] }, + { toolCalls: [{ name: 'list_questions', arguments: {} }] }, + { content: 'stopping' }, + ]) + let call = 0 + const rootChat: ToolLoopChat = async (messages, tools, context) => { + const index = call + call += 1 + if (index === 1) { + await descendantLive + cancelWorker(dir, 'run-deep:s0:s0', 'op-deep', { source: 'test' }) + } + return rootScript(messages, tools, context) + } + + const root = driverAgent(driverOpts('root', rootChat, makeAgent, blobs, { controlDir: dir })) + await createSupervisor().run(root, 'x', { + budget: { maxIterations: 100, maxTokens: 100_000 }, + runId: 'run-deep', + journal, + blobs, + executors: withDriverExecutor(createExecutorRegistry()), + maxDepth: 4, + }) + + expect(readWorkerCancellation(dir, 'op-deep')).toBeUndefined() + expect(cancelWorker(dir, 'run-deep:s0:s0', 'op-deep').effect).toBe('unknown') + }) + it('the cancelled worker reaches a terminal down state visible on the settle path', async () => { const dir = runDir() const blobs = new InMemoryResultBlobStore() diff --git a/tests/tui/top-model.test.ts b/tests/tui/top-model.test.ts index 46fc4c11..7043c31f 100644 --- a/tests/tui/top-model.test.ts +++ b/tests/tui/top-model.test.ts @@ -11,7 +11,13 @@ import { workerControlLogFile, writeWorkerSteer, } from '../../src/runtime/supervise/run-layout' -import { loadTopSnapshot, renderTopFrame, renderTopFrameWithLayout } from '../../src/tui/top-model' +import { + loadTopSnapshot, + renderTopFrame, + renderTopFrameWithLayout, + type WorkerView, + workerCancelRoute, +} from '../../src/tui/top-model' describe('supervisor top model', () => { const roots: string[] = [] @@ -462,6 +468,55 @@ describe('supervisor top model', () => { expect(frame).toContain(join(root, '.agent', 'supervisor')) }) + it('routes a worker cancel to a direct child, and a nested descendant to its top-level lead', () => { + // Node ids mirror the journal's hierarchy: the root ('run') never appears in the worker list, + // so a parent that matches no listed worker marks a top-level worker. + const workers = [ + worker('run:s0', 'lead', 'run'), + worker('run:s0:s0', 'd1', 'run:s0'), + worker('run:s0:s0:s1', 'd1-child', 'run:s0:s0'), + worker('run:s1', 'peer', 'run'), + ] + expect(workerCancelRoute(workers, 'run:s0')).toEqual({ kind: 'direct', worker: workers[0] }) + expect(workerCancelRoute(workers, 'run:s1')).toEqual({ kind: 'direct', worker: workers[3] }) + // One and two levels down both resolve to the SAME top-level lead — the only node the + // runtime acknowledger can apply an abort to. + expect(workerCancelRoute(workers, 'run:s0:s0')).toEqual({ + kind: 'nested', + worker: workers[1], + lead: workers[0], + }) + expect(workerCancelRoute(workers, 'run:s0:s0:s1')).toEqual({ + kind: 'nested', + worker: workers[2], + lead: workers[0], + }) + expect(workerCancelRoute(workers, 'missing')).toEqual({ kind: 'unknown' }) + // A worker with no recorded parent is treated as top-level: refusing it would strand a + // cancel the acknowledger may well be able to apply. + expect(workerCancelRoute([worker('run:s2', 'orphan')], 'run:s2')).toEqual({ + kind: 'direct', + worker: worker('run:s2', 'orphan'), + }) + // A malformed parent cycle must terminate, not hang the TUI. + const cyclic = [worker('a', 'a', 'b'), worker('b', 'b', 'a')] + expect(workerCancelRoute(cyclic, 'a').kind).toBe('nested') + }) + + function worker(id: string, label: string, parent?: string): WorkerView { + const spend = { iterations: 0, tokensInput: 0, tokensOutput: 0, usd: 0, ms: 0 } + return { + id, + label, + ...(parent === undefined ? {} : { parent }), + status: 'running', + latencyMs: 0, + spend, + metered: spend, + liveTail: [], + } + } + function fixtureRoot(): string { const root = mkdtempSync(join(tmpdir(), 'agent-runtime-top-')) roots.push(root)