fix(coding-agent): clear bash auto-background threshold timer

waitForManagedBashJob raced job completion against a bare
Bun.sleep(thresholdMs), which cannot be cancelled. When completion,
abort, or steering won the race, the losing Bun.sleep timer stayed
scheduled and ref'd, keeping Bun's event loop alive until the threshold
expired — delaying SDK/headless shutdown and accumulating timers under
fast command rates.

Replace the Bun.sleep with a Promise.withResolvers settled by a
cancellable setTimeout, and route every outcome (including the former
no-signal early return) through one try/finally that clears the timer
and removes the abort/steer listeners.

Add a child-process regression test that runs the real auto-background
path for a fast command against a 30s threshold and asserts the process
exits promptly instead of being held for the full threshold.

Fixes #7235
This commit is contained in:
roboomp
2026-08-01 05:14:43 +00:00
parent 80627462b4
commit fe4e553be3
4 changed files with 113 additions and 5 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed Bash auto-background leaving a live `Bun.sleep` threshold timer scheduled after a command completes (or abort/steering wins) first, which could keep the event loop alive and delay SDK/headless shutdown until the threshold expired ([#7235](https://github.com/can1357/oh-my-pi/issues/7235)).
## [17.2.2] - 2026-07-31
### Added
+11 -5
View File
@@ -843,13 +843,18 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
return { kind: "steer" };
}
// Cancellable threshold: a bare Bun.sleep(thresholdMs) leaves a live, ref'd
// timer for the full threshold after the command finishes (or abort/steer)
// wins the race first — delaying SDK/headless shutdown and accumulating
// timers under fast command rates. Settle a withResolvers promise from
// setTimeout so the finally can clear it regardless of which waiter wins.
const { promise: thresholdPromise, resolve: resolveThreshold } = Promise.withResolvers<{
kind: "running";
}>();
const thresholdTimer = setTimeout(() => resolveThreshold({ kind: "running" }), thresholdMs);
const waiters: Array<
Promise<ManagedBashJobCompletion | { kind: "running" } | { kind: "steer" } | { kind: "aborted" }>
> = [job.completion, Bun.sleep(thresholdMs).then(() => ({ kind: "running" as const }))];
if (!signal && !steeringSignal) {
return await Promise.race(waiters);
}
> = [job.completion, thresholdPromise];
const { promise: abortedPromise, resolve: resolveAborted } = Promise.withResolvers<{ kind: "aborted" }>();
const onAbort = () => resolveAborted({ kind: "aborted" });
@@ -866,6 +871,7 @@ export class BashTool implements AgentTool<typeof bashSchemaBase | typeof bashSc
try {
return await Promise.race(waiters);
} finally {
clearTimeout(thresholdTimer);
signal?.removeEventListener("abort", onAbort);
steeringSignal?.removeEventListener("abort", onSteer);
}
@@ -0,0 +1,52 @@
/**
* Regression test for #7235: Bash auto-background must release the threshold
* timer once the command finishes first, instead of leaving a live `Bun.sleep`
* timer that keeps the event loop alive until the threshold expires (delaying
* SDK/headless shutdown). Timer keep-alive is only observable in a child
* process, so this spawns the real BashTool auto-background path against a 30s
* threshold and asserts the process exits promptly rather than after 30s.
*/
import { describe, expect, it } from "bun:test";
import * as path from "node:path";
const PROBE_PATH = path.join(import.meta.dir, "fixtures", "bash-autobg-exit-probe.ts");
const REPO_ROOT = path.resolve(import.meta.dir, "../../..");
// Fixed: probe exits ~0.5s. Buggy: held ~30s by the retained threshold timer.
const PROMPT_EXIT_MS = 15_000;
describe("bash auto-background threshold timer (#7235)", () => {
it("does not keep the event loop alive after a fast command completes", async () => {
const start = performance.now();
const proc = Bun.spawn([process.execPath, PROBE_PATH], {
cwd: REPO_ROOT,
stdin: "ignore",
stdout: "pipe",
stderr: "pipe",
});
// Integration test of real event-loop keep-alive across a process boundary:
// the child's wall-clock exit time IS the contract, so fake timers cannot
// apply (they cannot control another process's clock). This real-timer
// watchdog only bounds a wedged child — a retained threshold timer would
// otherwise hold it for the full 30s.
const watchdog = setTimeout(() => {
try {
proc.kill("SIGKILL");
} catch {}
}, 28_000);
try {
const [exitCode, stdout, stderr] = await Promise.all([
proc.exited,
new Response(proc.stdout).text(),
new Response(proc.stderr).text(),
]);
const elapsedMs = performance.now() - start;
expect(stderr).toBe("");
expect(exitCode).toBe(0);
expect(JSON.parse(stdout.trim())).toEqual({ done: true, output: "hi" });
expect(elapsedMs).toBeLessThan(PROMPT_EXIT_MS);
} finally {
clearTimeout(watchdog);
}
}, 30_000);
});
@@ -0,0 +1,46 @@
/**
* Child-process probe for #7235: the Bash auto-background threshold race must not
* leave a live, ref'd timer after the command finishes first. Runs the real
* `BashTool` auto-background path for a fast command against a 30s threshold and
* exits without disposing the job manager. A retained threshold timer would keep
* the event loop alive until the threshold expires; the fix clears it, so the
* process must exit promptly. The parent test measures wall-clock exit time.
*/
import { AsyncJobManager } from "@oh-my-pi/pi-coding-agent/async";
import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
import { BashTool } from "@oh-my-pi/pi-coding-agent/tools/bash";
const THRESHOLD_MS = 30_000;
// retentionMs:0 evicts the completed job immediately; the eviction timer is
// unref'd regardless, so the only ref'd timer under test is the race threshold.
const manager = new AsyncJobManager({ retentionMs: 0 });
const session = {
cwd: "/tmp",
hasUI: false,
skills: [],
getSessionFile: () => null,
getSessionId: () => "autobg-probe",
getAgentId: () => null,
asyncJobManager: manager,
settings: {
get(key: string) {
if (key === "bash.autoBackground.enabled") return true;
if (key === "bash.autoBackground.thresholdMs") return THRESHOLD_MS;
return undefined;
},
getBashInterceptorRules() {
return [];
},
},
getClientBridge: () => undefined,
} as unknown as ToolSession;
const tool = new BashTool(session);
const result = await tool.execute("autobg-probe-call", { command: "printf hi" });
const text = result.content.find(c => c.type === "text")?.text ?? "";
// First line only: the completed-result text carries a dynamic "Wall time" footer.
const firstLine = text.split("\n", 1)[0]?.trim() ?? "";
process.stdout.write(`${JSON.stringify({ done: true, output: firstLine })}\n`);
// Deliberately no manager.dispose(): the process must exit on its own.