🤖 fix: use stdin.abort() to prevent hangs on SSH commands #504
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Commands that don't read from stdin (like 'cat file | jq') would sometimes hang forever over SSH because stdin.close() waits for graceful acknowledgment from the remote end, which may never arrive due to buffering or SSH channel issues. Root cause: - WritableStream.close() is async and waits for all writes to be acknowledged - Over SSH with ControlMaster, this can hang indefinitely - Even timeouts couldn't help once the promise was stuck Solution: - Use stdin.abort() for immediate, forceful closure (no waiting) - Keep stdin.close() only for operations that write data to stdin Changes: - bash tool: Use abort() instead of close() for immediate stdin closure - SSHRuntime: Use abort() for commands that don't use stdin (mv, rm, stat) - Kept close() for writeFile() which pipes data through stdin Testing: - All 45 bash tool tests pass - SSH integration tests pass (runtimeExecuteBash, renameWorkspace, runtimeFileEditing) - Verified file operations that write to stdin still work correctly Generated with `cmux`
Adds integration test that verifies commands like 'cat file | jq' don't hang when piping through stdin-reading commands. Tests both SSH and local runtimes. The test: - Creates a JSON file - Pipes it through jq (which reads from stdin) - Verifies command completes quickly (< 8s SSH, < 5s local) - Would hang forever before the stdin.abort() fix Regression test for the stdin.close() hang issue. Generated with `cmux`
ammario
approved these changes
Nov 4, 2025
ammar-agent
added a commit
that referenced
this pull request
Nov 8, 2025
## Problem Commands like `grep -RIn ... | head -n 200` would hang over SSH runtime, not respecting timeouts. Previous fix (PR #504) using stdin.abort() addressed a symptom but not the root cause. ## Root Cause bash.ts waited for THREE conditions before finalizing: - exitCode !== null - stdoutEnded (stream 'close' event) - stderrEnded (stream 'close' event) Over SSH with ControlMaster multiplexing, stream 'close' events don't propagate reliably. The 50ms grace period fallback was insufficient, allowing hangs on commands that take longer or during high SSH latency. ## Solution **Single source of truth: process exit triggers all cleanup** Created DisposableProcess wrapper that: 1. Registers cleanup callbacks (stream destruction, etc.) 2. Auto-executes cleanup when process exits 3. Makes it IMPOSSIBLE to wait for stream events that never arrive Key changes: - SSHRuntime.exec(): Use DisposableProcess, cancel streams on exit - LocalRuntime.exec(): Same pattern for consistency - bash.ts: Simplified from complex multi-condition wait to simple `await exitCode` ## Benefits - ✅ Fixes the hang completely - ✅ Simpler: Removed ~130 LoC of complex finalization logic - ✅ Faster: No 50ms grace period on every command - ✅ Correct by construction: Process lifetime bounds all resources ## Impact **Invariant**: When exitCode resolves, streams are already destroyed. **Proof**: DisposableProcess registers cleanup on process 'close' event, which fires before exitCode promise resolves. Therefore: exitCode resolved ⟹ streams destroyed. No race conditions possible. ## Testing - All 955 unit tests pass - Added regression test for grep|head pattern - bash.ts tests validate no hangs - LocalRuntime/SSHRuntime tests pass --- _Generated with `cmux`_
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 endclose()promise waits for acknowledgment that may never comeSolution
Use
stdin.abort()instead ofstdin.close()for immediate closure:abort()- Immediate, synchronous force-close. Marks stream as errored, releases locks, doesn't waitclose()- Graceful, async close. Waits for all writes to be acknowledgedApplied selectively:
abort()- commands don't need stdin at allabort()- commands don't read stdinclose()- needs to flush data written to stdinTesting
runtimeExecuteBash- verifies bash commands over SSH workrenameWorkspace- verifies mv operations workruntimeFileEditing- verifies file writes (which use stdin) work correctlyImplementation Details
Changed 4 locations:
src/services/tools/bash.ts- Useabort()to close stdin immediatelysrc/runtime/SSHRuntime.ts(3 places) - Useabort()for mv/rm/stat commandsKept
close()in 1 location:SSHRuntime.writeFile()- Needs graceful close to ensure all data is writtenGenerated with
cmux