fix(bash): avoided aborting native timeout signal

Prevented explicit bash timeouts from also aborting the AbortSignal passed to pi-natives while streamed output is still draining. Native timeout_ms now owns cancellation, and the JavaScript timer only reports the fallback timeout result.

Added regression coverage for streamed output before an explicit timeout.

Fixes #5021
This commit is contained in:
roboomp
2026-07-10 03:45:36 +00:00
parent e8d0a93db6
commit 9002f4ff0a
3 changed files with 24 additions and 10 deletions
+4
View File
@@ -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
@@ -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);
}
@@ -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<never>().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 () => {