Open in native app - #53
Merged
Merged
Conversation
mutewinter
force-pushed
the
worktree-open-in-native-app
branch
from
July 9, 2026 19:20
380fccf to
09260ca
Compare
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds cross-platform task-file opening with default or selected applications. Electron resolves app targets and candidates, exposes RPC handlers, and client hooks/components replace reveal-in-folder actions across menus, previews, fallbacks, and the file viewer. ChangesOpen task file feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant FileViewer
participant useOpenTaskFile
participant rpcClient
participant utilsRoute
participant fileOpenTarget
FileViewer->>useOpenTaskFile: openTaskFile(file)
useOpenTaskFile->>rpcClient: call utils.openTaskFile
rpcClient->>utilsRoute: send task-file request
utilsRoute->>fileOpenTarget: resolve file path
utilsRoute->>utilsRoute: open file with default app
utilsRoute-->>rpcClient: return success or error
rpcClient-->>useOpenTaskFile: return mutation result
sequenceDiagram
participant OpenWithMenu
participant OpenWithCandidates
participant useTaskFileOpenCandidates
participant useOpenTaskFileWith
OpenWithMenu->>OpenWithCandidates: mount submenu
OpenWithCandidates->>useTaskFileOpenCandidates: fetch candidate apps
useTaskFileOpenCandidates-->>OpenWithCandidates: return app list
OpenWithCandidates->>useOpenTaskFileWith: open selected app
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
apps/studio/src/client/components/file-preview-card.tsx (1)
374-389: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffOpen-target resolution fires eagerly for every rendered media card.
Unlike
FileRowCard's hover-based prefetch,useFileActionsresolves the open target unconditionally as soon asImagePreviewCard/VideoPreviewCardmount. In a grid with many files this could fan out several concurrent Electron IPC calls at once. Per-extension caching likely limits repeat cost, but consider verifying this is acceptable for typical grid sizes, or gate resolution similarly to the row's hover-prefetch pattern.🤖 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 `@apps/studio/src/client/components/file-preview-card.tsx` around lines 374 - 389, `useFileActions` currently calls `useTaskFileOpenTarget(file)` eagerly for every mounted media card, which can trigger many simultaneous open-target resolutions in grid views. Adjust the `ImagePreviewCard`/`VideoPreviewCard` path to avoid unconditional resolution, either by gating it behind a user interaction or by reusing the same hover-prefetch approach as `FileRowCard`, and keep `showOpen` driven by the resolved target state from `useTaskFileOpenTarget`.
🤖 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 `@apps/studio/src/client/components/file-actions-menu.tsx`:
- Around line 89-90: The open-visibility logic is duplicated across
file-actions-menu.tsx, useFileActions in file-preview-card.tsx, and
file-viewer.tsx, so update the shared source of truth instead of recomputing it
in each consumer. Fold the `showOpen`/`showOpenWith` derivation into
`useTaskFileOpenTarget` (or a small wrapper around it) and have
`file-actions-menu`, `useFileActions`, and `file-viewer` consume that shared
result so the visibility rules stay consistent in one place.
- Around line 83-90: The dropdown can render an empty menu when a file only has
the task-based Open action because the local showOpen state from
useTaskFileOpenTarget is not included in hasFileActions, causing the
FileActionsMenu early return to null. Update the FileActionsMenu rendering guard
so it considers showOpen alongside the other action flags, and keep the
trigger/button visibility logic aligned with the same Open state used by the
menu content.
In `@apps/studio/src/client/components/file-preview-card.tsx`:
- Around line 158-161: The `hasFileActions` guard in `file-preview-card.tsx` is
missing `fileActions.showOpen`, which causes open-only files to lose both the
actions trigger and the context menu. Update the `hasFileActions` calculation in
`FilePreviewCard` to include `showOpen` so the "..." button and `ContextMenu`
still render when Open is the only available action.
In `@apps/studio/src/electron-main/lib/file-open-target.ts`:
- Around line 319-330: The helper-process lookup path in file-open-target.ts can
hang indefinitely because the cached promises in inFlightTargets,
sessionTargets, and candidatesCache never settle if osascript, xdg-mime, or
powershell stalls. Add explicit timeouts around the lookup helpers used by these
caches, and make sure timed-out calls reject so the cached promise entries are
cleared and future lookups can retry. Apply the timeout handling in the lookup
flow that builds candidate targets and returns the cached results.
- Around line 211-227: loadDiskCache is assigning diskCache before the async
file read/parsing finishes, which can let concurrent callers see and mutate an
empty cache. Move the shared cache assignment to after the persisted cache has
been fully read and validated, and make sure loadDiskCache either returns the
already-populated Map or initializes it once from the parsed data so late
completion cannot overwrite newer entries.
In `@apps/studio/src/electron-main/rpc/routes/utils.ts`:
- Around line 337-364: The route in utils.ts currently accepts any absolute app
path even though execFile is used safely; before calling the open launcher in
the handler for the file-open flow, validate input.appPath against
getFileOpenCandidates(fullPath) and reject unauthorized apps with an existing
error path. Use the surrounding handler logic, resolvePathWithinTaskDir, and the
execFileAsync("open", ["-a", ...]) call site to add the check just before
launch.
---
Nitpick comments:
In `@apps/studio/src/client/components/file-preview-card.tsx`:
- Around line 374-389: `useFileActions` currently calls
`useTaskFileOpenTarget(file)` eagerly for every mounted media card, which can
trigger many simultaneous open-target resolutions in grid views. Adjust the
`ImagePreviewCard`/`VideoPreviewCard` path to avoid unconditional resolution,
either by gating it behind a user interaction or by reusing the same
hover-prefetch approach as `FileRowCard`, and keep `showOpen` driven by the
resolved target state from `useTaskFileOpenTarget`.
🪄 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: ae43e33a-e999-43a3-b316-b415c28d4656
📒 Files selected for processing (13)
apps/studio/src/client/components/file-actions-menu.tsxapps/studio/src/client/components/file-preview-card.tsxapps/studio/src/client/components/file-preview-fallback.tsxapps/studio/src/client/components/file-viewer.tsxapps/studio/src/client/components/open-target-icon.tsxapps/studio/src/client/components/open-with-menu.tsxapps/studio/src/client/hooks/use-file-action-visibility.tsapps/studio/src/client/hooks/use-open-task-file.tsapps/studio/src/client/hooks/use-task-file-open-target.tsapps/studio/src/electron-main/lib/file-open-target.tsapps/studio/src/electron-main/rpc/initialize.tsapps/studio/src/electron-main/rpc/routes/utils.tsapps/studio/src/vite-env.d.ts
Comment on lines
+89
to
+90
| const showOpen = isPending || appName != null; | ||
| const showOpenWith = appName != null && isMacOS(); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Consolidate duplicated open-visibility derivation.
showOpen/showOpenWith are independently recomputed with identical logic in file-actions-menu.tsx, file-preview-card.tsx (useFileActions), and file-viewer.tsx. Consider folding these into useTaskFileOpenTarget (or a small wrapper) so all consumers share one source of truth — this would have caught the early-return bug above before it shipped.
🤖 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 `@apps/studio/src/client/components/file-actions-menu.tsx` around lines 89 -
90, The open-visibility logic is duplicated across file-actions-menu.tsx,
useFileActions in file-preview-card.tsx, and file-viewer.tsx, so update the
shared source of truth instead of recomputing it in each consumer. Fold the
`showOpen`/`showOpenWith` derivation into `useTaskFileOpenTarget` (or a small
wrapper around it) and have `file-actions-menu`, `useFileActions`, and
`file-viewer` consume that shared result so the visibility rules stay consistent
in one place.
Contributor
Author
|
@coderabbitai review |
mutewinter
force-pushed
the
worktree-open-in-native-app
branch
2 times, most recently
from
July 10, 2026 22:34
af0fe61 to
f5a1d65
Compare
- openTaskFile + getTaskFileIcon RPC handlers (shell.openPath, app.getFileIcon) - Open action with the native file icon in shared file menus and viewer header - image/video hover overlays swap reveal for open; reveal stays in context menus
- per-platform default-app resolver (NSWorkspace via JXA, registry via
PowerShell, xdg-mime + .desktop) cached per extension in the main process
- menus, viewer header, and preview fallback label the action "Open in {app}"
with the real app icon; file-type icon when the app can't be resolved
- preview-not-available promotes open over download when an app is known
- resolve the real associated app (name + icon) via NSWorkspace/registry
instead of the file-type icon; persist per-extension across runs (SWR TTL)
and warm on hover so the affordance appears only once ready
- add an "Open with" submenu listing every capable app (macOS), plus an
openTaskFileWith handler; gate open on a resolved app so unopenable types
(no association) don't offer a failing action
- treat "no app for this file" as an expected condition: quiet the RPC
exception capture and toast instead of logging a stack
- move Download into the file-viewer overflow menu behind Open in {app}
- fix header overlap: filename flexes/truncates, actions stay pinned
mutewinter
force-pushed
the
worktree-open-in-native-app
branch
from
July 15, 2026 18:23
f5a1d65 to
d06b8f6
Compare
mutewinter
marked this pull request as ready for review
July 15, 2026 18:24
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/studio/src/client/components/file-viewer.tsx`:
- Around line 419-431: Add aria-label={openLabel} to the Open button rendered in
the showOpen branch, preserving the existing visible label and click behavior
while providing an accessible name when the label is hidden.
In `@apps/studio/src/electron-main/lib/file-open-target.ts`:
- Around line 411-419: Preserve the distinction between resolver errors and
confirmed absence: in resolveTarget, propagate all resolveAssociatedApp failures
instead of converting non-timeout errors to null; platform resolvers already
return null when nothing is found. Also update the candidate-resolution catch at
apps/studio/src/electron-main/lib/file-open-target.ts lines 359-363 to propagate
process, parse, and schema errors so its rejection handler removes
candidatesCache; the script’s empty-array result already represents no
applications.
🪄 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: 548fbf00-2be9-43d1-a43f-d3d35bff734d
📒 Files selected for processing (13)
apps/studio/src/client/components/file-actions-menu.tsxapps/studio/src/client/components/file-preview-card.tsxapps/studio/src/client/components/file-preview-fallback.tsxapps/studio/src/client/components/file-viewer.tsxapps/studio/src/client/components/open-target-icon.tsxapps/studio/src/client/components/open-with-menu.tsxapps/studio/src/client/hooks/use-file-action-visibility.tsapps/studio/src/client/hooks/use-open-task-file.tsapps/studio/src/client/hooks/use-task-file-open-target.tsapps/studio/src/electron-main/lib/file-open-target.tsapps/studio/src/electron-main/rpc/initialize.tsapps/studio/src/electron-main/rpc/routes/utils.tsapps/studio/src/vite-env.d.ts
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/studio/src/client/hooks/use-file-action-visibility.ts
- apps/studio/src/vite-env.d.ts
- apps/studio/src/client/hooks/use-open-task-file.ts
- apps/studio/src/client/components/file-preview-fallback.tsx
- apps/studio/src/client/components/open-target-icon.tsx
- apps/studio/src/client/components/open-with-menu.tsx
- apps/studio/src/client/hooks/use-task-file-open-target.ts
- apps/studio/src/client/components/file-actions-menu.tsx
- apps/studio/src/electron-main/rpc/initialize.ts
- apps/studio/src/client/components/file-preview-card.tsx
- apps/studio/src/electron-main/rpc/routes/utils.ts
This was referenced Jul 16, 2026
This was referenced Jul 24, 2026
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.
Summary by CodeRabbit