Skip to content

Commit 7a2ab30

Browse files
committed
Fail leftover exec dispose without skipping sibling teardown
1 parent 1f6c66c commit 7a2ab30

6 files changed

Lines changed: 111 additions & 14 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
2020
exits 1; SIGINT, SIGTERM, and SIGHUP still exit 128+n.
2121
- Persist close_agent surfaces leftover-child dispose failure so a worker
2222
that survives reap is not reported as a successful shutdown.
23+
- Leftover exec dispose is reported as a failed run (stderr + status failed), and
24+
parent toolset dispose finishes remaining workers and posix teardown before
25+
surfacing leftover-child failure.
2326

2427
## [0.3.18] - 2026-09-08
2528

src/agent/fleet-verbs-mount.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,58 @@ describe("primary fleet verb mount", () => {
122122
await expect(toolset.dispose()).rejects.toThrow(/still live after 2000ms reap/);
123123
});
124124

125+
test("createAgentToolset dispose closes remaining retained workers after the first leftover", async () => {
126+
const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-"));
127+
const { createAgentToolset } = await import("./tools.js");
128+
const permissionGate = {
129+
check: async () => ({ allowed: true }),
130+
getSkipPermissions: () => false,
131+
} as never;
132+
const sessions = createSubAgentSessionStore();
133+
const first = sessions.start({
134+
description: "d1",
135+
agentId: "a",
136+
brief: "b",
137+
retained: true,
138+
});
139+
const second = sessions.start({
140+
description: "d2",
141+
agentId: "a",
142+
brief: "b",
143+
retained: true,
144+
});
145+
let firstCloseCalls = 0;
146+
let secondCloseCalls = 0;
147+
sessions.registerClose(first.id, async () => {
148+
firstCloseCalls += 1;
149+
throw new Error("1 shell child process still live after 2000ms reap");
150+
});
151+
sessions.registerClose(second.id, async () => {
152+
secondCloseCalls += 1;
153+
});
154+
sessions.complete(first.id, "done", { agentRetained: true });
155+
sessions.complete(second.id, "done", { agentRetained: true });
156+
157+
const toolset = await createAgentToolset({
158+
cwd,
159+
permissionGate,
160+
onOperatorGate: async () => ({ kind: "option", index: 0 }),
161+
subAgent: {
162+
provider: {
163+
providerName: "test",
164+
baseURL: "http://127.0.0.1:0",
165+
model: "test-model",
166+
},
167+
getWorkdirBase: () => cwd,
168+
sessions,
169+
},
170+
});
171+
172+
await expect(toolset.dispose()).rejects.toThrow(/still live after 2000ms reap/);
173+
expect(firstCloseCalls).toBe(1);
174+
expect(secondCloseCalls).toBe(1);
175+
});
176+
125177
test("createAgentToolset dispose rejects when a retained running persist worker leaves children", async () => {
126178
const cwd = mkdtempSync(join(tmpdir(), "corbits-fleet-mount-"));
127179
const { createAgentToolset } = await import("./tools.js");

src/agent/tools.ts

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,13 @@ export const ASK_OPERATOR_OPTION_MAX_CHARS = 48;
9595
/** Cap on the ask_operator question (UTF-16 code units). */
9696
export const ASK_OPERATOR_QUESTION_MAX_CHARS = 160;
9797

98+
function rethrowToolsetDisposeFailures(failures: unknown[]): void {
99+
const first = failures[0];
100+
if (first === undefined) return;
101+
if (failures.length === 1) throw first;
102+
throw new AggregateError(failures, "toolset leftover dispose failed");
103+
}
104+
98105
const SubmitOutputArgs = type({
99106
"summary?": "string",
100107
"step?": "string",
@@ -943,11 +950,20 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
943950
disposed = true;
944951
mcpAbortController.abort(new Error("MCP toolset disposed"));
945952
disposal = (async () => {
953+
const failures: unknown[] = [];
946954
const fleetSessions = fleetSessionsForDispose;
947955
if (fleetSessions !== undefined) {
948-
await fleetSessions.cancelAll("parent session closed");
956+
try {
957+
await fleetSessions.cancelAll("parent session closed");
958+
} catch (err: unknown) {
959+
failures.push(err);
960+
}
949961
for (const session of [...fleetSessions.list()].reverse()) {
950-
await fleetSessions.closeOne(session.id, DEFAULT_CLOSE_DEADLINE_MS);
962+
try {
963+
await fleetSessions.closeOne(session.id, DEFAULT_CLOSE_DEADLINE_MS);
964+
} catch (err: unknown) {
965+
failures.push(err);
966+
}
951967
}
952968
}
953969
await Promise.allSettled([...inFlightConnections.values()]);
@@ -958,8 +974,13 @@ export async function createAgentToolset(args: AgentToolsetArgs): Promise<AgentT
958974
[...connectedClients.values()].map((client) => client.close().catch(() => undefined)),
959975
);
960976
connectedClients.clear();
961-
await posixTools.dispose();
977+
try {
978+
await posixTools.dispose();
979+
} catch (err: unknown) {
980+
failures.push(err);
981+
}
962982
await disposeWebSearchClients();
983+
rethrowToolsetDisposeFailures(failures);
963984
})();
964985
return disposal;
965986
};

src/exec/runner.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -888,11 +888,13 @@ export async function runExec(config: Config): Promise<ExecResult> {
888888
try {
889889
await disposeExecRuntime({ agent, toolset, subAgentSessions });
890890
} catch (err: unknown) {
891-
logger.debug("disposeExecRuntime failed: {error}", {
892-
error: formatCaughtError(err),
893-
});
894-
if (result !== undefined && result.exitCode === 0) {
891+
const message = formatCaughtError(err);
892+
logger.error("runtime dispose failed: {error}", { error: message });
893+
stderr.write(`Error: runtime dispose failed: ${message}\n`);
894+
if (result !== undefined) {
895895
result.exitCode = 1;
896+
result.status = "failed";
897+
result.error = `runtime dispose failed: ${message}`;
896898
}
897899
}
898900
clearActiveDisposeHost();

src/subagent/retain-salvage.test.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,11 @@ describe("retained session lifecycle", () => {
1717
// agentRetained:false, exactly as its real call site does whenever
1818
// result.agentRetained isn't true.
1919
store.complete(s.id, "Stopped: deadline\n\nPartial work...", { agentRetained: false });
20-
const after = store.get(s.id);
21-
console.log("lifecycleStatus:", after?.lifecycleStatus, "retained:", after?.retained);
2220
const outcome = store.resumeOne(s.id, "more");
23-
console.log("resumeOne outcome:", JSON.stringify(outcome));
2421
expect(outcome.ok).toBe(false);
2522
});
2623

27-
test("cancelAll does not close retained completed sessions", async () => {
24+
test("cancelAll closes salvaged retained completed sessions", async () => {
2825
const store = createSubAgentSessionStore({ maxCompleted: 5 });
2926
const s = store.start({
3027
description: "worker",
@@ -37,8 +34,7 @@ describe("retained session lifecycle", () => {
3734
closed = true;
3835
});
3936
store.complete(s.id, "done");
40-
const cancelled = await store.cancelAll("parent stop");
41-
console.log("cancelAll returned:", cancelled, "| close invoked:", closed);
37+
await store.cancelAll("parent stop");
4238
expect(closed).toBe(true);
4339
});
4440

@@ -63,7 +59,6 @@ describe("retained session lifecycle", () => {
6359
store.registerClose(s.id, async () => {});
6460
store.complete(s.id, "done");
6561
}
66-
console.log("sessions retained despite maxRetained=3:", store.list().length);
6762
expect(store.list().length).toBeLessThanOrEqual(3);
6863
});
6964

tests/unit/exec/runner.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,12 @@ describe("runExec", () => {
298298
const sessionId = "exec-dispose-fail";
299299
let disposeCalls = 0;
300300
const dummySource = { id: "test", provider: "test", model: "test" } as InferenceSource;
301+
const stderrChunks: string[] = [];
302+
const origWrite = process.stderr.write.bind(process.stderr);
303+
process.stderr.write = ((chunk: string | Uint8Array, ...rest: unknown[]) => {
304+
stderrChunks.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8"));
305+
return origWrite(chunk as never, ...(rest as never[]));
306+
}) as typeof process.stderr.write;
301307
try {
302308
await withMockedModuleDuring(
303309
import.meta.resolve("node:os"),
@@ -347,6 +353,9 @@ describe("runExec", () => {
347353
providers: [],
348354
});
349355
expect(result.exitCode).toBe(1);
356+
expect(result.status).toBe("failed");
357+
expect(result.error).toMatch(/plugin dispose failed|runtime dispose failed/i);
358+
expect(stderrChunks.join("")).toMatch(/runtime dispose failed/i);
350359
expect(disposeCalls).toBe(1);
351360
expect(getActiveDisposeHost()).toBeNull();
352361
},
@@ -356,6 +365,7 @@ describe("runExec", () => {
356365
},
357366
);
358367
} finally {
368+
process.stderr.write = origWrite;
359369
if (previous !== null) setActiveRun(previous);
360370
else clearActiveRun();
361371
rmSync(cwd, { recursive: true, force: true });
@@ -415,6 +425,20 @@ describe("disposeExecRuntime", () => {
415425
expect(calls).toEqual(["agent", "toolset"]);
416426
});
417427

428+
test("rejects leftover-child dispose from the toolset", async () => {
429+
await expect(
430+
disposeExecRuntime({
431+
agent: { close: async () => undefined },
432+
toolset: {
433+
dispose: async () => {
434+
throw new Error("1 shell child process still live after 2000ms reap");
435+
},
436+
},
437+
subAgentSessions: null,
438+
}),
439+
).rejects.toThrow(/still live after 2000ms reap/);
440+
});
441+
418442
test("rejects when toolset dispose fails", async () => {
419443
await expect(
420444
disposeExecRuntime({

0 commit comments

Comments
 (0)