Repository navigation
Fix Cmd+T opening in home after agent-resume session restore (#6617) - #6621
Conversation
|
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 ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds guarded working directory logic to session restore and two regression tests. Workspace.swift now registers guarded directories more broadly during terminal restore (whenever the startup command handles ChangesSession Restore Guarded Directory and Regression Tests
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 (1 error, 1 warning)
✅ Passed checks (21 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 a regression in Cmd+T cwd-inheritance after agent-resume session restore (#6617). When a terminal whose startup command cds itself (agent resume, tmux attach) was restored, its shell reported home via OSC-7 before the cd ran, clobbering
Confidence Score: 5/5Safe to merge — the change is a targeted extension of an existing guard mechanism with no new state representations and two test cases that directly exercise the fixed paths. The guard arming and one-shot suppress logic are well-bounded: the guard is indexed by panel ID, cleared after first use on mounted volumes, and the FileManager.fileExists check is a single stat that fires only once per restore. The three branches (match / unmounted / mounted) cover all distinct cases, and the deleted-directory companion test confirms the important edge case. No new test/debug seams are introduced to production source, and all #if DEBUG blocks gate legitimate diagnostic logs. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["OSC-7 liveReport received\nupdatePanelDirectory(.liveReport)"] --> B{Guard entry exists\nfor panelId?}
B -- No --> C[Update panelDirectories\nand currentDirectory]
B -- Yes --> D{reportedDir ==\nrestoredDir?}
D -- Yes --> E[Clear guard\nAllow update]
D -- No --> F{Restored dir on\nunmounted volume?}
F -- Yes --> G[Persistent ignore\nguard stays]
F -- No --> H[Remove guard\none-shot check]
H --> I{FileManager.fileExists\nrestoredDir still exists?}
I -- Yes --> J[Ignore spurious\nhome report\nproject cwd preserved]
I -- No --> K[Accept home report\ndeleted-dir fallback honored]
%%{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"}}}%%
flowchart TD
A["OSC-7 liveReport received\nupdatePanelDirectory(.liveReport)"] --> B{Guard entry exists\nfor panelId?}
B -- No --> C[Update panelDirectories\nand currentDirectory]
B -- Yes --> D{reportedDir ==\nrestoredDir?}
D -- Yes --> E[Clear guard\nAllow update]
D -- No --> F{Restored dir on\nunmounted volume?}
F -- Yes --> G[Persistent ignore\nguard stays]
F -- No --> H[Remove guard\none-shot check]
H --> I{FileManager.fileExists\nrestoredDir still exists?}
I -- Yes --> J[Ignore spurious\nhome report\nproject cwd preserved]
I -- No --> K[Accept home report\ndeleted-dir fallback honored]
Reviews (3): Last reviewed commit: "fix: keep restored workspace cwd against..." | Re-trigger Greptile |
| @Test func cmdTAfterAgentResumeRestoreKeepsProjectCwdDespiteSpuriousHomePwdReport() throws { | ||
| try withRestoredDefaults(key: AgentSessionAutoResumeSettings.autoResumeAgentSessionsKey) { | ||
| UserDefaults.standard.set(true, forKey: AgentSessionAutoResumeSettings.autoResumeAgentSessionsKey) | ||
|
|
||
| let projectDir = "/tmp/cmux-cmdt-resume-project-\(UUID().uuidString)" | ||
| let sessionId = "claude-cmdt-resume-session" | ||
| let source = Workspace() | ||
| source.currentDirectory = projectDir | ||
| let sourcePanelId = try #require(source.focusedPanelId) | ||
|
|
||
| let agent = SessionRestorableAgentSnapshot( | ||
| kind: .claude, | ||
| sessionId: sessionId, | ||
| workingDirectory: projectDir, | ||
| launchCommand: AgentLaunchCommandSnapshot( | ||
| launcher: "claude", | ||
| executablePath: "/usr/local/bin/claude", | ||
| arguments: ["/usr/local/bin/claude", "--resume", sessionId], | ||
| workingDirectory: projectDir, | ||
| environment: [:], | ||
| capturedAt: 1_777_777_777, | ||
| source: "process" | ||
| ) | ||
| ) | ||
| source.updatePanelShellActivityState(panelId: sourcePanelId, state: .commandRunning) | ||
| source.setRestoredAgentSnapshotForTesting(agent, panelId: sourcePanelId) | ||
|
|
||
| let bindingIndex = SurfaceResumeBindingIndex(bindingsByPanel: [ | ||
| SurfaceResumeBindingIndex.PanelKey(workspaceId: source.id, panelId: sourcePanelId): SurfaceResumeBindingSnapshot( | ||
| name: "Claude", | ||
| kind: "claude", | ||
| command: "{ cd -- '\(projectDir)' 2>/dev/null || [ ! -d '\(projectDir)' ]; } && 'claude' '--resume' '\(sessionId)'", | ||
| cwd: projectDir, | ||
| checkpointId: sessionId, | ||
| source: "agent-hook", | ||
| autoResume: true, | ||
| updatedAt: 1_777_777_777 | ||
| ), | ||
| ]) | ||
|
|
||
| let snapshot = source.sessionSnapshot( | ||
| includeScrollback: false, | ||
| surfaceResumeBindingIndex: bindingIndex | ||
| ) | ||
| #expect(snapshot.currentDirectory == projectDir) | ||
|
|
||
| let restored = Workspace() | ||
| restored.restoreSessionSnapshot(snapshot) | ||
| let restoredPanelId = try #require(restored.focusedPanelId) | ||
|
|
||
| // Restore replays the persisted project directory onto the workspace | ||
| // and the resumed panel. | ||
| #expect(restored.currentDirectory == projectDir) | ||
| #expect(restored.panelDirectories[restoredPanelId] == projectDir) | ||
|
|
||
| // The resumed shell starts before its agent-resume command cds, so | ||
| // shell integration reports home first. This spurious live report must | ||
| // be ignored so the restored project cwd survives. | ||
| let spuriousHomeReport = FileManager.default.homeDirectoryForCurrentUser.path | ||
| try #require(spuriousHomeReport != projectDir) | ||
| restored.updatePanelDirectory(panelId: restoredPanelId, directory: spuriousHomeReport) | ||
|
|
||
| #expect(restored.currentDirectory == projectDir) | ||
| #expect(restored.panelDirectories[restoredPanelId] == projectDir) | ||
|
|
||
| // Cmd+T must open the new tab in the project directory, not home. | ||
| let createdPanel = try #require(restored.newTerminalSurfaceInFocusedPane(focus: false)) | ||
| #expect(createdPanel.requestedWorkingDirectory == projectDir) | ||
| } | ||
| } |
There was a problem hiding this comment.
Test will fail — production fix is absent
The new test asserts that updatePanelDirectory ignores a spurious home report after session restore, but the guard that enforces this was never committed. In Sources/Workspace.swift (line 1380-1388), restoredGuardedWorkingDirectoriesByPanelId is only armed when Self.unmountedVolumeRoot(for: guardedWorkingDirectory) != nil — i.e., the saved path must be on an unmounted volume. For a live, accessible /tmp/… project path the condition is false, the guard is never set, and shouldIgnoreRestoredGuardedDirectoryReport at line 4507-4511 falls into the guard missingVolumeRoot != nil else { …; return false } branch and lets the home write through. Both #expect(restored.currentDirectory == projectDir) assertions will fail. The PR description's "two-commit structure (red → green)" names a fix: commit that does not appear in the branch — git log da3d1a03..794b0b4c shows only the test: commit.
There was a problem hiding this comment.
This thread reviewed the test-only commit (I pushed the failing test first, then the fix). The fix is present in the current head: Workspace.swift now arms the restored-cwd guard for every local startup-handles-cwd restore (not just unmounted volumes), so shouldIgnoreRestoredGuardedDirectoryReport ignores the spurious post-resume home report. Both regression tests pass on CI (shard 2), and your latest review of the head commit reports 'Safe to merge'.
— Claude Code
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
After Cmd+Q/restore of a workspace whose focused terminal runs an auto-resumed agent, the resumed shell spawns in its default directory and shell integration reports it (typically home) before the agent-resume command cds into the project. While the project directory still exists that spurious live pwd report must not overwrite the restored workspace cwd, otherwise Cmd+T opens the next tab in home (~) instead of the project directory. The main test asserts the spurious home report is ignored (saved dir still exists) so Cmd+T inherits the project dir; it fails until the restore path guards the restored working directory. A companion test asserts the home report IS honored when the saved dir was deleted between sessions, so the guard cannot strand the cwd on a missing path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
794b0b4 to
fa6727c
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 1390-1393: The guard registration condition at the
startupHandlesWorkingDirectory check is too broad and includes remote restore
paths where localWorkingDirectory is nil, but the subsequent existence check
only uses FileManager.default.fileExists which operates on the local filesystem.
This causes remote saved paths like /home/... that don't exist locally to be
incorrectly accepted. Separate the logic by carrying context about whether the
terminal is local or remote throughout the guard and restoration flow, then
apply the local FileManager existence check only for local terminals while
implementing a remote-aware existence signal or policy for remote terminals that
does not treat local absence as a deleted-remote-cwd.
- Around line 4523-4528: The DEBUG log statements in the cmuxDebugLog calls are
logging sensitive directory paths that can expose usernames, customer names, or
project details. Replace the logging of the full restoredDirectory and
reportedDirectory path values with non-sensitive metadata alternatives such as
byte counts, branch outcome, or other safe indicators instead. Apply this change
to both debug log occurrences at lines 4523-4528 and 4546-4550, while keeping
panelId in the logs as it is safe to expose. Ensure the log message remains
clear and useful for debugging without revealing sensitive file system
information.
🪄 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: c10033b1-271a-4446-8b6f-740675132d8f
⛔ 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
…ort (#6617) After session restore, a terminal whose startup command cds itself (agent resume, tmux attach, agent-hook) is spawned without a working directory so it tolerates a since-deleted saved directory. Its shell therefore starts in the default directory and shell integration reports it (typically home) before the startup command cds into the saved one. That spurious live pwd report overwrote the restored panelDirectories/currentDirectory, so Cmd+T's cwd-inheritance fallback resolved to home — the new tab opened in ~ instead of the project directory the agent is in. This regressed the #6047 fix, whose Cmd+T workspace-cwd fallback is correct but was fed a clobbered value. The restore path already had a guard (restoredGuardedWorkingDirectoriesByPanelId) to ignore such spurious reports, but it only armed when the saved directory was on an unmounted volume. Arm it for every LOCAL startup-handles-cwd restore with a saved directory, and have shouldIgnoreRestoredGuardedDirectoryReport: - ignore the first mismatched report ONCE while the saved directory still exists (the startup command will cd into it), - accept the report when the saved directory was deleted between sessions (the shell's reported cwd is then the real fallback — dropping it would strand the cwd on a missing path and make Cmd+T inherit an invalid dir), - keep the existing persistent-ignore for unmounted volumes (#5278). The guard is scoped to local terminals: its existence check stats the local Mac, so remote restores (whose saved cwd is a remote path) keep the prior behavior, matching the original unmounted-volume guard which was local-only. A report matching the restored directory still clears the guard immediately. Closes #6617 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
fa6727c to
2084b4f
Compare
Integrate #6625 (revert sidebar row-height layout feedback), #6617/#6621, and #6624. ContentView conflict: #6625 removed the render-context/row `sidebarWidth` threading while this branch added `pinResolutionContext` — took #6625's sidebarWidth removal and kept pinResolutionContext. Font fix (`magnifiedFont`/@Environment), lazy tooltips, and #6625's rowHeight probe all preserved. swift-file-length-budget.tsv regenerated via --write-budget (only ContentView's entry changes, +40 lines). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Fixes #6617 — after Cmd+Q and session restore, Cmd+T opened the new tab in home (
~) instead of the directory the resumed terminal/agent is in. This regressed the #6047 fix (whose Cmd+T workspace-cwd fallback is correct, but was being fed a clobbered value).Root cause
The snapshot does persist the workspace cwd, and restore does replay it onto
currentDirectory/panelDirectories. The directory was being lost after restore:localWorkingDirectory == nilbranch).report_pwd(OSC-7) for that directory — typically home — before the resume command cds into the saved one.report_pwd → updateSurfaceDirectory → Workspace.updatePanelDirectory(.liveReport)and overwrote the restoredpanelDirectories[focused]and the focused workspacecurrentDirectory.panelDirectories[source]→ … →currentDirectory) then resolved to home.A fresh (non-restored) terminal doesn't hit this: it's spawned in the project directory, so its first report already is the project directory.
The restore path already had a guard (
restoredGuardedWorkingDirectoriesByPanelId) meant to ignore exactly these spurious post-resume reports — but it only armed when the saved directory was on an unmounted volume, so a normal (existing) project directory was left unprotected.Fix
shouldIgnoreRestoredGuardedDirectoryReportnow:This is surgical: only a report that differs from the restored directory is suppressed, and only the first one for accessible directories.
savedWorkingDirectoryis the panel's last-known directory, so legitimate same-directory reports (e.g. tmux panes) match and clear the guard immediately.Tests
Two-commit structure (red → green):
test:addscmdTAfterAgentResumeRestoreKeepsProjectCwdDespiteSpuriousHomePwdReportinAgentSessionAutoResumeSwiftTests(a.serializedsession-restore suite). It saves a workspace whose focused terminal runs an auto-resumed Claude agent in a project dir, restores it, simulates the resumed shell's spurious homereport_pwd, and assertscurrentDirectorysurvives and a new Cmd+T surface inherits the project directory. Fails without the fix (home clobbers the cwd).fix:the guard change above.Localization
No user-facing strings changed (guard logic + a
#if DEBUGdiagnostic only); noLocalizable.xcstringsupdates required.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes #6617: after Cmd+Q and session restore, Cmd+T now opens in the project directory instead of home when resuming terminals whose startup command cds itself (agent resume, tmux attach, agent-hook). The restore path now guards the restored cwd for local terminals, ignoring the first spurious home PWD report and accepting it if the saved directory was deleted.
.github/swift-file-length-budget.tsv.Written for commit 2084b4f. Summary will update on new commits.
Summary by CodeRabbit
Release Notes
Tests
Bug Fixes