Skip to content

fix(desktop): v2 Open In editor is per-project, not per-workspace #3393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 2 commits into from
Apr 13, 2026
Merged

fix(desktop): v2 Open In editor is per-project, not per-workspace #3393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e55bf5f
fix(desktop): v2 Open In editor is per-project, not per-workspace
saddlepaddle Apr 13, 2026
d2722b8
fix(desktop): use ensureProjectInSidebar helper in V2OpenInMenuButton
saddlepaddle Apr 13, 2026
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
44 changes: 27 additions & 17 deletions ...ticated/_dashboard/components/TopBar/components/V2OpenInMenuButton/V2OpenInMenuButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,49 +8,59 @@ import {
import { toast } from "@superset/ui/sonner";
import { Tooltip, TooltipContent, TooltipTrigger } from "@superset/ui/tooltip";
import { cn } from "@superset/ui/utils";
import { useCallback, useMemo, useState } from "react";
import { eq } from "@tanstack/db";
import { useLiveQuery } from "@tanstack/react-db";
import { useCallback, useMemo } from "react";
import { HiChevronDown } from "react-icons/hi2";
import {
getAppOption,
OpenInExternalDropdownItems,
} from "renderer/components/OpenInExternalDropdown";
import { HotkeyLabel, useHotkey, useHotkeyDisplay } from "renderer/hotkeys";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { useDashboardSidebarState } from "renderer/routes/_authenticated/hooks/useDashboardSidebarState";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { useThemeStore } from "renderer/stores";

interface V2OpenInMenuButtonProps {
worktreePath: string;
branch: string;
workspaceId: string;
projectId: string;
}

export function V2OpenInMenuButton({
worktreePath,
branch,
workspaceId,
projectId,
}: V2OpenInMenuButtonProps) {
const collections = useCollections();
const { ensureProjectInSidebar } = useDashboardSidebarState();
const activeTheme = useThemeStore((state) => state.activeTheme);

const localState = collections.v2WorkspaceLocalState.get(workspaceId);
const [defaultApp, setDefaultApp] = useState<ExternalApp>(
(localState?.defaultOpenInApp as ExternalApp) ?? "finder",
const { data: sidebarProjectRows = [] } = useLiveQuery(
(q) =>
q
.from({ sp: collections.v2SidebarProjects })
.where(({ sp }) => eq(sp.projectId, projectId))
.select(({ sp }) => ({ defaultOpenInApp: sp.defaultOpenInApp })),
[collections, projectId],
);
const resolvedApp: ExternalApp =
(sidebarProjectRows[0]?.defaultOpenInApp as ExternalApp | null) ?? "finder";
Comment on lines +48 to +49

@greptile-apps greptile-apps Bot Apr 13, 2026

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.

P2 Unsafe cast from string | null to ExternalApp

defaultOpenInApp is typed as string | null in the schema, but it is cast directly to ExternalApp | null here. If a stale or manually-edited localStorage entry contains a string that is not a member of the ExternalApp union, resolvedApp will silently carry an invalid value. getAppOption(resolvedApp) does guard against this (returning null → currentApp is null → the button shows "Select an editor from the dropdown"), so there is no crash, but the mismatch between the declared type and the runtime value could hide bugs in callers that trust the type annotation.

Consider validating against the known ExternalApp values before the cast, or narrowing the schema to an enum so Zod rejects unknown strings at parse time:

const rawApp = sidebarProjectRows[0]?.defaultOpenInApp ?? null;
const resolvedApp: ExternalApp = (isExternalApp(rawApp) ? rawApp : null) ?? "finder";

where isExternalApp is a simple type-guard against the ExternalApp union.


const handleDefaultAppChange = useCallback(
const persistDefaultApp = useCallback(
(app: ExternalApp) => {
setDefaultApp(app);
collections.v2WorkspaceLocalState.update(workspaceId, (draft) => {
ensureProjectInSidebar(projectId);
collections.v2SidebarProjects.update(projectId, (draft) => {
draft.defaultOpenInApp = app;
});
},
[collections, workspaceId],
[collections, ensureProjectInSidebar, projectId],
);
Comment on lines +51 to 59

@greptile-apps greptile-apps Bot Apr 13, 2026

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.

P1 Defensive insert can create a phantom sidebar-project entry

When no v2SidebarProjects record exists for projectId (e.g. the user navigated directly to a workspace URL without ever opening the project in the sidebar), the fallback insert creates a record with tabOrder: 0 and isCollapsed: false. Because the v2SidebarProjects collection is keyed by projectId, this synthetic entry will be visible to every component that reads the collection — including the sidebar renderer. Depending on how the sidebar orders and filters its project list, this could:

  1. Surface a "ghost" project row with tabOrder: 0 (likely sorting it to the very top).
  2. Block the sidebar's own insert when it later tries to create its canonical entry for the same projectId, potentially swallowing the error silently or producing an unexpected no-op.

A safer approach would be to scope this field write so it only mutates an already-existing record, and treats a missing record as "no preference stored yet":

const persistDefaultApp = useCallback(
    (app: ExternalApp) => {
        if (collections.v2SidebarProjects.get(projectId)) {
            collections.v2SidebarProjects.update(projectId, (draft) => {
                draft.defaultOpenInApp = app;
            });
        }
        // No insert fallback: the sidebar manages the authoritative record.
    },
    [collections, projectId],
);

If the intent is genuinely to persist the choice before the sidebar creates its record, the insertion should use the sidebar's own "create project entry" logic so that tabOrder is computed correctly rather than hard-coded to 0.


const openInApp = electronTrpc.external.openInApp.useMutation({
onSuccess: (_data, variables) => {
handleDefaultAppChange(variables.app);
persistDefaultApp(variables.app);
},
onError: (error) => toast.error(`Failed to open: ${error.message}`),
});
Expand All @@ -60,8 +70,8 @@ export function V2OpenInMenuButton({
});

const currentApp = useMemo(
() => getAppOption(defaultApp) ?? null,
[defaultApp],
() => getAppOption(resolvedApp) ?? null,
[resolvedApp],
);
const openInDisplay = useHotkeyDisplay("OPEN_IN_APP");
const copyPathDisplay = useHotkeyDisplay("COPY_PATH");
Expand All @@ -72,8 +82,8 @@ export function V2OpenInMenuButton({

const handleOpenInEditor = useCallback(() => {
if (openInApp.isPending || copyPath.isPending) return;
openInApp.mutate({ path: worktreePath, app: defaultApp });
}, [worktreePath, defaultApp, openInApp, copyPath.isPending]);
openInApp.mutate({ path: worktreePath, app: resolvedApp });
}, [worktreePath, resolvedApp, openInApp, copyPath.isPending]);

const handleOpenInOtherApp = useCallback(
(appId: ExternalApp) => {
Expand Down Expand Up @@ -162,12 +172,12 @@ export function V2OpenInMenuButton({
<DropdownMenuContent align="end" className="w-48">
<OpenInExternalDropdownItems
isDark={isDark}
activeApp={defaultApp}
activeApp={resolvedApp}
onOpenIn={handleOpenInOtherApp}
onCopyPath={handleCopyPath}
renderAppTrailing={(appId, group) => {
if (
appId !== defaultApp ||
appId !== resolvedApp ||
!showOpenInShortcut ||
group === "jetbrains"
) {
Expand Down
3 changes: 2 additions & 1 deletion ...ashboard/components/TopBar/components/V2WorkspaceOpenInButton/V2WorkspaceOpenInButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export function V2WorkspaceOpenInButton({
.select(({ workspaces, hosts }) => ({
id: workspaces.id,
branch: workspaces.branch,
projectId: workspaces.projectId,
hostMachineId: hosts?.machineId ?? null,
})),
[collections, workspaceId],
Expand Down Expand Up @@ -56,7 +57,7 @@ export function V2WorkspaceOpenInButton({
<V2OpenInMenuButton
branch={workspace.branch}
worktreePath={workspaceQuery.data.worktreePath}
workspaceId={workspace.id}
projectId={workspace.projectId}
/>
);
}
2 changes: 1 addition & 1 deletion ...derer/routes/_authenticated/providers/CollectionsProvider/dashboardSidebarLocal/schema.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ export const dashboardSidebarProjectSchema = z.object({
createdAt: persistedDateSchema,
isCollapsed: z.boolean().default(false),
tabOrder: z.number().int().default(0),
defaultOpenInApp: z.string().nullable().default(null),
});

const paneWorkspaceStateSchema = z.custom<WorkspaceState<unknown>>();
Expand Down Expand Up @@ -39,7 +40,6 @@ export const workspaceLocalStateSchema = z.object({
}),
paneLayout: paneWorkspaceStateSchema,
rightSidebarOpen: z.boolean().default(false),
defaultOpenInApp: z.string().nullable().default(null),
viewedFiles: z.array(z.string()).default([]),
});

Expand Down
Loading