From a5fbc980416661f4ac89ffcb34e7d306422261ef Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Wed, 4 Nov 2026 11:32:07 +0530 Subject: [PATCH 1/7] feat(frontend-v1): version switch and route registry (UN-4271) Typed route registry for v1 pages (declare in PAGES, go live with one line in REGISTERED_PAGES), validated by a unit test that fails CI on overlapping patterns, unmapped v0 routes, missing components or a toggle outside Human Review (D14). VersionSwitch wraps v0's routes in Router.jsx: unregistered URLs render v0 unchanged, registered pages render v1 as their own lazily loaded tree, and their v0 URLs redirect to the mapped v1 route. Human Review's old-version preference is per user, per browser (localStorage); setOldVersion() moves the user to the mapped screen without a reload. VITE_V1_DEV_ROUTES (set only for the E2E image in ci-test.yaml) registers demo pages that prove the switch and honours a preview override for declared-but-unregistered pages. resolveUrl() resolves a page's v1 route to the URL the user gets, for global search (UN-4278). --- .github/workflows/ci-test.yaml | 1 + docker/dockerfiles/frontend.Dockerfile | 5 + frontend/src/routes/Router.jsx | 232 ++++++++++-------- frontend/src/v1/README.md | 28 ++- frontend/src/v1/app/.gitkeep | 0 frontend/src/v1/app/V1App.tsx | 32 +++ frontend/src/v1/app/VersionSwitch.test.tsx | 136 ++++++++++ frontend/src/v1/app/VersionSwitch.tsx | 103 ++++++++ frontend/src/v1/app/pages.ts | 21 ++ frontend/src/v1/app/registry/entries.ts | 48 ++++ frontend/src/v1/app/registry/live.test.ts | 70 ++++++ frontend/src/v1/app/registry/live.ts | 111 +++++++++ frontend/src/v1/app/registry/patterns.ts | 60 +++++ frontend/src/v1/app/registry/resolve.test.ts | 218 ++++++++++++++++ frontend/src/v1/app/registry/resolve.ts | 120 +++++++++ frontend/src/v1/app/registry/types.ts | 49 ++++ frontend/src/v1/app/registry/validate.test.ts | 164 +++++++++++++ frontend/src/v1/app/registry/validate.ts | 104 ++++++++ frontend/src/v1/index.ts | 48 +++- frontend/src/v1/lib/oldVersion.ts | 72 ++++++ frontend/src/v1/pages/.gitkeep | 0 frontend/src/v1/pages/demo/index.tsx | 35 +++ frontend/src/v1/stores/oldVersion.test.ts | 104 ++++++++ frontend/src/v1/stores/oldVersion.ts | 87 +++++++ 24 files changed, 1748 insertions(+), 100 deletions(-) delete mode 100644 frontend/src/v1/app/.gitkeep create mode 100644 frontend/src/v1/app/V1App.tsx create mode 100644 frontend/src/v1/app/VersionSwitch.test.tsx create mode 100644 frontend/src/v1/app/VersionSwitch.tsx create mode 100644 frontend/src/v1/app/pages.ts create mode 100644 frontend/src/v1/app/registry/entries.ts create mode 100644 frontend/src/v1/app/registry/live.test.ts create mode 100644 frontend/src/v1/app/registry/live.ts create mode 100644 frontend/src/v1/app/registry/patterns.ts create mode 100644 frontend/src/v1/app/registry/resolve.test.ts create mode 100644 frontend/src/v1/app/registry/resolve.ts create mode 100644 frontend/src/v1/app/registry/types.ts create mode 100644 frontend/src/v1/app/registry/validate.test.ts create mode 100644 frontend/src/v1/app/registry/validate.ts create mode 100644 frontend/src/v1/lib/oldVersion.ts delete mode 100644 frontend/src/v1/pages/.gitkeep create mode 100644 frontend/src/v1/pages/demo/index.tsx create mode 100644 frontend/src/v1/stores/oldVersion.test.ts create mode 100644 frontend/src/v1/stores/oldVersion.ts diff --git a/.github/workflows/ci-test.yaml b/.github/workflows/ci-test.yaml index dc79608971..ffcb574a9d 100644 --- a/.github/workflows/ci-test.yaml +++ b/.github/workflows/ci-test.yaml @@ -230,6 +230,7 @@ jobs: set: | *.context=. *.args.VERSION=${{ github.sha }} + frontend.args.VITE_V1_DEV_ROUTES=true frontend.cache-from=type=gha,scope=frontend frontend.cache-to=type=gha,mode=max,scope=frontend backend.cache-from=type=gha,scope=backend diff --git a/docker/dockerfiles/frontend.Dockerfile b/docker/dockerfiles/frontend.Dockerfile index 0c3fdcc92a..f9d4af6ab3 100644 --- a/docker/dockerfiles/frontend.Dockerfile +++ b/docker/dockerfiles/frontend.Dockerfile @@ -38,6 +38,11 @@ CMD ["/bin/sh", "-c", "/app/generate-runtime-config.sh && bun run start"] FROM base AS builder ARG BUILD_CONTEXT_PATH ENV VITE_BACKEND_URL="" +# "true" only for the E2E image (ci-test.yaml): registers the v1 demo pages +# that prove the version switch, and honours the v1 preview override. +# Release builds leave it empty. +ARG VITE_V1_DEV_ROUTES="" +ENV VITE_V1_DEV_ROUTES=${VITE_V1_DEV_ROUTES} # Copy package files and install dependencies COPY ${BUILD_CONTEXT_PATH}/package.json ${BUILD_CONTEXT_PATH}/bun.lock ./ diff --git a/frontend/src/routes/Router.jsx b/frontend/src/routes/Router.jsx index 05bcea6b18..93f21a2ac4 100644 --- a/frontend/src/routes/Router.jsx +++ b/frontend/src/routes/Router.jsx @@ -1,5 +1,6 @@ +import { setOldVersion, VersionSwitch, v1DevRoutesEnabled } from "@v1"; import { Suspense } from "react"; -import { Route, Routes, useLocation } from "react-router-dom"; +import { Route, Routes, useLocation, useParams } from "react-router-dom"; import { GenericError } from "../components/error/GenericError/GenericError.jsx"; import { @@ -17,6 +18,7 @@ import { lazyNamed } from "../helpers/lazyNamed.js"; import { isModuleMissing } from "../helpers/pluginLoader.js"; import { lazyPlugin } from "../helpers/pluginRegistry.js"; import { LandingPage } from "../pages/LandingPage.jsx"; +import { useSessionStore } from "../store/session-store.js"; import { useMainAppRoutes } from "./useMainAppRoutes.jsx"; // Heavy route pages are code-split so the unauthenticated /landing page does @@ -145,115 +147,151 @@ try { } } +// Stand-ins for the v0 side of the v1 demo pages (UN-4271), which prove the +// version switch. Rendered only in a build with VITE_V1_DEV_ROUTES=true. +function V0DemoPage() { + return

v0 demo page

; +} + +function V0DemoReviewPage() { + const { orgName } = useParams(); + return ( +
+

v0 demo review

+

{orgName}

+ +
+ ); +} + function Router() { const location = useLocation(); const MainAppRoute = useMainAppRoutes(); + const userId = useSessionStore((state) => state.sessionDetails?.id); return ( - // App-wide backstop: catches shell-level chunk failures and errors on - // routes that don't sit inside a LazyOutlet layout. Per-page load errors - // are handled by the scoped boundaries inside the layouts. resetKeys lets - // navigation clear the error without a full reload. - } - > - }> - - } /> - }> - {/* public routes */} - - {/* public routes accessible only to unauthenticated users */} - }> - } /> - + // Renders v1 for a URL whose page is registered in the v1 route + // registry, and these v0 routes for everything else (UN-4271). + + {/* App-wide backstop: catches shell-level chunk failures and errors on + routes that don't sit inside a LazyOutlet layout. Per-page load + errors are handled by the scoped boundaries inside the layouts. + resetKeys lets navigation clear the error without a full reload. */} + } + > + }> + + } /> + }> + {/* public routes */} + + {/* public routes accessible only to unauthenticated users */} + }> + } /> + - {/* public routes accessible to both authenticated and unauthenticated users */} - {SimplePromptStudioHelper && - SimplePromptStudio && - SpsLanding && - SpsUpload && ( + {/* public routes accessible to both authenticated and unauthenticated users */} + {SimplePromptStudioHelper && + SimplePromptStudio && + SpsLanding && + SpsUpload && ( + } + > + } /> + } /> + } /> + + )} + {PublicPromptStudioHelper && ( } + path="/promptStudio/share/:id" + element={} > - } /> - } /> - } /> + } /> + } + /> )} - {PublicPromptStudioHelper && ( + + + {/* protected routes */} + } /> + {SelectProduct && ( + } /> + )} + {UnstractSubscriptionEndPage && ( } - > - } /> - } - /> - + path="/subscription-expired" + element={} + /> )} - - - {/* protected routes */} - } /> - {SelectProduct && ( - } /> - )} - {UnstractSubscriptionEndPage && ( - } - /> - )} - {PaymentSuccessful && ( - } /> - )} - {MarketplaceLandingEntry && ( - } - /> - )} - {MarketplaceStripeConflictEntry && ( - } - /> - )} - {LlmWhispererMarketplaceLandingEntry && ( - } - /> - )} - {CustomPlanCheckoutPage && ( - } - /> - )} - {LlmWhispererCustomCheckoutPage && ( - } - /> - )} - }> - {MainAppRoute} - {llmWhispererRouter && ( - {llmWhispererRouter()} + {PaymentSuccessful && ( + } + /> + )} + {MarketplaceLandingEntry && ( + } + /> + )} + {MarketplaceStripeConflictEntry && ( + } + /> + )} + {LlmWhispererMarketplaceLandingEntry && ( + } + /> )} + {CustomPlanCheckoutPage && ( + } + /> + )} + {LlmWhispererCustomCheckoutPage && ( + } + /> + )} + }> + {MainAppRoute} + {llmWhispererRouter && ( + {llmWhispererRouter()} + )} + {v1DevRoutesEnabled && ( + <> + } /> + } + /> + + )} + + {verticalsRouter && verticalsRouter()} - {verticalsRouter && verticalsRouter()} - - } /> - } /> - - - + } /> + } /> + + + + ); } diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index 0bf54e99f7..a59ca32f8c 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -64,6 +64,32 @@ is convention. icon's `size` prop. Put icons in a `ui:flex`/`ui:inline-flex` parent that is not one of those tags, or size the parent's font. +## Version switch and route registry + +v0's router (`routes/Router.jsx`) wraps its routes in `VersionSwitch`, which +renders v1 for a URL whose page is live in the registry (`app/registry/`) and +v0 for everything else. v1 renders as its own tree (`app/V1App.tsx`), loaded +only when a v1 page first renders. + +- **Declare** a page in `app/registry/entries.ts` (`PAGES`) with its v1 routes, + the v0 routes it replaces and the mapping both ways, and add its component + to `app/pages.ts`. Declared pages stay v0 for users. +- **Register** it, in its go-live task only (D9), by adding its key to + `REGISTERED_PAGES`. Its v0 URLs then redirect to v1; reverting is that line. +- `app/registry/validate.test.ts` fails CI on an invalid entry: overlapping + patterns across pages, an unmapped v0 route, a page without a component, or + `toggle` on any page but Human Review (UN11, D14). +- Human Review's old-version preference is per user, per browser, in + localStorage (`unstract.v1.oldVersion.`). `setOldVersion(on)` sets + it, and the switch then moves the user to the mapped screen without a + reload. +- `VITE_V1_DEV_ROUTES=true` (the E2E image, or `.env` locally) registers the + demo pages (`/:orgName/v1-demo`, `/:orgName/v1-demo-review`, and their v0 + stand-ins `/:orgName/v0-demo…`). It also renders declared pages as v1 before + they go live: `localStorage["unstract.v1.preview"] = '["UN05"]'`. +- `VITE_UNSTRACT_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) drops + entries for distributions the build lacks. + ## Folders and what each may import Every folder may import from itself. Test files (`*.test.ts(x)`, @@ -71,7 +97,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | Folder | Holds | May import from | | --- | --- | --- | -| `index.ts` | Public entry; the version switch (UN-4271) will mount v1 through it | everything | +| `index.ts` | Public entry: the version switch, `setOldVersion`, `resolveUrl` | everything | | `app/` | Entry, providers, router, route registry | everything | | `components/ui/` | Core components | `lib/`, `styles/`, `assets/` | | `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/`, `assets/` | diff --git a/frontend/src/v1/app/.gitkeep b/frontend/src/v1/app/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/frontend/src/v1/app/V1App.tsx b/frontend/src/v1/app/V1App.tsx new file mode 100644 index 0000000000..00ec09eb6c --- /dev/null +++ b/frontend/src/v1/app/V1App.tsx @@ -0,0 +1,32 @@ +/* + * The v1 tree: its own providers and routes, never inside a v0 layout or + * guard. Loaded lazily by the version switch, so v1's code and styles reach + * the browser only when a v1 page renders. Session, guards (P0-T06) and the + * shell (P2-T01) mount here. + */ + +import type { RegistryEntry } from "@v1/app/registry/types"; +import { ThemeProvider } from "@v1/app/theme/ThemeProvider"; +import { type JSX, Suspense } from "react"; +import { Route, Routes } from "react-router-dom"; +import { PAGE_COMPONENTS } from "./pages"; + +export type V1AppProps = Readonly<{ entry: RegistryEntry }>; + +export function V1App({ entry }: V1AppProps): JSX.Element { + const Page = PAGE_COMPONENTS[entry.page]; + if (!Page) { + throw new Error(`v1 page ${entry.page} is registered but has no component`); + } + return ( + + + + {entry.v1Routes.map((path) => ( + } /> + ))} + + + + ); +} diff --git a/frontend/src/v1/app/VersionSwitch.test.tsx b/frontend/src/v1/app/VersionSwitch.test.tsx new file mode 100644 index 0000000000..cf5a54ee2d --- /dev/null +++ b/frontend/src/v1/app/VersionSwitch.test.tsx @@ -0,0 +1,136 @@ +import { act, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { buildLiveRegistry, type LiveRegistry } from "@v1/app/registry/live"; +import { oldVersionStorageKey } from "@v1/lib/oldVersion"; +import { useOldVersionStore } from "@v1/stores/oldVersion"; +import { beforeEach, describe, expect, it } from "vitest"; +import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; +import { VersionSwitch } from "./VersionSwitch"; + +const demo = buildLiveRegistry({ + devRoutes: true, + distribution: "oss", + storage: null, +}); +const empty: LiveRegistry = { pages: [], entries: [] }; + +function Where() { + return {useLocation().pathname}; +} + +/** Stands in for v0's routes. */ +const v0 = ( + + v0 demo

} /> + v0 review

} + /> + v0 other

} /> + +); + +function renderAt( + path: string, + { + registry = demo, + userId = "7", + }: { registry?: LiveRegistry; userId?: string | null } = {}, +) { + const view = (user: string | null) => ( + + + + {v0} + + + ); + const result = render(view(userId)); + return { + ...result, + setUser: (user: string | null) => result.rerender(view(user)), + }; +} + +const where = () => screen.getByTestId("where").textContent; + +beforeEach(() => { + window.localStorage.clear(); + useOldVersionStore.setState({ userId: null, oldVersion: null, chosen: {} }); +}); + +describe("VersionSwitch", () => { + it("with an empty registry renders v0 for every URL, including v0 URLs of demo pages", () => { + for (const path of ["/acme/v0-demo", "/acme/v1-demo", "/acme/workflows"]) { + const { unmount } = renderAt(path, { registry: empty }); + expect(screen.getByTestId("v0")).toBeTruthy(); + expect(where()).toBe(path); + unmount(); + } + }); + + it("renders v0 for an unregistered URL", () => { + renderAt("/acme/workflows"); + expect(screen.getByTestId("v0").textContent).toBe("v0 other"); + }); + + it("renders a registered page as its own v1 tree, and redirects its v0 URL there", async () => { + renderAt("/acme/v0-demo?x=1"); + expect(await screen.findByTestId("v1-demo")).toBeTruthy(); + expect(where()).toBe("/acme/v1-demo"); + expect(screen.queryByTestId("v0")).toBeNull(); + expect( + screen.getByTestId("v1-demo").closest("[data-ui-version='v1']"), + ).not.toBeNull(); + }); + + it("Human Review round trip: old version maps to v0 and back, with no reload", async () => { + const user = userEvent.setup(); + renderAt("/acme/v1-demo-review/details"); + await user.click( + await screen.findByRole("button", { name: "Switch to old version" }), + ); + + // The v1-only sub-route lands on its nearest v0 route. + expect(where()).toBe("/acme/v0-demo-review"); + expect(screen.getByTestId("v0").textContent).toBe("v0 review"); + expect(window.localStorage.getItem(oldVersionStorageKey("7"))).toBe("true"); + + act(() => { + useOldVersionStore.getState().setOldVersion(false); + }); + expect(where()).toBe("/acme/v1-demo-review"); + expect(await screen.findByTestId("v1-demo-review")).toBeTruthy(); + expect(window.localStorage.getItem(oldVersionStorageKey("7"))).toBeNull(); + }); + + it("a stored old-version choice applies on load (reload, re-login)", () => { + window.localStorage.setItem(oldVersionStorageKey("7"), "true"); + renderAt("/acme/v1-demo-review"); + expect(where()).toBe("/acme/v0-demo-review"); + expect(screen.getByTestId("v0")).toBeTruthy(); + }); + + it("the preference does nothing on a page without the toggle", async () => { + window.localStorage.setItem(oldVersionStorageKey("7"), "true"); + renderAt("/acme/v0-demo"); + expect(await screen.findByTestId("v1-demo")).toBeTruthy(); + }); + + it("keeps Human Review's v0 URL on v0 until the user is known, then applies their choice", async () => { + const { setUser } = renderAt("/acme/v0-demo-review", { userId: null }); + expect(screen.getByTestId("v0")).toBeTruthy(); + expect(where()).toBe("/acme/v0-demo-review"); + + setUser("7"); + expect(await screen.findByTestId("v1-demo-review")).toBeTruthy(); + expect(where()).toBe("/acme/v1-demo-review"); + }); + + it("is per user: another user signing in does not inherit the old version", async () => { + window.localStorage.setItem(oldVersionStorageKey("7"), "true"); + useOldVersionStore.setState({ userId: "7", oldVersion: true }); + renderAt("/acme/v0-demo-review", { userId: "8" }); + expect(await screen.findByTestId("v1-demo-review")).toBeTruthy(); + }); +}); diff --git a/frontend/src/v1/app/VersionSwitch.tsx b/frontend/src/v1/app/VersionSwitch.tsx new file mode 100644 index 0000000000..d715082b6d --- /dev/null +++ b/frontend/src/v1/app/VersionSwitch.tsx @@ -0,0 +1,103 @@ +/* + * The version switch (UN-4271): renders v0 or v1 per URL. v0's router mounts + * it around its own routes, as `children`, which render untouched whenever + * the URL resolves to v0. + */ + +import { LIVE_REGISTRY, type LiveRegistry } from "@v1/app/registry/live"; +import { resolveRoute } from "@v1/app/registry/resolve"; +import type { RegistryEntry } from "@v1/app/registry/types"; +import { useOldVersionStore } from "@v1/stores/oldVersion"; +import { + Component, + type JSX, + lazy, + type ReactNode, + Suspense, + useLayoutEffect, +} from "react"; +import { Navigate, useLocation } from "react-router-dom"; + +const V1App = lazy(() => import("./V1App").then((m) => ({ default: m.V1App }))); + +export type VersionSwitchProps = Readonly<{ + /** The signed-in user's id from the v0 session; null or absent while signed out. */ + userId?: string | number | null; + /** The v0 app, rendered when the URL resolves to v0. */ + children: ReactNode; + /** For tests; defaults to this build's registry. */ + registry?: LiveRegistry; +}>; + +type BoundaryState = { failed: boolean }; + +/** Keeps a v1 failure (a stale chunk after a deploy) from unmounting the whole app. */ +class V1ErrorBoundary extends Component< + { children: ReactNode }, + BoundaryState +> { + override state: BoundaryState = { failed: false }; + + static getDerivedStateFromError(): BoundaryState { + return { failed: true }; + } + + override render(): ReactNode { + if (this.state.failed) { + return ( +

+ This page could not be loaded. Reload to try again. +

+ ); + } + return this.props.children; + } +} + +function V1Root({ entry }: Readonly<{ entry: RegistryEntry }>): JSX.Element { + return ( + + + + + + ); +} + +function toUserId(userId: string | number | null | undefined): string | null { + return userId === undefined || userId === null || userId === "" + ? null + : String(userId); +} + +export function VersionSwitch({ + userId, + children, + registry = LIVE_REGISTRY, +}: VersionSwitchProps): JSX.Element { + const location = useLocation(); + const user = toUserId(userId); + const storeUser = useOldVersionStore((s) => s.userId); + const storedChoice = useOldVersionStore((s) => s.oldVersion); + + useLayoutEffect(() => { + useOldVersionStore.getState().setUser(user); + }, [user]); + + // Until the store has caught up with a newly signed-in user, their choice + // is unknown, so the previous user's never redirects them. + const oldVersion = user !== null && user !== storeUser ? null : storedChoice; + const resolution = resolveRoute(location, { + entries: registry.entries, + oldVersion, + }); + + switch (resolution.kind) { + case "v0": + return <>{children}; + case "redirect": + return ; + case "v1": + return ; + } +} diff --git a/frontend/src/v1/app/pages.ts b/frontend/src/v1/app/pages.ts new file mode 100644 index 0000000000..9cb543e7c6 --- /dev/null +++ b/frontend/src/v1/app/pages.ts @@ -0,0 +1,21 @@ +/* + * The component for each declared page, loaded on first render. A page's + * go-live task adds its line here with its registry entry; the registry + * test fails if a declared page has none. + */ + +import type { PageKey } from "@v1/app/registry/types"; +import { type JSX, type LazyExoticComponent, lazy } from "react"; + +type PageComponent = LazyExoticComponent<() => JSX.Element>; + +export const PAGE_COMPONENTS: Readonly< + Partial> +> = { + "DEMO-PAGE": lazy(() => + import("@v1/pages/demo").then((m) => ({ default: m.DemoPage })), + ), + "DEMO-REVIEW": lazy(() => + import("@v1/pages/demo").then((m) => ({ default: m.DemoReviewPage })), + ), +}; diff --git a/frontend/src/v1/app/registry/entries.ts b/frontend/src/v1/app/registry/entries.ts new file mode 100644 index 0000000000..9286a6022c --- /dev/null +++ b/frontend/src/v1/app/registry/entries.ts @@ -0,0 +1,48 @@ +/* + * The route registry data (UN-4271). Pure data: see the note in ./types.ts. + * + * A page is declared in PAGES once its v1 routes exist, and goes live when + * its go-live task adds its key to REGISTERED_PAGES (D9). Going live, and + * reverting it, is that one line. + */ + +import type { PageKey, RegistryEntry } from "./types"; + +/** Every declared v1 page, live or not. */ +export const PAGES: readonly RegistryEntry[] = []; + +/** The pages users get as v1. Empty: the app renders v0 everywhere. */ +export const REGISTERED_PAGES: readonly PageKey[] = []; + +/** + * Development-only demo pages that prove the switch. Registered only in a + * build with VITE_V1_DEV_ROUTES=true (the E2E image), never in a release. + * Their v0 side is the stand-in rendered by routes/Router.jsx. + */ +export const DEMO_PAGES: readonly RegistryEntry[] = [ + { + page: "DEMO-PAGE", + v1Routes: ["/:orgName/v1-demo"], + replaces: ["/:orgName/v0-demo"], + toV0: {}, + toV1: { "/:orgName/v0-demo": "/:orgName/v1-demo" }, + toggle: false, + distributions: ["oss", "cloud", "onprem"], + registeredBy: "P0-T03", + }, + { + // Shaped like Human Review: honours the old-version preference, and has + // a v1-only sub-route that maps back to the nearest v0 route. + page: "DEMO-REVIEW", + v1Routes: ["/:orgName/v1-demo-review", "/:orgName/v1-demo-review/details"], + replaces: ["/:orgName/v0-demo-review"], + toV0: { + "/:orgName/v1-demo-review": "/:orgName/v0-demo-review", + "/:orgName/v1-demo-review/details": "/:orgName/v0-demo-review", + }, + toV1: { "/:orgName/v0-demo-review": "/:orgName/v1-demo-review" }, + toggle: true, + distributions: ["oss", "cloud", "onprem"], + registeredBy: "P0-T03", + }, +]; diff --git a/frontend/src/v1/app/registry/live.test.ts b/frontend/src/v1/app/registry/live.test.ts new file mode 100644 index 0000000000..5ea0b458b3 --- /dev/null +++ b/frontend/src/v1/app/registry/live.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from "vitest"; +import { DEMO_PAGES } from "./entries"; +import { + buildLiveRegistry, + PREVIEW_STORAGE_KEY, + parseDistribution, + readPreviewPages, +} from "./live"; + +const storageWith = (value: string | null) => ({ getItem: () => value }); + +describe("parseDistribution", () => { + it("defaults to OSS and accepts any case", () => { + expect(parseDistribution(undefined)).toBe("oss"); + expect(parseDistribution("")).toBe("oss"); + expect(parseDistribution(" Cloud ")).toBe("cloud"); + expect(parseDistribution("onprem")).toBe("onprem"); + }); + it("fails the build on an unknown value", () => { + expect(() => parseDistribution("saas")).toThrow( + "VITE_UNSTRACT_DISTRIBUTION=saas", + ); + }); +}); + +describe("readPreviewPages", () => { + it("reads a JSON array of page keys", () => { + expect(readPreviewPages(storageWith('["UN05", 3, "LW02"]'))).toEqual([ + "UN05", + "LW02", + ]); + }); + it("treats missing, malformed or unreadable storage as no preview", () => { + expect(readPreviewPages(null)).toEqual([]); + expect(readPreviewPages(storageWith(null))).toEqual([]); + expect(readPreviewPages(storageWith("{"))).toEqual([]); + expect(readPreviewPages(storageWith('{"UN05":1}'))).toEqual([]); + expect( + readPreviewPages({ + getItem: () => { + throw new Error("denied"); + }, + }), + ).toEqual([]); + }); + it("uses its own key", () => { + expect(PREVIEW_STORAGE_KEY).toBe("unstract.v1.preview"); + }); +}); + +describe("buildLiveRegistry", () => { + it("without dev routes: no demo pages, and the preview override is ignored", () => { + const r = buildLiveRegistry({ + devRoutes: false, + distribution: "oss", + storage: storageWith('["DEMO-PAGE"]'), + }); + expect(r.pages.some((p) => p.page.startsWith("DEMO-"))).toBe(false); + expect(r.entries).toEqual([]); + }); + + it("with dev routes: the demo pages are declared and live", () => { + const r = buildLiveRegistry({ + devRoutes: true, + distribution: "oss", + storage: null, + }); + expect(r.entries.map((e) => e.page)).toEqual(DEMO_PAGES.map((p) => p.page)); + }); +}); diff --git a/frontend/src/v1/app/registry/live.ts b/frontend/src/v1/app/registry/live.ts new file mode 100644 index 0000000000..7ed758dd1b --- /dev/null +++ b/frontend/src/v1/app/registry/live.ts @@ -0,0 +1,111 @@ +/* + * The registry this build and browser run with: the pure data in ./entries, + * narrowed by build flags and the dev/test preview override. + */ + +import { DEMO_PAGES, PAGES, REGISTERED_PAGES } from "./entries"; +import { activeEntries } from "./resolve"; +import { + DISTRIBUTIONS, + type Distribution, + type PageKey, + type RegistryEntry, +} from "./types"; +import { registryProblems } from "./validate"; + +/** + * VITE_V1_DEV_ROUTES=true registers the demo pages and honours the preview + * override. Set only for the E2E image and local development, never in a + * release build. + */ +export const V1_DEV_ROUTES: boolean = + import.meta.env.VITE_V1_DEV_ROUTES === "true"; + +/** + * Dev/test override: a JSON array of declared page keys to render as v1 + * before they are registered (D9), e.g. `["UN05"]`. Honoured only with + * V1_DEV_ROUTES. + */ +export const PREVIEW_STORAGE_KEY = "unstract.v1.preview"; + +function isDistribution(value: unknown): value is Distribution { + return (DISTRIBUTIONS as readonly unknown[]).includes(value); +} + +/** The build's distribution, from VITE_UNSTRACT_DISTRIBUTION; OSS when unset. */ +export function parseDistribution(raw: string | undefined): Distribution { + const value = raw?.trim().toLowerCase() || "oss"; + if (!isDistribution(value)) { + throw new Error( + `VITE_UNSTRACT_DISTRIBUTION=${raw} is not one of ${DISTRIBUTIONS.join(", ")}`, + ); + } + return value; +} + +/** Page keys in the preview override; anything unreadable or malformed means none. */ +export function readPreviewPages( + storage: Pick | null | undefined, +): PageKey[] { + try { + const parsed: unknown = JSON.parse( + storage?.getItem(PREVIEW_STORAGE_KEY) ?? "[]", + ); + return Array.isArray(parsed) + ? parsed.filter((k): k is PageKey => typeof k === "string") + : []; + } catch { + return []; + } +} + +export type LiveRegistry = Readonly<{ + /** Every page declared in this build, live or not. */ + pages: readonly RegistryEntry[]; + /** The pages that render as v1. */ + entries: readonly RegistryEntry[]; +}>; + +export function buildLiveRegistry(options: { + devRoutes: boolean; + distribution: Distribution; + storage: Pick | null; +}): LiveRegistry { + const { devRoutes, distribution, storage } = options; + const pages = devRoutes ? [...PAGES, ...DEMO_PAGES] : [...PAGES]; + const live = new Set(REGISTERED_PAGES); + if (devRoutes) { + for (const key of [ + ...DEMO_PAGES.map((p) => p.page), + ...readPreviewPages(storage), + ]) { + live.add(key); + } + } + return { pages, entries: activeEntries(pages, live, distribution) }; +} + +function browserStorage(): Storage | null { + try { + return typeof window === "undefined" ? null : window.localStorage; + } catch { + return null; + } +} + +function load(): LiveRegistry { + const registry = buildLiveRegistry({ + devRoutes: V1_DEV_ROUTES, + distribution: parseDistribution(import.meta.env.VITE_UNSTRACT_DISTRIBUTION), + storage: browserStorage(), + }); + // CI rejects an invalid registry through its unit test; fail loudly in development too. + const problems = registryProblems(registry.pages); + if (import.meta.env.DEV && problems.length) { + throw new Error(`Invalid v1 route registry:\n${problems.join("\n")}`); + } + return registry; +} + +/** Read once at load: the preview override is set before the app starts. */ +export const LIVE_REGISTRY: LiveRegistry = load(); diff --git a/frontend/src/v1/app/registry/patterns.ts b/frontend/src/v1/app/registry/patterns.ts new file mode 100644 index 0000000000..ed6367425b --- /dev/null +++ b/frontend/src/v1/app/registry/patterns.ts @@ -0,0 +1,60 @@ +/** Route-pattern helpers for the registry: the subset of React Router patterns it accepts. */ + +const PARAM = /^:[A-Za-z_]\w*$/; +const STATIC = /^[A-Za-z0-9._~-]+$/; + +function segments(pattern: string): string[] { + return pattern.split("/").filter((s) => s !== ""); +} + +/** + * Why a pattern is not accepted, or null. Accepted: absolute, static + * segments, `:name` params and a final `*`. Optional segments (`:id?`) are + * not, so that overlap checking stays exact. + */ +export function patternProblem(pattern: string): string | null { + if (!pattern.startsWith("/")) { + return "must start with /"; + } + const segs = segments(pattern); + for (const [i, seg] of segs.entries()) { + if (seg === "*") { + if (i !== segs.length - 1) { + return "* is allowed only as the last segment"; + } + } else if (!PARAM.test(seg) && !STATIC.test(seg)) { + return `segment '${seg}' is not a static name, :param or a final *`; + } + } + return null; +} + +/** The `:name` params of a pattern, plus `*` when it ends in a splat. */ +export function paramNames(pattern: string): string[] { + return segments(pattern) + .filter((s) => s === "*" || s.startsWith(":")) + .map((s) => (s === "*" ? "*" : s.slice(1))); +} + +/** True when some URL matches both patterns. Static segments compare case-insensitively, as React Router does. */ +export function patternsOverlap(a: string, b: string): boolean { + const sa = segments(a); + const sb = segments(b); + for (let i = 0; ; i++) { + const x = sa[i]; + const y = sb[i]; + if (x === "*" || y === "*") { + return true; // a splat also matches nothing, so this holds even when the other pattern ended + } + if (x === undefined || y === undefined) { + return x === y; + } + if ( + !x.startsWith(":") && + !y.startsWith(":") && + x.toLowerCase() !== y.toLowerCase() + ) { + return false; + } + } +} diff --git a/frontend/src/v1/app/registry/resolve.test.ts b/frontend/src/v1/app/registry/resolve.test.ts new file mode 100644 index 0000000000..1633dea2fa --- /dev/null +++ b/frontend/src/v1/app/registry/resolve.test.ts @@ -0,0 +1,218 @@ +import { describe, expect, it } from "vitest"; +import { DEMO_PAGES, PAGES } from "./entries"; +import { activeEntries, resolvePageUrl, resolveRoute } from "./resolve"; +import type { RegistryEntry } from "./types"; +import { registryProblems } from "./validate"; + +const plain: RegistryEntry = { + page: "UN05", + v1Routes: ["/:orgName/workflows", "/:orgName/flows/:id/*"], + replaces: ["/:orgName/workflows", "/:orgName/workflows/:id"], + toV0: {}, + toV1: { + "/:orgName/workflows": "/:orgName/workflows", + "/:orgName/workflows/:id": "/:orgName/flows/:id/*", + }, + toggle: false, + distributions: ["oss", "cloud"], + registeredBy: "P3-T01", +}; + +const review: RegistryEntry = { + page: "UN11", + v1Routes: ["/:orgName/hitl", "/:orgName/hitl/:id/approve"], + replaces: ["/:orgName/review", "/:orgName/review/approve"], + toV0: { + "/:orgName/hitl": "/:orgName/review", + "/:orgName/hitl/:id/approve": "/:orgName/review/approve", + }, + toV1: { + "/:orgName/review": "/:orgName/hitl", + "/:orgName/review/approve": "/:orgName/hitl", + }, + toggle: true, + distributions: ["cloud"], + registeredBy: "P6-T07", +}; + +const entries = [plain, review]; + +it("the fixtures are valid registry entries", () => { + expect(registryProblems(entries)).toEqual([]); +}); +const at = (pathname: string, oldVersion: boolean | null = false) => + resolveRoute({ pathname }, { entries, oldVersion }); + +describe("resolveRoute", () => { + it("renders v0 for an unregistered URL, and everywhere with an empty registry", () => { + expect(at("/acme/logs")).toEqual({ kind: "v0" }); + for (const path of ["/acme/workflows", "/acme/review", "/acme/hitl"]) { + expect( + resolveRoute({ pathname: path }, { entries: [], oldVersion: true }), + ).toEqual({ + kind: "v0", + }); + } + }); + + it("renders v1 for a registered page's v1 route, whatever the preference", () => { + for (const pref of [true, false, null]) { + expect(at("/acme/flows/7/runs", pref)).toEqual({ + kind: "v1", + entry: plain, + }); + // A route that is both v0 and v1 renders v1 without redirecting. + expect(at("/acme/workflows", pref)).toEqual({ kind: "v1", entry: plain }); + } + }); + + it("redirects a registered page's v0 URL to its v1 route, keeping query and hash", () => { + expect( + resolveRoute( + { pathname: "/acme/workflows/7", search: "?q=1", hash: "#x" }, + { entries, oldVersion: false }, + ), + ).toEqual({ kind: "redirect", to: "/acme/flows/7?q=1#x" }); + }); + + it("toggle page, new version: v1 renders and v0 URLs redirect", () => { + expect(at("/acme/hitl", false)).toEqual({ kind: "v1", entry: review }); + expect(at("/acme/review/approve", false)).toEqual({ + kind: "redirect", + to: "/acme/hitl", + }); + }); + + it("toggle page, old version: v0 renders and v1 URLs redirect to the mapped v0 route", () => { + expect(at("/acme/review", true)).toEqual({ kind: "v0" }); + expect(at("/acme/hitl/9/approve", true)).toEqual({ + kind: "redirect", + to: "/acme/review/approve", + }); + }); + + it("toggle page, user unknown: v0 URLs stay on v0 and v1 URLs on v1", () => { + expect(at("/acme/review", null)).toEqual({ kind: "v0" }); + expect(at("/acme/hitl", null)).toEqual({ kind: "v1", entry: review }); + }); + + it("the preference changes nothing on a page without the toggle", () => { + expect(at("/acme/workflows/7", true)).toEqual( + at("/acme/workflows/7", false), + ); + }); + + it("the most specific pattern wins when pages share a prefix", () => { + const logs: RegistryEntry = { + ...plain, + page: "UN10", + v1Routes: ["/:orgName/flows/:id/logs"], + replaces: [], + toV1: {}, + }; + const both = { entries: [plain, logs], oldVersion: false }; + expect(resolveRoute({ pathname: "/acme/flows/7/logs" }, both)).toEqual({ + kind: "v1", + entry: logs, + }); + expect(resolveRoute({ pathname: "/acme/flows/7/runs" }, both)).toEqual({ + kind: "v1", + entry: plain, + }); + }); +}); + +describe("activeEntries", () => { + it("keeps registered pages shipped in the distribution only", () => { + const live = new Set(["UN05", "UN11"] as const); + expect(activeEntries(entries, live, "oss")).toEqual([plain]); + expect(activeEntries(entries, live, "cloud")).toEqual([plain, review]); + expect(activeEntries(entries, live, "onprem")).toEqual([]); + expect(activeEntries(entries, new Set(["UN11"] as const), "cloud")).toEqual( + [review], + ); + }); +}); + +describe("resolvePageUrl", () => { + const params = { orgName: "acme", id: "9" }; + it("gives the v1 URL for a live page", () => { + expect( + resolvePageUrl("UN11", "/:orgName/hitl/:id/approve", params, entries, { + entries, + oldVersion: false, + }), + ).toBe("/acme/hitl/9/approve"); + }); + it("gives the mapped v0 URL in old-version mode or when the page is not live", () => { + for (const ctx of [ + { entries, oldVersion: true }, + { entries: [], oldVersion: false }, + ]) { + expect( + resolvePageUrl( + "UN11", + "/:orgName/hitl/:id/approve", + params, + entries, + ctx, + ), + ).toBe("/acme/review/approve"); + } + }); + it("is null for an undeclared page, an unknown pattern or a route with no v0 mapping", () => { + const ctx = { entries: [], oldVersion: false }; + expect( + resolvePageUrl("UN99", "/:orgName/hitl", params, entries, ctx), + ).toBeNull(); + expect( + resolvePageUrl("UN11", "/:orgName/nope", params, entries, ctx), + ).toBeNull(); + expect( + resolvePageUrl("UN05", "/:orgName/workflows", params, entries, ctx), + ).toBeNull(); + }); +}); + +/** Mapping both ways, for every entry the app declares. */ +describe.each([...PAGES, ...DEMO_PAGES])("$page mapping", (entry) => { + const self = { entries: [entry] }; + const fill = (pattern: string) => pattern.replaceAll(/:(\w+)/g, "p-$1"); + + it("each v0 route redirects to its mapped v1 route, which renders v1", () => { + for (const [v0, v1] of Object.entries(entry.toV1)) { + const r = resolveRoute( + { pathname: fill(v0) }, + { ...self, oldVersion: false }, + ); + if (v0 === v1) { + expect(r).toEqual({ kind: "v1", entry }); + } else { + expect(r).toEqual({ kind: "redirect", to: fill(v1) }); + expect( + resolveRoute({ pathname: fill(v1) }, { ...self, oldVersion: false }), + ).toEqual({ kind: "v1", entry }); + } + } + }); + + it.runIf(entry.toggle)( + "in old-version mode each v1 route redirects to its mapped v0 route, which renders v0", + () => { + for (const [v1, v0] of Object.entries(entry.toV0)) { + const r = resolveRoute( + { pathname: fill(v1) }, + { ...self, oldVersion: true }, + ); + expect(r).toEqual( + v1 === v0 ? { kind: "v0" } : { kind: "redirect", to: fill(v0) }, + ); + expect( + resolveRoute({ pathname: fill(v0) }, { ...self, oldVersion: true }), + ).toEqual({ + kind: "v0", + }); + } + }, + ); +}); diff --git a/frontend/src/v1/app/registry/resolve.ts b/frontend/src/v1/app/registry/resolve.ts new file mode 100644 index 0000000000..f9c7c169d0 --- /dev/null +++ b/frontend/src/v1/app/registry/resolve.ts @@ -0,0 +1,120 @@ +import { generatePath, matchRoutes, type Params } from "react-router-dom"; +import type { Distribution, PageKey, RegistryEntry } from "./types"; + +/** What the version switch does with a URL. */ +export type Resolution = + | Readonly<{ kind: "v0" }> + | Readonly<{ kind: "v1"; entry: RegistryEntry }> + | Readonly<{ kind: "redirect"; to: string }>; + +export type ResolveContext = Readonly<{ + /** The pages that are live in this build and session (see activeEntries). */ + entries: readonly RegistryEntry[]; + /** + * The user's old-version preference: honoured by toggle pages only. `null` + * while the user is unknown, which leaves a toggle page's v0 URLs on v0 so + * v0 can load the session (and with it the preference). + */ + oldVersion: boolean | null; +}>; + +type Location = Readonly<{ pathname: string; search?: string; hash?: string }>; + +/** The live entries: declared, registered (or previewed) and shipped in this distribution. */ +export function activeEntries( + pages: readonly RegistryEntry[], + live: ReadonlySet, + distribution: Distribution, +): RegistryEntry[] { + return pages.filter( + (e) => live.has(e.page) && e.distributions.includes(distribution), + ); +} + +type Owned = Readonly<{ path: string; entry: RegistryEntry }>; + +/** The registry route a pathname lands on; React Router ranks the most specific pattern first. */ +function match( + pathname: string, + entries: readonly RegistryEntry[], +): { route: Owned; params: Params } | null { + const routes: Owned[] = entries.flatMap((entry) => + [...new Set([...entry.v1Routes, ...entry.replaces])].map((path) => ({ + path, + entry, + })), + ); + const found = matchRoutes(routes, pathname)?.[0]; + return found ? { route: found.route, params: found.params } : null; +} + +function redirect( + target: string, + params: Params, + location: Location, +): Resolution { + return { + kind: "redirect", + to: `${generatePath(target, params)}${location.search ?? ""}${location.hash ?? ""}`, + }; +} + +/** + * The resolution rule. An unregistered URL renders v0. A registered page + * renders v1, and its v0 URLs redirect to their mapped v1 route; except that + * a toggle page (Human Review) does the reverse while the user has chosen the + * old version. + */ +export function resolveRoute( + location: Location, + ctx: ResolveContext, +): Resolution { + const found = match(location.pathname, ctx.entries); + if (!found) { + return { kind: "v0" }; + } + const { entry, path } = found.route; + const isV1 = entry.v1Routes.includes(path); + const isV0 = entry.replaces.includes(path); + + if (entry.toggle && ctx.oldVersion !== false) { + if (isV0) { + return { kind: "v0" }; + } + if (ctx.oldVersion === true) { + const target = entry.toV0[path]; + return target ? redirect(target, found.params, location) : { kind: "v0" }; + } + // Unknown user on a v1-only URL: v1 is the default. + } + if (isV1) { + return { kind: "v1", entry }; + } + const target = entry.toV1[path]; + return target ? redirect(target, found.params, location) : { kind: "v0" }; +} + +/** + * The URL to open a page's v1 route with, in whichever version the user gets: + * the filled v1 route when the page is live, else its mapped v0 route. Null + * when the page is not declared or has no v0 mapping for that route. + */ +export function resolvePageUrl( + page: PageKey, + v1Pattern: string, + params: Readonly>, + pages: readonly RegistryEntry[], + ctx: ResolveContext, +): string | null { + const entry = pages.find((e) => e.page === page); + if (!entry?.v1Routes.includes(v1Pattern)) { + return null; + } + const live = ctx.entries.includes(entry); + const useV0 = !live || (entry.toggle && ctx.oldVersion === true); + if (!useV0) { + return generatePath(v1Pattern, params); + } + const target = entry.toV0[v1Pattern]; + return target ? generatePath(target, params) : null; +} diff --git a/frontend/src/v1/app/registry/types.ts b/frontend/src/v1/app/registry/types.ts new file mode 100644 index 0000000000..06eb4be78d --- /dev/null +++ b/frontend/src/v1/app/registry/types.ts @@ -0,0 +1,49 @@ +/* + * Route registry types (UN-4271). + * + * This folder's data files (types.ts, entries.ts) import only each other, with + * `./`, and never touch `import.meta.env`: the E2E harness imports them from + * Playwright, which applies neither Vite nor the `@v1` alias. + */ + +/** A build the frontend ships in. */ +export type Distribution = "oss" | "cloud" | "onprem"; + +export const DISTRIBUTIONS: readonly Distribution[] = [ + "oss", + "cloud", + "onprem", +]; + +/** + * Target page key: UN01 to UN20 (Unstract), LW01 to LW07 (LLMWhisperer), or a + * development-only demo page (`DEMO-…`) that exists only when + * VITE_V1_DEV_ROUTES is on. + */ +export type PageKey = `UN${string}` | `LW${string}` | `DEMO-${string}`; + +/** + * One v1 page. Route patterns are absolute React Router patterns + * (`/:orgName/workflows/:id`); a trailing `/*` matches the rest of the URL. + */ +export type RegistryEntry = Readonly<{ + page: PageKey; + /** The v1 route patterns, including v1-only sub-routes (tabs, stages). */ + v1Routes: readonly string[]; + /** The v0 route patterns this page takes over. */ + replaces: readonly string[]; + /** + * v1 pattern -> the v0 pattern it maps back to. Read by Human Review's + * old-version mode, so a toggle page maps every v1 route; a v1-only + * sub-route maps to the nearest v0 route. + */ + toV0: Readonly>; + /** v0 pattern -> the v1 pattern it redirects to; covers every `replaces` pattern. */ + toV1: Readonly>; + /** Honours the old-version preference. True for UN11 (Human Review) only, D14. */ + toggle: boolean; + /** Builds the page exists in; an entry the current build lacks is ignored. */ + distributions: readonly Distribution[]; + /** The go-live task that registers the page, e.g. "P6-T07". */ + registeredBy: string; +}>; diff --git a/frontend/src/v1/app/registry/validate.test.ts b/frontend/src/v1/app/registry/validate.test.ts new file mode 100644 index 0000000000..c4258e9abe --- /dev/null +++ b/frontend/src/v1/app/registry/validate.test.ts @@ -0,0 +1,164 @@ +import { PAGE_COMPONENTS } from "@v1/app/pages"; +import { describe, expect, it } from "vitest"; +import { DEMO_PAGES, PAGES, REGISTERED_PAGES } from "./entries"; +import { patternProblem, patternsOverlap } from "./patterns"; +import type { RegistryEntry } from "./types"; +import { registryProblems } from "./validate"; + +describe("the app's registry", () => { + const declared = [...PAGES, ...DEMO_PAGES]; + + it("is valid", () => { + expect(registryProblems(declared)).toEqual([]); + }); + + it("registers only declared pages", () => { + const keys = PAGES.map((p) => p.page); + expect(REGISTERED_PAGES.filter((k) => !keys.includes(k))).toEqual([]); + }); + + it("has a component for every declared page", () => { + expect( + declared.filter((p) => !PAGE_COMPONENTS[p.page]).map((p) => p.page), + ).toEqual([]); + }); +}); + +const base: RegistryEntry = { + page: "UN05", + v1Routes: ["/:orgName/workflows"], + replaces: ["/:orgName/old-workflows"], + toV0: {}, + toV1: { "/:orgName/old-workflows": "/:orgName/workflows" }, + toggle: false, + distributions: ["oss"], + registeredBy: "P3-T01", +}; + +const problemsOf = (...entries: RegistryEntry[]) => registryProblems(entries); + +describe("registryProblems", () => { + it("accepts a well-formed entry", () => { + expect(problemsOf(base)).toEqual([]); + }); + + it("rejects the toggle on any page but Human Review (D14)", () => { + expect( + problemsOf({ + ...base, + toggle: true, + toV0: { "/:orgName/workflows": "/:orgName/old-workflows" }, + }), + ).toEqual([ + "UN05: only Human Review (UN11) may honour the old-version preference (D14)", + ]); + expect( + problemsOf({ + ...base, + page: "UN11", + toggle: true, + toV0: { "/:orgName/workflows": "/:orgName/old-workflows" }, + }), + ).toEqual([]); + }); + + it("requires a toggle page to map every v1 route back to v0", () => { + expect(problemsOf({ ...base, page: "UN11", toggle: true })).toEqual([ + "UN11: v1 route /:orgName/workflows has no v0 route for old-version mode", + ]); + }); + + it("requires every replaced v0 route to redirect somewhere", () => { + expect(problemsOf({ ...base, toV1: {} })).toEqual([ + "UN05: v0 route /:orgName/old-workflows has no v1 route to redirect to", + ]); + }); + + it("rejects mappings to routes the page does not own, or needing params the source lacks", () => { + expect( + problemsOf({ + ...base, + toV1: { "/:orgName/old-workflows": "/:orgName/x" }, + }), + ).toEqual([ + "UN05: toV1[/:orgName/old-workflows] -> /:orgName/x is not one of its routes", + ]); + expect( + problemsOf({ + ...base, + v1Routes: ["/:orgName/workflows/:id"], + toV1: { "/:orgName/old-workflows": "/:orgName/workflows/:id" }, + }), + ).toEqual([ + "UN05: toV1[/:orgName/old-workflows] -> /:orgName/workflows/:id needs params /:orgName/old-workflows lacks: id", + ]); + }); + + it("rejects overlapping patterns across pages, but not shared prefixes", () => { + const other: RegistryEntry = { + ...base, + page: "UN10", + v1Routes: ["/:orgName/workflows/*"], + replaces: [], + toV1: {}, + }; + expect(problemsOf(base, other)).toEqual([ + "UN05 /:orgName/workflows overlaps UN10 /:orgName/workflows/*", + ]); + expect( + problemsOf(base, { ...other, v1Routes: ["/:orgName/workflows/logs"] }), + ).toEqual([]); + }); + + it("rejects duplicates, empty routes or distributions, and malformed patterns", () => { + expect( + problemsOf(base, { + ...base, + v1Routes: ["/:orgName/other"], + replaces: [], + toV1: {}, + }), + ).toContain("UN05: declared twice"); + expect( + problemsOf({ + ...base, + v1Routes: [], + replaces: [], + toV1: {}, + distributions: [], + }), + ).toEqual(["UN05: has no v1 routes", "UN05: ships in no distribution"]); + expect( + problemsOf({ + ...base, + v1Routes: ["workflows", "/:orgName/:id?", "/a/*/b"], + replaces: [], + toV1: {}, + }), + ).toEqual([ + "UN05: route workflows must start with /", + "UN05: route /:orgName/:id? segment ':id?' is not a static name, :param or a final *", + "UN05: route /a/*/b * is allowed only as the last segment", + ]); + }); +}); + +describe("patterns", () => { + it.each([ + ["/:o/a", "/:o/a", true], + ["/:o/a", "/x/a", true], + ["/:o/a", "/:o/b", false], + ["/:o/a", "/:o/a/b", false], + ["/:o/a/*", "/:o/a", true], + ["/:o/a/*", "/:o/a/b/c", true], + ["/:o/a/*", "/:o/b/c", false], + ["/:o/A", "/:o/a", true], + ])("%s and %s overlap: %s", (a, b, expected) => { + expect(patternsOverlap(a, b)).toBe(expected); + expect(patternsOverlap(b, a)).toBe(expected); + }); + + it("accepts static, param and final-splat segments", () => { + expect(patternProblem("/llm-whisperer/:orgName/api-keys/*")).toBeNull(); + }); +}); diff --git a/frontend/src/v1/app/registry/validate.ts b/frontend/src/v1/app/registry/validate.ts new file mode 100644 index 0000000000..c60b4d347d --- /dev/null +++ b/frontend/src/v1/app/registry/validate.ts @@ -0,0 +1,104 @@ +import { paramNames, patternProblem, patternsOverlap } from "./patterns"; +import type { PageKey, RegistryEntry } from "./types"; + +/** Pages allowed to honour the old-version preference: Human Review (D14) and its demo stand-in. */ +export const TOGGLE_PAGES: readonly PageKey[] = ["UN11", "DEMO-REVIEW"]; + +function checkMapping( + entry: RegistryEntry, + name: "toV0" | "toV1", + from: readonly string[], + to: readonly string[], + problems: string[], +): void { + for (const [source, target] of Object.entries(entry[name])) { + if (!from.includes(source)) { + problems.push( + `${entry.page}: ${name} key ${source} is not one of its routes`, + ); + } + if (!to.includes(target)) { + problems.push( + `${entry.page}: ${name}[${source}] -> ${target} is not one of its routes`, + ); + } + // A splat may be empty, so a target ending in `*` needs nothing from the source. + const available = [...paramNames(source), "*"]; + const missing = paramNames(target).filter((p) => !available.includes(p)); + if (missing.length) { + problems.push( + `${entry.page}: ${name}[${source}] -> ${target} needs params ${source} lacks: ${missing.join(", ")}`, + ); + } + } +} + +function checkEntry(entry: RegistryEntry, problems: string[]): void { + const { page, v1Routes, replaces } = entry; + for (const pattern of [...v1Routes, ...replaces]) { + const problem = patternProblem(pattern); + if (problem) { + problems.push(`${page}: route ${pattern} ${problem}`); + } + } + if (!v1Routes.length) { + problems.push(`${page}: has no v1 routes`); + } + if (!entry.distributions.length) { + problems.push(`${page}: ships in no distribution`); + } + if (entry.toggle && !TOGGLE_PAGES.includes(page)) { + problems.push( + `${page}: only Human Review (UN11) may honour the old-version preference (D14)`, + ); + } + checkMapping(entry, "toV1", replaces, v1Routes, problems); + checkMapping(entry, "toV0", v1Routes, replaces, problems); + for (const pattern of replaces) { + if (!(pattern in entry.toV1)) { + problems.push( + `${page}: v0 route ${pattern} has no v1 route to redirect to`, + ); + } + } + if (entry.toggle) { + for (const pattern of v1Routes) { + if (!(pattern in entry.toV0)) { + problems.push( + `${page}: v1 route ${pattern} has no v0 route for old-version mode`, + ); + } + } + } +} + +/** + * Every problem with a registry, as readable lines; empty when it is valid. + * Run over the real registry by its unit test, so a bad entry fails CI. + */ +export function registryProblems(entries: readonly RegistryEntry[]): string[] { + const problems: string[] = []; + const seen = new Set(); + for (const entry of entries) { + if (seen.has(entry.page)) { + problems.push(`${entry.page}: declared twice`); + } + seen.add(entry.page); + checkEntry(entry, problems); + } + // Two pages may share a prefix (the longest match wins), but no URL may match both. + const owned = entries.flatMap((entry) => + [...new Set([...entry.v1Routes, ...entry.replaces])].map((pattern) => ({ + page: entry.page, + pattern, + })), + ); + for (const [i, a] of owned.entries()) { + for (const b of owned.slice(i + 1)) { + if (a.page !== b.page && patternsOverlap(a.pattern, b.pattern)) { + problems.push(`${a.page} ${a.pattern} overlaps ${b.page} ${b.pattern}`); + } + } + } + return problems; +} diff --git a/frontend/src/v1/index.ts b/frontend/src/v1/index.ts index 48afca1c00..5ffd5174ca 100644 --- a/frontend/src/v1/index.ts +++ b/frontend/src/v1/index.ts @@ -2,6 +2,50 @@ * Public entry of the v1 frontend — the only v1 module v0 may import, as * `@v1`. Lint rejects any other v0 → v1 import (see src/v1/README.md). * - * Empty until the version switch (UN-4271) mounts v1 through it. + * Keep this light: v0 loads it eagerly, so it holds the version switch and + * the registry, and the v1 tree itself loads only when a v1 page renders. */ -export {}; + +import { LIVE_REGISTRY, V1_DEV_ROUTES } from "@v1/app/registry/live"; +import { resolvePageUrl } from "@v1/app/registry/resolve"; +import type { PageKey } from "@v1/app/registry/types"; +import { useOldVersionStore } from "@v1/stores/oldVersion"; + +export { VersionSwitch, type VersionSwitchProps } from "@v1/app/VersionSwitch"; + +/** True in a build with the development-only demo routes (VITE_V1_DEV_ROUTES). */ +export const v1DevRoutesEnabled: boolean = V1_DEV_ROUTES; + +/** + * Sets the signed-in user's old-version preference, which only Human Review + * honours (D14). The version switch then moves the user to the equivalent + * screen without a reload. False, and no change, while no user is known. + * For the switch controls on both versions of Human Review (P6-T07). + */ +export function setOldVersion(on: boolean): boolean { + return useOldVersionStore.getState().setOldVersion(on); +} + +/** + * The URL that opens `page` at one of its v1 route patterns, in the version + * the user gets: the v1 URL when the page is live, else its mapped v0 URL. + * Null when the page is not declared or that route has no v0 mapping. + * + * resolveUrl("UN05", "/:orgName/workflows/:id", { orgName, id }) + */ +export function resolveUrl( + page: PageKey, + v1Pattern: string, + params: Readonly>, +): string | null { + return resolvePageUrl(page, v1Pattern, params, LIVE_REGISTRY.pages, { + entries: LIVE_REGISTRY.entries, + oldVersion: useOldVersionStore.getState().oldVersion, + }); +} + +export type { + Distribution, + PageKey, + RegistryEntry, +} from "@v1/app/registry/types"; diff --git a/frontend/src/v1/lib/oldVersion.ts b/frontend/src/v1/lib/oldVersion.ts new file mode 100644 index 0000000000..38d41bb7d0 --- /dev/null +++ b/frontend/src/v1/lib/oldVersion.ts @@ -0,0 +1,72 @@ +/* + * Storage for the old-version preference (UN-4271, D14): per user, per + * browser, no backend. Only toggle pages (Human Review) read it. + */ + +const KEY_PREFIX = "unstract.v1.oldVersion."; + +/** The last user signed in on this browser, so a reload can resolve before the session loads. */ +export const LAST_USER_STORAGE_KEY = "unstract.v1.lastUser"; + +/** The value stored while a user has chosen the old version; absent means v1. */ +export const OLD_VERSION_ON = "true"; + +/** The storage key holding one user's preference. */ +export function oldVersionStorageKey(userId: string): string { + return `${KEY_PREFIX}${userId}`; +} + +/** The stored preference; missing or unreadable means v1 (false). */ +export function readOldVersion( + storage: Pick | null | undefined, + userId: string, +): boolean { + try { + return storage?.getItem(oldVersionStorageKey(userId)) === OLD_VERSION_ON; + } catch { + return false; + } +} + +/** Stores the preference; false if storage is absent or refuses the write. */ +export function writeOldVersion( + storage: Pick | null | undefined, + userId: string, + on: boolean, +): boolean { + if (!storage) { + return false; + } + try { + const key = oldVersionStorageKey(userId); + if (on) { + storage.setItem(key, OLD_VERSION_ON); + } else { + storage.removeItem(key); + } + return true; + } catch { + return false; + } +} + +export function readLastUser( + storage: Pick | null | undefined, +): string | null { + try { + return storage?.getItem(LAST_USER_STORAGE_KEY) || null; + } catch { + return null; + } +} + +export function writeLastUser( + storage: Pick | null | undefined, + userId: string, +): void { + try { + storage?.setItem(LAST_USER_STORAGE_KEY, userId); + } catch { + // Unwritable storage: the next load waits for the session instead. + } +} diff --git a/frontend/src/v1/pages/.gitkeep b/frontend/src/v1/pages/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/frontend/src/v1/pages/demo/index.tsx b/frontend/src/v1/pages/demo/index.tsx new file mode 100644 index 0000000000..8044fa3c9e --- /dev/null +++ b/frontend/src/v1/pages/demo/index.tsx @@ -0,0 +1,35 @@ +/* + * Development-only demo pages that prove the version switch (UN-4271). They + * exist only in a build with VITE_V1_DEV_ROUTES=true. DemoReviewPage stands in + * for Human Review: its "Switch to old version" control is the shape of the + * one P6-T07 builds. + */ + +import { useOldVersionStore } from "@v1/stores/oldVersion"; +import type { JSX } from "react"; +import { Link, useParams } from "react-router-dom"; + +export function DemoPage(): JSX.Element { + return ( +
+

v1 demo page

+
+ ); +} + +export function DemoReviewPage(): JSX.Element { + const { orgName = "" } = useParams(); + const setOldVersion = useOldVersionStore((s) => s.setOldVersion); + return ( +
+

v1 demo review

+ + +
+ ); +} diff --git a/frontend/src/v1/stores/oldVersion.test.ts b/frontend/src/v1/stores/oldVersion.test.ts new file mode 100644 index 0000000000..9488d0c191 --- /dev/null +++ b/frontend/src/v1/stores/oldVersion.test.ts @@ -0,0 +1,104 @@ +import { + LAST_USER_STORAGE_KEY, + oldVersionStorageKey, + readOldVersion, + writeOldVersion, +} from "@v1/lib/oldVersion"; +import { describe, expect, it } from "vitest"; +import { createOldVersionStore } from "./oldVersion"; + +function memoryStorage(initial: Record = {}) { + const data = new Map(Object.entries(initial)); + return { + data, + getItem: (k: string) => data.get(k) ?? null, + setItem: (k: string, v: string) => { + data.set(k, v); + }, + removeItem: (k: string) => { + data.delete(k); + }, + }; +} + +const refusing = { + getItem: () => { + throw new Error("denied"); + }, + setItem: () => { + throw new Error("denied"); + }, + removeItem: () => { + throw new Error("denied"); + }, +}; + +describe("old-version storage", () => { + it("is keyed per user and absent means v1", () => { + const s = memoryStorage(); + expect(readOldVersion(s, "7")).toBe(false); + expect(writeOldVersion(s, "7", true)).toBe(true); + expect(s.data.get(oldVersionStorageKey("7"))).toBe("true"); + expect(readOldVersion(s, "7")).toBe(true); + expect(readOldVersion(s, "8")).toBe(false); + writeOldVersion(s, "7", false); + expect(s.data.has(oldVersionStorageKey("7"))).toBe(false); + }); + + it("unreadable or refusing storage reads as v1 and reports the failed write", () => { + expect(readOldVersion(refusing, "7")).toBe(false); + expect(writeOldVersion(refusing, "7", true)).toBe(false); + expect(writeOldVersion(null, "7", true)).toBe(false); + }); +}); + +describe("old-version store", () => { + it("knows no user and no choice on a fresh browser, and cannot be set", () => { + const store = createOldVersionStore(memoryStorage()); + expect(store.getState()).toMatchObject({ userId: null, oldVersion: null }); + expect(store.getState().setOldVersion(true)).toBe(false); + expect(store.getState().oldVersion).toBeNull(); + }); + + it("survives reload and re-login: a new store reads the last user's choice", () => { + const s = memoryStorage(); + const first = createOldVersionStore(s); + first.getState().setUser("7"); + expect(first.getState().oldVersion).toBe(false); + expect(first.getState().setOldVersion(true)).toBe(true); + expect(s.data.get(LAST_USER_STORAGE_KEY)).toBe("7"); + + const reloaded = createOldVersionStore(s); + expect(reloaded.getState()).toMatchObject({ + userId: "7", + oldVersion: true, + }); + reloaded.getState().setUser(null); // signed out: keeps the last user + expect(reloaded.getState()).toMatchObject({ + userId: "7", + oldVersion: true, + }); + reloaded.getState().setUser("7"); + expect(reloaded.getState().oldVersion).toBe(true); + }); + + it("is per user", () => { + const s = memoryStorage({ [oldVersionStorageKey("7")]: "true" }); + const store = createOldVersionStore(s); + store.getState().setUser("8"); + expect(store.getState().oldVersion).toBe(false); + store.getState().setUser("7"); + expect(store.getState().oldVersion).toBe(true); + }); + + it("with refusing storage, defaults to v1 and the choice still holds for the session", () => { + const store = createOldVersionStore(refusing); + store.getState().setUser("7"); + expect(store.getState().oldVersion).toBe(false); + expect(store.getState().setOldVersion(true)).toBe(true); + expect(store.getState().oldVersion).toBe(true); + store.getState().setUser("8"); + store.getState().setUser("7"); + expect(store.getState().oldVersion).toBe(true); + }); +}); diff --git a/frontend/src/v1/stores/oldVersion.ts b/frontend/src/v1/stores/oldVersion.ts new file mode 100644 index 0000000000..25c61c5aef --- /dev/null +++ b/frontend/src/v1/stores/oldVersion.ts @@ -0,0 +1,87 @@ +import { + readLastUser, + readOldVersion, + writeLastUser, + writeOldVersion, +} from "@v1/lib/oldVersion"; +import { create, type UseBoundStore } from "zustand"; +import { createStore, type StateCreator, type StoreApi } from "zustand/vanilla"; + +export type OldVersionState = { + /** The user the preference belongs to: the signed-in user, else the last one on this browser. */ + userId: string | null; + /** That user's choice; null while no user is known. Only toggle pages read it. */ + oldVersion: boolean | null; + /** Choices made this session, so they hold when storage refuses the write. */ + chosen: Readonly>; + /** Follows the signed-in user. Signing out (null) keeps the last user. */ + setUser: (userId: string | null) => void; + /** + * Sets the current user's choice and persists it. The version switch then + * moves the user to the equivalent screen, with no reload. False (and no + * change) while no user is known. + */ + setOldVersion: (on: boolean) => boolean; +}; + +type PreferenceStorage = Pick< + Storage, + "getItem" | "setItem" | "removeItem" +> | null; + +function oldVersionState( + storage: PreferenceStorage, +): StateCreator { + const lastUser = readLastUser(storage); + return (set, get) => ({ + userId: lastUser, + oldVersion: lastUser === null ? null : readOldVersion(storage, lastUser), + chosen: {}, + setUser: (userId) => { + if (userId === null || userId === get().userId) { + return; + } + writeLastUser(storage, userId); + set((state) => ({ + userId, + oldVersion: state.chosen[userId] ?? readOldVersion(storage, userId), + })); + }, + setOldVersion: (on) => { + const { userId } = get(); + if (userId === null) { + return false; + } + writeOldVersion(storage, userId, on); + set((state) => ({ + oldVersion: on, + chosen: { ...state.chosen, [userId]: on }, + })); + return true; + }, + }); +} + +/** A standalone store, for tests and isolated trees. */ +export function createOldVersionStore( + storage: PreferenceStorage = null, +): StoreApi { + return createStore()(oldVersionState(storage)); +} + +/** Reading localStorage can throw in sandboxed documents; treat that as no storage. */ +function browserStorage(): Storage | null { + if (typeof window === "undefined") { + return null; + } + try { + return window.localStorage; + } catch { + return null; + } +} + +export const useOldVersionStore: UseBoundStore> = + create()(oldVersionState(browserStorage())); + +export const selectOldVersion = (s: OldVersionState) => s.oldVersion; From e62aec1002cdf406662b42c023653186adbeaa1b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:03:56 +0000 Subject: [PATCH 2/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- frontend/src/v1/app/VersionSwitch.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/v1/app/VersionSwitch.test.tsx b/frontend/src/v1/app/VersionSwitch.test.tsx index cf5a54ee2d..e69e22db46 100644 --- a/frontend/src/v1/app/VersionSwitch.test.tsx +++ b/frontend/src/v1/app/VersionSwitch.test.tsx @@ -3,8 +3,8 @@ import userEvent from "@testing-library/user-event"; import { buildLiveRegistry, type LiveRegistry } from "@v1/app/registry/live"; import { oldVersionStorageKey } from "@v1/lib/oldVersion"; import { useOldVersionStore } from "@v1/stores/oldVersion"; -import { beforeEach, describe, expect, it } from "vitest"; import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; +import { beforeEach, describe, expect, it } from "vitest"; import { VersionSwitch } from "./VersionSwitch"; const demo = buildLiveRegistry({ From 217c586af790c1e187895d38400bc90992eaec18 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Wed, 4 Nov 2026 11:45:18 +0530 Subject: [PATCH 3/7] fix(frontend-v1): address self-review of the version switch (UN-4271) - Validation rejects a mapping whose URL lands on a route of the other version (an endless redirect), malformed page keys, and any v1 route without a toV0 mapping, so resolveUrl never returns null for a declared page's route. - Redirects and resolveUrl encode param values, so %2F, %3F and %23 stay inside their segment instead of becoming separators. - resolveUrl decides liveness by page key, not object identity. - An unknown VITE_UNSTRACT_DISTRIBUTION fails `vite build`; at runtime it logs and falls back to oss instead of blanking the app. - Registry validation runs only in development (CI covers release). - The v1 error boundary reloads once on a stale chunk (as v0 does) and offers a Reload button. - Dockerfile build args move below the install layer so CI and release share the cached install; VITE_UNSTRACT_DISTRIBUTION is settable. - Public helpers move to app/api.ts so v1 code can import them without the entry. - Tests now catch the guards mutation testing showed were unprotected (stale-user bounce, history replace, preview gating, distribution, error boundary, encoding); comments corrected. --- docker/dockerfiles/frontend.Dockerfile | 20 ++- frontend/src/routes/Router.jsx | 9 +- frontend/src/v1/README.md | 19 ++- frontend/src/v1/app/VersionSwitch.test.tsx | 68 ++++++-- frontend/src/v1/app/VersionSwitch.tsx | 32 +++- frontend/src/v1/app/api.ts | 43 ++++++ frontend/src/v1/app/pages.ts | 6 +- frontend/src/v1/app/registry/entries.ts | 4 +- frontend/src/v1/app/registry/live.test.ts | 74 ++++++--- frontend/src/v1/app/registry/live.ts | 55 ++++--- frontend/src/v1/app/registry/resolve.test.ts | 80 ++++++++-- frontend/src/v1/app/registry/resolve.ts | 59 +++++-- frontend/src/v1/app/registry/types.ts | 29 ++-- frontend/src/v1/app/registry/validate.test.ts | 145 ++++++++++++------ frontend/src/v1/app/registry/validate.ts | 51 ++++-- frontend/src/v1/index.ts | 44 +----- frontend/src/v1/lib/chunkReload.test.ts | 56 +++++++ frontend/src/v1/lib/chunkReload.ts | 45 ++++++ frontend/src/v1/pages/demo/index.tsx | 6 +- frontend/vite.config.js | 10 ++ 20 files changed, 634 insertions(+), 221 deletions(-) create mode 100644 frontend/src/v1/app/api.ts create mode 100644 frontend/src/v1/lib/chunkReload.test.ts create mode 100644 frontend/src/v1/lib/chunkReload.ts diff --git a/docker/dockerfiles/frontend.Dockerfile b/docker/dockerfiles/frontend.Dockerfile index f9d4af6ab3..c8f61763fd 100644 --- a/docker/dockerfiles/frontend.Dockerfile +++ b/docker/dockerfiles/frontend.Dockerfile @@ -38,11 +38,6 @@ CMD ["/bin/sh", "-c", "/app/generate-runtime-config.sh && bun run start"] FROM base AS builder ARG BUILD_CONTEXT_PATH ENV VITE_BACKEND_URL="" -# "true" only for the E2E image (ci-test.yaml): registers the v1 demo pages -# that prove the version switch, and honours the v1 preview override. -# Release builds leave it empty. -ARG VITE_V1_DEV_ROUTES="" -ENV VITE_V1_DEV_ROUTES=${VITE_V1_DEV_ROUTES} # Copy package files and install dependencies COPY ${BUILD_CONTEXT_PATH}/package.json ${BUILD_CONTEXT_PATH}/bun.lock ./ @@ -51,8 +46,21 @@ RUN bun install --frozen-lockfile --ignore-scripts # Copy the rest of the application files COPY ${BUILD_CONTEXT_PATH}/ . +# Declared after the install layer so a different value (the E2E image sets +# VITE_V1_DEV_ROUTES) does not invalidate the cached install it shares with +# release builds. +# VITE_V1_DEV_ROUTES: "true" only for the E2E image (ci-test.yaml); registers +# the v1 demo pages that prove the version switch and honours the v1 preview +# override. Release builds leave it empty. +# VITE_UNSTRACT_DISTRIBUTION: oss (default when empty), cloud or onprem; the +# v1 route registry ignores pages the distribution lacks. +ARG VITE_V1_DEV_ROUTES="" +ARG VITE_UNSTRACT_DISTRIBUTION="" + # Build with Vite -RUN bun run build +RUN VITE_V1_DEV_ROUTES="${VITE_V1_DEV_ROUTES}" \ + VITE_UNSTRACT_DISTRIBUTION="${VITE_UNSTRACT_DISTRIBUTION}" \ + bun run build # Production stage FROM nginx:1.29.1-alpine AS production diff --git a/frontend/src/routes/Router.jsx b/frontend/src/routes/Router.jsx index 93f21a2ac4..cfce1c446f 100644 --- a/frontend/src/routes/Router.jsx +++ b/frontend/src/routes/Router.jsx @@ -148,7 +148,9 @@ try { } // Stand-ins for the v0 side of the v1 demo pages (UN-4271), which prove the -// version switch. Rendered only in a build with VITE_V1_DEV_ROUTES=true. +// version switch; routed only in a build with VITE_V1_DEV_ROUTES=true. The +// demo page is always live there, so V0DemoPage renders only if the switch +// fails to redirect: that is what the E2E check would catch, not dead code. function V0DemoPage() { return

v0 demo page

; } @@ -174,8 +176,9 @@ function Router() { // Renders v1 for a URL whose page is registered in the v1 route // registry, and these v0 routes for everything else (UN-4271). - {/* App-wide backstop: catches shell-level chunk failures and errors on - routes that don't sit inside a LazyOutlet layout. Per-page load + {/* v0-wide backstop (the v1 tree has its own, in VersionSwitch): + catches shell-level chunk failures and errors on routes that don't + sit inside a LazyOutlet layout. Per-page load errors are handled by the scoped boundaries inside the layouts. resetKeys lets navigation clear the error without a full reload. */} `). `setOldVersion(on)` sets it, and the switch then moves the user to the mapped screen without a - reload. + reload. `unstract.v1.lastUser` lets a reload apply it before the session + loads; on a shared browser that is the previous user's choice until the + session says otherwise. - `VITE_V1_DEV_ROUTES=true` (the E2E image, or `.env` locally) registers the demo pages (`/:orgName/v1-demo`, `/:orgName/v1-demo-review`, and their v0 stand-ins `/:orgName/v0-demo…`). It also renders declared pages as v1 before - they go live: `localStorage["unstract.v1.preview"] = '["UN05"]'`. -- `VITE_UNSTRACT_DISTRIBUTION` (`oss`, the default, `cloud` or `onprem`) drops - entries for distributions the build lacks. + they go live: set `localStorage["unstract.v1.preview"] = '["UN05"]'`, then + reload (it is read when the app loads). +- `VITE_UNSTRACT_DISTRIBUTION` (`oss` by default, `cloud` or `onprem`) selects + the build's distribution; entries whose `distributions` omit it are ignored. + An unknown value fails `vite build`. ## Folders and what each may import @@ -97,7 +102,7 @@ Every folder may import from itself. Test files (`*.test.ts(x)`, | Folder | Holds | May import from | | --- | --- | --- | -| `index.ts` | Public entry: the version switch, `setOldVersion`, `resolveUrl` | everything | +| `index.ts` | Public entry: the version switch and its helpers | everything | | `app/` | Entry, providers, router, route registry | everything | | `components/ui/` | Core components | `lib/`, `styles/`, `assets/` | | `components/patterns/` | Composites built from core components | `components/ui/`, `lib/`, `services/`, `stores/`, `hooks/`, `assets/` | diff --git a/frontend/src/v1/app/VersionSwitch.test.tsx b/frontend/src/v1/app/VersionSwitch.test.tsx index e69e22db46..561845fbe2 100644 --- a/frontend/src/v1/app/VersionSwitch.test.tsx +++ b/frontend/src/v1/app/VersionSwitch.test.tsx @@ -1,10 +1,20 @@ import { act, render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import { setOldVersion } from "@v1/app/api"; +import { DEMO_PAGES } from "@v1/app/registry/entries"; import { buildLiveRegistry, type LiveRegistry } from "@v1/app/registry/live"; import { oldVersionStorageKey } from "@v1/lib/oldVersion"; import { useOldVersionStore } from "@v1/stores/oldVersion"; -import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; -import { beforeEach, describe, expect, it } from "vitest"; +import { useEffect } from "react"; +import { + MemoryRouter, + type NavigateFunction, + Route, + Routes, + useLocation, + useNavigate, +} from "react-router-dom"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { VersionSwitch } from "./VersionSwitch"; const demo = buildLiveRegistry({ @@ -14,8 +24,17 @@ const demo = buildLiveRegistry({ }); const empty: LiveRegistry = { pages: [], entries: [] }; +/** Every pathname the router passes through, and a handle to navigate. */ +let visited: string[] = []; +let navigate: NavigateFunction = () => undefined; + function Where() { - return {useLocation().pathname}; + const { pathname } = useLocation(); + navigate = useNavigate(); + useEffect(() => { + visited.push(pathname); + }, [pathname]); + return {pathname}; } /** Stands in for v0's routes. */ @@ -31,14 +50,15 @@ const v0 = ( ); function renderAt( - path: string, + path: string | string[], { registry = demo, userId = "7", }: { registry?: LiveRegistry; userId?: string | null } = {}, ) { + const entries = Array.isArray(path) ? path : [path]; const view = (user: string | null) => ( - + {v0} @@ -55,6 +75,7 @@ function renderAt( const where = () => screen.getByTestId("where").textContent; beforeEach(() => { + visited = []; window.localStorage.clear(); useOldVersionStore.setState({ userId: null, oldVersion: null, chosen: {} }); }); @@ -84,6 +105,15 @@ describe("VersionSwitch", () => { ).not.toBeNull(); }); + it("a redirect replaces the history entry, so Back skips the v0 URL", async () => { + renderAt(["/acme/start", "/acme/v0-demo"]); + await screen.findByTestId("v1-demo"); + act(() => { + navigate(-1); + }); + expect(where()).toBe("/acme/start"); + }); + it("Human Review round trip: old version maps to v0 and back, with no reload", async () => { const user = userEvent.setup(); renderAt("/acme/v1-demo-review/details"); @@ -96,21 +126,29 @@ describe("VersionSwitch", () => { expect(screen.getByTestId("v0").textContent).toBe("v0 review"); expect(window.localStorage.getItem(oldVersionStorageKey("7"))).toBe("true"); + // v0's "Switch to new version" control calls the public entry. act(() => { - useOldVersionStore.getState().setOldVersion(false); + expect(setOldVersion(false)).toBe(true); }); expect(where()).toBe("/acme/v1-demo-review"); expect(await screen.findByTestId("v1-demo-review")).toBeTruthy(); expect(window.localStorage.getItem(oldVersionStorageKey("7"))).toBeNull(); }); - it("a stored old-version choice applies on load (reload, re-login)", () => { + it("a stored old-version choice applies on load (re-login)", () => { window.localStorage.setItem(oldVersionStorageKey("7"), "true"); renderAt("/acme/v1-demo-review"); expect(where()).toBe("/acme/v0-demo-review"); expect(screen.getByTestId("v0")).toBeTruthy(); }); + it("on reload, applies the last user's choice before the session loads", () => { + useOldVersionStore.setState({ userId: "7", oldVersion: true }); + renderAt("/acme/v1-demo-review", { userId: null }); + expect(where()).toBe("/acme/v0-demo-review"); + expect(screen.queryByTestId("v1-demo-review")).toBeNull(); + }); + it("the preference does nothing on a page without the toggle", async () => { window.localStorage.setItem(oldVersionStorageKey("7"), "true"); renderAt("/acme/v0-demo"); @@ -127,10 +165,22 @@ describe("VersionSwitch", () => { expect(where()).toBe("/acme/v1-demo-review"); }); - it("is per user: another user signing in does not inherit the old version", async () => { + it("is per user: another user signing in never passes through the previous user's old version", async () => { window.localStorage.setItem(oldVersionStorageKey("7"), "true"); useOldVersionStore.setState({ userId: "7", oldVersion: true }); - renderAt("/acme/v0-demo-review", { userId: "8" }); + renderAt("/acme/v1-demo-review", { userId: "8" }); expect(await screen.findByTestId("v1-demo-review")).toBeTruthy(); + expect(visited).not.toContain("/acme/v0-demo-review"); + }); + + it("contains a v1 failure in its own boundary instead of unmounting the app", async () => { + vi.spyOn(console, "error").mockImplementation(() => undefined); + const broken = { ...DEMO_PAGES[0], page: "DEMO-BROKEN" } as const; + renderAt("/acme/v1-demo", { + registry: { pages: [broken], entries: [broken] }, + }); + expect(await screen.findByRole("alert")).toBeTruthy(); + expect(screen.getByRole("button", { name: "Reload" })).toBeTruthy(); + vi.restoreAllMocks(); }); }); diff --git a/frontend/src/v1/app/VersionSwitch.tsx b/frontend/src/v1/app/VersionSwitch.tsx index d715082b6d..6e0db05a80 100644 --- a/frontend/src/v1/app/VersionSwitch.tsx +++ b/frontend/src/v1/app/VersionSwitch.tsx @@ -7,6 +7,7 @@ import { LIVE_REGISTRY, type LiveRegistry } from "@v1/app/registry/live"; import { resolveRoute } from "@v1/app/registry/resolve"; import type { RegistryEntry } from "@v1/app/registry/types"; +import { isChunkLoadError, reloadOnce } from "@v1/lib/chunkReload"; import { useOldVersionStore } from "@v1/stores/oldVersion"; import { Component, @@ -31,7 +32,21 @@ export type VersionSwitchProps = Readonly<{ type BoundaryState = { failed: boolean }; -/** Keeps a v1 failure (a stale chunk after a deploy) from unmounting the whole app. */ +function sessionStore(): Storage | null { + try { + return window.sessionStorage; + } catch { + return null; + } +} + +/** + * Keeps a v1 failure from unmounting the whole app. A stale chunk after a + * deploy reloads the page once to fetch the new one (React caches a failed + * lazy import, so nothing short of a reload retries it); anything else, or a + * repeat failure, shows the fallback. Unstyled when the failed chunk is the + * one carrying v1's styles. + */ class V1ErrorBoundary extends Component< { children: ReactNode }, BoundaryState @@ -42,12 +57,21 @@ class V1ErrorBoundary extends Component< return { failed: true }; } + override componentDidCatch(error: unknown): void { + if (isChunkLoadError(error)) { + reloadOnce(sessionStore(), () => window.location.reload()); + } + } + override render(): ReactNode { if (this.state.failed) { return ( -

- This page could not be loaded. Reload to try again. -

+
+

This page could not be loaded.

+ +
); } return this.props.children; diff --git a/frontend/src/v1/app/api.ts b/frontend/src/v1/app/api.ts new file mode 100644 index 0000000000..70ddd61a20 --- /dev/null +++ b/frontend/src/v1/app/api.ts @@ -0,0 +1,43 @@ +/* + * The helpers the public entry (`@v1`, src/v1/index.ts) exposes to v0. They + * live here so v1 code and tests can import them without the entry. + */ + +import { LIVE_REGISTRY, V1_DEV_ROUTES } from "@v1/app/registry/live"; +import { resolvePageUrl } from "@v1/app/registry/resolve"; +import type { PageKey } from "@v1/app/registry/types"; +import { useOldVersionStore } from "@v1/stores/oldVersion"; + +/** True in a build with the development-only demo routes (VITE_V1_DEV_ROUTES). */ +export const v1DevRoutesEnabled: boolean = V1_DEV_ROUTES; + +/** + * Sets the signed-in user's old-version preference, which only Human Review + * honours (D14). The version switch then moves the user to the equivalent + * screen without a reload. False, and no change, while no user is known. + * For the switch controls on both versions of Human Review (P6-T07). + */ +export function setOldVersion(on: boolean): boolean { + return useOldVersionStore.getState().setOldVersion(on); +} + +/** + * The URL that opens `page` at one of its v1 route patterns, in the version + * the user gets: the v1 URL when the page is live (unless it is Human Review + * and the user chose the old version), else its mapped v0 URL. Params are raw + * values and are encoded here. Null when the page is not declared or the + * pattern is not one of its v1 routes. Follows the last user known to this + * browser while a new user's session loads. + * + * resolveUrl("UN05", "/:orgName/workflows/:id", { orgName, id }) + */ +export function resolveUrl( + page: PageKey, + v1Pattern: string, + params: Readonly>, +): string | null { + return resolvePageUrl(page, v1Pattern, params, LIVE_REGISTRY.pages, { + entries: LIVE_REGISTRY.entries, + oldVersion: useOldVersionStore.getState().oldVersion, + }); +} diff --git a/frontend/src/v1/app/pages.ts b/frontend/src/v1/app/pages.ts index 9cb543e7c6..8aff24a2bb 100644 --- a/frontend/src/v1/app/pages.ts +++ b/frontend/src/v1/app/pages.ts @@ -1,7 +1,7 @@ /* - * The component for each declared page, loaded on first render. A page's - * go-live task adds its line here with its registry entry; the registry - * test fails if a declared page has none. + * The component for each declared page, loaded on first render. Add a + * page's line here when it is declared in PAGES; validate.test.ts fails if a + * declared page has none. */ import type { PageKey } from "@v1/app/registry/types"; diff --git a/frontend/src/v1/app/registry/entries.ts b/frontend/src/v1/app/registry/entries.ts index 9286a6022c..98375f476a 100644 --- a/frontend/src/v1/app/registry/entries.ts +++ b/frontend/src/v1/app/registry/entries.ts @@ -11,7 +11,7 @@ import type { PageKey, RegistryEntry } from "./types"; /** Every declared v1 page, live or not. */ export const PAGES: readonly RegistryEntry[] = []; -/** The pages users get as v1. Empty: the app renders v0 everywhere. */ +/** The pages users get as v1 (D9): a page's go-live task adds its key here. */ export const REGISTERED_PAGES: readonly PageKey[] = []; /** @@ -24,7 +24,7 @@ export const DEMO_PAGES: readonly RegistryEntry[] = [ page: "DEMO-PAGE", v1Routes: ["/:orgName/v1-demo"], replaces: ["/:orgName/v0-demo"], - toV0: {}, + toV0: { "/:orgName/v1-demo": "/:orgName/v0-demo" }, toV1: { "/:orgName/v0-demo": "/:orgName/v1-demo" }, toggle: false, distributions: ["oss", "cloud", "onprem"], diff --git a/frontend/src/v1/app/registry/live.test.ts b/frontend/src/v1/app/registry/live.test.ts index 5ea0b458b3..66ca799719 100644 --- a/frontend/src/v1/app/registry/live.test.ts +++ b/frontend/src/v1/app/registry/live.test.ts @@ -1,5 +1,4 @@ -import { describe, expect, it } from "vitest"; -import { DEMO_PAGES } from "./entries"; +import { describe, expect, it, vi } from "vitest"; import { buildLiveRegistry, PREVIEW_STORAGE_KEY, @@ -7,7 +6,30 @@ import { readPreviewPages, } from "./live"; +// A declared but unregistered cloud-only page, beside the real demo pages. +vi.mock("./entries", async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + PAGES: [ + { + page: "UN05", + v1Routes: ["/:orgName/flows"], + replaces: ["/:orgName/workflows"], + toV0: { "/:orgName/flows": "/:orgName/workflows" }, + toV1: { "/:orgName/workflows": "/:orgName/flows" }, + toggle: false, + distributions: ["cloud"], + registeredBy: "P3-T01", + }, + ], + REGISTERED_PAGES: [], + }; +}); + const storageWith = (value: string | null) => ({ getItem: () => value }); +const livePages = (options: Parameters[0]) => + buildLiveRegistry(options).entries.map((e) => e.page); describe("parseDistribution", () => { it("defaults to OSS and accepts any case", () => { @@ -16,19 +38,16 @@ describe("parseDistribution", () => { expect(parseDistribution(" Cloud ")).toBe("cloud"); expect(parseDistribution("onprem")).toBe("onprem"); }); - it("fails the build on an unknown value", () => { - expect(() => parseDistribution("saas")).toThrow( - "VITE_UNSTRACT_DISTRIBUTION=saas", - ); + it("is null for an unknown value (vite.config.js fails the build on it)", () => { + expect(parseDistribution("saas")).toBeNull(); }); }); describe("readPreviewPages", () => { - it("reads a JSON array of page keys", () => { - expect(readPreviewPages(storageWith('["UN05", 3, "LW02"]'))).toEqual([ - "UN05", - "LW02", - ]); + it("reads a JSON array of valid page keys", () => { + expect( + readPreviewPages(storageWith('["UN05", 3, "LW02", "UN5", "x"]')), + ).toEqual(["UN05", "LW02"]); }); it("treats missing, malformed or unreadable storage as no preview", () => { expect(readPreviewPages(null)).toEqual([]); @@ -49,22 +68,33 @@ describe("readPreviewPages", () => { }); describe("buildLiveRegistry", () => { - it("without dev routes: no demo pages, and the preview override is ignored", () => { + const preview = storageWith('["UN05"]'); + + it("a declared page stays v0 until registered", () => { + expect( + livePages({ devRoutes: false, distribution: "cloud", storage: null }), + ).toEqual([]); + }); + + it("a release build ignores the preview override and has no demo pages", () => { const r = buildLiveRegistry({ devRoutes: false, - distribution: "oss", - storage: storageWith('["DEMO-PAGE"]'), + distribution: "cloud", + storage: preview, }); - expect(r.pages.some((p) => p.page.startsWith("DEMO-"))).toBe(false); expect(r.entries).toEqual([]); + expect(r.pages.map((p) => p.page)).toEqual(["UN05"]); }); - it("with dev routes: the demo pages are declared and live", () => { - const r = buildLiveRegistry({ - devRoutes: true, - distribution: "oss", - storage: null, - }); - expect(r.entries.map((e) => e.page)).toEqual(DEMO_PAGES.map((p) => p.page)); + it("with dev routes, the demo pages and previewed pages are live", () => { + expect( + livePages({ devRoutes: true, distribution: "cloud", storage: preview }), + ).toEqual(["UN05", "DEMO-PAGE", "DEMO-REVIEW"]); + }); + + it("drops pages the build's distribution lacks, previewed or not", () => { + expect( + livePages({ devRoutes: true, distribution: "oss", storage: preview }), + ).toEqual(["DEMO-PAGE", "DEMO-REVIEW"]); }); }); diff --git a/frontend/src/v1/app/registry/live.ts b/frontend/src/v1/app/registry/live.ts index 7ed758dd1b..f1f497dbb6 100644 --- a/frontend/src/v1/app/registry/live.ts +++ b/frontend/src/v1/app/registry/live.ts @@ -11,7 +11,7 @@ import { type PageKey, type RegistryEntry, } from "./types"; -import { registryProblems } from "./validate"; +import { isPageKey, registryProblems } from "./validate"; /** * VITE_V1_DEV_ROUTES=true registers the demo pages and honours the preview @@ -23,27 +23,24 @@ export const V1_DEV_ROUTES: boolean = /** * Dev/test override: a JSON array of declared page keys to render as v1 - * before they are registered (D9), e.g. `["UN05"]`. Honoured only with - * V1_DEV_ROUTES. + * before they are registered (D9), e.g. `["UN05"]`, read when the app loads. + * Honoured only with V1_DEV_ROUTES. */ export const PREVIEW_STORAGE_KEY = "unstract.v1.preview"; -function isDistribution(value: unknown): value is Distribution { - return (DISTRIBUTIONS as readonly unknown[]).includes(value); -} - -/** The build's distribution, from VITE_UNSTRACT_DISTRIBUTION; OSS when unset. */ -export function parseDistribution(raw: string | undefined): Distribution { +/** + * The build's distribution from VITE_UNSTRACT_DISTRIBUTION: OSS when unset, + * null when the value is not a distribution. vite.config.js fails the build + * on such a value, so null is reached only by a build that skipped it. + */ +export function parseDistribution( + raw: string | undefined, +): Distribution | null { const value = raw?.trim().toLowerCase() || "oss"; - if (!isDistribution(value)) { - throw new Error( - `VITE_UNSTRACT_DISTRIBUTION=${raw} is not one of ${DISTRIBUTIONS.join(", ")}`, - ); - } - return value; + return DISTRIBUTIONS.find((d) => d === value) ?? null; } -/** Page keys in the preview override; anything unreadable or malformed means none. */ +/** Valid page keys in the preview override; anything unreadable or malformed means none. */ export function readPreviewPages( storage: Pick | null | undefined, ): PageKey[] { @@ -51,9 +48,7 @@ export function readPreviewPages( const parsed: unknown = JSON.parse( storage?.getItem(PREVIEW_STORAGE_KEY) ?? "[]", ); - return Array.isArray(parsed) - ? parsed.filter((k): k is PageKey => typeof k === "string") - : []; + return Array.isArray(parsed) ? parsed.filter(isPageKey) : []; } catch { return []; } @@ -94,15 +89,27 @@ function browserStorage(): Storage | null { } function load(): LiveRegistry { + const raw = import.meta.env.VITE_UNSTRACT_DISTRIBUTION; + let distribution = parseDistribution(raw); + if (distribution === null) { + // Never blank the app over it: v0 loads this module eagerly. + console.error( + `VITE_UNSTRACT_DISTRIBUTION=${raw} is not one of ${DISTRIBUTIONS.join(", ")}; using oss`, + ); + distribution = "oss"; + } const registry = buildLiveRegistry({ devRoutes: V1_DEV_ROUTES, - distribution: parseDistribution(import.meta.env.VITE_UNSTRACT_DISTRIBUTION), + distribution, storage: browserStorage(), }); - // CI rejects an invalid registry through its unit test; fail loudly in development too. - const problems = registryProblems(registry.pages); - if (import.meta.env.DEV && problems.length) { - throw new Error(`Invalid v1 route registry:\n${problems.join("\n")}`); + // CI rejects an invalid registry through validate.test.ts; development + // builds fail loudly too. Release builds skip the check (and its code). + if (import.meta.env.DEV) { + const problems = registryProblems(registry.pages); + if (problems.length) { + throw new Error(`Invalid v1 route registry:\n${problems.join("\n")}`); + } } return registry; } diff --git a/frontend/src/v1/app/registry/resolve.test.ts b/frontend/src/v1/app/registry/resolve.test.ts index 1633dea2fa..841e7b0aa4 100644 --- a/frontend/src/v1/app/registry/resolve.test.ts +++ b/frontend/src/v1/app/registry/resolve.test.ts @@ -1,6 +1,11 @@ import { describe, expect, it } from "vitest"; import { DEMO_PAGES, PAGES } from "./entries"; -import { activeEntries, resolvePageUrl, resolveRoute } from "./resolve"; +import { + activeEntries, + fillPattern, + resolvePageUrl, + resolveRoute, +} from "./resolve"; import type { RegistryEntry } from "./types"; import { registryProblems } from "./validate"; @@ -8,7 +13,10 @@ const plain: RegistryEntry = { page: "UN05", v1Routes: ["/:orgName/workflows", "/:orgName/flows/:id/*"], replaces: ["/:orgName/workflows", "/:orgName/workflows/:id"], - toV0: {}, + toV0: { + "/:orgName/workflows": "/:orgName/workflows", + "/:orgName/flows/:id/*": "/:orgName/workflows/:id", + }, toV1: { "/:orgName/workflows": "/:orgName/workflows", "/:orgName/workflows/:id": "/:orgName/flows/:id/*", @@ -75,6 +83,13 @@ describe("resolveRoute", () => { ).toEqual({ kind: "redirect", to: "/acme/flows/7?q=1#x" }); }); + it("keeps encoded params inside their segment when redirecting", () => { + expect(at("/a%2Fb/workflows/x%3Fy%23z")).toEqual({ + kind: "redirect", + to: "/a%2Fb/flows/x%3Fy%23z", + }); + }); + it("toggle page, new version: v1 renders and v0 URLs redirect", () => { expect(at("/acme/hitl", false)).toEqual({ kind: "v1", entry: review }); expect(at("/acme/review/approve", false)).toEqual({ @@ -102,14 +117,19 @@ describe("resolveRoute", () => { ); }); - it("the most specific pattern wins when pages share a prefix", () => { + it("defensively, the most specific pattern wins if two pages overlap", () => { const logs: RegistryEntry = { ...plain, page: "UN10", v1Routes: ["/:orgName/flows/:id/logs"], replaces: [], + toV0: {}, toV1: {}, }; + // A valid registry has no such overlap; validation rejects it. + expect(registryProblems([plain, logs])).toContain( + "UN05 /:orgName/flows/:id/* overlaps UN10 /:orgName/flows/:id/logs", + ); const both = { entries: [plain, logs], oldVersion: false }; expect(resolveRoute({ pathname: "/acme/flows/7/logs" }, both)).toEqual({ kind: "v1", @@ -160,24 +180,66 @@ describe("resolvePageUrl", () => { ).toBe("/acme/review/approve"); } }); - it("is null for an undeclared page, an unknown pattern or a route with no v0 mapping", () => { + it("ignores the old-version choice on a page without the toggle", () => { + expect( + resolvePageUrl("UN05", "/:orgName/flows/:id/*", params, entries, { + entries, + oldVersion: true, + }), + ).toBe("/acme/flows/9"); + }); + it("gives a declared, not-live page's v0 URL", () => { + expect( + resolvePageUrl("UN05", "/:orgName/flows/:id/*", params, entries, { + entries: [], + oldVersion: false, + }), + ).toBe("/acme/workflows/9"); + }); + it("decides liveness by page key, not object identity", () => { + expect( + resolvePageUrl("UN11", "/:orgName/hitl", params, entries, { + entries: [{ ...review }], + oldVersion: false, + }), + ).toBe("/acme/hitl"); + }); + it("encodes raw param values", () => { + expect( + resolvePageUrl( + "UN11", + "/:orgName/hitl/:id/approve", + { orgName: "a/b", id: "c?d" }, + entries, + { + entries, + oldVersion: false, + }, + ), + ).toBe("/a%2Fb/hitl/c%3Fd/approve"); + }); + it("is null for an undeclared page or an unknown pattern", () => { const ctx = { entries: [], oldVersion: false }; expect( - resolvePageUrl("UN99", "/:orgName/hitl", params, entries, ctx), + resolvePageUrl("UN09", "/:orgName/hitl", params, entries, ctx), ).toBeNull(); expect( resolvePageUrl("UN11", "/:orgName/nope", params, entries, ctx), ).toBeNull(); - expect( - resolvePageUrl("UN05", "/:orgName/workflows", params, entries, ctx), - ).toBeNull(); }); }); /** Mapping both ways, for every entry the app declares. */ describe.each([...PAGES, ...DEMO_PAGES])("$page mapping", (entry) => { const self = { entries: [entry] }; - const fill = (pattern: string) => pattern.replaceAll(/:(\w+)/g, "p-$1"); + // Placeholder params; a splat is left empty, as generatePath drops it. + const fill = (pattern: string) => + fillPattern(pattern, { + ...Object.fromEntries( + [...pattern.matchAll(/:(\w+)/g)].map(([, name]) => [name, `p-${name}`]), + ), + "*": "", + }); it("each v0 route redirects to its mapped v1 route, which renders v1", () => { for (const [v0, v1] of Object.entries(entry.toV1)) { diff --git a/frontend/src/v1/app/registry/resolve.ts b/frontend/src/v1/app/registry/resolve.ts index f9c7c169d0..112c694015 100644 --- a/frontend/src/v1/app/registry/resolve.ts +++ b/frontend/src/v1/app/registry/resolve.ts @@ -18,7 +18,11 @@ export type ResolveContext = Readonly<{ oldVersion: boolean | null; }>; -type Location = Readonly<{ pathname: string; search?: string; hash?: string }>; +type RouteLocation = Readonly<{ + pathname: string; + search?: string; + hash?: string; +}>; /** The live entries: declared, registered (or previewed) and shipped in this distribution. */ export function activeEntries( @@ -34,7 +38,7 @@ export function activeEntries( type Owned = Readonly<{ path: string; entry: RegistryEntry }>; /** The registry route a pathname lands on; React Router ranks the most specific pattern first. */ -function match( +export function matchRegistryRoute( pathname: string, entries: readonly RegistryEntry[], ): { route: Owned; params: Params } | null { @@ -48,28 +52,49 @@ function match( return found ? { route: found.route, params: found.params } : null; } +/** + * Fills a pattern with raw (decoded) param values, encoding each one, so a + * value holding `/`, `?` or `#` stays inside its segment. A splat keeps its + * slashes and encodes each of its segments. + */ +export function fillPattern( + pattern: string, + params: Readonly>, +): string { + const encoded: Record = {}; + for (const [name, value] of Object.entries(params)) { + if (value !== undefined) { + encoded[name] = + name === "*" + ? value.split("/").map(encodeURIComponent).join("/") + : encodeURIComponent(value); + } + } + return generatePath(pattern, encoded); +} + function redirect( target: string, params: Params, - location: Location, + location: RouteLocation, ): Resolution { return { kind: "redirect", - to: `${generatePath(target, params)}${location.search ?? ""}${location.hash ?? ""}`, + to: `${fillPattern(target, params)}${location.search ?? ""}${location.hash ?? ""}`, }; } /** * The resolution rule. An unregistered URL renders v0. A registered page - * renders v1, and its v0 URLs redirect to their mapped v1 route; except that - * a toggle page (Human Review) does the reverse while the user has chosen the - * old version. + * renders v1, and its v0 URLs redirect to their mapped v1 route. A toggle page + * (Human Review) does the reverse while the user has chosen the old version, + * and keeps its v0 URLs on v0 while the user is unknown. */ export function resolveRoute( - location: Location, + location: RouteLocation, ctx: ResolveContext, ): Resolution { - const found = match(location.pathname, ctx.entries); + const found = matchRegistryRoute(location.pathname, ctx.entries); if (!found) { return { kind: "v0" }; } @@ -85,7 +110,7 @@ export function resolveRoute( const target = entry.toV0[path]; return target ? redirect(target, found.params, location) : { kind: "v0" }; } - // Unknown user on a v1-only URL: v1 is the default. + // Unknown user on one of the page's v1 URLs: render v1, the default. } if (isV1) { return { kind: "v1", entry }; @@ -95,9 +120,11 @@ export function resolveRoute( } /** - * The URL to open a page's v1 route with, in whichever version the user gets: - * the filled v1 route when the page is live, else its mapped v0 route. Null - * when the page is not declared or has no v0 mapping for that route. + * The URL that opens a page's v1 route in the version the user gets: the v1 + * URL when the page is live (unless it is a toggle page and the user chose the + * old version), else its mapped v0 URL. Params are raw values; they are + * encoded here. Null when the page is not declared or the pattern is not one + * of its v1 routes. */ export function resolvePageUrl( page: PageKey, @@ -110,11 +137,11 @@ export function resolvePageUrl( if (!entry?.v1Routes.includes(v1Pattern)) { return null; } - const live = ctx.entries.includes(entry); + const live = ctx.entries.some((e) => e.page === page); const useV0 = !live || (entry.toggle && ctx.oldVersion === true); if (!useV0) { - return generatePath(v1Pattern, params); + return fillPattern(v1Pattern, params); } const target = entry.toV0[v1Pattern]; - return target ? generatePath(target, params) : null; + return target ? fillPattern(target, params) : null; } diff --git a/frontend/src/v1/app/registry/types.ts b/frontend/src/v1/app/registry/types.ts index 06eb4be78d..16c90d457f 100644 --- a/frontend/src/v1/app/registry/types.ts +++ b/frontend/src/v1/app/registry/types.ts @@ -2,23 +2,19 @@ * Route registry types (UN-4271). * * This folder's data files (types.ts, entries.ts) import only each other, with - * `./`, and never touch `import.meta.env`: the E2E harness imports them from - * Playwright, which applies neither Vite nor the `@v1` alias. + * `./`, and never touch `import.meta.env`, so a Playwright E2E harness (no + * Vite, no `@v1` alias) can import them; P0-T09 (UN-4345) wires one in. */ -/** A build the frontend ships in. */ -export type Distribution = "oss" | "cloud" | "onprem"; +/** The builds the frontend ships in. */ +export const DISTRIBUTIONS = ["oss", "cloud", "onprem"] as const; -export const DISTRIBUTIONS: readonly Distribution[] = [ - "oss", - "cloud", - "onprem", -]; +export type Distribution = (typeof DISTRIBUTIONS)[number]; /** * Target page key: UN01 to UN20 (Unstract), LW01 to LW07 (LLMWhisperer), or a - * development-only demo page (`DEMO-…`) that exists only when - * VITE_V1_DEV_ROUTES is on. + * development-only demo page (`DEMO-…`) that is live only when + * VITE_V1_DEV_ROUTES is on. The exact format is checked by registryProblems. */ export type PageKey = `UN${string}` | `LW${string}` | `DEMO-${string}`; @@ -33,14 +29,17 @@ export type RegistryEntry = Readonly<{ /** The v0 route patterns this page takes over. */ replaces: readonly string[]; /** - * v1 pattern -> the v0 pattern it maps back to. Read by Human Review's - * old-version mode, so a toggle page maps every v1 route; a v1-only - * sub-route maps to the nearest v0 route. + * v1 pattern -> the v0 pattern it maps back to; covers every v1 route. Used + * by Human Review's old-version mode, and by resolveUrl while the page is + * not live. A v1-only sub-route maps to the nearest v0 route. */ toV0: Readonly>; /** v0 pattern -> the v1 pattern it redirects to; covers every `replaces` pattern. */ toV1: Readonly>; - /** Honours the old-version preference. True for UN11 (Human Review) only, D14. */ + /** + * Honours the old-version preference. Allowed only for UN11 (Human Review, + * D14) and its demo stand-in; see TOGGLE_PAGES. + */ toggle: boolean; /** Builds the page exists in; an entry the current build lacks is ignored. */ distributions: readonly Distribution[]; diff --git a/frontend/src/v1/app/registry/validate.test.ts b/frontend/src/v1/app/registry/validate.test.ts index c4258e9abe..4bcef4c869 100644 --- a/frontend/src/v1/app/registry/validate.test.ts +++ b/frontend/src/v1/app/registry/validate.test.ts @@ -28,13 +28,30 @@ const base: RegistryEntry = { page: "UN05", v1Routes: ["/:orgName/workflows"], replaces: ["/:orgName/old-workflows"], - toV0: {}, + toV0: { "/:orgName/workflows": "/:orgName/old-workflows" }, toV1: { "/:orgName/old-workflows": "/:orgName/workflows" }, toggle: false, distributions: ["oss"], registeredBy: "P3-T01", }; +/** An entry with only v1 routes, each mapped back to one v0 route. */ +function v1Only( + page: RegistryEntry["page"], + v1Routes: string[], +): RegistryEntry { + return { + ...base, + page, + v1Routes, + replaces: ["/:orgName/old-workflows"], + toV0: Object.fromEntries( + v1Routes.map((r) => [r, "/:orgName/old-workflows"]), + ), + toV1: { "/:orgName/old-workflows": v1Routes[0] ?? "" }, + }; +} + const problemsOf = (...entries: RegistryEntry[]) => registryProblems(entries); describe("registryProblems", () => { @@ -43,29 +60,20 @@ describe("registryProblems", () => { }); it("rejects the toggle on any page but Human Review (D14)", () => { - expect( - problemsOf({ - ...base, - toggle: true, - toV0: { "/:orgName/workflows": "/:orgName/old-workflows" }, - }), - ).toEqual([ + expect(problemsOf({ ...base, toggle: true })).toEqual([ "UN05: only Human Review (UN11) may honour the old-version preference (D14)", ]); - expect( - problemsOf({ - ...base, - page: "UN11", - toggle: true, - toV0: { "/:orgName/workflows": "/:orgName/old-workflows" }, - }), - ).toEqual([]); + expect(problemsOf({ ...base, page: "UN11", toggle: true })).toEqual([]); }); - it("requires a toggle page to map every v1 route back to v0", () => { - expect(problemsOf({ ...base, page: "UN11", toggle: true })).toEqual([ - "UN11: v1 route /:orgName/workflows has no v0 route for old-version mode", - ]); + it("requires every v1 route to map back to v0, on every page", () => { + for (const page of ["UN05", "UN11"] as const) { + expect( + problemsOf({ ...base, page, toggle: page === "UN11", toV0: {} }), + ).toEqual([ + `${page}: v1 route /:orgName/workflows has no v0 route to map back to`, + ]); + } }); it("requires every replaced v0 route to redirect somewhere", () => { @@ -87,6 +95,7 @@ describe("registryProblems", () => { problemsOf({ ...base, v1Routes: ["/:orgName/workflows/:id"], + toV0: { "/:orgName/workflows/:id": "/:orgName/old-workflows" }, toV1: { "/:orgName/old-workflows": "/:orgName/workflows/:id" }, }), ).toEqual([ @@ -94,52 +103,92 @@ describe("registryProblems", () => { ]); }); - it("rejects overlapping patterns across pages, but not shared prefixes", () => { - const other: RegistryEntry = { - ...base, - page: "UN10", - v1Routes: ["/:orgName/workflows/*"], - replaces: [], - toV1: {}, - }; - expect(problemsOf(base, other)).toEqual([ - "UN05 /:orgName/workflows overlaps UN10 /:orgName/workflows/*", + it("rejects a mapping whose URL lands on a route of the other version (a redirect loop)", () => { + // v0 `/hr` outranks the v1 splat `/hr/*` for the URL `/org/hr`. + expect( + problemsOf({ + ...base, + v1Routes: ["/:orgName/hr/*"], + replaces: ["/:orgName/hr"], + toV0: { "/:orgName/hr/*": "/:orgName/hr" }, + toV1: { "/:orgName/hr": "/:orgName/hr/*" }, + }), + ).toEqual([ + "UN05: toV1[/:orgName/hr] -> /:orgName/hr/* lands on /:orgName/hr, which redirects back", + ]); + // The same shape for old-version mode on Human Review. + expect( + problemsOf({ + ...base, + page: "UN11", + toggle: true, + v1Routes: ["/:orgName/review"], + replaces: ["/:orgName/review/*"], + toV0: { "/:orgName/review": "/:orgName/review/*" }, + toV1: { "/:orgName/review/*": "/:orgName/review" }, + }), + ).toEqual([ + "UN11: toV0[/:orgName/review] -> /:orgName/review/* lands on /:orgName/review, which redirects back", ]); + }); + + it("accepts a route both versions share (same-URL takeover)", () => { expect( - problemsOf(base, { ...other, v1Routes: ["/:orgName/workflows/logs"] }), + problemsOf({ + ...base, + v1Routes: ["/:orgName/workflows"], + replaces: ["/:orgName/workflows"], + toV0: { "/:orgName/workflows": "/:orgName/workflows" }, + toV1: { "/:orgName/workflows": "/:orgName/workflows" }, + }), ).toEqual([]); }); - it("rejects duplicates, empty routes or distributions, and malformed patterns", () => { + it("rejects overlapping patterns across pages, but not shared prefixes", () => { + expect(problemsOf(base, v1Only("UN10", ["/:orgName/workflows/*"]))).toEqual( + expect.arrayContaining([ + "UN05 /:orgName/workflows overlaps UN10 /:orgName/workflows/*", + ]), + ); expect( problemsOf(base, { - ...base, - v1Routes: ["/:orgName/other"], - replaces: [], - toV1: {}, + ...v1Only("UN10", ["/:orgName/workflows/logs"]), + replaces: ["/:orgName/old-logs"], + toV0: { "/:orgName/workflows/logs": "/:orgName/old-logs" }, + toV1: { "/:orgName/old-logs": "/:orgName/workflows/logs" }, }), - ).toContain("UN05: declared twice"); + ).toEqual([]); + }); + + it("rejects malformed page keys", () => { + for (const page of ["UN5", "UN21", "UNKNOWN", "LW08", "DEMO-"] as const) { + expect(problemsOf({ ...base, page })).toEqual([ + `${page}: not a page key (UN01–UN20, LW01–LW07 or DEMO-…)`, + ]); + } + }); + + it("rejects duplicates, empty routes or distributions, and malformed patterns", () => { + expect(problemsOf(base, base)).toContain("UN05: declared twice"); expect( problemsOf({ ...base, v1Routes: [], replaces: [], + toV0: {}, toV1: {}, distributions: [], }), ).toEqual(["UN05: has no v1 routes", "UN05: ships in no distribution"]); expect( - problemsOf({ - ...base, - v1Routes: ["workflows", "/:orgName/:id?", "/a/*/b"], - replaces: [], - toV1: {}, - }), - ).toEqual([ - "UN05: route workflows must start with /", - "UN05: route /:orgName/:id? segment ':id?' is not a static name, :param or a final *", - "UN05: route /a/*/b * is allowed only as the last segment", - ]); + problemsOf(v1Only("UN05", ["workflows", "/:orgName/:id?", "/a/*/b"])), + ).toEqual( + expect.arrayContaining([ + "UN05: route workflows must start with /", + "UN05: route /:orgName/:id? segment ':id?' is not a static name, :param or a final *", + "UN05: route /a/*/b * is allowed only as the last segment", + ]), + ); }); }); diff --git a/frontend/src/v1/app/registry/validate.ts b/frontend/src/v1/app/registry/validate.ts index c60b4d347d..01be09cc92 100644 --- a/frontend/src/v1/app/registry/validate.ts +++ b/frontend/src/v1/app/registry/validate.ts @@ -1,9 +1,17 @@ import { paramNames, patternProblem, patternsOverlap } from "./patterns"; +import { fillPattern, matchRegistryRoute } from "./resolve"; import type { PageKey, RegistryEntry } from "./types"; /** Pages allowed to honour the old-version preference: Human Review (D14) and its demo stand-in. */ export const TOGGLE_PAGES: readonly PageKey[] = ["UN11", "DEMO-REVIEW"]; +const PAGE_KEY = /^(UN(0[1-9]|1\d|20)|LW0[1-7]|DEMO-[A-Z0-9]+(-[A-Z0-9]+)*)$/; + +/** True for UN01–UN20, LW01–LW07 and `DEMO-…` keys. */ +export function isPageKey(value: unknown): value is PageKey { + return typeof value === "string" && PAGE_KEY.test(value); +} + function checkMapping( entry: RegistryEntry, name: "toV0" | "toV1", @@ -12,22 +20,37 @@ function checkMapping( problems: string[], ): void { for (const [source, target] of Object.entries(entry[name])) { + const where = `${entry.page}: ${name}[${source}] -> ${target}`; if (!from.includes(source)) { problems.push( `${entry.page}: ${name} key ${source} is not one of its routes`, ); } if (!to.includes(target)) { - problems.push( - `${entry.page}: ${name}[${source}] -> ${target} is not one of its routes`, - ); + problems.push(`${where} is not one of its routes`); + continue; } // A splat may be empty, so a target ending in `*` needs nothing from the source. const available = [...paramNames(source), "*"]; const missing = paramNames(target).filter((p) => !available.includes(p)); if (missing.length) { problems.push( - `${entry.page}: ${name}[${source}] -> ${target} needs params ${source} lacks: ${missing.join(", ")}`, + `${where} needs params ${source} lacks: ${missing.join(", ")}`, + ); + } + if (patternProblem(target)) { + continue; // reported with the page's routes + } + // The URL a redirect produces must land on a route of the version it + // redirects to; a pattern of the other version outranking it would + // redirect straight back, forever. + const sample = Object.fromEntries( + paramNames(target).map((p) => [p, p === "*" ? "" : `p-${p}`]), + ); + const landed = matchRegistryRoute(fillPattern(target, sample), [entry]); + if (!landed || !to.includes(landed.route.path)) { + problems.push( + `${where} lands on ${landed?.route.path ?? "no route"}, which redirects back`, ); } } @@ -35,6 +58,9 @@ function checkMapping( function checkEntry(entry: RegistryEntry, problems: string[]): void { const { page, v1Routes, replaces } = entry; + if (!isPageKey(page)) { + problems.push(`${page}: not a page key (UN01–UN20, LW01–LW07 or DEMO-…)`); + } for (const pattern of [...v1Routes, ...replaces]) { const problem = patternProblem(pattern); if (problem) { @@ -61,13 +87,13 @@ function checkEntry(entry: RegistryEntry, problems: string[]): void { ); } } - if (entry.toggle) { - for (const pattern of v1Routes) { - if (!(pattern in entry.toV0)) { - problems.push( - `${page}: v1 route ${pattern} has no v0 route for old-version mode`, - ); - } + // Every v1 route maps back: Human Review's old-version mode needs it, and + // so does resolveUrl for a page that is declared but not live yet. + for (const pattern of v1Routes) { + if (!(pattern in entry.toV0)) { + problems.push( + `${page}: v1 route ${pattern} has no v0 route to map back to`, + ); } } } @@ -86,7 +112,8 @@ export function registryProblems(entries: readonly RegistryEntry[]): string[] { seen.add(entry.page); checkEntry(entry, problems); } - // Two pages may share a prefix (the longest match wins), but no URL may match both. + // Two pages may share a prefix (/:o/workflows and /:o/workflows/logs), but + // no URL may match patterns of both. const owned = entries.flatMap((entry) => [...new Set([...entry.v1Routes, ...entry.replaces])].map((pattern) => ({ page: entry.page, diff --git a/frontend/src/v1/index.ts b/frontend/src/v1/index.ts index 5ffd5174ca..4ce98a3cfb 100644 --- a/frontend/src/v1/index.ts +++ b/frontend/src/v1/index.ts @@ -6,46 +6,14 @@ * the registry, and the v1 tree itself loads only when a v1 page renders. */ -import { LIVE_REGISTRY, V1_DEV_ROUTES } from "@v1/app/registry/live"; -import { resolvePageUrl } from "@v1/app/registry/resolve"; -import type { PageKey } from "@v1/app/registry/types"; -import { useOldVersionStore } from "@v1/stores/oldVersion"; - -export { VersionSwitch, type VersionSwitchProps } from "@v1/app/VersionSwitch"; - -/** True in a build with the development-only demo routes (VITE_V1_DEV_ROUTES). */ -export const v1DevRoutesEnabled: boolean = V1_DEV_ROUTES; - -/** - * Sets the signed-in user's old-version preference, which only Human Review - * honours (D14). The version switch then moves the user to the equivalent - * screen without a reload. False, and no change, while no user is known. - * For the switch controls on both versions of Human Review (P6-T07). - */ -export function setOldVersion(on: boolean): boolean { - return useOldVersionStore.getState().setOldVersion(on); -} - -/** - * The URL that opens `page` at one of its v1 route patterns, in the version - * the user gets: the v1 URL when the page is live, else its mapped v0 URL. - * Null when the page is not declared or that route has no v0 mapping. - * - * resolveUrl("UN05", "/:orgName/workflows/:id", { orgName, id }) - */ -export function resolveUrl( - page: PageKey, - v1Pattern: string, - params: Readonly>, -): string | null { - return resolvePageUrl(page, v1Pattern, params, LIVE_REGISTRY.pages, { - entries: LIVE_REGISTRY.entries, - oldVersion: useOldVersionStore.getState().oldVersion, - }); -} - +export { + resolveUrl, + setOldVersion, + v1DevRoutesEnabled, +} from "@v1/app/api"; export type { Distribution, PageKey, RegistryEntry, } from "@v1/app/registry/types"; +export { VersionSwitch, type VersionSwitchProps } from "@v1/app/VersionSwitch"; diff --git a/frontend/src/v1/lib/chunkReload.test.ts b/frontend/src/v1/lib/chunkReload.test.ts new file mode 100644 index 0000000000..9043472adf --- /dev/null +++ b/frontend/src/v1/lib/chunkReload.test.ts @@ -0,0 +1,56 @@ +import { describe, expect, it, vi } from "vitest"; +import { isChunkLoadError, reloadOnce } from "./chunkReload"; + +function memoryStorage() { + const data = new Map(); + return { + getItem: (k: string) => data.get(k) ?? null, + setItem: (k: string, v: string) => { + data.set(k, v); + }, + }; +} + +describe("isChunkLoadError", () => { + it.each([ + new TypeError("Failed to fetch dynamically imported module: /a.js"), + new TypeError("error loading dynamically imported module"), + new TypeError("Importing a module script failed."), + Object.assign(new Error("x"), { name: "ChunkLoadError" }), + ])("matches %s", (error) => { + expect(isChunkLoadError(error)).toBe(true); + }); + + it("does not match render errors or non-errors", () => { + expect(isChunkLoadError(new Error("Cannot read properties of null"))).toBe( + false, + ); + expect( + isChunkLoadError("Failed to fetch dynamically imported module"), + ).toBe(false); + }); +}); + +describe("reloadOnce", () => { + it("reloads once, then not again within ten seconds, then again after", () => { + const storage = memoryStorage(); + const reload = vi.fn(); + expect(reloadOnce(storage, reload, 100_000)).toBe(true); + expect(reloadOnce(storage, reload, 105_000)).toBe(false); + expect(reloadOnce(storage, reload, 111_000)).toBe(true); + expect(reload).toHaveBeenCalledTimes(2); + }); + + it("never reloads without usable session storage", () => { + const reload = vi.fn(); + expect(reloadOnce(null, reload)).toBe(false); + const refusing = { + getItem: () => null, + setItem: () => { + throw new Error("denied"); + }, + }; + expect(reloadOnce(refusing, reload)).toBe(false); + expect(reload).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/v1/lib/chunkReload.ts b/frontend/src/v1/lib/chunkReload.ts new file mode 100644 index 0000000000..8680021520 --- /dev/null +++ b/frontend/src/v1/lib/chunkReload.ts @@ -0,0 +1,45 @@ +/* + * Recovery from a stale code chunk after a deploy: the browser's page names a + * hashed file the server no longer has. Mirrors v0's handleRouteError + * (components/error/LazyOutlet), which v1 may not import. + */ + +const RELOAD_KEY = "unstract.v1.chunkReloadAt"; +const RELOAD_WINDOW_MS = 10_000; + +/** A failed dynamic import; matching is best-effort across browsers. */ +export function isChunkLoadError(error: unknown): boolean { + if (!(error instanceof Error)) { + return false; + } + const { message } = error; + return ( + error.name === "ChunkLoadError" || + message.includes("Failed to fetch dynamically imported module") || + message.includes("error loading dynamically imported module") || + message.includes("Importing a module script failed") + ); +} + +/** + * Reloads once to fetch the new deploy's chunks. A repeat failure within ten + * seconds does not reload again, so a chunk that is really gone shows the + * manual fallback instead of looping. True when it reloaded. + */ +export function reloadOnce( + storage: Pick | null, + reload: () => void, + now: number = Date.now(), +): boolean { + try { + const last = Number(storage?.getItem(RELOAD_KEY) ?? 0); + if (!storage || now - last <= RELOAD_WINDOW_MS) { + return false; + } + storage.setItem(RELOAD_KEY, String(now)); + } catch { + return false; // no session storage: leave recovery to the fallback + } + reload(); + return true; +} diff --git a/frontend/src/v1/pages/demo/index.tsx b/frontend/src/v1/pages/demo/index.tsx index 8044fa3c9e..bbe0f0ca14 100644 --- a/frontend/src/v1/pages/demo/index.tsx +++ b/frontend/src/v1/pages/demo/index.tsx @@ -1,8 +1,8 @@ /* * Development-only demo pages that prove the version switch (UN-4271). They - * exist only in a build with VITE_V1_DEV_ROUTES=true. DemoReviewPage stands in - * for Human Review: its "Switch to old version" control is the shape of the - * one P6-T07 builds. + * ship in every build but are reachable only with VITE_V1_DEV_ROUTES=true. + * DemoReviewPage stands in for Human Review: its "Switch to old version" + * control is the shape of the one P6-T07 builds. */ import { useOldVersionStore } from "@v1/stores/oldVersion"; diff --git a/frontend/vite.config.js b/frontend/vite.config.js index fc4f72eb42..6bc9994033 100644 --- a/frontend/vite.config.js +++ b/frontend/vite.config.js @@ -139,6 +139,16 @@ export default defineConfig(({ mode }) => { // Load env file based on `mode` in the current working directory. const env = loadEnv(mode, process.cwd(), ""); + // The v1 route registry drops pages the build's distribution lacks. Fail + // the build on a value it would not recognise; the list mirrors + // DISTRIBUTIONS in src/v1/app/registry/types.ts. + const distribution = env.VITE_UNSTRACT_DISTRIBUTION?.trim().toLowerCase(); + if (distribution && !["oss", "cloud", "onprem"].includes(distribution)) { + throw new Error( + `VITE_UNSTRACT_DISTRIBUTION=${env.VITE_UNSTRACT_DISTRIBUTION} is not one of oss, cloud, onprem`, + ); + } + return { plugins: [ optionalPluginImports(), From 3a8c30080534c3b4acda800894fc0b90fc2837c7 Mon Sep 17 00:00:00 2001 From: Jaseem Jas Date: Wed, 4 Nov 2026 15:24:06 +0530 Subject: [PATCH 4/7] fix(frontend-v1): reject same-page route overlaps, encode demo links (UN-4271) - Validation rejects a v1 and a v0 route of the same page that match the same URL (identical patterns excepted). This covers the redirect loop the placeholder-URL check caught and the case it missed: a param equal to a static segment (/old/settings -> /new/settings -> /settings). - The demo review page encodes orgName in its links, so an encoded org name stays one segment. - Use a Set for the param lookup (Sonar S7776). --- frontend/src/v1/README.md | 5 +-- frontend/src/v1/app/VersionSwitch.test.tsx | 8 +++++ frontend/src/v1/app/registry/validate.test.ts | 23 +++++++++----- frontend/src/v1/app/registry/validate.ts | 31 ++++++++----------- frontend/src/v1/pages/demo/index.tsx | 7 +++-- 5 files changed, 44 insertions(+), 30 deletions(-) diff --git a/frontend/src/v1/README.md b/frontend/src/v1/README.md index e176bece9f..ff3e2a491c 100644 --- a/frontend/src/v1/README.md +++ b/frontend/src/v1/README.md @@ -77,8 +77,9 @@ only when a v1 page first renders. - **Register** it, in its go-live task only (D9), by adding its key to `REGISTERED_PAGES`. Its v0 URLs then redirect to v1; reverting is that line. - `app/registry/validate.test.ts` fails CI on an invalid entry: overlapping - patterns across pages, a route without a mapping to the other version, a - mapping that would redirect back, a page without a component, or `toggle` + patterns across pages, a v1 and a v0 route of one page that match the same + URL, a route without a mapping to the other version, a page without a + component, or `toggle` on any page but Human Review (UN11, D14) and its demo stand-in. - Human Review's old-version preference is per user, per browser, in localStorage (`unstract.v1.oldVersion.`). `setOldVersion(on)` sets diff --git a/frontend/src/v1/app/VersionSwitch.test.tsx b/frontend/src/v1/app/VersionSwitch.test.tsx index 561845fbe2..b6c5c9f43e 100644 --- a/frontend/src/v1/app/VersionSwitch.test.tsx +++ b/frontend/src/v1/app/VersionSwitch.test.tsx @@ -135,6 +135,14 @@ describe("VersionSwitch", () => { expect(window.localStorage.getItem(oldVersionStorageKey("7"))).toBeNull(); }); + it("demo links keep an encoded org name in one segment", async () => { + const user = userEvent.setup(); + renderAt("/a%2Fb/v1-demo-review"); + await user.click(await screen.findByRole("link", { name: "Details" })); + expect(where()).toBe("/a%2Fb/v1-demo-review/details"); + expect(screen.getByTestId("v1-demo-review")).toBeTruthy(); + }); + it("a stored old-version choice applies on load (re-login)", () => { window.localStorage.setItem(oldVersionStorageKey("7"), "true"); renderAt("/acme/v1-demo-review"); diff --git a/frontend/src/v1/app/registry/validate.test.ts b/frontend/src/v1/app/registry/validate.test.ts index 4bcef4c869..00e1341810 100644 --- a/frontend/src/v1/app/registry/validate.test.ts +++ b/frontend/src/v1/app/registry/validate.test.ts @@ -103,8 +103,8 @@ describe("registryProblems", () => { ]); }); - it("rejects a mapping whose URL lands on a route of the other version (a redirect loop)", () => { - // v0 `/hr` outranks the v1 splat `/hr/*` for the URL `/org/hr`. + it("rejects a v1 and a v0 route of the same page that match the same URL", () => { + // v0 `/hr` outranks the v1 splat `/hr/*` for `/org/hr`: an endless redirect. expect( problemsOf({ ...base, @@ -113,10 +113,8 @@ describe("registryProblems", () => { toV0: { "/:orgName/hr/*": "/:orgName/hr" }, toV1: { "/:orgName/hr": "/:orgName/hr/*" }, }), - ).toEqual([ - "UN05: toV1[/:orgName/hr] -> /:orgName/hr/* lands on /:orgName/hr, which redirects back", - ]); - // The same shape for old-version mode on Human Review. + ).toEqual(["UN05: v1 route /:orgName/hr/* overlaps v0 route /:orgName/hr"]); + // The same shape in Human Review's old-version mode. expect( problemsOf({ ...base, @@ -128,8 +126,19 @@ describe("registryProblems", () => { toV1: { "/:orgName/review/*": "/:orgName/review" }, }), ).toEqual([ - "UN11: toV0[/:orgName/review] -> /:orgName/review/* lands on /:orgName/review, which redirects back", + "UN11: v1 route /:orgName/review overlaps v0 route /:orgName/review/*", ]); + // A param equal to a static segment: `/old/settings` would redirect to + // `/new/settings`, which the v0 route takes, and on to `/settings`. + expect( + problemsOf({ + ...base, + v1Routes: ["/new/:id", "/settings"], + replaces: ["/old/:id", "/new/settings"], + toV0: { "/new/:id": "/old/:id", "/settings": "/new/settings" }, + toV1: { "/old/:id": "/new/:id", "/new/settings": "/settings" }, + }), + ).toEqual(["UN05: v1 route /new/:id overlaps v0 route /new/settings"]); }); it("accepts a route both versions share (same-URL takeover)", () => { diff --git a/frontend/src/v1/app/registry/validate.ts b/frontend/src/v1/app/registry/validate.ts index 01be09cc92..c14243ca17 100644 --- a/frontend/src/v1/app/registry/validate.ts +++ b/frontend/src/v1/app/registry/validate.ts @@ -1,5 +1,4 @@ import { paramNames, patternProblem, patternsOverlap } from "./patterns"; -import { fillPattern, matchRegistryRoute } from "./resolve"; import type { PageKey, RegistryEntry } from "./types"; /** Pages allowed to honour the old-version preference: Human Review (D14) and its demo stand-in. */ @@ -31,28 +30,13 @@ function checkMapping( continue; } // A splat may be empty, so a target ending in `*` needs nothing from the source. - const available = [...paramNames(source), "*"]; - const missing = paramNames(target).filter((p) => !available.includes(p)); + const available = new Set([...paramNames(source), "*"]); + const missing = paramNames(target).filter((p) => !available.has(p)); if (missing.length) { problems.push( `${where} needs params ${source} lacks: ${missing.join(", ")}`, ); } - if (patternProblem(target)) { - continue; // reported with the page's routes - } - // The URL a redirect produces must land on a route of the version it - // redirects to; a pattern of the other version outranking it would - // redirect straight back, forever. - const sample = Object.fromEntries( - paramNames(target).map((p) => [p, p === "*" ? "" : `p-${p}`]), - ); - const landed = matchRegistryRoute(fillPattern(target, sample), [entry]); - if (!landed || !to.includes(landed.route.path)) { - problems.push( - `${where} lands on ${landed?.route.path ?? "no route"}, which redirects back`, - ); - } } } @@ -78,6 +62,17 @@ function checkEntry(entry: RegistryEntry, problems: string[]): void { `${page}: only Human Review (UN11) may honour the old-version preference (D14)`, ); } + // A URL matching both a v1 route and a different v0 route of the page is + // ambiguous: the more specific pattern wins, so a redirect can land on the + // other version and bounce on to the wrong screen, or back, forever. + // Identical patterns (the page keeps its v0 URL) are fine. + for (const v1 of v1Routes) { + for (const v0 of replaces) { + if (v1 !== v0 && patternsOverlap(v1, v0)) { + problems.push(`${page}: v1 route ${v1} overlaps v0 route ${v0}`); + } + } + } checkMapping(entry, "toV1", replaces, v1Routes, problems); checkMapping(entry, "toV0", v1Routes, replaces, problems); for (const pattern of replaces) { diff --git a/frontend/src/v1/pages/demo/index.tsx b/frontend/src/v1/pages/demo/index.tsx index bbe0f0ca14..c3c873fa49 100644 --- a/frontend/src/v1/pages/demo/index.tsx +++ b/frontend/src/v1/pages/demo/index.tsx @@ -18,14 +18,15 @@ export function DemoPage(): JSX.Element { } export function DemoReviewPage(): JSX.Element { - const { orgName = "" } = useParams(); + // useParams decodes, so encode again to keep e.g. `a%2Fb` one segment. + const org = encodeURIComponent(useParams().orgName ?? ""); const setOldVersion = useOldVersionStore((s) => s.setOldVersion); return (

v1 demo review