From a376feb673ff1f03a60c4420fa5eb18f5db6be95 Mon Sep 17 00:00:00 2001 From: bnz183 Date: Wed, 24 Jun 2026 05:25:31 +0200 Subject: [PATCH] fix(studio): stop onboarding detect-site refetch loop on failed detection The detect-site effect keyed its guard on `report === null`, which conflated "not fetched yet" with "fetched and failed", so a failed/empty detection re-armed the effect and refetched /api/setup/detect in a tight loop. Track the attempted state via a resolved-attempt token (set only when a fetch settles), derive loading/error/loaded status, surface a real error state with a Retry control, and block Continue on failure. Add e2e coverage asserting no refetch loop and single-request retry recovery. Co-Authored-By: Claude Opus 4.8 --- .../e2e/onboarding-detection-loop.spec.ts | 84 +++++++++++++++++++ .../src/components/OnboardingWizard.tsx | 59 ++++++++++--- 2 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 apps/studio/e2e/onboarding-detection-loop.spec.ts diff --git a/apps/studio/e2e/onboarding-detection-loop.spec.ts b/apps/studio/e2e/onboarding-detection-loop.spec.ts new file mode 100644 index 0000000..3eec1b7 --- /dev/null +++ b/apps/studio/e2e/onboarding-detection-loop.spec.ts @@ -0,0 +1,84 @@ +import { expect, test } from "@playwright/test"; +import { enterDemoMode } from "./helpers.js"; + +const DETECT_GLOB = "**/api/setup/detect"; + +async function openWizardToDetectStep(page: import("@playwright/test").Page) { + await enterDemoMode(page); + await page + .getByRole("button", { name: /Run setup wizard|Review setup wizard/ }) + .click(); + await expect(page.getByRole("dialog")).toBeVisible(); + await expect( + page.getByRole("heading", { name: "Welcome", level: 1 }), + ).toBeVisible(); + await page.getByRole("button", { name: "Continue" }).click(); + await expect( + page.getByRole("heading", { name: "Your site", level: 1 }), + ).toBeVisible(); +} + +test.describe("Onboarding detect-site failure handling", () => { + test("failed detection does not refetch in a loop", async ({ page }) => { + let hits = 0; + await page.route(DETECT_GLOB, async (route) => { + hits += 1; + await route.fulfill({ + status: 500, + contentType: "application/json", + body: "{}", + }); + }); + + await openWizardToDetectStep(page); + + // Sit on the detect-site step and measure the request rate. + const baseline = hits; + await page.waitForTimeout(3000); + const delta = hits - baseline; + expect( + delta, + `expected at most 1 detect request while idle on a failed scan, saw ${delta}`, + ).toBeLessThanOrEqual(1); + + // The failure must surface as a real error state, and Continue must be blocked. + await expect(page.getByText("Could not scan your project")).toBeVisible(); + await expect(page.getByRole("button", { name: "Continue" })).toBeDisabled(); + }); + + test("retry triggers exactly one refetch and recovers on success", async ({ + page, + }) => { + let hits = 0; + let mode: "fail" | "pass" = "fail"; + await page.route(DETECT_GLOB, async (route) => { + hits += 1; + if (mode === "fail") { + await route.fulfill({ + status: 500, + contentType: "application/json", + body: "{}", + }); + return; + } + await route.continue(); + }); + + await openWizardToDetectStep(page); + await expect(page.getByText("Could not scan your project")).toBeVisible(); + + // Next call should reach the real backend and succeed. + const before = hits; + mode = "pass"; + await page.getByRole("button", { name: "Try again" }).click(); + + // Recovery: error clears and a real detection result renders. + await expect(page.getByText("Could not scan your project")).toBeHidden(); + const detected = page.getByText(/We found an? /); + const notDetected = page.getByText("We could not detect your site"); + await expect(detected.or(notDetected)).toBeVisible({ timeout: 15_000 }); + + // Exactly one additional request was made by the retry. + expect(hits - before).toBe(1); + }); +}); diff --git a/apps/studio/src/components/OnboardingWizard.tsx b/apps/studio/src/components/OnboardingWizard.tsx index 1fec4af..ab7a01a 100644 --- a/apps/studio/src/components/OnboardingWizard.tsx +++ b/apps/studio/src/components/OnboardingWizard.tsx @@ -43,7 +43,9 @@ export function OnboardingWizard({ }: OnboardingWizardProps) { const [step, setStep] = useState("welcome"); const [report, setReport] = useState(null); - const [loadingDetection, setLoadingDetection] = useState(false); + const [detectionError, setDetectionError] = useState(false); + const [retryCount, setRetryCount] = useState(0); + const [resolvedAttempt, setResolvedAttempt] = useState(-1); const [selectedAdapter, setSelectedAdapter] = useState(null); const [selectedContentRoot, setSelectedContentRoot] = useState( null, @@ -64,21 +66,47 @@ export function OnboardingWizard({ activeSuggestion?.contentDir ?? null; + // "attempted" is tracked by resolvedAttempt (set only when a fetch settles), + // kept distinct from the result so a failed scan does not re-arm the effect. + const detectionStatus: "idle" | "loading" | "loaded" | "error" = + step !== "detect-site" + ? "idle" + : resolvedAttempt !== retryCount + ? "loading" + : detectionError + ? "error" + : "loaded"; + useEffect(() => { - if (step !== "detect-site" || report !== null || loadingDetection) { + if (step !== "detect-site" || resolvedAttempt === retryCount) { return; } - setLoadingDetection(true); + let ignore = false; void fetchSetupDetection().then((next) => { + if (ignore) { + return; + } + + if (next === null) { + setDetectionError(true); + setResolvedAttempt(retryCount); + return; + } + setReport(next); - setLoadingDetection(false); - if (next?.primary) { + setDetectionError(false); + setResolvedAttempt(retryCount); + if (next.primary) { setSelectedAdapter(next.primary.adapter); setSelectedContentRoot(next.primary.contentRoot); } }); - }, [step, report, loadingDetection]); + + return () => { + ignore = true; + }; + }, [step, retryCount, resolvedAttempt]); const choices = detectionChoices(report); const ambiguous = report?.primary !== null && report?.safeToApply !== true; @@ -209,22 +237,29 @@ export function OnboardingWizard({ {step === "detect-site" && ( <> - {loadingDetection && ( + {detectionStatus === "loading" && (

Scanning your project…

)} - {!loadingDetection && report === null && ( + {detectionStatus === "error" && (

Could not scan your project

Confirm the publish service is running, then try again.

+
)} - {!loadingDetection && report && !report.primary && ( + {detectionStatus === "loaded" && report && !report.primary && (

We could not detect your site

@@ -234,7 +269,7 @@ export function OnboardingWizard({

)} - {report && activeSuggestion && ( + {detectionStatus === "loaded" && report && activeSuggestion && ( <>

{friendlyDetectionHeadline(activeSuggestion)} @@ -489,7 +524,9 @@ export function OnboardingWizard({ disabled={ applying || (step === "detect-site" && - (loadingDetection || (report !== null && !report.primary))) + (detectionStatus === "loading" || + detectionStatus === "error" || + (report !== null && !report.primary))) } onClick={() => { void handleNext();