Repository navigation
Fix Claude restore cwd drift for session resume - #6205
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:
📝 WalkthroughWalkthroughAdds CWD-drift correction for agent-hook resume bindings during session restore. A new ChangesCWD Drift Correction for Agent-Hook Resume Bindings
Surface Argument Handling and Resolution Priority Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (5 errors, 1 warning)
✅ Passed checks (16 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 |
Greptile SummaryThis PR fixes Claude session resume failures caused by cwd/
Confidence Score: 4/5The core cwd-retargeting and CLAUDE_CONFIG_DIR self-healing logic is well-scoped and thoroughly tested. A known pre-existing issue in the surface-resolution path (an internal surface UUID can appear in a user-facing error under a specific stale-explicit-surface scenario) carries over from the previous review cycle. The main production paths — retargeting bindings on restore, clearing stale agent snapshots, self-healing CLAUDE_CONFIG_DIR in the wrapper, and the surface-resolution precedence refactor — are all covered by new regression tests. The one outstanding concern is in resolvePreferredSurfaceForClaudeHookDetailed: when an explicit --surface fails and the code falls back to resolveStrictSurfaceForClaudeHookDetailed(preferred, …) with an internal session-mapping UUID, a failure there emits 'Surface not found: ' rather than an error referencing the user-supplied --surface value. This was flagged in the prior review cycle and has not been addressed in this PR. CLI/cmux.swift — the resolvePreferredSurfaceForClaudeHookDetailed fallback to resolveStrictSurfaceForClaudeHookDetailed(preferred, …) can produce an error message containing an internal surface UUID when both the explicit --surface and the mapped session surface are stale. Important Files Changed
Reviews (30): Last reviewed commit: "fix: bound Claude resume resolution" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cmuxTests/CLINotifyProcessIntegrationRegressionTests.swift`:
- Around line 297-415: In the test function
testClaudeMappedSessionOverridesTTYBindingWithoutExplicitSurface, the
CMUX_SURFACE_ID environment variable is currently set to mappedSurfaceId, which
is the same value the test expects to win. To prevent regressions where the code
simply reuses the caller's environment, change CMUX_SURFACE_ID to a distinct
ambient surface ID (different from both mappedSurfaceId and ttySurfaceId) so
that the test properly verifies the persisted mapped surface is selected over
the ambient caller surface. Keep all existing assertions unchanged so they
continue to verify the mapped surface is correctly chosen when the ambient
surface differs.
🪄 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: eb81d22e-a58a-4cc6-8022-4212591a1ebf
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
CLI/cmux.swiftcmuxTests/CLINotifyProcessIntegrationRegressionTests.swift
When cmux is launched with a foreign CLAUDE_CONFIG_DIR (e.g. the .app is opened from a terminal whose agent set one), a restored `claude --resume <id>` resumes against the wrong config root and reports "No conversation found", dropping the user to a bare shell (#6194). This test fails until the wrapper self-heals CLAUDE_CONFIG_DIR to the config root that actually holds the transcript. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A restored `claude --resume <id>` only resolves a session under the current CLAUDE_CONFIG_DIR. When the cmux app inherits a foreign CLAUDE_CONFIG_DIR (e.g. the .app is opened by cmd-clicking a link from a terminal whose agent set one), it propagates that dir to every restored pane, so sessions created under a different config root resume against the wrong namespace and fail with "No conversation found" — leaving the user at a bare shell instead of their conversation (#6194). The wrapper now relocates CLAUDE_CONFIG_DIR to the config root that actually holds the transcript when resuming an explicit session id, and only when the current root lacks it (a correct resume is never repointed). Session ids are filename-token validated before any glob-walk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…store-resume-cwd # Conflicts: # .github/swift-file-length-budget.tsv
Summary
claude --resumereports "No conversation found" and the SessionEnd hook is cancelled #6194 locally in cmux96: runningclaude --resume/cmux claude-teams --resumefor a valid Claude session from a drifted cwd reportsNo conversation found with session ID.AgentResumeWorkingDirectoryso directory-namespaced agents like Claude resume from the captured launch cwd instead of a drifted runtime cwd.Testing
git show --check --stat --oneline HEAD && git show --check --stat --oneline HEAD~1Fixes #6194
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fix Claude session resume when cwd/workspace/surface or
CLAUDE_CONFIG_DIRdrift, and preserve auth selection on resume. Restores reliably return to the intended conversation and avoid “No conversation found”. Fixes #6194.--surfacewith localized errors; invalid explicit → mapped session else error. No flags: mapped session > Claude process binding (TTY, then PID with socket auth) > caller TTY > env. Ambient TTY is ignored when workspace/surface flags are set; stale TTY workspace bindings are ignored.workspace_id.cmux-claude-wrapper: on--resume <id>, self‑healCLAUDE_CONFIG_DIRto the root that holds the transcript only when the current root lacks it; validate the id token before searching; bound the resume lookup to avoid scanning unrelated paths; tolerate value flags before--resume; stop parsing at prompt text and after--; preserve safe auth selection values on resume; define the resume parser before passthrough.claude/grokwrappers after startup so user functions don’t override wrapper dispatch.Written for commit ac299ea. Summary will update on new commits.
Summary by CodeRabbit
--surfacereliably overrides conflicting environment and TTY-derived candidates, with clearer and more deterministic fallback behavior.--surface).