Repository navigation
Fix Claude fork cwd drift - #5149
lawrencecchen wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThreads optional transcriptPath through agent resume/fork command builders, adds Claude transcript-based project-directory resolution for working-directory selection, stores transcriptPath in snapshots and hydrations, updates SessionIndexStore to use a centralized encoder, and adds tests for cwd-drift behavior. ChangesTranscript-based working directory resolution
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related issues
Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning, 1 inconclusive)
✅ Passed checks (14 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9e28257ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let transcriptURL = projectsDir | ||
| .appendingPathComponent(RestorableAgentSessionIndex.encodeClaudeProjectDir(projectCwd.path), isDirectory: true) | ||
| .appendingPathComponent("\(sessionId).jsonl", isDirectory: false) | ||
| try writeClaudeTranscript(sessionId: sessionId, transcriptURL: transcriptURL, cwd: projectCwd) |
There was a problem hiding this comment.
Create the Claude projects directory before writing
This new test builds transcriptURL under claude-config/projects/<encoded cwd>/..., but the only directory created beforehand is hookCwd under the project tree. Unlike the neighboring tests, nothing creates projectsDir or the encoded project directory, so writeClaudeTranscript(..., transcriptURL: ...) fails with a missing-parent-directory error before any assertions run, making the test suite fail whenever this test is executed.
Useful? React with 👍 / 👎.
Greptile SummaryFixes Claude resume/fork failing with "No conversation found with session ID" when the hook cwd has drifted (e.g. into a git worktree) while the transcript still lives under the original project directory.
Confidence Score: 5/5Safe to merge; the cwd selection logic is strictly additive and falls back to prior behavior when the transcript path is absent or unrecognized. All three previously flagged issues (lossy hyphen decoding, hardcoded FileManager, duplicate encode helper) are resolved in this revision. The new shellWorkingDirectory path only activates for Claude sessions with a valid transcriptPath; all other agents and all nil-transcript Claude sessions follow the original fallback. Regression tests cover the drift scenario and the hyphenated-name edge case. No new blocking issues found. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[resumeShellCommand / forkShellCommand] --> B[shellWorkingDirectory]
B --> C{includeWorkingDirectoryPrefix AND cwd != .ignore?}
C -- No --> D[return nil]
C -- Yes --> E[fallback = normalized workingDirectory ?? launchCmd.workingDirectory]
E --> F{kind == .claude AND transcriptPath extractable?}
F -- No --> G[return fallback]
F -- Yes --> H[Extract projectDirName from transcriptPath]
H --> I[Encode workingDirectory compare to projectDirName]
I -- Match --> J[return workingDirectory]
I -- No match --> K[Encode launchCmd.workingDirectory compare to projectDirName]
K -- Match --> L[return launchCmd.workingDirectory]
K -- No match --> M[return fallback]
J --> N[cd prefix in shell command]
L --> N
M --> N
G --> N
Reviews (2): Last reviewed commit: "Share Claude project directory encoder" | Re-trigger Greptile |
| private static func decodeClaudeProjectDir(_ raw: String) -> String? { | ||
| guard !raw.isEmpty else { return nil } | ||
| let stripped = raw.hasPrefix("-") ? String(raw.dropFirst()) : raw | ||
| let candidate = "/" + stripped.replacingOccurrences(of: "-", with: "/") | ||
| var isDirectory: ObjCBool = false | ||
| guard FileManager.default.fileExists(atPath: candidate, isDirectory: &isDirectory), | ||
| isDirectory.boolValue else { | ||
| return nil | ||
| } | ||
| return candidate | ||
| } |
There was a problem hiding this comment.
decodeClaudeProjectDir inverts encoding incorrectly for hyphenated paths
The encoding replaces / → -, but hyphens already present in directory names are also stored as -. Decoding by replacing ALL - → / conflates both, producing a wrong candidate path. For a project at /Users/alice/my-project the encoded name is -Users-alice-my-project; decoding yields /Users/alice/my/project, which almost certainly doesn't exist, so fileExists returns false and the function returns nil. The outer caller then falls back to the hook cwd — exactly the drifted cwd the fix is meant to avoid. Any session whose launchCommand?.workingDirectory is absent or nil will silently use the wrong prefix when the project root contains a hyphen.
| guard FileManager.default.fileExists(atPath: candidate, isDirectory: &isDirectory), | ||
| isDirectory.boolValue else { | ||
| return nil |
There was a problem hiding this comment.
FileManager.default hardcoded in a static helper
Every other filesystem access in the loading pipeline accepts a FileManager parameter for testability and sandbox correctness. This helper hardcodes FileManager.default, making it impossible to test with a mock filesystem in isolation and inconsistent with the surrounding codebase style.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| private static func encodeClaudeProjectDir(_ path: String) -> String { | ||
| path.replacingOccurrences(of: "/", with: "-") | ||
| } |
There was a problem hiding this comment.
Duplicate
encodeClaudeProjectDir implementations
AgentResumeCommandBuilder.encodeClaudeProjectDir (added here) is identical to RestorableAgentSessionIndex.encodeClaudeProjectDir (line 1372) and SessionIndexStore.encodeClaudeProjectDir (SessionIndexStore.swift line 877). Three copies of the same one-liner with no shared source of truth means any future fix or change to the encoding scheme must be applied in three places.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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 `@Sources/RestorableAgentSession.swift`:
- Around line 451-461: The decodeClaudeProjectDir function performs a lossy
decode of encoded paths containing literal hyphens (e.g., "my-project" ->
"my/project"), so add a brief inline comment above the private static func
decodeClaudeProjectDir(_ raw: String) noting that the encode→decode
transformation is not invertible for paths containing hyphens, that this is an
intentional last-resort fallback used only after other matching logic and
guarded by FileManager.fileExists, and that callers should not rely on this
function for exact reversible encoding/decoding if hyphens may be present.
🪄 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: d2dacd4f-f552-48ab-b234-3a33ef4bcc30
📒 Files selected for processing (2)
Sources/RestorableAgentSession.swiftcmuxTests/RestorableAgentSessionIndexTests.swift
There was a problem hiding this comment.
3 issues found across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Summary
transcriptPathon restorable snapshots.Testing
./scripts/reload.sh --tag cldforkpassed.Issues
No conversation found with session IDwhen the transcript lived under the original project cwd but the hook cwd had moved to a worktree.Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Note
Medium Risk
Changes how Claude resume/fork shell commands choose the working directory; incorrect matching could still run Claude from the wrong folder, though scope is limited to restorable Claude sessions and regression tests were added.
Overview
Fixes Claude resume/fork when the hook cwd has moved (e.g. into a worktree) but the conversation transcript still lives under the original project folder.
Restorable snapshots now carry Claude
transcriptPathfrom hook records intoSessionRestorableAgentSnapshot, andAgentResumeCommandBuilderuses it when building the shellcdprefix for Claude resume/fork commands.For Claude only, the builder reads the encoded project directory name from the transcript path (
…/projects/<encoded>/session.jsonl) and picks a working directory by matching that name againstencodeClaudeProjectDiron hook/launch cwd candidates (with tilde expansion)—not by reversing hyphens to slashes. If nothing matches, behavior falls back to the previous cwd logic.SessionIndexStoredrops its localencodeClaudeProjectDirduplicate in favor ofRestorableAgentSessionIndex.encodeClaudeProjectDir. New tests cover cwd drift and avoiding lossy decode on hyphenated project dir names.Reviewed by Cursor Bugbot for commit 170a021. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Fixes Claude resume/fork when the hook cwd drifts by deriving the cd prefix from the transcript’s project directory using a shared encoder for consistent matching. Also avoids lossy decoding of encoded transcript project folders, preventing bad cd paths and the “No conversation found with session ID” error.
transcriptPathon restorable snapshots so resume/fork can locate the original project.Written for commit 170a021. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
Tests