fix(coding-agent): report the shell exit code when bash times out after the shell exited

A background process that inherited the output pipes and kept writing made the
tool wait past the shell's exit; the timeout then hid the real exit code.

fixes #11613

packages/coding-agent/CHANGELOG.md

@@ -19,6 +19,7 @@
 
 ### Fixed
 
+- Fixed the bash tool reporting only `Command timed out` when the command had already exited but a background process it started kept writing to its output until the timeout. The error now also reports the shell's exit code and when it exited ([#11613](https://github.com/earendil-works/pi/issues/11613))
 - Images in steering and follow-up messages, including `prompt()` while streaming, are now resized like images in idle prompts instead of being sent and saved at full size ([#11738](https://github.com/earendil-works/pi/issues/11738))
 - A failed token refresh of an MCP connection no longer registers an OAuth client or replaces the PKCE verifier of a sign-in in progress, which made that sign-in fail. MCP sign-in now refreshes the stored tokens with the stored client even when the loopback callback port changed, instead of registering a new client and opening the browser. Ctrl+C in `/mcp` and the `--timeout` of `pi mcp login` now also stop a sign-in while it waits up to 25 seconds for another pi process's token refresh, and a refresh lock that fails for another reason than that wait now fails the sign-in instead of opening the browser.
 - Fixed `pi mcp list` reporting MCP servers with `"auth": { "provider": "<provider>" }`, such as Radius, as needing sign-in although the provider was signed in: the shell command sent no token. It now sends the provider's `/login` token and refreshes it when it expired. For these servers the sign-in hint names `/login <provider>` instead of `pi mcp login`, and `pi mcp login` and `pi mcp logout` point to `/login` and `/logout` instead of failing with "does not use OAuth"

packages/coding-agent/src/core/tools/bash.ts

@@ -92,6 +92,20 @@ export interface BashOperations {
 	) => Promise<{ exitCode: number | null }>;
 }
 
+/**
+ * Timeout of the local shell backend. Its message keeps the `timeout:<seconds>` form custom operations throw.
+ * `shellExit` is set when the shell itself exited before the deadline, so the tool was only still waiting
+ * because background processes kept writing to the inherited stdout/stderr pipes (#11613).
+ */
+class ShellTimeoutError extends Error {
+	readonly shellExit: { exitCode: number; afterSeconds: number } | undefined;
+
+	constructor(timeout: number | undefined, shellExit: { exitCode: number; afterSeconds: number } | undefined) {
+		super(`timeout:${timeout}`);
+		this.shellExit = shellExit;
+	}
+}
+
 /** Shared process execution used by the built-in shell tools. */
 export function createLocalShellOperations(shellName: string, resolveShellConfig: () => ShellConfig): BashOperations {
 	return {
@@ -120,7 +134,13 @@ export function createLocalShellOperations(shellName: string, resolveShellConfig
 				child.stdin?.end(command);
 			}
 			if (child.pid) trackDetachedChildPid(child.pid);
+			const startedAt = performance.now();
+			let shellExitedAfterMs: number | undefined;
+			child.once("exit", () => {
+				shellExitedAfterMs = performance.now() - startedAt;
+			});
 			let timedOut = false;
+			let exitedBeforeDeadlineAfterMs: number | undefined;
 			let timeoutHandle: NodeJS.Timeout | undefined;
 			const onAbort = () => {
 				if (child.pid) killProcessTree(child.pid);
@@ -131,6 +151,9 @@ export function createLocalShellOperations(shellName: string, resolveShellConfig
 				if (timeoutMs !== undefined) {
 					timeoutHandle = setTimeout(() => {
 						timedOut = true;
+						exitedBeforeDeadlineAfterMs = shellExitedAfterMs;
+						// Also when the shell already exited: on Unix the group kill still reaches background
+						// processes holding the pipes. Windows cannot find them once the shell pid is gone.
 						if (child.pid) killProcessTree(child.pid);
 					}, timeoutMs);
 				}
@@ -148,13 +171,19 @@ export function createLocalShellOperations(shellName: string, resolveShellConfig
 				if (signal?.aborted) {
 					throw new Error("aborted");
 				}
-				if (timedOut) {
-					throw new Error(`timeout:${timeout}`);
-				}
 				// A signal-killed shell has no exit code. Use the standard shell convention so
 				// callers do not mistake the termination for a successful command.
 				const signalCode = child.signalCode;
-				return { exitCode: exitCode ?? (signalCode ? 128 + (osConstants.signals[signalCode] ?? 0) : 1) };
+				const resolvedExitCode = exitCode ?? (signalCode ? 128 + (osConstants.signals[signalCode] ?? 0) : 1);
+				if (timedOut) {
+					throw new ShellTimeoutError(
+						timeout,
+						exitedBeforeDeadlineAfterMs !== undefined
+							? { exitCode: resolvedExitCode, afterSeconds: Math.round(exitedBeforeDeadlineAfterMs / 100) / 10 }
+							: undefined,
+					);
+				}
+				return { exitCode: resolvedExitCode };
 			} finally {
 				if (child.pid) untrackDetachedChildPid(child.pid);
 				if (timeoutHandle) clearTimeout(timeoutHandle);
@@ -375,7 +404,13 @@ export function createShellToolDefinition(
 					}
 					if (err instanceof Error && err.message.startsWith("timeout:")) {
 						const timeoutSecs = err.message.split(":")[1];
-						throw new Error(appendStatus(text, `Command timed out after ${timeoutSecs} seconds`));
+						let status = `Command timed out after ${timeoutSecs} seconds`;
+						if (err instanceof ShellTimeoutError && err.shellExit) {
+							// Both facts matter: the command's real result, and that its background processes
+							// were still writing output when the timeout hit (#11613).
+							status += `. The shell exited with code ${err.shellExit.exitCode} after ${err.shellExit.afterSeconds} seconds, but background processes it started kept writing output until the timeout.`;
+						}
+						throw new Error(appendStatus(text, status));
 					}
 					throw err;
 				}

packages/coding-agent/test/suite/regressions/11613-bash-timeout-after-shell-exit.test.ts

@@ -0,0 +1,43 @@
+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$/);
+	});
+});