Skip to content

Include claude attach in claude_builtin_command_name - #10117

Merged
austinywang merged 2 commits into
manaflow-ai:mainfrom
sjiang647:claude-attach-passthrough
Aug 14, 2026
Merged

Include claude attach in claude_builtin_command_name#10117
austinywang merged 2 commits into
manaflow-ai:mainfrom
sjiang647:claude-attach-passthrough

Conversation

@sjiang647

@sjiang647 sjiang647 commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Problem

claude attach <id> is a real Claude Code subcommand — the attach door printed for --bg background sessions — but it's hidden from claude --help, so it never made it into claude_builtin_command_name in cmux-claude-wrapper. Inside a cmux surface the wrapper therefore classifies it as a session entrypoint and execs the real CLI as:

claude --session-id <fresh-uuid> --settings '{...hooks...}' attach <id>

The CLI eats the injected flags and reads attach as the [prompt] positional: instead of attaching to the running background session, it mints a brand-new session with "attach" pre-filled in the composer.

Repro (any cmux surface, Claude Code ≥ 2.1.x): claude attach <any-id> → new session with "attach" in the prompt box, instead of No job matching '<any-id>' / attaching.

Fix

Add attach to the builtin-command list so it passes through untouched, like agents/mcp/etc. Hook injection is meaningless for attach anyway — hooks live in the already-running background writer process, and a fresh --session-id on an attach invocation is exactly the bug.

Two-commit structure per the regression-test policy: commit 1 adds the failing test (test_hidden_attach_subcommand_bypasses_hook_injection in tests/test_claude_wrapper_hooks.py), commit 2 the one-word fix. Full wrapper suite passes locally.

Supersedes #10115 (same change, reopened from the right account).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Passes the hidden claude attach <id> subcommand through the cmux-claude-wrapper so attaches work inside cmux surfaces. Previously the wrapper injected --session-id/--settings, the CLI treated "attach" as the prompt, and created a new session; now argv is forwarded unchanged and no hook or NODE_OPTIONS injection occurs.

Review notes

  • Add attach to the builtin command list in claude_builtin_command_name, matching other pass-through subcommands.
  • Add regression test test_hidden_attach_subcommand_bypasses_hook_injection to assert raw argv and absence of injected flags.
  • No migration required; hooks are irrelevant for attach because they run in the existing background writer process.

Written for commit 8157248. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Preserved the behavior of the hidden claude attach <id> command so it runs without unintended hook settings, session identifiers, or NODE_OPTIONS changes.
  • Tests

    • Added regression coverage to verify attach commands pass through unchanged.

sjiang647 and others added 2 commits August 13, 2026 10:36
…k injection

`claude attach <id>` (the attach door for --bg background sessions) is a real
subcommand but hidden from `claude --help`, so it's missing from the wrapper's
builtin-command list. The wrapper classifies it as a session entrypoint and
injects --session-id/--settings ahead of it, which makes the real CLI treat
"attach" as the [prompt] positional: instead of attaching, it mints a brand-new
session with "attach" pre-filled in the composer.

Test-only commit (two-commit regression policy): CI should go red here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add `attach` to claude_builtin_command_name so the wrapper classifies
`claude attach <id>` as a command invocation and execs the real CLI with argv
untouched, like agents/mcp/etc. Hook settings are meaningless for attach
anyway: hooks live in the already-running background writer process, and the
injected fresh --session-id is exactly what turned an attach into a new
session.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e00c6903-12e5-4bfc-a859-41e95105557d

📥 Commits

Reviewing files that changed from the base of the PR and between 3ee68ba and 8157248.

📒 Files selected for processing (2)
  • Resources/bin/cmux-claude-wrapper
  • tests/test_claude_wrapper_hooks.py

📝 Walkthrough

Walkthrough

The change adds regression coverage for hidden claude attach <id> commands. The test verifies raw argument passthrough without hook settings, session ID, or NODE_OPTIONS injection. The wrapper command list remains unchanged.

Changes

Claude attach passthrough

Layer / File(s) Summary
Attach passthrough coverage
Resources/bin/cmux-claude-wrapper, tests/test_claude_wrapper_hooks.py
The test suite verifies hidden attach commands preserve raw arguments and bypass hook, session, and NODE_OPTIONS injection. The test is registered in main().

Estimated code review effort: 1 (Trivial) | ~5 minutes

Mergeability Score: ⚪ Minimal · up to 81572

The PR makes the hidden attach command bypass wrapper-only session setup and adds regression coverage; no actionable merge-blocking risk remains after normal checks and review.

Possibly related PRs

Suggested reviewers: azooz2003-bit

🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Swift Actor Isolation ✅ Passed The two-commit diff changes only a shell wrapper and a Python test; it introduces no production Swift code or Swift actor-isolation behavior.
Cmux Swift Blocking Runtime ✅ Passed The PR changes only a shell wrapper and a Python regression test; it introduces no production Swift code or blocking/timing synchronization primitives.
Cmux Browser Automation Off-Main ✅ Passed The two-commit PR changes only the Claude wrapper and its Python regression test; it adds no browser socket command or WebKit/AppKit routing change in the policy scope.
Cmux Expensive Synchronous Load ✅ Passed The PR changes only a shell wrapper and Python tests; both commit diffs contain no Swift files or synchronous agent-history loads on interactive paths.
Cmux Cache Substitution Correctness ✅ Passed The diff changes only a Bash wrapper and a Python test. It contains no production Swift, TypeScript, or JavaScript change, so this cache-substitution check is inapplicable.
Cmux No Hacky Sleeps ✅ Passed The diff adds only attach to the shell command list and a deterministic regression test; it introduces no fixed sleeps, timers, polling, or wall-clock synchronization.
Cmux Algorithmic Complexity ✅ Passed The PR adds one entry to a fixed case command list and a regression test; it introduces no scalable collection scan, nested loop, sort, filter, join, or slower algorithm.
Cmux Swift Concurrency ✅ Passed The PR changes only the shell wrapper and a Python regression test; the two-commit diff contains no Swift paths or Swift concurrency changes.
Cmux Swift @Concurrent ✅ Passed The cumulative PR diff changes only the shell wrapper and Python test; it contains no .swift paths or Swift concurrency annotations, so this check is inapplicable.
Cmux Swift Package Boundaries ✅ Passed The complete PR diff changes only a shell wrapper and a Python test; it introduces no production Swift code, so the Swift package-boundary rule does not apply.
Cmux Swiftpm Lockfiles ✅ Passed The PR diff changes only the Claude wrapper and its Python regression test; it contains no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency changes covered by thi...
Cmux Swift Logging ✅ Passed The complete diff changes only the shell wrapper and its Python test; it adds no Swift code or logging, so the Swift logging rule is not applicable.
Cmux User-Facing Error Privacy ✅ Passed The diff only adds attach to command passthrough logic and adds regression test code/comments; it adds no user-facing error, alert, command output, or recovery text.
Cmux Full Internationalization ✅ Passed The diff adds only the literal attach command token and regression-test code; no Swift, web, catalog, locale, or metadata files changed. Command tokens and tests are allowed.
Cmux Swiftui State Layout ✅ Passed The full PR diff changes only a shell wrapper and a Python test; it adds no SwiftUI files, state, GeometryReader, lazy rows, or render-time mutation.
Cmux Architecture Rethink ✅ Passed The diff changes only a shell wrapper and Python regression test; no Swift files or listed architecture symptom patches are introduced.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The diff changes only a shell wrapper and Python tests; it adds no Swift-owned window code, so the auxiliary-window shortcut rule is not applicable.
Cmux Source Artifacts ✅ Passed The two-commit diff changes only the hand-written wrapper script and its regression test; no logs, caches, build output, scratch directories, or copied artifacts are added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The PR diff changes only Resources/bin/cmux-claude-wrapper and tests/test_claude_wrapper_hooks.py; it changes no Swift file under a production Sources/ path.
Cmux No Ambient Global State ✅ Passed The PR changes only a shell wrapper and Python tests; the two-commit diff contains no production Swift changes, so this check is inapplicable.
Title check ✅ Passed The title clearly identifies the main change: adding the hidden claude attach subcommand to the builtin-command list.
Description check ✅ Passed The description clearly explains the problem, fix, regression test, and local test result, but it omits several template sections.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sjiang647 sjiang647 changed the title Pass the hidden claude attach subcommand through the claude wrapper Include claude attach in claude_builtin_command_name Aug 13, 2026
@sjiang647

Copy link
Copy Markdown
Contributor Author

small fix @lawrencecchen PTAL, ty!

@austinywang
austinywang merged commit 1329f5a into manaflow-ai:main Aug 14, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants