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
@@ -1,26 +1,28 @@
import { SidebarCard } from "@superset/ui/sidebar-card";
import { useNavigate } from "@tanstack/react-router";
import { AnimatePresence, motion } from "framer-motion";
import { useIsV2CloudEnabled } from "renderer/hooks/useIsV2CloudEnabled";
import { track } from "renderer/lib/analytics";
import { useV2AvailableBannerStore } from "renderer/stores/v2-available-banner";
import { useV2LocalOverrideStore } from "renderer/stores/v2-local-override";

export function V2AvailableBanner() {
const isV2CloudEnabled = useIsV2CloudEnabled();
const dismissed = useV2AvailableBannerStore((s) => s.dismissed);
const dismiss = useV2AvailableBannerStore((s) => s.dismiss);
const setOptInV2 = useV2LocalOverrideStore((s) => s.setOptInV2);
const navigate = useNavigate();

function handleSwitch() {
track("surface_toggled", { from: "v1", to: "v2", source: "v1_banner" });
setOptInV2(true);
function handleManage() {
track("v2_banner_manage_clicked");
navigate({ to: "/settings/experimental" });
}

function handleDismiss() {
track("v2_banner_dismissed");
dismiss();
}

if (isV2CloudEnabled) return null;

return (
<AnimatePresence>
{!dismissed && (
Expand All @@ -34,9 +36,9 @@ export function V2AvailableBanner() {
<SidebarCard
badge="New"
title="Superset v2 is here"
description="The new cloud workspace experience is now available."
actionLabel={isV2CloudEnabled ? undefined : "Switch to v2"}
onAction={isV2CloudEnabled ? undefined : handleSwitch}
description="Try the new cloud workspace experience."
actionLabel="Try new version of Superset"
onAction={handleManage}
onDismiss={handleDismiss}
/>
</motion.div>
Expand Down
4 changes: 4 additions & 0 deletions apps/desktop/src/renderer/hooks/useEnsureV2Project/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {
type EnsureV2ProjectResult,
useEnsureV2Project,
} from "./useEnsureV2Project";
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import { useCallback } from "react";
import { getHostServiceClientByUrl } from "renderer/lib/host-service-client";
import { useLocalHostService } from "renderer/routes/_authenticated/providers/LocalHostServiceProvider";

export interface EnsureV2ProjectResult {
hostUrl: string;
projectId: string;
repoPath: string;
mainWorkspaceId: string | null;
}

export function useEnsureV2Project(): (args: {
repoPath: string;
name: string;
}) => Promise<EnsureV2ProjectResult> {
const { activeHostUrl } = useLocalHostService();

return useCallback(
async ({ repoPath, name }) => {
if (!activeHostUrl) {
throw new Error("No active host service");
}
const hostService = getHostServiceClientByUrl(activeHostUrl);

const found = await hostService.project.findByPath.query({ repoPath });
const candidate = found.candidates[0];
if (candidate) {
try {
const setupResult = await hostService.project.setup.mutate({
projectId: candidate.id,
mode: { kind: "import", repoPath },
});
return {
hostUrl: activeHostUrl,
projectId: candidate.id,
repoPath: setupResult.repoPath,
mainWorkspaceId: setupResult.mainWorkspaceId,
};
} catch (err) {
// findByPath returns local sqlite rows even when no cloud v2 project
// exists for that id; setup → v2Project.get → NOT_FOUND. Only that
// case is safe to fall through to create — every other error
// (network, auth, 5xx) must propagate so retries don't silently mint
// duplicate cloud projects.
const code = (err as { data?: { code?: string } } | null | undefined)
?.data?.code;
if (code !== "NOT_FOUND") {
throw err;
}
}
}
Comment on lines +25 to +51

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 | 🟠 Major | ⚡ Quick win

Multiple findByPath candidates are silently ignored — risks linking to the wrong project.

findByPath returns a candidates list, and the sibling useFolderFirstImport explicitly treats candidates.length > 1 as an error case (surfacing it to the caller via onMultipleProjects / onError) precisely because more than one local sqlite row can match the same repoPath. Here, the hook just picks candidates[0] and proceeds to setup.mutate against it.

When this hook is used during adopt-worktrees, this means a project whose mainRepoPath matches multiple historical entries will be linked to whichever one happens to come first — potentially the wrong cloud project — without any error or user prompt.

Consider mirroring useFolderFirstImport's behavior: if found.candidates.length > 1, throw (or surface to the caller) so the caller can decide rather than silently disambiguating.

🛡️ Suggested guard
 const found = await hostService.project.findByPath.query({ repoPath });
-const candidate = found.candidates[0];
-if (candidate) {
+if (found.candidates.length > 1) {
+	throw new Error(
+		`Multiple projects use this repository (${found.candidates.length}). Open the intended one from settings before importing.`,
+	);
+}
+const candidate = found.candidates[0];
+if (candidate) {
 	try {
🤖 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/desktop/src/renderer/hooks/useEnsureV2Project/useEnsureV2Project.ts`
around lines 25 - 51, The hook useEnsureV2Project currently picks
found.candidates[0] from hostService.project.findByPath.query and proceeds,
which can silently link the wrong project; instead, mirror useFolderFirstImport:
after calling hostService.project.findByPath.query({ repoPath }) check if
found.candidates.length > 1 and abort by surfacing an error (throw a specific
Error or call the same onMultipleProjects/onError path used elsewhere) rather
than selecting candidates[0]; only when candidates.length === 1 proceed to call
hostService.project.setup.mutate with candidate.id, and preserve the existing
NOT_FOUND handling for setup errors.


const created = await hostService.project.create.mutate({
name,
mode: { kind: "importLocal", repoPath },
});
return {
hostUrl: activeHostUrl,
projectId: created.projectId,
repoPath: created.repoPath,
mainWorkspaceId: created.mainWorkspaceId,
};
},
[activeHostUrl],
);
}
Loading
Loading