Repository navigation
Fix sidebar branch refresh after checkout (issue #666) - #905
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughReplaces mtime/zstat-based Git HEAD change detection with a content-signature approach in Bash and Zsh shell integrations; adds HEAD path resolution and signature state, updates prompt/probe restart logic to react to HEAD content changes, and adds a regression test ensuring the signature-related logic is present. Changes
Sequence Diagram(s)sequenceDiagram
participant Prompt
participant HeadResolver
participant GitProbe
participant PRProbe
Prompt->>HeadResolver: resolve HEAD path & read first-line signature
HeadResolver-->>Prompt: current HEAD signature
Prompt->>Prompt: compare with cached _CMUX_GIT_HEAD_SIGNATURE
alt signature changed
Prompt->>GitProbe: terminate/restart probe (invalidate state)
Prompt->>PRProbe: terminate/restart PR probe (invalidate PR run)
Prompt->>Prompt: update _CMUX_GIT_HEAD_SIGNATURE cache
else no change
Prompt-->>Prompt: continue normal flow
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 unit tests (beta)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes issue #666 by replacing the mtime-based Zsh path: Correctly triggers both Bash path: Restarts the git probe immediately on HEAD changes, but does not reset Test: Adds a regression guard that validates the presence of key HEAD-signature tracking variables and comparison logic in both shell integration files. The core fix is sound and addresses the stated issue (#666). However, the bash integration should also invalidate the PR probe on checkout to maintain consistency with zsh. Confidence Score: 3/5
Sequence DiagramsequenceDiagram
participant Shell as Shell Prompt
participant HT as HEAD Tracker
participant GP as Git Probe (async)
participant PRP as PR Probe (async)
participant Sidebar as cmux Sidebar
Note over Shell,Sidebar: User runs: git checkout feature-branch
Shell->>HT: _cmux_precmd / _cmux_prompt_command fires
HT->>HT: Read .git/HEAD content (signature)
HT->>HT: Compare with _CMUX_GIT_HEAD_SIGNATURE
HT->>HT: Signature changed → set git_head_changed=1 (bash) / _CMUX_GIT_FORCE=1 (zsh)
alt Bash integration
HT->>GP: kill in-flight git probe (if running)
HT->>GP: Launch new git probe immediately
GP->>Sidebar: report_git_branch feature-branch
Note over PRP,Sidebar: ⚠ PR probe NOT invalidated — waits up to 60s
else Zsh integration
HT->>GP: kill in-flight git probe (_CMUX_GIT_FORCE=1)
HT->>PRP: kill in-flight PR probe (_CMUX_PR_FORCE=1)
HT->>GP: Launch new git probe immediately
HT->>PRP: Launch new PR probe immediately
GP->>Sidebar: report_git_branch feature-branch
PRP->>Sidebar: report_pr / clear_pr (updated)
end
Last reviewed commit: bcd6e7b
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Resources/shell-integration/cmux-bash-integration.bash (1)
167-182: Consider forcing PR refresh on HEAD changes for bash parity.
git_head_changedrestarts git probes immediately, but PR refresh still waits for cwd change or the 60s cadence. After checkout, PR metadata can lag even though branch updates immediately.♻️ Suggested parity patch
- if [[ -n "$_CMUX_PR_JOB_PID" ]] && kill -0 "$_CMUX_PR_JOB_PID" 2>/dev/null; then - if [[ "$pwd" != "$_CMUX_PR_LAST_PWD" ]]; then + if [[ -n "$_CMUX_PR_JOB_PID" ]] && kill -0 "$_CMUX_PR_JOB_PID" 2>/dev/null; then + if [[ "$pwd" != "$_CMUX_PR_LAST_PWD" || "$git_head_changed" == "1" ]]; then kill "$_CMUX_PR_JOB_PID" >/dev/null 2>&1 || true _CMUX_PR_JOB_PID="" _CMUX_PR_JOB_STARTED_AT=0 fi fi - if [[ "$pwd" != "$_CMUX_PR_LAST_PWD" ]] || (( now - _CMUX_PR_LAST_RUN >= 60 )); then + if [[ "$pwd" != "$_CMUX_PR_LAST_PWD" || "$git_head_changed" == "1" ]] || (( now - _CMUX_PR_LAST_RUN >= 60 )); then if [[ -z "$_CMUX_PR_JOB_PID" ]] || ! kill -0 "$_CMUX_PR_JOB_PID" 2>/dev/null; thenAlso applies to: 217-257
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/shell-integration/cmux-bash-integration.bash` around lines 167 - 182, When a new HEAD signature is detected (git_head_changed becomes 1 via _cmux_git_head_signature updating _CMUX_GIT_HEAD_SIGNATURE) we should immediately trigger the PR metadata refresh instead of waiting for cwd change or the 60s cadence; update the code paths that currently set git_head_changed (the block using _CMUX_GIT_HEAD_LAST_PWD, _CMUX_GIT_HEAD_PATH, _CMUX_GIT_HEAD_SIGNATURE and head_signature via _cmux_git_head_signature) to also invoke the PR refresh routine (or set the same force-refresh flag the cwd-change handler uses) so PR metadata is refreshed immediately on HEAD changes, and apply the same change to the other occurrence noted around lines 217-257.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_issue_666_sidebar_branch_checkout_refresh.py`:
- Around line 26-105: The file defines a main() function with the regression
checks but no pytest test, so CI won't run it; either convert main() into a
pytest test function (rename main to
test_issue_666_sidebar_branch_checkout_refresh and remove the if __name__ ==
"__main__" exit stub) so pytest picks it up, or keep main() but add an explicit
CI invocation to run the script (e.g., call python3
tests/test_issue_666_sidebar_branch_checkout_refresh.py in the pipeline); update
any top-level SystemExit handling accordingly so the test runner or CI sees
failures via non-zero exit codes.
- Around line 11-18: Replace the non-absolute "git" executable in the
subprocess.run call with a resolved path from shutil.which to satisfy S607:
import shutil, call git_path = shutil.which("git"), check for None and raise a
clear RuntimeError if not found, and then call subprocess.run([git_path,
"rev-parse", "--show-toplevel"], ...). Update the same pattern in the other test
that uses subprocess.run(["git", ...]) (test_sidebar_cwd_git.py) so both use the
resolved git_path and explicit error handling.
---
Nitpick comments:
In `@Resources/shell-integration/cmux-bash-integration.bash`:
- Around line 167-182: When a new HEAD signature is detected (git_head_changed
becomes 1 via _cmux_git_head_signature updating _CMUX_GIT_HEAD_SIGNATURE) we
should immediately trigger the PR metadata refresh instead of waiting for cwd
change or the 60s cadence; update the code paths that currently set
git_head_changed (the block using _CMUX_GIT_HEAD_LAST_PWD, _CMUX_GIT_HEAD_PATH,
_CMUX_GIT_HEAD_SIGNATURE and head_signature via _cmux_git_head_signature) to
also invoke the PR refresh routine (or set the same force-refresh flag the
cwd-change handler uses) so PR metadata is refreshed immediately on HEAD
changes, and apply the same change to the other occurrence noted around lines
217-257.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5cda9bd-088e-4bc2-af32-448166f2657f
📒 Files selected for processing (3)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshtests/test_issue_666_sidebar_branch_checkout_refresh.py
| result = subprocess.run( | ||
| ["git", "rev-parse", "--show-toplevel"], | ||
| capture_output=True, | ||
| text=True, | ||
| ) | ||
| if result.returncode == 0: | ||
| return Path(result.stdout.strip()) | ||
| return Path.cwd() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, confirm the file exists and check the code at lines 11-18
cat -n tests/test_issue_666_sidebar_branch_checkout_refresh.py | head -25Repository: manaflow-ai/cmux
Length of output: 823
🏁 Script executed:
# Run the provided rg command to find other subprocess.run calls with partial executable paths
rg -nP 'subprocess\.run\(\s*\[\s*"[^/"]+"' tests -C2Repository: manaflow-ai/cmux
Length of output: 3959
🏁 Script executed:
# Also check the current file for all subprocess.run calls to understand the pattern
rg -n 'subprocess\.run' tests/test_issue_666_sidebar_branch_checkout_refresh.py -A5Repository: manaflow-ai/cmux
Length of output: 240
🏁 Script executed:
# Check if there are any imports of shutil or similar in the file
rg -n 'import' tests/test_issue_666_sidebar_branch_checkout_refresh.pyRepository: manaflow-ai/cmux
Length of output: 143
🏁 Script executed:
# Check for ruff configuration files
fd -t f '\.(toml|yaml|yml|ini)$' | xargs grep -l ruff 2>/dev/null || echo "No ruff config found"Repository: manaflow-ai/cmux
Length of output: 80
🏁 Script executed:
# Also check for pyproject.toml, setup.cfg, or ruff.toml
fd -t f '(pyproject\.toml|setup\.cfg|ruff\.toml)$'Repository: manaflow-ai/cmux
Length of output: 69
🌐 Web query:
Ruff S607 security rule Python linting
💡 Result:
Ruff S607 (start-process-with-partial-path, from flake8-bandit) flags code that starts a subprocess using a non-absolute executable name (e.g., "git", "ruff"). The concern is that an attacker could influence which executable runs by manipulating the PATH environment variable. Fix by using a full path to the executable. [1]
Example (flagged) → fixed: [1]
subprocess.Popen(["ruff", "check", "file.py"]) # S607
subprocess.Popen(["/usr/bin/ruff", "check", "file.py"]) # OKIf it’s acceptable in your context, you can suppress/disable it:
- Inline:
# noqa: S607[2] - Project-wide: add to
lint.ignoreinpyproject.toml/ruff.toml[2]
[tool.ruff.lint]
ignore = ["S607"]Sources: [1] [2]
🏁 Script executed:
# Check ruff.toml contents
cat ruff.tomlRepository: manaflow-ai/cmux
Length of output: 976
🏁 Script executed:
# Check if there are other uses of subprocess with "git" and no full path resolution
rg -n '"git"' tests -C1 | grep -E '(subprocess|shell|run)'Repository: manaflow-ai/cmux
Length of output: 2113
🏁 Script executed:
# Specifically look at the git call pattern in test_sidebar_cwd_git.py
cat -n tests/test_sidebar_cwd_git.py | sed -n '95,105p'Repository: manaflow-ai/cmux
Length of output: 500
Resolve S607 security violation by using resolved git binary path.
Ruff S607 (flake8-bandit) flags subprocess calls with non-absolute executable paths due to PATH pollution risk. The project's ruff.toml enables the "S" rule set without ignoring S607, making this a real linting violation.
Replace ["git", ...] with a resolved binary path using shutil.which(). The same pattern appears in at least one other test file (test_sidebar_cwd_git.py line 99) and should be corrected there as well.
Suggested fix
import subprocess
+import shutil
from pathlib import Path
def get_repo_root() -> Path:
- result = subprocess.run(
- ["git", "rev-parse", "--show-toplevel"],
- capture_output=True,
- text=True,
- )
+ git_bin = shutil.which("git")
+ if not git_bin:
+ return Path.cwd()
+ result = subprocess.run(
+ [git_bin, "rev-parse", "--show-toplevel"],
+ capture_output=True,
+ text=True,
+ )
if result.returncode == 0:
return Path(result.stdout.strip())
return Path.cwd()🧰 Tools
🪛 Ruff (0.15.2)
[error] 12-12: Starting a process with a partial executable path
(S607)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_issue_666_sidebar_branch_checkout_refresh.py` around lines 11 -
18, Replace the non-absolute "git" executable in the subprocess.run call with a
resolved path from shutil.which to satisfy S607: import shutil, call git_path =
shutil.which("git"), check for None and raise a clear RuntimeError if not found,
and then call subprocess.run([git_path, "rev-parse", "--show-toplevel"], ...).
Update the same pattern in the other test that uses subprocess.run(["git", ...])
(test_sidebar_cwd_git.py) so both use the resolved git_path and explicit error
handling.
| def main() -> int: | ||
| repo_root = get_repo_root() | ||
| zsh_path = repo_root / "Resources" / "shell-integration" / "cmux-zsh-integration.zsh" | ||
| bash_path = repo_root / "Resources" / "shell-integration" / "cmux-bash-integration.bash" | ||
|
|
||
| required_paths = [zsh_path, bash_path] | ||
| missing_paths = [str(path) for path in required_paths if not path.exists()] | ||
| if missing_paths: | ||
| print("Missing expected files:") | ||
| for path in missing_paths: | ||
| print(f" - {path}") | ||
| return 1 | ||
|
|
||
| zsh_content = zsh_path.read_text(encoding="utf-8") | ||
| bash_content = bash_path.read_text(encoding="utf-8") | ||
|
|
||
| failures: list[str] = [] | ||
|
|
||
| require( | ||
| zsh_content, | ||
| "_CMUX_GIT_HEAD_SIGNATURE", | ||
| "zsh integration is missing git HEAD signature tracking", | ||
| failures, | ||
| ) | ||
| require( | ||
| zsh_content, | ||
| "_cmux_git_head_signature", | ||
| "zsh integration is missing git HEAD signature helper", | ||
| failures, | ||
| ) | ||
| require( | ||
| zsh_content, | ||
| '"$head_signature" != "$_CMUX_GIT_HEAD_SIGNATURE"', | ||
| "zsh integration no longer compares git HEAD signatures", | ||
| failures, | ||
| ) | ||
| require( | ||
| zsh_content, | ||
| "_CMUX_GIT_FORCE=1", | ||
| "zsh integration no longer forces git probe refresh on HEAD changes", | ||
| failures, | ||
| ) | ||
|
|
||
| require( | ||
| bash_content, | ||
| "_CMUX_GIT_HEAD_SIGNATURE", | ||
| "bash integration is missing git HEAD signature tracking", | ||
| failures, | ||
| ) | ||
| require( | ||
| bash_content, | ||
| "_cmux_git_head_signature", | ||
| "bash integration is missing git HEAD signature helper", | ||
| failures, | ||
| ) | ||
| require( | ||
| bash_content, | ||
| "git_head_changed=1", | ||
| "bash integration no longer flags HEAD changes for immediate refresh", | ||
| failures, | ||
| ) | ||
| require( | ||
| bash_content, | ||
| '|| "$git_head_changed" == "1"', | ||
| "bash integration no longer restarts running git probes on HEAD change", | ||
| failures, | ||
| ) | ||
|
|
||
| if failures: | ||
| print("FAIL: issue #666 regression(s) detected") | ||
| for failure in failures: | ||
| print(f"- {failure}") | ||
| return 1 | ||
|
|
||
| print("PASS: issue #666 checkout refresh guards are present") | ||
| return 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| raise SystemExit(main()) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check whether this file exposes pytest-style tests ==="
rg -n '^def test_' tests/test_issue_666_sidebar_branch_checkout_refresh.py || echo "No pytest-style tests found"
echo
echo "=== Check whether automation explicitly invokes this script ==="
rg -n --hidden --glob '!**/.git/**' 'test_issue_666_sidebar_branch_checkout_refresh\.py' || echo "No references found in codebase"Repository: manaflow-ai/cmux
Length of output: 243
🏁 Script executed:
#!/bin/bash
# Check for common CI configuration files that might invoke this module
echo "=== Looking for CI/automation configuration files ==="
fd -t f '\.(yml|yaml|sh|makefile|Makefile|toml|cfg|ini)$' -x grep -l 'test_issue_666' {} \; 2>/dev/null || echo "No direct references in config files"
echo
echo "=== Check if there's a test runner or pytest config that might include this ==="
cat pytest.ini tox.ini setup.cfg pyproject.toml 2>/dev/null | head -100 || echo "No standard config found"Repository: manaflow-ai/cmux
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Search for any python scripts that might invoke this module or run it
echo "=== Check for Python test runners or scripts ==="
find . -maxdepth 3 -name '*.py' -type f ! -path './.git/*' -exec grep -l 'test_issue_666_sidebar_branch_checkout_refresh' {} \; 2>/dev/null || echo "No Python files reference this module"
echo
echo "=== Look at workflow files if they exist ==="
find .github -name '*.yml' -o -name '*.yaml' 2>/dev/null | xargs cat 2>/dev/null || echo "No .github workflows found"Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
# Search for patterns of explicit python test invocation in workflows
echo "=== Look for python test invocation patterns in CI workflows ==="
find .github/workflows -name '*.yml' -o -name '*.yaml' | xargs grep -l 'python.*test' 2>/dev/null || echo "No matches"
echo
echo "=== Check if any scripts invoke tests/test_issue_666* ==="
grep -r 'test_issue_666' .github scripts 2>/dev/null || echo "No matches found"
echo
echo "=== Look for the pattern of how workflow-guard-tests runs .sh tests ==="
find .github/workflows -name '*.yml' | xargs grep -A 5 'test_ci.*\.sh' | head -50Repository: manaflow-ai/cmux
Length of output: 1111
🏁 Script executed:
#!/bin/bash
# Search for any Python test invocation in workflow files more carefully
echo "=== Search for 'python' or 'python3' in all workflow files ==="
find .github/workflows -type f \( -name '*.yml' -o -name '*.yaml' \) -exec grep -n 'python' {} + | head -100
echo
echo "=== Specifically check if test_issue_666*.py is mentioned anywhere ==="
grep -r 'test_issue_666' . --include='*.sh' --include='*.yml' --include='*.yaml' --include='*.py' 2>/dev/null | grep -v '.git'Repository: manaflow-ai/cmux
Length of output: 1480
🏁 Script executed:
#!/bin/bash
# Check if there's a Makefile or test runner that might invoke this
echo "=== Look for Makefile or test runners ==="
find . -maxdepth 2 -name 'Makefile' -o -name 'makefile' -o -name 'test*.sh' | head -20
echo
echo "=== Check the tests directory structure ==="
ls -la tests/ 2>/dev/null | head -30Repository: manaflow-ai/cmux
Length of output: 2861
Add this regression guard to the CI pipeline.
Currently, this file contains no def test_... functions and is not invoked by any CI workflow. The regression checks for issue #666 in main() will never execute during CI. Either convert this to a pytest-compatible test by renaming main() to a def test_...() function, or add an explicit invocation (e.g., python3 tests/test_issue_666_sidebar_branch_checkout_refresh.py) to the CI workflow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_issue_666_sidebar_branch_checkout_refresh.py` around lines 26 -
105, The file defines a main() function with the regression checks but no pytest
test, so CI won't run it; either convert main() into a pytest test function
(rename main to test_issue_666_sidebar_branch_checkout_refresh and remove the if
__name__ == "__main__" exit stub) so pytest picks it up, or keep main() but add
an explicit CI invocation to run the script (e.g., call python3
tests/test_issue_666_sidebar_branch_checkout_refresh.py in the pipeline); update
any top-level SystemExit handling accordingly so the test runner or CI sees
failures via non-zero exit codes.
When HEAD changes (e.g. git checkout), the bash integration now resets _CMUX_PR_LAST_RUN=0 so the PR probe is forced to re-run immediately. This matches the zsh integration which already sets _CMUX_PR_FORCE=1 on HEAD change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nch-update # Conflicts: # Resources/shell-integration/cmux-zsh-integration.zsh
…naflow-ai#905) * Fix sidebar branch refresh after checkout * Fix bash PR probe not refreshing on checkout (PR review feedback) When HEAD changes (e.g. git checkout), the bash integration now resets _CMUX_PR_LAST_RUN=0 so the PR probe is forced to re-run immediately. This matches the zsh integration which already sets _CMUX_PR_FORCE=1 on HEAD change. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This branch was successfully deployed
1 participant
Summary
.git/HEADcontent signaturestests/test_issue_666_sidebar_branch_checkout_refresh.pyTesting
python3 tests/test_issue_494_sleep_wake_git_branch_recovery.pypython3 tests/test_issue_582_sidebar_git_branch_fast_path.pypython3 tests/test_issue_666_sidebar_branch_checkout_refresh.py./scripts/reload.sh --tag issue-666-branch-update(fails in current workspace due pre-existing linker errors)xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug -destination 'platform=macOS' build(fails in current workspace due pre-existing linker errors: undefinedghostty_surface_clear_selection/ghostty_surface_select_cursor_cell)Summary by cubic
Fixes the sidebar not updating the current branch after git checkout by tracking .git/HEAD content signatures and restarting probes when it changes. Also refreshes PR metadata immediately on checkout in bash to match zsh (issue #666).
Written for commit c9ff46d. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests