fix: expand exec abort/timeout coverage
This commit is contained in:
@@ -67,6 +67,7 @@
|
|||||||
- Fix: normalize pairing CLI aliases, allow extension channels, and harden Zalo webhook payload parsing. (#991) — thanks @longmaba.
|
- 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: 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: 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
|
## 2026.1.14-1
|
||||||
|
|
||||||
|
|||||||
@@ -8,6 +8,8 @@ import {
|
|||||||
} from "./bash-process-registry";
|
} from "./bash-process-registry";
|
||||||
import { killProcessTree } from "./shell-utils";
|
import { killProcessTree } from "./shell-utils";
|
||||||
|
|
||||||
|
const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
resetProcessRegistryForTests();
|
resetProcessRegistryForTests();
|
||||||
});
|
});
|
||||||
@@ -27,7 +29,7 @@ test("background exec is not killed when tool signal aborts", async () => {
|
|||||||
|
|
||||||
abortController.abort();
|
abortController.abort();
|
||||||
|
|
||||||
await new Promise((resolve) => setTimeout(resolve, 150));
|
await sleep(150);
|
||||||
|
|
||||||
const running = getSession(sessionId);
|
const running = getSession(sessionId);
|
||||||
const finished = getFinishedSession(sessionId);
|
const finished = getFinishedSession(sessionId);
|
||||||
@@ -40,3 +42,97 @@ test("background exec is not killed when tool signal aborts", async () => {
|
|||||||
if (pid) killProcessTree(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);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -262,22 +262,28 @@ export function createExecTool(
|
|||||||
fn();
|
fn();
|
||||||
};
|
};
|
||||||
|
|
||||||
const onAbort = () => {
|
// Tool-call abort should not kill backgrounded sessions; timeouts still must.
|
||||||
|
const onAbortSignal = () => {
|
||||||
if (yielded || session.backgrounded) return;
|
if (yielded || session.backgrounded) return;
|
||||||
killSession(session);
|
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) {
|
else if (signal) {
|
||||||
signal.addEventListener("abort", onAbort, { once: true });
|
signal.addEventListener("abort", onAbortSignal, { once: true });
|
||||||
}
|
}
|
||||||
|
|
||||||
const effectiveTimeout =
|
const effectiveTimeout =
|
||||||
typeof params.timeout === "number" ? params.timeout : defaultTimeoutSec;
|
typeof params.timeout === "number" ? params.timeout : defaultTimeoutSec;
|
||||||
if (effectiveTimeout > 0) {
|
if (effectiveTimeout > 0) {
|
||||||
timeoutTimer = setTimeout(() => {
|
timeoutTimer = setTimeout(() => {
|
||||||
timedOut = true;
|
onTimeout();
|
||||||
onAbort();
|
|
||||||
}, effectiveTimeout * 1000);
|
}, effectiveTimeout * 1000);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user