Fix Ctrl-Z killing suspended foreground jobs in bash sessions#2663
Fix Ctrl-Z killing suspended foreground jobs in bash sessions#2663austinywang wants to merge 2 commits intomainfrom
Conversation
When a foreground job is suspended with Ctrl-Z (e.g. vim), bash sends
SIGTSTP to the foreground process, reclaims the terminal, and runs
PROMPT_COMMAND. cmux's _cmux_prompt_command then spawned background
subshells with `{ ... } & disown`. The disowned children inherited
the shell's process group: when the background subshell exited, bash
noticed a stopped job in the same pgid and ran its cleanup, which
prints `bash: warning: deleting stopped job 1` and SIGHUPs the whole
process group — killing the suspended foreground job.
The fix is to put each background block in its own process group so
bash's stopped-job cleanup cannot reach it. This patch wraps every
`{ ... } & disown` site in `( set -m; { ... } & )` for fire-and-forget
work, and `$( set -m; { ... } & echo $! )` for PID-tracked work
(_cmux_start_pr_poll_loop, the git branch probe, and the PR probe
subshell). `set -m` enables job control inside the subshell so the
inner `&` gets its own pgid; `echo $!` relays the inner PID so existing
kill -0 / kill cleanup paths still work. setsid is not used because it
is not in macOS's default PATH.
All call sites in cmux-bash-integration.bash were updated:
_cmux_relay_rpc_bg, _cmux_report_tty_once, _cmux_report_shell_activity_state,
_cmux_ports_kick, _cmux_emit_pr_command_hint, the report_pwd block in
_cmux_prompt_command, _cmux_run_pr_probe_with_timeout,
_cmux_start_pr_poll_loop, and the git branch probe in _cmux_prompt_command.
Diagnosis credit: @anthhub. Initial fix approach: @freshtonic.
Closes #2105
Co-authored-by: anthhub <anthhub@users.noreply.github.com>
Co-authored-by: freshtonic <freshtonic@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This review could not be run because your cubic account has exceeded the monthly review limit. If you need help restoring access, please contact contact@cubic.dev. |
📝 WalkthroughWalkthroughReplaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryFixes the Ctrl-Z / stopped-job SIGHUP bug (#2105) by replacing every Confidence Score: 5/5Safe to merge — all background spawn sites are correctly isolated; the only finding is a cosmetic P2 inconsistency in kill semantics for the git probe. All 9 call sites are consistently updated with the correct No files require special attention; Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["PROMPT_COMMAND fires\n(e.g. after Ctrl-Z + fg)"] --> B["_cmux_prompt_command"]
B --> C["_cmux_report_shell_activity_state"]
B --> D["_cmux_report_tty_once"]
B --> E["report_pwd block"]
B --> F["_cmux_start_pr_poll_loop"]
B --> G["git branch probe"]
B --> H["_cmux_ports_kick"]
B --> I["_cmux_emit_pr_command_hint"]
C --> C1["( set -m; \n{ _cmux_send } & )\nown pgid"]
D --> D1["( set -m; \n{ _cmux_send } & )\nown pgid"]
E --> E1["( set -m; \n{ _cmux_send } & )\nown pgid"]
F --> F1["_CMUX_PR_POLL_PID=$(\n set -m; { loop } &; echo $! )\nown pgid — kill via -pgid"]
G --> G1["_CMUX_GIT_JOB_PID=$(\n set -m; { git } &; echo $! )\nown pgid"]
H --> H1["( set -m; \n{ _cmux_send } & )\nown pgid"]
I --> I1["( set -m; \n{ _cmux_send } & )\nown pgid"]
style C1 fill:#d4edda,stroke:#28a745
style D1 fill:#d4edda,stroke:#28a745
style E1 fill:#d4edda,stroke:#28a745
style F1 fill:#d4edda,stroke:#28a745
style G1 fill:#d4edda,stroke:#28a745
style H1 fill:#d4edda,stroke:#28a745
style I1 fill:#d4edda,stroke:#28a745
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Cursor Bugbot caught a real bash bug in _cmux_run_pr_probe_with_timeout: the probe PID comes out of a command-substitution subshell, so the probe is not a direct child of the caller and bash always falls back to 127. I fixed that by keeping the isolated process-group launch, but having the probe write its exit status to a mktemp status file instead. The timeout path now removes that file and returns 1 explicitly, and the success path does a short post-exit sleep before reading the status file to avoid the flush race. I also re-checked the rest of cmux-bash-integration.bash for the same plus shape and didn’t find any other wait-on-non-child sites. |
|
Cursor Bugbot caught a real bash bug in |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9039a6a. Configure here.
| echo $? > "$status_file" | ||
| ) & | ||
| echo $! | ||
| ) |
There was a problem hiding this comment.
Nested set -m lets probes escape process-group kill
Medium Severity
_cmux_run_pr_probe_with_timeout uses set -m to give the probe its own process group, but it only runs inside the poll loop (which already has its own process group from _cmux_start_pr_poll_loop). When _cmux_halt_pr_poll_loop does kill -KILL -- -"$_CMUX_PR_POLL_PID", the probe is in a different process group and survives. The timeout-enforcement loop dies with the poll loop, leaving an unsupervised gh process. Temp status files also leak. Previously the probe inherited the poll loop's process group and was killed together with it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9039a6a. Configure here.
1 participant
Summary
{ ... } & disownsite incmux-bash-integration.bashso the background work runs in its own process group, isolating it from bash's stopped-job SIGHUP cleanup.Root cause
When a foreground job is suspended with Ctrl-Z, bash sends SIGTSTP to the foreground process, reclaims the terminal, and runs
PROMPT_COMMAND. cmux's_cmux_prompt_commandthen spawned background subshells with{ ... } & disown.disownonly removes the job from bash's job table — it does not put the child in a separate process group. The disowned child stays in the shell's pgid. When that background subshell exits, bash notices a stopped job in the same pgid and runs its cleanup, which printsbash: warning: deleting stopped job 1 with process group <pgid>and SIGHUPs the whole pgid — killing the suspended foreground job.Diagnosis credit goes to @anthhub in the issue thread; initial fix approach goes to @freshtonic.
The fix
Replace every
{ ... } >/dev/null 2>&1 & disownwith one of:( set -m; { ... } >/dev/null 2>&1 & )— the outer subshell enables job control viaset -m, so the inner&gets its own pgid, then exits. Bash's stopped-job cleanup can no longer reach it.var=$( set -m; { ... } & echo $! )— same trick, but the inner PID is relayed viaecho $!so existingkill -0/killcleanup paths still work.setsidwas rejected because it is not in macOS's default PATH (per the discussion in the issue thread).Call sites updated
All in
Resources/shell-integration/cmux-bash-integration.bash:_cmux_relay_rpc_bg_cmux_report_tty_once_cmux_report_shell_activity_state_cmux_ports_kick_cmux_emit_pr_command_hint_cmux_prompt_command(report_pwd block)_cmux_run_pr_probe_with_timeout(PID-tracked)_cmux_start_pr_poll_loop(PID-tracked)_cmux_prompt_commandgit branch probe (PID-tracked)After this patch,
grep 'disown\| & *$' cmux-bash-integration.bashshows zero unwrappeddisowns.Test plan
bash -n cmux-bash-integration.bashpasses (no syntax regressions)../scripts/reload.sh --tag issue-2105-ctrl-zbuilds successfully.bash -i), runvim, hit Ctrl-Z. Expected:[1]+ Stopped vim, prompt returns, nobash: warning: deleting stopped jobline,fgresumes vim cleanly.less,nano,sleep 30,python3 -i.cdandgit checkout(regression check for the PID-tracked rewrites).ports_kickstill fires (open a workspace, runpython3 -m http.server 8765, see the port appear in the sidebar).🤖 Generated with Claude Code
Note
Medium Risk
Touches bash job-control/process management across multiple async paths; a mistake could break PR polling, git/TTY/ports reporting, or leave orphaned processes, but scope is limited to shell integration.
Overview
Fixes a bash job-control bug where cmux’s background subshells could trigger bash stopped-job cleanup and SIGHUP suspended foreground apps after Ctrl-Z.
All
{ ... } & disowncall sites incmux-bash-integration.bashare rewritten to run under an isolated process group viaset -m, including TTY/shell-state/PWD/ports reporting and relay RPC.Async workflows that need PID tracking (PR probe timeout handling, PR poll loop, git branch probe) are updated to launch in an isolated process group while still returning a usable PID; PR probe exit status is now captured via a temp status file instead of
wait.Reviewed by Cursor Bugbot for commit 9039a6a. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes Ctrl-Z killing suspended foreground jobs in cmux bash sessions by running background tasks in their own process groups. Suspended apps like vim now survive, no warning, and
fgresumes cleanly. Fixes #2105.{ ... } & disownwith isolated wrappers:( set -m; { ... } >/dev/null 2>&1 & )$( set -m; { ... } & echo $! )cmux-bash-integration.bashfor PR probes/polling, git branch probe, ports kick, TTY and shell-state reports,report_pwd, and RPC relay.wait, preserving timeouts and kill checks with isolated pgids.set -mgives the inner&its own process group, avoiding bash’s stopped-job SIGHUP afterPROMPT_COMMAND.setsidto keep macOS default PATH compatibility.Written for commit 9039a6a. Summary will update on new commits.
Summary by CodeRabbit