mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
06aa6563e4
Fixes #11613. ## Problem The bash tool waits for stdout/stderr to close, not only for the shell to exit (#5303). A background process that inherited those pipes and keeps writing keeps the tool waiting after the shell exited. When the timeout then hits, the tool reported only `Command timed out after N seconds`. Example, timeout 3s: ```bash bash -c 'while true; do echo tick; sleep 0.05; done' & sleep 1; echo boom >&2; exit 1 ``` The shell exits with code 1 after ~1s. The tool still times out at the deadline, because the loop is still writing to its output. Both facts are true: the command failed with code 1, and something it started was still running at the timeout. The old message dropped the first, so the model read a failed command as a slow one and retried with a longer timeout, which never helps here. ## Fix The timeout stays a timeout, since the tool did hit its deadline while background processes were still writing. When the shell had already exited before the deadline, the message now also says so: ``` Command timed out after 3 seconds. The shell exited with code 1 after 1.1 seconds, but background processes it started kept writing output until the timeout. ``` A shell still running at the deadline keeps the plain `Command timed out after N seconds`. The local backend throws a module-private `ShellTimeoutError` with the message `timeout:<seconds>` as before, so custom `BashOperations` that throw `timeout:N` are unaffected. ## Behavior per platform - Linux and macOS: the shell runs in its own process group (`detached: true`). At the deadline `kill(-pgid)` still reaches the background processes after the shell exited, the pipes close, and the tool returns at the deadline. Tested on Linux; macOS uses the same mechanism and matches the macOS repros in the issue. - Windows: still not fully solved. At the deadline `taskkill /T /PID <shell>` fails with `process not found`, because the shell pid is gone. Walking parent pids does not help: Git's `bin\bash.exe` is a launcher for `usr\bin\bash.exe`, and the orphaned loop's parent is that already-exited inner bash, not the pid pi holds. The tool keeps reading until the output pauses for 100ms. Measured with Git Bash: the message is now correct, but the tool returned after 14s instead of 3s; with a loop that never pauses it never returns. This PR only fixes the reporting. ## Same pattern elsewhere - `packages/durable/src/env/node.ts` (`NodeExecutionEnv.exec`): same code, same false `timeout:N` after the shell exited. Same Windows limitation. - `packages/coding-agent/src/core/exec.ts` (`pi.exec` for extensions): the timeout sends SIGTERM/SIGKILL to the direct child only, no process group. A descendant that keeps writing keeps the call waiting past the timeout on every platform, and the result has `killed: true` even when the child had already exited with its own code. - `packages/coding-agent/src/core/package-manager.ts` (capture commands for git/npm): waits for `close` and kills only the direct child on timeout, so a descendant holding the pipes makes the timeout ineffective on every platform, even when the descendant writes nothing. - `packages/mcp/src/transports/stdio.ts`: on Windows, closing a server that already exited skips taskkill, so children holding its pipes survive. The code documents this as a known limitation. Possible follow-up for Windows and the cases above: once the deadline passed, stop waiting for the pipes (destroy our ends). On Windows this also ends most writers, which die from a broken pipe (tested with Git Bash). ## Tests - `test/suite/regressions/11613-bash-timeout-after-shell-exit.test.ts` (Unix only; it would hang on Windows for the reason above): background writer after the shell exits with code 3, and a shell still running at the deadline. The first case fails without the fix. - Checked manually on Windows 11 with Git Bash through the built bundle: both messages are as above. - `npm run check`, `test/tools.test.ts`, the #5303 and #5208 regressions, `bash-close-hang-windows`, `powershell-tool`, `shell-kill-process-tree`. This PR description is AI-generated.
43 lines
2 KiB
TypeScript
43 lines
2 KiB
TypeScript
import { tmpdir } from "node:os";
|
|
import { describe, expect, it } from "vitest";
|
|
import { createBashTool } from "../../../src/core/tools/bash.ts";
|
|
|
|
/**
|
|
* Regression test for https://github.com/earendil-works/pi/issues/11613
|
|
*
|
|
* The bash tool waits for stdout/stderr to close, not only for the shell to exit (#5303). A background
|
|
* process that inherited those pipes and keeps writing keeps the tool waiting after the shell exited.
|
|
* When the timeout then hit, the tool reported only "Command timed out", so the shell's real exit code
|
|
* and the fact that it had already finished were lost. The tool now reports both.
|
|
*
|
|
* Unix only: on Windows the deadline cannot kill background processes once the shell pid is gone, so the
|
|
* tool keeps waiting until their output pauses; this test would hang there.
|
|
*/
|
|
describe.skipIf(process.platform === "win32")("issue #11613 bash timeout after the shell exited", () => {
|
|
const bashTool = createBashTool(tmpdir());
|
|
|
|
async function runExpectingTimeout(command: string, timeout: number): Promise<string> {
|
|
const error = await bashTool.execute("test-11613", { command, timeout }).then(
|
|
() => undefined,
|
|
(err: unknown) => err,
|
|
);
|
|
expect(error).toBeInstanceOf(Error);
|
|
return (error as Error).message;
|
|
}
|
|
|
|
it("reports the shell's exit code when background processes kept writing until the timeout", async () => {
|
|
const message = await runExpectingTimeout(
|
|
"(while true; do echo tick; sleep 0.02; done) & sleep 0.2; echo boom >&2; exit 3",
|
|
1,
|
|
);
|
|
expect(message).toContain("boom");
|
|
expect(message).toMatch(
|
|
/Command timed out after 1 seconds\. The shell exited with code 3 after 0\.\d seconds, but background processes it started kept writing output until the timeout\.$/,
|
|
);
|
|
});
|
|
|
|
it("keeps the plain timeout message when the shell was still running at the deadline", async () => {
|
|
const message = await runExpectingTimeout("echo started; sleep 30", 0.3);
|
|
expect(message).toMatch(/started\s+Command timed out after 0\.3 seconds$/);
|
|
});
|
|
});
|