Track remote cwd from ssh/mosh prompt OSC signals - #3812
Closed
austinywang wants to merge 45 commits into
Closed
austinywang wants to merge 45 commits into
austinywang wants to merge 45 commits into
Conversation
Plain SSH or mosh prompt escapes need to carry a remote host, path, and branch through terminal state before sidebar rendering can be correct. These regression tests encode the expected OSC 7 signal and the reusable remote shell snippet behavior before adding the implementation. Constraint: Keep the bug-fix history test-first for issue 3791 Rejected: Sidebar-only string assertions | they would not prove terminal escape handling or shell prompt emission Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: Local test execution is prohibited by workspace policy
|
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 OSC 7-based remote cwd and Git-branch reporting: TerminalLocation model and parsers, RemoteShellIntegrationSnippet + ChangesRemote Terminal Location Detection
Sequence Diagram(s)sequenceDiagram
participant CLI as cmux CLI
participant Shell as Remote shell (bash/zsh)
participant TM as TabManager
participant WS as Workspace
CLI->>Shell: cmux remote-shell-init (print snippet)
Shell->>Shell: source snippet -> install prompt hook
Shell->>TM: emit OSC7 file://... with cmux_git_branch/cmux_git_dirty
TM->>WS: updatePanelLocation(panelId, TerminalLocation)
TM->>TM: gate git/PR probes (skip if remote)
WS->>WS: persist terminalLocation in session snapshot
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (12 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR adds first-class per-surface
Confidence Score: 5/5The change is safe to merge; OSC7 remote panes are excluded from git probing, Finder resolution, listening-port snapshots, and session restore at every entry point. TerminalLocation is a clean nonisolated value type with thorough test coverage. TabManager remote-aware guards are consistent across all probe paths. Shell snippet escaping and hostname caching are correct. Session persistence is forward-compatible. The only open item is a narrow fallback edge in sidebarBranchDirectoryEntriesInDisplayOrder that does not affect the primary code paths. Sources/Workspace.swift — sidebarBranchDirectoryEntriesInDisplayOrder defaultDirectory fallback warrants a second look; all other files are clean. Important Files Changed
Reviews (44): 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}
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
2 times, most recently
from
May 10, 2026 02:57
ad933a7 to
0efb191
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@docs/ghostty-fork.md`:
- Around line 179-185: Update the docs/ghostty-fork.md note to include a clear
merge timeline and action item: mention that the temporary branch
issue-3791-mosh-remote-cwd-detection (commit 6b8d558ce) will be merged into the
fork main once the fork main PR is approved, specify the expected date or
milestone for that merge, and add an instruction to regenerate and checksum-pin
the xcframework-6b8d558ce... release after the merge so the submodule can move
from the temporary branch to a stable main pointer.
In `@Sources/RemoteRelayZshBootstrap.swift`:
- Around line 60-67: The OSC 7 emitter __cmux_remote_report_prompt currently
inserts raw output from __cmux_remote_hostname and PWD (via
__cmux_remote_path_escape) which can contain control/non-printable bytes; update
__cmux_remote_report_prompt to sanitize both host and path by running them
through a control-character stripping helper (e.g. __cmux_strip_control_chars or
similar) before building the query and calling printf, and add/ensure such a
helper function exists and is used for both the hostname (output of
__cmux_remote_hostname) and the path (result of __cmux_remote_path_escape
"${PWD:-/}") so the emitted OSC 7 file:// URI cannot contain control characters
or enable terminal-sequence injection.
- Around line 75-79: Bash PROMPT_COMMAND may be an array in Bash 5.1+, so the
current string-based case that manipulates PROMPT_COMMAND can clobber array-mode
hooks; update the block handling PROMPT_COMMAND to first detect array-mode (use
declare -p PROMPT_COMMAND 2>/dev/null and test for "declare -a" or [[ "$(declare
-p PROMPT_COMMAND 2>/dev/null)" =~ "declare -a" ]]), and if it is an array,
check for the presence of "__cmux_remote_report_prompt" in
"${PROMPT_COMMAND[@]}" and, if missing, prepend or append it via
PROMPT_COMMAND=( "__cmux_remote_report_prompt" "${PROMPT_COMMAND[@]}" ) or
PROMPT_COMMAND+=( "__cmux_remote_report_prompt" ); otherwise keep the existing
string-mode case that checks PROMPT_COMMAND and sets
PROMPT_COMMAND="__cmux_remote_report_prompt${PROMPT_COMMAND:+;$PROMPT_COMMAND}".
Ensure you reference PROMPT_COMMAND and __cmux_remote_report_prompt in the
updated logic.
In `@Sources/TabManager.swift`:
- Around line 3940-3955: The switch handling location.gitBranchSignal doesn't
always clear existing panel pull-request state when the path is remote; update
the .branch and .unspecified cases (and the similar block around the other
occurrence at lines referenced) to call tab.clearPanelPullRequest(panelId:
surfaceId) whenever location.isRemote (in addition to existing
tab.clearPanelGitBranch / clearWorkspacePullRequestTracking calls), so remote
classification always clears stale panel pull-request state; locate uses of
location.gitBranchSignal, tab.updatePanelGitBranch, tab.clearPanelGitBranch,
tab.clearPanelPullRequest, and clearWorkspacePullRequestTracking to make the
change.
In `@Sources/Workspace.swift`:
- Around line 6689-6847: Move the entire TerminalLocation struct and the private
String extension (including symbols TerminalLocation, Source, GitBranchSignal,
parseOSC7Sequence(_:), parseReportedDirectory(_:source:),
gitBranchSignal(from:isRemote:), normalizedHost(_:), isLocalHost(_:),
localHostnames(), and prefixBeforeFirstTerminalTerminator()) into a new
dedicated Swift file as a standalone model; remove the original definition from
Workspace.swift so Workspace only imports/uses TerminalLocation and
SidebarGitBranchState, ensure the new file has the same access levels (or mark
types/internal methods public if needed by other modules), add any required
imports at the top, and run the build to update references where Workspace used
these functions so callers reference the moved symbols.
🪄 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: cbeff522-6d47-4e76-8eb7-8db9fc25c723
📒 Files selected for processing (13)
CLI/cmux.swiftREADME.mdResources/Localizable.xcstringsSources/RemoteRelayZshBootstrap.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/Workspace.swiftcmuxTests/SidebarOrderingTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftdocs/cli-contract.mddocs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txt
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
from
May 10, 2026 03:01
0efb191 to
4b60077
Compare
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
from
May 10, 2026 03:14
4b60077 to
d18da68
Compare
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
from
May 10, 2026 03:26
d18da68 to
5156d7a
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/TabManager.swift (1)
3934-3974:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTrigger an immediate refresh when the OSC 7 branch hint changes in-place.
Line 3963 only schedules refreshes on directory changes. After this refactor, local branch changes can arrive through
location.gitBranchSignalwith the same cwd, sogit checkoutin place can keep showing the old PR until the next 10s/60s poll.💡 Suggested fix
func updateSurfaceLocation(tabId: UUID, surfaceId: UUID, location: TerminalLocation) { guard let tab = tabs.first(where: { $0.id == tabId }) else { return } let shouldProbeLocalGit = !location.isRemote && !tab.isRemoteWorkspace let shouldClearLocalPullRequestState = !shouldProbeLocalGit let previousDirectory = shouldProbeLocalGit ? gitProbeDirectory(for: tab, panelId: surfaceId) : nil let probeKey = WorkspaceGitProbeKey(workspaceId: tabId, panelId: surfaceId) + let previousBranch = tab.panelGitBranches[surfaceId] tab.updatePanelLocation(panelId: surfaceId, location: location) + + var explicitLocalBranchChange = false if shouldClearLocalPullRequestState { tab.clearPanelPullRequest(panelId: surfaceId) clearWorkspacePullRequestTracking(for: probeKey) } switch location.gitBranchSignal { case .branch(let branch): let normalizedBranch = Self.normalizedBranchName(branch.branch) ?? branch.branch + explicitLocalBranchChange = + shouldProbeLocalGit && + (previousBranch?.branch != normalizedBranch || previousBranch?.isDirty != branch.isDirty) tab.updatePanelGitBranch(panelId: surfaceId, branch: normalizedBranch, isDirty: branch.isDirty) if !shouldClearLocalPullRequestState { clearWorkspacePullRequestTracking(for: probeKey) } case .clear where location.isRemote: tab.clearPanelGitBranch(panelId: surfaceId) case .unspecified where location.isRemote: tab.clearPanelGitBranch(panelId: surfaceId) case .clear, .unspecified: break } let nextDirectory = shouldProbeLocalGit ? normalizedWorkingDirectory(location.path) : nil - if shouldProbeLocalGit, previousDirectory != nextDirectory { + if shouldProbeLocalGit, previousDirectory != nextDirectory || explicitLocalBranchChange { + let reason = explicitLocalBranchChange && previousDirectory == nextDirectory + ? "branchChange" + : "directoryChange" scheduleWorkspacePullRequestRefresh( workspaceId: tabId, panelId: surfaceId, - reason: "directoryChange" + reason: reason ) scheduleWorkspaceGitMetadataRefreshIfPossible( workspaceId: tabId, panelId: surfaceId, - reason: "directoryChange" + reason: reason ) } }🤖 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/TabManager.swift` around lines 3934 - 3974, updateSurfaceLocation currently only schedules pull-request and git-metadata refreshes when the working directory changes; you need to also trigger an immediate refresh when the OSC7 gitBranchSignal changes in-place. Before calling tab.updatePanelGitBranch/clearPanelGitBranch, capture the panel's previous branch state (e.g. the panel's git branch name/isDirty from the tab or via gitProbeDirectory lookup), then after handling location.gitBranchSignal compare previous vs new branch state (treat normalized names and clear/unspecified as branch removals). If the branch changed and shouldProbeLocalGit is true (even if previousDirectory == nextDirectory), call scheduleWorkspacePullRequestRefresh(workspaceId: tabId, panelId: surfaceId, reason: "branchChange") and scheduleWorkspaceGitMetadataRefreshIfPossible(workspaceId: tabId, panelId: surfaceId, reason: "branchChange"). Ensure you reference updateSurfaceLocation, location.gitBranchSignal, tab.updatePanelGitBranch, tab.clearPanelGitBranch, previousDirectory/nextDirectory and the two schedule... methods when making the change.Sources/Workspace.swift (1)
8541-8557:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep remote cwd out of the generic local-directory fields.
updatePanelLocationnow writeslocation.pathintopanelDirectories,surfaceTabBarDirectory, andcurrentDirectoryfor both local and remote locations. Those fields are still reused later in this file as local spawn/base cwd inputs, so splitting a plainssh/moshpane can try to open a new local terminal in a remote-only path. Please keep remote locations separate here, or only copy into these fields when the location is local.🤖 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 8541 - 8557, updatePanelLocation currently copies location.path into panelDirectories, surfaceTabBarDirectory, and currentDirectory regardless of whether the TerminalLocation is local or remote; change it so panelTerminalLocations[panelId] is always updated but only assign trimmed into panelDirectories, surfaceTabBarDirectory, and currentDirectory when the location represents a local filesystem (i.e. check TerminalLocation's transport/type or an isLocal/isRemote flag on location before writing local fields). Keep using the trimmed path variable and preserve the existing focused-panel checks, but skip the assignments to panelDirectories, surfaceTabBarDirectory and currentDirectory when the location is remote.
🤖 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/WorkspaceRemoteConnectionTests.swift`:
- Around line 89-100: The test calls runProcess with executablePath
"/usr/bin/env" and passes environment variables via the arguments array, but it
omits the "-i" flag so host shell/git env can leak; update the arguments for the
runProcess invocation(s) (the one that builds the arguments list containing
"CMUX_REMOTE_HOST=remotehost", "GIT_AUTHOR_NAME=cmux", etc., and the similar
call around lines 158-166) to include "-i" immediately after "/usr/bin/env"
(i.e., arguments should start with "-i", then the explicit VAR=VALUE entries,
followed by shellPath and "-c" and script) so the subprocess runs with an
isolated environment.
In `@Sources/RemoteRelayZshBootstrap.swift`:
- Around line 77-79: The Zsh branch currently appends
__cmux_remote_report_prompt to the precmd_functions array unconditionally,
causing duplicate registrations when resourcing; update the else branch that
manipulates precmd_functions to check whether __cmux_remote_report_prompt is
already present before appending (mirror the deduplication logic used in the
Bash path for the PROMPT_COMMAND/array case), so only add
__cmux_remote_report_prompt to precmd_functions if it's not already in the
array.
In `@Sources/TerminalLocation.swift`:
- Around line 124-137: The isLocalHost(_:) function currently only treats
"127.0.0.1" as loopback; update it to treat the whole 127.0.0.0/8 range as local
by adding a check that returns true when the normalized host starts with "127."
(i.e., first octet is 127) before falling back to the localhost/name checks and
the localHostnames() candidates; keep the existing "::1" and "localhost" checks
and candidate set logic intact so short/full hostnames still work.
In `@Sources/Workspace.swift`:
- Around line 8871-8874: Sidebar now uses panelTerminalLocations but
move/restore/detach/attach paths still only preserve panelDirectories, so remote
panes lose TerminalLocation; update every place that assigns or preserves
panelDirectories (e.g., the move/restore/detach/attach handlers and any
functions that call normalizedSidebarDirectory) to also carry and assign the
corresponding TerminalLocation into panelTerminalLocations[panelId] (using the
same TerminalLocation instance whose displayDirectory yields the
normalizedSidebarDirectory), and when restoring use
panelTerminalLocations[panelId] if present instead of only panelDirectories so
the host:/path form is retained.
---
Outside diff comments:
In `@Sources/TabManager.swift`:
- Around line 3934-3974: updateSurfaceLocation currently only schedules
pull-request and git-metadata refreshes when the working directory changes; you
need to also trigger an immediate refresh when the OSC7 gitBranchSignal changes
in-place. Before calling tab.updatePanelGitBranch/clearPanelGitBranch, capture
the panel's previous branch state (e.g. the panel's git branch name/isDirty from
the tab or via gitProbeDirectory lookup), then after handling
location.gitBranchSignal compare previous vs new branch state (treat normalized
names and clear/unspecified as branch removals). If the branch changed and
shouldProbeLocalGit is true (even if previousDirectory == nextDirectory), call
scheduleWorkspacePullRequestRefresh(workspaceId: tabId, panelId: surfaceId,
reason: "branchChange") and
scheduleWorkspaceGitMetadataRefreshIfPossible(workspaceId: tabId, panelId:
surfaceId, reason: "branchChange"). Ensure you reference updateSurfaceLocation,
location.gitBranchSignal, tab.updatePanelGitBranch, tab.clearPanelGitBranch,
previousDirectory/nextDirectory and the two schedule... methods when making the
change.
In `@Sources/Workspace.swift`:
- Around line 8541-8557: updatePanelLocation currently copies location.path into
panelDirectories, surfaceTabBarDirectory, and currentDirectory regardless of
whether the TerminalLocation is local or remote; change it so
panelTerminalLocations[panelId] is always updated but only assign trimmed into
panelDirectories, surfaceTabBarDirectory, and currentDirectory when the location
represents a local filesystem (i.e. check TerminalLocation's transport/type or
an isLocal/isRemote flag on location before writing local fields). Keep using
the trimmed path variable and preserve the existing focused-panel checks, but
skip the assignments to panelDirectories, surfaceTabBarDirectory and
currentDirectory when the location is remote.
🪄 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: 157018c2-7c21-4acc-bbaf-9349b836356b
📒 Files selected for processing (15)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojREADME.mdResources/Localizable.xcstringsSources/RemoteRelayZshBootstrap.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/TerminalLocation.swiftSources/Workspace.swiftcmuxTests/SidebarOrderingTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftdocs/cli-contract.mddocs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txt
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
from
May 10, 2026 03:42
5156d7a to
b41f32a
Compare
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 `@Sources/RemoteRelayZshBootstrap.swift`:
- Around line 52-63: The git-status probe in __cmux_remote_git_branch_query runs
on every prompt and should be controllable; add an opt-out guard that checks a
new env var (e.g. CMUX_REMOTE_DISABLE_GIT=1 or CMUX_REMOTE_GIT_DIRTY=0) at the
top of __cmux_remote_git_branch_query and, when set, skip running `git status
--porcelain --untracked-files=normal` (still determine branch via git
symbolic-ref/rev-parse if desired) and emit cmux_git_dirty=0 (or omit the dirty
field) so the function remains backward-compatible while avoiding expensive
per-prompt status checks on slow remote filesystems.
- Around line 65-71: The host inserted into the file:// URI in
__cmux_remote_report_prompt is only stripped of control chars and can contain
characters that break the URI; update the composition to URI-escape the host by
passing the stripped host through __cmux_remote_uri_escape (same helper used for
branch names) before inserting into the printf that builds the
'\\033]7;file://%s%s%s\\033\\\\' string so the authority is always well-formed;
locate the host assignment and the printf in __cmux_remote_report_prompt and
replace the direct host use with the escaped-host variable.
In `@Sources/TabManager.swift`:
- Around line 3943-3945: When clearing local PR bookkeeping (the branch with
shouldClearLocalPullRequestState that calls tab.clearPanelPullRequest(panelId:
surfaceId) and clearWorkspacePullRequestTracking(for: probeKey)), also cancel or
mark as invalid any in-flight workspacePullRequestRefreshTask for that
WorkspaceGitProbeKey and update the surface classification state (e.g. set a
"remote" flag or clear cached classification) so that
applyWorkspacePullRequestRefreshResults(...) will re-check current
classification before writing; additionally, inside
applyWorkspacePullRequestRefreshResults ensure it reads the latest surface
classification (or the cancelled/invalidated flag for the probe key) and returns
without applying results if the surface is now classified as remote or the
refresh task was cancelled/invalidated.
- Around line 4005-4012: Move the remote-surface guard so it runs before calling
tab.updatePanelGitBranch(...) to avoid mutating local branch state for remote
surfaces; specifically, check tab.isRemoteWorkspace and
tab.terminalLocation(for: surfaceId)?.isRemote and perform
tab.clearPanelPullRequest(panelId: surfaceId) and
clearWorkspacePullRequestTracking(for: WorkspaceGitProbeKey(workspaceId: tabId,
panelId: surfaceId)) then return, only after that call
tab.updatePanelGitBranch(panelId: surfaceId, branch: normalizedBranch, isDirty:
isDirty). Ensure the guard uses the same expressions (tab.isRemoteWorkspace and
tab.terminalLocation(for: surfaceId)?.isRemote) and keep
WorkspaceGitProbeKey/clearWorkspacePullRequestTracking logic unchanged.
In `@Sources/TerminalLocation.swift`:
- Around line 84-86: The code currently trims whitespace from the decoded path
(variables path and trimmedPath) which can remove significant trailing spaces
from valid cwd values; update the logic in TerminalLocation (the path
decoding/validation around components.percentEncodedPath, path, and trimmedPath)
to stop trimming characters and instead validate emptiness using the decoded
path exactly (e.g., treat an empty string as nil but preserve any whitespace
characters), and apply the same change to the analogous check at the other
occurrence (the block referenced by lines 165-166) so session/sidebar restore
uses the exact decoded path.
- Around line 88-95: The persisted TerminalLocation is keeping local machine
names in host (e.g., "localhost" or this-Mac), causing local panes to be treated
as remote; update the normalization so that Self.normalizedHost(...) or the
logic that assigns `host` returns nil for local hosts by using
Self.isLocalHost(...) (i.e., set `host` to nil when isLocalHost is true) before
creating TerminalLocation (the current code that computes `host`, `remote`, and
`branchSignal` should assign `host = host.flatMap { Self.isLocalHost($0) ? nil :
$0 }` or equivalent), and apply the same change where TerminalLocation is
constructed later (the block around lines 168–176) so local file:// hosts are
not persisted as non-nil host values.
In `@Sources/Workspace.swift`:
- Around line 8711-8713: The code is incorrectly clearing panelTerminalLocations
during context resets which loses remote host classification used by
terminalLocation(for:) and sidebar fallback; in resetSidebarContext(reason:)
stop clearing panelTerminalLocations (remove or comment out the
panelTerminalLocations.removeAll() call) so that panelDirectories can continue
to map to host:/path entries and remote-aware git/PR suppression remains
intact—only clear panelGitBranches and pullRequest as before (or add a guarded
clear that only empties panelTerminalLocations when the actual host identity
changes).
🪄 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: f5db88c1-5fa1-4248-a05e-ff6dc11d4907
📒 Files selected for processing (19)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojREADME.mdResources/Localizable.xcstringsSources/RemoteRelayZshBootstrap.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/TerminalLocation.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/SidebarOrderingTests.swiftcmuxTests/TabManagerUnitTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftdocs/cli-contract.mddocs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txt
austinywang
force-pushed
the
issue-3791-mosh-remote-cwd-detection
branch
from
May 10, 2026 03:54
b41f32a to
9dcfc56
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
Sources/TerminalLocation.swift (1)
67-76:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPreserve significant spaces in plain-path cwd updates.
This still trims valid leading/trailing spaces from non-URI paths. A cwd like
/tmp/repowill be stored as/tmp/repo, which can misgroup panes and restore the wrong directory.Suggested fix
- let trimmed = directory.trimmingCharacters(in: .whitespacesAndNewlines) + let trimmed = directory.trimmingCharacters(in: .newlines) guard !trimmed.isEmpty else { return nil }🤖 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/TerminalLocation.swift` around lines 67 - 76, The code currently trims both whitespace and newlines into the variable trimmed which removes significant leading/trailing spaces for plain paths; instead only strip newline characters for non-URI paths so a path like "/tmp/repo " is preserved. Modify the logic around trimmed and the guard that decides between parseOSC7Sequence(trimmed) / local(path: trimmed, source: source) so that you use a newline-only trim (e.g. trimmingCharacters(in: .newlines)) when determining/returning a plain local(path:), while still detecting OSC7/URI prefixes and using full whitespace trimming only for those branches; update references to trimmed, parseOSC7Sequence(trimmed), and local(path: trimmed, source: source) accordingly.
🤖 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/SidebarOrderingTests.swift`:
- Around line 580-617: Add parallel test cases for the alternate URI scheme
"kitty-shell-cwd://" in TerminalOSC7LocationTests to prevent regressions: create
tests that call TerminalLocation.parseReportedDirectory and
TerminalLocation.parseOSC7Sequence (or a new helper if needed) with
"kitty-shell-cwd://..." inputs mirroring the existing file:// cases (remote with
git params, loopback IPv4 treated as local, percent-decoded path preserving
whitespace, and localhost normalizing sessionSnapshot.host to nil) and assert
the same properties (isRemote, remoteHost, path, displayDirectory, and gitBranch
via SidebarGitBranchState) so both URI forms are covered.
In `@cmuxTests/TabManagerUnitTests.swift`:
- Around line 706-718: The test is flaky because the commandRunnerForTesting
closure only checks executable == "git" and misses resolved paths like
"/usr/bin/git"; change the guard to compare the executable's lastPathComponent
(e.g. URL(fileURLWithPath: executable).lastPathComponent == "git") so any git
path will hit the probe, and add an explicit precondition/assertion that the
workspace has no active probes (e.g.
XCTAssertTrue(workspace.activeProbes.isEmpty)) before calling
workspace.updatePanelDirectory(panelId:directory:) to avoid race conditions;
keep using the releaseProbe semaphore in the closure and signal it at the
intended point after the OSC7 update completes.
In `@ghostty`:
- Line 1: The repo currently pins the unmerged commit
6b8d558cea313b09c10ac107254d6ee41a5044d9 from branch
issue-3791-mosh-remote-cwd-detection while origin/main is at 41ab6c5ab; fix this
by either (A) preparing and opening a merge PR to bring
issue-3791-mosh-remote-cwd-detection into Ghostty main (create a branch, run CI,
request reviewers, and after merge update any dependency pins to the resulting
main commit or tag), or (B) if you must continue pinning the feature branch, add
a documented mitigation in the repo (e.g., DEPENDENCIES.md or README) that
records the commit hash, the reason for pinning, a review/retention policy, a
plan for rebasing/snapshots, and an automated alert to revisit the pin when the
branch is merged or changes; choose one of these two paths and implement the
corresponding changes and metadata.
In `@Sources/RemoteRelayZshBootstrap.swift`:
- Around line 33-41: The path-escaping function __cmux_remote_path_escape is not
percent-encoding backslashes, so a remote cwd like /tmp/a\b can produce an
invalid file:// payload; update __cmux_remote_path_escape to replace backslash
characters with %5C (and add the same replacement in the other analogous escape
sites referenced around lines 75-77) so backslashes are percent-encoded in
emitted OSC 7 paths, ensuring the replacement uses the shell string-replace
operation consistent with the other encodings in that function.
In `@Sources/Workspace.swift`:
- Around line 8545-8560: updatePanelLocation currently copies location.path into
panelDirectories, surfaceTabBarDirectory, and currentDirectory regardless of
whether the TerminalLocation is remote (OSC 7). Change it so
panelTerminalLocations[panelId] = location remains the single source of truth
for all locations, but only mirror location.path into the legacy local fields
(panelDirectories, surfaceTabBarDirectory, currentDirectory) when the location
represents a local directory; i.e., add a guard that checks the
TerminalLocation's locality flag (e.g., location.isLocal or !location.isRemote /
whatever TerminalLocation exposes to detect OSC 7 remote paths) before assigning
to panelDirectories[panelId], surfaceTabBarDirectory, and currentDirectory,
leaving remote OSC 7 paths only in panelTerminalLocations.
---
Duplicate comments:
In `@Sources/TerminalLocation.swift`:
- Around line 67-76: The code currently trims both whitespace and newlines into
the variable trimmed which removes significant leading/trailing spaces for plain
paths; instead only strip newline characters for non-URI paths so a path like
"/tmp/repo " is preserved. Modify the logic around trimmed and the guard that
decides between parseOSC7Sequence(trimmed) / local(path: trimmed, source:
source) so that you use a newline-only trim (e.g. trimmingCharacters(in:
.newlines)) when determining/returning a plain local(path:), while still
detecting OSC7/URI prefixes and using full whitespace trimming only for those
branches; update references to trimmed, parseOSC7Sequence(trimmed), and
local(path: trimmed, source: source) accordingly.
🪄 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: da302cab-5f47-4175-ab15-44e39a31e004
📒 Files selected for processing (19)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojREADME.mdResources/Localizable.xcstringsSources/RemoteRelayZshBootstrap.swiftSources/SessionPersistence.swiftSources/TabManager.swiftSources/TerminalController.swiftSources/TerminalLocation.swiftSources/Workspace+DetachedSurfaceTransfer.swiftSources/Workspace.swiftcmuxTests/SessionPersistenceTests.swiftcmuxTests/SidebarOrderingTests.swiftcmuxTests/TabManagerUnitTests.swiftcmuxTests/WorkspaceRemoteConnectionTests.swiftdocs/cli-contract.mddocs/ghostty-fork.mdghosttyscripts/ghosttykit-checksums.txt
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 065d8cf. Configure here.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cmux tracks local pane cwd/branch primarily through its shell prompt integration: zsh/bash hooks call the cmux socket (
report_pwd,report_git_branch,clear_git_branch) and Ghostty also has an OSC 7 cwd action path for terminal-emitted cwd. A plainssh/moshlaunched inside a normal pane crosses into a shell that has neither cmux's local socket integration nor the dedicatedcmux sshrelay bootstrap, and Ghostty was also rejecting non-local OSC 7 hosts before the app could classify them. The smallest robust fix is to treat terminal cwd as first-class per-surface location state, let the embedded app receive non-local OSC 7 as a URI, and shipcmux remote-shell-initso remote bash/zsh prompts can emit OSC 7 with host/path plus cmux git-branch query hints over the terminal stream.Closes #3791.
Fix
TerminalLocationstate per surface with local/remote host+path parsing from OSC 7 (file://host/path) and branch query hints.TabManager.updateSurfaceDirectorythrough remote-aware location updates and skipped local git/PR probes for remote locations.host:/pathwhile preserving raw paths for local filesystem behavior.cmux remote-shell-init, printing a bash/zsh snippet that installs prompt hooks and emits OSC 7 with cwd, branch, and dirty state.Tests
RemoteShellIntegrationSnippet.script()that assert the emitted OSC 7 bytes include host, branch, dirty state, and ST terminator.77e1dfbef, then the implementation in commit98a2124fb.Local Validation
git diff --checkResources/Localizable.xcstringsparsed as JSON with Ruby.6b8d558cea313b09c10ac107254d6ee41a5044d9withCMUX_GHOSTTYKIT_NO_PREBUILT=1 ./scripts/ensure-ghosttykit.shto generate the real archive checksum.Manual Repro Plan After CI
CMUX_SKIP_ZIG_BUILD=1 ./scripts/reload.sh --tag issue-3791-mosh-remote-cwd-detection --launch.cdinto a local git repo, and confirm the sidebar shows the local cwd/branch.ssh localhostormosh <host>.eval "$(cmux remote-shell-init)"or append the printed snippet to~/.zshrc/~/.bashrc, thencdinto a git worktree.host:/pathand branch.Notes
manaflow-ai/ghosttybranchissue-3791-mosh-remote-cwd-detection. Direct push to Ghosttymainwas rejected by branch protection, so the commit is reachable but not yet on Ghosttymain.Note
Medium Risk
Touches sidebar directory/branch/PR tracking, session snapshot/restore, and terminal socket reporting paths; mistakes could cause incorrect git/PR state or directory display, but changes are mostly additive with extensive new tests.
Overview
Adds first-class per-surface
TerminalLocationstate that can parse OSC 7/file:///kitty-shell-cwd://cwd reports (including remote host) and optional git-branch hints, and updates sidebar directory resolution to display remote locations ashost:/path.Makes git/PR probing and listening-port tracking remote-aware: remote locations skip local git metadata probes/PR refreshes and clear stale local PR/branch state; session snapshot/restore and detached-surface transfer now persist terminal location when appropriate while avoiding restoring remote display paths as local directories.
Introduces
cmux remote-shell-initto print a bash/zsh prompt-hook snippet that emits OSC 7 cwd + branch/dirty hints for plainssh/moshpanes (with localized help/errors), updates docs/README accordingly, and bumps the pinned GhosttyKit fork/checksums to forward non-local OSC 7 URIs to the app. Also includes a small browser history restore fix and broad new unit/smoke test coverage for parsing, persistence, and shell snippet output.Reviewed by Cursor Bugbot for commit a98992e. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Track remote cwd and git branch from OSC 7/
kitty-shell-cwd://(andfile://) in plain ssh/mosh panes and show them per‑pane as host:/path. Addscmux remote-shell-init, makes sidebar/Finder/git/PR/port scans and session restore remote‑aware, and updates theghosttyfork to forward non‑local OSC 7. Closes #3791.New Features
TerminalLocationfrom OSC 7/kitty-shell-cwd:///file://with optional git hints; displayshost:/path, treats loopback as local, preserves trailing spaces and remotefile://URIs, escapes;/:in hosts, and clears stale branch on the first local update after a remote.host:/path; Finder opens only local paths; skip local git/PR and listening‑port scans for remote panes; avoid re‑publishing remote git‑branch hints; detached transfer and session snapshots carryTerminalLocationand ignore remote display strings for local restore; normalize terminal URI paths; never treathost:/pathorhost:portas local cwd.cmux remote-shell-initprints a zsh/bash snippet that emits OSC 7 with escaped host/path and optional git hints; supportsCMUX_REMOTE_HOSTandCMUX_REMOTE_DISABLE_GIT=1; zsh‑safe, preserves PATH, with localized help/error strings.ghosttyfork: forwards non‑local OSC 7 URIs to the app; submodule pin and checksums updated.Migration
ssh/moshpane: runcmux remote-shell-init >> ~/.zshrcor>> ~/.bashrc, then source it. Optionally setCMUX_REMOTE_HOST=nameto override the displayed host, orCMUX_REMOTE_DISABLE_GIT=1to report cwd only.Written for commit a98992e. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Behavior
Documentation
Localization
Tests
Chores