Skip to content

Commit 7a213e5

Browse files
authored
🤖 fix: use stdin.abort() to prevent hangs on SSH commands (#504)
## Problem Commands like `cat /tmp/results.json | jq '.'` would sometimes hang forever over SSH, not respecting timeouts. This was observed over SSH with the bash tool, and it's unclear if it happens on local workspaces. ## Root Cause The issue stems from using `WritableStream.close()` to close stdin: - **`close()` is async** and waits for graceful acknowledgment from the remote end - Over SSH with ControlMaster multiplexing, the SSH channel's stdin buffer might not be immediately flushed - The `close()` promise waits for acknowledgment that may never come - Even timeouts couldn't help because the promise was already stuck in Node.js stream machinery ## Solution **Use `stdin.abort()` instead of `stdin.close()` for immediate closure:** - **`abort()`** - Immediate, synchronous force-close. Marks stream as errored, releases locks, doesn't wait - **`close()`** - Graceful, async close. Waits for all writes to be acknowledged Applied selectively: - **bash tool**: Use `abort()` - commands don't need stdin at all - **SSHRuntime mv/rm/stat**: Use `abort()` - commands don't read stdin - **SSHRuntime writeFile**: Keep `close()` - needs to flush data written to stdin ## Testing - All 45 bash tool tests pass - SSH integration tests pass: - `runtimeExecuteBash` - verifies bash commands over SSH work - `renameWorkspace` - verifies mv operations work - `runtimeFileEditing` - verifies file writes (which use stdin) work correctly - All 921 unit tests pass - Typecheck passes ## Implementation Details Changed 4 locations: 1. `src/services/tools/bash.ts` - Use `abort()` to close stdin immediately 2. `src/runtime/SSHRuntime.ts` (3 places) - Use `abort()` for mv/rm/stat commands Kept `close()` in 1 location: - `SSHRuntime.writeFile()` - Needs graceful close to ensure all data is written --- _Generated with `cmux`_
1 parent f664385 commit 7a213e5

File tree

4 files changed

+90
-7
lines changed

4 files changed

+90
-7
lines changed

src/runtime/SSHRuntime.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,8 @@ export class SSHRuntime implements Runtime {
929929
abortSignal,
930930
});
931931

932-
await stream.stdin.close();
932+
// Command doesn't use stdin - abort to close immediately without waiting
933+
await stream.stdin.abort();
933934
const exitCode = await stream.exitCode;
934935

935936
if (exitCode !== 0) {
@@ -999,7 +1000,8 @@ export class SSHRuntime implements Runtime {
9991000
abortSignal,
10001001
});
10011002

1002-
await checkStream.stdin.close();
1003+
// Command doesn't use stdin - abort to close immediately without waiting
1004+
await checkStream.stdin.abort();
10031005
const checkExitCode = await checkStream.exitCode;
10041006

10051007
// Handle check results
@@ -1072,7 +1074,8 @@ export class SSHRuntime implements Runtime {
10721074
abortSignal,
10731075
});
10741076

1075-
await stream.stdin.close();
1077+
// Command doesn't use stdin - abort to close immediately without waiting
1078+
await stream.stdin.abort();
10761079
const exitCode = await stream.exitCode;
10771080

10781081
if (exitCode !== 0) {

src/services/tools/bash.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,8 @@ describe("bash tool", () => {
637637
const startTime = performance.now();
638638

639639
// cat without input should complete immediately
640-
// This used to hang because cat would wait for stdin
640+
// This used to hang because stdin.close() would wait for acknowledgment
641+
// Fixed by using stdin.abort() for immediate closure
641642
const args: BashToolArgs = {
642643
script: "echo test | cat",
643644
timeout_secs: 5,

src/services/tools/bash.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,9 +147,11 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
147147
abortSignal.addEventListener("abort", abortListener);
148148
}
149149

150-
// Close stdin immediately - we don't need to send any input
151-
// This is critical: not closing stdin can cause the runtime to wait forever
152-
execStream.stdin.close().catch(() => {
150+
// Force-close stdin immediately - we don't need to send any input
151+
// Use abort() instead of close() for immediate, synchronous closure
152+
// close() is async and waits for acknowledgment, which can hang over SSH
153+
// abort() immediately marks stream as errored and releases locks
154+
execStream.stdin.abort().catch(() => {
153155
// Ignore errors - stream might already be closed
154156
});
155157

tests/ipcMain/runtimeExecuteBash.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,83 @@ describeIntegration("Runtime Bash Execution", () => {
266266
},
267267
type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS
268268
);
269+
270+
test.concurrent(
271+
"should not hang on commands that read stdin without input",
272+
async () => {
273+
const env = await createTestEnvironment();
274+
const tempGitRepo = await createTempGitRepo();
275+
276+
try {
277+
// Setup provider
278+
await setupProviders(env.mockIpcRenderer, {
279+
anthropic: {
280+
apiKey: getApiKey("ANTHROPIC_API_KEY"),
281+
},
282+
});
283+
284+
// Create workspace
285+
const branchName = generateBranchName("bash-stdin");
286+
const runtimeConfig = getRuntimeConfig(branchName);
287+
const { workspaceId, cleanup } = await createWorkspaceWithInit(
288+
env,
289+
tempGitRepo,
290+
branchName,
291+
runtimeConfig,
292+
true, // waitForInit
293+
type === "ssh"
294+
);
295+
296+
try {
297+
// Create a test file with JSON content
298+
await sendMessageAndWait(
299+
env,
300+
workspaceId,
301+
'Run bash: echo \'{"test": "data"}\' > /tmp/test.json',
302+
HAIKU_MODEL,
303+
BASH_ONLY
304+
);
305+
306+
// Test command that pipes file through stdin-reading command (jq)
307+
// This would hang forever if stdin.close() was used instead of stdin.abort()
308+
// Regression test for: https://github.com/coder/cmux/issues/503
309+
const startTime = Date.now();
310+
const events = await sendMessageAndWait(
311+
env,
312+
workspaceId,
313+
"Run bash with 3s timeout: cat /tmp/test.json | jq '.'",
314+
HAIKU_MODEL,
315+
BASH_ONLY,
316+
15000 // 15s max wait - should complete in < 5s
317+
);
318+
const duration = Date.now() - startTime;
319+
320+
// Extract response text
321+
const responseText = extractTextFromEvents(events);
322+
323+
// Verify command completed successfully (not timeout)
324+
expect(responseText).toContain("test");
325+
expect(responseText).toContain("data");
326+
327+
// Verify command completed quickly (not hanging until timeout)
328+
// Should complete in under 5 seconds for SSH, 3 seconds for local
329+
const maxDuration = type === "ssh" ? 8000 : 5000;
330+
expect(duration).toBeLessThan(maxDuration);
331+
332+
// Verify bash tool was called
333+
const toolCallStarts = events.filter((e: any) => e.type === "tool-call-start");
334+
const bashCalls = toolCallStarts.filter((e: any) => e.toolName === "bash");
335+
expect(bashCalls.length).toBeGreaterThan(0);
336+
} finally {
337+
await cleanup();
338+
}
339+
} finally {
340+
await cleanupTempGitRepo(tempGitRepo);
341+
await cleanupTestEnvironment(env);
342+
}
343+
},
344+
type === "ssh" ? TEST_TIMEOUT_SSH_MS : TEST_TIMEOUT_LOCAL_MS
345+
);
269346
}
270347
);
271348
});

0 commit comments

Comments
 (0)