From 45a22c2dfdf111985999ff45b9bd743a54fbc645 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 9 Nov 2025 18:04:25 +0000 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A4=96=20fix:=20check=20abort=20signa?= =?UTF-8?q?l=20in=20bash=20tool=20stream=20consumption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds abort signal handling to the consumeStream() loop to prevent hangs when bash commands are interrupted while producing output. ## Problem When a new message arrives during an active bash tool execution, the streamManager aborts the current tool call. The bash tool passes the abort signal to runtime.exec(), which kills the process. However, the consumeStream() function was not checking the abort signal during its read loop. In some environments (especially over SSH or with buffered output), the streams don't close immediately after the process is killed, causing reader.read() to hang until the streams eventually close or timeout. ## Solution 1. Register abort event listener that calls reader.cancel() when fired - This interrupts reader.read() immediately if it's blocked - Eliminates race condition of checking abort before read 2. Clean up abort listener in finally block 3. Add abort detection in Promise.all error handler 4. Add abort detection after Promise.all succeeds (belt-and-suspenders) ## Testing - Added unit test that aborts a bash command producing continuous output - Verified abort completes quickly (< 2s) - All 44 bash unit tests pass - All 8 executeBash integration tests pass - All 963 unit tests pass _Generated with `cmux`_ --- src/services/tools/bash.test.ts | 60 +++++++++++++++++++++++++++++++++ src/services/tools/bash.ts | 33 ++++++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index d46478e5a..078cdc2e5 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -1151,6 +1151,66 @@ fi expect(remainingProcesses).toBe(0); }); + + it("should abort quickly when command produces continuous output", async () => { + using testEnv = createTestBashTool(); + const tool = testEnv.tool; + + // Create AbortController to simulate user interruption + const abortController = new AbortController(); + + // Command that produces slow, continuous output + // The key is it keeps running, so the abort happens while reader.read() is waiting + const args: BashToolArgs = { + script: ` + # Produce continuous output slowly (prevents hitting truncation limits) + for i in {1..1000}; do + echo "Output line \$i" + sleep 0.1 + done + `, + timeout_secs: 120, + }; + + // Start the command + const resultPromise = tool.execute!(args, { + ...mockToolCallOptions, + abortSignal: abortController.signal, + }) as Promise; + + // Wait for output to start (give it time to produce a few lines) + await new Promise((resolve) => setTimeout(resolve, 250)); + + // Abort the operation while it's still producing output + const abortTime = Date.now(); + abortController.abort(); + + // Wait for the result with a timeout to detect hangs + const timeoutPromise = new Promise((_, reject) => + setTimeout(() => reject(new Error("Test timeout - tool did not abort quickly")), 5000) + ); + + const result = await Promise.race([resultPromise, timeoutPromise]) as BashToolResult; + const duration = Date.now() - abortTime; + + // Command should be aborted + expect(result.success).toBe(false); + if (!result.success) { + // Error should mention abort or indicate the process was killed + const errorText = result.error.toLowerCase(); + expect( + errorText.includes("abort") || + errorText.includes("killed") || + errorText.includes("signal") || + result.exitCode === -1 + ).toBe(true); + } + + // CRITICAL: Tool should return quickly after abort (< 2s) + // This is the regression test - without checking abort signal in consumeStream(), + // the tool hangs until the streams close (which can take a long time) + expect(duration).toBeLessThan(2000); + }); }); describe("SSH runtime redundant cd detection", () => { diff --git a/src/services/tools/bash.ts b/src/services/tools/bash.ts index 1ca6bae80..45a626e4f 100644 --- a/src/services/tools/bash.ts +++ b/src/services/tools/bash.ts @@ -300,6 +300,16 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { const reader = stream.getReader(); const decoder = new TextDecoder("utf-8"); let carry = ""; + + // Set up abort handler to cancel reader when abort signal fires + // This interrupts reader.read() if it's blocked, preventing hangs + const abortHandler = () => { + reader.cancel().catch(() => { + /* ignore - reader may already be closed */ + }); + }; + abortSignal?.addEventListener("abort", abortHandler); + try { while (true) { if (truncationState.fileTruncated) { @@ -336,6 +346,9 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { if (truncationState.fileTruncated) break; } } finally { + // Clean up abort listener + abortSignal?.removeEventListener("abort", abortHandler); + // Flush decoder for any trailing bytes and emit the last line (if any) try { const tail = decoder.decode(); @@ -358,6 +371,15 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { try { [exitCode] = await Promise.all([execStream.exitCode, consumeStdout, consumeStderr]); } catch (err: unknown) { + // Check if this was an abort + if (abortSignal?.aborted) { + return { + success: false, + error: "Command execution was aborted", + exitCode: -1, + wall_duration_ms: Math.round(performance.now() - startTime), + }; + } return { success: false, error: `Failed to execute command: ${err instanceof Error ? err.message : String(err)}`, @@ -366,6 +388,17 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => { }; } + // Check if command was aborted (exitCode will be EXIT_CODE_ABORTED = -997) + // This can happen if abort signal fired after Promise.all resolved but before we check + if (abortSignal?.aborted) { + return { + success: false, + error: "Command execution was aborted", + exitCode: -1, + wall_duration_ms: Math.round(performance.now() - startTime), + }; + } + // Round to integer to preserve tokens const wall_duration_ms = Math.round(performance.now() - startTime); From 56fb1b33257af06b3e822728a4d06712595a33a7 Mon Sep 17 00:00:00 2001 From: Ammar Date: Sun, 9 Nov 2025 22:19:58 +0000 Subject: [PATCH 2/2] fix: remove unnecessary escape in test --- src/services/tools/bash.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/services/tools/bash.test.ts b/src/services/tools/bash.test.ts index 078cdc2e5..c663ad148 100644 --- a/src/services/tools/bash.test.ts +++ b/src/services/tools/bash.test.ts @@ -1165,7 +1165,7 @@ fi script: ` # Produce continuous output slowly (prevents hitting truncation limits) for i in {1..1000}; do - echo "Output line \$i" + echo "Output line $i" sleep 0.1 done `, @@ -1190,7 +1190,7 @@ fi setTimeout(() => reject(new Error("Test timeout - tool did not abort quickly")), 5000) ); - const result = await Promise.race([resultPromise, timeoutPromise]) as BashToolResult; + const result = (await Promise.race([resultPromise, timeoutPromise])) as BashToolResult; const duration = Date.now() - abortTime; // Command should be aborted @@ -1200,9 +1200,9 @@ fi const errorText = result.error.toLowerCase(); expect( errorText.includes("abort") || - errorText.includes("killed") || - errorText.includes("signal") || - result.exitCode === -1 + errorText.includes("killed") || + errorText.includes("signal") || + result.exitCode === -1 ).toBe(true); }