fix(cli): settle signal-terminated bash commands - #12679
Conversation
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The previous suggestion (missing logging on the Files Reviewed (1 file)
Previous Review Summary (commit 3dd59a5)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 3dd59a5)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
The fix itself is sound: converting the expected Files Reviewed (3 files)
Reviewed by claude-sonnet-5 · Input: 20 · Output: 2.3K · Cached: 372.5K Review guidance: REVIEW.md from base branch |
|
Closing in favor of #12698, which fixes the same root cause one layer down: the spawner now reports signal termination as the conventional 128 + signum exit code (139 for SIGSEGV), so every exitCode consumer settles, not just the bash tool race, and the signal information survives instead of collapsing to exit 1. Your raceAll analysis was spot on, thanks for digging into this. |
Issue
Fixes #12677
Context
When a command terminated the shell with a signal such as
SIGSEGV, the process exit effect failed without producing a numeric exit code. The Bash tool passed that effect directly toEffect.raceAll, which continued waiting for the abort or timeout branches after the exit branch failed. As a result, the tool appeared to hang instead of reporting command failure.Implementation
Convert expected exit-code lookup failures into a non-zero command status before entering the race. This lets signal-terminated commands settle immediately while preserving the existing abort, timeout, output-drain, and process-cleanup behavior.
A regression test terminates the real shell with
SIGSEGVand verifies that the tool completes with a failed status instead of waiting for its configured timeout.Screenshots / Video
N/A — this is a non-visual CLI process-lifecycle fix.
How to Test
Manual/local verification
bun test test/tool/shell.test.tsinpackages/opencode— 24 passed.bun run typecheckinpackages/opencode— passed.bun run lintat the repository root — completed with 0 errors and existing warnings only.bun run script/check-opencode-annotations.ts --worktree— passed.Reviewer test steps
kill -SEGV $$on Linux or macOS.Blocked checks and substitute verification
Checklist
Get in Touch
Please reach out through GitHub on this PR.