From f49d0e5476847cbfd1eb859d8a6ff5b8693c809c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 16 Jan 2026 10:43:08 +0000 Subject: [PATCH] fix: expand exec abort/timeout coverage --- CHANGELOG.md | 1 + .../bash-tools.exec.background-abort.test.ts | 98 ++++++++++++++++++- src/agents/bash-tools.exec.ts | 16 ++- 3 files changed, 109 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 712f4a34c..8362803c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ - Fix: normalize pairing CLI aliases, allow extension channels, and harden Zalo webhook payload parsing. (#991) — thanks @longmaba. - Fix: allow local Tailscale Serve hostnames without treating tailnet clients as direct. (#885) — thanks @oswalpalash. - Fix: reset sessions after role-ordering conflicts to recover from consecutive user turns. (#998) +- Fix: keep background exec aborts from killing backgrounded sessions while honoring timeouts. ## 2026.1.14-1 diff --git a/src/agents/bash-tools.exec.background-abort.test.ts b/src/agents/bash-tools.exec.background-abort.test.ts index bb72f61b7..403bc064d 100644 --- a/src/agents/bash-tools.exec.background-abort.test.ts +++ b/src/agents/bash-tools.exec.background-abort.test.ts @@ -8,6 +8,8 @@ import { } from "./bash-process-registry"; import { killProcessTree } from "./shell-utils"; +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + afterEach(() => { resetProcessRegistryForTests(); }); @@ -27,7 +29,7 @@ test("background exec is not killed when tool signal aborts", async () => { abortController.abort(); - await new Promise((resolve) => setTimeout(resolve, 150)); + await sleep(150); const running = getSession(sessionId); const finished = getFinishedSession(sessionId); @@ -40,3 +42,97 @@ test("background exec is not killed when tool signal aborts", async () => { 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); + } +}); + +test("yielded background exec is not killed when tool signal aborts", async () => { + const tool = createExecTool({ allowBackground: true, backgroundMs: 10 }); + const abortController = new AbortController(); + + const result = await tool.execute( + "toolcall", + { command: "node -e \"setTimeout(() => {}, 5000)\"", yieldMs: 5 }, + abortController.signal, + ); + + expect(result.details.status).toBe("running"); + const sessionId = (result.details as { sessionId: string }).sessionId; + + abortController.abort(); + + await sleep(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("yielded background exec still times out", async () => { + const tool = createExecTool({ allowBackground: true, backgroundMs: 10 }); + + const result = await tool.execute("toolcall", { + command: "node -e \"setTimeout(() => {}, 5000)\"", + yieldMs: 5, + timeout: 0.2, + }); + + expect(result.details.status).toBe("running"); + const sessionId = (result.details as { sessionId: string }).sessionId; + + 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); + } +}); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index fb088557c..d97b0e653 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -262,22 +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); }