🤖 feat: background bash with event-based API and blocking output #978
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
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
ammar-agent
force-pushed
the
cleanup-shell-feature-api
branch
29 times, most recently
from
47c7fd3 to
77735ea
Compare
…d bash state - Add 'change' event emission to BackgroundProcessManager for state changes - Replace list() and getForegroundToolCallIds() API with single subscribe() endpoint - Update frontend hook to use async iterator subscription instead of polling - Add parallel work guidance to bash tool description - Move compareSets to useStableReference.ts - Fix storybook test to find visible text
… processes - Replace separate stdout.log/stderr.log with single output.log using 2>&1 redirection - Add tempDir() method to Runtime interface for runtime-agnostic temp directory resolution - Remove bgOutputDir parameter from spawnProcess() - now uses runtime.tempDir() internally - Simplify BackgroundHandle.readOutput() to always read from output.log - Update OutputReadPosition to use single outputBytes instead of separate stdout/stderr tracking - Fix test cleanup to remove output files from /tmp/mux-bashes/ This makes background process code fully runtime-agnostic - no more SSH-specific branching.
… process code - Add createAsyncEventQueue utility for converting events to async iterators - Simplify router subscription handlers using the new utility - Extract computeOutputPaths() and constants (BG_OUTPUT_SUBDIR, OUTPUT_FILENAME, EXIT_CODE_FILENAME) - Reuse computeOutputPaths() in both spawnProcess and migrateToBackground - Fix lint errors in tempDir() implementations
…ol tests The tests use hardcoded workspace IDs and the executor writes output files to /tmp/mux-bashes/<workspaceId>/. Without cleanup, stale exit_code files from previous test runs cause processes to appear already 'exited'.
- Add quotePathForShell() helper that combines toPosixPath + shellQuote - Use shell-safe quoting for paths in mkdir/touch commands - Fix double-quoting issue by not pre-quoting paths passed to buildWrapperScript (buildWrapperScript already uses shellQuote internally) This fixes spawn failures when display_name contains spaces (e.g., 'Dev Server').
The processId is now the display_name (e.g., 'Sleep Process') rather than the old bash_\d+ format. Update extractProcessId regex to match any non-empty string after 'Background process started with ID: '.
- Rename App.backgroundBash.stories.tsx to App.bash.stories.tsx - Move WithBashTool and WithBashToolWaiting from App.chat.stories.tsx - Rename to ForegroundComplete and ForegroundWaiting for clarity - Group foreground and background bash stories together
- Make timeout_secs required on both bash and bash_output tools - bash_output blocks up to timeout_secs waiting for output (max 15s) - Background processes auto-terminate after timeout_secs expires - This reduces polling spam in UI by shifting wait to tool level The blocking approach means one tool call with timeout=10 replaces ten polling calls at 1-second intervals.
- Add platform check in bash.ts to return clear error on Windows - Add Windows to CI test-unit matrix (runs bash tool tests only) - Skip background process tests on Windows (require Unix features) Background bash relies on Unix-specific features (nohup, process groups, kill signals) that don't exist on Windows. This makes the failure explicit with an actionable error message guiding users to foreground execution.
The trap command in buildWrapperScript used single quotes which broke when the exit code path contained spaces (e.g., 'PR Checks 978'). The fix uses double quotes for the trap command with escaped $? to allow proper nesting of single-quoted paths. Also: - Replace isWindows/rootDir with FALLBACK_CWD="/tmp" (assumes POSIX) - Extract errorMsg() helper for consistent error logging - Fix macOS compat: use 'wc -c' instead of 'stat -c%s' for file size - Fix perf: use 'tail -c +N' instead of slow 'dd bs=1 skip=N' Includes regression test that executes the wrapper script and verifies exit code is captured correctly.
Re-adds validation that blocks bash commands starting with 'sleep'. Agents should use sleep in polling loops instead of standalone sleeps which waste time waiting. Example error message guides toward: - 'while ! condition; do sleep 1; done' - 'until condition; do sleep 1; done'
ammar-agent
force-pushed
the
cleanup-shell-feature-api
branch
from
dd89175 to
15237bf
Compare
Reduces UI review burden by combining all bash tool states into 3 comprehensive stories: - Foreground: complete + waiting states - BackgroundWorkflow: spawn, output (running/exited/error/filtered/empty), list, terminate - Mixed: foreground vs background side-by-side with error state
ammar-agent
changed the title
Three new tests in backgroundBashDirect.test.ts: - should migrate foreground bash to background and continue running - should preserve output across stream boundaries - should handle migration when process exits during send Tests verify the complete fg→bg migration flow including: - Process continues running after migration - Output is accessible via bash_output after new stream begins - Manager state transitions correctly (foreground→background)
When a foreground bash is migrated to background and then the original stream is aborted (e.g., user sends a new message), the process would previously be killed with exit code -997 (EXIT_CODE_ABORTED). Fix: Wrap the abort signal in bash.ts with a controllable layer. When migrating to background, set abortDetached=true to prevent the abort from propagating to the runtime. This keeps the Runtime interface minimal - no changes needed there. Added regression test: 'should not kill backgrounded process when abort signal fires'
ammar-agent
force-pushed
the
cleanup-shell-feature-api
branch
2 times, most recently
from
14474cc to
17578ba
Compare
… tools in stories - Add 'backgrounded' ToolStatus for foreground→background migration - Override status in BashToolCall when result has backgroundProcessId - Add cyan color (--color-backgrounded) across all 4 theme sections - Add play functions to bash stories to auto-expand tool details - Add createMigratedBashTool mock helper for stories - Update Mixed story to show all three states: foreground, background spawn, and migrated _Generated with mux_
ammar-agent
force-pushed
the
cleanup-shell-feature-api
branch
from
17578ba to
f925cdf
Compare
Display the wait timeout duration in the tool header when > 0. Helps debug/understand blocking behavior when checking background process output. _Generated with mux_
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.
Summary
Refactored background bash feature with an event-based subscription API that aligns with Claude Code's tool conventions. Adds blocking timeout for
bash_outputto reduce polling spam, and includes a UI banner for managing running processes.Key Changes
Background Bash Tools
bash: Addedrun_in_backgroundandtimeout_secs(required) parameters. Background processes returnprocess_idfor trackingbash_output: New tool with blockingtimeout_secs(0-15s) - waits for output instead of returning immediately. Returns incremental output via byte-offset trackingbash_background_list: Lists all background processes with status/uptimebash_background_terminate: Terminates a process by IDBlocking Timeout Implementation
timeout_secsis required onbash_output(0-15 seconds)Background Processes Banner UI
Bug Fixes
wc -cinstead ofstat -c%stail -c +Ninstead of slowdd bs=1 skip=Nsleepcommands at script startTest Coverage
bash_outputtimeout behavior (blocking wait, early return, immediate return)Generated with
mux