Repository navigation
Pin Claude auto-resume binding to the launch cwd (#4256) - #6741
endmoseung wants to merge 3 commits into
Conversation
…anscript Claude files each session transcript under the project directory derived from the cwd the session was *created* in, and `claude --resume <id>` only finds it from that same directory. A resume that `cd`s into a directory the agent later drifted into (e.g. repo root -> worktree) fails with "No conversation found". Add `ClaudeResumeWorkingDirectory` to the shared CMUXAgentLaunch package as the single source of truth for resolving a Claude resume cwd: - match a candidate dir's encoding to the transcript path's project-dir segment, - else recover the launch cwd from the transcript file's top-level `cwd` field, accepting it only when it re-encodes to that same project dir (no lossy decode), - else probe the Claude config roots on disk. Also extract `ClaudeProjectDirEncoding` (// and . both map to -) so the app and CLI agree on Claude's project-dir naming. Covered by unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Claude auto-resume binding (source: agent-hook) pinned the agent's runtime cwd, so after a mid-session `cd` (e.g. home -> a subdirectory) cmux's automatic restore ran `cd '<runtime-cwd>' && claude --resume <id>` and failed with "No conversation found" — the transcript lives under the launch cwd. PR manaflow-ai#5154 fixed the snapshot fork/restore path for this same drift and explicitly left the auto-resume *binding* path (the CLI hook) as a follow-up. This is that follow-up. - `publishAgentSurfaceResumeBinding` (CLI/cmux.swift) now resolves the Claude resume cwd through the shared `ClaudeResumeWorkingDirectory` before the existing namespacing fallback, gated to `kind == "claude"` so other agents are untouched. `transcriptPath` is threaded through all 8 call sites. - The app snapshot resolver delegates to the same shared type, removing the duplicated transcript-verification logic. Reproduces in the real failure where the launch capture itself collapsed to the drift (SessionStart fired after the agent moved, no trusted CMUX_AGENT_LAUNCH_CWD): both candidates are the drift, so the launch cwd is recovered from the transcript content. Covered by a CLI hook integration test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@endmoseung is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds shared Claude transcript resume helpers, wires them into restorable-session and CLI resume paths, propagates transcript paths through session records, and adds tests for transcript-based working-directory recovery and resume binding. ChangesClaude Resume Working-Directory Recovery
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (21 passed)
✨ Finishing Touches🧪 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 SummaryFixes
Confidence Score: 5/5Safe to merge — the fix is well-scoped, tested end-to-end, and the shared resolver is created once outside the session loop so no new per-record disk scans are introduced on the app's index load path. The three-step resolution logic (encoding match → transcript-content recovery → config-root probe) is correct and guarded by a strict round-trip check that prevents spoofed paths from being accepted. The shared ClaudeResumeWorkingDirectory instance in RestorableAgentSessionIndex.load() preserves the single config-root scan per load pass. The integration test reproduces the exact drift scenario from the bug report and verifies the correct cwd is pinned. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "Address review: fix sessionId optional, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ClaudeResumeWorkingDirectory(fileManager: fileManager, homeDirectory: homeDirectory) | ||
| .verifiedWorkingDirectory( | ||
| sessionId: record.sessionId ?? "", | ||
| transcriptPath: record.transcriptPath, | ||
| claudeConfigDir: record.launchCommand?.environment?["CLAUDE_CONFIG_DIR"], | ||
| candidateWorkingDirectories: [launchCwd, recordedCwd].compactMap { $0 } | ||
| ) |
There was a problem hiding this comment.
claudeVerifiedRestorableWorkingDirectory now constructs a fresh ClaudeResumeWorkingDirectory (and therefore a fresh TranscriptLookupCache) for every Claude session record in the for record in state.sessions.values loop inside RestorableAgentSessionIndex.load(). The old code received the shared claudeTranscriptLookup (created once at line 1078, still used at lines 1106 and 1124), so configRoots() — which calls contentsOfDirectory(atPath: accountRoot) on ~/.codex-accounts/claude — ran once per index load. The new code re-runs that scan per session. On top of that, the new step (b) recordedCwd(inTranscriptAtPath:) adds a synchronous FileHandle.read(upToCount: 64*1024) + JSONSerialization.jsonObject per session whenever no candidate matches the expected project-dir name. The fix is to thread the existing shared claudeTranscriptLookup into restorableWorkingDirectory / claudeVerifiedRestorableWorkingDirectory (as the old signature did) and push the new transcript-content read into the same shared cache or an off-main Task.
Rule Used: Flag production Swift that reads, decodes, or scan... (source)
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!
| public enum ClaudeProjectDirEncoding { | ||
| public static func projectDirName(forPath path: String) -> String { | ||
| path.replacingOccurrences(of: "/", with: "-") | ||
| .replacingOccurrences(of: ".", with: "-") | ||
| } | ||
| } |
There was a problem hiding this comment.
ClaudeProjectDirEncoding is a caseless enum whose only member is a public static func — precisely the pattern the cmux-no-ambient-global-state rule flags as a prohibited static-only namespace type. The encoding belongs naturally as a static func on ClaudeResumeWorkingDirectory (or as a private helper), which would keep the API surface coherent and remove the standalone namespace type.
Rule Used: Flag new ambient global state in production Swift:... (source)
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!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@CLI/cmux.swift`:
- Around line 26749-26754: The resume working directory selection for Claude is
too permissive because `verifiedClaudeWorkingDirectory` falling back to
`AgentResumeWorkingDirectory.resolve(...)` can reuse an untrusted hook cwd.
Update the logic around `resumeWorkingDirectory` so Claude only resumes when the
launch working directory is explicitly verified/trusted, and otherwise skip or
clear the resume binding instead of calling the generic fallback. Keep the fix
localized to the `verifiedClaudeWorkingDirectory` /
`AgentResumeWorkingDirectory.resolve` flow.
In `@cmuxTests/CLIGenericHookPersistenceTests.swift`:
- Around line 3745-3750: The setup call in runClaudeHook for the hooks claude
session-start path is currently ignored, which can hide an earlier failure and
cause misleading later assertions. Update the test in
CLIGenericHookPersistenceTests to capture the result of the session-start
invocation, assert that it succeeds before proceeding, and keep the resume
binding validation separate so failures point to the correct step.
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTranscriptResume.swift`:
- Around line 62-63: Make config-root discovery reusable across the load pass:
`verifiedWorkingDirectory` is creating a new `TranscriptLookupCache` for every
record, which causes repeated synchronous scans of Claude account roots during
the session-index load loop. Hoist the `TranscriptLookupCache`/`configRoots`
resolver out of the per-record path in `ClaudeTranscriptResume` and reuse it
across the loop, or make the cache injectable so `verifiedWorkingDirectory` can
share the same lookup state for all records.
- Around line 74-90: The cwd recovery logic in ClaudeTranscriptResume should not
accept a best-effort encoded candidate when it can be ambiguous between paths
like /a.b and /a/b. Update the candidate selection in the function that uses
candidates.first(where:) and recordedCwd(inTranscriptAtPath:) to prefer the
transcript’s top-level cwd when it round-trips cleanly, and otherwise fail
closed instead of returning a lossy/drifted match. Keep the
expectedProjectDirName check, but ensure
ClaudeProjectDirEncoding.projectDirName(forPath:) is only used to validate an
authoritative cwd source rather than selecting among ambiguous candidates.
- Around line 65-72: The transcript path fallback in
ClaudeTranscriptResume.swift is too shallow and uses only the immediate parent
directory, which breaks recovery for nested Claude layouts like
<project>/<sessionId>/messages/<sessionId>.jsonl when
projectDirName(containingTranscriptPath:configRoots:) cannot resolve a root.
Update the logic around normalizedNonEmptyValue and expectedProjectDirName to
infer the project directory from the known transcript path shape before falling
back to the parent, so nested paths recover the project name correctly even when
the config root is unknown.
In
`@Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeResumeWorkingDirectoryTests.swift`:
- Around line 17-24: The transcript fixture only covers the direct
<project>/<sessionId>.jsonl layout, while the production resolver also handles
the nested <project>/<sessionId>/messages/<sessionId>.jsonl shape. Update the
fixture setup in ClaudeResumeWorkingDirectoryTests to create an additional
nested-layout variant alongside the existing projectDir/transcriptPath setup,
using ClaudeProjectDirEncoding.projectDirName(forPath:) and the
sessionId/messages path so both storage shapes are covered.
In `@Sources/RestorableAgentSession.swift`:
- Around line 1511-1517: The `verifiedWorkingDirectory` call in
`RestorableAgentSession` is passing `record.sessionId ?? ""`, but
`RestorableAgentHookSessionRecord.sessionId` is already a non-optional `String`,
so remove the nil-coalescing fallback and pass `record.sessionId` directly. Keep
the rest of the `ClaudeResumeWorkingDirectory` invocation unchanged.
🪄 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: 9c7633e6-bd25-4979-ba5e-71d3747ccb35
📒 Files selected for processing (5)
CLI/cmux.swiftPackages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTranscriptResume.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeResumeWorkingDirectoryTests.swiftSources/RestorableAgentSession.swiftcmuxTests/CLIGenericHookPersistenceTests.swift
| let resumeWorkingDirectory = verifiedClaudeWorkingDirectory | ||
| ?? AgentResumeWorkingDirectory().resolve( | ||
| kind: kind, | ||
| runtimeCwd: cwd, | ||
| launchWorkingDirectory: launchCommand?.workingDirectory | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fail closed when Claude cwd verification cannot prove the launch directory.
If verifiedWorkingDirectory returns nil, this falls back to AgentResumeWorkingDirectory.resolve(...), which can reuse the drifted hook cwd and bind claude --resume to the wrong project directory. For Claude, skip/clear the resume binding unless the launch cwd is explicitly trusted, or avoid the generic fallback after verification fails.
As per path instructions, correctness-critical session identity/lifecycle should use one authoritative source and fail closed rather than guessing when the reliable signal is missing.
🤖 Prompt for 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.
In `@CLI/cmux.swift` around lines 26749 - 26754, The resume working directory
selection for Claude is too permissive because `verifiedClaudeWorkingDirectory`
falling back to `AgentResumeWorkingDirectory.resolve(...)` can reuse an
untrusted hook cwd. Update the logic around `resumeWorkingDirectory` so Claude
only resumes when the launch working directory is explicitly verified/trusted,
and otherwise skip or clear the resume binding instead of calling the generic
fallback. Keep the fix localized to the `verifiedClaudeWorkingDirectory` /
`AgentResumeWorkingDirectory.resolve` flow.
Source: Path instructions
- Sources/RestorableAgentSession.swift: `record.sessionId` is non-optional — drop the `?? ""` that wouldn't compile (caught by review). Build one `ClaudeResumeWorkingDirectory` per index-load pass and reuse it across records so the config-root scan and transcript probes aren't redone per session; remove the now-inlined wrapper and the dead `claudeProjectDirName(containingTranscriptPath:)`. - ClaudeTranscriptResume.swift: infer the project dir from the transcript shape (including nested `<id>/messages/<id>.jsonl`) when the config root is unknown, instead of using only the immediate parent. Memoize config-root discovery in the shared cache so reusing one resolver across a load loop scans account roots once. - Tests: assert the session-start hook succeeds before validating the binding; add a nested-layout recovery case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the reviews — pushed fixes for the actionable ones (sessionId A few I deliberately skipped, with reasoning — happy to change if you disagree:
Note: I couldn't run the full Xcode build locally (this machine lacks |
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
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTranscriptResume.swift`:
- Around line 68-82: The transcript-path shortcut in ClaudeTranscriptResume
should only be trusted when it actually belongs to the current sessionId. Update
the logic around normalizedNonEmptyValue(transcriptPath),
projectDirName(containingTranscriptPath:sessionId:configRoots:), and the
candidate match in ClaudeProjectDirEncoding.projectDirName(forPath:) so it
accepts only the supported direct/nested transcript shapes whose file stem
matches sessionId; otherwise return nil and fall back to the verified
config-root disk probe.
🪄 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: 8a1d2677-e6bc-4a2b-8040-bab83a4cdc41
📒 Files selected for processing (4)
Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTranscriptResume.swiftPackages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/ClaudeResumeWorkingDirectoryTests.swiftSources/RestorableAgentSession.swiftcmuxTests/CLIGenericHookPersistenceTests.swift
| if let transcriptPath = normalizedNonEmptyValue(transcriptPath) { | ||
| let expandedTranscriptPath = (transcriptPath as NSString).expandingTildeInPath | ||
| // The transcript's own storage path names the project directory Claude looks in. | ||
| let expectedProjectDirName = projectDirName( | ||
| containingTranscriptPath: expandedTranscriptPath, | ||
| sessionId: sessionId, | ||
| configRoots: roots | ||
| ) | ||
|
|
||
| if let expectedProjectDirName, !expectedProjectDirName.isEmpty { | ||
| // (a) Prefer a candidate whose encoding matches that project directory. | ||
| if let matched = candidates.first(where: { | ||
| ClaudeProjectDirEncoding.projectDirName(forPath: $0) == expectedProjectDirName | ||
| }) { | ||
| return matched |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate transcriptPath belongs to sessionId before trusting its project dir.
Right now any path under projects/<project>/... can select <project> and return a candidate, even if the file stem is not \(sessionId).jsonl. A stale/mismatched transcript path can therefore bind resume to the wrong Claude project and skip the verified config-root probe. Parse only the supported direct and nested shapes for the current session, otherwise return nil and let the disk probe/fallback handle it.
🐛 Proposed fix
private func projectDirName(
containingTranscriptPath path: String,
sessionId: String,
configRoots: [String]
) -> String? {
let standardizedPath = (path as NSString).standardizingPath
+ let expectedFileName = "\(sessionId).jsonl"
+ guard (standardizedPath as NSString).lastPathComponent == expectedFileName else {
+ return nil
+ }
for root in configRoots {
let projectsRoot = ((root as NSString).appendingPathComponent("projects") as NSString)
.standardizingPath
let prefix = projectsRoot.hasSuffix("/") ? projectsRoot : projectsRoot + "/"
guard standardizedPath.hasPrefix(prefix) else { continue }
let relativePath = String(standardizedPath.dropFirst(prefix.count))
- guard let projectDirName = relativePath.split(separator: "/", maxSplits: 1).first,
- !projectDirName.isEmpty else {
- continue
- }
- return String(projectDirName)
+ let parts = relativePath.split(separator: "/", omittingEmptySubsequences: false).map(String.init)
+ if parts.count == 2, parts[1] == expectedFileName, !parts[0].isEmpty {
+ return parts[0]
+ }
+ if parts.count == 4,
+ parts[1] == sessionId,
+ parts[2] == "messages",
+ parts[3] == expectedFileName,
+ !parts[0].isEmpty {
+ return parts[0]
+ }
+ continue
}
// Config root unknown — infer from the transcript shape. Walk up from the file: the nestedAlso applies to: 145-174
🤖 Prompt for 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.
In
`@Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/ClaudeTranscriptResume.swift`
around lines 68 - 82, The transcript-path shortcut in ClaudeTranscriptResume
should only be trusted when it actually belongs to the current sessionId. Update
the logic around normalizedNonEmptyValue(transcriptPath),
projectDirName(containingTranscriptPath:sessionId:configRoots:), and the
candidate match in ClaudeProjectDirEncoding.projectDirName(forPath:) so it
accepts only the supported direct/nested transcript shapes whose file stem
matches sessionId; otherwise return nil and fall back to the verified
config-root disk probe.
Source: Path instructions
teamleaderleo
added
bug
|
Claude resume bindings on main still lack transcript-verified launch-directory recovery; keeping this open for a current-main port and review. |
This branch has not been deployed
2 participants
Summary
Fixes the Claude auto-resume binding path (
source: agent-hook) so a restored sessioncds into the directory Claude filed the transcript under, not a runtime cwd the agent drifted into mid-session. Resolves #4256.Claude namespaces each transcript under
<config>/projects/<encoded launch cwd>/<id>.jsonl, andclaude --resume <id>only locates it from that same cwd. When a session starts in one dir and the agent latercds elsewhere (repo root → worktree, or$HOME→ a subdir), the persisted binding pinned the drifted cwd, so auto-resume rancd '<drift>' && claude --resume <id>and failed withNo conversation found.This is the follow-up that #5154 explicitly deferred:
#5154 fixed the snapshot fork/restore path (
Sources/RestorableAgentSession.swift); the binding published bypublishAgentSurfaceResumeBindinginCLI/cmux.swiftwas untouched.Why a candidate match isn't enough
In the real failure (captured from a live
0.64.9session, see #4256) both inputs toAgentResumeWorkingDirectory().resolve(...)are the drift by the time the binding is published: the runtimecwdis the drift, and the capturedlaunchCommand.workingDirectoryalso collapsed to the drift (SessionStart fired after the agent moved, and the hook process had no trustedCMUX_AGENT_LAUNCH_CWD, soagentLaunchCommandFromEnvironmentfell back toparsedInput.cwd). The true launch cwd is therefore not among the candidates.What is reliable: every Claude transcript record carries the launch cwd in a top-level
"cwd", andtranscript_pathis in the hook payload. So the launch cwd is recovered from the transcript content and accepted only when it re-encodes to the transcript's project dir (no lossy decode).Changes
ClaudeResumeWorkingDirectory+ClaudeProjectDirEncodinginCMUXAgentLaunch(single source of truth for both targets).verifiedWorkingDirectory(...):"cwd", accepted only if it re-encodes to that same project dir;publishAgentSurfaceResumeBindingresolves the Claude resume cwd through it before the existing namespacing fallback, gated tokind == "claude"(other agents unchanged).transcriptPathis threaded through all 8 call sites.Testing
CMUXAgentLaunchpackage:swift testgreen (addedClaudeResumeWorkingDirectoryTests— candidate match, transcript-content recovery, round-trip guard rejection, config scan, empty inputs; full suite 161 tests pass).cmuxTests/CLIGenericHookPersistenceTests.swift) that reproduces the drift (both candidates = drift, transcript under the launch dir) and asserts the publishedsurface.resume.setcwd/command pin the launch dir.Residual notes for reviewers
ClaudeProjectDirEncodingis now the one place to update (used by both app and CLI).cwdin transcript content can't be honored.niland falls through safely.🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Pins Claude auto-resume to the original launch directory and verifies it against the transcript to prevent “No conversation found” after mid-session
cd. Resolves #4256.Bug Fixes
ClaudeResumeWorkingDirectory: match the transcript’s project dir, else recover from transcript content with a round-trip check, else scan config roots; supports nested<id>/messages/<id>.jsonl.publishAgentSurfaceResumeBinding(Claude only) to resolve via transcript before namespacing and threadstranscriptPath; fixes non-optionalsessionIdhandling.Refactors
ClaudeProjectDirEncodingand centralizes logic in@macOS/CMUXAgentLaunch.Written for commit daa09dd. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes
CLAUDE_CONFIG_DIR) when transcript lookup isn’t provided.Tests