Repository navigation
Stop exporting CLAUDE_CONFIG_DIR=$HOME/.claude in the Resume-in-new-tab launch command - #4401
Conversation
…ab launch command Fixes manaflow-ai#4255
|
@mvanhorn is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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 (1)
📝 WalkthroughWalkthroughThe ChangesConfig directory normalization
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 inconclusive)
✅ Passed checks (14 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve 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 |
Greptile SummaryThis PR adds a
Confidence Score: 3/5The wrapper change is internally correct but addresses only the secondary (defense-in-depth) fix site; the Swift code that generates the problematic launch command string is unchanged. The wrapper normalization works for the primary user path where the wrapper is The Swift files named in the PR description as the root-cause fix sites (AgentLaunchEnvironmentPolicy.swift, TerminalStartupEnvironment.swift, RestorableAgentSession.swift) are not present in this changeset and warrant follow-up. Important Files Changed
Sequence DiagramsequenceDiagram
participant GUI as cmux GUI (Swift)
participant Shell as User Shell
participant Wrapper as Resources/bin/claude (wrapper)
participant Claude as Real claude binary
GUI->>Shell: "emit command: env CLAUDE_CONFIG_DIR=$HOME/.claude claude --resume id"
Note over GUI: Swift fix NOT in this PR, still emits prefix
Shell->>Wrapper: "exec with CLAUDE_CONFIG_DIR=$HOME/.claude in env"
Wrapper->>Wrapper: normalize_claude_config_dir() unsets CLAUDE_CONFIG_DIR
Wrapper->>Claude: exec without CLAUDE_CONFIG_DIR
Claude-->>Shell: resumes session without re-auth
Note over Shell,Claude: If real claude is before wrapper in PATH, CLAUDE_CONFIG_DIR reaches claude directly and re-auth bug persists
Reviews (1): Last reviewed commit: "Stop exporting CLAUDE_CONFIG_DIR=$HOME/...." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| normalize_claude_config_dir() { | ||
| # Issue #4255: Claude treats CLAUDE_CONFIG_DIR being set to its default | ||
| # path differently from the variable being absent, so normalize that case. | ||
| if [[ "${CLAUDE_CONFIG_DIR:-}" == "$HOME/.claude" ]]; then | ||
| unset CLAUDE_CONFIG_DIR | ||
| fi | ||
| } | ||
|
|
||
| # Pass through if not in a cmux terminal, hooks are disabled, or the cmux | ||
| # socket is unavailable (stale env / app not running). | ||
| IN_CMUX=0 | ||
| if [[ -n "$CMUX_SURFACE_ID" ]]; then | ||
| IN_CMUX=1 | ||
| fi | ||
|
|
||
| normalize_claude_config_dir |
There was a problem hiding this comment.
The PR description identifies three Swift files (AgentLaunchEnvironmentPolicy.swift, TerminalStartupEnvironment.swift, RestorableAgentSession.swift) as the root-cause fix sites that should stop emitting CLAUDE_CONFIG_DIR=$HOME/.claude in the generated launch command, and explicitly labels this wrapper change as "defense-in-depth." None of those files appear in this changeset (or in the repository at all). The emitted "Resume in new tab" command string therefore still contains the problematic prefix — it is only neutralized at runtime because the wrapper intercepts it. A user who copies that command and runs it in a shell where the real claude binary appears before the cmux wrapper in PATH, or any execution path that bypasses the wrapper, will still trigger the forced re-auth. The fix is incomplete relative to the PR's own description of what constitutes a full resolution.
| IN_CMUX=1 | ||
| fi | ||
|
|
||
| normalize_claude_config_dir |
There was a problem hiding this comment.
normalize_claude_config_dir is called at line 53, before the IN_CMUX/socket guard on line 55. This means a user who is entirely outside cmux and has deliberately set CLAUDE_CONFIG_DIR=$HOME/.claude in their environment (e.g., to force the custom-config-dir branch for debugging) will have it silently unset by the wrapper before exec "$REAL_CLAUDE" "$@". The PR description acknowledges this as intentional for a "probably empty" set of users, but for a wrapper that is otherwise transparent when not in cmux, silently mutating the environment in the pass-through path may surprise users who rely on this being a no-op outside cmux.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
Greptile is right -- the PR description listed both fix sites but the diff only ships the wrapper-side defense-in-depth one. I went looking for the 3 Swift files I named in the issue body (AgentLaunchEnvironmentPolicy / TerminalStartupEnvironment / RestorableAgentSession) and grep does'nt find |
|
Thank you for this! The Claude config-dir normalization is on main in 136eb2a :) |
2 participants
Summary
There are two independent fix sites; the primary one (1) is sufficient on its own, and (2) is a defense-in-depth wrapper-side patch that masks future regressions.
Primary fix — stop emitting the redundant env prefix. Find where the Vault resume launch command is assembled and only include the
env CLAUDE_CONFIG_DIR=...prefix when the value to be exported is not equal to$HOME/.claude(i.e. when the user really is on a non-default custom config dir). Candidate sites the reporter flagged via grep:Packages/CMUXAgentLaunch/Sources/CMUXAgentLaunch/AgentLaunchEnvironmentPolicy.swift— likely owns the env-var allowlist policy.Sources/TerminalStartupEnvironment.swift— likely owns the terminal-side startup env composition.Sources/RestorableAgentSession.swift— likely owns the resume command string assembly (matches the grep hit pattern from Resume launch command uses panel's current cwd, but session may have been created in a different cwd → session-not-found after restart #4256, which is the companion issue at the same code path).Also drop the two
CMUX_PRESERVE_CLAUDE_AUTH_SELECTION_ENV*variables from the emitted command — they are wrapper-internal and the wrapper unsets them anyway. Keep them inside the wrapper code path; just stop exporting them to the user's external shell.Defense-in-depth in the wrapper (
Resources/bin/claude). At the end ofnormalize_claude_config_dir(), after the legacy-path rewrite block, add:This means even if a future code path regresses and re-exports the default value, the wrapper unsets it before exec'ing claude. Tiny behavior change for the (probably empty) set of users who deliberately set
CLAUDE_CONFIG_DIR=$HOME/.claudeto force the custom-config-dir code path — documented in the PR.Do not change the non-default
CLAUDE_CONFIG_DIRpath. Users with a genuinely custom config dir (multi-account workflows, container setups) must continue to see it exported. The fix is conditional on equality with$HOME/.claude.Why this matters
The GUI command emitted by cmux's "Vault by folder → Resume in new tab" feature always begins with:
$HOME/.claudeis the Claude CLI's default forCLAUDE_CONFIG_DIR. But because Claude CLI distinguishes "variable not set" from "variable set to the default value" (via something equivalent to!!process.env.CLAUDE_CONFIG_DIRrather than a value comparison), the act of exporting the var takes Claude down its "custom config dir" branch, which fails to read the existing keychain credential and forces the user to re-authenticate every Vault resume.Reporter (treerobin06) bisected this with five command variants on cmux 0.64.6 / macOS 26.4.1 / Apple Silicon. Removing the
env CLAUDE_CONFIG_DIR=...prefix is the only variable that flips behavior from "re-auth prompt" to "session resumes cleanly". The twoCMUX_PRESERVE_*env vars are inert outside the cmux-bundledResources/bin/claudewrapper (the wrapper already unsets them before exec'ing the real claude binary), so they are dead weight in the emitted command.Testing
CLAUDE_CONFIG_DIR=$HOME/.claudeand must not containCMUX_PRESERVE_CLAUDE_AUTH_SELECTION_ENV*. Pasting the command into a fresh shell resumes the session without re-auth prompt.CLAUDE_CONFIG_DIR=/tmp/alt-claude-config claudeonce; create a session; Cmd-Q; reopen and use Resume in new tab. The generated command MUST still exportCLAUDE_CONFIG_DIR=/tmp/alt-claude-configbecause the value is non-default.Resources/bin/claudeinvoked withCLAUDE_CONFIG_DIR=$HOME/.claudeenv set must unset the variable before exec; withCLAUDE_CONFIG_DIR=/some/other/pathmust preserve it.cd '/path' && ...portion of the same emitted command remains unchanged here; Resume launch command uses panel's current cwd, but session may have been created in a different cwd → session-not-found after restart #4256 is a separate plan.Fixes #4255
AI was used for assistance.
Need help on this PR? Tag
@codesmithwith what you need.Summary by cubic
Prevents forced re-auth when using “Resume in new tab” by normalizing the environment: if
CLAUDE_CONFIG_DIRequals$HOME/.claude, the wrapper unsets it so the CLI treats it as default. Fixes #4255.Resources/bin/claude, added and invokednormalize_claude_config_dirto unsetCLAUDE_CONFIG_DIRonly when it equals$HOME/.claude; non-default paths are preserved.Written for commit cf098c2. Summary will update on new commits. Review in cubic
Summary by CodeRabbit