Repository navigation
Conversation
…evAI#5850, ParadevAI#5893) In shipped builds `external.openInApp` (an Electron-IPC call typed to the desktop AppRouter) can be serviced by the host service instead of the desktop main process. The host service router has no `external` key, so the call fails with `No procedure found on path "external.openInApp"` and the "Open in app" button / CMD+O silently break. Add a locality-guarded `external` router to the host service so the misrouted call succeeds, but only when the host is the caller's own machine (`ctx.clientMachineId === getHostId()` — the same equality the desktop uses for `isLocalWorkspace`). Remote hosts reject with PRECONDITION_FAILED rather than opening an editor on the wrong machine. Scope is limited to the reported actions (openInApp, openInFinder, openUrl). The desktop path is untouched. This is a mitigation; the root-cause fix belongs on the desktop transport so `external.*` reaches the main-process router.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds host-service support for local-only external actions. The new router opens applications, reveals files, and opens allowlisted URLs using platform-specific commands, with path resolution, process execution helpers, router registration, and validation tests. ChangesExternal host actions
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant HostService
participant externalRouter
participant OS
Caller->>HostService: external.openInApp
HostService->>externalRouter: validate local machine and path
externalRouter->>OS: launch application or reveal file
OS-->>externalRouter: process result
externalRouter-->>Caller: mutation response
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
hknakn
marked this pull request as ready for review
July 23, 2026 16:21
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@packages/host-service/src/trpc/router/external/external.ts`:
- Around line 127-131: Update the openInFinder mutation to validate that its
input is an absolute path before calling revealInFileManager, rejecting relative
paths while preserving valid absolute-path behavior. Add a matching test
covering rejection of relative input and ensuring revealInFileManager is not
invoked.
- Around line 55-57: Update the Windows branch in the target-opening function to
avoid passing the caller-controlled target to cmd.exe via spawnAsync. Use a
direct Windows shell-association executable or API that opens the target without
shell command parsing, while preserving the existing target-opening behavior for
valid URLs, including mailto links.
In `@packages/host-service/src/trpc/router/external/helpers.ts`:
- Around line 69-71: Update the Linux fallback in the helper that invokes
xdg-open so it passes nodePath.dirname(targetPath) rather than targetPath,
ensuring files open their containing directory while preserving existing
behavior for directories.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b66cf52-8a90-4a9c-8e1f-afca10007d01
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
packages/host-service/package.jsonpackages/host-service/src/trpc/router/external/external.test.tspackages/host-service/src/trpc/router/external/external.tspackages/host-service/src/trpc/router/external/helpers.tspackages/host-service/src/trpc/router/router.ts
Address CodeRabbit review on ParadevAI#5900: - Drop the win32 branches so targets are never routed through cmd.exe (which parses `&`/`|` in a URL or path as command separators). Only macOS and Linux are handled, matching the desktop's getAppCommand platform support. - Guard openInFinder against relative paths, like openInApp (extracted a shared assertAbsolutePath helper). - On Linux, reveal a path by opening its containing directory instead of xdg-open'ing the file itself (which would launch its default app). Add a test for the openInFinder absolute-path guard.
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
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.
Problem
On shipped builds, the "Open in app" button /
CMD+Ofail with:(#5850, #5893). The renderer calls
external.openInAppthrough the Electron-IPC client typed to the desktopAppRouter, but in the running build that call is serviced by the host service rather than the desktop main process. The host-service router has noexternalkey, so it throwsNo procedure found on path "external.openInApp". The error string is emitted only by the host-service tRPC stack, confirming that's where the call lands.The misroute is a transport-level issue that isn't visible in the renderer source (every
external.*call site usesipcLink), which is why it hasn't been reproducible from source alone. Users confirm it still reproduces on the current stable, on local project dashboards.Solution
Add a locality-guarded
externalrouter to the host service so the misrouted call succeeds — but only when the host is the caller's own machine (ctx.clientMachineId === getHostId(), the same equality the desktop uses forisLocalWorkspace). For a remote host these actions are meaningless (they'd open an editor on the server), so they reject withPRECONDITION_FAILEDinstead of doing the wrong thing.openInApp,openInFinder,openUrl.open/xdg-open/explorer) — no Electron; theno-electron-couplingguard still passes.This is a mitigation, not the root-cause fix. The correct fix belongs on the desktop transport so
external.*always reaches the main-process router; this makes the local case work in the meantime and stays safe for remote hosts.Verification
bun run typecheck(host-service) — cleanbiome check— cleanbun run build:host— bundles;openInAppnow present indist/host-service.jsexternal.test.ts, 4 cases): guard rejects remote / missing machine id; rejects relative paths and disallowed URL schemesno-electron-couplingtest still passesRelated
No procedure found on path "external.openInApp"#5893 (@bvidakovic).Summary by CodeRabbit
:line[:col]references.