From 5c3e1a98e85cdf9cf789f9e2defb6674e1931eef Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 16 Sep 2026 15:26:04 -0700 Subject: [PATCH 1/2] fix(subagent): collect terminal wait results even when the wait yields CL-8028: an occupancy yield on an already-terminal record reported done with no report while leaving the record uncollected, so resume_agent kept demanding wait_agents first and every later wait yielded the same way. Terminal records are now always taken and projected, even on yield or timeout; only live records stay uncollected for occupancy. --- src/subagent/agent-fleet.test.ts | 124 +++++++++++++++++++++++++++++++ src/subagent/agent-fleet.ts | 14 +++- 2 files changed, 136 insertions(+), 2 deletions(-) diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index e4b8de3c3..98f1b5755 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -21,6 +21,7 @@ import { createSendInputTool, } from "./lifecycle-tools.js"; import { createSubAgentSessionStore } from "./session-store.js"; +import { occupancyShouldYieldWait } from "./mailbox-mail-drive.js"; import { INTENT_DEFAULT_DIRECTOR } from "../agent/directors/registry.js"; import { createPermissionGate } from "../permission/gate.js"; import { agentLaneIsLive, fleetProgress } from "../tui/agent-progress.js"; @@ -3792,6 +3793,129 @@ describe("wait_agents occupancy yield (CL-7518)", () => { }); }); +describe("wait_agents timed_out collection (CL-8028)", () => { + test("a yield on an already-terminal record still delivers its report and collects it", async () => { + const deps = makeDeps(async () => ({ report: "shipped" })); + const spawn = createSpawnAgentTool(deps); + const wait = createWaitAgentsTool({ + sessions: deps.sessions, + fleetRecords: deps.fleetRecords, + shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords), + }); + const spawned = await callTool(spawn, { + description: "lane", + prompt: "do it", + intent: "explore", + }); + const id = spawned.agent_id as string; + await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, id); + // The real occupancy predicate trips on the uncollected terminal, so + // this wait yields — but it must still hand over the report, never a + // bare done that strands the record for resume. + expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(true); + const waited = await callTool(wait, { targets: [id], timeout_ms: 5_000 }); + expect(waited.timed_out).toBe(true); + const row = defined((waited.results as Record[])[0]); + expect(row.status).toBe("done"); + expect(row.report).toBe("shipped"); + expect(deps.fleetRecords.peek(id)?.collected).toBe(true); + // The collected report no longer trips the predicate, so the next wait + // proceeds instead of yielding the same way forever. + expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(false); + }); + + test("a second wait after the yielded wait is not stranded without a report", async () => { + const deps = makeDeps(async () => ({ report: "shipped" })); + const spawn = createSpawnAgentTool(deps); + const wait = createWaitAgentsTool({ + sessions: deps.sessions, + fleetRecords: deps.fleetRecords, + shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords), + }); + const spawned = await callTool(spawn, { + description: "lane", + prompt: "do it", + intent: "explore", + }); + const id = spawned.agent_id as string; + await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, id); + const first = await callTool(wait, { targets: [id], timeout_ms: 5_000 }); + expect(first.timed_out).toBe(true); + expect(defined((first.results as { report?: string }[])[0]).report).toBe( + "shipped", + ); + + const second = await callTool(wait, { targets: [id], timeout_ms: 5_000 }); + expect(second.timed_out).toBe(false); + const row = defined((second.results as Record[])[0]); + expect(row.status).toBe("done"); + expect(row.report).toBeUndefined(); + }); + + test("resume_agent is allowed after a timed_out wait that delivered the report", async () => { + const sessions = createSubAgentSessionStore(); + const fleetRecords = createFleetMailbox(sessions); + const followupGate = deferred(); + const worker = sessions.start({ + description: "retained", + agentId: "explorer", + brief: "b", + retained: true, + }); + sessions.markRunning(worker.id); + fleetRecords.register(worker.id); + sessions.registerInterrupt(worker.id, () => undefined); + sessions.registerFollowup(worker.id, async () => followupGate.promise); + sessions.complete(worker.id, "first report"); + await waitUntilMailboxTerminal(fleetRecords, sessions, worker.id); + + const wait = createWaitAgentsTool({ + sessions, + fleetRecords, + shouldYieldWait: () => occupancyShouldYieldWait(fleetRecords), + }); + const resume = createResumeAgentTool({ sessions, fleetRecords }); + + const waited = await callTool(wait, { + targets: [worker.id], + timeout_ms: 5_000, + }); + expect(waited.timed_out).toBe(true); + expect(defined((waited.results as { report?: string }[])[0]).report).toBe( + "first report", + ); + + // The collected first turn must not read as "not collected": resume + // proceeds to the followup instead of demanding another wait first. + if (resume.kind !== "full") throw new Error("expected full tool"); + const resumed = await resume.handler( + { + id: "resume-after-yield", + name: "resume_agent", + arguments: { target: worker.id, message: "go on" }, + }, + new AbortController().signal, + ); + expect(resumed.isError).not.toBe(true); + expect(String(resumed.content)).toContain("running"); + + followupGate.resolve("second report"); + await waitUntilMailboxTerminal(fleetRecords, sessions, worker.id); + // Occupancy still asks to yield on the fresh uncollected terminal, but + // the yielded wait hands over the followup report and collects it — + // the resumed turn is not stranded the way the first turn was. + const done = await callTool(wait, { + targets: [worker.id], + timeout_ms: 5_000, + }); + expect(done.timed_out).toBe(true); + expect(defined((done.results as { report?: string }[])[0]).report).toBe( + "second report", + ); + expect(fleetRecords.peek(worker.id)?.collected).toBe(true); + }); +}); + describe("wait_agents tool copy", () => { test("names the doom-loop exemption for still-pending polls", () => { expect(waitAgentsToolDefinition.description).toContain("doom-loop guard"); diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index f80965396..c7acbe497 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -1694,7 +1694,13 @@ export function createWaitAgentsTool(deps: WaitAgentsDeps): AgentTool { // Terminal overlay/session projections are marked collected once // delivered here; a running record is only peeked, so it stays waitable. - // A yield leaves reports for occupancy (mailbox mail / ask-wake). + // A yield leaves live records for occupancy (mailbox mail / ask-wake): + // running/queued stay peeked and the awaiting_director question above + // stays with the wake path. Terminal records are always taken, even on + // a yield or timeout — CL-8028: reporting done with no report while the + // record stays uncollected blocks resume_agent ("call wait_agents + // first") and the occupancy yield predicate stays true on the + // uncollected terminal, so every later wait yields the same way. const results = targets.map((id) => { const record = deps.fleetRecords.peek(id); if (record === undefined) { @@ -1727,7 +1733,11 @@ export function createWaitAgentsTool(deps: WaitAgentsDeps): AgentTool { if (isLiveWaitStatus(record.status)) { return { agent_id: id, status: record.status }; } - if (yielded || record.bodyHanded === true) { + // CL-8028: a terminal status shown here must be collected. Falling + // through to takeAndProjectMailboxRecord even when yielded (see the + // note above) so a timed_out wait never reports done without its + // report while leaving the record uncollected. + if (record.bodyHanded === true) { return { agent_id: id, status: record.status, From abff675169050b42d3361bca227b173167519f27 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Wed, 16 Sep 2026 15:58:33 -0700 Subject: [PATCH 2/2] docs(subagent): document yield take of terminal wait results --- src/subagent/agent-fleet.test.ts | 44 ++++++++++++++++++++++++++++++++ src/subagent/agent-fleet.ts | 13 ++++++---- 2 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index 98f1b5755..1d87fa992 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -3914,6 +3914,50 @@ describe("wait_agents timed_out collection (CL-8028)", () => { ); expect(fleetRecords.peek(worker.id)?.collected).toBe(true); }); + + test("a mixed yield takes the done report and peeks the running sibling", async () => { + const liveGate = deferred(); + const deps = makeDeps(async (params) => { + if (params.description === "done lane") return { report: "shipped" }; + return liveGate.promise; + }); + const spawn = createSpawnAgentTool(deps); + const wait = createWaitAgentsTool({ + sessions: deps.sessions, + fleetRecords: deps.fleetRecords, + shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords), + }); + const doneSpawn = await callTool(spawn, { + description: "done lane", + prompt: "do it", + intent: "explore", + }); + const liveSpawn = await callTool(spawn, { + description: "live lane", + prompt: "do it", + intent: "explore", + }); + const doneId = doneSpawn.agent_id as string; + const liveId = liveSpawn.agent_id as string; + await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, doneId); + expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(true); + const waited = await callTool(wait, { + targets: [doneId, liveId], + timeout_ms: 5_000, + }); + expect(waited.timed_out).toBe(true); + const results = waited.results as Record[]; + const doneRow = defined(results.find((row) => row.agent_id === doneId)); + const liveRow = defined(results.find((row) => row.agent_id === liveId)); + expect(doneRow.status).toBe("done"); + expect(doneRow.report).toBe("shipped"); + expect(deps.fleetRecords.peek(doneId)?.collected).toBe(true); + expect(liveRow.status).toBe("running"); + expect(liveRow.report).toBeUndefined(); + expect(deps.sessions.get(liveId)?.status).toBe("running"); + expect(deps.fleetRecords.peek(liveId)?.collected).not.toBe(true); + liveGate.resolve({ report: "ok" }); + }); }); describe("wait_agents tool copy", () => { diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index c7acbe497..69a380d95 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -1559,8 +1559,10 @@ interface WaitAgentsDeps { /** * Supported but unwired in production: no current mount passes this * (TUI does not mount wait_agents; exec mounts without the predicate). - * When true, finish the wait as a timeout (workers untouched, no take) - * so occupancy can deliver mailbox mail or a queued operator steer. + * When true, finish the wait as a timeout so occupancy can deliver + * mailbox mail or a queued operator steer. Live and awaiting_director + * records stay peeked (workers keep running); terminals are taken + * unless already bodyHanded. */ shouldYieldWait?: () => boolean; } @@ -1575,9 +1577,10 @@ function isWaitTerminal(id: string, fleetRecords: FleetMailboxHandle): boolean { * elapses, or the optional (currently unwired in production) * `shouldYieldWait` predicate is true. Driven by the session * store's mailbox (`subscribe`) raced against a timer and the parent tool - * signal; never polls. Timeout, abort, and yield have no side effects: - * workers keep running and remain waitable. Overlay writers wake this wait - * via `sessions.wake()`. + * signal; never polls. Timeout, abort, and yield do not interrupt + * workers: live and awaiting_director stay peeked; terminals are taken + * unless already bodyHanded. Overlay writers wake this wait via + * `sessions.wake()`. */ async function waitForTerminal( sessions: SubAgentSessionStore,