diff --git a/packages/coding-agent/src/core/tools/bash.ts b/packages/coding-agent/src/core/tools/bash.ts index 02dd4bc..d655ba4 100644 --- a/packages/coding-agent/src/core/tools/bash.ts +++ b/packages/coding-agent/src/core/tools/bash.ts @@ -26,11 +26,23 @@ function getTempFilePath(): string { return join(tmpdir(), `pi-bash-${id}.log`); } +/** + * Default timeout applied when the LLM does not specify one. + * Prevents indefinite hangs from long-running processes (dev servers, watchers, etc.) + */ +const DEFAULT_TIMEOUT_SECONDS = 120; + +/** + * If no stdout/stderr output is received for this many seconds, kill the process. + * Catches processes that are alive but idle (e.g. a backgrounded server after startup). + */ +const DEFAULT_NO_OUTPUT_TIMEOUT_SECONDS = 30; + 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)", + description: `Timeout in seconds. Defaults to ${DEFAULT_TIMEOUT_SECONDS}s if not specified. Use a higher value for long-running builds or installations.`, }), ), }); @@ -176,6 +188,27 @@ function resolveSpawnContext( return spawnHook ? spawnHook(baseContext) : baseContext; } +/** + * Combine multiple AbortSignals into one. Aborts when any input signal fires. + */ +function combineAbortSignals( + ...signals: (AbortSignal | undefined)[] +): AbortSignal | undefined { + const defined = signals.filter((s): s is AbortSignal => s !== undefined); + if (defined.length === 0) return undefined; + if (defined.length === 1) return defined[0]; + + const controller = new AbortController(); + for (const sig of defined) { + if (sig.aborted) { + controller.abort(); + return controller.signal; + } + sig.addEventListener("abort", () => controller.abort(), { once: true }); + } + return controller.signal; +} + export interface BashToolOptions { /** Custom operations for command execution. Default: local shell */ operations?: BashOperations; @@ -183,6 +216,10 @@ export interface BashToolOptions { commandPrefix?: string; /** Hook to adjust command, cwd, or env before execution */ spawnHook?: BashSpawnHook; + /** Default timeout in seconds when LLM does not specify one. Set to 0 to disable. Default: 120 */ + defaultTimeoutSeconds?: number; + /** Kill process if no output received for this many seconds. Set to 0 to disable. Default: 30 */ + noOutputTimeoutSeconds?: number; } export function createBashTool( @@ -192,11 +229,13 @@ export function createBashTool( const ops = options?.operations ?? defaultBashOperations; const commandPrefix = options?.commandPrefix; const spawnHook = options?.spawnHook; + const configDefaultTimeout = options?.defaultTimeoutSeconds ?? DEFAULT_TIMEOUT_SECONDS; + const configNoOutputTimeout = options?.noOutputTimeoutSeconds ?? DEFAULT_NO_OUTPUT_TIMEOUT_SECONDS; return { name: "bash", label: "bash", - description: `Execute a bash command in the current working directory. Returns stdout and stderr. Output is truncated to last ${DEFAULT_MAX_LINES} lines or ${DEFAULT_MAX_BYTES / 1024}KB (whichever is hit first). If truncated, full output is saved to a temp file. Optionally provide a timeout in seconds.`, + description: `Execute a bash command in the current working directory. Returns stdout and stderr. Output is truncated to last ${DEFAULT_MAX_LINES} lines or ${DEFAULT_MAX_BYTES / 1024}KB (whichever is hit first). If truncated, full output is saved to a temp file. Default timeout is ${configDefaultTimeout}s - provide a higher timeout for long builds or installations.`, parameters: bashSchema, execute: async ( _toolCallId: string, @@ -204,6 +243,9 @@ export function createBashTool( signal?: AbortSignal, onUpdate?, ) => { + // Apply default timeout when LLM does not specify one + const effectiveTimeout = timeout ?? (configDefaultTimeout > 0 ? configDefaultTimeout : undefined); + // Apply command prefix if configured (e.g., "shopt -s expand_aliases" for alias support) const resolvedCommand = commandPrefix ? `${commandPrefix}\n${command}` @@ -216,6 +258,33 @@ export function createBashTool( let tempFileStream: ReturnType | undefined; let totalBytes = 0; + // No-output timeout: abort via AbortController when process goes silent. + // This keeps the kill mechanism at the tool level, not inside BashOperations. + let noOutputTimer: NodeJS.Timeout | undefined; + let noOutputTriggered = false; + const noOutputAbort = configNoOutputTimeout > 0 ? new AbortController() : undefined; + + // Combine caller signal and no-output signal + const combinedSignal = noOutputAbort + ? combineAbortSignals(signal, noOutputAbort.signal) + : signal; + + const clearNoOutputTimer = () => { + if (noOutputTimer) { + clearTimeout(noOutputTimer); + noOutputTimer = undefined; + } + }; + + const resetNoOutputTimer = () => { + if (!noOutputAbort) return; + clearNoOutputTimer(); + noOutputTimer = setTimeout(() => { + noOutputTriggered = true; + noOutputAbort.abort(); + }, configNoOutputTimeout * 1000); + }; + // Keep a rolling buffer of the last chunk for tail truncation const chunks: Buffer[] = []; let chunksBytes = 0; @@ -225,6 +294,9 @@ export function createBashTool( const handleData = (data: Buffer) => { totalBytes += data.length; + // Reset no-output timer on every chunk of output + resetNoOutputTimer(); + // Start writing to temp file once we exceed the threshold if (totalBytes > DEFAULT_MAX_BYTES && !tempFilePath) { tempFilePath = getTempFilePath(); @@ -265,86 +337,78 @@ export function createBashTool( } }; + // Collect output and build final text (shared by success and error paths) + const buildOutput = (): { text: string; details?: BashToolDetails } => { + const fullBuffer = Buffer.concat(chunks); + const fullOutput = fullBuffer.toString("utf-8"); + const truncation = truncateTail(fullOutput); + let outputText = truncation.content || "(no output)"; + let details: BashToolDetails | undefined; + + if (truncation.truncated) { + details = { truncation, fullOutputPath: tempFilePath }; + const startLine = truncation.totalLines - truncation.outputLines + 1; + const endLine = truncation.totalLines; + + if (truncation.lastLinePartial) { + const lastLineSize = formatSize( + Buffer.byteLength(fullOutput.split("\n").pop() || "", "utf-8"), + ); + outputText += `\n\n[Showing last ${formatSize(truncation.outputBytes)} of line ${endLine} (line is ${lastLineSize}). Full output: ${tempFilePath}]`; + } else if (truncation.truncatedBy === "lines") { + outputText += `\n\n[Showing lines ${startLine}-${endLine} of ${truncation.totalLines}. Full output: ${tempFilePath}]`; + } else { + outputText += `\n\n[Showing lines ${startLine}-${endLine} of ${truncation.totalLines} (${formatSize(DEFAULT_MAX_BYTES)} limit). Full output: ${tempFilePath}]`; + } + } + + return { text: outputText, details }; + }; + + // Start the no-output timer (will fire if command produces no output at all) + resetNoOutputTimer(); + ops .exec(spawnContext.command, spawnContext.cwd, { onData: handleData, - signal, - timeout, + signal: combinedSignal, + timeout: effectiveTimeout, env: spawnContext.env, }) .then(({ exitCode }) => { - // Close temp file stream - if (tempFileStream) { - tempFileStream.end(); - } + clearNoOutputTimer(); + if (tempFileStream) tempFileStream.end(); - // Combine all buffered chunks - const fullBuffer = Buffer.concat(chunks); - const fullOutput = fullBuffer.toString("utf-8"); + const { text, details } = buildOutput(); - // Apply tail truncation - const truncation = truncateTail(fullOutput); - let outputText = truncation.content || "(no output)"; - - // Build details with truncation info - let details: BashToolDetails | undefined; - - if (truncation.truncated) { - details = { - truncation, - fullOutputPath: tempFilePath, - }; - - // Build actionable notice - const startLine = - truncation.totalLines - truncation.outputLines + 1; - const endLine = truncation.totalLines; - - if (truncation.lastLinePartial) { - // Edge case: last line alone > 30KB - const lastLineSize = formatSize( - Buffer.byteLength( - fullOutput.split("\n").pop() || "", - "utf-8", - ), - ); - outputText += `\n\n[Showing last ${formatSize(truncation.outputBytes)} of line ${endLine} (line is ${lastLineSize}). Full output: ${tempFilePath}]`; - } else if (truncation.truncatedBy === "lines") { - outputText += `\n\n[Showing lines ${startLine}-${endLine} of ${truncation.totalLines}. Full output: ${tempFilePath}]`; - } else { - outputText += `\n\n[Showing lines ${startLine}-${endLine} of ${truncation.totalLines} (${formatSize(DEFAULT_MAX_BYTES)} limit). Full output: ${tempFilePath}]`; - } + if (noOutputTriggered) { + reject(new Error( + `${text}\n\nProcess was killed after ${configNoOutputTimeout}s of no output. If this is a long-running server or build, use a higher timeout or run the process in the background with '&' and redirect output.`, + )); + return; } if (exitCode !== 0 && exitCode !== null) { - outputText += `\n\nCommand exited with code ${exitCode}`; - reject(new Error(outputText)); + reject(new Error(`${text}\n\nCommand exited with code ${exitCode}`)); } else { - resolve({ - content: [{ type: "text", text: outputText }], - details, - }); + resolve({ content: [{ type: "text", text }], details }); } }) .catch((err: Error) => { - // Close temp file stream - if (tempFileStream) { - tempFileStream.end(); - } + clearNoOutputTimer(); + if (tempFileStream) tempFileStream.end(); - // Combine all buffered chunks for error output - const fullBuffer = Buffer.concat(chunks); - let output = fullBuffer.toString("utf-8"); + const { text } = buildOutput(); - if (err.message === "aborted") { - if (output) output += "\n\n"; - output += "Command aborted"; - reject(new Error(output)); + if (noOutputTriggered) { + reject(new Error( + `${text}\n\nProcess was killed after ${configNoOutputTimeout}s of no output. If this is a long-running server or build, use a higher timeout or run the process in the background with '&' and redirect output.`, + )); + } else if (err.message === "aborted") { + reject(new Error(text ? `${text}\n\nCommand aborted` : "Command aborted")); } else if (err.message.startsWith("timeout:")) { const timeoutSecs = err.message.split(":")[1]; - if (output) output += "\n\n"; - output += `Command timed out after ${timeoutSecs} seconds`; - reject(new Error(output)); + reject(new Error(text ? `${text}\n\nCommand timed out after ${timeoutSecs} seconds` : `Command timed out after ${timeoutSecs} seconds`)); } else { reject(err); }