smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(bash): support disabled command deadlines

Browse Source 
Treat timeout 0 as an explicit no-deadline contract across the bash tool, executor, async job, and PTY paths.

Signed-off-by: Christian Stewart <christian@aperture.us>
This commit is contained in:
Christian Stewart
2026-06-25 15:43:27 -07:00
parent 79a397e97a
commit 78d4978c51
6 changed files with 92 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/exec/bash-executor.ts
+14 -9
View File
@@ -14,6 +14,7 @@ import { buildNonInteractiveEnv } from "./non-interactive-env";
export interface BashExecutorOptions {
cwd?: string;
/** Milliseconds before aborting the command; 0 disables the executor deadline. */
timeout?: number;
onChunk?: (chunk: string) => void;
chunkThrottleMs?: number;
@@ -296,11 +297,15 @@ export async function executeBash(command: string, options?: BashExecutorOptions
let timeoutTimer: NodeJS.Timeout | undefined;
const timeoutDeferred = Promise.withResolvers<"timeout">();
const baseTimeoutMs = Math.max(1_000, options?.timeout ?? 300_000);
timeoutTimer = setTimeout(() => {
abortCurrentExecution();
timeoutDeferred.resolve("timeout");
}, baseTimeoutMs);
const requestedTimeoutMs = options?.timeout;
const deadlineTimeoutMs = requestedTimeoutMs === 0 ? undefined : Math.max(1_000, requestedTimeoutMs ?? 300_000);
const nativeTimeoutMs = requestedTimeoutMs !== undefined && requestedTimeoutMs > 0 ? requestedTimeoutMs : undefined;
if (deadlineTimeoutMs !== undefined) {
timeoutTimer = setTimeout(() => {
abortCurrentExecution();
timeoutDeferred.resolve("timeout");
}, deadlineTimeoutMs);
}
let resetSession = false;
@@ -311,7 +316,7 @@ export async function executeBash(command: string, options?: BashExecutorOptions
command: finalCommand,
cwd: commandCwd,
env: commandEnv,
timeoutMs: options?.timeout,
timeoutMs: nativeTimeoutMs,
signal: runAbortController.signal,
},
(err, chunk) => {
@@ -328,7 +333,7 @@ export async function executeBash(command: string, options?: BashExecutorOptions
sessionEnv: shellEnv,
snapshotPath: snapshotPath ?? undefined,
minimizer,
timeoutMs: options?.timeout,
timeoutMs: nativeTimeoutMs,
signal: runAbortController.signal,
},
(err, chunk) => {
@@ -359,8 +364,8 @@ export async function executeBash(command: string, options?: BashExecutorOptions
exitCode: undefined,
cancelled: true,
...(await sink.dump(
winner.kind === "timeout"
? `Command timed out after ${Math.round(baseTimeoutMs / 1000)} seconds`
winner.kind === "timeout" && deadlineTimeoutMs !== undefined
? `Command timed out after ${Math.round(deadlineTimeoutMs / 1000)} seconds`
: "Command cancelled",
)),
};
packages/coding-agent/src/prompts/tools/bash.md
+3 -3
View File
@@ -41,9 +41,9 @@ Anything below → `eval` cell, not bash:
{{#if asyncEnabled}}
# Timeout and async
- `timeout` is seconds, clamped to `1..3600`; the process is killed on elapse.
- `async: true` defers only reporting — it does NOT extend the timeout; a daemon with `async: true` is still killed at the clamped timeout.
- Need >3600s? Detach/manage lifecycle yourself (`cmd &`, supervisor, self-restarting script). The shell session persists across calls.
- `timeout` is seconds; nonzero values are clamped to `1..3600` and the process is killed on elapse. Set `timeout: 0` only for commands that must run until completion or explicit cancellation.
- `async: true` defers only reporting — it does NOT extend a nonzero timeout; use `timeout: 0` when a daemon or watcher must be cancellation-owned.
- Need a daemon or >3600s run? Use `async: true` with `timeout: 0` when the harness should keep it alive until cancellation, or detach/manage lifecycle yourself (`cmd &`, supervisor, self-restarting script). The shell session persists across calls.
{{/if}}
{{#if autoBackgroundEnabled}}
packages/coding-agent/src/tools/bash-interactive.ts
+1 -1
View File
@@ -300,7 +300,7 @@ export async function runInteractiveBashPty(
options: {
command: string;
cwd: string;
timeoutMs: number;
timeoutMs?: number;
signal?: AbortSignal;
env?: Record<string, string>;
artifactPath?: string;
packages/coding-agent/src/tools/bash.ts
+50 -29
View File
@@ -131,7 +131,7 @@ async function saveBashOriginalArtifact(session: ToolSession, originalText: stri
}
}
const BASH_TIMEOUT_DESCRIPTION = `timeout in seconds; clamped to ${TOOL_TIMEOUTS.bash.min}-${TOOL_TIMEOUTS.bash.max}`;
const BASH_TIMEOUT_DESCRIPTION = `timeout in seconds; 0 disables the command deadline; nonzero values are clamped to ${TOOL_TIMEOUTS.bash.min}-${TOOL_TIMEOUTS.bash.max}`;
const bashSchemaBase = type({
command: type("string").describe("command to execute"),
@@ -166,6 +166,7 @@ export interface BashToolDetails {
meta?: OutputMeta;
timeoutSeconds?: number;
requestedTimeoutSeconds?: number;
timeoutDisabled?: boolean;
wallTimeMs?: number;
/** Exit code of a command that ran to completion but failed (non-zero). */
exitCode?: number;
@@ -421,7 +422,11 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
* completed command that failed; #buildCompletedResult surfaces it as an
* error *result* (carrying execution details) rather than a throw.
*/
#throwIfUnfinished(result: BashResult | BashInteractiveResult, timeoutSec: number, outputText: string): void {
#throwIfUnfinished(
result: BashResult | BashInteractiveResult,
timeoutSec: number | undefined,
outputText: string,
): void {
if (result.cancelled) {
// executeBash output already carries a `[Command cancelled]` notice from
// the sink; PTY/bridge interactive output does not, so annotate it here.
@@ -431,11 +436,9 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
}
if (isInteractiveResult(result) && result.timedOut) {
const out = normalizeResultOutput(result);
throw new ToolError(
out
? `${out}\n\n[Command timed out after ${timeoutSec} seconds]`
: `Command timed out after ${timeoutSec} seconds`,
);
const message =
timeoutSec === undefined ? "Command timed out" : `Command timed out after ${timeoutSec} seconds`;
throw new ToolError(out ? `${out}\n\n[${message}]` : message);
}
if (result.exitCode === undefined) {
throw new ToolError(`${outputText}\n\nCommand failed: missing exit status`);
@@ -444,7 +447,7 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
async #buildCompletedResult(
result: BashResult | BashInteractiveResult,
timeoutSec: number,
timeoutSec: number | undefined,
options: {
requestedTimeoutSec?: number;
notices?: readonly string[];
@@ -472,7 +475,12 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
// Aborts / timeouts / missing-status still propagate as thrown errors.
this.#throwIfUnfinished(result, timeoutSec, outputText);
const details: BashToolDetails = { timeoutSeconds: timeoutSec };
const details: BashToolDetails = {};
if (timeoutSec === undefined) {
details.timeoutDisabled = true;
} else {
details.timeoutSeconds = timeoutSec;
}
if (options.requestedTimeoutSec !== undefined && options.requestedTimeoutSec !== timeoutSec) {
details.requestedTimeoutSeconds = options.requestedTimeoutSec;
}
@@ -503,13 +511,17 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
jobId: string,
label: string,
previewText: string,
timeoutSec: number,
timeoutSec: number | undefined,
options: { requestedTimeoutSec?: number; notices?: readonly string[] } = {},
): AgentToolResult<BashToolDetails> {
const details: BashToolDetails = {
timeoutSeconds: timeoutSec,
async: { state: "running", jobId, type: "bash" },
};
if (timeoutSec === undefined) {
details.timeoutDisabled = true;
} else {
details.timeoutSeconds = timeoutSec;
}
if (options.requestedTimeoutSec !== undefined && options.requestedTimeoutSec !== timeoutSec) {
details.requestedTimeoutSeconds = options.requestedTimeoutSec;
}
@@ -539,8 +551,8 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
#startManagedBashJob(options: {
command: string;
commandCwd: string;
timeoutMs: number;
timeoutSec: number;
timeoutMs: number | undefined;
timeoutSec: number | undefined;
requestedTimeoutSec?: number;
notices?: readonly string[];
@@ -569,7 +581,7 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
const result = await executeBash(options.command, {
cwd: options.commandCwd,
sessionKey: `${this.session.getSessionId?.() ?? ""}:async:${jobId}`,
timeout: options.timeoutMs,
timeout: options.timeoutMs ?? 0,
signal: runSignal,
env: options.resolvedEnv,
artifactPath,
@@ -661,8 +673,9 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
}
}
#resolveAutoBackgroundWaitMs(timeoutMs: number): number {
#resolveAutoBackgroundWaitMs(timeoutMs: number | undefined): number {
if (this.#autoBackgroundThresholdMs <= 0) return 0;
if (timeoutMs === undefined) return this.#autoBackgroundThresholdMs;
const timeoutBufferMs = 1_000;
return Math.max(0, Math.min(this.#autoBackgroundThresholdMs, timeoutMs - timeoutBufferMs));
}
@@ -765,13 +778,17 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
throw new ToolError(`Working directory is not a directory: ${commandCwd}`);
}
// Clamp to reasonable range: 1s - 3600s (1 hour)
// A timeout of 0 is an explicit long-running-command contract: the user
// must still cancel the call or job, but OMP does not impose a deadline.
const requestedTimeoutSec = rawTimeout;
const timeoutSec = clampTimeout("bash", requestedTimeoutSec);
const timeoutMs = timeoutSec * 1000;
const timeoutDisabled = requestedTimeoutSec === 0;
const timeoutSec = timeoutDisabled ? undefined : clampTimeout("bash", requestedTimeoutSec);
const timeoutMs = timeoutSec === undefined ? undefined : timeoutSec * 1000;
const pendingNotices: string[] = [];
const timeoutClampNotice = formatTimeoutClampNotice(requestedTimeoutSec, timeoutSec);
if (timeoutClampNotice) pendingNotices.push(timeoutClampNotice);
if (timeoutSec !== undefined) {
const timeoutClampNotice = formatTimeoutClampNotice(requestedTimeoutSec, timeoutSec);
if (timeoutClampNotice) pendingNotices.push(timeoutClampNotice);
}
if (asyncRequested) {
if (!this.session.asyncJobManager) {
@@ -909,14 +926,16 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
throw new ToolAbortError("Command aborted");
}
const timeoutPromise = Bun.sleep(timeoutMs).then(() => ({ kind: "timeout" as const }));
const timeoutPromise = timeoutMs
? Bun.sleep(timeoutMs).then(() => ({ kind: "timeout" as const }))
: undefined;
// Poll until the process exits, times out, or the caller aborts.
for (;;) {
const racers: Array<Promise<BridgeRaceResult>> = [
exitPromise.then(s => ({ kind: "exit" as const, status: s })),
timeoutPromise,
Bun.sleep(250).then(() => ({ kind: "poll" as const })),
];
if (timeoutPromise) racers.push(timeoutPromise);
if (signal) {
racers.push(abortedP.then(() => ({ kind: "aborted" as const })));
}
@@ -1053,7 +1072,7 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
: await executeBash(command, {
cwd: commandCwd,
sessionKey: this.session.getSessionId?.() ?? undefined,
timeout: timeoutMs,
timeout: timeoutMs ?? 0,
signal,
env: resolvedEnv,
artifactPath,
@@ -1074,11 +1093,9 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
}
if (isInteractiveResult(result) && result.timedOut) {
const out = normalizeResultOutput(result);
throw new ToolError(
out
? `${out}\n\n[Command timed out after ${timeoutSec} seconds]`
: `Command timed out after ${timeoutSec} seconds`,
);
const message =
timeoutSec === undefined ? "Command timed out" : `Command timed out after ${timeoutSec} seconds`;
throw new ToolError(out ? `${out}\n\n[${message}]` : message);
}
return this.#buildCompletedResult(result, timeoutSec, {
requestedTimeoutSec,
@@ -1286,13 +1303,17 @@ export function createShellRenderer<TArgs>(config: ShellRendererConfig<TArgs>) {
const showingFullOutput = expanded && renderContext?.isFullOutput === true;
// Build truncation warning
const timeoutSeconds = details?.timeoutSeconds ?? renderContext?.timeout;
const timeoutDisabled = details?.timeoutDisabled === true || renderContext?.timeout === 0;
const timeoutSeconds = timeoutDisabled ? undefined : (details?.timeoutSeconds ?? renderContext?.timeout);
const requestedTimeoutSeconds = details?.requestedTimeoutSeconds;
const wallTimeMs = details?.wallTimeMs;
const statsParts: string[] = [];
if (wallTimeMs !== undefined) {
statsParts.push(`Wall: ${formatWallTimeSeconds(wallTimeMs)}s`);
}
if (timeoutDisabled) {
statsParts.push("Timeout: disabled");
}
if (typeof timeoutSeconds === "number") {
statsParts.push(
requestedTimeoutSeconds !== undefined && requestedTimeoutSeconds !== timeoutSeconds
packages/coding-agent/test/bash-executor.test.ts
+9
View File
@@ -402,6 +402,15 @@ exit 64
expect(result.output).not.toContain("done");
});
it("does not arm a deadline when timeout is zero", async () => {
if (process.platform === "win32") {
return;
}
const result = await executeBash("sleep 0.1; echo done", { cwd: tempDir, timeout: 0 });
expect(result.cancelled).toBe(false);
expect(result.output.trim()).toBe("done");
});
it("aborts commands", async () => {
if (process.platform === "win32") {
return;
packages/coding-agent/test/tools.test.ts
+15
View File
@@ -1492,6 +1492,21 @@ function b() {
expect(result.details?.requestedTimeoutSeconds).toBe(7200);
});
it("should disable the command deadline when timeout is zero", async () => {
vi.spyOn(toolTimeouts, "clampTimeout").mockReturnValue(0.05);
const result = await bashTool.execute("test-call-timeout-disabled", {
command: "printf 'start\\n'; sleep 0.1; printf 'done\\n'",
timeout: 0,
});
const output = getTextOutput(result);
expect(output).toContain("start");
expect(output).toContain("done");
expect(result.details?.timeoutDisabled).toBe(true);
expect(result.details?.timeoutSeconds).toBeUndefined();
});
it("should respect timeout", async () => {
// Reduce the effective timeout through the production clamp seam; the
// real subprocess kill-on-timeout path is still exercised, just faster.