fix: keep background execs alive on abort

This commit is contained in:
Peter Steinberger
2026-01-16 09:56:42 +00:00
parent b969c216fc
commit fc91be8033
2 changed files with 88 additions and 5 deletions

View File

@@ -0,0 +1,76 @@
import { afterEach, expect, test } from "vitest";
import { getFinishedSession, getSession, resetProcessRegistryForTests } from "./bash-process-registry.js";
import { createExecTool } from "./bash-tools.exec.js";
import { killProcessTree } from "./shell-utils.js";
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
afterEach(() => {
resetProcessRegistryForTests();
});
test("background exec is not killed when tool signal aborts", async () => {
const tool = createExecTool({ allowBackground: true, backgroundMs: 0 });
const abortController = new AbortController();
const result = await tool.execute(
"toolcall",
{ command: "node -e \"setTimeout(() => {}, 5000)\"", background: true },
abortController.signal,
);
expect(result.details.status).toBe("running");
const sessionId = (result.details as { sessionId: string }).sessionId;
abortController.abort();
await new Promise((resolve) => setTimeout(resolve, 150));
const running = getSession(sessionId);
const finished = getFinishedSession(sessionId);
try {
expect(finished).toBeUndefined();
expect(running?.exited).toBe(false);
} finally {
const pid = running?.pid;
if (pid) killProcessTree(pid);
}
});
test("background exec still times out after tool signal abort", async () => {
const tool = createExecTool({ allowBackground: true, backgroundMs: 0 });
const abortController = new AbortController();
const result = await tool.execute(
"toolcall",
{
command: "node -e \"setTimeout(() => {}, 5000)\"",
background: true,
timeout: 0.2,
},
abortController.signal,
);
expect(result.details.status).toBe("running");
const sessionId = (result.details as { sessionId: string }).sessionId;
abortController.abort();
let finished = getFinishedSession(sessionId);
const deadline = Date.now() + 2000;
while (!finished && Date.now() < deadline) {
await sleep(20);
finished = getFinishedSession(sessionId);
}
const running = getSession(sessionId);
try {
expect(finished?.status).toBe("failed");
} finally {
const pid = running?.pid;
if (pid) killProcessTree(pid);
}
});

View File

@@ -262,21 +262,28 @@ export function createExecTool(
fn();
};
const onAbort = () => {
// Tool-call abort should not kill backgrounded sessions; timeouts still must.
const onAbortSignal = () => {
if (yielded || session.backgrounded) return;
killSession(session);
};
if (signal?.aborted) onAbort();
// Timeouts always terminate, even for backgrounded sessions.
const onTimeout = () => {
timedOut = true;
killSession(session);
};
if (signal?.aborted) onAbortSignal();
else if (signal) {
signal.addEventListener("abort", onAbort, { once: true });
signal.addEventListener("abort", onAbortSignal, { once: true });
}
const effectiveTimeout =
typeof params.timeout === "number" ? params.timeout : defaultTimeoutSec;
if (effectiveTimeout > 0) {
timeoutTimer = setTimeout(() => {
timedOut = true;
onAbort();
onTimeout();
}, effectiveTimeout * 1000);
}