feat(desktop): per-project default IDE for Open in actions#1598
Conversation
Move preferred IDE from global settings.lastUsedApp to per-project projects.defaultApp so each project remembers its own editor choice independently. Resolution: project.defaultApp → "cursor" fallback.
📝 WalkthroughWalkthroughThis PR migrates from global last-used app tracking stored in settings to per-project default app management in the projects table. It adds a Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as UI Component
participant TRPC as TRPC Router
participant DB as Database
participant Cache as React Query Cache
User->>UI: Click "Open in App"
UI->>TRPC: getDefaultApp({ projectId })
TRPC->>DB: SELECT projects WHERE id = projectId
DB-->>TRPC: defaultApp value
TRPC-->>UI: defaultApp
UI->>Cache: Update getDefaultApp cache
User->>UI: Select app to open file
UI->>TRPC: openInApp({ projectId, app, path })
TRPC->>DB: UPDATE projects SET default_app = app WHERE id = projectId
DB-->>TRPC: Success
TRPC-->>UI: Result
UI->>Cache: Invalidate getDefaultApp cache for projectId
Cache-->>UI: Cache cleared
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx (1)
51-56:⚠️ Potential issue | 🟠 MajorSame
workspace.project?.idvsworkspace?.projectIdinconsistency — apply the same fix.
OpenInMenuButtonreceivesworkspace.project?.idhere. If theprojectrelation is not eagerly included in theworkspaces.getresponse, this will beundefinedwhileworkspace.projectIdhas the correct value (see root-cause comment inFilesView.tsx). Replace withworkspace?.projectIdfor consistency and correctness.🛠️ Proposed fix
<OpenInMenuButton worktreePath={workspace.worktreePath} branch={workspace.worktree?.branch} - projectId={workspace.project?.id} + projectId={workspace.projectId} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx` around lines 51 - 56, Replace the inconsistent prop passed to OpenInMenuButton: currently it uses workspace.project?.id which can be undefined if the project relation isn't loaded; change it to pass workspace?.projectId instead. Update the JSX where OpenInMenuButton is rendered (the component and props: worktreePath, branch, projectId) to use workspace?.projectId so the component receives the correct ID even when relations are not eagerly loaded.
🧹 Nitpick comments (8)
apps/desktop/src/renderer/routes/_authenticated/settings/components/ClickablePath/ClickablePath.tsx (1)
30-30: Extract the"cursor"fallback to a shared constant to avoid silent divergence.The PR description notes the project-level fallback is also
"cursor". Having the literal in two separate locations means a future change to the fallback could be applied in one place and silently missed here (or vice versa). A single exported constant (e.g.,DEFAULT_APP) imported in both this component and the project-default resolution logic would prevent that drift.♻️ Suggested refactor
// e.g. in renderer/components/OpenInButton/constants.ts (or wherever APP_OPTIONS lives) +export const DEFAULT_APP: ExternalApp = "cursor";-import { +import { APP_OPTIONS, + DEFAULT_APP, JETBRAINS_OPTIONS, VSCODE_OPTIONS, } from "renderer/components/OpenInButton"; -const defaultApp: ExternalApp = "cursor"; +const defaultApp = DEFAULT_APP;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/components/ClickablePath/ClickablePath.tsx` at line 30, The literal fallback "cursor" is duplicated as const defaultApp: ExternalApp = "cursor"; — extract that literal into a single exported constant (e.g., DEFAULT_APP or DEFAULT_EXTERNAL_APP) and replace the local defaultApp with an import of that constant; update any project-level fallback/resolution code that also uses "cursor" to import the same exported constant so both ClickablePath (defaultApp) and the project-default resolution reference the shared DEFAULT_APP symbol.apps/desktop/src/lib/trpc/routers/external/index.ts (2)
109-117: Default-app resolution logic is duplicated acrossopenFileInEditorandgetDefaultApp.This handler replicates the same
project?.defaultApp ?? "cursor"fallback found inprojects.getDefaultApp(inprojects/projects.ts, Line 326). Consider extracting this into a shared helper function (e.g.,resolveDefaultApp(projectId)) to keep the fallback logic in one place.♻️ Example shared helper
// In a shared utils file or alongside localDb function resolveDefaultApp(projectId?: string): ExternalApp { if (!projectId) return "cursor"; const project = localDb .select() .from(projects) .where(eq(projects.id, projectId)) .get(); return project?.defaultApp ?? "cursor"; }Then use it in both
getDefaultAppandopenFileInEditor:.mutation(async ({ input }) => { const filePath = resolvePath(input.path, input.cwd); - let app: ExternalApp = "cursor"; - if (input.projectId) { - const project = localDb - .select() - .from(projects) - .where(eq(projects.id, input.projectId)) - .get(); - app = project?.defaultApp ?? "cursor"; - } + const app = resolveDefaultApp(input.projectId); await openPathInApp(filePath, app); }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 109 - 117, Duplicate default-app fallback logic exists in openFileInEditor and projects.getDefaultApp; extract it into a shared helper (e.g., resolveDefaultApp) that accepts projectId, queries localDb/select().from(projects).where(eq(projects.id, projectId)).get(), and returns project?.defaultApp ?? "cursor"; replace the inline logic in openFileInEditor and projects.getDefaultApp with calls to resolveDefaultApp(projectId) to centralize the fallback behavior.
83-89: No validation thatprojectIdcorresponds to an existing project before the UPDATE.The
localDb.update(...).where(eq(projects.id, input.projectId))will silently affect zero rows if the project doesn't exist. This is acceptable fire-and-forget behavior, but worth noting — a typo or stale ID would silently fail to persist the user's IDE choice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/external/index.ts` around lines 83 - 89, Validate that the project exists before performing the update: query the projects table for input.projectId (e.g., using localDb.select/.where or a getById helper) and if no row is returned, handle it (return an error/throw or log and skip) instead of blindly calling localDb.update(projects).set({ defaultApp: input.app }).where(eq(projects.id, input.projectId)).run(); this ensures update on the correct project and surfaces typos or stale IDs rather than silently affecting zero rows.apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
317-327:getDefaultAppsilently returns"cursor"for non-existent projects.Unlike the
getprocedure (Line 308) which throwsNOT_FOUND, this query silently falls back to"cursor"when the project doesn't exist. This is fine for UI resilience, but it could mask bugs where a stale or invalidprojectIdis passed. Consider whether aNOT_FOUNDerror would be more appropriate here to surface misuse early, or if the silent fallback is the intended behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts` around lines 317 - 327, The getDefaultApp procedure currently returns the fallback string "cursor" when the project lookup in getDefaultApp returns no row, which hides missing-project errors; change getDefaultApp to mirror the get procedure's behavior by throwing a NOT_FOUND trpc error when localDb.select().from(projects).where(eq(projects.id, input.projectId)).get() yields no project instead of returning "cursor" — import and use TRPCError (or the same error helper used by the get procedure) and throw new TRPCError({ code: "NOT_FOUND", message: "Project not found" }) so callers get an explicit error for invalid/stale projectId.apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/$workspaceId/page.tsx (1)
303-307:projectId as stringcast — prefer a non-assertion alternative.
projectIdisstring | undefined; theenabled: !!projectIdguard prevents the query from running when it'sundefined, but theas stringbypasses TypeScript's type system. UsingprojectId ?? ""(or the non-null assertionprojectId!) makes the intent clearer and avoids the broad cast.🛠️ Proposed fix
- const { data: defaultApp = "cursor" } = - electronTrpc.projects.getDefaultApp.useQuery( - { projectId: projectId as string }, - { enabled: !!projectId }, - ); + const { data: defaultApp = "cursor" } = + electronTrpc.projects.getDefaultApp.useQuery( + { projectId: projectId ?? "" }, + { enabled: !!projectId }, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx around lines 303 - 307, The query call electronTrpc.projects.getDefaultApp.useQuery currently uses a type assertion (projectId as string); instead replace that assertion with a safe alternative like projectId ?? "" (or projectId! if you prefer a non-null assertion) so the argument matches the declared type without bypassing the type system, keeping the existing enabled: !!projectId guard around the query.apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx (2)
86-87: Stale comment: "opens in last used app" → should reference the project's default app.The comment on line 87 still says "last used app" but the logic now uses the per-project
defaultApp.Proposed fix
- {/* Main button - opens in last used app */} + {/* Main button - opens in project's default app */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx` around lines 86 - 87, Update the stale inline comment inside the OpenInMenuButton component: change the comment that currently says "opens in last used app" to reference the per-project default app (e.g., "opens in project's default app" or similar) so it matches the implementation that uses defaultApp; locate the comment near the main button within the OpenInMenuButton JSX (the div with className "flex items-center no-drag") and update the text accordingly.
42-46: InconsistentstaleTimebetweenOpenInMenuButtonandOpenInButton.This query specifies
staleTime: 30000but the identicalgetDefaultAppquery inOpenInButton.tsx(line 130) does not setstaleTime. This can lead to different cache behavior for the same data depending on which component mounts first. Consider unifying: either both should specify astaleTimeor neither should (relying on a global default).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx` around lines 42 - 46, The two components OpenInMenuButton and OpenInButton use the same electronTrpc.projects.getDefaultApp.useQuery but with inconsistent cache behavior (OpenInMenuButton sets staleTime: 30000 while OpenInButton does not); unify their behavior by either adding the same { staleTime: 30000 } option to the getDefaultApp.query call in OpenInButton or removing the staleTime from OpenInMenuButton so both rely on the global default—update the useQuery options in the component(s) accordingly to ensure identical caching for electronTrpc.projects.getDefaultApp.useQuery.apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx (1)
158-161: Stale function name:handleOpenLastUsed→handleOpenDefaultor similar.The function now opens in
defaultApprather than a "last used" app. Consider renaming for clarity.Proposed fix
- const handleOpenLastUsed = () => { + const handleOpenDefault = () => { if (!path) return; openInApp.mutate({ path, app: defaultApp, projectId }); };Also update the reference on line 172:
- onClick={handleOpenLastUsed} + onClick={handleOpenDefault}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx` around lines 158 - 161, Rename the stale handler handleOpenLastUsed to a clearer name like handleOpenDefault (or handleOpenDefaultApp) and update all references to it (e.g., where the click/handler is assigned) to use the new name; ensure the function body still calls openInApp.mutate({ path, app: defaultApp, projectId }) and that any export or prop passed matches the renamed identifier so TypeScript/JS references remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx`:
- Around line 248-250: Replace the hardcoded "⌘O" and the incorrect condition:
use the derived boolean showOpenInShortcut (which already checks openInShortcut
!== "Unassigned") instead of showShortcuts, and render the actual openInShortcut
variable rather than the literal "⌘O"; update the JSX around the defaultApp
check (where app.id === defaultApp) to show <span> containing {openInShortcut}
only when showOpenInShortcut is true.
In `@packages/local-db/drizzle/0027_per_project_default_app.sql`:
- Around line 1-2: The migration drops settings.last_used_app and adds
projects.default_app without backfilling existing preferences; add a migration
step that copies last_used_app into projects.default_app before dropping the
column (e.g., an UPDATE that joins projects and settings on the project
identifier and sets projects.default_app = COALESCE(settings.last_used_app,
'cursor') or settings.last_used_app where present), then verify null-handling
and only drop settings.last_used_app after the copy completes; reference the
projects table, projects.default_app column, settings table, and
settings.last_used_app column when implementing the change.
---
Outside diff comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/TopBar.tsx`:
- Around line 51-56: Replace the inconsistent prop passed to OpenInMenuButton:
currently it uses workspace.project?.id which can be undefined if the project
relation isn't loaded; change it to pass workspace?.projectId instead. Update
the JSX where OpenInMenuButton is rendered (the component and props:
worktreePath, branch, projectId) to use workspace?.projectId so the component
receives the correct ID even when relations are not eagerly loaded.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/external/index.ts`:
- Around line 109-117: Duplicate default-app fallback logic exists in
openFileInEditor and projects.getDefaultApp; extract it into a shared helper
(e.g., resolveDefaultApp) that accepts projectId, queries
localDb/select().from(projects).where(eq(projects.id, projectId)).get(), and
returns project?.defaultApp ?? "cursor"; replace the inline logic in
openFileInEditor and projects.getDefaultApp with calls to
resolveDefaultApp(projectId) to centralize the fallback behavior.
- Around line 83-89: Validate that the project exists before performing the
update: query the projects table for input.projectId (e.g., using
localDb.select/.where or a getById helper) and if no row is returned, handle it
(return an error/throw or log and skip) instead of blindly calling
localDb.update(projects).set({ defaultApp: input.app }).where(eq(projects.id,
input.projectId)).run(); this ensures update on the correct project and surfaces
typos or stale IDs rather than silently affecting zero rows.
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 317-327: The getDefaultApp procedure currently returns the
fallback string "cursor" when the project lookup in getDefaultApp returns no
row, which hides missing-project errors; change getDefaultApp to mirror the get
procedure's behavior by throwing a NOT_FOUND trpc error when
localDb.select().from(projects).where(eq(projects.id, input.projectId)).get()
yields no project instead of returning "cursor" — import and use TRPCError (or
the same error helper used by the get procedure) and throw new TRPCError({ code:
"NOT_FOUND", message: "Project not found" }) so callers get an explicit error
for invalid/stale projectId.
In `@apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx`:
- Around line 158-161: Rename the stale handler handleOpenLastUsed to a clearer
name like handleOpenDefault (or handleOpenDefaultApp) and update all references
to it (e.g., where the click/handler is assigned) to use the new name; ensure
the function body still calls openInApp.mutate({ path, app: defaultApp,
projectId }) and that any export or prop passed matches the renamed identifier
so TypeScript/JS references remain consistent.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx`:
- Around line 86-87: Update the stale inline comment inside the OpenInMenuButton
component: change the comment that currently says "opens in last used app" to
reference the per-project default app (e.g., "opens in project's default app" or
similar) so it matches the implementation that uses defaultApp; locate the
comment near the main button within the OpenInMenuButton JSX (the div with
className "flex items-center no-drag") and update the text accordingly.
- Around line 42-46: The two components OpenInMenuButton and OpenInButton use
the same electronTrpc.projects.getDefaultApp.useQuery but with inconsistent
cache behavior (OpenInMenuButton sets staleTime: 30000 while OpenInButton does
not); unify their behavior by either adding the same { staleTime: 30000 } option
to the getDefaultApp.query call in OpenInButton or removing the staleTime from
OpenInMenuButton so both rely on the global default—update the useQuery options
in the component(s) accordingly to ensure identical caching for
electronTrpc.projects.getDefaultApp.useQuery.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/workspace/`$workspaceId/page.tsx:
- Around line 303-307: The query call
electronTrpc.projects.getDefaultApp.useQuery currently uses a type assertion
(projectId as string); instead replace that assertion with a safe alternative
like projectId ?? "" (or projectId! if you prefer a non-null assertion) so the
argument matches the declared type without bypassing the type system, keeping
the existing enabled: !!projectId guard around the query.
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/components/ClickablePath/ClickablePath.tsx`:
- Line 30: The literal fallback "cursor" is duplicated as const defaultApp:
ExternalApp = "cursor"; — extract that literal into a single exported constant
(e.g., DEFAULT_APP or DEFAULT_EXTERNAL_APP) and replace the local defaultApp
with an import of that constant; update any project-level fallback/resolution
code that also uses "cursor" to import the same exported constant so both
ClickablePath (defaultApp) and the project-default resolution reference the
shared DEFAULT_APP symbol.
| {showShortcuts && app.id === defaultApp && ( | ||
| <span className="text-xs text-muted-foreground">⌘O</span> | ||
| )} |
There was a problem hiding this comment.
Hardcoded ⌘O shortcut instead of using openInShortcut variable.
Line 249 uses ⌘O while lines 219 and 281 correctly use {openInShortcut}. Additionally, the condition uses showShortcuts instead of the derived showOpenInShortcut (which also checks openInShortcut !== "Unassigned"). This means the VS Code sub-menu will display a shortcut even when unassigned.
Proposed fix
- {showShortcuts && app.id === defaultApp && (
- <span className="text-xs text-muted-foreground">⌘O</span>
+ {showOpenInShortcut && app.id === defaultApp && (
+ <span className="text-xs text-muted-foreground">
+ {openInShortcut}
+ </span>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx` around
lines 248 - 250, Replace the hardcoded "⌘O" and the incorrect condition: use the
derived boolean showOpenInShortcut (which already checks openInShortcut !==
"Unassigned") instead of showShortcuts, and render the actual openInShortcut
variable rather than the literal "⌘O"; update the JSX around the defaultApp
check (where app.id === defaultApp) to show <span> containing {openInShortcut}
only when showOpenInShortcut is true.
| ALTER TABLE `projects` ADD `default_app` text;--> statement-breakpoint | ||
| ALTER TABLE `settings` DROP COLUMN `last_used_app`; No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all references to last_used_app
rg -n 'last_used_app' --type=ts --type=sql -C3Repository: superset-sh/superset
Length of output: 907
🏁 Script executed:
#!/bin/bash
# List all migration files to see the sequence
echo "=== Migration files ==="
git ls-files packages/local-db/drizzle/ | sort
echo -e "\n=== Search for migration runner/handler in TypeScript ==="
rg -n 'migration|backfill|default_app' packages/local-db --type=ts -A2 -B2
echo -e "\n=== Check package.json for migration scripts ==="
cat packages/local-db/package.json 2>/dev/null | grep -A5 -B5 'script\|migration' || echo "File not found or no matches"
echo -e "\n=== Look for database initialization code ==="
rg -n 'settings|default_app' packages/local-db/src --type=ts -lRepository: superset-sh/superset
Length of output: 4718
🏁 Script executed:
#!/bin/bash
# Read the schema file to see defaultApp definition
echo "=== defaultApp in schema ==="
sed -n '40,50p' packages/local-db/src/schema/schema.ts
# Search for all references to defaultApp in the codebase
echo -e "\n=== All defaultApp references ==="
rg -n 'defaultApp|default_app' --type=ts -B2 -A2
# Look for settings initialization code
echo -e "\n=== Settings initialization/usage ==="
rg -n 'settings' packages/local-db/src --type=ts -l
# Search for "cursor" string which is mentioned as the fallback value in the review
echo -e "\n=== References to fallback app value ==="
rg -n '"cursor"' --type=tsRepository: superset-sh/superset
Length of output: 23028
Existing last_used_app preference is dropped without migration.
The migration drops last_used_app from settings and adds default_app to projects in a single step without backfilling data. Although the column appears unused in the codebase (no TypeScript references found), any users who had set a preference will lose it, with the application defaulting to "cursor". If this data loss is unintentional, add a migration step to copy last_used_app values to projects.default_app before dropping the column.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/local-db/drizzle/0027_per_project_default_app.sql` around lines 1 -
2, The migration drops settings.last_used_app and adds projects.default_app
without backfilling existing preferences; add a migration step that copies
last_used_app into projects.default_app before dropping the column (e.g., an
UPDATE that joins projects and settings on the project identifier and sets
projects.default_app = COALESCE(settings.last_used_app, 'cursor') or
settings.last_used_app where present), then verify null-handling and only drop
settings.last_used_app after the copy completes; reference the projects table,
projects.default_app column, settings table, and settings.last_used_app column
when implementing the change.
Summary
settings.lastUsedAppto per-projectprojects.defaultAppso each project remembers its own editor choice independentlydefaultAppcolumn toprojectstable, removelastUsedAppfromsettingstable (migration0027_per_project_default_app)projectIdthrough all "Open in" UI consumers (TopBar, OpenInButton, ChangesView file tree, FilesView file tree, context menus) so the correct per-project default is resolved everywhereTest plan
Summary by CodeRabbit
Release Notes