Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/subagent/agent-fleet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from "./agent-fleet.js";
import { createSubAgentSessionStore } from "./session-store.js";
import { createPermissionGate } from "../permission/gate.js";
import { forcedStopReport } from "./stop-policy.js";
import type { RunSubAgentParams, RunSubAgentResult } from "./types.js";

const testPermissionGate = createPermissionGate({
Expand Down Expand Up @@ -238,6 +239,53 @@ describe("spawn_agent + wait_agents", () => {
expect(result.report).toBe("irrelevant");
}
});

// CL-6915: operator cancel aborts the child signal, but run() still returns a
// salvage body (partial findings). Dropping that body left fleetRecords
// "running" forever so wait_agents never saw the salvage.
test("cancelled spawn_agent still resolves wait_agents with salvage findings", async () => {
const deps = makeDeps(async (params) => {
await new Promise<void>((resolve) => {
if (params.signal?.aborted) {
resolve();
return;
}
params.signal?.addEventListener("abort", () => resolve(), { once: true });
});
await new Promise((r) => setTimeout(r, 10));
return {
report: forcedStopReport("cancelled", "Found path in gate.ts"),
stopReason: "cancelled",
};
});
const spawn = createSpawnAgentTool(deps);
const wait = createWaitAgentsTool({ sessions: deps.sessions, fleetRecords: deps.fleetRecords });

const spawned = await callTool(spawn, {
description: "cancel salvage",
prompt: "probe",
intent: "explore",
});
const id = spawned.agent_id as string;

expect(deps.sessions.cancel(id)).toBe(true);
expect(deps.sessions.get(id)?.status).toBe("cancelled");

const waited = await callTool(wait, { targets: [id], timeout_ms: 5000 });
expect(waited.timed_out).toBe(false);
const results = waited.results as {
agent_id: string;
status: string;
report?: string;
}[];
expect(results).toHaveLength(1);
expect(results[0]!.status).toBe("done");
expect(results[0]!.report).toContain("## Summary");
expect(results[0]!.report).toContain("## Findings");
expect(results[0]!.report).toContain("gate.ts");
// Strip stays cancelled — salvage is for wait_agents, not a resurrection.
expect(deps.sessions.get(id)?.status).toBe("cancelled");
});
});

describe("spawn_agent same-cwd concurrency", () => {
Expand Down
10 changes: 8 additions & 2 deletions src/subagent/agent-fleet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,24 +467,30 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
deps
.run(params)
.then((result) => {
if (childCtl.signal.aborted) return;
// interrupt_agent already flipped this session to "interrupted"
// synchronously (session-store.interruptOne) — do not let the
// settling promise's normal bookkeeping overwrite that with a
// "completed" status.
if (result.interrupted === true) return;
// Operator cancel may race after run resolves (childCtl aborted).
// Keep strip status cancelled when sessions.cancel already flipped
// it, but never discard a returned body (including salvage) —
// wait_agents reads fleetRecords, not the strip.
deps.fleetRecords.resolve(session.id, result.report);
// result.agentRetained is only true on run.ts's clean-completion
// path when persist actually skipped teardown — a deadline/cancel
// salvage resolves through the same promise but always disposed
// its agent first, so the store must not treat it as resumable
// just because retained:true was requested at spawn.
// complete() no-ops when status is already cancelled.
deps.sessions.complete(session.id, result.report, {
agentRetained: result.agentRetained === true,
});
})
.catch((err) => {
if (childCtl.signal.aborted) return;
// Always terminalize fleetRecords — including pre-progress cancel that
// rethrows with no salvage — so wait_agents does not hang. fail()
// no-ops when cancel already flipped the strip status.
const message = err instanceof Error ? err.message : String(err);
deps.fleetRecords.reject(session.id, message);
deps.sessions.fail(session.id, message);
Expand Down
Loading