mirror of
https://github.com/harivansh-afk/clanker-agent.git
synced 2026-04-17 05:00:17 +00:00
refactor: clean up no-output timeout architecture
- Remove onKillHandle from BashOperations interface (implementation detail leak) - Use AbortController + combineAbortSignals to kill from tool level instead - Extract shared buildOutput() to deduplicate .then()/.catch() paths - Cleaner separation: BashOperations stays unchanged, timeouts are tool-level Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
479124d945
commit
973baf58e1
1 changed files with 85 additions and 91 deletions
|
|
@ -74,8 +74,6 @@ export interface BashOperations {
|
||||||
signal?: AbortSignal;
|
signal?: AbortSignal;
|
||||||
timeout?: number;
|
timeout?: number;
|
||||||
env?: NodeJS.ProcessEnv;
|
env?: NodeJS.ProcessEnv;
|
||||||
/** Called with a function that can kill the process tree. Used by no-output timeout. */
|
|
||||||
onKillHandle?: (kill: () => void) => void;
|
|
||||||
},
|
},
|
||||||
) => Promise<{ exitCode: number | null }>;
|
) => Promise<{ exitCode: number | null }>;
|
||||||
}
|
}
|
||||||
|
|
@ -84,7 +82,7 @@ export interface BashOperations {
|
||||||
* Default bash operations using local shell
|
* Default bash operations using local shell
|
||||||
*/
|
*/
|
||||||
const defaultBashOperations: BashOperations = {
|
const defaultBashOperations: BashOperations = {
|
||||||
exec: (command, cwd, { onData, signal, timeout, env, onKillHandle }) => {
|
exec: (command, cwd, { onData, signal, timeout, env }) => {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
const { shell, args } = getShellConfig();
|
const { shell, args } = getShellConfig();
|
||||||
|
|
||||||
|
|
@ -104,15 +102,6 @@ const defaultBashOperations: BashOperations = {
|
||||||
stdio: ["ignore", "pipe", "pipe"],
|
stdio: ["ignore", "pipe", "pipe"],
|
||||||
});
|
});
|
||||||
|
|
||||||
// Expose kill handle for no-output timeout
|
|
||||||
if (onKillHandle) {
|
|
||||||
onKillHandle(() => {
|
|
||||||
if (child.pid) {
|
|
||||||
killProcessTree(child.pid);
|
|
||||||
}
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
let timedOut = false;
|
let timedOut = false;
|
||||||
|
|
||||||
// Set timeout if provided
|
// Set timeout if provided
|
||||||
|
|
@ -199,6 +188,27 @@ function resolveSpawnContext(
|
||||||
return spawnHook ? spawnHook(baseContext) : baseContext;
|
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 {
|
export interface BashToolOptions {
|
||||||
/** Custom operations for command execution. Default: local shell */
|
/** Custom operations for command execution. Default: local shell */
|
||||||
operations?: BashOperations;
|
operations?: BashOperations;
|
||||||
|
|
@ -248,18 +258,30 @@ export function createBashTool(
|
||||||
let tempFileStream: ReturnType<typeof createWriteStream> | undefined;
|
let tempFileStream: ReturnType<typeof createWriteStream> | undefined;
|
||||||
let totalBytes = 0;
|
let totalBytes = 0;
|
||||||
|
|
||||||
// No-output timeout: kill process if it goes silent
|
// 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 noOutputTimer: NodeJS.Timeout | undefined;
|
||||||
let noOutputTriggered = false;
|
let noOutputTriggered = false;
|
||||||
// Store kill function to call from no-output timeout
|
const noOutputAbort = configNoOutputTimeout > 0 ? new AbortController() : undefined;
|
||||||
let killFn: (() => void) | 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 = () => {
|
const resetNoOutputTimer = () => {
|
||||||
if (configNoOutputTimeout <= 0) return;
|
if (!noOutputAbort) return;
|
||||||
if (noOutputTimer) clearTimeout(noOutputTimer);
|
clearNoOutputTimer();
|
||||||
noOutputTimer = setTimeout(() => {
|
noOutputTimer = setTimeout(() => {
|
||||||
noOutputTriggered = true;
|
noOutputTriggered = true;
|
||||||
killFn?.();
|
noOutputAbort.abort();
|
||||||
}, configNoOutputTimeout * 1000);
|
}, configNoOutputTimeout * 1000);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|
@ -315,54 +337,22 @@ export function createBashTool(
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Start the no-output timer (will fire if command produces no output at all)
|
// Collect output and build final text (shared by success and error paths)
|
||||||
resetNoOutputTimer();
|
const buildOutput = (): { text: string; details?: BashToolDetails } => {
|
||||||
|
|
||||||
ops
|
|
||||||
.exec(spawnContext.command, spawnContext.cwd, {
|
|
||||||
onData: handleData,
|
|
||||||
signal,
|
|
||||||
timeout: effectiveTimeout,
|
|
||||||
env: spawnContext.env,
|
|
||||||
onKillHandle: (kill) => { killFn = kill; },
|
|
||||||
})
|
|
||||||
.then(({ exitCode }) => {
|
|
||||||
if (noOutputTimer) clearTimeout(noOutputTimer);
|
|
||||||
|
|
||||||
// Close temp file stream
|
|
||||||
if (tempFileStream) {
|
|
||||||
tempFileStream.end();
|
|
||||||
}
|
|
||||||
|
|
||||||
// Combine all buffered chunks
|
|
||||||
const fullBuffer = Buffer.concat(chunks);
|
const fullBuffer = Buffer.concat(chunks);
|
||||||
const fullOutput = fullBuffer.toString("utf-8");
|
const fullOutput = fullBuffer.toString("utf-8");
|
||||||
|
|
||||||
// Apply tail truncation
|
|
||||||
const truncation = truncateTail(fullOutput);
|
const truncation = truncateTail(fullOutput);
|
||||||
let outputText = truncation.content || "(no output)";
|
let outputText = truncation.content || "(no output)";
|
||||||
|
|
||||||
// Build details with truncation info
|
|
||||||
let details: BashToolDetails | undefined;
|
let details: BashToolDetails | undefined;
|
||||||
|
|
||||||
if (truncation.truncated) {
|
if (truncation.truncated) {
|
||||||
details = {
|
details = { truncation, fullOutputPath: tempFilePath };
|
||||||
truncation,
|
const startLine = truncation.totalLines - truncation.outputLines + 1;
|
||||||
fullOutputPath: tempFilePath,
|
|
||||||
};
|
|
||||||
|
|
||||||
// Build actionable notice
|
|
||||||
const startLine =
|
|
||||||
truncation.totalLines - truncation.outputLines + 1;
|
|
||||||
const endLine = truncation.totalLines;
|
const endLine = truncation.totalLines;
|
||||||
|
|
||||||
if (truncation.lastLinePartial) {
|
if (truncation.lastLinePartial) {
|
||||||
// Edge case: last line alone > 30KB
|
|
||||||
const lastLineSize = formatSize(
|
const lastLineSize = formatSize(
|
||||||
Buffer.byteLength(
|
Buffer.byteLength(fullOutput.split("\n").pop() || "", "utf-8"),
|
||||||
fullOutput.split("\n").pop() || "",
|
|
||||||
"utf-8",
|
|
||||||
),
|
|
||||||
);
|
);
|
||||||
outputText += `\n\n[Showing last ${formatSize(truncation.outputBytes)} of line ${endLine} (line is ${lastLineSize}). Full output: ${tempFilePath}]`;
|
outputText += `\n\n[Showing last ${formatSize(truncation.outputBytes)} of line ${endLine} (line is ${lastLineSize}). Full output: ${tempFilePath}]`;
|
||||||
} else if (truncation.truncatedBy === "lines") {
|
} else if (truncation.truncatedBy === "lines") {
|
||||||
|
|
@ -372,49 +362,53 @@ export function createBashTool(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If killed by no-output timeout, report it clearly
|
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: combinedSignal,
|
||||||
|
timeout: effectiveTimeout,
|
||||||
|
env: spawnContext.env,
|
||||||
|
})
|
||||||
|
.then(({ exitCode }) => {
|
||||||
|
clearNoOutputTimer();
|
||||||
|
if (tempFileStream) tempFileStream.end();
|
||||||
|
|
||||||
|
const { text, details } = buildOutput();
|
||||||
|
|
||||||
if (noOutputTriggered) {
|
if (noOutputTriggered) {
|
||||||
if (outputText) outputText += "\n\n";
|
reject(new Error(
|
||||||
outputText += `Process 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.`;
|
`${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.`,
|
||||||
reject(new Error(outputText));
|
));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (exitCode !== 0 && exitCode !== null) {
|
if (exitCode !== 0 && exitCode !== null) {
|
||||||
outputText += `\n\nCommand exited with code ${exitCode}`;
|
reject(new Error(`${text}\n\nCommand exited with code ${exitCode}`));
|
||||||
reject(new Error(outputText));
|
|
||||||
} else {
|
} else {
|
||||||
resolve({
|
resolve({ content: [{ type: "text", text }], details });
|
||||||
content: [{ type: "text", text: outputText }],
|
|
||||||
details,
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
.catch((err: Error) => {
|
.catch((err: Error) => {
|
||||||
if (noOutputTimer) clearTimeout(noOutputTimer);
|
clearNoOutputTimer();
|
||||||
|
if (tempFileStream) tempFileStream.end();
|
||||||
|
|
||||||
// Close temp file stream
|
const { text } = buildOutput();
|
||||||
if (tempFileStream) {
|
|
||||||
tempFileStream.end();
|
|
||||||
}
|
|
||||||
|
|
||||||
// Combine all buffered chunks for error output
|
|
||||||
const fullBuffer = Buffer.concat(chunks);
|
|
||||||
let output = fullBuffer.toString("utf-8");
|
|
||||||
|
|
||||||
if (noOutputTriggered) {
|
if (noOutputTriggered) {
|
||||||
if (output) output += "\n\n";
|
reject(new Error(
|
||||||
output += `Process 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.`;
|
`${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.`,
|
||||||
reject(new Error(output));
|
));
|
||||||
} else if (err.message === "aborted") {
|
} else if (err.message === "aborted") {
|
||||||
if (output) output += "\n\n";
|
reject(new Error(text ? `${text}\n\nCommand aborted` : "Command aborted"));
|
||||||
output += "Command aborted";
|
|
||||||
reject(new Error(output));
|
|
||||||
} else if (err.message.startsWith("timeout:")) {
|
} else if (err.message.startsWith("timeout:")) {
|
||||||
const timeoutSecs = err.message.split(":")[1];
|
const timeoutSecs = err.message.split(":")[1];
|
||||||
if (output) output += "\n\n";
|
reject(new Error(text ? `${text}\n\nCommand timed out after ${timeoutSecs} seconds` : `Command timed out after ${timeoutSecs} seconds`));
|
||||||
output += `Command timed out after ${timeoutSecs} seconds`;
|
|
||||||
reject(new Error(output));
|
|
||||||
} else {
|
} else {
|
||||||
reject(err);
|
reject(err);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue