mirror of
https://github.com/harivansh-afk/clanker-agent.git
synced 2026-04-15 06:04:40 +00:00
Merge pull request #254 from getcompanion-ai/fix/bash-default-timeout
fix: add default timeout and no-output timeout to bash tool
This commit is contained in:
Hari
GitHub
commit
df702d95a3
1 changed files with 127 additions and 63 deletions
|
|
@ -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<typeof createWriteStream> | 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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue