From fe4e553be33b46318b14fba92d48bce404f88e3f Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 1 Aug 2026 05:14:43 +0000 Subject: [PATCH] fix(coding-agent): clear bash auto-background threshold timer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/coding-agent/CHANGELOG.md | 4 ++ packages/coding-agent/src/tools/bash.ts | 16 ++++-- .../test/bash-autobg-timer.test.ts | 52 +++++++++++++++++++ .../test/fixtures/bash-autobg-exit-probe.ts | 46 ++++++++++++++++ 4 files changed, 113 insertions(+), 5 deletions(-) create mode 100644 packages/coding-agent/test/bash-autobg-timer.test.ts create mode 100644 packages/coding-agent/test/fixtures/bash-autobg-exit-probe.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 3f76ce653..ccdeb37ac 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -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 diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index 6b1e79ec5..6f3caf196 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -843,13 +843,18 @@ export class BashTool implements AgentTool(); + const thresholdTimer = setTimeout(() => resolveThreshold({ kind: "running" }), thresholdMs); const waiters: Array< Promise - > = [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 { + 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); +}); diff --git a/packages/coding-agent/test/fixtures/bash-autobg-exit-probe.ts b/packages/coding-agent/test/fixtures/bash-autobg-exit-probe.ts new file mode 100644 index 000000000..c91a55d76 --- /dev/null +++ b/packages/coding-agent/test/fixtures/bash-autobg-exit-probe.ts @@ -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.