fix(coding-agent): cap pi.exec output at 256 MiB (#11546)

Closes #11360.

## Summary
- Cap combined stdout/stderr with `ExecOptions.maxBuffer`, defaulting to
the smaller of 256 MiB and the runtime string limit. Invalid explicit
limits reject before spawning.
- Terminate oversized commands and return `truncated: true`, `killed:
true`, and a nonzero exit code. Close output pipes so writing
descendants cannot keep the call pending, and discard incomplete UTF-8
suffixes rather than adding replacement characters.
- Fix SIGKILL escalation for children that ignore SIGTERM, and simplify
completion to `await` with one cleanup block and result.
- Document the API and add regressions for byte limits, shell
descendants, UTF-8 chunk boundaries, and 32-bit runtime limits.

`ExecResult.truncated` is required. Existing extensions reading
execution results continue working; typed mocks or wrappers constructing
results must add the field.

## Verification
- `npm run build` and the post-rebase `npm run build:offline`
- `npm run check`
- 39 focused execution and extension tests
- `./test.sh` passes after building the env daemon. Earlier runs hit
remote-cleanup and reftable-watcher timeouts; both passed in focused
reruns and the final full run.

packages/coding-agent/CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Added
 
 - Extension packages can now show their changelog after an update. Pi reads `pi.changelogPath` from the package's `package.json`, or `CHANGELOG.md` in the package root, and on the next fresh launch shows entries newer than the last seen version under "Extension Updates". The `collapseChangelog` setting condenses this to one line per package ([#5958](https://github.com/earendil-works/pi/issues/5958), [#8790](https://github.com/earendil-works/pi/pull/8790))
+- `pi.exec()` accepts a `maxBuffer` option that limits stdout and stderr combined, and its result has a `truncated` field that is true when output exceeded the limit ([#11360](https://github.com/earendil-works/pi/issues/11360))
 - Device-code login (for example GitHub Copilot) now opens the verification page and copies the code when you press Enter. Providers can turn either action off with `openBrowserOnConfirm: false` or `copyCodeOnConfirm: false` ([#9282](https://github.com/earendil-works/pi/issues/9282), [#9301](https://github.com/earendil-works/pi/pull/9301))
 
 ### Changed
@@ -15,6 +16,8 @@
 ### Fixed
 
 - Fixed a redundant system prompt update (tools, rules) when an extension set the same active tools in a different order, for example a reversed `pi.setActiveTools()` list. Tools that stay active now keep their order and newly activated tools are appended ([#11495](https://github.com/earendil-works/pi/issues/11495))
+- Fixed `pi.exec()` crashing pi with `RangeError: Invalid string length` when a command printed more output than fits in a string. The default output cap is the smaller of 256 MiB and the runtime string limit; past the cap, the command is terminated and the result has `truncated: true` and a nonzero exit code ([#11360](https://github.com/earendil-works/pi/issues/11360))
+- Fixed `pi.exec()` never returning when a command ignored SIGTERM after a timeout or abort. pi now sends SIGKILL 5 seconds later, as intended
 - Fixed `pi install` and `pi remove` adding ever deeper nested entries to the npm `package-lock.json` when the agent directory is reached through a symlink, such as `~/.pi` linked into a dotfiles repo. pi now passes the resolved real path to npm ([#11358](https://github.com/earendil-works/pi/issues/11358))
 - MCP sign-in now rejects authorization endpoints and other OAuth metadata URLs that are not `http:` or `https:`. Before, a malicious MCP server could supply a `file:` or custom protocol URL, which pi passed to the platform URL handler.
 - Automatic `fd` and `rg` downloads no longer fetch x86-64 binaries on ARM32 and other architectures without a prebuilt release. pi now warns and recommends installing the tool with the system package manager ([#11209](https://github.com/earendil-works/pi/issues/11209))

packages/coding-agent/docs/extensions.md

@@ -1868,13 +1868,15 @@ pi.on("session_start", () => {
 
 ### pi.exec(command, args, options?)
 
-Execute a shell command.
+Execute a command without a shell.
 
 ```typescript
 const result = await pi.exec("git", ["status"], { signal, timeout: 5000 });
-// result.stdout, result.stderr, result.code, result.killed
+// result.stdout, result.stderr, result.code, result.killed, result.truncated
 ```
 
+Options: `signal` and `timeout` terminate the command, `cwd` sets the working directory, and `maxBuffer` limits stdout and stderr combined (default 256 MiB). When output exceeds `maxBuffer`, pi terminates the command and returns the output up to the limit with `truncated: true`, `killed: true`, and a nonzero `code`. `killed` is true whenever pi terminated the command: timeout, abort, or `maxBuffer`. A command that ignores SIGTERM gets SIGKILL after 5 seconds.
+
 ### pi.getActiveTools() / pi.getAllTools() / pi.setActiveTools(names)
 
 Manage active tools. This works for both built-in tools and dynamically registered tools. `pi.getActiveTools()` returns the active tool names as `string[]`; `pi.getAllTools()` returns metadata for all configured tools.

packages/coding-agent/src/core/exec.ts

@@ -2,10 +2,14 @@
  * Shared command execution utilities for extensions and custom tools.
  */
 
+import { constants as bufferConstants } from "node:buffer";
 import { spawn } from "node:child_process";
 import { StringDecoder } from "node:string_decoder";
 import { waitForChildProcess } from "../utils/child-process.ts";
 
+const DEFAULT_EXEC_MAX_BUFFER = Math.min(256 * 1024 * 1024, bufferConstants.MAX_STRING_LENGTH);
+const FORCE_KILL_DELAY_MS = 5000;
+
 /**
  * Options for executing shell commands.
  */
@@ -16,6 +20,11 @@ export interface ExecOptions {
 	timeout?: number;
 	/** Working directory */
 	cwd?: string;
+	/**
+	 * Combined stdout/stderr byte limit; exceeding it terminates the command with `truncated: true`.
+	 * Nonnegative integer up to the runtime string limit. Default: 256 MiB, capped by that limit.
+	 */
+	maxBuffer?: number;
 }
 
 /**
@@ -25,12 +34,15 @@ export interface ExecResult {
 	stdout: string;
 	stderr: string;
 	code: number;
+	/** The command was terminated by pi: timeout, abort, or output over `maxBuffer`. */
 	killed: boolean;
+	/** Output exceeded `maxBuffer`; stdout and stderr are incomplete and `code` is nonzero. */
+	truncated: boolean;
 }
 
 /**
  * Execute a shell command and return stdout/stderr/code.
- * Supports timeout and abort signal.
+ * Supports timeout, abort signal, and an output cap (`maxBuffer`).
  */
 export async function execCommand(
 	command: string,
@@ -38,79 +50,109 @@ export async function execCommand(
 	cwd: string,
 	options?: ExecOptions,
 ): Promise<ExecResult> {
-	return new Promise((resolve) => {
-		const proc = spawn(command, args, {
-			cwd,
-			shell: false,
-			stdio: ["ignore", "pipe", "pipe"],
-		});
+	const maxBuffer = options?.maxBuffer ?? DEFAULT_EXEC_MAX_BUFFER;
+	if (!Number.isInteger(maxBuffer) || maxBuffer < 0 || maxBuffer > bufferConstants.MAX_STRING_LENGTH) {
+		throw new RangeError(`maxBuffer must be an integer between 0 and ${bufferConstants.MAX_STRING_LENGTH} bytes`);
+	}
 
-		let stdout = "";
-		let stderr = "";
-		// Decode across chunks: a multi-byte UTF-8 character can be split between two chunks,
-		// and decoding each chunk separately would corrupt it (earendil-works/pi#11312).
-		const stdoutDecoder = new StringDecoder("utf8");
-		const stderrDecoder = new StringDecoder("utf8");
-		let killed = false;
-		let timeoutId: NodeJS.Timeout | undefined;
-
-		const killProcess = () => {
-			if (!killed) {
-				killed = true;
-				proc.kill("SIGTERM");
-				// Force kill after 5 seconds if SIGTERM doesn't work
-				setTimeout(() => {
-					if (!proc.killed) {
-						proc.kill("SIGKILL");
-					}
-				}, 5000);
-			}
-		};
-
-		// Handle abort signal
-		if (options?.signal) {
-			if (options.signal.aborted) {
-				killProcess();
-			} else {
-				options.signal.addEventListener("abort", killProcess, { once: true });
-			}
-		}
-
-		// Handle timeout
-		if (options?.timeout && options.timeout > 0) {
-			timeoutId = setTimeout(() => {
-				killProcess();
-			}, options.timeout);
-		}
-
-		proc.stdout?.on("data", (data) => {
-			stdout += stdoutDecoder.write(data);
-		});
-
-		proc.stderr?.on("data", (data) => {
-			stderr += stderrDecoder.write(data);
-		});
-
-		// Wait for process termination without hanging on inherited stdio handles
-		// held open by detached descendants.
-		waitForChildProcess(proc)
-			.then((code) => {
-				if (timeoutId) clearTimeout(timeoutId);
-				stdout += stdoutDecoder.end();
-				stderr += stderrDecoder.end();
-				if (options?.signal) {
-					options.signal.removeEventListener("abort", killProcess);
-				}
-				resolve({ stdout, stderr, code: code ?? 0, killed });
-			})
-			.catch((error: Error) => {
-				if (timeoutId) clearTimeout(timeoutId);
-				stdout += stdoutDecoder.end();
-				stderr += stderrDecoder.end();
-				if (options?.signal) {
-					options.signal.removeEventListener("abort", killProcess);
-				}
-				resolve({ stdout, stderr: stderr || error.message, code: 1, killed });
-			});
+	const proc = spawn(command, args, {
+		cwd,
+		shell: false,
+		stdio: ["ignore", "pipe", "pipe"],
 	});
+
+	let stdout = "";
+	let stderr = "";
+	// Decode across chunks: a multi-byte UTF-8 character can be split between two chunks,
+	// and decoding each chunk separately would corrupt it (earendil-works/pi#11312).
+	const stdoutDecoder = new StringDecoder("utf8");
+	const stderrDecoder = new StringDecoder("utf8");
+	let outputBytes = 0;
+	let truncated = false;
+	let killed = false;
+	let timeoutId: NodeJS.Timeout | undefined;
+	let forceKillId: NodeJS.Timeout | undefined;
+
+	const killProcess = () => {
+		if (killed) return;
+		killed = true;
+		proc.kill("SIGTERM");
+		forceKillId = setTimeout(() => {
+			// `proc.killed` only says a signal was sent, so check whether the child actually exited.
+			if (proc.exitCode === null && proc.signalCode === null) {
+				proc.kill("SIGKILL");
+			}
+		}, FORCE_KILL_DELAY_MS);
+	};
+
+	if (options?.signal) {
+		if (options.signal.aborted) {
+			killProcess();
+		} else {
+			options.signal.addEventListener("abort", killProcess, { once: true });
+		}
+	}
+
+	if (options?.timeout && options.timeout > 0) {
+		timeoutId = setTimeout(killProcess, options.timeout);
+	}
+
+	// Keep only bytes within the combined cap (#11360). Close both pipes on overflow so writing
+	// descendants cannot keep waitForChildProcess busy after the child exits. Pausing is not
+	// enough: Node resumes the child's stdio on exit.
+	const takeWithinCap = (data: Buffer): Buffer | undefined => {
+		if (truncated) return undefined;
+		const remaining = maxBuffer - outputBytes;
+		if (data.length <= remaining) {
+			outputBytes += data.length;
+			return data;
+		}
+		truncated = true;
+		proc.stdout?.destroy();
+		proc.stderr?.destroy();
+		killProcess();
+		return remaining > 0 ? data.subarray(0, remaining) : undefined;
+	};
+
+	proc.stdout?.on("data", (data: Buffer) => {
+		const chunk = takeWithinCap(data);
+		if (chunk) stdout += stdoutDecoder.write(chunk);
+	});
+
+	proc.stderr?.on("data", (data: Buffer) => {
+		const chunk = takeWithinCap(data);
+		if (chunk) stderr += stderrDecoder.write(chunk);
+	});
+
+	let code: number | null;
+	try {
+		// Do not hang on inherited stdio handles held open by detached descendants.
+		code = await waitForChildProcess(proc);
+	} catch (error) {
+		if (!(error instanceof Error)) throw error;
+		// Child-process errors (e.g. ENOENT) are failed executions, not host failures.
+		code = 1;
+		if (!stderr) stderr = error.message;
+	} finally {
+		clearTimeout(timeoutId);
+		clearTimeout(forceKillId);
+		options?.signal?.removeEventListener("abort", killProcess);
+	}
+
+	// On truncation, discard any incomplete UTF-8 suffix buffered by either decoder.
+	// Flushing it would append a replacement character that was not in the accepted output.
+	if (!truncated) {
+		stdout += stdoutDecoder.end();
+		stderr += stderrDecoder.end();
+	}
+
+	const exitCode = code ?? 0;
+	return {
+		stdout,
+		stderr,
+		// A truncated result must never read as success.
+		code: truncated && exitCode === 0 ? 1 : exitCode,
+		killed,
+		truncated,
+	};
 }

packages/coding-agent/test/exec-runtime-limit.test.ts

@@ -0,0 +1,25 @@
+import * as buffer from "node:buffer";
+import { expect, it, vi } from "vitest";
+import { execCommand } from "../src/core/exec.ts";
+
+vi.mock("node:buffer", async (importOriginal) => {
+	const actual = await importOriginal<typeof buffer>();
+	return {
+		...actual,
+		// 32-bit Node's maximum is 16 bytes below 256 MiB.
+		constants: { ...actual.constants, MAX_STRING_LENGTH: 268_435_440 },
+	};
+});
+
+// Regression tests for https://github.com/earendil-works/pi/issues/11360
+it("executes with the default cap when the runtime string limit is below 256 MiB", async () => {
+	const result = await execCommand(process.execPath, ["-e", 'process.stdout.write("ok")'], process.cwd());
+
+	expect(result).toEqual({ stdout: "ok", stderr: "", code: 0, killed: false, truncated: false });
+});
+
+it("still rejects an explicit cap above the runtime string limit", async () => {
+	await expect(
+		execCommand(process.execPath, ["-e", ""], process.cwd(), { maxBuffer: buffer.constants.MAX_STRING_LENGTH + 1 }),
+	).rejects.toThrow(RangeError);
+});

packages/coding-agent/test/exec-utf8-chunks.test.ts

@@ -0,0 +1,53 @@
+import type * as ChildProcessModule from "node:child_process";
+import { ChildProcess, spawn } from "node:child_process";
+import { PassThrough } from "node:stream";
+import { setImmediate } from "node:timers/promises";
+import { expect, it, vi } from "vitest";
+import { execCommand } from "../src/core/exec.ts";
+
+vi.mock("node:child_process", async (importOriginal) => ({
+	...(await importOriginal<typeof ChildProcessModule>()),
+	spawn: vi.fn(),
+}));
+
+// #11360: real streams control chunk boundaries without relying on OS pipe timing.
+// Only process creation and termination are stubbed; decoding and process waiting are real.
+it.each(["stdout", "stderr"] as const)(
+	"discards pending %s UTF-8 bytes when either stream exceeds the cap",
+	async (stream) => {
+		const child = new ChildProcess();
+		const stdout = new PassThrough();
+		const stderr = new PassThrough();
+		child.stdout = stdout;
+		child.stderr = stderr;
+		vi.mocked(spawn).mockReturnValueOnce(child);
+		vi.spyOn(child, "kill").mockImplementation(() => {
+			queueMicrotask(() => {
+				child.emit("exit", null, "SIGTERM");
+				child.emit("close", null, "SIGTERM");
+			});
+			return true;
+		});
+
+		const resultPromise = execCommand("test-command", [], process.cwd(), { maxBuffer: 3 });
+		const output = stream === "stdout" ? stdout : stderr;
+		const otherOutput = stream === "stdout" ? stderr : stdout;
+		// The decoder first returns "a" and buffers the first byte of €.
+		output.write(Buffer.from([0x61, 0xe2]));
+		await setImmediate();
+		// The other decoder also buffers the first byte of € before the shared cap is exceeded.
+		otherOutput.write(Buffer.from([0xe2]));
+		await setImmediate();
+		output.write(Buffer.from([0x82, 0xac]));
+
+		const result = await resultPromise;
+		expect(result).toMatchObject({
+			stdout: stream === "stdout" ? "a" : "",
+			stderr: stream === "stderr" ? "a" : "",
+			truncated: true,
+			killed: true,
+		});
+		expect(result.code).not.toBe(0);
+		expect(Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(3);
+	},
+);

packages/coding-agent/test/exec.test.ts

@@ -1,3 +1,4 @@
+import { constants as bufferConstants } from "node:buffer";
 import { describe, expect, it } from "vitest";
 import { execCommand } from "../src/core/exec.ts";
 
@@ -9,6 +10,7 @@ describe("execCommand", () => {
 		expect(result).toMatchObject({
 			code: 1,
 			killed: false,
+			truncated: false,
 			stdout: "",
 		});
 		expect(result.stderr).toMatch(/ENOENT/);
@@ -17,6 +19,110 @@ describe("execCommand", () => {
 	it("keeps stderr empty when a command exits silently", async () => {
 		const result = await execCommand(process.execPath, ["-e", "process.exit(1)"], process.cwd());
 
-		expect(result).toEqual({ stdout: "", stderr: "", code: 1, killed: false });
+		expect(result).toEqual({ stdout: "", stderr: "", code: 1, killed: false, truncated: false });
 	});
+
+	// Regression tests for https://github.com/earendil-works/pi/issues/11360
+	it.each([
+		-1,
+		0.5,
+		Number.NaN,
+		Number.POSITIVE_INFINITY,
+		Number.NEGATIVE_INFINITY,
+		bufferConstants.MAX_STRING_LENGTH + 1,
+	])("rejects invalid maxBuffer %s instead of attempting execution", async (maxBuffer) => {
+		// A missing command normally resolves to a failed result; invalid options must reject instead.
+		await expect(
+			execCommand("pi-command-that-does-not-exist-11360", [], process.cwd(), { maxBuffer }),
+		).rejects.toThrow(RangeError);
+	});
+
+	it.each([0, bufferConstants.MAX_STRING_LENGTH])("accepts maxBuffer boundary %s", async (maxBuffer) => {
+		const result = await execCommand(process.execPath, ["-e", ""], process.cwd(), { maxBuffer });
+
+		expect(result).toEqual({ stdout: "", stderr: "", code: 0, killed: false, truncated: false });
+	});
+
+	it("truncates all output when maxBuffer is zero", async () => {
+		const result = await execCommand(process.execPath, ["-e", 'process.stdout.write("x")'], process.cwd(), {
+			maxBuffer: 0,
+		});
+
+		expect(result).toMatchObject({ stdout: "", stderr: "", truncated: true, killed: true });
+		expect(result.code).not.toBe(0);
+	});
+
+	it("caps combined output at maxBuffer, terminates the command, and reports truncation", async () => {
+		const script =
+			'process.stderr.write("e".repeat(600)); process.stdout.write("o".repeat(600)); setInterval(() => process.stdout.write("x".repeat(1000)), 10);';
+		const result = await execCommand(process.execPath, ["-e", script], process.cwd(), { maxBuffer: 1000 });
+
+		expect(result).toMatchObject({ truncated: true, killed: true });
+		expect(result.code).not.toBe(0);
+		expect(Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr)).toBe(1000);
+	});
+
+	// #11360: a byte cap must not turn a partial UTF-8 character into replacement output.
+	it.each([
+		["stdout", "€", 1, ""],
+		["stderr", "€", 1, ""],
+		["stdout", "a€🙂", 5, "a€"],
+		["stderr", "a€🙂", 5, "a€"],
+	] as const)(
+		"keeps complete UTF-8 characters from %s output %s capped at %i bytes",
+		async (stream, output, maxBuffer, expected) => {
+			const script = `process.${stream}.write(${JSON.stringify(output)})`;
+			const result = await execCommand(process.execPath, ["-e", script], process.cwd(), { maxBuffer });
+
+			expect(result).toMatchObject({ [stream]: expected, truncated: true, killed: true });
+			expect(result.code).not.toBe(0);
+			expect(Buffer.byteLength(result.stdout) + Buffer.byteLength(result.stderr)).toBeLessThanOrEqual(maxBuffer);
+		},
+	);
+
+	it("still flushes incomplete UTF-8 at normal EOF when output was not truncated", async () => {
+		const script = "process.stdout.write(Buffer.from([0xe2])); process.stderr.write(Buffer.from([0xf0]));";
+		const result = await execCommand(process.execPath, ["-e", script], process.cwd());
+
+		expect(result).toEqual({ stdout: "<22>", stderr: "<22>", code: 0, killed: false, truncated: false });
+	});
+
+	it("leaves output up to maxBuffer unchanged", async () => {
+		const result = await execCommand(
+			process.execPath,
+			["-e", 'process.stdout.write("a".repeat(1000))'],
+			process.cwd(),
+			{
+				maxBuffer: 1000,
+			},
+		);
+
+		expect(result).toEqual({ stdout: "a".repeat(1000), stderr: "", code: 0, killed: false, truncated: false });
+	});
+
+	it.skipIf(process.platform === "win32")(
+		"returns when a shell's child keeps writing after the shell is terminated",
+		async () => {
+			// SIGTERM kills only `sh`; `yes` keeps the output pipes open and writing.
+			const result = await execCommand("sh", ["-c", "yes; echo done"], process.cwd(), { maxBuffer: 1000 });
+
+			expect(result).toMatchObject({ killed: true, truncated: true });
+			expect(result.stdout).toBe("y\n".repeat(500));
+		},
+		10_000,
+	);
+
+	it.skipIf(process.platform === "win32")(
+		"force-kills a command that ignores SIGTERM",
+		async () => {
+			// The handler is installed before the first write, so the kill triggered by maxBuffer
+			// cannot arrive before the child ignores SIGTERM.
+			const script = 'process.on("SIGTERM", () => {}); process.stdout.write("xx"); setInterval(() => {}, 1000);';
+			const result = await execCommand(process.execPath, ["-e", script], process.cwd(), { maxBuffer: 1 });
+
+			expect(result).toMatchObject({ stdout: "x", killed: true, truncated: true });
+			expect(result.code).not.toBe(0);
+		},
+		15_000,
+	);
 });

packages/coding-agent/test/git-merge-and-resolve-extension.test.ts

@@ -7,8 +7,8 @@ import type { ExecResult, ExtensionAPI, ExtensionContext } from "../src/core/ext
 
 type AgentEndHandler = (event: { type: "agent_end" }, ctx: ExtensionContext) => Promise<undefined>;
 
-const ok: ExecResult = { stdout: "", stderr: "", code: 0, killed: false };
-const fail: ExecResult = { stdout: "", stderr: "error", code: 1, killed: false };
+const ok: ExecResult = { stdout: "", stderr: "", code: 0, killed: false, truncated: false };
+const fail: ExecResult = { stdout: "", stderr: "error", code: 1, killed: false, truncated: false };
 
 /** Standard exec results for a clean repo tracking origin/main, not in a merge. */
 function withUpstream(results: Map<string, ExecResult>): Map<string, ExecResult> {

packages/coding-agent/test/input-transform-streaming-example.test.ts

@@ -35,9 +35,9 @@ function setup(execResult: ExecResult) {
 
 describe("input-transform-streaming example", () => {
 	const diffOutput = " src/index.ts | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)";
-	const gitSuccess: ExecResult = { stdout: diffOutput, stderr: "", code: 0, killed: false };
-	const gitEmpty: ExecResult = { stdout: "", stderr: "", code: 0, killed: false };
-	const gitFail: ExecResult = { stdout: "", stderr: "not a git repo", code: 128, killed: false };
+	const gitSuccess: ExecResult = { stdout: diffOutput, stderr: "", code: 0, killed: false, truncated: false };
+	const gitEmpty: ExecResult = { stdout: "", stderr: "", code: 0, killed: false, truncated: false };
+	const gitFail: ExecResult = { stdout: "", stderr: "not a git repo", code: 128, killed: false, truncated: false };
 
 	it("skips exec during steering", async () => {
 		const { emit, exec } = setup(gitSuccess);