Repository navigation
Fix Claude shim mutual exec loop - #7010
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe wrapper now detects shim-loop re-entry, routes resolved Claude launches through a guarded exec helper, refreshes re-entry metadata and Node options, and adds regression coverage for mutual loops, finite chains, child execution, interactive hook behavior, and CI execution. ChangesClaude wrapper re-exec guarding
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (22 passed)
✨ 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Resources/bin/cmux-claude-wrapper`:
- Around line 98-116: The loop guard is being cleared too early in
cmux_claude_wrapper_target_is_obvious_real_claude, because any target whose
first line mentions node is treated as the real Claude binary. Tighten this
check so Node-based script shims are not considered “obvious real” and keep the
hop/seen guard intact when execing them. Only clear the guard after stronger
validation that the target is the configured real binary, preserving the
mutual-exec protection in the wrapper flow.
- Around line 51-60: The guard diagnostic in cmux-claude-wrapper should be made
user-safe and localizable instead of printing hard-coded English/vendor-specific
text. Update the stderr message path in the claude shim loop guard to use
generic localized copy, and remove the raw target emission from the
repeated-target branch so internal paths, usernames, or IDs are not exposed.
Keep the loop-detection behavior intact, but ensure any remaining user-facing
output follows the localization and redaction rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9173d029-dd71-4c9b-9326-c45ff1030f22
📒 Files selected for processing (3)
.github/workflows/ci.ymlResources/bin/cmux-claude-wrappertests/test_claude_wrapper_mutual_shim_loop.py
Summary
claudeshim exec loopcmux-claude-wrapperso same-process shim ping-pong fails loudly instead of spinningclaudelaunches fresh by resetting inherited guard state when the wrapper is entered from a different PIDFixes #7009
Follow-up: issue fix #2, persisting a Claude binary path in
cmux.jsonso the setting survives app relaunch/self-updater clean environments, remains separate from this intrinsic loop breaker.Tests
python3 tests/test_claude_wrapper_mutual_shim_loop.pypython3 tests/test_claude_wrapper_user_binary_resolution.pypython3 tests/test_claude_wrapper_hooks.pybash -n Resources/bin/cmux-claude-wrapperNeed help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Prevents infinite mutual
claudeshim exec loops with a hop‑limited guard and stricter shim detection. Restores hook metadata andNODE_OPTIONSon shim re‑entry while allowing valid child launches, finite shim chains, and real shell launchers (e.g.,@anthropic-ai/claude-code/cli.js). Fixes #7009.CMUX_CUSTOM_CLAUDE_PATHor set Claude Binary Path); guard env is not leaked to passthrough realclaude.--settings/--session-id, refreshes per‑process hook metadata (CMUX_CLAUDE_PID,CMUX_CLAUDE_HOOK_CMUX_BIN), restoresNODE_OPTIONS(reinstalls cmux--requirewhile preserving original flags), clears inherited auth env (e.g.,CLAUDECODE), and keeps argv unchanged to avoid duplicate hook injection; narrowed shim detection scans shebang/first lines and common shell/Node/Python re‑exec patterns, treats Node‑shebang scripts and the@anthropic-ai/claude-codeshell launcher as non‑shims, skipsasdf/mise/volta, and routes execs through one launcher.Written for commit 6efcb54. Summary will update on new commits.
Summary by CodeRabbit
claudetarget appears to be part of a shim loop, failing safely with a conflict/remedy message.