Fix SSH Files explorer using local $HOME and follow remote cd (#5360)#6037
Open
FunJim wants to merge 10 commits into
Open
Fix SSH Files explorer using local $HOME and follow remote cd (#5360)#6037FunJim wants to merge 10 commits into
FunJim wants to merge 10 commits into
Conversation
…m local cwd (manaflow-ai#5360) Adds FileExplorerSSHRootSeedingTests covering: - Workspace.init seeds currentDirectory with .localSeed origin so the SSH file explorer falls back to remote $HOME instead of /Users/<user>. - updatePanelDirectory on a remote workspace flips the origin to .remoteReport, exposing the directory via fileExplorerRemoteRootPath. - Legacy SessionWorkspaceSnapshot JSON without currentDirectoryOrigin decodes back-compat as nil (the restorer treats it as .localSeed). - FileExplorerError.sshCommandFailed embeds the underlying SSH stderr in its errorDescription rather than dropping it on the floor. Test fails to build until the helper API lands in the next commit.
…anaflow-ai#5360) restoreSessionSnapshot calls updatePanelDirectory(source:.restoredSnapshotMetadata) per panel, which would have re-promoted a stale .localSeed snapshot value to .remoteReport on app launch — defeating the fix for SSH workspaces whose persisted currentDirectory is the legacy local /Users/<user> path. Gate the origin write on .liveReport so only freshly-observed paths flip the origin.
…5360) The SSH file-explorer mounts in ContentView.syncFileExplorerDirectory and RightSidebarToolPanel.syncFileExplorerRoot were forwarding tab.currentDirectory as the remote rootPath. Workspace.init seeds currentDirectory with the local macOS $HOME, so on a fresh SSH workspace that path was sent to the remote and listing /Users/<user> failed. Use the new Workspace.fileExplorerRemoteRootPath helper, which returns nil unless currentDirectoryOrigin is .remoteReport. When nil, FileExplorerStore.applyRemoteSSHWorkspaceRoot's existing resolveRemoteHome fallback fires and roots at the actual remote $HOME.
…anaflow-ai#5360) Plan-suggested JSON used hand-rolled placeholder field names ('kind', 'pane.id', 'pane.tabIds') that did not match the actual SessionWorkspaceLayoutSnapshot/SessionPaneLayoutSnapshot Codable shape ('type', 'pane.panelIds'). Decoder failed with 'Key type not found' on a real run. Use the actual field names so the back-compat test exercises real Codable behavior.
…s panel (manaflow-ai#5360) Background: Workspace.currentDirectory and the file explorer that follows it are wired to the cmux shell-integration's report_pwd hook. Locally the hook delivers via _cmux_send_bg over the Unix socket. For SSH workspaces the shell on the remote host has CMUX_SOCKET_PATH set to a TCP relay (127.0.0.1:port), and _cmux_send_bg's "nc -U" Unix-socket call silently fails — every cd from the remote shell was getting dropped on the floor, so the Files panel stayed pinned at the resolved $HOME forever. Fix: - Add a JSON-RPC counterpart to the v1 sidebar report_pwd: surface.report_pwd(workspace_id, surface_id, path) on the relay. Mirrors surface.report_tty: ControlCommandCoordinator+Surface dispatches, ControlCommandCoordinator+Surface3 parses params, the new ControlSurfaceReportPWDResolution carries the outcome, and ControlSurfaceContext exposes the seam. TerminalController+ControlSurfaceContext4 implements it the same way controlSidebarScheduleScopedDirectoryUpdate does — resolve the surface id, then tabManager.updateSurfaceDirectory(...) (which routes through Workspace.updatePanelDirectory(.liveReport), already flips currentDirectoryOrigin to .remoteReport for SSH workspaces). - bash + zsh integrations gain _cmux_report_pwd_via_relay and use it from _cmux_prompt_command when _cmux_socket_uses_remote_relay is true. Local Unix-socket path is unchanged.
…file panel (manaflow-ai#5360) The previous commit added a relay-side surface.report_pwd so the remote shell can update the file panel via cd. But the local Ghostty PWD action (GHOSTTY_ACTION_PWD) was still firing for the parent local shell that ran `ssh user@host`, carrying the LOCAL pwd (e.g. /Users/<mac-user>). With currentDirectoryOrigin promotion gated on `isRemoteWorkspace` and .liveReport source, that local report was incorrectly flipping origin to .remoteReport and pinning the file panel at the local pwd. For SSH workspaces, treat OSC 7 from the local Ghostty surface as unreliable — its parent shell sees only local cwds. The relay-delivered surface.report_pwd is the only authoritative pwd source. Skip the GHOSTTY_ACTION_PWD dispatch entirely when isRemoteWorkspace is true. Local workspaces are unchanged.
…rompt_command (manaflow-ai#5360) _cmux_prompt_command guards every block on cmux_has_unix_socket and returns early when the socket is a TCP relay (lines 1473–1478 bash, 1665–1670 zsh). The downstream CWD-report block was only ever reached on Unix-socket sessions, so SSH-workspace shells never sent report_pwd at all. The previous commit added a relay-only RPC helper but it was unreachable from the prompt hook. Move the CWD report into the relay-only branch (right next to the ports-kick refresh) and revert the now-redundant relay branch in the downstream block. Local Unix-socket workspaces still use the legacy text-protocol report; SSH workspaces now send surface.report_pwd over the relay every time PWD changes.
|
Someone is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds remote shell PWD reporting via a new ChangesRemote PWD Reporting Integration
Sequence Diagram(s)sequenceDiagram
participant Shell as Remote Shell
participant Integration as bash/zsh Integration
participant Relay as Relay Transport
participant Coordinator as ControlCommandCoordinator
participant Handler as TerminalController
participant Workspace as Workspace Model
participant UI as File Explorer UI
Shell->>Integration: cwd changes
Integration->>Relay: surface.report_pwd (workspace_id, surface_id?, path)
Relay->>Coordinator: forward JSON-RPC request
Coordinator->>Handler: surfaceReportPWD(params)
Handler->>Workspace: updateSurfaceDirectory(path)
Handler->>Workspace: set currentDirectoryOrigin = .remoteReport
Workspace->>UI: fileExplorerRemoteRootPath (trimmed path)
UI->>UI: render remote file explorer at reported path
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (3 errors, 1 warning)
✅ Passed checks (17 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 |
Contributor
Greptile SummaryFixes two layered root causes behind the SSH Files explorer mounting at the local
Confidence Score: 5/5Safe to merge; the fix is well-scoped, backward-compatible with old snapshots, and covered by targeted unit tests. The two root causes are addressed at their origins rather than patched at the call sites, snapshot decoding is backward-compatible via an optional field with nil-as-localSeed semantics, the OSC 7 guard is narrowly targeted to SSH workspaces, and the known applyTabSelectionNow bypass is explicitly acknowledged and has no user-visible impact on the hot path. The bash and zsh shell integration comments slightly overstate retry guarantees for socket-level failures, but this does not affect correctness on the normal flow. Important Files Changed
Sequence DiagramsequenceDiagram
participant Shell as Remote Shell (bash/zsh)
participant Relay as cmux relay CLI
participant RPC as ControlCommandCoordinator
participant TC as TerminalController
participant WS as Workspace
participant FE as FileExplorerStore
Note over Shell: user does cd /data/workspace
Shell->>Shell: _cmux_precmd / _cmux_prompt_command
Shell->>Relay: "surface.report_pwd {workspace_id, path} (bg)"
Relay->>RPC: JSON-RPC surface.report_pwd
RPC->>TC: controlSurfaceReportPWD(workspaceID, surfaceID, path)
TC->>WS: tabManager.updateSurfaceDirectory(tabId, surfaceId, path)
WS->>WS: updatePanelDirectory(source: .liveReport)
WS->>WS: "currentDirectory = /data/workspace"
WS->>WS: "currentDirectoryOrigin = .remoteReport"
Note over FE: view re-renders, reads fileExplorerRemoteRootPath
FE->>WS: fileExplorerRemoteRootPath?
WS-->>FE: "/data/workspace (origin == .remoteReport)"
FE->>FE: applyWorkspaceRoot(/data/workspace)
Note over Shell: local macOS parent shell OSC 7 fires
Shell->>WS: "OSC 7 pwd = /Users/mac-user"
WS->>WS: isRemoteWorkspace? drop (no update)
Reviews (3): Last reviewed commit: "Address PR review feedback (#5360)" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+720
to
+732
| case .sshCommandFailed(let detail): | ||
| let trimmed = detail.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| if trimmed.isEmpty { | ||
| return String( | ||
| localized: "fileExplorer.error.sshFailed", | ||
| defaultValue: "SSH command failed" | ||
| ) | ||
| } | ||
| let format = String( | ||
| localized: "fileExplorer.error.sshFailedWithDetail", | ||
| defaultValue: "SSH command failed: %@" | ||
| ) | ||
| return String(format: format, trimmed) |
Contributor
There was a problem hiding this comment.
errorDescription. SSH stderr can be multi-line (connection banners, host-key warnings, IP addresses) and will appear as-is in the Files panel error label. Per the cmux user-facing error rule, raw upstream error output belongs in sanitized logs; the user-facing copy should stay in cmux terms. The detail should be passed to a details/telemetry path and the public message kept as the bare localized string, or at minimum the first non-empty trimmed line should be extracted to avoid banner noise.
Suggested change
| case .sshCommandFailed(let detail): | |
| let trimmed = detail.trimmingCharacters(in: .whitespacesAndNewlines) | |
| if trimmed.isEmpty { | |
| return String( | |
| localized: "fileExplorer.error.sshFailed", | |
| defaultValue: "SSH command failed" | |
| ) | |
| } | |
| let format = String( | |
| localized: "fileExplorer.error.sshFailedWithDetail", | |
| defaultValue: "SSH command failed: %@" | |
| ) | |
| return String(format: format, trimmed) | |
| case .sshCommandFailed(let detail): | |
| let trimmed = detail.trimmingCharacters(in: .whitespacesAndNewlines) | |
| // Surface only the first non-empty line so multi-line SSH banners | |
| // (host-key notices, connection warnings) don't appear verbatim in | |
| // the UI. Full stderr belongs in sanitized logs. | |
| let firstLine = trimmed | |
| .components(separatedBy: .newlines) | |
| .first(where: { !$0.trimmingCharacters(in: .whitespaces).isEmpty }) ?? "" | |
| if firstLine.isEmpty { | |
| return String( | |
| localized: "fileExplorer.error.sshFailed", | |
| defaultValue: "SSH command failed" | |
| ) | |
| } | |
| let format = String( | |
| localized: "fileExplorer.error.sshFailedWithDetail", | |
| defaultValue: "SSH command failed: %@" | |
| ) | |
| return String(format: format, firstLine) |
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!
Author
There was a problem hiding this comment.
Applied in ee25eebdc. errorDescription now extracts the first non-empty line of the trimmed stderr; full stderr stays in the .sshCommandFailed(detail) associated value for callers that want it for logs/telemetry.
Comment on lines
+131630
to
+131652
| "fileExplorer.error.sshFailedWithDetail": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "SSH command failed: %@" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "SSH コマンドに失敗しました: %@" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "SSH 명령에 실패했습니다: %@" | ||
| } | ||
| } | ||
| } | ||
| }, |
Contributor
There was a problem hiding this comment.
fileExplorer.error.sshFailedWithDetail is added with translations for en/ja/ko only, but Resources/Localizable.xcstrings already carries German (de) and Chinese Simplified (zh-Hans) entries (1,300+ each). Per the cmux full-internationalization rule, new string-catalog additions must include translated entries for every locale already supported by the touched catalog. The sibling fileExplorer.error.sshFailed has the same gap, but the new key introduced by this PR should have complete coverage.
Rule Used: Flag production user-facing text that is not fully... (source)
Author
There was a problem hiding this comment.
Skipping. The sibling key fileExplorer.error.sshFailed and the adjacent fileExplorer.status.sshHomeFailed (rendered side-by-side with this one in the Files panel) ship en/ja/ko only — going to 20 locales for one new key would be inconsistent with the catalog's actual coverage in this neighborhood. If the project wants a broader rollout, it should backfill the sibling keys at the same time as a separate localization sweep.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/FileExplorerSSHRootSeedingTests.swift`:
- Around line 62-79: The test named
testLegacySnapshotMissingOriginDecodesAsLocalSeed currently only checks that
decoding yields a nil currentDirectoryOrigin on SessionWorkspaceSnapshot; either
rename the test to testLegacySnapshotMissingOriginDecodesAsNil to reflect that
it only asserts nil, or extend the test to complete the back-compat contract by
creating a Workspace (or calling the restore method used in production) from the
decoded snapshot and assert the resulting Workspace's currentDirectoryOrigin
equals .localSeed; locate the test function
testLegacySnapshotMissingOriginDecodesAsLocalSeed and update its name or add the
workspace restoration and final XCTAssert for .localSeed on the restored
object's currentDirectoryOrigin accordingly.
In `@Resources/shell-integration/cmux-bash-integration.bash`:
- Around line 1480-1483: In both affected files update the logic so the
last-reported PWD is only set after the relay dispatch succeeds: in
Resources/shell-integration/cmux-bash-integration.bash (lines 1480-1483) call
_cmux_report_pwd_via_relay "$relay_pwd" and, only if it returns success, then
set _CMUX_PWD_LAST_PWD="$relay_pwd" (i.e., move the assignment to run on
successful return of _cmux_report_pwd_via_relay); in
Resources/shell-integration/cmux-zsh-integration.zsh (lines 1672-1675) apply the
same pattern—invoke _cmux_report_pwd_via_relay "$relay_pwd" first and update
_CMUX_PWD_LAST_PWD="$relay_pwd" only when that call succeeds so failed
dispatches can be retried on the next prompt.
In `@Sources/Workspace.swift`:
- Around line 295-299: The code currently rehydrates currentDirectory and
currentDirectoryOrigin from a snapshot even when snapshot.currentDirectoryOrigin
== .remoteReport, which lets a stale remote cached path drive
fileExplorerRemoteRootPath; change the logic so you only accept the
normalizedCurrentDirectory from the snapshot if snapshot.currentDirectoryOrigin
is not .remoteReport (or if you also perform explicit freshness validation),
otherwise ignore the snapshot's normalizedCurrentDirectory and leave
currentDirectory/currentDirectoryOrigin at their safe defaults (or set origin to
.localSeed); update the block that references normalizedCurrentDirectory,
currentDirectory, and currentDirectoryOrigin (and the check of
snapshot.currentDirectoryOrigin) to implement this conditional acceptance of
remoteReport-derived paths.
🪄 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: fdbd35fb-7638-4cfb-b74a-eb7ce79c5ad5
📒 Files selected for processing (16)
Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface3.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceContext.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceReportPWDResolution.swiftResources/Localizable.xcstringsResources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshSources/ContentView.swiftSources/FileExplorerStore.swiftSources/GhosttyTerminalView.swiftSources/RightSidebarToolPanel.swiftSources/SessionPersistence.swiftSources/TerminalController+ControlSurfaceContext4.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/FileExplorerSSHRootSeedingTests.swift
- FileExplorerError.sshCommandFailed: surface only the first non-empty line of the SSH stderr in the user-visible message so multi-line SSH banners (host-key warnings, connection notices) don't appear verbatim in the Files panel error label. Full stderr is still passed in via the associated value for log/telemetry callers. - restoreSessionSnapshot: never trust a persisted .remoteReport. The semantic of .remoteReport is "confirmed by a live remote shell", but a relaunched workspace has not re-confirmed anything yet — the remote shell may have started in a different cwd, or the persisted path may no longer exist. Coerce restored .remoteReport to .localSeed so the SSH file explorer falls back to remote $HOME via resolveRemoteHome on reconnect; the first .liveReport from the reconnected shell flips the origin back to .remoteReport. - bash/zsh integration: cache _CMUX_PWD_LAST_PWD only after the relay dispatch succeeds. The previous unconditional cache suppressed retries when the relay CLI path or remote-relay socket detection transiently fails on the first prompt of a new shell. - FileExplorerSSHRootSeedingTests: rename testLegacySnapshotMissingOriginDecodesAsLocalSeed to ...DecodesAsNil to accurately describe what's asserted (the decoder contract — Workspace.restoreSessionSnapshot's nil → .localSeed coercion is exercised end-to-end via the manual restore path).
Author
|
Addressed review feedback in
Regression test still green: |
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/FileExplorerStore.swift (1)
736-740: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse modern localization interpolation instead of String(format:).
Lines 736-740 fetch a localized format string then apply
String(format:)separately. Per repo learning, when the format template is retrieved as a plainString(notString.LocalizationValue), preferString.localizedStringWithFormator the modern interpolation overload.Recommend:
- let format = String( - localized: "fileExplorer.error.sshFailedWithDetail", - defaultValue: "SSH command failed: %@" - ) - return String(format: format, firstLine) + return String( + localized: "fileExplorer.error.sshFailedWithDetail", + defaultValue: "SSH command failed: \(firstLine)" + )This uses the modern
String.LocalizationValueinterpolation, which the xcstrings runtime handles automatically.🤖 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/FileExplorerStore.swift` around lines 736 - 740, Replace the separate format lookup + String(format:) call with the modern localized interpolation overload: instead of creating `format` and then `return String(format: format, firstLine)`, return a single interpolated localized string such as `return String(localized: "fileExplorer.error.sshFailedWithDetail", defaultValue: "SSH command failed: \(firstLine)")`; this uses the `String(localized:...)` interpolation path and avoids `String(format:)`.Source: Learnings
🤖 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/FileExplorerStore.swift`:
- Around line 736-740: Replace the separate format lookup + String(format:) call
with the modern localized interpolation overload: instead of creating `format`
and then `return String(format: format, firstLine)`, return a single
interpolated localized string such as `return String(localized:
"fileExplorer.error.sshFailedWithDetail", defaultValue: "SSH command failed:
\(firstLine)")`; this uses the `String(localized:...)` interpolation path and
avoids `String(format:)`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f221c23-4470-472c-a5b2-7b4940fc94a9
📒 Files selected for processing (5)
Resources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshSources/FileExplorerStore.swiftSources/Workspace.swiftcmuxTests/FileExplorerSSHRootSeedingTests.swift
FunJim
force-pushed
the
fix-5360-ssh-files-explorer-root
branch
from
ee25eeb to
3db84fa
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 `@Resources/shell-integration/cmux-bash-integration.bash`:
- Around line 152-167: In the _cmux_report_pwd_via_relay function, replace the
call to _cmux_relay_rpc_bg with _cmux_relay_rpc on the last line. This change
ensures the RPC call completes synchronously before the function returns,
allowing the caller to cache _CMUX_PWD_LAST_PWD only after the RPC has actually
succeeded, rather than just after the background spawn returns. This prevents
suppressing retries on transient relay or server failures.
🪄 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: d1bb5e9a-9126-4c57-85cd-e731b07b9c63
📒 Files selected for processing (16)
Packages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlCommandCoordinator+Surface3.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceContext.swiftPackages/CmuxControlSocket/Sources/CmuxControlSocket/Coordinator/Surface/ControlSurfaceReportPWDResolution.swiftResources/Localizable.xcstringsResources/shell-integration/cmux-bash-integration.bashResources/shell-integration/cmux-zsh-integration.zshSources/ContentView.swiftSources/FileExplorerStore.swiftSources/GhosttyTerminalView.swiftSources/RightSidebarToolPanel.swiftSources/SessionPersistence.swiftSources/TerminalController+ControlSurfaceContext4.swiftSources/Workspace.swiftcmux.xcodeproj/project.pbxprojcmuxTests/FileExplorerSSHRootSeedingTests.swift
Comment on lines
+152
to
+167
| _cmux_report_pwd_via_relay() { | ||
| _cmux_socket_uses_remote_relay || return 1 | ||
| local workspace_id="" | ||
| workspace_id="$(_cmux_relay_workspace_id)" || return 1 | ||
| local pwd_value="$1" | ||
| [[ -n "$pwd_value" ]] || return 1 | ||
|
|
||
| local pwd_json params | ||
| pwd_json="$(_cmux_json_escape "$pwd_value")" | ||
| params="{\"workspace_id\":\"$workspace_id\",\"path\":\"$pwd_json\"" | ||
| if [[ -n "$CMUX_PANEL_ID" ]]; then | ||
| params+=",\"surface_id\":\"$CMUX_PANEL_ID\"" | ||
| fi | ||
| params+="}" | ||
| _cmux_relay_rpc_bg "surface.report_pwd" "$params" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the helper/caller flow is fire-and-forget today.
# 1) PWD helper + cache update callsites
rg -n -C2 '_cmux_report_pwd_via_relay|_cmux_relay_rpc_bg "surface.report_pwd"|_CMUX_PWD_LAST_PWD' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zsh
# 2) Show relay bg path returns after background launch (no success wait)
rg -n -C3 '_cmux_relay_rpc_bg|_cmux_detach_bg|&!| \& \)|\|\| true' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zshRepository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
#!/bin/bash
# Verify synchronous _cmux_relay_rpc exists and check for other _cmux_relay_rpc_bg patterns
# 1) Show _cmux_relay_rpc (synchronous) definition
rg -n -A5 '_cmux_relay_rpc\(\)' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zsh
# 2) Find all callers of _cmux_relay_rpc_bg to assess scope of issue
rg -n '_cmux_relay_rpc_bg' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zshRepository: manaflow-ai/cmux
Length of output: 1681
🏁 Script executed:
#!/bin/bash
# Check if _cmux_ports_kick_via_relay has the same caching issue
# 1) Show _cmux_ports_kick_via_relay definition + callers
rg -n -B2 -A10 '_cmux_ports_kick_via_relay' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zsh | head -80
# 2) Show caller context for both relay functions (check if caching follows return)
rg -n -B3 -A5 'if.*_cmux_report_pwd_via_relay|if.*_cmux_ports_kick_via_relay' \
Resources/shell-integration/cmux-bash-integration.bash \
Resources/shell-integration/cmux-zsh-integration.zshRepository: manaflow-ai/cmux
Length of output: 6646
Change _cmux_relay_rpc_bg to _cmux_relay_rpc in pwd-reporting helpers to wait for RPC completion before caching.
_cmux_report_pwd_via_relay currently calls _cmux_relay_rpc_bg, which backgrounds the RPC and returns immediately. The caller then caches _CMUX_PWD_LAST_PWD on return success, suppressing retries on transient relay/server failures. The comments at lines 1479–1481 (bash) and 1671–1673 (zsh) explicitly state the intent: "Cache _CMUX_PWD_LAST_PWD only after the dispatch succeeds" — but "succeeds" is being interpreted as "background spawn" rather than "RPC completed." Using the synchronous _cmux_relay_rpc variant instead ensures the RPC completes before cache update.
Suggested fix
--- a/Resources/shell-integration/cmux-bash-integration.bash
+++ b/Resources/shell-integration/cmux-bash-integration.bash
@@ -163,7 +163,7 @@ _cmux_report_pwd_via_relay() {
params+=",\"surface_id\":\"$CMUX_PANEL_ID\""
fi
params+="}"
- _cmux_relay_rpc_bg "surface.report_pwd" "$params"
+ _cmux_relay_rpc "surface.report_pwd" "$params"
}--- a/Resources/shell-integration/cmux-zsh-integration.zsh
+++ b/Resources/shell-integration/cmux-zsh-integration.zsh
@@ -165,7 +165,7 @@ _cmux_report_pwd_via_relay() {
params+=",\"surface_id\":\"$CMUX_PANEL_ID\""
fi
params+="}"
- _cmux_relay_rpc_bg "surface.report_pwd" "$params"
+ _cmux_relay_rpc "surface.report_pwd" "$params"
}🤖 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 `@Resources/shell-integration/cmux-bash-integration.bash` around lines 152 -
167, In the _cmux_report_pwd_via_relay function, replace the call to
_cmux_relay_rpc_bg with _cmux_relay_rpc on the last line. This change ensures
the RPC call completes synchronously before the function returns, allowing the
caller to cache _CMUX_PWD_LAST_PWD only after the RPC has actually succeeded,
rather than just after the background spawn returns. This prevents suppressing
retries on transient relay or server failures.
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
1 participant
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.
Closes #5360.
Summary
Two layered root causes; both fixed:
Workspace.currentDirectorywas seeded with the local macOS$HOMEat workspace init, and the SSH file-explorer call sites unconditionally forwarded that value as the remote rootPath — so a fresh SSH workspace tried tols /Users/<mac-user>on the Linux remote and failed with a bare"SSH command failed". Track the origin ofcurrentDirectory(localSeed/localKnown/remoteReport) and only forward.remoteReport-confirmed paths. When the helper returnsnil,FileExplorerStore.applyRemoteSSHWorkspaceRoot's existingresolveRemoteHomefallback correctly roots at the remote$HOME.cdon the remote shell never updated the Files panel. The cmux shell integration'sreport_pwdhook delivers via_cmux_send_bg(Unix-socket only). For SSH workspacesCMUX_SOCKET_PATH=127.0.0.1:<relayPort>(TCP), so every cwd change was dropped on the floor. Add a JSON-RPC counterpartsurface.report_pwdand call it from the relay-only branch of_cmux_prompt_command/_cmux_precmd.Also surfaces the SSH stderr in
FileExplorerError.sshCommandFailed(detail)'s localized message instead of dropping it, so the panel shows e.g.SSH command failed: ls: cannot access '/foo'rather than the bare string.Implementation notes
currentDirectoryOriginis promoted to.remoteReportonly whensource == .liveReport; the restore path that replays persisted panel directories (source: .restoredSnapshotMetadata) cannot re-promote a stale.localSeedsnapshot path.surface.report_pwdmirrorssurface.report_tty— same param parsing inPackages/CmuxControlSocket/.../ControlCommandCoordinator+Surface{,3}.swift, samesurfaceReportSurfaceFieldsreuse, same shape of resolution enum (minus.pendingsince the cmuxd-attached shell already has a known surface id), implementation inSources/TerminalController+ControlSurfaceContext4.swift._cmux_report_pwd_via_relay; the cwd-report fires inside the relay-only early-return branch of_cmux_prompt_command(the downstream cwd block is gated on a Unix socket and was unreachable for SSH workspaces).ssh user@hostare now ignored on remote workspaces (the parent shell's pwd is always a local path; only the relay-delivered report is authoritative for SSH workspaces).SessionWorkspaceSnapshot.currentDirectoryOriginisOptionalwith defaultnilso oldsession-com.cmuxterm.app.jsonfiles decode unchanged; restore treats missing as.localSeed.85c0ea7ccaddscmuxTests/FileExplorerSSHRootSeedingTests(six cases),140085f1eadds the helpers; CI on the first commit fails to compile on the unresolved helper symbols, on the green tip all six tests pass.fileExplorer.error.sshFailedWithDetailxcstrings entry in en/ja/ko, matching the locales of the siblingfileExplorer.error.sshFailed.Test plan
xcodebuild test -scheme cmux-unit -only-testing:cmuxTests/FileExplorerSSHRootSeedingTests— all 6 cases pass on HEAD; the same target on the regression-test commit alone fails to compile with exactly the predicted unresolved symbols (currentDirectoryOrigin,fileExplorerRemoteRootPath,debugSetFocusedPanelIdForTests,CurrentDirectoryOrigin.localSeed/.localKnown/.remoteReport,SessionWorkspaceSnapshot.currentDirectoryOrigin)../scripts/lint-pbxproj-test-wiring.sh— ok (191 test files), the new test file has all four pbxproj entries.cmux ssh devcloudfrom macOS to a Linux remote where remote$HOME = /root≠ local/Users/funjim. Files panel mounted at/root(the actual remote home), not at the macOS home.cd /tmp && cd /data/workspacein the remote shell — Files panel re-rooted on eachcd.Known minor follow-ups (not blocking)
Workspace.applyTabSelectionNow(Sources/Workspace.swift:17317) and a sibling site (~18893) writecurrentDirectory = panelDirectories[panelId]directly without going throughupdatePanelDirectory, bypassing origin tracking. Not on the SSH workspace Files explorer uses local $HOME as remote root → "SSH command failed" #5360 hot path; the abstraction is leaky there but the practical state is correct because for remote workspaces everypanelDirectories[panelId]was last written via.liveReportalready. Could be tightened in a follow-up by routing those two writes throughupdatePanelDirectory.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes the SSH Files explorer mounting at the local $HOME and not following cd. It now roots at the remote $HOME and updates when the remote cwd changes (fixes #5360).
currentDirectoryOriginand only forwardfileExplorerRemoteRootPathwhen it’s.remoteReport; otherwise fall back to the remote$HOME..remoteReportto.localSeeduntil a live report arrives, preventing stale paths from seeding the SSH root.surface.report_pwdand send from bash/zsh via the relay; fire from the relay-only branch and cache the last PWD only after a successful send.Written for commit 3db84fa. Summary will update on new commits.
Summary by CodeRabbit
surface.report_pwd) with relay/RPC support, and exposed the reported remote root in the UI/file explorer.