diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 76af5a22..7b89b46e 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixed +- Fixed oversized harness shell execution timeouts to fail with a clear validation error instead of being clamped to an immediate timeout ([#6181](https://github.com/earendil-works/pi/issues/6181)). - Fixed `Agent.prepareNextTurn` to keep receiving the run abort signal instead of the next-turn context. ## [0.80.2] - 2026-06-23 diff --git a/packages/agent/src/harness/env/nodejs.ts b/packages/agent/src/harness/env/nodejs.ts index 3d929c82..4cf3331e 100644 --- a/packages/agent/src/harness/env/nodejs.ts +++ b/packages/agent/src/harness/env/nodejs.ts @@ -28,6 +28,23 @@ import { toError, } from "../types.ts"; +const MAX_TIMEOUT_MS = 2_147_483_647; +const MAX_TIMEOUT_SECONDS = MAX_TIMEOUT_MS / 1000; + +function resolveTimeoutMs(timeout: number | undefined): Result { + if (timeout === undefined) return ok(undefined); + if (!Number.isFinite(timeout)) { + return err(new ExecutionError("timeout", "Invalid timeout: must be a finite number of seconds")); + } + if (timeout <= 0) return ok(undefined); + + const timeoutMs = timeout * 1000; + if (timeoutMs > MAX_TIMEOUT_MS) { + return err(new ExecutionError("timeout", `Invalid timeout: maximum is ${MAX_TIMEOUT_SECONDS} seconds`)); + } + return ok(timeoutMs); +} + function resolvePath(cwd: string, path: string): string { return isAbsolute(path) ? path : resolve(cwd, path); } @@ -258,6 +275,9 @@ export class NodeExecutionEnv implements ExecutionEnv { }, ): Promise> { if (options?.abortSignal?.aborted) return err(new ExecutionError("aborted", "aborted")); + const timeoutMsResult = resolveTimeoutMs(options?.timeout); + if (!timeoutMsResult.ok) return err(timeoutMsResult.error); + const timeoutMs = timeoutMsResult.value; const cwd = options?.cwd ? resolvePath(this.cwd, options.cwd) : this.cwd; const shellConfig = await getShellConfig(this.shellPath); @@ -310,13 +330,13 @@ export class NodeExecutionEnv implements ExecutionEnv { } timeoutId = - typeof options?.timeout === "number" + timeoutMs !== undefined ? setTimeout(() => { timedOut = true; if (child?.pid) { killProcessTree(child.pid); } - }, options.timeout * 1000) + }, timeoutMs) : undefined; if (options?.abortSignal) { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c5e0ca1d..0ee4422a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed oversized bash tool timeouts to fail with a clear validation error instead of being clamped to an immediate timeout ([#6181](https://github.com/earendil-works/pi/issues/6181)). + ## [0.80.3] - 2026-06-30 ### New Features diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index da6934e7..d5e080cc 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -21,6 +21,23 @@ import { getTextOutput, invalidArgText, str } from "./render-utils.ts"; import { wrapToolDefinition } from "./tool-definition-wrapper.ts"; import { DEFAULT_MAX_BYTES, DEFAULT_MAX_LINES, formatSize, type TruncationResult } from "./truncate.ts"; +const MAX_TIMEOUT_MS = 2_147_483_647; +const MAX_TIMEOUT_SECONDS = MAX_TIMEOUT_MS / 1000; + +function resolveTimeoutMs(timeout: number | undefined): number | undefined { + if (timeout === undefined) return undefined; + if (!Number.isFinite(timeout)) { + throw new Error("Invalid timeout: must be a finite number of seconds"); + } + if (timeout <= 0) return undefined; + + const timeoutMs = timeout * 1000; + if (timeoutMs > MAX_TIMEOUT_MS) { + throw new Error(`Invalid timeout: maximum is ${MAX_TIMEOUT_SECONDS} seconds`); + } + return timeoutMs; +} + const bashSchema = Type.Object({ command: Type.String({ description: "Bash command to execute" }), timeout: Type.Optional(Type.Number({ description: "Timeout in seconds (optional, no default timeout)" })), @@ -66,15 +83,16 @@ export interface BashOperations { export function createLocalBashOperations(options?: { shellPath?: string }): BashOperations { return { exec: async (command, cwd, { onData, signal, timeout, env }) => { + const timeoutMs = resolveTimeoutMs(timeout); + if (signal?.aborted) { + throw new Error("aborted"); + } const shellConfig = getShellConfig(options?.shellPath); try { await fsAccess(cwd, constants.F_OK); } catch { throw new Error(`Working directory does not exist: ${cwd}\nCannot execute bash commands.`); } - if (signal?.aborted) { - throw new Error("aborted"); - } const commandFromStdin = shellConfig.commandTransport === "stdin"; const child = spawn(shellConfig.shell, commandFromStdin ? shellConfig.args : [...shellConfig.args, command], { @@ -97,11 +115,11 @@ export function createLocalBashOperations(options?: { shellPath?: string }): Bas try { // Set timeout if provided. - if (timeout !== undefined && timeout > 0) { + if (timeoutMs !== undefined) { timeoutHandle = setTimeout(() => { timedOut = true; if (child.pid) killProcessTree(child.pid); - }, timeout * 1000); + }, timeoutMs); } // Stream stdout and stderr. child.stdout?.on("data", onData);