Skip to content

Commit d8d4dfb

Browse files
committed
Fix report eviction and add cwd write-lane refusal for spawn_agent
fleetRecords is a never-capped map of terminal spawn_agent results, written before the session store's complete()/fail() so wait_agents never loses a report to the store's TUI-sized finished-session cap. spawn_agent also now refuses a second concurrent implement-intent spawn against the same cwd (no worktree isolation yet), releasing the lane once the running one finishes; explore/plan/review-intent spawns are unaffected and may still run concurrently.
1 parent c044a60 commit d8d4dfb

3 files changed

Lines changed: 286 additions & 46 deletions

File tree

src/subagent/agent-fleet.test.ts

Lines changed: 126 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { describe, expect, test } from "bun:test";
22

3-
import { createSpawnAgentTool, createWaitAgentsTool, type AgentFleetDeps } from "./agent-fleet.js";
3+
import {
4+
createFleetRecords,
5+
createSpawnAgentTool,
6+
createWaitAgentsTool,
7+
type AgentFleetDeps,
8+
} from "./agent-fleet.js";
49
import { createSubAgentSessionStore } from "./session-store.js";
510
import { createPermissionGate } from "../permission/gate.js";
611
import type { RunSubAgentParams } from "./types.js";
@@ -31,28 +36,40 @@ function deferred<T>(): {
3136
return { promise, resolve, reject };
3237
}
3338

34-
function makeDeps(run: (params: RunSubAgentParams) => Promise<string>): AgentFleetDeps {
39+
function makeDeps(
40+
run: (params: RunSubAgentParams) => Promise<string>,
41+
opts: { cwd?: string } = {},
42+
): AgentFleetDeps {
3543
return {
3644
permissionGate: testPermissionGate,
37-
cwd: "/tmp",
45+
cwd: opts.cwd ?? "/tmp",
3846
getWorkdirBase: () => "/tmp/workdir",
3947
provider,
4048
run,
4149
sessions: createSubAgentSessionStore(),
50+
fleetRecords: createFleetRecords(),
4251
};
4352
}
4453

45-
async function callTool(
54+
async function callToolRaw(
4655
tool: ReturnType<typeof createSpawnAgentTool> | ReturnType<typeof createWaitAgentsTool>,
4756
args: Record<string, unknown>,
48-
): Promise<Record<string, unknown>> {
57+
): Promise<{ content: string; isError?: boolean }> {
4958
if (tool.kind !== "full") throw new Error(`expected full tool, got ${tool.kind}`);
5059
const result = await tool.handler(
5160
{ id: `call-${Math.random()}`, name: tool.definition.name, arguments: args },
5261
new AbortController().signal,
5362
);
5463
const content =
5564
typeof result.content === "string" ? result.content : JSON.stringify(result.content);
65+
return { content, ...(result.isError !== undefined ? { isError: result.isError } : {}) };
66+
}
67+
68+
async function callTool(
69+
tool: ReturnType<typeof createSpawnAgentTool> | ReturnType<typeof createWaitAgentsTool>,
70+
args: Record<string, unknown>,
71+
): Promise<Record<string, unknown>> {
72+
const { content } = await callToolRaw(tool, args);
5673
return JSON.parse(content);
5774
}
5875

@@ -90,7 +107,7 @@ describe("spawn_agent + wait_agents", () => {
90107
return gates[i]!.promise;
91108
});
92109
const spawn = createSpawnAgentTool(deps);
93-
const wait = createWaitAgentsTool({ sessions: deps.sessions });
110+
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });
94111

95112
const spawned = await Promise.all(
96113
[0, 1, 2].map((i) =>
@@ -120,7 +137,7 @@ describe("spawn_agent + wait_agents", () => {
120137
const gate = deferred<string>();
121138
const deps = makeDeps(async () => gate.promise);
122139
const spawn = createSpawnAgentTool(deps);
123-
const wait = createWaitAgentsTool({ sessions: deps.sessions });
140+
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });
124141

125142
const spawned = await callTool(spawn, {
126143
description: "slow job",
@@ -155,7 +172,7 @@ describe("spawn_agent + wait_agents", () => {
155172
let callIndex = 0;
156173
const deps = makeDeps(async () => gates[callIndex++]!.promise);
157174
const spawn = createSpawnAgentTool(deps);
158-
const wait = createWaitAgentsTool({ sessions: deps.sessions });
175+
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });
159176

160177
await callTool(spawn, { description: "a", prompt: "do it", intent: "explore" });
161178
await callTool(spawn, { description: "b", prompt: "do it", intent: "explore" });
@@ -169,4 +186,105 @@ describe("spawn_agent + wait_agents", () => {
169186

170187
gates[1]!.resolve("b done");
171188
});
189+
190+
test("reports survive well past the session store's display cap (20) until wait_agents collects them", async () => {
191+
// DEFAULT_MAX_COMPLETED on SubAgentSessionStore is 20 finished sessions;
192+
// spawn (and complete) enough workers to blow well past it before any of
193+
// them is collected, proving fleetRecords — not the store — is what
194+
// wait_agents actually reads from.
195+
const COUNT = 25;
196+
const deps = makeDeps(async () => "irrelevant");
197+
const spawn = createSpawnAgentTool(deps);
198+
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });
199+
200+
const ids: string[] = [];
201+
for (let i = 0; i < COUNT; i++) {
202+
const spawned = await callTool(spawn, {
203+
description: `job-${i}`,
204+
prompt: `report-${i}`,
205+
intent: "explore",
206+
});
207+
ids.push(spawned.agent_id as string);
208+
}
209+
210+
// Let every spawn's run() resolve and complete() land before collecting.
211+
await new Promise((resolve) => setTimeout(resolve, 20));
212+
213+
// The store itself has already evicted all but the most recent 20.
214+
expect(deps.sessions.get(ids[0]!)).toBeUndefined();
215+
216+
// But every single one is still retrievable through wait_agents.
217+
const waited = await callTool(wait, { targets: ids, timeout_ms: 5000 });
218+
const results = waited.results as { agent_id: string; status: string; report?: string }[];
219+
expect(results).toHaveLength(COUNT);
220+
for (const result of results) {
221+
expect(result.status).toBe("done");
222+
expect(result.report).toBe("irrelevant");
223+
}
224+
});
225+
});
226+
227+
describe("spawn_agent write-lane isolation", () => {
228+
test("refuses a second concurrent implement-intent spawn against the same cwd", async () => {
229+
const gate = deferred<string>();
230+
const deps = makeDeps(async () => gate.promise, { cwd: "/repo" });
231+
const spawn = createSpawnAgentTool(deps);
232+
233+
const first = await callTool(spawn, {
234+
description: "build one",
235+
prompt: "implement thing one",
236+
intent: "implement",
237+
});
238+
expect(first.status).toBe("running");
239+
240+
const second = await callToolRaw(spawn, {
241+
description: "build two",
242+
prompt: "implement thing two",
243+
intent: "implement",
244+
});
245+
expect(second.isError).toBe(true);
246+
expect(second.content).toContain("Error:");
247+
expect(second.content).toContain(first.agent_id as string);
248+
249+
gate.resolve("done");
250+
});
251+
252+
test("does not refuse a second concurrent explore-intent spawn against the same cwd", async () => {
253+
const deps = makeDeps(async () => "explored", { cwd: "/repo" });
254+
const spawn = createSpawnAgentTool(deps);
255+
256+
const first = await callTool(spawn, {
257+
description: "explore one",
258+
prompt: "look around",
259+
intent: "explore",
260+
});
261+
const second = await callTool(spawn, {
262+
description: "explore two",
263+
prompt: "look around more",
264+
intent: "explore",
265+
});
266+
267+
expect(first.status).toBe("running");
268+
expect(second.status).toBe("running");
269+
});
270+
271+
test("releases the write lane once the implement worker finishes, allowing another", async () => {
272+
const deps = makeDeps(async () => "built", { cwd: "/repo" });
273+
const spawn = createSpawnAgentTool(deps);
274+
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });
275+
276+
const first = await callTool(spawn, {
277+
description: "build one",
278+
prompt: "implement thing one",
279+
intent: "implement",
280+
});
281+
await callTool(wait, { targets: [first.agent_id as string], timeout_ms: 5000 });
282+
283+
const second = await callTool(spawn, {
284+
description: "build two",
285+
prompt: "implement thing two",
286+
intent: "implement",
287+
});
288+
expect(second.status).toBe("running");
289+
});
172290
});

0 commit comments

Comments
 (0)