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
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,12 @@ export function ImportPresetsPage({ organizationId }: ImportPresetsPageProps) {
[collections],
);

const importedV1Ids = useMemo(
() => new Set(v2Presets.map((p) => p.id)),
const importedAgentIds = useMemo(
() => new Set(v2Presets.flatMap((p) => (p.agentId ? [p.agentId] : []))),
[v2Presets],
);
const importedNames = useMemo(
() => new Set(v2Presets.flatMap((p) => (p.agentId ? [] : [p.name]))),
[v2Presets],
Comment on lines +36 to 38
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Name-based dedup collides with builtin display labels

importedNames is built from all v2 preset names, including builtins imported under their display labels (e.g. "Claude Code"). If a user has a custom v1 preset whose name happens to match a builtin's display label, importedNames.has(v2Name) returns true and the row is permanently stuck as "Imported" — even though no custom preset with that name was ever actually imported. Narrowing importedNames to only presets where agentId is absent would avoid the collision.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/components/V1ImportModal/ImportPresetsPage/ImportPresetsPage.tsx
Line: 36-38

Comment:
**Name-based dedup collides with builtin display labels**

`importedNames` is built from all v2 preset names, including builtins imported under their display labels (e.g. `"Claude Code"`). If a user has a custom v1 preset whose name happens to match a builtin's display label, `importedNames.has(v2Name)` returns `true` and the row is permanently stuck as "Imported" — even though no custom preset with that name was ever actually imported. Narrowing `importedNames` to only presets where `agentId` is absent would avoid the collision.

How can I resolve this? If you propose a fix, please make it concise.

);

Expand All @@ -56,29 +60,46 @@ export function ImportPresetsPage({ organizationId }: ImportPresetsPageProps) {
onRefresh={refresh}
isRefreshing={isRefreshing}
>
{presets.map((preset, index) => (
<PresetRow
key={preset.id}
preset={preset}
tabOrder={index}
alreadyImported={importedV1Ids.has(preset.id)}
organizationId={organizationId}
/>
))}
{presets.map((preset, index) => {
const linkedAgentId = BUILTIN_AGENT_IDS.has(preset.name)
? (preset.name as AgentType)
: undefined;
const v2Name = linkedAgentId
? AGENT_LABELS[linkedAgentId]
: preset.name;
const alreadyImported = linkedAgentId
? importedAgentIds.has(linkedAgentId)
: importedNames.has(v2Name);
return (
<PresetRow
key={preset.id}
preset={preset}
tabOrder={index}
linkedAgentId={linkedAgentId}
v2Name={v2Name}
alreadyImported={alreadyImported}
organizationId={organizationId}
/>
);
})}
</ImportPageShell>
);
}

interface PresetRowProps {
preset: TerminalPreset;
tabOrder: number;
linkedAgentId: AgentType | undefined;
v2Name: string;
alreadyImported: boolean;
organizationId: string;
}

function PresetRow({
preset,
tabOrder,
linkedAgentId,
v2Name,
alreadyImported,
organizationId,
}: PresetRowProps) {
Expand All @@ -90,15 +111,9 @@ function PresetRow({
setRunning(true);
setErrorMessage(null);
try {
const linkedAgentId: AgentType | undefined = BUILTIN_AGENT_IDS.has(
preset.name,
)
? (preset.name as AgentType)
: undefined;

const row: V2TerminalPresetRow = {
id: preset.id,
name: linkedAgentId ? AGENT_LABELS[linkedAgentId] : preset.name,
id: crypto.randomUUID(),
name: v2Name,
description: preset.description,
cwd: preset.cwd,
commands: preset.commands,
Expand Down Expand Up @@ -137,7 +152,7 @@ function PresetRow({
return (
<ImportRow
icon={<LuTerminal className="size-3.5" strokeWidth={2} />}
primary={preset.name}
primary={v2Name}
secondary={preset.description ?? preset.commands[0]}
action={action}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
V1_IMPORT_PAGE_ORDER,
} from "renderer/stores/v1-import-modal";
import { MOCK_ORG_ID } from "shared/constants";
import { IntroPage } from "./components/IntroPage";
import { WelcomePage } from "./components/WelcomePage";
import { ImportPresetsPage } from "./ImportPresetsPage";
import { ImportProjectsPage } from "./ImportProjectsPage";
Expand Down Expand Up @@ -51,15 +52,20 @@ export function V1ImportModal() {
onInteractOutside={(event) => event.preventDefault()}
>
<DialogTitle className="sr-only">
{page === "welcome" ? "Welcome to Superset v2" : "Import from v1"}
{page === "welcome"
? "Welcome to Superset v2"
: page === "intro"
? "Let's get you started"
: "Import from v1"}
</DialogTitle>
<DialogDescription className="sr-only">
Bring projects, workspaces, and terminal presets from Superset v1 into
v2.
Let's get your workspaces and projects ported over. Terminal sessions
won't be carried over, but you can still access v1 at any time.
</DialogDescription>

<div key={page} className="animate-in fade-in duration-200">
{page === "welcome" && <WelcomePage />}
{page === "intro" && <IntroPage />}
{(page === "projects" || page === "workspaces") && !activeHostUrl && (
<div className="flex h-[454px] items-center justify-center bg-background px-6 text-center text-sm text-muted-foreground">
Host service is not ready yet. This window will populate as soon
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export function IntroPage() {
return (
<div className="flex h-[454px] flex-col items-center justify-center bg-background px-14 text-center">
<div className="text-2xl font-semibold text-foreground">
Let's get you started
</div>
<p className="mt-3 max-w-md text-sm text-muted-foreground">
Let's get your workspaces and projects ported over. Terminal sessions
won't be carried over, but you can still access v1 at any time.
</p>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { IntroPage } from "./IntroPage";
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ export function WelcomePage() {
<div className="text-3xl font-semibold text-white">
Welcome to Superset v2
</div>
<p className="mt-3 max-w-md text-sm text-white/80">
Bring your v1 projects, workspaces, and terminal presets over. Import
each one with a single click — nothing happens automatically.
</p>
</div>
</div>
);
Expand Down
8 changes: 7 additions & 1 deletion apps/desktop/src/renderer/stores/v1-import-modal.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
import { create } from "zustand";
import { devtools } from "zustand/middleware";

export type V1ImportPage = "welcome" | "projects" | "workspaces" | "presets";
export type V1ImportPage =
| "welcome"
| "intro"
| "projects"
| "workspaces"
| "presets";

export const V1_IMPORT_PAGE_ORDER: V1ImportPage[] = [
"welcome",
"intro",
"projects",
"workspaces",
"presets",
Expand Down
Loading