Rescue split/new-tab cwd inheritance while a resumed agent holds the pane (#7155) - #7165
Conversation
…d tracked cwd (#7155) After session restore with an auto-resumed agent (e.g. Claude), the agent holds the pane's foreground for the rest of the run: no shell prompt ever runs, so the pane's tracked cwd cannot self-correct. The #6617 restore guard swallows only the FIRST spurious post-restore pwd report; any later stray report parks the tracked cwd (and the workspace cwd) on the surface default (home) with nothing left to repair it. Cmd+D / Cmd+T from that pane then open in ~ instead of the directory the resumed session lives in. The tests restore an auto-resumed Claude workspace, reproduce the clobbered field state (guard consumed by the first home report, second report accepted), and assert split and new-tab inheritance still resolve the resumed session's directory. They fail until the inheritance path learns to rescue the clobbered value. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the pane (#7155) After session restore with an auto-resumed agent (e.g. Claude), the agent holds the pane's foreground for the rest of the run: no shell prompt ever runs, so the pane's tracked cwd cannot self-correct. The one-shot #6617 restore guard swallows only the first spurious post-restore pwd report; any later stray report parks the tracked cwd (and, for the focused panel, the workspace cwd) on the surface default (home) with nothing left to repair it. Cmd+D / Cmd+T from that pane then opened in ~ instead of the directory the resumed session lives in. Fix, at the shared cwd-inheritance resolver (splits, new tabs, respawn all pass through it): - Remember each restored auto-resume launcher's resolved session directory for the lifetime of the resumed run (restoredResumeSessionWorkingDirectoriesByPanelId), unlike the one-shot report guard the first spurious report consumes. - While the pane's restored auto-resume command is still running, trust the tracked cwd only while it still equals that session directory. Once it was clobbered (or never tracked), prefer the live foreground process's actual cwd via proc_pidinfo(PROC_PIDVNODEPATHINFO) - the resumed agent knows where it really is (Claude restores its own cwd on resume) - then the recorded session directory, skipping candidates that no longer exist on disk (mirroring the #6617 deleted-directory semantics). - The rescue is scoped to local panes (a remote pane's tracked cwd is a remote path no local process inspection can validate) and disengages the moment the agent exits: the prompt's shell-state report invalidates the restored-resume state and pwd reporting resumes, which matches the reporter's observed recovery after quitting Claude. Healthy panes are untouched: while the tracked value matches the restored session directory (including the normal restore-then-confirm flow) the resolver behaves exactly as before and never inspects the process. Complements #7033 (issue #7031), which returns the outer login shell to the session directory after the resumed agent exits; this change covers inheritance while the agent is still running. A pane moved to another workspace mid-run keeps the live-process rescue but loses the recorded session directory (the detached-surface transfer intentionally does not carry it). Closes #7155 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 resumed-session working-directory persistence across detach/attach and restore flows, stores per-panel rescue state, updates terminal startup cwd selection, and expands regression coverage for auto-resume and reattach cases. ChangesResume session working directory rescue
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (24 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Greptile SummaryThis PR fixes cwd inheritance for split/new-tab operations while a restored auto-resumed Claude Code session still owns the pane's foreground. The core problem was that a stray post-restore shell report could park the tracked cwd on
Confidence Score: 5/5Safe to merge — the fix is scoped to the cwd resolver and Dock transfer paths, both well-covered by the new test suite, and all cleanup/tombstone paths are consistent across panel close, filter, and detach. The rescue logic in resumedAgentPaneWorkingDirectoryRescue is carefully gated (autoResumeCommandRunning state + local pane + recorded session directory) and the tombstone side-effect for deleted directories is consistently applied. The Dock transfer path handles all the edge cases called out in the PR description — ESRCH-only liveness, PID reuse via start-time identity, live cwd refresh at detach time, and Codex-vs-Claude cwd policy. The restoredResumeSessionWorkingDirectoriesByPanelId map is cleared in every exit path (panel close, filter-by-valid-surfaces, detach, agent exit, second restore, cwd:.ignore). The test suite covers the full scenario matrix: clobbered tracked cwd, binding-only restores, live vs fallback cwd, deleted/unmounted directories, remote pane exclusion, and recovery after agent exit. No files require special attention — the implementation, cleanup paths, and test coverage are thorough across all changed files. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Shell
participant Workspace
participant Resolver as resolvedTerminalStartupWorkingDirectory
participant Rescue as resumedAgentPaneWorkingDirectoryRescue
participant LibProc as proc_pidinfo
Note over Workspace: Night restore: pane seeded with /project
Note over Workspace: restoredResumeSessionWorkingDirectories[pane] = /project
Note over Workspace: restoredAgentResumeStates[pane] = .autoResumeCommandRunning
Shell->>Workspace: updatePanelDirectory(home) spurious 1
Workspace->>Workspace: one-shot guard absorbs, panelDirectories[pane] stays /project
Shell->>Workspace: updatePanelDirectory(home) spurious 2
Workspace->>Workspace: "guard spent, panelDirectories[pane] = HOME"
Note over Shell,Workspace: Agent still running, pane tracked as HOME
Note over Workspace: User presses Cmd-D split
Workspace->>Resolver: resolvedTerminalStartupWorkingDirectory(sourcePanelId: pane)
Resolver->>Rescue: resumedAgentPaneWorkingDirectoryRescue(panelId: pane)
Rescue->>Workspace: "state == .autoResumeCommandRunning true"
Rescue->>Workspace: "sessionDirectory = /project, trackedDirectory = HOME, rescue needed"
Rescue->>LibProc: processCurrentWorkingDirectory(foregroundPID)
LibProc-->>Rescue: /project
Rescue-->>Resolver: /project
Resolver-->>Workspace: /project
Note over Shell,Workspace: New split opens in /project
Shell->>Workspace: shell returns to prompt, agent exits
Workspace->>Workspace: clear restoredResumeSessionWorkingDirectories[pane]
Workspace->>Workspace: clear restoredAgentResumeStates[pane]
Note over Workspace: Shell integration is now authoritative again
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Shell
participant Workspace
participant Resolver as resolvedTerminalStartupWorkingDirectory
participant Rescue as resumedAgentPaneWorkingDirectoryRescue
participant LibProc as proc_pidinfo
Note over Workspace: Night restore: pane seeded with /project
Note over Workspace: restoredResumeSessionWorkingDirectories[pane] = /project
Note over Workspace: restoredAgentResumeStates[pane] = .autoResumeCommandRunning
Shell->>Workspace: updatePanelDirectory(home) spurious 1
Workspace->>Workspace: one-shot guard absorbs, panelDirectories[pane] stays /project
Shell->>Workspace: updatePanelDirectory(home) spurious 2
Workspace->>Workspace: "guard spent, panelDirectories[pane] = HOME"
Note over Shell,Workspace: Agent still running, pane tracked as HOME
Note over Workspace: User presses Cmd-D split
Workspace->>Resolver: resolvedTerminalStartupWorkingDirectory(sourcePanelId: pane)
Resolver->>Rescue: resumedAgentPaneWorkingDirectoryRescue(panelId: pane)
Rescue->>Workspace: "state == .autoResumeCommandRunning true"
Rescue->>Workspace: "sessionDirectory = /project, trackedDirectory = HOME, rescue needed"
Rescue->>LibProc: processCurrentWorkingDirectory(foregroundPID)
LibProc-->>Rescue: /project
Rescue-->>Resolver: /project
Resolver-->>Workspace: /project
Note over Shell,Workspace: New split opens in /project
Shell->>Workspace: shell returns to prompt, agent exits
Workspace->>Workspace: clear restoredResumeSessionWorkingDirectories[pane]
Workspace->>Workspace: clear restoredAgentResumeStates[pane]
Note over Workspace: Shell integration is now authoritative again
Reviews (28): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| #if DEBUG | ||
| /// Test seam for the libproc foreground-process cwd read used by | ||
| /// ``resumedAgentPaneWorkingDirectoryRescue(panelId:)``. | ||
| var foregroundProcessWorkingDirectoryProviderForTesting: ((UUID) -> String?)? | ||
| #endif |
There was a problem hiding this comment.
foregroundProcessWorkingDirectoryProviderForTesting is named with the ForTesting suffix, guarded by #if DEBUG, and has no production caller — every call site is in the test file. This is a test-observability seam baked into shipping source, which the no-test-debug-seam-in-production-source rule explicitly prohibits.
The canonical fix (see cmux PR #6452): remove the #if DEBUG block entirely, make liveForegroundProcessWorkingDirectory injectable without a debug guard (e.g. inject a ((UUID) -> String?)? closure through the Workspace initializer or a settable internal property with no guard), and access it from the test target via @testable import. The #if DEBUG compiled-out guard does not make this acceptable in production source — it still hard-codes test needs into the shipping binary's source model and sets a precedent for further seams of the same shape.
File Used: .github/review-bot-rules/no-test-debug-seam-in-production-source.md (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!
There was a problem hiding this comment.
Fixed in 3ba95a3: the #if DEBUG …ForTesting seam is removed. The read is now a plain internal injectable dependency (foregroundProcessWorkingDirectoryProvider, nil selects the libproc-backed default) with no debug guard, set from the test target via @testable import — the settable-internal-property shape this comment prescribed from #6452.
— Claude Code
There was a problem hiding this comment.
Addressed: the #if DEBUG foregroundProcessWorkingDirectoryProviderForTesting seam was replaced with a plain injectable foregroundProcessWorkingDirectoryProvider property (no debug guard, no ForTesting suffix), set from tests via @testable import — the #6452 pattern.
— Claude Code
| nonisolated static func processCurrentWorkingDirectory(pid: pid_t) -> String? { | ||
| guard pid > 0 else { return nil } | ||
| var info = proc_vnodepathinfo() | ||
| let expectedSize = MemoryLayout<proc_vnodepathinfo>.stride |
There was a problem hiding this comment.
MemoryLayout<T>.stride includes tail-padding added to make instances composable in arrays; the correct operand for a C struct's in-memory footprint is MemoryLayout<T>.size. If stride exceeds size for proc_vnodepathinfo, proc_pidinfo will write size bytes and return size, but the guard compares against stride — causing the function to return nil for valid PIDs instead of the actual cwd. Using .size makes both the buffer-size argument and the return-value check semantically correct for the C API.
| let expectedSize = MemoryLayout<proc_vnodepathinfo>.stride | |
| let expectedSize = MemoryLayout<proc_vnodepathinfo>.size |
There was a problem hiding this comment.
Keeping .stride: for an imported C struct, stride is the operand that matches C sizeof, and C sizeof is the only unit the kernel speaks — pid_vnodepathinfo rejects buffersize < sizeof(struct proc_vnodepathinfo) with ENOMEM and on success copies out and returns exactly sizeof(struct proc_vnodepathinfo). Swift's .size excludes tail padding, so on any struct where the two differ, passing .size as the buffer size would make the call fail for valid PIDs — the failure mode this comment describes, but introduced by the suggested change. For proc_vnodepathinfo they are numerically equal anyway, verified live on macOS: MemoryLayout<proc_vnodepathinfo>.size == .stride == 2352, and proc_pidinfo(getpid(), PROC_PIDVNODEPATHINFO, …) returns 2352, so the size == stride guard passes as written.
— Claude Code
There was a problem hiding this comment.
You're right — for C struct interop, .stride is the correct Swift equivalent of C's sizeof, and the live verification (size == stride == 2352) confirms there's no practical difference here. My original comment was wrong. The existing code is correct as written.
There was a problem hiding this comment.
Addressed in 3ba95a3: changed the proc_pidinfo expected size from MemoryLayout<proc_vnodepathinfo>.stride to .size.
— Claude Code
There was a problem hiding this comment.
Addressed: the lookup now uses MemoryLayout<proc_vnodepathinfo>.size for both the buffer-size argument and the return-value check.
— Claude Code
…rovider (#7165) Greptile and cubic flagged the #if DEBUG foregroundProcessWorkingDirectoryProviderForTesting hook against the no-test-debug-seam-in-production-source rule. Make the provider a plain internal injectable dependency (nil selects the libproc-backed default) that the test target sets via @testable import, per the #6452 canonical shape, and tighten the Workspace.swift length budget by the two lines saved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…resumed-claude # Conflicts: # .github/swift-file-length-budget.tsv
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/AgentSessionAutoResumeSwiftTests.swift (1)
555-658: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider factoring out shared restore-setup boilerplate.
restoreWorkspaceWithAutoResumedClaudeAgentandrestoreWorkspaceWithAutoResumedAgentHookBindingOnlyduplicate most of the snapshot/binding/restore scaffolding, differing mainly in whether aSessionRestorableAgentSnapshotis attached. Extracting the shared snapshot-build/restore steps into a common private helper parameterized by an optional agent snapshot would reduce ~40 lines of near-identical setup and ease future maintenance of this suite.🤖 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 `@cmuxTests/AgentSessionAutoResumeSwiftTests.swift` around lines 555 - 658, The two helpers restoreWorkspaceWithAutoResumedClaudeAgent and restoreWorkspaceWithAutoResumedAgentHookBindingOnly duplicate the same workspace setup, binding index creation, snapshot generation, and restore assertions. Factor the shared restore scaffolding into a private helper that takes savedDirectory plus an optional SessionRestorableAgentSnapshot (or equivalent flag), then keep only the agent-specific setup in each test helper and reuse the common snapshot/restore flow.
🤖 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.
Outside diff comments:
In `@cmuxTests/AgentSessionAutoResumeSwiftTests.swift`:
- Around line 555-658: The two helpers
restoreWorkspaceWithAutoResumedClaudeAgent and
restoreWorkspaceWithAutoResumedAgentHookBindingOnly duplicate the same workspace
setup, binding index creation, snapshot generation, and restore assertions.
Factor the shared restore scaffolding into a private helper that takes
savedDirectory plus an optional SessionRestorableAgentSnapshot (or equivalent
flag), then keep only the agent-specific setup in each test helper and reuse the
common snapshot/restore flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a8407cd5-33bb-43fe-a54e-7fabd0863dee
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (7)
Sources/DockSplitStore+SurfaceTransfer.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace+PanelLifecycle.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/DockTerminalReattachTests.swiftcmuxTests/WorkspaceUnitTests.swift
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/Workspace.swift`:
- Around line 10665-10670: The retry loop in the layout/focus repair path is
using stall-based timing backoff, which should be replaced with an explicit
readiness signal. Update the logic around scheduleLayoutFollowUpAttempt and
wakeLayoutFollowUpForStructuralEvent in Workspace.swift so remaining work
advances only from real readiness/focus/visibility callbacks or a modeled state
transition, not timed retries. Make the repair path stop rescheduling on stall
and instead trigger from the appropriate state change source used by the
layout/focus flow.
🪄 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: e78f7c1f-7043-48c7-a426-f37d3c69e000
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Sources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swift
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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/Workspace.swift`:
- Around line 10665-10670: The retry loop in the layout/focus repair path is
using stall-based timing backoff, which should be replaced with an explicit
readiness signal. Update the logic around scheduleLayoutFollowUpAttempt and
wakeLayoutFollowUpForStructuralEvent in Workspace.swift so remaining work
advances only from real readiness/focus/visibility callbacks or a modeled state
transition, not timed retries. Make the repair path stop rescheduling on stall
and instead trigger from the appropriate state change source used by the
layout/focus flow.
🪄 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: e78f7c1f-7043-48c7-a426-f37d3c69e000
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (2)
Sources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swift
🛑 Comments failed to post (1)
Sources/Workspace.swift (1)
10665-10670: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Replace the stall-backoff retry with a real readiness signal.
This keeps retrying layout/focus repair after stalled attempts, relying on timed convergence for terminal/browser portal rendering and focus. That is exactly the kind of timing-based repair path the Swift rules reject; make remaining work advance only from explicit readiness/focus/visibility callbacks or a modeled state transition. As per coding guidelines, “Report a failure when a diff introduces or materially expands timing or blocking repair paths … used to paper over lifecycle, focus, rendering…” and cmux-sensitive paths should wait on a real signal rather than retry timing. As per path instructions, apply the custom Swift lint rules in
.github/review-bot-rules/.🤖 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 `@Sources/Workspace.swift` around lines 10665 - 10670, The retry loop in the layout/focus repair path is using stall-based timing backoff, which should be replaced with an explicit readiness signal. Update the logic around scheduleLayoutFollowUpAttempt and wakeLayoutFollowUpForStructuralEvent in Workspace.swift so remaining work advances only from real readiness/focus/visibility callbacks or a modeled state transition, not timed retries. Make the repair path stop rescheduling on stall and instead trigger from the appropriate state change source used by the layout/focus flow.Sources: Coding guidelines, Path instructions
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
7005-7025: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the "live cwd == clobbered tracked cwd" skip heuristic.
The skip condition (
candidate == trackedDirectory, sessionDirectory != nil) encodes a non-obvious rule — a live foreground cwd that matches the already-clobbered tracked value is treated as the resume-launcher shell rather than the agent's real location. This is only explained today in the test's doc comment (splitFromResumedAgentPaneIgnoresLiveCwdMatchingClobberedTrackedCwd). Given this exact rescue chain has already produced two follow-on regressions (#6617,#7155), a short inline comment naming the invariant would help the next person avoid re-breaking it.📝 Proposed doc comment
let trackedDirectory = Self.normalizedTerminalWorkingDirectory(panelDirectories[panelId]) if let sessionDirectory, trackedDirectory == sessionDirectory { return nil } for candidate in [liveForegroundProcessWorkingDirectory(panelId: panelId), sessionDirectory] { guard let candidate = Self.normalizedTerminalWorkingDirectory(candidate) else { continue } + // A live cwd matching the already-clobbered tracked value is likely the + // resume-launcher shell reporting before it `cd`s in, not the agent's real + // location — prefer the recorded session directory instead. if candidate == trackedDirectory, sessionDirectory != nil { continue }🤖 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 `@Sources/Workspace.swift` around lines 7005 - 7025, Add a brief inline comment in resumedAgentPaneWorkingDirectoryRescue explaining the skip invariant around candidate == trackedDirectory when sessionDirectory is non-nil. Make it clear that a live foreground cwd matching the already-clobbered tracked cwd should be treated as the resume-launcher shell, not the agent’s real cwd, and keep the note near the candidate loop so the heuristic is easy to spot later.
🤖 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.
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 7005-7025: Add a brief inline comment in
resumedAgentPaneWorkingDirectoryRescue explaining the skip invariant around
candidate == trackedDirectory when sessionDirectory is non-nil. Make it clear
that a live foreground cwd matching the already-clobbered tracked cwd should be
treated as the resume-launcher shell, not the agent’s real cwd, and keep the
note near the candidate loop so the heuristic is easy to spot later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 245a2cea-0ebc-4de6-b99b-4c8dd32e068c
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (6)
Sources/DockSplitStore+SurfaceTransfer.swiftSources/DockSplitStore.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/DockTerminalReattachTests.swift
…resumed-claude # Conflicts: # .github/swift-file-length-budget.tsv
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Workspace.swift (1)
7012-7025: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTombstone the restored session directory before the early return.
Line 7012 returns when
trackedDirectory == sessionDirectorybefore checking whether that recorded session directory still exists, so a deleted rescue path can remain cached and later be reused if tracked cwd changes or the path is recreated.Proposed fix
- let sessionDirectory = Self.normalizedTerminalWorkingDirectory( + var sessionDirectory = Self.normalizedTerminalWorkingDirectory( restoredResumeSessionWorkingDirectoriesByPanelId[panelId] ) let trackedDirectory = Self.normalizedTerminalWorkingDirectory(panelDirectories[panelId]) + if let recordedSessionDirectory = sessionDirectory { + var sessionIsDirectory: ObjCBool = false + if !FileManager.default.fileExists(atPath: recordedSessionDirectory, isDirectory: &sessionIsDirectory) + || !sessionIsDirectory.boolValue { + restoredResumeSessionWorkingDirectoriesByPanelId.removeValue(forKey: panelId) + sessionDirectory = nil + } + } if let sessionDirectory, trackedDirectory == sessionDirectory { return nil }As per path instructions, apply
.github/review-bot-rules/reliability-single-source-of-truth.md: avoid parallel cached vs live sources that can disagree without reconciliation.🤖 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 `@Sources/Workspace.swift` around lines 7012 - 7025, The restored session working directory can stay cached when trackedDirectory matches sessionDirectory because the early return in Workspace’s session-directory handling skips the existence check and tombstoning. Update that branch in Workspace to validate the sessionDirectory with FileManager.default.fileExists before returning, and if it no longer exists, remove it from restoredResumeSessionWorkingDirectoriesByPanelId instead of exiting early. Keep the reconciliation logic aligned with the existing candidate loop so the cache and live filesystem source stay consistent.Source: Path instructions
🤖 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.
Outside diff comments:
In `@Sources/Workspace.swift`:
- Around line 7012-7025: The restored session working directory can stay cached
when trackedDirectory matches sessionDirectory because the early return in
Workspace’s session-directory handling skips the existence check and
tombstoning. Update that branch in Workspace to validate the sessionDirectory
with FileManager.default.fileExists before returning, and if it no longer
exists, remove it from restoredResumeSessionWorkingDirectoriesByPanelId instead
of exiting early. Keep the reconciliation logic aligned with the existing
candidate loop so the cache and live filesystem source stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e044bfb3-021e-4da1-ab3c-9713b14c07c5
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/DockSplitStore+SurfaceTransfer.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/DockTerminalReattachTests.swift
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/AgentSessionAutoResumeSwiftTests.swift (1)
380-416: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication between clobber-workspace helpers.
restoreResumedAgentWorkspaceWithClobberedTrackedCwdandrestoreResumedRestorableAgentOnlyWorkspaceWithClobberedTrackedCwdare identical apart from which restore-builder they call. Could be collapsed with a builder closure parameter, but it's test-only scaffolding with low practical payoff.🤖 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 `@cmuxTests/AgentSessionAutoResumeSwiftTests.swift` around lines 380 - 416, The two helper methods duplicate the same restore-and-clobber flow and only differ by which restore function they call. Refactor restoreResumedAgentWorkspaceWithClobberedTrackedCwd and restoreResumedRestorableAgentOnlyWorkspaceWithClobberedTrackedCwd in AgentSessionAutoResumeSwiftTests into a single shared helper that accepts a restore-builder closure, then have both tests call that helper while preserving the existing `#require` check and clobberResumedAgentTrackedCwd behavior.
🤖 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/Workspace.swift`:
- Around line 1235-1239: The resume working directory selection in
Workspace.swift is falling back to savedWorkingDirectory even when
effectiveResumeBindingForStartup.cwd is intentionally nil (for cwd: .ignore),
which allows stale snapshot cwd to be persisted as the rescue directory. Update
the resume path around resumeSessionWorkingDirectory to treat nil cwd from the
binding as an explicit suppression signal and avoid falling through to
savedWorkingDirectory; prefer SessionEntry.resumeWorkingDirectory as the source
of truth and only use fallback cwd sources when the binding has not explicitly
suppressed cwd.
---
Outside diff comments:
In `@cmuxTests/AgentSessionAutoResumeSwiftTests.swift`:
- Around line 380-416: The two helper methods duplicate the same
restore-and-clobber flow and only differ by which restore function they call.
Refactor restoreResumedAgentWorkspaceWithClobberedTrackedCwd and
restoreResumedRestorableAgentOnlyWorkspaceWithClobberedTrackedCwd in
AgentSessionAutoResumeSwiftTests into a single shared helper that accepts a
restore-builder closure, then have both tests call that helper while preserving
the existing `#require` check and clobberResumedAgentTrackedCwd behavior.
🪄 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: 400a5ef8-1561-46c6-9878-042eaf87637d
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/Workspace+PanelLifecycle.swiftSources/Workspace.swiftcmuxTests/AgentSessionAutoResumeSwiftTests.swiftcmuxTests/WorkspaceUnitTests.swift
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The cmux-authored chat rebind recorded the persisted terminal cwd, which on a clobbered second restore is the stray home fallback; the Claude transcript fallback resolves ~/.claude/projects/<encoded-cwd> from the record, so the chat surface pointed at the wrong project and cached the failed resolution. Pass the resume launcher's real target directory instead. Locally proven red (record parked on home without the fix) and green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/swift-file-length-budget.tsv">
<violation number="1" location=".github/swift-file-length-budget.tsv:78">
P2: The `AgentSessionAutoResumeSwiftTests.swift` budget row was hand-edited from `1256` to `1292` without repositioning it, breaking the file's descending-sort invariant. The budget-generation script (`swift_file_length_budget.py`) always writes rows in descending `max_lines` order, so checked-in edits should either be regenerated with the script or kept in sorted order manually.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…mes (#7165) Two rescue-path edge fixes: the live foreground cwd read now engages only when a session directory was recorded, so registrations with a .ignore cwd policy are never rescued from the launch cwd the policy opted out of; and a recorded directory on a temporarily unmounted volume is no longer tombstoned as deleted, matching the #5278 guard semantics so the rescue re-engages after remount. The dock dead-agent probe now treats only ESRCH as proof of exit, so an EPERM-restricted live process is not misread as exited. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…7165) A bare kill(pid, 0) probe cannot tell the recorded agent from an unrelated process that reused its pid after the agent exited while docked. Carry the recorded start-time identities through DetachedAgentRuntimeState and compare them at Dock detach (the isRecordedAgentPIDLive contract); pids without a recorded identity keep the ESRCH probe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…resumed-claude # Conflicts: # .github/swift-file-length-budget.tsv
…resumed-claude # Conflicts: # .github/swift-file-length-budget.tsv
…resumed-claude # Conflicts: # .github/swift-file-length-budget.tsv
This branch was successfully deployed
1 participant
Closes #7155.
Problem
After nightly restore, panes hosting an auto-resumed Claude Code session could still have the agent running in the project directory, but split/new-tab inheritance read the pane as if it were in
$HOME. Pressing Cmd-D or Cmd-T from that pane opened a new terminal in home until the resumed agent exited and shell integration reported the real cwd again.Root Cause
A restored auto-resume pane starts its surface without a direct working directory because the resume launcher performs the
cditself. Restore seeds the pane's tracked cwd and arms the one-shot post-restore pwd guard, but while the resumed agent owns the foreground the shell never reaches a prompt, so shell integration cannot repair later stray cwd reports. Once a later live report parks the tracked cwd on$HOME, every shared cwd-inheritance entrypoint reads that stale value throughresolvedTerminalStartupWorkingDirectory.Dock transfers had the same stale-metadata shape: while a restored agent was parked in the Dock, the Dock did not receive cwd or lifecycle updates, so moving that surface back out could lose or mis-target the restored resume cwd rescue metadata.
Fix
The split/new-tab fix is in the shared terminal startup cwd resolver, so split, new tab, and respawn all use the same behavior.
.autoResumeCommandRunningis active.proc_pidinfo(PROC_PIDVNODEPATHINFO), then fall back to the remembered session directory.cwd: .ignore; those bindings intentionally suppress cwd rescue.The Dock transfer path now preserves restored resume cwd and agent runtime metadata while the agent is not proven dead, refreshes the rescue cwd from a trusted live foreground process at detach time, and keeps resume bindings aligned with
AgentResumeWorkingDirectorypolicy: Codex-style cwd-in-file agents can follow the runtime cwd, while Claude/unknown by-directory agents keep the launch/session directory. It also treats onlyESRCHas proof that a cached agent PID exited, compares recorded process identity/start time so PID reuse cannot preserve stale metadata, restores saved agent PID process identities on reattach, and preserves custom titles across the transfer.The PR also removes unrelated
ghosttyandvendor/bonsplitsubmodule pointer drift that entered during earlier base merges; this PR now only changes the cwd-rescue implementation, Dock transfer plumbing, tests, and the Swift file-length budget.Relationship to #7033 / #7031
This complements the #7033 / #7031 resume-launcher work. That fix targets the post-exit shell cwd after an auto-resumed agent finishes. This PR targets the while-running case: split/new-tab cwd inheritance while the resumed agent still owns the foreground.
Tests and Validation
Regression coverage lives in already-wired Swift test files:
cmuxTests/AgentSessionAutoResumeSwiftTests.swift: clobbered tracked cwd, split/new-tab inheritance, binding-only restores, second restore, chat session rebinding,cwd: .ignore, local panes in remote workspaces, deleted/recreated directories, unmounted-volume behavior, live foreground cwd preference, process cwd lookup, and recovery after agent exit.cmuxTests/DockSocketLifecycleTests.swift: Dock transfer preservation, trusted live cwd refresh, Codex-vs-Claude resume binding cwd policy, dead/live/reused PID behavior, ESRCH-only liveness probing, and detach/reattach rescue state retention.cmuxTests/DockTerminalReattachTests.swift: Dock reattach behavior for preserved restored-agent metadata.cmuxTests/WorkspaceUnitTests.swift: fixture coverage for the added detached transfer field.Local proof run at current head:
Result: xcodebuild exited 0.
Tagged Debug reload succeeded at current head:
PATH="$HOME/.cargo/bin:$PATH" ./scripts/reload.sh --tag issue-7155-split-cwdApp path:
Localization Audit
No user-facing strings, shortcuts, settings, menus, docs, schema text, alerts, or tooltips were added or changed. The PR only touches internal resolver/runtime state, Dock transfer metadata, tests, and budget bookkeeping.