Skip to content

Commit 8b08823

Browse files
fix(subagent): collect terminal wait results even when the wait yields (#1110)
* 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. * docs(subagent): document yield take of terminal wait results
1 parent 4fcdf09 commit 8b08823

2 files changed

Lines changed: 188 additions & 7 deletions

File tree

‎src/subagent/agent-fleet.test.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
createSendInputTool,
2222
} from "./lifecycle-tools.js";
2323
import { createSubAgentSessionStore } from "./session-store.js";
24+
import { occupancyShouldYieldWait } from "./mailbox-mail-drive.js";
2425
import { INTENT_DEFAULT_DIRECTOR } from "../agent/directors/registry.js";
2526
import { createPermissionGate } from "../permission/gate.js";
2627
import { agentLaneIsLive, fleetProgress } from "../tui/agent-progress.js";
@@ -3792,6 +3793,173 @@ describe("wait_agents occupancy yield (CL-7518)", () => {
37923793
});
37933794
});
37943795

3796+
describe("wait_agents timed_out collection (CL-8028)", () => {
3797+
test("a yield on an already-terminal record still delivers its report and collects it", async () => {
3798+
const deps = makeDeps(async () => ({ report: "shipped" }));
3799+
const spawn = createSpawnAgentTool(deps);
3800+
const wait = createWaitAgentsTool({
3801+
sessions: deps.sessions,
3802+
fleetRecords: deps.fleetRecords,
3803+
shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords),
3804+
});
3805+
const spawned = await callTool(spawn, {
3806+
description: "lane",
3807+
prompt: "do it",
3808+
intent: "explore",
3809+
});
3810+
const id = spawned.agent_id as string;
3811+
await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, id);
3812+
// The real occupancy predicate trips on the uncollected terminal, so
3813+
// this wait yields — but it must still hand over the report, never a
3814+
// bare done that strands the record for resume.
3815+
expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(true);
3816+
const waited = await callTool(wait, { targets: [id], timeout_ms: 5_000 });
3817+
expect(waited.timed_out).toBe(true);
3818+
const row = defined((waited.results as Record<string, unknown>[])[0]);
3819+
expect(row.status).toBe("done");
3820+
expect(row.report).toBe("shipped");
3821+
expect(deps.fleetRecords.peek(id)?.collected).toBe(true);
3822+
// The collected report no longer trips the predicate, so the next wait
3823+
// proceeds instead of yielding the same way forever.
3824+
expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(false);
3825+
});
3826+
3827+
test("a second wait after the yielded wait is not stranded without a report", async () => {
3828+
const deps = makeDeps(async () => ({ report: "shipped" }));
3829+
const spawn = createSpawnAgentTool(deps);
3830+
const wait = createWaitAgentsTool({
3831+
sessions: deps.sessions,
3832+
fleetRecords: deps.fleetRecords,
3833+
shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords),
3834+
});
3835+
const spawned = await callTool(spawn, {
3836+
description: "lane",
3837+
prompt: "do it",
3838+
intent: "explore",
3839+
});
3840+
const id = spawned.agent_id as string;
3841+
await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, id);
3842+
const first = await callTool(wait, { targets: [id], timeout_ms: 5_000 });
3843+
expect(first.timed_out).toBe(true);
3844+
expect(defined((first.results as { report?: string }[])[0]).report).toBe(
3845+
"shipped",
3846+
);
3847+
3848+
const second = await callTool(wait, { targets: [id], timeout_ms: 5_000 });
3849+
expect(second.timed_out).toBe(false);
3850+
const row = defined((second.results as Record<string, unknown>[])[0]);
3851+
expect(row.status).toBe("done");
3852+
expect(row.report).toBeUndefined();
3853+
});
3854+
3855+
test("resume_agent is allowed after a timed_out wait that delivered the report", async () => {
3856+
const sessions = createSubAgentSessionStore();
3857+
const fleetRecords = createFleetMailbox(sessions);
3858+
const followupGate = deferred<string>();
3859+
const worker = sessions.start({
3860+
description: "retained",
3861+
agentId: "explorer",
3862+
brief: "b",
3863+
retained: true,
3864+
});
3865+
sessions.markRunning(worker.id);
3866+
fleetRecords.register(worker.id);
3867+
sessions.registerInterrupt(worker.id, () => undefined);
3868+
sessions.registerFollowup(worker.id, async () => followupGate.promise);
3869+
sessions.complete(worker.id, "first report");
3870+
await waitUntilMailboxTerminal(fleetRecords, sessions, worker.id);
3871+
3872+
const wait = createWaitAgentsTool({
3873+
sessions,
3874+
fleetRecords,
3875+
shouldYieldWait: () => occupancyShouldYieldWait(fleetRecords),
3876+
});
3877+
const resume = createResumeAgentTool({ sessions, fleetRecords });
3878+
3879+
const waited = await callTool(wait, {
3880+
targets: [worker.id],
3881+
timeout_ms: 5_000,
3882+
});
3883+
expect(waited.timed_out).toBe(true);
3884+
expect(defined((waited.results as { report?: string }[])[0]).report).toBe(
3885+
"first report",
3886+
);
3887+
3888+
// The collected first turn must not read as "not collected": resume
3889+
// proceeds to the followup instead of demanding another wait first.
3890+
if (resume.kind !== "full") throw new Error("expected full tool");
3891+
const resumed = await resume.handler(
3892+
{
3893+
id: "resume-after-yield",
3894+
name: "resume_agent",
3895+
arguments: { target: worker.id, message: "go on" },
3896+
},
3897+
new AbortController().signal,
3898+
);
3899+
expect(resumed.isError).not.toBe(true);
3900+
expect(String(resumed.content)).toContain("running");
3901+
3902+
followupGate.resolve("second report");
3903+
await waitUntilMailboxTerminal(fleetRecords, sessions, worker.id);
3904+
// Occupancy still asks to yield on the fresh uncollected terminal, but
3905+
// the yielded wait hands over the followup report and collects it —
3906+
// the resumed turn is not stranded the way the first turn was.
3907+
const done = await callTool(wait, {
3908+
targets: [worker.id],
3909+
timeout_ms: 5_000,
3910+
});
3911+
expect(done.timed_out).toBe(true);
3912+
expect(defined((done.results as { report?: string }[])[0]).report).toBe(
3913+
"second report",
3914+
);
3915+
expect(fleetRecords.peek(worker.id)?.collected).toBe(true);
3916+
});
3917+
3918+
test("a mixed yield takes the done report and peeks the running sibling", async () => {
3919+
const liveGate = deferred<RunSubAgentResult>();
3920+
const deps = makeDeps(async (params) => {
3921+
if (params.description === "done lane") return { report: "shipped" };
3922+
return liveGate.promise;
3923+
});
3924+
const spawn = createSpawnAgentTool(deps);
3925+
const wait = createWaitAgentsTool({
3926+
sessions: deps.sessions,
3927+
fleetRecords: deps.fleetRecords,
3928+
shouldYieldWait: () => occupancyShouldYieldWait(deps.fleetRecords),
3929+
});
3930+
const doneSpawn = await callTool(spawn, {
3931+
description: "done lane",
3932+
prompt: "do it",
3933+
intent: "explore",
3934+
});
3935+
const liveSpawn = await callTool(spawn, {
3936+
description: "live lane",
3937+
prompt: "do it",
3938+
intent: "explore",
3939+
});
3940+
const doneId = doneSpawn.agent_id as string;
3941+
const liveId = liveSpawn.agent_id as string;
3942+
await waitUntilMailboxTerminal(deps.fleetRecords, deps.sessions, doneId);
3943+
expect(occupancyShouldYieldWait(deps.fleetRecords)).toBe(true);
3944+
const waited = await callTool(wait, {
3945+
targets: [doneId, liveId],
3946+
timeout_ms: 5_000,
3947+
});
3948+
expect(waited.timed_out).toBe(true);
3949+
const results = waited.results as Record<string, unknown>[];
3950+
const doneRow = defined(results.find((row) => row.agent_id === doneId));
3951+
const liveRow = defined(results.find((row) => row.agent_id === liveId));
3952+
expect(doneRow.status).toBe("done");
3953+
expect(doneRow.report).toBe("shipped");
3954+
expect(deps.fleetRecords.peek(doneId)?.collected).toBe(true);
3955+
expect(liveRow.status).toBe("running");
3956+
expect(liveRow.report).toBeUndefined();
3957+
expect(deps.sessions.get(liveId)?.status).toBe("running");
3958+
expect(deps.fleetRecords.peek(liveId)?.collected).not.toBe(true);
3959+
liveGate.resolve({ report: "ok" });
3960+
});
3961+
});
3962+
37953963
describe("wait_agents tool copy", () => {
37963964
test("names the doom-loop exemption for still-pending polls", () => {
37973965
expect(waitAgentsToolDefinition.description).toContain("doom-loop guard");

‎src/subagent/agent-fleet.ts‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1559,8 +1559,10 @@ interface WaitAgentsDeps {
15591559
/**
15601560
* Supported but unwired in production: no current mount passes this
15611561
* (TUI does not mount wait_agents; exec mounts without the predicate).
1562-
* When true, finish the wait as a timeout (workers untouched, no take)
1563-
* so occupancy can deliver mailbox mail or a queued operator steer.
1562+
* When true, finish the wait as a timeout so occupancy can deliver
1563+
* mailbox mail or a queued operator steer. Live and awaiting_director
1564+
* records stay peeked (workers keep running); terminals are taken
1565+
* unless already bodyHanded.
15641566
*/
15651567
shouldYieldWait?: () => boolean;
15661568
}
@@ -1575,9 +1577,10 @@ function isWaitTerminal(id: string, fleetRecords: FleetMailboxHandle): boolean {
15751577
* elapses, or the optional (currently unwired in production)
15761578
* `shouldYieldWait` predicate is true. Driven by the session
15771579
* store's mailbox (`subscribe`) raced against a timer and the parent tool
1578-
* signal; never polls. Timeout, abort, and yield have no side effects:
1579-
* workers keep running and remain waitable. Overlay writers wake this wait
1580-
* via `sessions.wake()`.
1580+
* signal; never polls. Timeout, abort, and yield do not interrupt
1581+
* workers: live and awaiting_director stay peeked; terminals are taken
1582+
* unless already bodyHanded. Overlay writers wake this wait via
1583+
* `sessions.wake()`.
15811584
*/
15821585
async function waitForTerminal(
15831586
sessions: SubAgentSessionStore,
@@ -1694,7 +1697,13 @@ export function createWaitAgentsTool(deps: WaitAgentsDeps): AgentTool {
16941697

16951698
// Terminal overlay/session projections are marked collected once
16961699
// delivered here; a running record is only peeked, so it stays waitable.
1697-
// A yield leaves reports for occupancy (mailbox mail / ask-wake).
1700+
// A yield leaves live records for occupancy (mailbox mail / ask-wake):
1701+
// running/queued stay peeked and the awaiting_director question above
1702+
// stays with the wake path. Terminal records are always taken, even on
1703+
// a yield or timeout — CL-8028: reporting done with no report while the
1704+
// record stays uncollected blocks resume_agent ("call wait_agents
1705+
// first") and the occupancy yield predicate stays true on the
1706+
// uncollected terminal, so every later wait yields the same way.
16981707
const results = targets.map((id) => {
16991708
const record = deps.fleetRecords.peek(id);
17001709
if (record === undefined) {
@@ -1727,7 +1736,11 @@ export function createWaitAgentsTool(deps: WaitAgentsDeps): AgentTool {
17271736
if (isLiveWaitStatus(record.status)) {
17281737
return { agent_id: id, status: record.status };
17291738
}
1730-
if (yielded || record.bodyHanded === true) {
1739+
// CL-8028: a terminal status shown here must be collected. Falling
1740+
// through to takeAndProjectMailboxRecord even when yielded (see the
1741+
// note above) so a timed_out wait never reports done without its
1742+
// report while leaving the record uncollected.
1743+
if (record.bodyHanded === true) {
17311744
return {
17321745
agent_id: id,
17331746
status: record.status,

0 commit comments

Comments
 (0)