From 0ccadc2669fc7649b54149fe47f1ff51c6b2c978 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Artur=20M=C4=99dryga=C5=82?= Date: Wed, 7 Oct 2026 16:46:01 +0200 Subject: [PATCH] fix(runner): mount the preview once when npm latest is ahead of the buckets (DEV-3344) A bare visit starts on DEFAULT_VERSION (the newest committed bucket pin) and swaps in npm `latest` when /api/versions answers. The mount effect did not wait for that answer, so once handsontable 18.1.2 was published ahead of the weekly bucket re-pin (18.1.1), every unpinned visit booted the preview on 18.1.1, tore it down, and booted again on 18.1.2: two Sandpack boots (or two container sessions) and an `abandoned` preview.ready_ms. That is what turned the o11y e2e workflow red on master. Hold the mount, not the starter fetch, until the version is known for a visit that pinned nothing. The deterministic regression test holds the versions response until the starter has loaded and asserts no `abandoned` point; it fails on the previous code. Co-Authored-By: Claude Opus 5.5 --- runner/apps/authoring/src/App.tsx | 16 ++++++--- runner/e2e/telemetry-faro.spec.ts | 57 +++++++++++++++++++++++++++++-- 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/runner/apps/authoring/src/App.tsx b/runner/apps/authoring/src/App.tsx index 35e1542cf..d181c9164 100644 --- a/runner/apps/authoring/src/App.tsx +++ b/runner/apps/authoring/src/App.tsx @@ -2053,10 +2053,18 @@ function Authoring({ }); return () => { cancelled = true; }; }, [version, nextVersion, versionsResolved]); - // True while a next-format version's real availability is still unknown: - // either /api/versions hasn't resolved yet, or the exists-check above is - // in flight. Blocks the runtime-mount effect until it's settled. - const versionPending = isNextPrereleaseVersion(version) && (!versionsResolved || versionCheckPending); + // True while the version to mount is still unknown, which blocks the + // runtime-mount effect until it's settled. Two cases: + // - a next-format version whose real availability is unknown: /api/versions + // hasn't resolved yet, or the exists-check above is in flight; + // - a visit that pinned nothing, still on the DEFAULT_VERSION placeholder: + // the versions fetch swaps in npm `latest`, and between an npm release and + // the next bucket re-pin that is a different version, so mounting first + // boots the preview twice (DEV-3344). Only the mount waits; the starter + // artifact still loads in parallel. + const versionPending = + (isNextPrereleaseVersion(version) && (!versionsResolved || versionCheckPending)) || + (!versionsResolved && !hadUrlVersion.current && version === DEFAULT_VERSION); // A candidate bucket absent from the static index (Sentry DEMOS-1C) is a // by-design outcome (ADR-0021 #2/#3, most selectable versions have no diff --git a/runner/e2e/telemetry-faro.spec.ts b/runner/e2e/telemetry-faro.spec.ts index 8ca620d94..c56985092 100644 --- a/runner/e2e/telemetry-faro.spec.ts +++ b/runner/e2e/telemetry-faro.spec.ts @@ -1,8 +1,18 @@ import { test, expect, type Route, type Page } from "@playwright/test"; import { spawn, execSync, type ChildProcess } from "node:child_process"; import { fileURLToPath } from "node:url"; -import { activeEditor, flushFaro, previewReady, stubShell, abortSandpackHosts } from "./helpers.js"; +import { readFileSync } from "node:fs"; +import { activeEditor, flushFaro, previewReady, stubShell, abortSandpackHosts, workspaceFiles } from "./helpers.js"; import { fingerprint } from "../packages/runtime/src/telemetry/fingerprint.js"; +import { stableBucketVersions } from "../packages/runtime/src/version.js"; + +// The version a bare visit starts on (`catalog.ts`'s DEFAULT_VERSION), read +// from `catalog.json` so the weekly bucket re-pin cannot leave it stale here. +const DEFAULT_VERSION = stableBucketVersions( + (JSON.parse(readFileSync(new URL("../catalog.json", import.meta.url), "utf8")) as { bucketVersions: Record }) + .bucketVersions, +)[0]; +const newerPatch = (v: string) => v.replace(/\d+$/, (patch) => String(Number(patch) + 1)); // Faro in the authoring app. Gated: needs a dist built with // VITE_TELEMETRY_LOCAL=1 (contract ยง10), served on its own port (never @@ -656,8 +666,8 @@ test.describe("Faro in the authoring app", () => { await stubShell(page); const captured = captureTelemetry(page); await page.goto("/"); - // The stubbed version list remounts the preview once after load, and a - // remount closes the open burst; drive the ladder after it. + // The preview mounts once the stubbed version list resolves, and a mount + // closes the open burst; drive the ladder after it. await expect(page).toHaveURL(/[?&]v=18\.0\.0\b/); const relay = (message: string, sentry = false) => @@ -921,6 +931,47 @@ test.describe("Faro in the authoring app", () => { expect(record!.type).toBe("DemoError"); expect(uncaught()[0]!.context).toMatchObject({ "hot.surface": "demo-runtime", "hot.framework": "javascript" }); }); + + // Between an npm release and the weekly bucket re-pin, npm `latest` is newer + // than the default version a bare visit starts on. Mounting before + // /api/versions answered booted the preview on the default, then remounted on + // `latest`: two Sandpack boots and an `abandoned` preview.ready_ms on every + // such visit (DEV-3344). The versions response is held until the starter + // artifact has loaded, which is the order that remounted. The oracle is the + // `abandoned` point the first mount's cleanup emits; the aborted bundler + // keeps the preview booting, so nothing else settles the tracker first. + test("a bare visit mounts the preview once when npm latest is newer than the default version", async ({ page }) => { + await stubShell(page); + const latest = newerPatch(DEFAULT_VERSION); + let releaseVersions!: () => void; + const versionsHeld = new Promise((resolve) => { releaseVersions = resolve; }); + // Registered after stubShell, so it takes precedence over the stub there. + await page.route("**/api/versions", async (route) => { + await versionsHeld; + await route.fulfill({ json: { latest, next: "19.0.0-next.1", versions: [latest, DEFAULT_VERSION] } }); + }); + const captured = captureTelemetry(page); + const bundlerNavigations: string[] = []; + page.on("request", (r) => { + if (r.isNavigationRequest() && /sandpack/.test(new URL(r.url()).hostname)) bundlerNavigations.push(r.url()); + }); + const pinnedCore = async () => /"handsontable": "([^"]+)"/.exec((await workspaceFiles(page))["/package.json"] ?? "")?.[1]; + + await page.goto("/?example=react"); + await page.waitForFunction(() => "__HOT_FILES__" in window); + await expect.poll(pinnedCore, { message: "the starter artifact loaded on the default version" }).toBe(DEFAULT_VERSION); + releaseVersions(); + // The same-bucket re-pin runs in the commit that would have torn down the first mount. + await expect.poll(pinnedCore, { message: "the workspace moved to npm latest" }).toBe(latest); + await expect.poll(() => bundlerNavigations.length, { message: "the preview mounted" }).toBeGreaterThan(0); + await flushFaro(page, eventSeen(captured)); + + const abandoned = captured + .flatMap((b) => b.measurements ?? []) + .filter((m) => m.type === "preview.ready_ms") + .filter((m) => (m.context as Record | undefined)?.["hot.outcome"] === "abandoned"); + expect(abandoned, "no preview was torn down by the version swap").toEqual([]); + }); }); // ---- Sentry scope switch = uncaught ----------------------------------------