diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6c5d526e2..2c7330a93 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed Windows bash tool crashes when an explicit timeout fires while a piped command is still streaming output; the JavaScript fallback now reports the timeout without also aborting the native timeout signal. ([#5021](https://github.com/can1357/oh-my-pi/issues/5021)) + ## [16.3.15] - 2026-07-09 ### Changed diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index fc7d7dc69..505feca96 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -300,9 +300,16 @@ export async function executeBash(command: string, options?: BashExecutorOptions const requestedTimeoutMs = options?.timeout; const deadlineTimeoutMs = requestedTimeoutMs === 0 ? undefined : Math.max(1_000, requestedTimeoutMs ?? 300_000); const nativeTimeoutMs = requestedTimeoutMs !== undefined && requestedTimeoutMs > 0 ? requestedTimeoutMs : undefined; + const nativeOwnsTimeout = nativeTimeoutMs !== undefined; if (deadlineTimeoutMs !== undefined) { timeoutTimer = setTimeout(() => { - abortCurrentExecution(); + // Explicit timeouts are already enforced inside pi-natives via + // `timeoutMs`. Do not also abort the JS AbortSignal here: on Windows, + // aborting that signal while a piped command is still forwarding output + // can terminate the Bun host before the native timeout result resolves. + if (!nativeOwnsTimeout) { + abortCurrentExecution(); + } timeoutDeferred.resolve("timeout"); }, deadlineTimeoutMs); } diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index b11a15f96..f428d3ea9 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -520,11 +520,7 @@ exit 64 expect(next.output.trim()).toBe("still_persistent"); }); - it("returns at the JavaScript timeout when native timeout cleanup stalls", async () => { - if (process.platform === "win32") { - return; - } - + it("does not abort the native signal when the JavaScript timeout fallback returns streamed output", async () => { // Compress the JS-side fallback timer (floored at 1000ms in the source) so // the safety-net fires deterministically without a real 1s wait. Only long // timers are shrunk — fs/subprocess setup keeps real scheduling — and the @@ -537,8 +533,12 @@ exit 64 ...rest, )) as typeof globalThis.setTimeout); - vi.spyOn(piNatives.Shell.prototype, "run").mockImplementation((_options, onChunk) => { - onChunk?.(null, "started\n"); + let nativeSignal: AbortSignal | undefined; + vi.spyOn(piNatives.Shell.prototype, "run").mockImplementation((options, onChunk) => { + if (options.signal instanceof AbortSignal) { + nativeSignal = options.signal; + } + onChunk?.(null, "streamed-before-timeout\n"); return Promise.withResolvers().promise; }); const abortSpy = vi.spyOn(piNatives.Shell.prototype, "abort").mockResolvedValue(); @@ -546,12 +546,15 @@ exit 64 const result = await executeBash("sleep 10", { cwd: tempDir, timeout: 1000, - sessionKey: "hung-native-timeout", + sessionKey: "explicit-timeout-keeps-native-signal", }); expect(result.cancelled).toBe(true); + expect(result.output).toContain("streamed-before-timeout"); expect(result.output).toContain("Command timed out after 1 seconds"); - expect(abortSpy).toHaveBeenCalled(); + expect(nativeSignal).toBeDefined(); + expect(nativeSignal?.aborted).toBe(false); + expect(abortSpy).not.toHaveBeenCalled(); }); it("aborts before follow-up output", async () => {