Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 56 additions & 11 deletions apps/desktop/src/lib/trpc/routers/external/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import { EXTERNAL_APPS, projects } from "@superset/local-db";
import {
EXTERNAL_APPS,
NON_EDITOR_APPS,
projects,
settings,
} from "@superset/local-db";
import { TRPCError } from "@trpc/server";
import { eq } from "drizzle-orm";
import { clipboard, shell } from "electron";
Expand All @@ -14,6 +19,39 @@ import {

const ExternalAppSchema = z.enum(EXTERNAL_APPS);

const nonEditorSet = new Set<ExternalApp>(NON_EDITOR_APPS);

/** Sets the global default editor if one hasn't been set yet. Skips non-editor apps. */
function ensureGlobalDefaultEditor(app: ExternalApp) {
if (nonEditorSet.has(app)) return;

const row = localDb.select().from(settings).get();
if (!row?.defaultEditor) {
localDb
.insert(settings)
.values({ id: 1, defaultEditor: app })
.onConflictDoUpdate({
target: settings.id,
set: { defaultEditor: app },
})
.run();
}
}

/** Resolves the default editor from project setting, then global setting. */
export function resolveDefaultEditor(projectId?: string): ExternalApp | null {
if (projectId) {
const project = localDb
.select()
.from(projects)
.where(eq(projects.id, projectId))
.get();
if (project?.defaultApp) return project.defaultApp;
}
const row = localDb.select().from(settings).get();
return row?.defaultEditor ?? null;
}
Comment on lines +41 to +53
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

resolveDefaultEditor can return a non-editor app from project.defaultApp.

The global default (settings.defaultEditor) is properly guarded against non-editor apps by both ensureGlobalDefaultEditor and setDefaultEditor. However, project.defaultApp stores whatever app was last used (including Finder, Terminal, etc.), and resolveDefaultEditor returns it without filtering.

If a user's last action on a project was "Open in Finder", then openFileInEditor would resolve to "finder" and call shell.showItemInFolder instead of launching an editor.

Consider filtering out non-editor apps from the project-level lookup:

Proposed fix
 export function resolveDefaultEditor(projectId?: string): ExternalApp | null {
 	if (projectId) {
 		const project = localDb
 			.select()
 			.from(projects)
 			.where(eq(projects.id, projectId))
 			.get();
-		if (project?.defaultApp) return project.defaultApp;
+		if (project?.defaultApp && !nonEditorSet.has(project.defaultApp))
+			return project.defaultApp;
 	}
 	const row = localDb.select().from(settings).get();
 	return row?.defaultEditor ?? null;
 }
🤖 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 41 - 53,
resolveDefaultEditor currently returns project.defaultApp without verifying it's
an editor, allowing non-editor values (e.g., "finder", "terminal") to be used;
update resolveDefaultEditor to only return project.defaultApp when it is a valid
editor (e.g., via an existing helper like isEditorApp or by checking against the
canonical editor list) and otherwise fall back to settings.defaultEditor;
reference resolveDefaultEditor, project.defaultApp, settings.defaultEditor, and
the related helpers ensureGlobalDefaultEditor/setDefaultEditor so the
project-level lookup mirrors the global guard.


async function openPathInApp(
filePath: string,
app: ExternalApp,
Expand Down Expand Up @@ -80,14 +118,26 @@ export const createExternalRouter = () => {
}),
)
.mutation(async ({ input }) => {
await openPathInApp(input.path, input.app);

// Persist defaults only after successful launch
if (input.projectId) {
localDb
.update(projects)
.set({ defaultApp: input.app })
.where(eq(projects.id, input.projectId))
.run();
}
await openPathInApp(input.path, input.app);

// Auto-set global default editor on first successful use (best-effort)
try {
ensureGlobalDefaultEditor(input.app);
} catch (err) {
console.warn(
"[external/openInApp] Failed to persist global default editor:",
err,
);
}
}),

copyPath: publicProcedure.input(z.string()).mutation(async ({ input }) => {
Expand All @@ -106,15 +156,10 @@ export const createExternalRouter = () => {
)
.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";
}
// Preserve first-run behavior for terminal/file-link flows.
// If no project/global default editor is configured yet, fall back to Cursor.
const app = resolveDefaultEditor(input.projectId) ?? "cursor";

await openPathInApp(filePath, app);
}),
});
Expand Down
9 changes: 2 additions & 7 deletions apps/desktop/src/lib/trpc/routers/projects/projects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { PROJECT_COLOR_VALUES } from "shared/constants/project-colors";
import simpleGit from "simple-git";
import { z } from "zod";
import { publicProcedure, router } from "../..";
import { resolveDefaultEditor } from "../external";
import {
activateProject,
getBranchWorkspace,
Expand Down Expand Up @@ -279,13 +280,7 @@ export const createProjectsRouter = (getWindow: () => BrowserWindow | null) => {
getDefaultApp: publicProcedure
.input(z.object({ projectId: z.string() }))
.query(({ input }) => {
const project = localDb
.select()
.from(projects)
.where(eq(projects.id, input.projectId))
.get();

return project?.defaultApp ?? "cursor";
return resolveDefaultEditor(input.projectId);
}),

getRecents: publicProcedure.query((): Project[] => {
Expand Down
31 changes: 31 additions & 0 deletions apps/desktop/src/lib/trpc/routers/settings/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import {
BRANCH_PREFIX_MODES,
EXECUTION_MODES,
EXTERNAL_APPS,
FILE_OPEN_MODES,
NON_EDITOR_APPS,
settings,
TERMINAL_LINK_BEHAVIORS,
type TerminalPreset,
Expand Down Expand Up @@ -661,6 +663,35 @@ export const createSettingsRouter = () => {
return { success: true };
}),

getDefaultEditor: publicProcedure.query(() => {
const row = getSettings();
return row.defaultEditor ?? null;
}),

setDefaultEditor: publicProcedure
.input(
z.object({
editor: z
.enum(EXTERNAL_APPS)
.nullable()
.refine((val) => val === null || !NON_EDITOR_APPS.includes(val), {
message: "Non-editor apps cannot be set as the global default",
}),
}),
)
.mutation(({ input }) => {
localDb
.insert(settings)
.values({ id: 1, defaultEditor: input.editor })
.onConflictDoUpdate({
target: settings.id,
set: { defaultEditor: input.editor },
})
.run();

return { success: true };
}),

// TODO: remove telemetry procedures once telemetry_enabled column is dropped
getTelemetryEnabled: publicProcedure.query(() => {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ const ALL_APP_OPTIONS = [
];

export const getAppOption = (id: ExternalApp) =>
ALL_APP_OPTIONS.find((app) => app.id === id) ?? APP_OPTIONS[1];
ALL_APP_OPTIONS.find((app) => app.id === id);

export interface OpenInButtonProps {
path: string | undefined;
Expand All @@ -126,11 +126,10 @@ export function OpenInButton({
const showCopyPathShortcut =
showShortcuts && copyPathShortcut !== "Unassigned";

const { data: defaultApp = "cursor" } =
electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId },
);
const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId },
);
Comment on lines +129 to +132
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n apps/desktop/src/renderer/components/OpenInButton/OpenInButton.tsx

Repository: superset-sh/superset

Length of output: 12118


🏁 Script executed:

rg -n "OpenInButton" --type=tsx --type=ts -B 2 -A 5

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

rg -n "OpenInButton" -B 2 -A 5

Repository: superset-sh/superset

Length of output: 6954


🏁 Script executed:

cat -n apps/desktop/src/renderer/components/ConfigFilePreview/ConfigFilePreview.tsx | grep -A 10 -B 5 "OpenInButton"

Repository: superset-sh/superset

Length of output: 1277


🏁 Script executed:

cat -n apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/OpenInMenuButton/OpenInMenuButton.tsx | head -80

Repository: superset-sh/superset

Length of output: 3371


🏁 Script executed:

cat -n apps/desktop/src/renderer/routes/_authenticated/settings/components/ClickablePath/ClickablePath.tsx | head -80

Repository: superset-sh/superset

Length of output: 3199


🏁 Script executed:

rg "<OpenInButton" -B 3 -A 5

Repository: superset-sh/superset

Length of output: 1034


🏁 Script executed:

rg "OpenInButton" --type ts --type tsx | grep -v "import\|export\|interface\|from\|props\|Props"

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

cat -n apps/desktop/src/renderer/components/ConfigFilePreview/ConfigFilePreview.tsx | head -20

Repository: superset-sh/superset

Length of output: 762


projectId as string breaks type safety; replace with non-null assertion projectId!.

The idiomatic pattern for enabled-guarded tRPC queries is the non-null assertion projectId! rather than projectId as string. It's equally concise but maintains type safety:

Proposed fix
-	const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
-		{ projectId: projectId as string },
+	const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
+		{ projectId: projectId! },

Note: While projectId is optional in the props interface, OpenInButton is currently only rendered in ConfigFilePreview, which always supplies a projectId. The global default handling is out of scope for this component (see ClickablePath for the pattern used when handling settings without project context).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId },
);
const { data: defaultApp } = 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/components/OpenInButton/OpenInButton.tsx` around
lines 129 - 132, The query call using
electronTrpc.projects.getDefaultApp.useQuery currently casts projectId with
"projectId as string" which weakens type safety; change the argument to use the
non-null assertion "projectId!" instead. Locate the use of
electronTrpc.projects.getDefaultApp.useQuery in OpenInButton and replace the
cast with projectId! so the enabled guard ({ enabled: !!projectId }) remains
correct and TypeScript keeps the tighter non-null typing for the query input.
Ensure no other occurrences in OpenInButton use the "as string" cast for
projectId.


const openInApp = electronTrpc.external.openInApp.useMutation({
onSuccess: () => {
Expand All @@ -141,7 +140,7 @@ export function OpenInButton({
});
const copyPath = electronTrpc.external.copyPath.useMutation();

const currentApp = getAppOption(defaultApp);
const currentApp = defaultApp ? (getAppOption(defaultApp) ?? null) : null;

const handleOpenIn = (app: ExternalApp) => {
if (!path) return;
Expand All @@ -156,13 +155,13 @@ export function OpenInButton({
};

const handleOpenLastUsed = () => {
if (!path) return;
if (!path || !defaultApp) return;
openInApp.mutate({ path, app: defaultApp, projectId });
};

return (
<ButtonGroup>
{label && (
{label && currentApp && (
<Tooltip>
<TooltipTrigger asChild>
<Button
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { memo, useCallback, useMemo } from "react";
import { HiChevronDown } from "react-icons/hi2";
import { LuCopy } from "react-icons/lu";
import { LuCopy, LuExternalLink } from "react-icons/lu";
import jetbrainsIcon from "renderer/assets/app-icons/jetbrains.svg";
import vscodeIcon from "renderer/assets/app-icons/vscode.svg";
import {
Expand All @@ -39,11 +39,10 @@ export const OpenInMenuButton = memo(function OpenInMenuButton({
projectId,
}: OpenInMenuButtonProps) {
const utils = electronTrpc.useUtils();
const { data: defaultApp = "cursor" } =
electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId, staleTime: 30000 },
);
const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId, staleTime: 30000 },
);
const openInApp = electronTrpc.external.openInApp.useMutation({
onSuccess: () => {
if (projectId) {
Expand All @@ -57,7 +56,10 @@ export const OpenInMenuButton = memo(function OpenInMenuButton({
onError: (error) => toast.error(`Failed to copy path: ${error.message}`),
});

const currentApp = useMemo(() => getAppOption(defaultApp), [defaultApp]);
const currentApp = useMemo(
() => (defaultApp ? (getAppOption(defaultApp) ?? null) : null),
[defaultApp],
);
const openInShortcut = useHotkeyText("OPEN_IN_APP");
const copyPathShortcut = useHotkeyText("COPY_PATH");
const showOpenInShortcut = openInShortcut !== "Unassigned";
Expand All @@ -66,6 +68,12 @@ export const OpenInMenuButton = memo(function OpenInMenuButton({

const handleOpenInEditor = useCallback(() => {
if (openInApp.isPending || copyPath.isPending) return;
if (!defaultApp) {
toast.error("No default editor configured", {
description: "Open a project in an editor first to set a default.",
});
return;
}
openInApp.mutate({ path: worktreePath, app: defaultApp, projectId });
}, [worktreePath, defaultApp, projectId, openInApp, copyPath.isPending]);

Expand All @@ -90,22 +98,31 @@ export const OpenInMenuButton = memo(function OpenInMenuButton({
<button
type="button"
onClick={handleOpenInEditor}
disabled={isLoading}
aria-label={`Open in ${currentApp.displayLabel ?? currentApp.label}`}
disabled={isLoading || !currentApp}
aria-label={
currentApp
? `Open in ${currentApp.displayLabel ?? currentApp.label}`
: "Open in editor"
}
className={cn(
"group flex items-center gap-1.5 h-6 px-1.5 sm:pl-1.5 sm:pr-2 rounded-l border border-r-0 border-border/60 bg-secondary/50 text-xs font-medium",
"transition-all duration-150 ease-out",
"hover:bg-secondary hover:border-border",
"focus-visible:outline-none focus-visible:ring-1 focus-visible:ring-ring",
"active:scale-[0.98]",
isLoading && "opacity-50 pointer-events-none",
!currentApp && "opacity-50",
)}
>
<img
src={currentApp.icon}
alt=""
className="size-3.5 object-contain shrink-0"
/>
{currentApp ? (
<img
src={currentApp.icon}
alt=""
className="size-3.5 object-contain shrink-0"
/>
) : (
<LuExternalLink className="size-3.5 shrink-0" />
)}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
{branch && (
<span className="hidden lg:inline text-muted-foreground truncate max-w-[140px] tabular-nums">
/{branch}
Expand All @@ -119,8 +136,10 @@ export const OpenInMenuButton = memo(function OpenInMenuButton({
<TooltipContent side="bottom" sideOffset={6}>
<div className="flex flex-col gap-1">
<span className="flex items-center gap-1.5">
Open in {currentApp.displayLabel ?? currentApp.label}
{showOpenInShortcut && (
{currentApp
? `Open in ${currentApp.displayLabel ?? currentApp.label}`
: "Select an editor from the dropdown"}
{currentApp && showOpenInShortcut && (
<kbd className="px-1 py-0.5 text-[10px] font-mono bg-foreground/10 text-foreground/70 rounded">
{openInShortcut}
</kbd>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,10 @@ function WorkspacePage() {

// Open in last used app shortcut
const projectId = workspace?.projectId;
const { data: defaultApp = "cursor" } =
electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId },
);
const { data: defaultApp } = electronTrpc.projects.getDefaultApp.useQuery(
{ projectId: projectId as string },
{ enabled: !!projectId },
);
const utils = electronTrpc.useUtils();
const openInApp = electronTrpc.external.openInApp.useMutation({
onSuccess: () => {
Expand All @@ -316,7 +315,7 @@ function WorkspacePage() {
useAppHotkey(
"OPEN_IN_APP",
() => {
if (workspace?.worktreePath) {
if (workspace?.worktreePath && defaultApp) {
openInApp.mutate({
path: workspace.worktreePath,
app: defaultApp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,18 @@ interface ClickablePathProps {
className?: string;
}

const defaultApp: ExternalApp = "cursor";

export function ClickablePath({ path, className }: ClickablePathProps) {
const [isOpen, setIsOpen] = useState(false);
const utils = electronTrpc.useUtils();
// Uses global default editor (no project context on the settings page).
// No projectId is passed to openInApp, so per-project defaults are not affected.
const { data: defaultApp } =
electronTrpc.settings.getDefaultEditor.useQuery();

const openInApp = electronTrpc.external.openInApp.useMutation({
onSuccess: () => {
utils.settings.getDefaultEditor.invalidate();
},
onError: (error) => toast.error(`Failed to open: ${error.message}`),
});

Expand Down
Loading
Loading