Repository navigation
Fix Cmd+T cwd after session restore - #6055
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:
📝 WalkthroughWalkthroughCentralizes terminal startup working-directory resolution, adds fallback inheritance to terminal surface creation, updates terminal creation call sites to use it, and adds tests plus Xcode wiring for session-restore cwd behavior. ChangesTerminal Working-Directory Resolution and Validation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 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 SummaryFixes Cmd+T/new-tab cwd resolution after session restore by centralizing working-directory selection in
Confidence Score: 5/5Safe to merge — the fix is well-scoped, the fallback is opt-in, and the remote-terminal nil-cwd contract is preserved. The change centralizes an existing four-step cwd precedence rule that was already correct for split/respawn paths, and applies it consistently to all new-surface paths via an explicit opt-in flag. The gate on startupCommand == nil correctly excludes remote and command-driven terminals. Agent panels fall through cleanly to workspace currentDirectory because they produce no panelDirectories entry and terminalPanel(for:) returns nil for non-terminal panels. The regression is covered by a new Swift Testing suite. No files require special attention. Important Files Changed
Reviews (8): Last reviewed commit: "test: restore terminal controller manage..." | Re-trigger Greptile |
6ceaf2f to
71c86dd
Compare
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 14745-14751: Update the inline comment above the call to
resolvedTerminalStartupWorkingDirectory so it matches the actual resolver order:
state that an explicit requestedWorkingDirectory (the requestedWorkingDirectory
parameter) is preferred first, then the source panel's reported cwd (identified
by panelId) if present, and finally fall back to the workspace's current
directory; keep the call to resolvedTerminalStartupWorkingDirectory 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: 86720799-0e19-45e2-96bb-8b858dd1366d
📒 Files selected for processing (1)
Sources/Workspace.swift
008b745 to
97b9824
Compare
97b9824 to
2a6b378
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/WorkspaceTerminalTabWorkingDirectoryTests.swift`:
- Around line 189-208: The test uses a hard-reset pattern for
TerminalController.shared state management that can cause state leakage in
parallel test execution. In the
surfaceCreateInheritsWorkspaceCurrentDirectoryForAgentPane function, replace the
defer block that sets activeTabManager to nil at the start with a save/restore
pattern: first call activeTabManagerForCallerNotification() to capture the
previous manager state, then after the setActiveTabManager call with the manager
parameter, add a defer block that restores the previous manager by calling
setActiveTabManager with the saved previousManager value. This ensures tests do
not clobber each other's state and follows the established pattern used
elsewhere in the test suite.
🪄 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: 9058f8d0-3830-42aa-ab45-e4ddaebc44bf
📒 Files selected for processing (5)
Sources/TerminalController+ControlSurfaceContext2.swiftSources/TerminalController.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/WorkspaceTerminalTabWorkingDirectoryTests.swift
👮 Files not reviewed due to content moderation or server errors (1)
- Sources/TerminalController.swift
PRs included: - AppDelegate decomposition: CmuxSession session-snapshot repository (manaflow-ai#6030) - Fix Cmd+T cwd after session restore (manaflow-ai#6055) - Speed up iOS terminal scroll rendering (manaflow-ai#6035) - Preserve Pi sessions across workspace restore (manaflow-ai#5607) - Scope Biome checks to maintained JS sources (manaflow-ai#6008) - Fix manaflow-ai#5917: restore OSC 11 pane-local backgrounds (manaflow-ai#5997) - Expose stable window title templates (manaflow-ai#6059) - Honor macos-option-as-alt left/right - Fix macOS 27 SF Symbol rasterization crash (manaflow-ai#5999) - CmuxRemote* family: extract Workspace remote/cloud-VM connectivity - Fix iOS workspace swipe-delete confirmation crash (manaflow-ai#6051) - TabManager decomposition Wave 3+4 sub-models - Sidebar row cleanups: branchless frame anchor - CmuxIPCService: extract AppDelegate multi-window CLI routing - CmuxSidebarGit: extract TabManager git-metadata + PR-polling subsystem - CmuxTerminalCore: extract terminal core leaf Fork-side adjustments: - ghostty submodule: cherry-pick mouse-modifier-state fix onto our renderer-realized branch - Workspace.swift: take theirs (upstream extracted ~7700 lines into CmuxCore.Remote/CmuxRemoteSession packages); restore fork's renameTopLevelLayoutTabContaining/closeTopLevelLayoutTabContaining + surfaceTmuxClientTTYNames + WorkspaceLayoutTab integration - TabManager.swift: take theirs; re-add static allocatePortOrdinal() - BrowserPanelView, RenderableSystemSymbol: keep fork's cmuxSymbolPixelSize extension on top of upstream's cmuxSymbolRasterSize - Add CmuxWorkspaces / CMUXSessionDaemon / CmuxCommandPalette imports to TerminalController, Workspace, SessionPersistence - Sources/Workspace+P43Stubs.swift: thin shims for SplitEqualizer, WorkspaceRemoteSessionController.PortScanKickReason, WorkspaceGroupNewWorkspacePlacementSettings (legacy types fork TC still calls; replace with package APIs in P44+) - Sources/GhosttySurfaceSizeDeferralReason.swift: restore fork-only enum (deleted by upstream) - Sources/StableLayout/SessionBlueprintExportAction.swift: parked debug action (depends on legacy SessionPersistenceStore, gone) - Sources/GhosttyTerminalView.swift: stub ghostty_surface_select_cursor_line_compat (needs zig 0.15.2 xcframework rebuild) - pbxproj: keep-both, drop stale ProcessPipeReader/SplitEqualizer/Panels/BrowserProxyEndpoint refs, fix SurfaceHibernationPolicy UUID collision - Drop fork's WorkspaceRemoteConfiguration.swift + WorkspaceRemoteSSHBatchCommandBuilder.swift (extracted to CmuxCore package)
Summary
Fixes #6047.
Regression proof
test: cover restored agent Cmd+T cwdfix: restore Cmd+T workspace cwd fallbackLocal validation
git diff --check./scripts/check-pbxproj.sh./scripts/lint-pbxproj-test-wiring.shPer issue instructions, I did not run local xcodebuild tests or
./scripts/reload.sh.Localization
No user-facing strings changed; no localization catalog updates required.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes Cmd+T/new-tab cwd after session restore by centralizing cwd resolution and applying it across all terminal-creation paths, including JSON-RPC
surface.create. If no pane cwd is available, we use the workspacecurrentDirectory; “New Terminal to Right” inherits the anchor tab’s cwd even if it isn’t selected.Workspace(order: explicit → panel cwd → panel requested cwd → workspacecurrentDirectory), ignoring empty/whitespace values. Enabled viainheritWorkingDirectoryFallback(only when no startup command) and optionalworkingDirectoryFallbackSourcePanelIdacross new/split/respawn, empty-panel, focused-pane, shell-driven, sidebar new-tab, and v2surface.createflows (incl. “to the right”).WorkspaceTerminalTabWorkingDirectoryTestscovering Cmd+T restore uses workspace cwd; remote restore keeps nil; “to the right” inherits anchor; andsurface.createinherits workspace cwd from a focused agent pane. Fixes Cmd+T loses workspace cwd after Cmd+Q session restore #6047.Written for commit 5f2b6fb. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Tests