Repository navigation
Follow SSH workspaces in the Files sidebar - #3721
Merged
Merged
Conversation
Add regression coverage for the Files sidebar root-selection contract before changing production code. The tests model a local workspace switching to an SSH workspace and require the store to resolve the remote HOME through a mock SSH transport instead of retaining the local macOS home path. Constraint: Issue #3719 requires a failing regression commit before the fix Constraint: Local xcodebuild and local tests are prohibited for this task Confidence: high Scope-risk: narrow Directive: Keep SSH file explorer root selection behind a mockable transport seam; do not derive remote roots from local currentDirectory Tested: git diff --check -- cmuxTests/FileExplorerStoreTests.swift Not-tested: XCTest execution intentionally deferred to GitHub Actions
Remote workspaces were passing the terminal cwd through the local file explorer path flow, so a cmux ssh workspace could inherit the macOS home before the remote shell reported any usable directory. The sidebar now accepts a workspace-root request, resolves SSH roots through an injectable transport, and clears stale local trees while remote home resolution or availability status is pending. Constraint: Issue #3719 requires the Files sidebar to follow the focused workspace automatically instead of requiring a manual remote root Constraint: Local xcodebuild and local UI/XCUITests are prohibited in this workspace; CI owns build and test execution Rejected: Branch inside ContentView using the currentDirectory string as a guessed remote home | it preserves the stale local-path failure and duplicates provider knowledge outside the store Confidence: medium Scope-risk: moderate Directive: Keep file explorer root selection funneled through FileExplorerWorkspaceRoot so local and SSH providers do not diverge again Tested: git diff --cached --check; jq empty Resources/Localizable.xcstrings Not-tested: XCTest/build deferred to GitHub Actions per 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:
📝 WalkthroughWalkthroughImplements SSH workspace root support for the file explorer: adds connection/transport abstractions, refactors provider to use async HOME resolution with cancellation/deduping, centralizes workspace-root application in the store, surfaces root status messages to the UI, updates observer wiring, and adds tests and localization. ChangesSSH Workspace Root Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (3 errors, 1 warning, 2 inconclusive)
✅ Passed checks (8 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 |
The Swift 5 compiler used by CI does not infer an implicit return from a multi-branch computed property body. Make the local-display fallback explicit so the workspace-root abstraction compiles in Debug and unit-test builds. Constraint: CI logs showed FileExplorerStore.displayRootPath missing a return in the fallback branch Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: Local build/tests deferred to CI; xcodebuild is prohibited in this workspace
Contributor
Greptile SummaryRoutes all file-explorer root changes through a single
Confidence Score: 5/5Safe to merge; cancellation and provider-identity guards are correctly ordered on the main actor and the new async SSH home resolution is well-isolated. The core async state machine — resolveRemoteHome task lifecycle, the deduplication key, and the provider identity guard — is correctly implemented for cooperative threading on @mainactor. The try Task.checkCancellation() addition after listDirectory ensures stale root-load results cannot clear an SSH-unavailable status. The only structural concern is that setProvider relies on callers having first invoked setRootPath to trigger cancelAllLoads, which is upheld by all current call sites but is an undocumented ordering constraint. Sources/FileExplorerStore.swift — the implicit cancel-before-set ordering between setRootPath and setProvider deserves attention as the file grows. Important Files Changed
Sequence DiagramsequenceDiagram
participant CV as ContentView
participant SWDO as SelectedWorkspaceDirectoryObserver
participant FES as FileExplorerStore
participant SSH as SSHFileExplorerProvider
participant T as ProcessSSHFileExplorerTransport
SWDO->>SWDO: Snapshot(workspaceId, dir, remoteConfig, connectionState, connectionDetail, daemonStatus)
SWDO->>SWDO: removeDuplicates() on Snapshot.Equatable
SWDO->>CV: "directoryChangeGeneration += 1"
CV->>FES: applyWorkspaceRoot(.remoteSSH(...))
alt same SSH connection
FES->>SSH: updateAvailability(isAvailable, homePath: nil)
else new SSH connection
FES->>FES: cancelRemoteHomeResolution()
FES->>FES: setRootPath() reload() cancelAllLoads()
FES->>SSH: SSHFileExplorerProvider(connection, displayTarget)
FES->>FES: setProvider(sshProvider, reloadIfAvailable: false)
end
alt isAvailable false
FES->>FES: cancelRemoteHomeResolution()
FES->>FES: setRootStatusMessage(sshUnavailable)
else homePath already resolved
FES->>FES: setRootStatusMessage(nil)
FES->>FES: setRootPath(currentHomePath) reload()
else homePath empty
FES->>FES: setRootStatusMessage(sshResolvingHome)
FES->>SSH: resolveHomePath()
SSH->>T: resolveHomePath(connection) nonisolated GCD
T-->>SSH: /home/dev
SSH-->>FES: /home/dev
FES->>FES: MainActor.run guard key match and provider identity
FES->>SSH: updateAvailability(true, homePath: /home/dev)
FES->>FES: setRootStatusMessage(nil)
FES->>FES: setRootPath(/home/dev) reload()
end
CV->>FES: applyWorkspaceRoot(.local or .none)
FES->>FES: cancelRemoteHomeResolution()
FES->>FES: setRootPath() reload() cancelAllLoads()
Reviews (9): Last reviewed commit: "Refresh the PR after Files sidebar chang..." | Re-trigger Greptile |
Review feedback pointed out that the remote root observer could re-emit on repeated SSH state publications and that the new SSH transport used a legacy DispatchQueue continuation wrapper. Keep the store entry point unchanged, but deduplicate the effective workspace-root snapshot and move blocking SSH subprocess work into a detached task with Sendable inputs and outputs. Constraint: Greptile review flagged the observer noise and transport concurrency shape before CI completed Rejected: Leave both as follow-up items | the fixes are narrow and reduce future concurrency and invalidation risk Confidence: medium Scope-risk: narrow Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: Local XCTest/build deferred to GitHub Actions; xcodebuild is prohibited in this workspace
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 5 files
You’re at about 98% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Greptile's current-head review kept the protocol isolation contract as a medium hardening item. Mark the transport requirements nonisolated so future conformers cannot inherit caller-actor isolation when resolving SSH file roots. Constraint: SSH file listing and home resolution may run from UI-driven store updates but must not block the main actor Confidence: medium Scope-risk: narrow Tested: git diff --check Not-tested: Local build/tests deferred to GitHub Actions; xcodebuild is prohibited in this workspace
![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
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)
394-425:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftSSH processes are not terminated when the resolution
Taskis cancelled — threads remain blocked.
withCheckedThrowingContinuationdoes not automatically throwCancellationErrorwhen the enclosingTaskis cancelled; it only resumes oncecontinuation.resume(…)is called. Because theDispatchQueue.globalwork item is already in-flight at that point, andProcess.waitUntilExit()blocks until the SSH child exits, every call tocancelRemoteHomeResolution()leaves a runningsshprocess and a blocked background thread. WithConnectTimeout=5as the only bound, rapid workspace switching (or a slow remote host) can accumulate multiple blocked threads.Restructure to create the
Processoutside the continuation sowithTaskCancellationHandlercan reach it:🔧 Suggested fix — terminate process on cancellation
- func resolveHomePath(connection: SSHFileExplorerConnection) async throws -> String { - try await runOnBackground { - let output = try Self.runSSHCommand( - connection: connection, - command: #"printf '%s\n' "$HOME""# - ) - return output.trimmingCharacters(in: .whitespacesAndNewlines) - } - } + func resolveHomePath(connection: SSHFileExplorerConnection) async throws -> String { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/usr/bin/ssh") + process.arguments = Self.sshArguments(connection: connection, command: #"printf '%s\n' "$HOME""#) + let outPipe = Pipe() + let errPipe = Pipe() + process.standardOutput = outPipe + process.standardError = errPipe + return try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { continuation in + DispatchQueue.global(qos: .userInitiated).async { + do { + try process.run() + let data = outPipe.fileHandleForReading.readDataToEndOfFile() + let errData = errPipe.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + guard process.terminationStatus == 0 else { + throw FileExplorerError.sshCommandFailed( + String(data: errData, encoding: .utf8) ?? "") + } + continuation.resume(returning: + (String(data: data, encoding: .utf8) ?? "") + .trimmingCharacters(in: .whitespacesAndNewlines)) + } catch { + continuation.resume(throwing: error) + } + } + } + } onCancel: { + process.terminate() + } + }Apply the same pattern to
listDirectoryin this transport.🤖 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 394 - 425, The background wrapper runOnBackground causes SSH child processes started by Self.runSSHCommand and Self.runSSHListCommand (used by resolveHomePath and listDirectory) to be unreachable when the surrounding Task is cancelled, leaving ssh processes and threads blocked; fix by creating and configuring the Process (the SSH child) outside the withCheckedThrowingContinuation so you can wrap the continuation in withTaskCancellationHandler that calls process.terminate()/process.kill() on cancellation, then perform Process.run()/waitInside the continuation and resume/throw accordingly; apply the same pattern to both resolveHomePath (which calls runSSHCommand) and listDirectory (which calls runSSHListCommand) so cancellation reliably stops the SSH process and unblocks the background thread.
🤖 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/ContentView.swift`:
- Around line 2474-2476: The remote explorer availability is currently gated
with tab.remoteConnectionState != .error which leaves it enabled during
.connecting/.disconnected; update the predicate to require the connected state
instead. In the call site where isAvailable is set (currently isAvailable:
tab.remoteConnectionState != .error), change it to check for the explicit
connected enum (e.g. isAvailable: tab.remoteConnectionState == .connected) while
leaving unavailableDetail as-is (tab.remoteConnectionDetail ??
tab.remoteDaemonStatus.detail) so the UI only becomes available when the SSH
workspace is actually connected.
In `@Sources/FileExplorerStore.swift`:
- Around line 922-936: Replace the old String(format:) wrapping with the modern
String(localized:defaultValue:) interpolation: when building the SSH-unavailable
message in FileExplorerStore (the branch that checks detail / !detail.isEmpty),
pass the detail via Swift string interpolation in the defaultValue (e.g., "...
\(detail)") instead of using String(format:), and keep the existing
setRootStatusMessage calls and the other localized key unchanged; update the
code paths that construct the message (the conditional using detail and the
setRootStatusMessage invocation) accordingly.
- Line 839: Replace direct assignments to the `@Published` property
rootStatusMessage with calls to the setter method setRootStatusMessage(_:) so
the equality guard there prevents spurious objectWillChange notifications;
locate the two direct assignments (the success path near where resolveRemoteHome
is used and the other assignment at the second occurrence) and change them to
setRootStatusMessage(nil) or setRootStatusMessage(someMessage) as appropriate
rather than using rootStatusMessage = ... .
- Around line 574-582: In displayRootPath collapse the two optional casts by
binding provider once: use a single if-let like "if let sshProvider = provider
as? SSHFileExplorerProvider { … }" then inside check rootPath.isEmpty to return
either "ssh://\(sshProvider.displayTarget)" or
"ssh://\(sshProvider.displayTarget):\(rootPath)"; otherwise fall back to
FileExplorerRootResolver.displayPath(for: rootPath, homePath:
provider?.homePath). This removes the duplicate "as? SSHFileExplorerProvider"
casts while preserving the empty-rootPath branch and the fallback to
FileExplorerRootResolver.
- Around line 685-694: The method setProvider(_:reloadIfAvailable:) in
FileExplorerStore was made internal unintentionally; revert its visibility back
to private so external module code cannot bypass the FileExplorerStore
lifecycle; update the declaration of setProvider to private (leaving its
signature and behavior unchanged) and ensure callers remain applyWorkspaceRoot
and applyRemoteSSHWorkspaceRoot within FileExplorerStore and tests can continue
to call it via test accessors as needed.
---
Outside diff comments:
In `@Sources/FileExplorerStore.swift`:
- Around line 394-425: The background wrapper runOnBackground causes SSH child
processes started by Self.runSSHCommand and Self.runSSHListCommand (used by
resolveHomePath and listDirectory) to be unreachable when the surrounding Task
is cancelled, leaving ssh processes and threads blocked; fix by creating and
configuring the Process (the SSH child) outside the
withCheckedThrowingContinuation so you can wrap the continuation in
withTaskCancellationHandler that calls process.terminate()/process.kill() on
cancellation, then perform Process.run()/waitInside the continuation and
resume/throw accordingly; apply the same pattern to both resolveHomePath (which
calls runSSHCommand) and listDirectory (which calls runSSHListCommand) so
cancellation reliably stops the SSH process and unblocks the background thread.
🪄 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: b5f393ce-b142-4d93-a025-a171e53865da
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/ContentView.swiftSources/FileExplorerStore.swiftSources/FileExplorerView.swiftcmuxTests/FileExplorerStoreTests.swift
Review feedback showed remote file explorer subprocesses could survive root-switch cancellation, and the sidebar was still willing to probe SSH files before the workspace reached the connected state. Keep the provider lifecycle private, terminate reachable SSH children on cancellation, and route test-only provider injection through an explicit debug helper. Constraint: Do not run local xcodebuild or XCUITests for this workspace Rejected: Leave SSH probes available while connecting | this still lets the sidebar show transient remote roots before authentication is complete Confidence: medium Scope-risk: moderate Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: local Xcode build/tests per workspace policy
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)
425-434:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDrain both SSH pipes concurrently to prevent subprocess deadlock.
At lines 432–433, stdout and stderr are read to EOF sequentially. If the process writes to stderr beyond the pipe buffer capacity before the main thread drains stdout, the process blocks waiting for stderr to be consumed, while the main thread blocks on stdout EOF—creating a deadlock. Read both pipes concurrently to prevent this.
Suggested fix (concurrent draining)
let outPipe = Pipe() let errPipe = Pipe() process.standardOutput = outPipe process.standardError = errPipe try process.run() - // Read pipe data before waitUntilExit to avoid deadlock when pipe buffer fills - let data = outPipe.fileHandleForReading.readDataToEndOfFile() - let stderrData = errPipe.fileHandleForReading.readDataToEndOfFile() + // Drain both pipes concurrently to avoid blocking if either pipe fills. + var data = Data() + var stderrData = Data() + let group = DispatchGroup() + let readQueue = DispatchQueue(label: "com.cmux.fileExplorer.sshRead", attributes: .concurrent) + group.enter() + readQueue.async { + data = outPipe.fileHandleForReading.readDataToEndOfFile() + group.leave() + } + group.enter() + readQueue.async { + stderrData = errPipe.fileHandleForReading.readDataToEndOfFile() + group.leave() + } + group.wait() process.waitUntilExit()🤖 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 425 - 434, The code currently reads stdout then stderr sequentially on the main thread (using outPipe.fileHandleForReading.readDataToEndOfFile() and errPipe.fileHandleForReading.readDataToEndOfFile()), which can deadlock if one buffer fills; change this to drain both pipes concurrently (e.g. spawn two background reads or use DispatchGroup/async tasks to read outPipe and errPipe in parallel), collect both Data results, then call process.waitUntilExit(); update the logic around process.standardOutput/standardError and the variables capturing stdout/stderr so the concurrent reads return the same Data values used downstream.
🤖 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 425-434: The code currently reads stdout then stderr sequentially
on the main thread (using outPipe.fileHandleForReading.readDataToEndOfFile() and
errPipe.fileHandleForReading.readDataToEndOfFile()), which can deadlock if one
buffer fills; change this to drain both pipes concurrently (e.g. spawn two
background reads or use DispatchGroup/async tasks to read outPipe and errPipe in
parallel), collect both Data results, then call process.waitUntilExit(); update
the logic around process.standardOutput/standardError and the variables
capturing stdout/stderr so the concurrent reads return the same Data values used
downstream.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce391f7a-1348-4e7b-b66b-b9fc915a1039
📒 Files selected for processing (2)
Sources/ContentView.swiftSources/FileExplorerStore.swift
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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/FileExplorerStore.swift`:
- Around line 1029-1040: The catch path can clear state for a replaced provider
because it only checks remoteHomeResolutionKey; update the failure handler in
the async MainActor block to also confirm the provider instance matches the SSH
provider (same check used in the success branch) before clearing
remoteHomeResolutionTask/remoteHomeResolutionKey and calling
setRootPath/setRootStatusMessage; i.e., add the same provider identity check
(self.provider === sshProvider) alongside the existing remoteHomeResolutionKey
comparison so stale tasks cannot overwrite a new provider's state.
🪄 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: 676769b0-f37b-420f-9d51-af048004dd97
📒 Files selected for processing (3)
Sources/ContentView.swiftSources/FileExplorerStore.swiftcmuxTests/FileExplorerStoreTests.swift
Review feedback found two stale-state paths in the SSH file explorer follow behavior. The selected-workspace observer now derives snapshots from Combine's emitted values instead of re-reading @published properties during willSet, and the SSH home failure path now validates the provider identity before clearing root state. Constraint: Local xcodebuild and local XCUITests are prohibited for this task Rejected: Keep reading workspace properties from the merged signal callback | @published emits before assignment, so isolated property changes can be dropped by removeDuplicates Confidence: high Scope-risk: narrow Tested: git diff --check; jq empty Resources/Localizable.xcstrings Not-tested: Local build/tests deferred to GitHub Actions per workspace policy
CI showed the Combine map closure needs an explicit return after introducing tuple destructuring for emitted @published values. Add the return without changing the observer semantics. Constraint: Local xcodebuild and local tests are prohibited for this task Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: Local build/tests deferred to GitHub Actions per workspace policy
The SSH provider is shared between main-thread root switching and async SSH resolution/listing tasks, so availability and resolved home state now live behind a short lock-protected snapshot. Constraint: FileExplorerStore remains main-thread oriented while SSH subprocess work runs asynchronously. Rejected: MainActor-isolate the provider | would couple remote file listing to UI actor hops instead of protecting the small shared state directly Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: Local XCTest/XCUITest/build per project policy and pending CI
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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/ContentView.swift`:
- Around line 2492-2495: The current cmuxDebugLog call logs sensitive SSH target
fields (config.destination and config.displayTarget); change the log to omit or
mask those values and only emit non-sensitive metadata such as
tab.remoteConnectionState.rawValue, presence flags, byte counts or a boolean
indicating whether destination/displayTarget are present. Locate the
cmuxDebugLog invocation and replace the string to exclude config.destination and
config.displayTarget (or replace them with masked/boolean indicators like
destinationPresent/displayTargetPresent) while keeping the rest of the debug
context.
🪄 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: a95c6da4-bb17-4215-ac8b-bf164f04d6f4
📒 Files selected for processing (1)
Sources/ContentView.swift
Remote file explorer diagnostics must not leak SSH targets, and SSH directory probes must not block Swift's cooperative executor. The transport now keeps cancellation-safe subprocess termination while running blocking I/O on a GCD worker, and the DEBUG sync log emits only presence flags. Constraint: Do not add dependencies or widen the file explorer provider surface while addressing review feedback. Rejected: Keep Task.detached for the SSH subprocess wait | blocking Process and pipe reads can occupy cooperative executor threads Rejected: Log masked SSH host/user strings | presence flags provide enough correlation without retaining target data Confidence: high Scope-risk: narrow Tested: git diff --check; grep confirmed no raw SSH sync target fields and no FileExplorerStore Task.detached usage Not-tested: Local XCTest/XCUITest/build per project policy and pending CI
A root listing can finish successfully after a workspace switch has already cancelled it and marked SSH files unavailable. The success path now cooperates with cancellation before mutating the tree or clearing status, so stale listing results cannot overwrite the current workspace state. Constraint: Do not run local Xcode builds or XCUITests for this repo; CI owns test execution. Rejected: Check only the error path | successful subprocess completion after cancellation was the failing case. Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: local unit test execution, by repository instruction
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f4dd0d4. Configure here.
The process-backed SSH transport already trims command output into a usable path, so the provider only validates the returned home path instead of normalizing it a second time. Rejected: Leave duplicate trimming in place | it blurred the transport/provider boundary and kept a review thread unresolved. Confidence: high Scope-risk: narrow Tested: git diff --check Not-tested: local unit test execution, by repository instruction
Merged the latest origin/main into the feature branch so CI, reviewer bots, and mergeability are evaluated against the current base. The review findings I rechecked are already present in the branch code; this commit only brings in upstream main and resolves stale branch state. Constraint: iterate-pr requires syncing the PR branch with the latest base before CI iteration Rejected: Leave the PR on its older base | would keep branch protection and review automation tied to stale inputs Confidence: high Scope-risk: narrow Directive: Keep this branch evaluated against current main before interpreting skipped or stale CI results Tested: ./scripts/reload.sh --tag issue-3719-files-sidebar-ssh-remote before base sync Not-tested: Tagged rebuild and post-push CI are pending after this amended merge commit
austinywang
dismissed
coderabbitai[bot]’s stale review
May 8, 2026 20:49
Stale CodeRabbit change request after addressed feedback; latest CodeRabbit, Greptile, Cursor, CircleCI, GitHub Actions, Vercel, and Socket checks pass on head 9ee803c.
Merged current origin/main into the SSH Files sidebar branch and resolved the FileExplorerView conflict by preserving both sides: the new right-sidebar Find AppKit refresh path from main and this branch's SSH root status message in the empty-state visibility path. Constraint: GitHub reported the PR could not merge because Sources/FileExplorerView.swift conflicted with current main Rejected: Keep origin/main's two-argument updateVisibility call | would drop SSH unavailable/resolving status messages from the Files sidebar Rejected: Keep only the branch version without current main | would leave the PR conflicted and miss the Find typing-lag UI updates Confidence: high Scope-risk: narrow Directive: FileExplorerView updateVisibility must continue receiving rootStatusMessage so SSH unavailable states remain visible Tested: ./scripts/reload.sh --tag issue-3719-files-sidebar-ssh-remote Not-tested: Local XCTest suite per repository policy; CI will run after push
2 of 6 tasks
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
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.
Fixes #3719.
Summary
$HOME.ssh://header/tooltip for the active target.Verification
git diff --cached --checkjq empty Resources/Localizable.xcstringsNot run locally
xcodebuildwas run.Note
Medium Risk
Touches file-explorer root selection and SSH subprocess execution/cancellation; bugs here can cause missing/incorrect file trees or stuck status messages, but changes are scoped and covered by new tests.
Overview
The Files sidebar now routes all root changes through a single
applyWorkspaceRootAPI so local tabs track their local cwd while SSH workspaces resolve and display the remote$HOME(instead of reusing a stale macOS path).SSH file browsing is refactored to use an injectable
SSHFileExplorerTransportwith cancellation-safe process handling, and the UI surfaces inline, localized status while SSH home is resolving or unavailable (includingssh://header display + tooltips). New unit tests cover remote-home resolution, switching local→remote behavior, and ensuring cancelled loads don’t clear an SSH-unavailable status.Reviewed by Cursor Bugbot for commit 367b982. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Makes the Files sidebar follow the focused SSH workspace by resolving the remote $HOME and only loading files when connected. Fixes #3719; clears stale local paths, shows clear status while resolving or unavailable, preserves status after cancelled loads, and syncs with recent sidebar UI changes from
main.New Features
applyWorkspaceRootAPI; resolve SSH roots via an injectable transport; clear the tree while pending/unavailable; showssh://<target>(and:/<path>once resolved) in the header and tooltips.Bug Fixes
Written for commit 367b982. Summary will update on new commits.
Summary by CodeRabbit