fix: prefer Ghostty inherited cwd for terminal creation - #7614
austinywang wants to merge 31 commits into
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:
📝 WalkthroughWalkthroughTerminal startup cwd resolution now considers an inherited cwd candidate before existing rescue and default candidates. Split and surface terminal creation now thread source-panel context into that selection and only use inherited fallback cwd on the enabled path. ChangesTerminal startup cwd inheritance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant newTerminalSplitLocal
participant newTerminalSurfaceLocal
participant resolvedTerminalStartupWorkingDirectory
participant inheritedWorkingDirectoryForTerminalStartup
participant resumedAgentPaneWorkingDirectoryRescue
newTerminalSplitLocal->>resolvedTerminalStartupWorkingDirectory: inheritedWorkingDirectory + sourcePanelId
newTerminalSurfaceLocal->>resolvedTerminalStartupWorkingDirectory: fallback sourcePanelId + inheritedWorkingDirectory
resolvedTerminalStartupWorkingDirectory->>inheritedWorkingDirectoryForTerminalStartup: evaluate inherited cwd
resolvedTerminalStartupWorkingDirectory->>resumedAgentPaneWorkingDirectoryRescue: evaluate rescued cwd
inheritedWorkingDirectoryForTerminalStartup-->>resolvedTerminalStartupWorkingDirectory: inherited cwd candidate
resumedAgentPaneWorkingDirectoryRescue-->>resolvedTerminalStartupWorkingDirectory: rescued cwd candidate
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 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 updates terminal startup directory handling and related delivery paths. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (38): Last reviewed commit: "fix: drop stale queued terminal render g..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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)
Sources/Workspace.swift (1)
7664-7689: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep startup cwd behind one resolver path.
Line 7664 disables inherited cwd fallback when
startupCommand != nil, but Lines 7668-7689 still pass an inherited config template whoseworkingDirectorycan be used downstream whenworkingDirectoryis nil. That bypasses the new resolver gate and can still start command-backed terminals in an inherited cwd.Either clear
inheritedConfig.workingDirectoryfor the non-fallback path, or route that cwd throughresolvedTerminalStartupWorkingDirectory(...)so the startup directory has one authoritative decision point. As per path instructions, startup cwd must have a single trusted source of truth.🤖 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 7664 - 7689, The startup cwd decision is still leaking through inherited config when startupCommand is set, bypassing the intended resolver gate. Update the logic around shouldInheritWorkingDirectoryFallback, inheritedTerminalConfig(inPane:), and resolvedTerminalStartupWorkingDirectory(...) so command-backed terminals have one authoritative cwd path: either strip inheritedConfig.workingDirectory when startupCommand != nil, or ensure the workingDirectory passed into TerminalPanel is always derived only from the resolver for that case.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 7664-7689: The startup cwd decision is still leaking through
inherited config when startupCommand is set, bypassing the intended resolver
gate. Update the logic around shouldInheritWorkingDirectoryFallback,
inheritedTerminalConfig(inPane:), and
resolvedTerminalStartupWorkingDirectory(...) so command-backed terminals have
one authoritative cwd path: either strip inheritedConfig.workingDirectory when
startupCommand != nil, or ensure the workingDirectory passed into TerminalPanel
is always derived only from the resolver for that case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: abf261af-a138-47ba-8791-6d710ca90581
📒 Files selected for processing (1)
Sources/Workspace.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?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)
Sources/Workspace.swift (1)
7356-7402: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStrip inherited config cwd so the resolver is the only cwd source.
inheritedTerminalConfig(...)can carryCmuxSurfaceConfigTemplate.workingDirectory, while these paths also passworkingDirectory:fromresolvedTerminalStartupWorkingDirectory(...). If the explicit cwd or fallback source differs from the config-inheritance candidate, terminal startup has competing cwd sources.Proposed fix
- var inheritedConfig = inheritedTerminalConfig(preferredPanelId: panelId, inPane: paneId) + var inheritedConfig = inheritedTerminalConfig(preferredPanelId: panelId, inPane: paneId) + inheritedConfig?.workingDirectory = nil- var inheritedConfig = inheritedTerminalConfig(inPane: paneId) + var inheritedConfig = inheritedTerminalConfig(inPane: paneId) + inheritedConfig?.workingDirectory = nilAs per path instructions, correctness-critical working-directory state should have one authoritative source and avoid competing sources that can disagree.
Also applies to: 7665-7686
🤖 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 7356 - 7402, The split-terminal setup in Workspace’s terminal creation path is passing two competing cwd sources: inheritedTerminalConfig(preferredPanelId:inPane:) may already carry CmuxSurfaceConfigTemplate.workingDirectory while resolvedTerminalStartupWorkingDirectory(...) is also provided to TerminalPanel. Strip the workingDirectory from the inherited config before building the new panel so the resolver output is the single authoritative cwd source, and apply the same fix in both affected split-terminal code paths.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 7356-7402: The split-terminal setup in Workspace’s terminal
creation path is passing two competing cwd sources:
inheritedTerminalConfig(preferredPanelId:inPane:) may already carry
CmuxSurfaceConfigTemplate.workingDirectory while
resolvedTerminalStartupWorkingDirectory(...) is also provided to TerminalPanel.
Strip the workingDirectory from the inherited config before building the new
panel so the resolver output is the single authoritative cwd source, and apply
the same fix in both affected split-terminal code paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 21bb4c39-1256-486a-9031-76ffc6acad8e
📒 Files selected for processing (1)
Sources/Workspace.swift
a9385b3 to
6685722
Compare
6685722 to
13b383a
Compare
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)
7657-7668: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the inherited-cwd derivation into a named helper.
The chained
&&+ ternary computingfallbackLiveWorkingDirectoryandinheritedWorkingDirectoryis dense for a correctness-critical cwd path, mixing two candidate sources (config cwd vs live-foreground cwd) and a source-panel match test. A small helper (e.g.inheritedStartupWorkingDirectory(fallbackSourcePanelId:inheritedConfigSourcePanelId:inheritedConfig:enabled:)) returning the resolved value would make the source-of-truth precedence explicit and easier to unit-test.🤖 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 7657 - 7668, The inherited working-directory selection in Workspace should be extracted into a named helper to make the precedence between config cwd and live foreground cwd explicit. Move the logic around fallbackLiveWorkingDirectory and inheritedWorkingDirectory into a helper such as inheritedStartupWorkingDirectory(fallbackSourcePanelId:inheritedConfigSourcePanelId:inheritedConfig:enabled:) and have it return the resolved cwd based on the source-panel match test and availability checks. Keep the existing behavior intact, but simplify the call site so the resolution path is easier to read and unit-test.
🤖 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 7657-7668: The inherited working-directory selection in Workspace
should be extracted into a named helper to make the precedence between config
cwd and live foreground cwd explicit. Move the logic around
fallbackLiveWorkingDirectory and inheritedWorkingDirectory into a helper such as
inheritedStartupWorkingDirectory(fallbackSourcePanelId:inheritedConfigSourcePanelId:inheritedConfig:enabled:)
and have it return the resolved cwd based on the source-panel match test and
availability checks. Keep the existing behavior intact, but simplify the call
site so the resolution path is easier to read and unit-test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9c1c6340-71a1-497d-9413-f1bb52f263f5
📒 Files selected for processing (1)
Sources/Workspace.swift
13b383a to
1169872
Compare
1169872 to
b051114
Compare
b051114 to
7cb498d
Compare
7cb498d to
d791713
Compare
dc7f10d to
e41e1a1
Compare
9bb24b7 to
bf7c1c5
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit bf7c1c5. Configure here.
| ? controlResumeBinding(from: ws.surfaceResumeBinding(panelId: panel.id)) | ||
| : nil | ||
| ) | ||
| } |
There was a problem hiding this comment.
Tmux mirror surfaces omitted
High Severity
controlSurfaceList now builds summaries with a plain map over orderedPanels, so remote tmux mirror wrapper panels are no longer expanded into their inner synthetic pane surfaces. The replaced controlSurfaceSummaries helper still uses flatMap and remoteTmuxWindowMirror projection, so list output can disagree with pane list and break mirror automation.
Reviewed by Cursor Bugbot for commit bf7c1c5. Configure here.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Replacement for external PR #7292, which could not be updated because GitHub reports push=false for the current token on jleechanorg/cmux despite maintainerCanModify=true.\n\nSummary:\n- merge current origin/main into the PR work\n- share terminal startup cwd resolution across split and inherited new-tab paths\n- prefer Ghostty's live inherited cwd for local live terminal sources\n- preserve explicit cwd, restored auto-resume rescue, guarded restore cwd, and remote terminal cwd behavior\n\nVerification:\n- python3 scripts/swift_file_length_budget.py\n- git diff --check origin/main...HEAD\n- /Users/austinwang/manaflow/cmuxterm-hq/skills/review/autoreview/scripts/autoreview --mode branch --base origin/main\n\nNot run: local xcodebuild/app launch/XCUITests, per PR iteration constraints.
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Note
High Risk
Removing transcript snapshots and post-teardown restore monitoring materially increases risk of agent session data loss on hibernation, and CI/SwiftPM tolerance changes could let real failures slip through if diagnostics are misclassified.
Overview
This diff is mostly infrastructure and lifecycle refactors, not the Ghostty cwd work described in the PR title (only a small remote-directory guard in command-click file open appears here).
Agent hibernation is greatly simplified: the whole Claude transcript snapshot / restore-monitor stack (
AgentHibernationTranscriptGuardand related teardown types) is deleted. After the idle confirmation window, the controller now SIGTERM-scoped processes and callsenterAgentHibernationdirectly, with planner logic inlined and a process-based fingerprint fallback when scrollback is unavailable. Eligibility no longer excludes panes that only have a live process.macOS hosted browser sign-in drops default-browser handoff (
openExternalURL,handedOffAttemptID). A non-auth WebAuth completion stops the sign-in UI but keeps the attempt alive for a latecmux://callback; handoff-specific tests are removed and replaced.iOS mobile shell skips queued render-grid frames older than the delivered high-water mark and ties replay-barrier ack to bypass deliveries, with new unit tests.
CI / release: App Store provisioning defaults and workflow copy move to
com.cmuxterm.app; primary profile secret is strict; SwiftPM tests go throughrun_swift_package_testswith scoped cosmetic-failure tolerance; iOSxcodebuildmay pass when tests succeeded before simulator diagnostics time out; the App Store lane identity check is removed. Swift file-length budgets are bumped for several large sources.Settings window presentation is consolidated (no deminiaturize settle loop / verification unhide); navigation delivery uses the shared presenter again from
SettingsWindowHostRoot.Reviewed by Cursor Bugbot for commit 9bb24b7. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Prefer Ghostty’s inherited cwd for terminal splits and eligible new tabs, and tighten cwd fallback so remote paths never influence local startup or UI. Also discard stale mobile terminal render frames and carry replay‑barrier acks to keep streams unblocked.
Bug Fixes
workingDirectorystill wins.terminalRequestedWorkingDirectoryForLocalFallbackso remote‑sourced or trust‑guarded paths never feed local startup fallback, window title, file open, mobile payloads, tmux topology, or layout capture; avoid using a remotecurrentDirectoryas a local startup cwd.CI
run_swift_package_testswith scoped tolerance for GhosttyKit andCMUXAuthCorerunner diagnostics when the all‑tests‑passed summary is present.xcodebuildtimeouts as pass only when tests finished before simulator diagnostics timed out, using a pre‑timeout log slice and/or the latest.xcresult; App Store/APNs paths usecom.cmuxterm.app.Written for commit bf7c1c5. Summary will update on new commits.
Summary by CodeRabbit