Reduce shell integration prompt latency - #2109
Conversation
Three changes to cut ~10-15ms from every precmd/preexec cycle: 1. Use zsh/net/unix (zsocket) for socket sends when available. Eliminates fork+exec of ncat/socat/nc for every telemetry send (~3ms per send, 3-4 sends per prompt cycle). Falls back to external tools if the module is unavailable. 2. Replace _cmux_kill_process_tree (synchronous /bin/ps -ax | awk) with direct kill in _cmux_stop_pr_poll_loop. The tree-kill enumerated all system processes on every command (~5-13ms). Orphaned children (gh, sleep) finish on their own within seconds. 3. Minor savings: guard _cmux_patch_ghostty_semantic_redraw after first success, make _cmux_clear_pr_for_panel async, cache bash send tool.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughShell integration scripts for both bash and zsh were refactored to optimize transport detection and process termination. Bash now caches send-tool selection via Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 34049e41b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" | ||
| } >/dev/null 2>&1 & disown |
There was a problem hiding this comment.
Keep clear_pr synchronous before restarting PR polling
_cmux_prompt_command calls _cmux_clear_pr_for_panel right before _cmux_start_pr_poll_loop when repo context changes, and this change makes clear_pr fire in a detached background job. That introduces a race where the new poll can send report_pr ... first and the delayed clear_pr arrives afterward, clearing a newly correct PR badge until the next poll cycle. This is a user-visible correctness regression during fast branch/directory switches.
Useful? React with 👍 / 👎.
| # Direct kill avoids the synchronous /bin/ps + awk of tree-kill (~5-13ms). | ||
| # Orphaned children (gh, sleep) finish on their own within seconds. | ||
| kill -KILL "$_CMUX_PR_POLL_PID" 2>/dev/null || true | ||
| _CMUX_PR_POLL_PID="" |
There was a problem hiding this comment.
Kill PR poll children when stopping the polling loop
Stopping the poller now sends SIGKILL only to _CMUX_PR_POLL_PID, but the loop can have an active _cmux_run_pr_probe_with_timeout child (gh pr view) that survives as an orphan and can still emit PR updates for stale context. That means old report_pr/clear_pr messages may arrive after context switches and overwrite newer sidebar state, which did not happen when the full process tree was terminated.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR reduces shell integration prompt latency by eliminating the most expensive synchronous operations in the zsh and bash hotpath. The primary gains come from three targeted changes: using
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Shell as Shell (preexec/precmd)
participant SendBg as _cmux_send_bg()
participant ZSocket as zsocket (zsh/net/unix)
participant Extern as ncat/socat/nc (fork+exec)
participant Server as cmux daemon (Unix socket)
participant PollLoop as PR Poll Loop (bg process)
participant GH as gh pr view (subprocess)
Note over Shell,Server: ZSH with zsh/net/unix available (new fast path)
Shell->>SendBg: _cmux_send_bg("report_shell_state ...")
SendBg->>ZSocket: zsocket $CMUX_SOCKET_PATH
ZSocket-->>SendBg: fd = REPLY (~0.2ms, no fork)
SendBg->>Server: print -u fd (payload + newline)
SendBg->>ZSocket: exec {fd}>&- (close)
Note over Shell,Server: Fallback path (no zsocket / bash)
Shell->>SendBg: _cmux_send_bg("report_shell_state ...")
SendBg->>Extern: { _cmux_send ... } &! (background fork)
Extern->>Server: pipe payload via ncat/socat/nc (~3ms fork cost)
Note over Shell,PollLoop: preexec — stopping the PR poll loop (new direct kill)
Shell->>PollLoop: kill -KILL $_CMUX_PR_POLL_PID
PollLoop--xPollLoop: terminated immediately
Note over GH: orphaned gh process self-terminates within seconds
GH-->>GH: (continues until network call completes)
Note over Shell,PollLoop: precmd — starting new PR poll loop
Shell->>PollLoop: _cmux_start_pr_poll_loop (new background job)
PollLoop->>GH: gh pr view (subshell)
GH-->>Server: _cmux_send("report_pr ...")
Reviews (1): Last reviewed commit: "Reduce shell integration prompt latency" | Re-trigger Greptile |
| _cmux_stop_pr_poll_loop() { | ||
| if [[ -n "$_CMUX_PR_POLL_PID" ]]; then | ||
| # Use SIGKILL directly to avoid blocking sleep in preexec. | ||
| # The poll loop is lightweight and safe to kill abruptly. | ||
| _cmux_kill_process_tree "$_CMUX_PR_POLL_PID" KILL | ||
| # Direct kill avoids the synchronous /bin/ps + awk of tree-kill (~5-13ms). | ||
| # Orphaned children (gh, sleep) finish on their own within seconds. | ||
| kill -KILL "$_CMUX_PR_POLL_PID" 2>/dev/null || true | ||
| _CMUX_PR_POLL_PID="" | ||
| fi | ||
| } |
There was a problem hiding this comment.
Orphaned
gh processes may accumulate across rapid commands
kill -KILL $_CMUX_PR_POLL_PID kills only the top-level poll loop process. The subshell chain it spawned—_cmux_run_pr_probe_with_timeout → _cmux_report_pr_for_path → gh pr view—is reparented to launchd and continues running until the network call completes (up to _CMUX_ASYNC_JOB_TIMEOUT = 20 s).
In normal interactive use this is fine (one gh call per 45 s poll, single orphan lives for ≤ 5 s). However, in rapid-fire scenarios—running many commands in quick succession, or an agent loop executing commands at < 5 s intervals—each preexec call to _cmux_stop_pr_poll_loop leaves a new orphaned gh pr view behind while the previous one is still in flight. This can stack up multiple simultaneous gh API calls, risk GitHub's per-minute rate limit, and add unnecessary background CPU/network pressure.
The same change exists in the bash integration at line 461.
If the trade-off is intentional (it is explicitly called out in the PR description), consider at minimum capping the orphan window by signaling the intermediate subshell group. For example, if _CMUX_PR_POLL_PID is set as a process group leader, kill -KILL -- -$_CMUX_PR_POLL_PID kills the entire group without the ps+awk cost.
| _CMUX_SEND_TOOL=nc | ||
| fi | ||
| } | ||
| _cmux_detect_send_tool |
There was a problem hiding this comment.
Tool detection runs before
_cmux_fix_path prepends Resources/bin
_cmux_detect_send_tool is called at line 14, but _cmux_fix_path (which prepends Resources/bin to PATH) is called near the bottom of the file (line 713). If any of the send tools ever ship inside Resources/bin, the cached _CMUX_SEND_TOOL would be empty because they aren't on PATH yet at detection time.
Today ncat, socat, and nc are all standard system tools and not in Resources/bin, so there is no practical regression. Just worth documenting or moving the detection call to after _cmux_fix_path is executed to keep ordering safe for future changes.
| _cmux_detect_send_tool | |
| _cmux_fix_path | |
| unset -f _cmux_fix_path | |
| _cmux_detect_send_tool | |
| _cmux_install_prompt_command |
(move the _cmux_detect_send_tool call just after _cmux_fix_path at the bottom of the file)
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Resources/shell-integration/cmux-bash-integration.bash">
<violation number="1" location="Resources/shell-integration/cmux-bash-integration.bash:461">
P2: Killing only the poll-loop PID can orphan in-flight PR probe children, which may emit stale PR updates for the panel after a poll-loop restart.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1. Use kill -KILL -- -$PID (process-group kill) instead of plain kill. Background jobs are process-group leaders, so this kills all descendants (gh, sleep) without /bin/ps overhead. 2. Keep bash _cmux_clear_pr_for_panel synchronous to prevent race with the next report_pr from the poll loop. Zsh version uses _cmux_send_bg which is synchronous when zsocket is available. 3. Move _cmux_detect_send_tool after _cmux_fix_path in bash so the cached tool lookup runs with the final PATH.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 84ef9e52c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [[ -n "$CMUX_TAB_ID" ]] || return 0 | ||
| [[ -n "$CMUX_PANEL_ID" ]] || return 0 | ||
| _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" | ||
| _cmux_send_bg "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" |
There was a problem hiding this comment.
Keep zsh clear_pr ordered before restarting PR polling
_cmux_clear_pr_for_panel now uses _cmux_send_bg, and in the fallback path (_CMUX_HAS_ZSOCKET=0) that send is detached. In _cmux_precmd, this clear runs immediately before _cmux_start_pr_poll_loop, so a delayed clear_pr can arrive after a fresh report_pr from the restarted poller and wipe a correct badge until the next poll cycle. This is a user-visible correctness regression on systems where zsh/net/unix is unavailable.
Useful? React with 👍 / 👎.
| _cmux_fix_path | ||
| unset -f _cmux_fix_path | ||
|
|
||
| _cmux_detect_send_tool |
There was a problem hiding this comment.
Re-resolve bash send transport after PATH changes
The transport is detected once at startup (_cmux_detect_send_tool) and then _cmux_send trusts the cached command forever. If the user later mutates PATH (common with direnv/venv/nix shells), the cached tool can disappear while another compatible tool is still present; _cmux_send then fails instead of falling back, so sidebar telemetry updates stop. The previous behavior re-checked availability per send and did not regress on post-startup PATH changes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
2 issues found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Resources/shell-integration/cmux-bash-integration.bash">
<violation number="1" location="Resources/shell-integration/cmux-bash-integration.bash:284">
P2: The new synchronous `clear_pr` send no longer suppresses transport stderr, so `ncat` errors can print into the prompt.</violation>
<violation number="2" location="Resources/shell-integration/cmux-bash-integration.bash:461">
P2: Process-group kill assumes `$!` is always a PGID leader; when that isn’t true, stop can fail silently and leave orphaned PR poll loops running.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| # Process-group kill: background jobs are process-group leaders, so | ||
| # negative PID kills the loop + all descendants (gh, sleep) without | ||
| # the synchronous /bin/ps + awk of tree-kill (~5-13ms). | ||
| kill -KILL -- -"$_CMUX_PR_POLL_PID" 2>/dev/null || true |
There was a problem hiding this comment.
P2: Process-group kill assumes $! is always a PGID leader; when that isn’t true, stop can fail silently and leave orphaned PR poll loops running.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/shell-integration/cmux-bash-integration.bash, line 461:
<comment>Process-group kill assumes `$!` is always a PGID leader; when that isn’t true, stop can fail silently and leave orphaned PR poll loops running.</comment>
<file context>
@@ -456,9 +455,10 @@ _cmux_run_pr_probe_with_timeout() {
+ # Process-group kill: background jobs are process-group leaders, so
+ # negative PID kills the loop + all descendants (gh, sleep) without
+ # the synchronous /bin/ps + awk of tree-kill (~5-13ms).
+ kill -KILL -- -"$_CMUX_PR_POLL_PID" 2>/dev/null || true
_CMUX_PR_POLL_PID=""
fi
</file context>
| kill -KILL -- -"$_CMUX_PR_POLL_PID" 2>/dev/null || true | |
| kill -KILL -- -"$_CMUX_PR_POLL_PID" 2>/dev/null || kill -KILL "$_CMUX_PR_POLL_PID" 2>/dev/null || true |
| [[ -n "$CMUX_TAB_ID" ]] || return 0 | ||
| [[ -n "$CMUX_PANEL_ID" ]] || return 0 | ||
| # Synchronous: must arrive before the next report_pr from the poll loop. | ||
| _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" |
There was a problem hiding this comment.
P2: The new synchronous clear_pr send no longer suppresses transport stderr, so ncat errors can print into the prompt.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/shell-integration/cmux-bash-integration.bash, line 284:
<comment>The new synchronous `clear_pr` send no longer suppresses transport stderr, so `ncat` errors can print into the prompt.</comment>
<file context>
@@ -280,9 +280,8 @@ _cmux_clear_pr_for_panel() {
- _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID"
- } >/dev/null 2>&1 & disown
+ # Synchronous: must arrive before the next report_pr from the poll loop.
+ _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID"
}
</file context>
| _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" | |
| _cmux_send "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" >/dev/null 2>&1 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Resources/shell-integration/cmux-zsh-integration.zsh (1)
403-408: Note:_cmux_clear_pr_for_panelmay have a race condition without zsocket.In bash, this send is explicitly synchronous with a comment explaining it must complete before the poll loop's next
report_pr. Here in zsh,_cmux_send_bgmakes it synchronous only when zsocket is available.When zsocket is unavailable and external tools are used, the backgrounded clear could race with the poll loop. The window is likely small, and the trade-off for reduced latency seems intentional per the PR objectives.
Consider adding a comment documenting this trade-off
_cmux_clear_pr_for_panel() { [[ -S "$CMUX_SOCKET_PATH" ]] || return 0 [[ -n "$CMUX_TAB_ID" ]] || return 0 [[ -n "$CMUX_PANEL_ID" ]] || return 0 + # Synchronous when zsocket is available; backgrounded otherwise. + # Small race window with poll loop is acceptable for latency trade-off. _cmux_send_bg "clear_pr --tab=$CMUX_TAB_ID --panel=$CMUX_PANEL_ID" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/shell-integration/cmux-zsh-integration.zsh` around lines 403 - 408, The helper _cmux_clear_pr_for_panel calls _cmux_send_bg which is synchronous only when zsocket is present, otherwise it backgrounds the clear and can race with the poll loop's next report_pr; update _cmux_clear_pr_for_panel to include a concise inline comment documenting this trade-off (that zsocket makes the send synchronous, external-tool fallback is backgrounded and may race, and this is an intentional latency vs. consistency choice) and reference _cmux_send_bg and the poll/report_pr flow so future readers know why no explicit synchronization was added.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Resources/shell-integration/cmux-zsh-integration.zsh`:
- Around line 403-408: The helper _cmux_clear_pr_for_panel calls _cmux_send_bg
which is synchronous only when zsocket is present, otherwise it backgrounds the
clear and can race with the poll loop's next report_pr; update
_cmux_clear_pr_for_panel to include a concise inline comment documenting this
trade-off (that zsocket makes the send synchronous, external-tool fallback is
backgrounded and may race, and this is an intentional latency vs. consistency
choice) and reference _cmux_send_bg and the poll/report_pr flow so future
readers know why no explicit synchronization was added.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ad659e2-03a3-4ea4-bfe8-2b88e8511bbf
📒 Files selected for processing (2)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zsh
Ingests all upstream fixes since 2026-03-22 including: - Fix Cmd+N crash: retain snapshot workspaces (manaflow-ai#2183, manaflow-ai#2181, manaflow-ai#2178, manaflow-ai#2173) - Fix browser pane restore after reopen (manaflow-ai#2141) - Fix Ghostty resize_split keybind (manaflow-ai#1899) - Reduce shell integration prompt latency (manaflow-ai#2109) - Fix command palette focus after terminal find (manaflow-ai#2089) - Add Codex CLI hooks (manaflow-ai#2103) - Add cmux.json custom commands (manaflow-ai#2011) - Fix window position restore on relaunch (manaflow-ai#2129) Conflict resolution: - BrowserPanel.swift: accepted upstream configureWebViewConfiguration() refactor (already includes our forMainFrameOnly:true CAPTCHA fix from PR manaflow-ai#1877) Fork-specific files preserved: - Sources/Panels/WebAuthn{Coordinator,BridgeJavaScript}.swift - Sources/FIDO2/module.modulemap - vendor/ctap2 submodule - cmux.entitlements (with camera/audio-input removed) - cmux.embedded.entitlements - .github/workflows/fork-{ci,release}.yml
* Reduce shell integration prompt latency Three changes to cut ~10-15ms from every precmd/preexec cycle: 1. Use zsh/net/unix (zsocket) for socket sends when available. Eliminates fork+exec of ncat/socat/nc for every telemetry send (~3ms per send, 3-4 sends per prompt cycle). Falls back to external tools if the module is unavailable. 2. Replace _cmux_kill_process_tree (synchronous /bin/ps -ax | awk) with direct kill in _cmux_stop_pr_poll_loop. The tree-kill enumerated all system processes on every command (~5-13ms). Orphaned children (gh, sleep) finish on their own within seconds. 3. Minor savings: guard _cmux_patch_ghostty_semantic_redraw after first success, make _cmux_clear_pr_for_panel async, cache bash send tool. * Address review: process-group kill, fix clear_pr race, reorder bash init 1. Use kill -KILL -- -$PID (process-group kill) instead of plain kill. Background jobs are process-group leaders, so this kills all descendants (gh, sleep) without /bin/ps overhead. 2. Keep bash _cmux_clear_pr_for_panel synchronous to prevent race with the next report_pr from the poll loop. Zsh version uses _cmux_send_bg which is synchronous when zsocket is available. 3. Move _cmux_detect_send_tool after _cmux_fix_path in bash so the cached tool lookup runs with the final PATH. --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
Summary
zsh/net/unix(zsocket) for socket sends, eliminating fork+exec of ncat/socat/nc per telemetry send (~3ms each, 3-4 per prompt cycle)_cmux_kill_process_tree(synchronous/bin/ps -ax | awk) with directkillin_cmux_stop_pr_poll_loop, removing ~5-13ms of synchronous process enumeration per command_cmux_patch_ghostty_semantic_redrawafter first success, make_cmux_clear_pr_for_panelasync, cache bash send tool choice at initEstimated savings: ~10-15ms per prompt cycle on zsh with zsocket, ~5-13ms on bash (from tree-kill removal).
Testing
zsh -nandbash -nsyntax checks pass--send-only)zsh/net/unixunavailablegh/sleepprocesses self-terminate within secondsRelated
Summary by cubic
Reduce shell prompt latency by ~10–15ms on zsh and ~5–13ms on bash with faster socket sends, cheaper poll shutdown, and fewer forks. Addresses the shell integration hotpath performance task.
zsh/net/unix(zsocket) for socket sends with fallback; add_cmux_send_bg(sync with zsocket, bg otherwise)._cmux_fix_path; keep_cmux_clear_pr_for_panelsynchronous to avoid races._cmux_kill_process_treewith process-group SIGKILL (kill -KILL -- -$PID) in_cmux_stop_pr_poll_loop; guard_cmux_patch_ghostty_semantic_redrawto run once.Written for commit 84ef9e5. Summary will update on new commits.
Summary by CodeRabbit