Skip to content

Include claude attach in claude_builtin_command_names - #10115

Closed
steve-wisdom wants to merge 2 commits into
manaflow-ai:mainfrom
steve-wisdom:claude-attach-passthrough
Closed

Include claude attach in claude_builtin_command_names#10115
steve-wisdom wants to merge 2 commits into
manaflow-ai:mainfrom
steve-wisdom:claude-attach-passthrough

Conversation

@steve-wisdom

@steve-wisdom steve-wisdom commented Aug 13, 2026

Copy link
Copy Markdown

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.

🤖 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 attach works again in cmux surfaces. Previously the wrapper injected --session-id/--settings (and NODE_OPTIONS), so the CLI treated attach as the prompt and started a new session; now it receives attach <id> untouched and attaches (or errors) as expected.

  • Adds attach to builtin command detection in claude_builtin_command_name, preventing --session-id, --settings, and NODE_OPTIONS injection.
  • Adds regression test test_hidden_attach_subcommand_bypasses_hook_injection.
  • Review focus: the one-word addition in Resources/bin/cmux-claude-wrapper; no changes to other subcommands or hook behavior.
  • No migration required; only affects claude attach <id> invocations in cmux surfaces.

Written for commit 97cbc20. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Fixed the claude attach command so it opens an existing session without being treated as a new prompt.
    • Preserved command arguments and prevented unintended hook, settings, session, and runtime option injection.
  • Tests

    • Added regression coverage for attaching to existing sessions.

…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>
@cursor

cursor Bot commented Aug 13, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@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: 066132b2-beae-4745-86ef-0b0c5540d90c

📥 Commits

Reviewing files that changed from the base of the PR and between bddca3f and 97cbc20.

📒 Files selected for processing (1)
  • Resources/bin/cmux-claude-wrapper

📝 Walkthrough

Walkthrough

The Claude wrapper now recognizes attach as a built-in command. The regression test confirms that claude attach <id> preserves arguments and bypasses hook, session-ID, settings, and NODE_OPTIONS injection.

Changes

Claude attach command handling

Layer / File(s) Summary
Recognize and validate attach passthrough
Resources/bin/cmux-claude-wrapper, tests/test_claude_wrapper_hooks.py
The wrapper recognizes attach as a built-in command. Tests verify argument passthrough and the absence of hook, session-ID, settings, and NODE_OPTIONS injection. The test is registered in main().

Estimated code review effort: 2 (Simple) | ~10 minutes

Mergeability Score: ⚪ Minimal · up to 97cbc

This narrowly changes claude attach <id> handling so it passes through without session-flag injection, with no actionable merge-blocking risk remaining after normal checks and review.

Suggested reviewers: austinywang, lawrencecchen

🚥 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 PR diff changes only the shell wrapper and adds no Swift declarations or production Swift code, so it cannot introduce Swift 6 actor-isolation mistakes.
Cmux Swift Blocking Runtime ✅ Passed The two-commit diff changes only a shell wrapper and a Python regression test; it introduces no production Swift code or blocking runtime synchronization.
Cmux Browser Automation Off-Main ✅ Passed The PR changes only the Claude shell wrapper and its Python regression test; it introduces no browser.* socket command, WebKit/AppKit access, worker routing, or main-actor routing change.
Cmux Expensive Synchronous Load ✅ Passed The actual diff changes only the non-Swift shell wrapper. It adds no Swift code or synchronous agent-history load on a main-actor or interactive path.
Cmux Cache Substitution Correctness ✅ Passed The diff changes only a Bash wrapper and a Python test; it contains no production Swift, TypeScript, or JavaScript cache substitution.
Cmux No Hacky Sleeps ✅ Passed The diff only adds attach to a shell command list and adds a regression test; no sleep, timer, polling, delay, or wall-clock wait was introduced.
Cmux Algorithmic Complexity ✅ Passed The diff adds one name to a small fixed shell command list; it introduces no scalable collection scan or slower algorithm. The added regression test is test-only.
Cmux Swift Concurrency ✅ Passed The complete PR range changes only the Claude wrapper and its Python test; it contains no Swift files or Swift concurrency patterns.
Cmux Swift @Concurrent ✅ Passed The complete PR range changes only the Claude wrapper and its Python test; no Swift files, async functions, or Swift call sites changed.
Cmux Swift Package Boundaries ✅ Passed The PR changes only a shell wrapper and a Python regression test; it introduces no production Swift changes or Swift package-boundary violation.
Cmux Swiftpm Lockfiles ✅ Passed The PR diff changes only the Claude wrapper and its Python regression test; it adds no Package.swift, Package.resolved, Xcode project, .gitignore, workflow, or dependency changes.
Cmux Swift Logging ✅ Passed The full PR diff changes only the Claude shell wrapper and a Python test; it adds no production Swift code or Swift logging.
Cmux User-Facing Error Privacy ✅ Passed The production diff only adds attach to command classification; it adds no user-facing error, alert, output, or recovery text. The added regression test is explicitly allowed.
Cmux Full Internationalization ✅ Passed The diff adds only the literal attach command token and a regression test; it changes no user-facing Swift/web/catalog text, and command names/tests are explicitly allowed.
Cmux Swiftui State Layout ✅ Passed The PR changes only the shell wrapper and its Python regression test; the diff contains no Swift or SwiftUI files, state, layout, or render code.
Cmux Architecture Rethink ✅ Passed The diff changes one Bash wrapper line and adds no Swift files or Swift lifecycle code; the custom Swift architecture failure conditions are not applicable.
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only a shell wrapper and a Python test; the PR range contains no Swift, AppKit, or SwiftUI window changes covered by this check.
Cmux Source Artifacts ✅ Passed The PR changes only the hand-written wrapper source and a regression test; no local, generated, cache, scratch, build, or copied artifact paths appear in the diff.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The PR diff contains 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 the Bash wrapper and its Python regression test; the two-commit diff contains no Swift production changes, so this check is inapplicable.
Title check ✅ Passed The title directly identifies the addition of attach to the Claude built-in command list, which is the primary code change.
Description check ✅ Passed The description clearly explains the problem, fix, affected behavior, and regression testing, but omits the template checklist and review-trigger 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.

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>
@steve-wisdom
steve-wisdom force-pushed the claude-attach-passthrough branch from bddca3f to 97cbc20 Compare August 13, 2026 17:34
@steve-wisdom steve-wisdom changed the title Pass the hidden claude attach subcommand through the claude wrapper Add claude attach to claude_builtin_command_names Aug 13, 2026
@steve-wisdom steve-wisdom changed the title Add claude attach to claude_builtin_command_names Include claude attach in claude_builtin_command_names Aug 13, 2026
@steve-wisdom

Copy link
Copy Markdown
Author

Superseded by #10117 — reopened from my personal account.

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.

1 participant