Repository navigation
fix(shell): OS open actions follow the file's owner, not the focused server - #27165
Conversation
…server Every shell open request (openPath, openInFileManager, openInExternalEditor, openFilePath, openFileUri) now carries the path's owning host. Main allows only a local owner, plus the SSH editor launch when the owner is that same SSH target, and no longer consults the focused server. The renderer reveal and Open-in checks drop their focused-server check to match.
📝 Walkthrough
Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Path-opening workflows appear mergeable. A malformed SSH editor request could fail without a structured response, but known callers do not send such a request. Pre-merge checks |
  |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 50 files. (4 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
-
Check name Status Explanation Linked Issues check Issue #16539is closed and completed. It supplies historical context only. No active, directly linked issue supplies coding requirements for this pull request.Out of Scope Changes check The changes stay within the stated shell-owner routing scope. They update main-process checks, preload types, renderer owner resolution, affected callers, and related automated tests. The AI Vault, sk… Title check The title clearly and concisely summarizes the main change: shell open actions now follow the file owner instead of the focused server. Description check The description covers the required sections, explains the user impact and implementation, links issue #16539, documents testing and limitations, and addresses remote, SSH, cross-platform, and compati…
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 42 functions across 50 files. (4 skipped: 4 over the file limit.)
✨ Finishing Touches 💡 1
-
-
📝 Generate docstrings 💡
-
-
- Commit to this branch
- Create a new PR
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
…only SSH workspaces The explorer toolbar and the source-control Open in menu took their SSH connection from the repo row, which a folder workspace or a host-only SSH repo does not have, so the route read as local. Both now derive the route from the catalog-confirmed owner.
…H rows The row resolver now returns the SSH connection it resolved alongside the runtime owner, so a folder workspace or host-only SSH repo with no repo connection no longer reads as local.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔇 Additional comments (55)
src/renderer/src/components/skills/SkillsPage.tsx (1)
10-11: LGTM!Also applies to: 353-355
src/renderer/src/components/skills/SkillsList.tsx (1)
2-2: LGTM!Also applies to: 4-4, 31-31, 45-46, 66-74, 144-148, 194-194
src/renderer/src/components/skills/SkillDetailDialog.tsx (1)
62-63: LGTM!Also applies to: 75-75, 149-149
src/renderer/src/components/skills/SkillRow.tsx (1)
56-56: LGTM!Also applies to: 79-79, 113-113
src/renderer/src/components/right-sidebar/FileExplorer.tsx (1)
4-5: LGTM!Also applies to: 57-63, 265-265
src/renderer/src/components/sidebar/WorktreeOpenInMenu.tsx (1)
11-15: LGTM!Also applies to: 57-92, 116-155, 208-213
src/renderer/src/components/sidebar/WorktreeRowOpenInSubMenu.tsx (1)
23-23: LGTM!src/renderer/src/lib/external-editor-open-capability.ts (1)
7-14: LGTM!src/renderer/src/lib/external-editor-open-capability.test.ts (1)
7-41: LGTM!src/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsx (1)
144-144: LGTM!Also applies to: 166-167, 174-194, 230-231, 240-240, 261-273, 290-291, 349-349, 359-359
src/renderer/src/components/right-sidebar/source-control-entry-context-menu.test.tsx (1)
16-19: LGTM!Also applies to: 76-76, 131-133, 165-171, 196-216
src/renderer/src/components/right-sidebar/source-control/listing/entry-context-menu.tsx (1)
1-1: LGTM!Also applies to: 19-22, 63-67, 92-94, 104-108, 144-146
src/renderer/src/lib/local-path-open-guard.test.ts (1)
3-7: LGTM!Also applies to: 41-59
src/renderer/src/lib/worktree-row-open-in-owner.ts (1)
1-3: LGTM!Also applies to: 14-36, 49-52, 61-64
src/renderer/src/lib/worktree-row-open-in-owner.test.ts (1)
38-38: LGTM!Also applies to: 46-48, 61-61, 73-106, 112-112
src/renderer/src/lib/reveal-in-file-manager.ts (1)
2-12: LGTM!Also applies to: 36-57
src/renderer/src/lib/reveal-in-file-manager.test.ts (1)
4-4: LGTM!Also applies to: 29-52, 70-72, 76-86, 91-91, 96-99, 109-109
src/renderer/src/components/editor/EditorPanelHeaderPath.tsx (1)
19-22: LGTM!Also applies to: 54-61, 220-220
src/renderer/src/components/editor/EditorPanelHeaderPath.test.tsx (1)
14-18: LGTM!Also applies to: 276-276
src/renderer/src/components/right-sidebar/file-explorer-row-context-menu.tsx (1)
33-36: LGTM!Also applies to: 123-129, 280-280
src/renderer/src/components/right-sidebar/file-explorer-row-context-menu.test.tsx (1)
16-18: LGTM!Also applies to: 23-26, 146-158
src/renderer/src/components/right-sidebar/FileExplorerToolbar.tsx (1)
21-21: LGTM!Also applies to: 43-43, 182-182
src/renderer/src/components/right-sidebar/AiVaultPanel.tsx (1)
35-35: LGTM!Also applies to: 396-397
src/renderer/src/components/right-sidebar/ai-vault-session-path-actions.ts (1)
8-8: LGTM!Also applies to: 40-49
src/renderer/src/components/right-sidebar/ai-vault-session-path-actions.test.ts (1)
1-11: LGTM!Also applies to: 63-78
src/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsx (1)
29-32: LGTM!Also applies to: 105-111, 251-251
src/renderer/src/components/terminal-pane/terminal-file-link-actions.test.ts (1)
200-200: LGTM!Also applies to: 217-217, 271-275
src/renderer/src/components/terminal-pane/terminal-file-link-actions.ts (1)
19-21: LGTM!Also applies to: 128-142
src/renderer/src/components/right-sidebar/FileExplorerRow.actions.test.tsx (1)
234-234: LGTM!src/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsx (1)
110-110: LGTM!Also applies to: 120-121, 412-418
src/renderer/src/lib/local-path-open-guard.ts (1)
41-54: LGTM!src/shared/shell-open-types.ts (1)
1-8: LGTM!src/preload/api/shell-api.ts (1)
1-26: LGTM!src/preload/api/shell-bridge.ts (1)
7-31: LGTM!src/main/ipc/shell.ts (2)
20-20: LGTM!Also applies to: 50-55, 61-63, 117-119, 132-135, 191-194, 223-223
84-86: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
⚠️ Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.Validate
connectionIdandownerHostIdtypes before the SSH owner comparison.The handler treats the renderer request as untrusted, but
request.connectionId?.trim()throws ifconnectionIdis a non-string truthy value. The strict comparison onownerHostIdis safe. The local path usesisLocallyOwned, which checks types. This is an inconsistent boundary check. The throw only rejects the IPC call, so impact is low. Consider coercing withtypeof request.connectionId === 'string'.Source: Learnings
src/main/ipc/shell.test.ts (1)
843-893: LGTM!src/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.ts (1)
1-1: LGTM!Also applies to: 247-247, 271-274
src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.ts (1)
107-107: LGTM!src/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.ts (1)
1-1: LGTM!Also applies to: 80-80
src/renderer/src/components/editor/MediaViewer.tsx (1)
1-1: LGTM!Also applies to: 74-76
src/renderer/src/components/editor/markdown-preview-link-actions.ts (1)
1-1: LGTM!Also applies to: 158-158
src/renderer/src/components/editor/markdown-preview-system-link-action.ts (1)
1-1: LGTM!Also applies to: 80-80, 84-84
src/renderer/src/components/editor/rich-markdown-editor-click-routing.ts (1)
22-25: LGTM!Also applies to: 261-268
src/renderer/src/components/editor/rich-markdown-editor-click-routing.test.ts (1)
191-193: LGTM!Also applies to: 204-206
src/renderer/src/components/right-sidebar/file-explorer-row-file-transfer.ts (1)
1-1: LGTM!Also applies to: 59-59
src/renderer/src/components/settings/CliSection.tsx (1)
1-1: LGTM!Also applies to: 297-301
src/renderer/src/components/settings/KeybindingsFileActions.tsx (1)
1-1: LGTM!Also applies to: 133-137
src/renderer/src/components/terminal-pane/terminal-file-open-routing.ts (1)
19-19: LGTM!Also applies to: 176-179, 187-187
src/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.ts (1)
146-146: LGTM!Also applies to: 313-314, 559-560
src/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.ts (1)
234-234: LGTM!Also applies to: 250-250
src/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.ts (1)
173-173: LGTM!src/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.ts (1)
97-97: LGTM!src/renderer/src/components/terminal-pane/terminal-remote-file-download-open.ts (1)
1-1: LGTM!Also applies to: 25-25
tests/e2e/ssh-orcad-open-in-owner.spec.ts (1)
76-80: LGTM!
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
f0cf5864-7963-4de8-a6d4-da704af0534a
📒 Files selected for processing (54)
src/main/ipc/shell.test.tssrc/main/ipc/shell.tssrc/preload/api/shell-api.tssrc/preload/api/shell-bridge.tssrc/renderer/src/components/browser-pane/navigate/use-browser-page-navigation-downloads.tssrc/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.test.tssrc/renderer/src/components/browser-pane/workspace-doc/doc-preview-document-actions.tssrc/renderer/src/components/editor/EditorPanelHeaderPath.test.tsxsrc/renderer/src/components/editor/EditorPanelHeaderPath.tsxsrc/renderer/src/components/editor/MediaViewer.tsxsrc/renderer/src/components/editor/markdown-preview-link-actions.tssrc/renderer/src/components/editor/markdown-preview-system-link-action.tssrc/renderer/src/components/editor/rich-markdown-editor-click-routing.test.tssrc/renderer/src/components/editor/rich-markdown-editor-click-routing.tssrc/renderer/src/components/right-sidebar/AiVaultPanel.tsxsrc/renderer/src/components/right-sidebar/FileExplorer.tsxsrc/renderer/src/components/right-sidebar/FileExplorerRow.actions.test.tsxsrc/renderer/src/components/right-sidebar/FileExplorerToolbar.tsxsrc/renderer/src/components/right-sidebar/ai-vault-session-path-actions.test.tssrc/renderer/src/components/right-sidebar/ai-vault-session-path-actions.tssrc/renderer/src/components/right-sidebar/file-explorer-row-context-menu.test.tsxsrc/renderer/src/components/right-sidebar/file-explorer-row-context-menu.tsxsrc/renderer/src/components/right-sidebar/file-explorer-row-file-transfer.tssrc/renderer/src/components/right-sidebar/source-control-entry-context-menu.test.tsxsrc/renderer/src/components/right-sidebar/source-control/listing/entry-context-menu.tsxsrc/renderer/src/components/settings/CliSection.tsxsrc/renderer/src/components/settings/KeybindingsFileActions.tsxsrc/renderer/src/components/sidebar/WorktreeOpenInMenu.test.tsxsrc/renderer/src/components/sidebar/WorktreeOpenInMenu.tsxsrc/renderer/src/components/sidebar/WorktreeRowOpenInSubMenu.tsxsrc/renderer/src/components/skills/SkillDetailDialog.tsxsrc/renderer/src/components/skills/SkillRow.tsxsrc/renderer/src/components/skills/SkillsList.tsxsrc/renderer/src/components/skills/SkillsPage.tsxsrc/renderer/src/components/tab-bar/EditorFileTabContextMenu.test.tsxsrc/renderer/src/components/tab-bar/EditorFileTabContextMenu.tsxsrc/renderer/src/components/terminal-pane/terminal-file-link-actions.test.tssrc/renderer/src/components/terminal-pane/terminal-file-link-actions.tssrc/renderer/src/components/terminal-pane/terminal-file-open-routing.tssrc/renderer/src/components/terminal-pane/terminal-link-click-fallback.test.tssrc/renderer/src/components/terminal-pane/terminal-link-file-open-routing.test.tssrc/renderer/src/components/terminal-pane/terminal-link-osc-file-targets.test.tssrc/renderer/src/components/terminal-pane/terminal-link-worktree-root-activation.test.tssrc/renderer/src/components/terminal-pane/terminal-remote-file-download-open.tssrc/renderer/src/lib/external-editor-open-capability.test.tssrc/renderer/src/lib/external-editor-open-capability.tssrc/renderer/src/lib/local-path-open-guard.test.tssrc/renderer/src/lib/local-path-open-guard.tssrc/renderer/src/lib/reveal-in-file-manager.test.tssrc/renderer/src/lib/reveal-in-file-manager.tssrc/renderer/src/lib/worktree-row-open-in-owner.test.tssrc/renderer/src/lib/worktree-row-open-in-owner.tssrc/shared/shell-open-types.tstests/e2e/ssh-orcad-open-in-owner.spec.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
ELI5
Orca can show a file in Finder / File Explorer, open it in an editor, or open it with the default app. Those actions only make sense for files on this computer. Until now the app decided "is this local?" partly by asking which paired server you last focused. Now it asks the machine that actually owns the file, and the main process checks the same thing.
What Changed
Before (as a user):
After: local files reveal and open whatever server is focused. Files on an SSH host or a paired server are refused (SSH projects keep "Open in VS Code" over Remote-SSH). If Orca cannot tell which machine owns a file, the action is refused instead of guessed.
Mechanism:
openPath,openInFileManager,openInExternalEditor,openFilePathandopenFileUri. The main process allows alocalowner, allows an SSH owner only for the editor's Remote-SSH launch and only when the owner is that same SSH target, and refuses anything else, including a missing owner. The check that read the focused server (hasActiveRuntime) is deleted.isRevealInFileManagerBlocked(settings, …)is replaced bygetWorkspaceFileRevealOwner. It takes the owner from the route, and when the route looks local it confirms that against the workspace's catalog row, using the same resolver the Markdown link guard uses.getExternalEditorOpenCapabilityandgetOpenInEntryAvailabilityno longer takesettings.revealInFileManager(path, owner)refuses non-local owners before it calls IPC.getRouteForLocalPathOpenOwnerturns an owner back into the route those menus take, and the sidebar row resolver now returns the SSH target it resolved.local: browser downloads, saved downloads, the CLI path, the keybindings file, and the Markdown/terminal/media/doc-preview opens that already passed a local-only check. AI Vault passes the session's own host. The skills list passes the host it scanned, and its two duplicate reveal handlers are merged into one inSkillsList.Why
The focused server tells you nothing about where a file lives. It was blocking local files and protecting nothing for remote ones. S8a (#27144) moved the renderer guard to an owner but had to keep the focused-server check, because main still refused on focus. This PR moves main to the owner and then removes the check, so menu items and clicks agree.
I made the owner a required argument instead of an optional one. That way the compiler found every caller, and an old call shape cannot silently fall back to "local". Alternatives considered:
Renderer and main ship together, so nothing changes on the wire between paired clients and servers.
Linked Issue
Fixes #16539
Refs #13929. This PR does not add a remote editor launch for paired-server workspaces. It makes local workspaces open while a server is focused and keeps refusing server-owned paths with a clear reason.
Supersedes #25932 and #16559 (their reveal-while-focused tests are carried here in owner form). Thanks to both authors.
Visual Proof
N/A. No new UI. Menu items that used to be disabled for local files while a server was focused are now enabled, and the existing "Local only" hint and refusal toast still show for remote files. Automated tests cover each state.
Testing
pnpm tc,oxlintandpnpm run check:code-quality:changedpass.Focused
pnpm testoversrc/main/ipc/shell.test.tsand the rendererlib,sidebar,right-sidebar,editor,tab-bar,terminal-pane,skills,settings,browser-paneandwebfolders: 23651 passed. The remaining failures also fail without this change:SortableTab.rename-shortcut.test.tsx(missingCopyin a lucide mock) and a palette performance budget that is sensitive to load.Acceptance tests that fail on origin/main and pass here (I ran the new tests against the main source files: 18 failures):
shell.test.ts› owner routing: a local owner reveals, opens and launches the editor while a server is focused. SSH, server, missing and malformed owners are refused by all five handlers before any filesystem access. An SSH editor launch requires the owner to matchconnectionId.ai-vault-session-path-actions.test.ts: an SSH session log reveal is refused, and a local one is revealed.source-control-entry-context-menu.test.tsxandworktree-row-open-in-owner.test.ts: a repo the catalog places on an SSH host, a host-only SSH row, and a direct-SSH folder workspace row route to their SSH target (reveal refused, VS Code gets Remote-SSH), even when the repo row has no connection.SSH: SSH-owned files stay refused for reveal and default-app open. "Open in VS Code" over Remote-SSH still works, and main now also checks that the owner matches the connection.
Folder workspaces: the catalog check resolves folder rows through the folder/project-group owner, and there is a test for revealing a local folder workspace.
Windows/WSL: path validation in main is unchanged (native
isAbsoluteandstat, and SSH paths accept both POSIX and Windows forms). A local WSL project's UNC path has alocalowner, so it reveals as before.I manually tested these changes locally
Automated tests added/updated, or explained why not below
Review
Agent skill upstream boundary
Notes
The main process now refuses an owner-less shell open. Every renderer caller was updated in this PR, and the web client's shell stub ignores the new argument.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)