Repository navigation
Conversation
Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
The git HEAD watcher polls once per second in every pane whose shell is inside a git repository. Reading HEAD through $(...) forked a subshell on each poll, so many open panes caused a steady stream of process launches. Return the signature through REPLY, as the tmux env signature already does. Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Thanks for opening your first cmux pull request! We're a small team and the outside-PR queue is long, so a reply can take a while, sometimes longer than we'd like. If this one goes quiet and you'd like eyes on it, comment here and we'll pick it up. A few things that help:
|
Contributor
|
All contributors have signed the CLA ✍️ ✅ |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Cmux Cloud Persistent Session And Early Input PASS. The pull request changes only zsh Git HEAD watcher logic and related Python test registration. It does not modify Cloud terminal creation, persistent cmux-tui transport, manual renderers, Ghostt… Cmux Swift Actor Isolation The pull request changes only a Zsh integration script, a Python regression test, and a TOML test registry. The reviewed diff contains no Swift files or Swift production code, so it introduces no Swif… Cmux Swift Blocking Runtime The pull request changes only a Zsh integration script, a Python test, and TOML test configuration. The authoritative diff contains no Swift files, so it introduces no production Swift blocking or tim… Cmux Browser Automation Off-Main PASS: This pull request changes only zsh Git HEAD watcher code and its Python test registration. It does not change browser socket automation commands, Sources/TerminalController.swift, `ControlComm…Cmux Expensive Synchronous Load The pull request changes only a zsh integration script and Python/TOML tests. The reviewed diff contains no production Swift changes and no agent-history load or Swift main-actor path. The custom fail… Cmux Cache Substitution Correctness PASS: The reviewed production change is Zsh, with a Python regression test and TOML test registration. It does not modify Swift, TypeScript, or JavaScript production code, so the cache-substitution co… Cmux No Hacky Sleeps The production diff replaces command substitution with a REPLYhelper. It does not add or expand a sleep, timer, or polling loop. The existing one-second_cmux_sleep_cs 100in `_cmux_start_git_hea…Cmux Algorithmic Complexity PASS: The production diff replaces command substitution with a direct REPLYhelper and reads one HEAD line per watcher poll. It does not scan a scalable collection, sort or filter records, perform p…Cmux Swift Concurrency The pull request changes only a zsh integration script, a Python test, and a TOML test registry. It introduces no Swift code or Swift concurrency pattern. Cmux Swift @ConcurrentThe pull request changes only Zsh, Python, and TOML files. The reviewed diff contains no Swift files and introduces no Swift async functions, actor isolation, or @concurrentusage. The Swift-specific …Cmux Swift Package Boundaries PASS: The pull request changes only Zsh integration code and Python/TOML test files. The authoritative diff contains no production Swift changes, so the Swift package boundary rule does not apply. Cmux Swiftpm Lockfiles The PR changes only the zsh integration and test files. It does not modify Package.swift, Package.resolved, .gitignore, workflows, or Xcode project package references. The SwiftPM lockfile policy is t… Cmux Swift Logging The pull request changes only a zsh integration script and Python test/config files. No Swift files or Swift logging statements are added or materially changed, so the Swift logging rule does not appl… Cmux User-Facing Error Privacy PASS. The production diff adds no user-facing error or diagnostic text. _cmux_git_head_signature_into_replyreturns the HEAD line throughREPLY, and the existing wrapper keeps its prior print inte…Cmux Full Internationalization PASS: The PR changes zsh watcher behavior and adds test/registry files only. It adds no Swift UI text, app catalog or Info.plist entries, web UI/API/markdown/changelog copy, or locale data. The new sh… Cmux Swiftui State Layout PASS: The pull request changes only the zsh integration and Python/TOML test files. It contains no Swift or SwiftUI changes, so the SwiftUI state-layout rules do not apply. Cmux Architecture Rethink PASS. The pull request changes only Zsh integration code and Python test registration. It contains no Swift architecture change, so the specified Swift rethink failure conditions do not apply. Cmux Swift Auxiliary Window Close Shortcuts PASS: The pull request changes only a zsh integration script and Python/TOML test files. It adds no Swift code and does not add or modify cmux-owned auxiliary windows. The auxiliary-window close-short… Cmux Source Artifacts All three changed paths are intentional source, test, and test-registry files. The zsh integration change is hand-written product source, the Python file is a targeted regression test with runtime-onl… Cmux No Test Or Debug Seam In Production Source PASS: The pull request changes only the zsh integration script and test files. It adds or changes no Swift file under a production Sources/path, so the specified production test/debug seam conditio…Title check The title clearly and concisely describes the primary change: avoiding subshell forks when the zsh Git HEAD watcher reads HEAD. Description check The description includes the required Summary, Testing, Changelog, Proof, and Checklist sections. It explains the problem, implementation, test coverage, verification results, and limits of validation…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (1 skipped: 1 unsupported.)
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
There was a problem hiding this comment.
🟡 Changes recommended
Read failures can terminate ERR_EXIT watchers, and the new regression test is not registered for CI execution.
2 open findings
What changed in this PR
Removes per-poll zsh subshell creation from the Git HEAD watcher.
Changes:
- Adds a
REPLY-based HEAD reader while preserving the printing wrapper. - Adds regression coverage for watcher process reuse.
| File | Description |
|---|---|
Resources/shell-integration/cmux-zsh-integration.zsh |
Reads HEAD without command substitution. |
tests/test_zsh_git_head_watch_no_fork.py |
Verifies polling occurs in the watcher process. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Restore the || true guard so a missing or unreadable HEAD cannot abort an ERR_EXIT shell, and register the no-fork regression in the macos-cli-no-socket lane. Signed-off-by: Sertac Ozercan <sozercan@gmail.com>
Author
|
I have read the CLA Document v2.2 and I hereby sign the CLA |
This branch has not been deployed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Each pane whose shell is inside a git repository runs a git HEAD watcher that polls once per second while a command is in the foreground. The poll read HEAD through
$(_cmux_git_head_signature ...), so every poll forked a subshell. With many panes open this adds a steady stream of short-lived processes, roughly one per pane per second, even when nothing is happening.The watcher now reads HEAD with
_cmux_git_head_signature_into_reply, which returns the value throughREPLYlike_cmux_tmux_shell_env_signature_into_replyalready does, so the loop no longer forks._cmux_git_head_signaturekeeps its printing interface as a thin wrapper for existing callers. Branch-change detection is unchanged, and the reads stay status-neutral (|| true) so a missing HEAD can't abort anERR_EXITshell.Testing
tests/test_zsh_git_head_watch_no_fork.py. It starts the real watcher loop and records the PID that performs each HEAD read.0f52094b74): fails,3 of 3 HEAD polls ran in a forked subshell.f09de881ff): passes,git HEAD watcher polled 3 times without forking.f09de881ff:test_issue_15066_zsh_watcher_sleep.py,test_issue_10926_watcher_pid_reuse_guard.py,test_shell_no_git_watch.pyandtest_shell_git_branch_stale_cwd.py.macos-cli-no-socketlane;validate_test_execution_registry.pypasses.python3 scripts/verify-local.py: 17/17 checks passed. This is a shell-script-only change, so I didn't run a native build or test it live in a tagged build.Changelog
Changed: The zsh git branch watcher no longer starts a new process every second in each pane
Proof
Not user-visible. The sidebar's branch and PR updates behave the same; only the per-poll process launch is gone.
Checklist
Need help on this PR? Tag
@codesmith-botwith what you need.Summary by CodeRabbit