From d82f8708b3d705eeed390c5f691bcdf3ec49179a Mon Sep 17 00:00:00 2001 From: GenWave Radio Date: Thu, 6 Aug 2026 09:07:39 -0600 Subject: [PATCH 1/2] =?UTF-8?q?fix(ui):=20catalog=20font=20detail=20?= =?UTF-8?q?=E2=80=94=20licence=20trio,=20honest=20installed=20state;=20War?= =?UTF-8?q?drobe=20rename=20(v3.1.1=20font=20half)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dean's T204 demo feedback. (1) The pre-install trust surface now shows the licence: CatalogEntryResponse projects FontLicense/Version/Subset via the hardened serializer (all six font args passed BY NAME — the null-soup guard), FontDetailPanel renders the same 'licence · version · subset' line the Wardrobe page uses. (2) Installed-state truth: /api/fonts rides the page load (fail-closed to [] — documented direction: a stale 'Install' costs an idempotent re-install; a false 'Installed' costs an operator a missing font), Installed chip + Re-install label, the specimen caption is state-neutral in both states, and a successful install flips state locally (failure flips nothing). (3) /library -> /wardrobe: nav 'Wardrobe', WardrobeClient/WardrobePage/WardrobeIcon, media-Libraries docstring disambiguated; backend route + DTO names stay (wire contracts don't chase UI labels). --- .../font-pack-shelf-specimen.spec.tsx | 85 ++++++++++++++++++- ...y.spec.tsx => font-pack-wardrobe.spec.tsx} | 28 +++--- .../__specs__/persona-catalog-page.spec.tsx | 38 +++++++++ admin-ui/app/(authed)/_components/icons.tsx | 12 +-- .../app/(authed)/_components/nav-items.ts | 25 ++++-- .../persona-catalog/FontDetailPanel.tsx | 57 +++++++++++-- .../persona-catalog/PersonaCatalogClient.tsx | 41 +++++++-- .../persona-catalog/SpecimenBlock.tsx | 2 +- .../(authed)/persona-catalog/font-format.ts | 29 +++++++ .../app/(authed)/persona-catalog/page.tsx | 39 ++++++++- .../app/(authed)/persona-catalog/types.ts | 19 +++-- .../WardrobeClient.tsx} | 39 +++------ .../(authed)/{library => wardrobe}/page.tsx | 16 ++-- .../(authed)/{library => wardrobe}/types.ts | 2 +- src/GenWave.Host/Api/CatalogController.cs | 7 +- src/GenWave.Host/Api/CatalogEntryResponse.cs | 18 +++- .../Specs/Story279_FontKindAssets.cs | 18 ++++ 17 files changed, 386 insertions(+), 89 deletions(-) rename admin-ui/__specs__/{font-pack-library.spec.tsx => font-pack-wardrobe.spec.tsx} (74%) rename admin-ui/app/(authed)/{library/FontLibraryClient.tsx => wardrobe/WardrobeClient.tsx} (72%) rename admin-ui/app/(authed)/{library => wardrobe}/page.tsx (87%) rename admin-ui/app/(authed)/{library => wardrobe}/types.ts (93%) diff --git a/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx b/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx index 0b9d4b08..cb190e93 100644 --- a/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx +++ b/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx @@ -137,6 +137,19 @@ const FONT_DETAIL: CatalogEntryDetailDto = { fontFamily: "Space Grotesk", fontByteTotal: 7844, fontSpecimenFile: "libre-grotesk-variable-latin.woff2", + fontLicense: "OFL-1.1", + fontVersion: "2.000", + fontSubset: "latin", +}; + +// PLAN T204 (Dean's post-v3.1.0 review): "no mention of license anywhere in the panel" — the +// all-null edge, mirroring a manifest that failed to parse server-side (CatalogController's own +// degrade-not-500 posture). +const FONT_DETAIL_WITHOUT_LICENCE: CatalogEntryDetailDto = { + ...FONT_DETAIL, + fontLicense: null, + fontVersion: null, + fontSubset: null, }; const ENTRY_URL = "/api/catalog/entries/libre-grotesk"; @@ -266,13 +279,20 @@ describe("Feature: packs on the shelf with an honest specimen", () => { }); /** Opens Libre Grotesk's detail panel via the shelf card's own title text (the fontFamily "Space - * Grotesk" — see this file's own fixture remarks for why it deliberately differs from the slug). */ - async function openLibreGroteskDetail(fetchMock: jest.MockedFunction): Promise { + * Grotesk" — see this file's own fixture remarks for why it deliberately differs from the slug). + * `installedFontSlugs` (PLAN T204) defaults to `[]`, the same "not installed" default + * `PersonaCatalogClient`'s own prop carries — pass `["libre-grotesk"]` to exercise the + * already-installed path. */ + async function openLibreGroteskDetail( + fetchMock: jest.MockedFunction, + installedFontSlugs: string[] = [] + ): Promise { global.fetch = fetchMock; render( <> @@ -425,6 +445,67 @@ describe("Feature: packs on the shelf with an honest specimen", () => { await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); expect(await screen.findByText('"Space Grotesk" installed.')).toBeInTheDocument(); }); + + it("flips the detail panel to Installed/Re-install locally once the install succeeds, no reload (PLAN T204)", async () => { + const fetchMock = fontFlowFetchMock(); + // Starts NOT installed — the default `installedFontSlugs=[]` — so the button starts "Install". + await openInstallDialog(fetchMock); + + const dialog = within(screen.getByRole("dialog")); + await act(async () => { + fireEvent.click(dialog.getByRole("button", { name: "Confirm install" })); + await Promise.resolve(); + }); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + + // The detail panel itself (still open — only the confirm dialog closed) now reads installed, + // with no second fetch and no page reload: PersonaCatalogClient.handleFontInstalled flips its + // own local state on the toast, the cheap path this task's own spec calls for. + expect(screen.getByText("Installed")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Re-install" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Install" })).not.toBeInTheDocument(); + }); + }); + + describe("Scenario: the licence is visible before install (PLAN T204, Dean's post-v3.1.0 review)", () => { + it("shows the licence · version · subset line on the pre-install review panel", async () => { + const fetchMock = fontFlowFetchMock(); + await openLibreGroteskDetail(fetchMock); + + expect(await screen.findByText("OFL-1.1 · v2.000 · latin")).toBeInTheDocument(); + }); + + it("degrades to 'Licence unknown' rather than a blank line when the manifest carries none", async () => { + const fetchMock = fontFlowFetchMock({ entry: makeJsonResponse(200, FONT_DETAIL_WITHOUT_LICENCE) }); + await openLibreGroteskDetail(fetchMock); + + expect(await screen.findByText("Licence unknown")).toBeInTheDocument(); + }); + }); + + describe("Scenario: installed-state awareness (PLAN T204, Dean's post-v3.1.0 review)", () => { + it("shows Install and a state-neutral specimen caption when the pack is not installed", async () => { + const fetchMock = fontFlowFetchMock(); + await openLibreGroteskDetail(fetchMock); + await screen.findByTestId("font-specimen"); + + expect(screen.getByRole("button", { name: "Install" })).toBeInTheDocument(); + expect(screen.queryByText("Installed")).not.toBeInTheDocument(); + expect(screen.getByText("Transient specimen — previewing installs nothing")).toBeInTheDocument(); + }); + + it("shows an Installed chip, Re-install, and the SAME neutral caption when the pack is already installed", async () => { + const fetchMock = fontFlowFetchMock(); + await openLibreGroteskDetail(fetchMock, ["libre-grotesk"]); + await screen.findByTestId("font-specimen"); + + expect(screen.getByText("Installed")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Re-install" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Install" })).not.toBeInTheDocument(); + // The specimen caption never claims install state either way (F104.4's own "transient, + // installs nothing" fact is true regardless) — see SpecimenBlock's own remarks. + expect(screen.getByText("Transient specimen — previewing installs nothing")).toBeInTheDocument(); + }); }); // ── SAD PATH ──────────────────────────────────────────────────────────── diff --git a/admin-ui/__specs__/font-pack-library.spec.tsx b/admin-ui/__specs__/font-pack-wardrobe.spec.tsx similarity index 74% rename from admin-ui/__specs__/font-pack-library.spec.tsx rename to admin-ui/__specs__/font-pack-wardrobe.spec.tsx index d6c28e12..5693fa17 100644 --- a/admin-ui/__specs__/font-pack-library.spec.tsx +++ b/admin-ui/__specs__/font-pack-wardrobe.spec.tsx @@ -1,7 +1,9 @@ // @jest-environment jsdom -// STORY-284 — The library is inspectable (SPEC F104.7 · PLAN T203); AC2 is the 🖐️ T204 gate. +// STORY-284 — The wardrobe is inspectable (SPEC F104.7 · PLAN T203); AC2 is the 🖐️ T204 gate. +// Nav label/route renamed "Library" → "Wardrobe" at PLAN T204 (Dean's ruling) — this file was +// font-pack-library.spec.tsx before that rename; no behavior changed, only names. // -// Runner: Jest. FontLibraryClient renders GET /api/fonts's own listing — family (title), faces +// Runner: Jest. WardrobeClient renders GET /api/fonts's own listing — family (title), faces // (style + byte size via the shared font-format.ts helper), the licence/version/subset line, and // the "Installed · · " db/25 provenance chip (AC1). `timeZone="UTC"` is pinned // explicitly (the StatusTiles/BoothLogFeed/PersonasClient/SettingsForm house idiom, T105/T187) so @@ -10,8 +12,8 @@ import { describe, it, expect } from "@jest/globals"; import { render, screen, within } from "@testing-library/react"; import "@testing-library/jest-dom"; -import { FontLibraryClient } from "../app/(authed)/library/FontLibraryClient"; -import type { FontLibraryPackDto } from "../app/(authed)/library/types"; +import { WardrobeClient } from "../app/(authed)/wardrobe/WardrobeClient"; +import type { FontLibraryPackDto } from "../app/(authed)/wardrobe/types"; const SPACE_GROTESK_PACK: FontLibraryPackDto = { slug: "space-grotesk", @@ -25,10 +27,10 @@ const SPACE_GROTESK_PACK: FontLibraryPackDto = { importedAt: "2026-08-05T12:00:00Z", }; -describe("Feature: the library is inspectable", () => { - describe("Scenario: the library lists installed packs", () => { +describe("Feature: the wardrobe is inspectable", () => { + describe("Scenario: the wardrobe lists installed packs", () => { it("shows family, faces, byte sizes, and licence per pack (T203, AC1)", () => { - render(); + render(); const list = screen.getByRole("list", { name: "Installed font packs" }); const card = within(list).getByText("Space Grotesk").closest("li"); @@ -40,18 +42,18 @@ describe("Feature: the library is inspectable", () => { }); it("shows 'Installed · · ' provenance per pack (T203, AC1)", () => { - render(); + render(); expect(screen.getByText("Installed · space-grotesk · Aug 5, 2026")).toBeInTheDocument(); }); }); - describe("Scenario: the library is empty", () => { + describe("Scenario: the wardrobe is empty", () => { // T203 review finding F3: the empty-state CTA must not point at /persona-catalog when the - // catalog is disabled — that route itself 404s off-catalog, the exact dead end the Library nav + // catalog is disabled — that route itself 404s off-catalog, the exact dead end the Wardrobe nav // item's own deliberate ungating (SPEC F104.8) exists to let an operator avoid. it("names the reason and offers the Community Catalog CTA when the catalog is enabled", () => { - render(); + render(); expect(screen.getByText("No packs installed")).toBeInTheDocument(); expect(screen.getByRole("link", { name: "Browse the Community Catalog" })).toHaveAttribute( @@ -61,7 +63,7 @@ describe("Feature: the library is inspectable", () => { }); it("points at Settings instead of the catalog when the catalog is disabled", () => { - render(); + render(); expect(screen.getByText("No packs installed")).toBeInTheDocument(); expect( @@ -82,7 +84,7 @@ describe("Feature: the library is inspectable", () => { version: null, subset: null, }; - render(); + render(); expect(screen.getByText("Licence unknown")).toBeInTheDocument(); }); diff --git a/admin-ui/__specs__/persona-catalog-page.spec.tsx b/admin-ui/__specs__/persona-catalog-page.spec.tsx index df7b69ca..73c2a652 100644 --- a/admin-ui/__specs__/persona-catalog-page.spec.tsx +++ b/admin-ui/__specs__/persona-catalog-page.spec.tsx @@ -507,6 +507,44 @@ describe("Feature: The Persona Catalog server page", () => { }); }); + describe("Scenario: installed-font-slug wiring (PLAN T204, Dean's post-v3.1.0 review)", () => { + const indexBody: CatalogIndexResponseDto = { + entries: [EVERYONE_ENTRY], + fetchedAt: "2026-07-26T00:00:00Z", + unreachable: false, + }; + + it("threads GET /api/fonts's own slugs into PersonaCatalogClient as installedFontSlugs", async () => { + global.fetch = jest.fn().mockImplementation(async (input) => { + const url = String(input); + if (url.endsWith("/api/catalog/index")) return makeJsonResponse(200, indexBody); + if (url.endsWith("/api/fonts")) return makeJsonResponse(200, [{ slug: "space-grotesk" }, { slug: "libre-grotesk" }]); + throw new Error(`unexpected fetch ${url}`); + }) as unknown as typeof fetch; + + const { default: PersonaCatalogPage } = await import("../app/(authed)/persona-catalog/page"); + const node = await PersonaCatalogPage(); + + const clientEl = findElementByType(node, PersonaCatalogClient); + expect(clientEl?.props["installedFontSlugs"]).toEqual(["space-grotesk", "libre-grotesk"]); + }); + + it("degrades to an empty list — never crashing the page — when GET /api/fonts fails", async () => { + global.fetch = jest.fn().mockImplementation(async (input) => { + const url = String(input); + if (url.endsWith("/api/catalog/index")) return makeJsonResponse(200, indexBody); + if (url.endsWith("/api/fonts")) return makeJsonResponse(500, {}); + throw new Error(`unexpected fetch ${url}`); + }) as unknown as typeof fetch; + + const { default: PersonaCatalogPage } = await import("../app/(authed)/persona-catalog/page"); + const node = await PersonaCatalogPage(); + + const clientEl = findElementByType(node, PersonaCatalogClient); + expect(clientEl?.props["installedFontSlugs"]).toEqual([]); + }); + }); + describe("Scenario: disabled surface is a bare 404 (SPEC F90.1, sad path)", () => { it("renders an inline 'Not found' page when GET /api/catalog/index 404s", async () => { global.fetch = jest diff --git a/admin-ui/app/(authed)/_components/icons.tsx b/admin-ui/app/(authed)/_components/icons.tsx index c6fb8d99..55e6fd7f 100644 --- a/admin-ui/app/(authed)/_components/icons.tsx +++ b/admin-ui/app/(authed)/_components/icons.tsx @@ -244,11 +244,13 @@ export function ScheduleIcon(props: IconProps): ReactNode { ); } -/** Library nav glyph (PLAN T203, SPEC F104.7) — an open book: two pages meeting at a center - * spine, reading as "the station's own shelf of installed components" — distinct from - * {@link PersonaCatalogIcon}'s three record-like shelf cards (the browsable Community Catalog, - * a different surface entirely) and {@link CatalogIcon}'s plain rule lines (the media library). */ -export function LibraryIcon(props: IconProps): ReactNode { +/** Wardrobe nav glyph (PLAN T203, SPEC F104.7; nav item renamed "Library" → "Wardrobe" at PLAN + * T204, Dean's ruling — this glyph itself is unchanged, only its export name follows the rename) — + * an open book: two pages meeting at a center spine, reading as "the station's own shelf of + * installed components" — distinct from {@link PersonaCatalogIcon}'s three record-like shelf cards + * (the browsable Community Catalog, a different surface entirely) and {@link CatalogIcon}'s plain + * rule lines (the media library). */ +export function WardrobeIcon(props: IconProps): ReactNode { return ( diff --git a/admin-ui/app/(authed)/_components/nav-items.ts b/admin-ui/app/(authed)/_components/nav-items.ts index 8f46ad0f..f3fb668b 100644 --- a/admin-ui/app/(authed)/_components/nav-items.ts +++ b/admin-ui/app/(authed)/_components/nav-items.ts @@ -4,13 +4,13 @@ import { CatalogIcon, DashboardIcon, HealthIcon, - LibraryIcon, LiveIcon, PersonaCatalogIcon, PersonaIcon, SafeContentIcon, ScheduleIcon, SettingsIcon, + WardrobeIcon, type IconProps, } from "./icons"; @@ -30,9 +30,15 @@ export interface NavItem { /** * Sidebar sections per SPEC F28.5, shared by the persistent desktop * `Sidebar` (≥1024px) and the `MobileNav` drawer (<1024px, SPEC F28.13) so - * the two never drift. Libraries is deliberately absent — it lives under - * the Catalog page's Libraries tab (Q7, SPEC F28.11); /libraries is now - * only a redirect into that tab, never its own rendered route. + * the two never drift. + * + * "Libraries" (plural — the MEDIA library, Q7, SPEC F28.11) is deliberately absent from this list: + * it lives under the Catalog page's Libraries tab, and /libraries is only a redirect into that tab, + * never its own rendered route. Do not confuse it with "Wardrobe" below — a DIFFERENT feature + * entirely (SPEC F104.7, installed font packs; named "Library" through v3.1.0, renamed "Wardrobe" at + * PLAN T204, Dean's ruling) that this same stale note used to read as ruling out too (PLAN T203 + * review finding, closed here): the two features share no code, and the naming collision was never + * intentional. */ export const NAV_ITEMS: NavItem[] = [ { href: "/dashboard", label: "Dashboard", Icon: DashboardIcon }, @@ -42,11 +48,12 @@ export const NAV_ITEMS: NavItem[] = [ { href: "/personas", label: "Personas", Icon: PersonaIcon }, { href: "/schedule", label: "Schedule", Icon: ScheduleIcon }, { href: "/persona-catalog", label: "Community Catalog", Icon: PersonaCatalogIcon, requiresCatalog: true }, - // Library (PLAN T203, SPEC F104.7) is deliberately NOT gated by `requiresCatalog` — unlike the - // Community Catalog browse surface, an installed pack keeps serving with the catalog disabled or - // unreachable (SPEC F104.8's offline floor), so the page that inspects what's ALREADY installed - // must stay reachable on that same axis too. - { href: "/library", label: "Library", Icon: LibraryIcon }, + // Wardrobe (PLAN T203, SPEC F104.7; renamed from "Library" at PLAN T204, Dean's ruling — nav label + // and route only, see this file's own class remarks) is deliberately NOT gated by + // `requiresCatalog` — unlike the Community Catalog browse surface, an installed pack keeps serving + // with the catalog disabled or unreachable (SPEC F104.8's offline floor), so the page that + // inspects what's ALREADY installed must stay reachable on that same axis too. + { href: "/wardrobe", label: "Wardrobe", Icon: WardrobeIcon }, { href: "/booth-log", label: "Booth log", Icon: BoothLogIcon }, { href: "/health", label: "Health", Icon: HealthIcon }, { href: "/settings", label: "Settings", Icon: SettingsIcon }, diff --git a/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx b/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx index 688085d3..9ab3e975 100644 --- a/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx +++ b/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx @@ -2,7 +2,7 @@ import type { ReactNode } from "react"; import { Button } from "@/components/ui/button"; -import { formatFontByteTotal } from "./font-format"; +import { formatFontByteTotal, licenceLine } from "./font-format"; import { prettifySlug } from "./format-slug"; import { SpecimenBlock } from "./SpecimenBlock"; import type { CatalogEntryDetailDto } from "./types"; @@ -10,6 +10,11 @@ import type { CatalogEntryDetailDto } from "./types"; export interface FontDetailPanelProps { slug: string; detail: CatalogEntryDetailDto; + /** Whether THIS slug already has an installed pack (PLAN T204, Dean's post-v3.1.0 review: + * reopening an installed pack's detail panel showed no sign it was already installed). Sourced + * from `GET /api/fonts`'s own listing — see `PersonaCatalogClient`'s own remarks for where that + * read happens and how a fresh install flips this without a reload. */ + isInstalled: boolean; onInstallClick: () => void; } @@ -35,16 +40,38 @@ export interface FontDetailPanelProps { * surface to install FROM. The T186 preview→confirm→POST precedent is the natural, minimal home, * so this panel's Install button opens `FontInstallModal` (confirm/cancel semantics mirrored from * `ThemeInstallModal`) rather than posting anything itself. + * + * Licence line (PLAN T204, Dean's post-v3.1.0 review). "<licence> · v<version> · + * <subset>" via the shared `licenceLine` helper (`font-format.ts`) — the SAME line the + * Wardrobe page's own installed-pack cards render, so the one trust fact a PRE-install review most + * needs (what licence am I about to agree to?) reads identically whether the pack is already + * installed or not. Degrades to "Licence unknown" rather than an empty line — see that helper's own + * remarks. + * + * Installed-state awareness (PLAN T204). Reopening an already-installed pack's detail panel + * used to show no sign of that — `SpecimenBlock`'s OLD "Admin-only specimen — not installed" caption + * read as a status claim it never was (it only ever described the SPECIMEN, an always-transient + * preview, never the pack itself), so that caption is now state-neutral in BOTH states (see + * `SpecimenBlock`'s own remarks) and the installed signal moved here instead. `isInstalled` (sourced + * by `PersonaCatalogClient` from `GET /api/fonts`, see its own remarks) drives an "Installed" chip — + * the same quiet bordered-pill treatment the Wardrobe page's own provenance chip uses — and the + * button's own label: "Re-install" when a pack under this slug is already installed + * (`FontPackController.Install` upserts, PLAN T199, so a re-install is a genuinely supported, + * non-destructive action), "Install" otherwise. */ -export function FontDetailPanel({ slug, detail, onInstallClick }: FontDetailPanelProps): ReactNode { +export function FontDetailPanel({ slug, detail, isInstalled, onInstallClick }: FontDetailPanelProps): ReactNode { return (
-

{prettifySlug(slug)}

- {/* Install (scope addition, see this component's own remarks) opens FontInstallModal's - confirm step — this click itself issues no request; the modal POSTs on confirm only. */} +
+

{prettifySlug(slug)}

+ {isInstalled && } +
+ {/* Install/Re-install (scope addition, see this component's own remarks) opens + FontInstallModal's confirm step — this click itself issues no request; the modal POSTs + on confirm only. */}
@@ -58,6 +85,10 @@ export function FontDetailPanel({ slug, detail, onInstallClick }: FontDetailPane

)} +

+ {licenceLine({ license: detail.fontLicense, version: detail.fontVersion, subset: detail.fontSubset })} +

+ {/* Plain text ONLY (mirrors DetailPanel's own persona-description rule, SPEC F90.6) — a bare `{detail.description}` JSX child, React's default escaping, never dangerouslySetInnerHTML. */} {detail.description !== null && detail.description !== "" && ( @@ -68,3 +99,17 @@ export function FontDetailPanel({ slug, detail, onInstallClick }: FontDetailPane
); } + +/** "Installed" chip (PLAN T204) — the SAME quiet bordered-pill treatment the Wardrobe page's own + * `ProvenanceChip` uses (`app/(authed)/wardrobe/WardrobeClient.tsx`), reused here as a plain status + * marker rather than a provenance stamp (no slug/date — this panel already names the slug in its own + * heading): a genuine shared component would need editing both files for a shape that already + * differs (provenance text vs a bare status word), the same reasoning that chip's own remarks give + * for not sharing with the persona/theme chips either. */ +function InstalledChip(): ReactNode { + return ( + + Installed + + ); +} diff --git a/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx b/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx index 6bf8b1fb..4007e60e 100644 --- a/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx +++ b/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx @@ -27,6 +27,17 @@ import type { interface PersonaCatalogClientProps { /** The index this page's server component already fetched (SPEC F90.2, F90.4). */ initialIndex: CatalogIndexResponseDto; + /** + * Slugs already installed, per `GET /api/fonts` (PLAN T204, Dean's post-v3.1.0 review: reopening + * an installed pack's detail panel showed no sign it was already installed). The page's own server + * component fetches this ALONGSIDE the index (the smaller diff over a lazy per-open fetch — one + * extra `Promise.all` leg server-side, versus threading a second client-side fetch/loading state + * through `loadDetail` for font entries only) and hands the slug list straight through; defaults to + * `[]` — fail closed, matching this file's own `catalogEnabled` default posture elsewhere in the + * app — so an isolated render with no live signal never CLAIMS a pack is installed that it has no + * evidence for. + */ + installedFontSlugs?: string[]; } type DetailState = @@ -68,12 +79,17 @@ type DetailState = * dedicated install-button task for M1, and T204's exit-check checklist has no other UI surface to * install a pack from. */ -export function PersonaCatalogClient({ initialIndex }: PersonaCatalogClientProps): ReactNode { +export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: PersonaCatalogClientProps): ReactNode { const router = useRouter(); const [detail, setDetail] = useState({ kind: "idle" }); const [reviewing, setReviewing] = useState(false); const [installingTheme, setInstallingTheme] = useState(false); const [installingFont, setInstallingFont] = useState(false); + // Seeded from the server-fetched prop above, then flipped locally the instant an install + // succeeds (handleFontInstalled below) — cheap, no reload/re-fetch needed for a set this small. + // `useState(() => ...)` (lazy initializer): this only needs to run once, not re-derive the Set on + // every render. + const [installedSlugs, setInstalledSlugs] = useState>(() => new Set(installedFontSlugs)); // Request token (T102 review, HIGH): loadDetail's fetch is not the only thing that can change // `detail` between when a request starts and when it resolves — the operator can also collapse @@ -171,11 +187,13 @@ export function PersonaCatalogClient({ initialIndex }: PersonaCatalogClientProps } /** SPEC F104.5's success path — mirrors `handleThemeInstalled`'s own remarks: no dedicated - * library-list page exists on THIS task's own owned files for the panel to route to (PLAN T203 - * builds that separately); closing the modal and toasting the family that just entered the - * station's library is the whole client-side job. */ - function handleFontInstalled(result: FontInstallResult): void { + * wardrobe-list page exists on THIS task's own owned files for the panel to route to (PLAN T203 + * builds that separately); closing the modal, toasting the family that just entered the station's + * Wardrobe, AND (PLAN T204) marking `slug` installed in local state — so `FontDetailPanel` flips + * to "Installed"/"Re-install" immediately, no reload — is the whole client-side job. */ + function handleFontInstalled(slug: string, result: FontInstallResult): void { setInstallingFont(false); + setInstalledSlugs((prev) => new Set(prev).add(slug)); toast.success(`"${result.family}" installed.`); } @@ -247,7 +265,12 @@ export function PersonaCatalogClient({ initialIndex }: PersonaCatalogClientProps return setReviewing(true)} />; case "font": return ( - setInstallingFont(true)} /> + setInstallingFont(true)} + /> ); default: return null; @@ -309,7 +332,11 @@ export function PersonaCatalogClient({ initialIndex }: PersonaCatalogClientProps the theme block above): FontInstallModal posts no body of its own, so it has nothing to read off `detail.detail.card` at all — only `selectedEntry?.kind === "font"` gates it. */} {installingFont && detail.kind === "loaded" && selectedEntry?.kind === "font" && ( - setInstallingFont(false)} onInstalled={handleFontInstalled} /> + setInstallingFont(false)} + onInstalled={(result) => handleFontInstalled(detail.slug, result)} + /> )} ); diff --git a/admin-ui/app/(authed)/persona-catalog/SpecimenBlock.tsx b/admin-ui/app/(authed)/persona-catalog/SpecimenBlock.tsx index 8d859897..1ed69ea3 100644 --- a/admin-ui/app/(authed)/persona-catalog/SpecimenBlock.tsx +++ b/admin-ui/app/(authed)/persona-catalog/SpecimenBlock.tsx @@ -158,7 +158,7 @@ export function SpecimenBlock({ slug, specimenFile }: SpecimenBlockProps): React AaBbCcDdEe 0123456789

- Admin-only specimen — not installed + Transient specimen — previewing installs nothing

); diff --git a/admin-ui/app/(authed)/persona-catalog/font-format.ts b/admin-ui/app/(authed)/persona-catalog/font-format.ts index 30122efb..7f069f74 100644 --- a/admin-ui/app/(authed)/persona-catalog/font-format.ts +++ b/admin-ui/app/(authed)/persona-catalog/font-format.ts @@ -12,3 +12,32 @@ export function formatFontByteTotal(bytes: number): string { if (bytes >= kib) return `${Math.round(bytes / kib)} KiB`; return `${bytes} B`; } + +/** The trio a font pack's licence line reads off (PLAN T204) — shared shape between the CATALOG + * detail wire (`CatalogEntryDetailDto.fontLicense`/`fontVersion`/`fontSubset`) and the installed + * Wardrobe wire (`FontLibraryPackDto.license`/`version`/`subset`): different DTOs, same three field + * names and the same "degrades gracefully, never blank" contract, so this shape only names what + * `licenceLine` below actually needs. */ +export interface FontLicenceFields { + license: string | null; + version: string | null; + subset: string | null; +} + +/** One pack's licence line — "<licence> · v<version> · <subset>" (PLAN T204, + * mirrors the Library/Wardrobe page's own pre-existing line) — omitting whichever of the three is + * absent (`version` is genuinely optional even on a cleanly-parsed manifest; `license`/`subset` are + * `null` only when the manifest failed to parse, degrade, or — on the catalog wire — the entry is + * unreachable). Degrades to "Licence unknown" rather than an empty line on the all-null edge — a + * reviewer never sees a blank fact where the panel's whole point is showing this one. Shared by + * `FontDetailPanel` (pre-install review) and the Wardrobe page's own pack cards so the two can never + * drift on the same wording. */ +export function licenceLine(fields: FontLicenceFields): string { + const parts = [ + fields.license, + fields.version !== null && fields.version !== "" ? `v${fields.version}` : null, + fields.subset, + ].filter((part): part is string => part !== null && part !== ""); + + return parts.length > 0 ? parts.join(" · ") : "Licence unknown"; +} diff --git a/admin-ui/app/(authed)/persona-catalog/page.tsx b/admin-ui/app/(authed)/persona-catalog/page.tsx index 3cf8f864..ff677baa 100644 --- a/admin-ui/app/(authed)/persona-catalog/page.tsx +++ b/admin-ui/app/(authed)/persona-catalog/page.tsx @@ -10,9 +10,44 @@ import type { CatalogIndexResponseDto } from "./types"; export const dynamic = "force-dynamic"; export const fetchCache = "force-no-store"; +/** Wire shape of a `GET /api/fonts` row (SPEC F104.7) — only the one field this page reads; mirrors + * `wardrobe/page.tsx`'s own local `SettingRow` idiom (cast to the one field a caller needs rather + * than importing the Wardrobe page's full `FontLibraryPackDto` for a single string). */ +interface InstalledFontPackRow { + slug: string; +} + +/** + * Every already-installed pack's slug (PLAN T204, Dean's post-v3.1.0 review: reopening an installed + * pack's detail panel showed no sign it was already installed) — fetched ALONGSIDE the index below, + * in the SAME server component, the smaller diff over a lazy per-open client fetch (the alternative + * this task's own dispatch note weighed): one more `Promise.all` leg here, versus a second + * client-side fetch/loading state threaded through `PersonaCatalogClient.loadDetail` for font + * entries only. Independent of `Community:CatalogIndexUrl` (SPEC F104.8's offline floor — an + * installed pack outlives the catalog, the same reasoning `wardrobe/page.tsx`'s own ungated nav item + * follows) — fetched unconditionally, never gated on the catalog being enabled. Any failure + * (network error, non-200, or an unexpected non-array body) degrades to `[]` — fail closed, matching + * `fetchCatalogEnabled`'s own posture below: no live signal means no pack gets FALSELY claimed + * installed. + */ +async function fetchInstalledFontSlugs(cookieHeader: string): Promise { + try { + const response = await apiGet("/api/fonts", { cookies: cookieHeader }); + if (!response.ok) return []; + const rows = (await response.json()) as InstalledFontPackRow[]; + return Array.isArray(rows) ? rows.map((row) => row.slug) : []; + } catch { + return []; + } +} + export default async function PersonaCatalogPage(): Promise { const cookieStore = await cookies(); - const response = await apiGet("/api/catalog/index", { cookies: cookieStore.toString() }); + const cookieHeader = cookieStore.toString(); + const [response, installedFontSlugs] = await Promise.all([ + apiGet("/api/catalog/index", { cookies: cookieHeader }), + fetchInstalledFontSlugs(cookieHeader), + ]); // Disabled (SPEC F90.1): CatalogController serves a bare, zero-byte 404 here — the same // per-resource 404 shape MediaDetailPage's own "Not found" branch renders inline for (house @@ -43,7 +78,7 @@ export default async function PersonaCatalogPage(): Promise {

Community Catalog

- +
); diff --git a/admin-ui/app/(authed)/persona-catalog/types.ts b/admin-ui/app/(authed)/persona-catalog/types.ts index 49c8a32c..9739e4c4 100644 --- a/admin-ui/app/(authed)/persona-catalog/types.ts +++ b/admin-ui/app/(authed)/persona-catalog/types.ts @@ -58,12 +58,16 @@ export interface CatalogIndexResponseDto { * reads the already-projected fields below, but `card` is exactly what `PersonaCardReviewModal` * (SPEC F90.5/F90.6, PLAN T103) both renders in full and POSTs byte-for-byte on confirm; `meta` * stays unused by this page. Every field but `unreachable` is `null` exactly when `unreachable` is - * `true`. `fontFamily`/`fontByteTotal`/`fontSpecimenFile` are `null` for every non-font entry - * (T202) — `fontFamily` here is parsed straight from `card` (the manifest, T194), a DETAIL-side - * sibling of `CatalogShelfEntryDto.fontFamily`'s own index-sourced field of the same name, not the - * same fetch; see `FontDetailPanel`'s own remarks for why this value is never interpolated into - * CSS. `fontSpecimenFile` is the bare filename `SpecimenBlock` passes to - * `GET /api/catalog/entries/{slug}/assets/{file}` to render the real face (SPEC F104.4). */ + * `true`. `fontFamily`/`fontByteTotal`/`fontSpecimenFile`/`fontLicense`/`fontVersion`/`fontSubset` + * are `null` for every non-font entry (T202, T204) — `fontFamily` here is parsed straight from + * `card` (the manifest, T194), a DETAIL-side sibling of `CatalogShelfEntryDto.fontFamily`'s own + * index-sourced field of the same name, not the same fetch; see `FontDetailPanel`'s own remarks for + * why this value is never interpolated into CSS. `fontSpecimenFile` is the bare filename + * `SpecimenBlock` passes to `GET /api/catalog/entries/{slug}/assets/{file}` to render the real face + * (SPEC F104.4). `fontLicense`/`fontVersion`/`fontSubset` (PLAN T204, Dean's post-v3.1.0 review: the + * pre-install review panel showed no licence anywhere) are the SAME manifest trio a Wardrobe pack's + * own `license`/`version`/`subset` carry once installed (`FontLibraryPackDto`) — see + * `font-format.ts`'s shared `licenceLine` for the one place both render identically. */ export interface CatalogEntryDetailDto { card: string | null; meta: string | null; @@ -77,4 +81,7 @@ export interface CatalogEntryDetailDto { fontFamily: string | null; fontByteTotal: number | null; fontSpecimenFile: string | null; + fontLicense: string | null; + fontVersion: string | null; + fontSubset: string | null; } diff --git a/admin-ui/app/(authed)/library/FontLibraryClient.tsx b/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx similarity index 72% rename from admin-ui/app/(authed)/library/FontLibraryClient.tsx rename to admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx index 2e0ea975..cd22cbf6 100644 --- a/admin-ui/app/(authed)/library/FontLibraryClient.tsx +++ b/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx @@ -3,10 +3,10 @@ import type { ReactNode } from "react"; import { EmptyState } from "@/components/ui/empty-state"; import { formatDateStamp } from "@/lib/format-clock"; -import { formatFontByteTotal } from "../persona-catalog/font-format"; +import { formatFontByteTotal, licenceLine } from "../persona-catalog/font-format"; import type { FontLibraryPackDto } from "./types"; -export interface FontLibraryClientProps { +export interface WardrobeClientProps { /** Every installed pack, from `GET /api/fonts` (SPEC F104.7). */ packs: FontLibraryPackDto[]; /** Test-only injection point for the provenance chip's `formatDateStamp` call; production omits @@ -15,7 +15,7 @@ export interface FontLibraryClientProps { timeZone?: string; /** * Whether the Community Catalog is currently enabled (SPEC F90.1, `Community:CatalogIndexUrl` - * non-empty) — swaps the empty-state CTA (PLAN T203 review finding F3). The Library nav item is + * non-empty) — swaps the empty-state CTA (PLAN T203 review finding F3). The Wardrobe nav item is * deliberately ungated (see nav-items.ts's own remarks: installed packs outlive the catalog, * F104.8), so a disabled catalog must never leave this page's own empty state pointing at * `/persona-catalog` — that page 404s off-catalog, the exact dead end the ungated nav exists to @@ -26,22 +26,6 @@ export interface FontLibraryClientProps { catalogEnabled?: boolean; } -/** One pack's licence line — "<licence> · v<version> · <subset>" — omitting - * whichever of the three the pack's stored `definition` carries no value for (`version` is - * genuinely optional even on a cleanly-parsed manifest; `license`/`subset` are `null` only if that - * manifest failed to re-parse server-side, see `FontLibraryPackDto`'s own remarks). Degrades to - * "Licence unknown" rather than an empty line on the all-null edge — this page never renders a - * blank fact where a reader would expect one. */ -function licenceLine(pack: FontLibraryPackDto): string { - const parts = [ - pack.license, - pack.version !== null && pack.version !== "" ? `v${pack.version}` : null, - pack.subset, - ].filter((part): part is string => part !== null && part !== ""); - - return parts.length > 0 ? parts.join(" · ") : "Licence unknown"; -} - /** Provenance chip — "Installed · <slug> · <date>" (SPEC F104.7 AC1, the db/25 pattern) * — mirrors `PersonasClient`'s own `ProvenanceBadge`/`SettingsForm`'s own `ThemeProvenanceBadge` * treatment (quiet bordered chip, T105/T187) rather than importing either: this page sits outside @@ -68,12 +52,15 @@ function ProvenanceChip({ } /** - * The Library page's client half (SPEC F104.7, STORY-284, PLAN T203) — every installed font pack, - * each rendered as its own card: family (title), faces with style + byte size (the shared - * `font-format.ts` helper, mirroring the shelf card's own T201 byte-total treatment), the licence - * line, and the "Installed · ⟨slug⟩ · ⟨date⟩" provenance chip (AC1). Read-only — this page lists - * what T199's install route already wrote; it issues no requests of its own. On an empty library, - * `catalogEnabled` picks the empty-state CTA (T203 review finding F3) — see that prop's own remarks. + * The Wardrobe page's client half (SPEC F104.7, STORY-284, PLAN T203; renamed "Library" → "Wardrobe" + * at PLAN T204, Dean's ruling — nav label and route only, see nav-items.ts's own remarks; the wire + * (`GET /api/fonts`, `FontLibraryPackDto`) keeps its name, backend DTOs don't chase UI labels) — + * every installed font pack, each rendered as its own card: family (title), faces with style + byte + * size (the shared `font-format.ts` helper, mirroring the shelf card's own T201 byte-total + * treatment), the licence line, and the "Installed · ⟨slug⟩ · ⟨date⟩" provenance chip (AC1). + * Read-only — this page lists what T199's install route already wrote; it issues no requests of its + * own. On an empty wardrobe, `catalogEnabled` picks the empty-state CTA (T203 review finding F3) — + * see that prop's own remarks. * * PLAIN TEXT ONLY (the T199/T200 stored-family/style obligation, closed here). * `pack.family` and each face's `style` are unbounded free-form prose (see `FontLibraryPackDto`'s @@ -81,7 +68,7 @@ function ProvenanceChip({ * never `dangerouslySetInnerHTML` and never interpolated into an inline `style` attribute or any * other CSS context anywhere on this page. */ -export function FontLibraryClient({ packs, timeZone, catalogEnabled = false }: FontLibraryClientProps): ReactNode { +export function WardrobeClient({ packs, timeZone, catalogEnabled = false }: WardrobeClientProps): ReactNode { if (packs.length === 0) { return catalogEnabled ? ( { } } -export default async function LibraryPage(): Promise { +export default async function WardrobePage(): Promise { const cookieStore = await cookies(); const cookieHeader = cookieStore.toString(); const [response, catalogEnabled] = await Promise.all([ @@ -55,8 +55,8 @@ export default async function LibraryPage(): Promise { if (!response.ok) { return (
-

Library

-

Unable to load the installed font library.

+

Wardrobe

+

Unable to load the installed font wardrobe.

); } @@ -65,9 +65,9 @@ export default async function LibraryPage(): Promise { return (
-

Library

+

Wardrobe

- +
); diff --git a/admin-ui/app/(authed)/library/types.ts b/admin-ui/app/(authed)/wardrobe/types.ts similarity index 93% rename from admin-ui/app/(authed)/library/types.ts rename to admin-ui/app/(authed)/wardrobe/types.ts index b5a3c90d..deaa697e 100644 --- a/admin-ui/app/(authed)/library/types.ts +++ b/admin-ui/app/(authed)/wardrobe/types.ts @@ -16,7 +16,7 @@ export interface FontLibraryFaceDto { * * `family`/`FontLibraryFaceDto.style` are UNBOUNDED free-form prose (the T199/T200 stored-family/ * style obligation) — this page renders both as plain text ONLY, never interpolated into a - * stylesheet or inline `style` attribute (see `FontLibraryClient`'s own remarks). */ + * stylesheet or inline `style` attribute (see `WardrobeClient`'s own remarks). */ export interface FontLibraryPackDto { slug: string; family: string; diff --git a/src/GenWave.Host/Api/CatalogController.cs b/src/GenWave.Host/Api/CatalogController.cs index 60c36f78..0d039216 100644 --- a/src/GenWave.Host/Api/CatalogController.cs +++ b/src/GenWave.Host/Api/CatalogController.cs @@ -129,7 +129,7 @@ public async Task Entry(string slug, CancellationToken ct) CatalogEntryFetchResult.Ok ok => Ok(ToEntryResponse(ok)), CatalogEntryFetchResult.NotFound => NotFound(UnknownEntryProblem(slug)), CatalogEntryFetchResult.Unreachable => Ok(new CatalogEntryResponse( - null, null, null, Unreachable: true, null, null, null, null, null, null, null, null, null)), + null, null, null, Unreachable: true, null, null, null, null, null, null, null, null, null, null, null, null)), CatalogEntryFetchResult.HashMismatch => StatusCode(StatusCodes.Status502BadGateway, WithheldProblem("failed its integrity check")), CatalogEntryFetchResult.Oversize => @@ -297,7 +297,10 @@ static CatalogEntryResponse ToEntryResponse(CatalogEntryFetchResult.Ok ok) meta.SamplePatter ?? [], FontFamily: fontManifest?.Family, FontByteTotal: isFont ? ok.Content.Assets.Sum(a => a.Bytes) : null, - FontSpecimenFile: ResolveSpecimenFile(fontManifest, ok.Content.Assets)); + FontSpecimenFile: ResolveSpecimenFile(fontManifest, ok.Content.Assets), + FontLicense: fontManifest?.License, + FontVersion: fontManifest?.Version, + FontSubset: fontManifest?.Subset); } /// diff --git a/src/GenWave.Host/Api/CatalogEntryResponse.cs b/src/GenWave.Host/Api/CatalogEntryResponse.cs index 313032ee..e2ecc165 100644 --- a/src/GenWave.Host/Api/CatalogEntryResponse.cs +++ b/src/GenWave.Host/Api/CatalogEntryResponse.cs @@ -65,6 +65,19 @@ namespace GenWave.Host.Api; /// straight to that route to render the transient specimen preview. for every /// non-font entry, when unreachable, or when no upright face resolves. /// +/// +/// A font entry's licence identifier (PLAN T204, Dean's post-v3.1.0 review: the pre-install review +/// panel showed no licence at all) — parsed off the SAME hash-verified manifest +/// already reads, via the same +/// call, zero extra cost. Paired with / to mirror the +/// admin UI's Wardrobe page's own "licence · version · subset" line (FontLibraryPackDto — the +/// wire DTO keeps its pre-rename name; only the UI label/route became "Wardrobe") so the SAME trust +/// fact reads identically whether an operator is reviewing a pack pre-install or inspecting one +/// already installed. for every non-font entry, when unreachable, or when the +/// manifest fails to parse (degrades — never a 500, see that method's own remarks). +/// +/// A font entry's manifest version (PLAN T204) — genuinely optional even on a cleanly-parsed manifest ('s own shape); for every non-font entry, when unreachable, or when absent/unparseable. +/// A font entry's manifest subset, e.g. "latin" (PLAN T204) — for every non-font entry, when unreachable, or when the manifest fails to parse. public sealed record CatalogEntryResponse( string? Card, string? Meta, @@ -78,4 +91,7 @@ public sealed record CatalogEntryResponse( IReadOnlyList? SamplePatter, string? FontFamily, long? FontByteTotal, - string? FontSpecimenFile); + string? FontSpecimenFile, + string? FontLicense, + string? FontVersion, + string? FontSubset); diff --git a/tests/GenWave.Host.Tests/Specs/Story279_FontKindAssets.cs b/tests/GenWave.Host.Tests/Specs/Story279_FontKindAssets.cs index bade17d7..f6dc6969 100644 --- a/tests/GenWave.Host.Tests/Specs/Story279_FontKindAssets.cs +++ b/tests/GenWave.Host.Tests/Specs/Story279_FontKindAssets.cs @@ -580,6 +580,24 @@ public async Task TheDetailRouteResolvesTheSpecimenFileToTheDeclaredAsset() Assert.Equal(FontShelfFixtures.AssetFile, body!.FontSpecimenFile); } + [Fact] + public async Task TheDetailRouteProjectsTheLicenceVersionAndSubsetTrio() + { + // PLAN T204 (Dean's post-v3.1.0 review): the pre-install review panel showed no licence + // at all — FontLicense/FontVersion/FontSubset are parsed off the SAME hash-verified + // manifest FontFamily already reads (golden.font.json: "OFL-1.1"/"2.000"/"text"), so this + // trust fact reaches the panel at zero extra fetch cost. + await using var factory = new FontShelfWebFactory(); + var client = await FontShelfWebFactory.LoggedInClientAsync(factory); + + var response = await client.GetAsync($"/api/catalog/entries/{FontShelfFixtures.FontSlug}"); + + var body = await response.Content.ReadFromJsonAsync(); + Assert.Equal("OFL-1.1", body!.FontLicense); + Assert.Equal("2.000", body.FontVersion); + Assert.Equal("text", body.FontSubset); + } + [Fact] public async Task TheShelfRouteProjectsTheSameByteTotalAsTheDetailRoute() { From 6dbb8eb86e7976600a0dc3b6c74b183fd9e8ca12 Mon Sep 17 00:00:00 2001 From: GenWave Radio Date: Thu, 6 Aug 2026 09:45:20 -0600 Subject: [PATCH 2/2] fix(ui): theme catalog detail tells the installed truth (gh-#375; v3.1.1 theme half) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors the font half (d82f870) for themes: the detail panel shows an Installed chip + 'Imported · · ' provenance (catalog slug or 'file'; NULL provenance — the future authored case — renders nothing, never 'Imported · null'), the button reads Re-install when installed, and a successful install flips state locally (2xx only; failed installs flip nothing — spec-pinned on BOTH kinds, closing the font-half N3 gap). Installed source = the existing /api/settings Station:Theme choices (importedFrom/importedAt already ride that wire — no new route), projected to three fields before the RSC payload, fail-closed to [] with the documented degrade direction. Inherited obligations closed: CatalogEntryResponse.UnreachableCatalog() factory kills the 16-null literal (CS0102 forced the name); shared extracted and adopted at all five duplicated sites (PersonasClient's ml-2 equivalence verified). ThemeImportResponse carries ImportedAt via a store READ-BACK after upsert — never a fabricated UtcNow; the spec pins response == stored row == settings choice, the whole gh-#375 contract in one walk. --- .../font-pack-shelf-specimen.spec.tsx | 24 +++++ .../__specs__/persona-catalog-page.spec.tsx | 69 ++++++++++++++ .../theme-catalog-preview-install.spec.tsx | 91 ++++++++++++++++++- .../persona-catalog/FontDetailPanel.tsx | 22 ++--- .../persona-catalog/PersonaCatalogClient.tsx | 84 +++++++++++++++-- .../persona-catalog/ThemeInstallModal.tsx | 12 ++- .../app/(authed)/persona-catalog/page.tsx | 70 +++++++++++++- .../app/(authed)/persona-catalog/types.ts | 22 +++++ .../app/(authed)/personas/PersonasClient.tsx | 16 ++-- .../app/(authed)/settings/SettingsForm.tsx | 25 ++--- .../app/(authed)/wardrobe/WardrobeClient.tsx | 24 +++-- admin-ui/components/ui/chip.tsx | 37 ++++++++ src/GenWave.Host/Api/CatalogController.cs | 3 +- src/GenWave.Host/Api/CatalogEntryResponse.cs | 38 +++++++- src/GenWave.Host/Api/ThemeImportResponse.cs | 12 ++- .../Api/ThemesImportController.cs | 14 ++- .../Specs/Story272_ThemeImport.cs | 54 +++++++++++ 17 files changed, 545 insertions(+), 72 deletions(-) create mode 100644 admin-ui/components/ui/chip.tsx diff --git a/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx b/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx index cb190e93..eee0721a 100644 --- a/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx +++ b/admin-ui/__specs__/font-pack-shelf-specimen.spec.tsx @@ -561,4 +561,28 @@ describe("Feature: packs on the shelf with an honest specimen", () => { expect(fetchMock.mock.calls.some(([url]) => String(url) === INSTALL_URL)).toBe(false); }); }); + + describe("Scenario: a failed install flips nothing (gh-#375 review carry-forward, N3)", () => { + it("flips nothing locally — the detail panel behind the dialog still reads Install, not Installed", async () => { + const fetchMock = fontFlowFetchMock({ + install: makeJsonResponse(409, { detail: "This pack is already installed under a different family." }), + }); + await openInstallDialog(fetchMock); + + const dialog = within(screen.getByRole("dialog")); + await act(async () => { + fireEvent.click(dialog.getByRole("button", { name: "Confirm install" })); + await Promise.resolve(); + }); + await screen.findByRole("alert"); + + // `onInstalled` (PersonaCatalogClient.handleFontInstalled) only ever fires on + // FontInstallModal's own resp.ok branch — a 409 never reaches it, so the detail panel's own + // Install button, still present behind the open dialog, never flips to Re-install. + // `getByText`, not `getByRole` (Radix marks the background `aria-hidden` while the dialog is + // open, which `*ByRole` correctly excludes but a plain text query does not). + expect(screen.getByText("Install")).toBeInTheDocument(); + expect(screen.queryByText("Installed")).not.toBeInTheDocument(); + }); + }); }); diff --git a/admin-ui/__specs__/persona-catalog-page.spec.tsx b/admin-ui/__specs__/persona-catalog-page.spec.tsx index 73c2a652..a87105d0 100644 --- a/admin-ui/__specs__/persona-catalog-page.spec.tsx +++ b/admin-ui/__specs__/persona-catalog-page.spec.tsx @@ -545,6 +545,75 @@ describe("Feature: The Persona Catalog server page", () => { }); }); + describe("Scenario: installed-theme-provenance wiring (gh-#375 — the theme half of PLAN T204's font-half fix)", () => { + const indexBody: CatalogIndexResponseDto = { + entries: [EVERYONE_ENTRY], + fetchedAt: "2026-07-26T00:00:00Z", + unreachable: false, + }; + + /** A `GET /api/settings` body carrying `Station:Theme`'s own choices (SPEC F103.11, PLAN + * T187) — only the fields `fetchInstalledThemeProvenance` reads. */ + const settingsBody = [ + { + key: "Station:Theme", + value: "cats-whisker", + source: "default", + applyMode: "live", + kind: "choice", + unit: "", + choices: [ + { value: "cats-whisker", label: "Cat's Whisker", isDefault: true }, + { + value: "midnight-drive", + label: "Midnight Drive", + importedFrom: "midnight-drive-catalog-entry", + importedAt: "2026-07-21T09:05:00Z", + }, + { value: "aurora-glow", label: "Aurora Glow", importedFrom: "file", importedAt: "2026-07-20T14:32:00Z" }, + ], + }, + ]; + + it("threads Station:Theme's own imported choices into PersonaCatalogClient as installedThemeProvenance", async () => { + global.fetch = jest.fn().mockImplementation(async (input) => { + const url = String(input); + if (url.endsWith("/api/catalog/index")) return makeJsonResponse(200, indexBody); + if (url.endsWith("/api/fonts")) return makeJsonResponse(200, []); + if (url.endsWith("/api/settings")) return makeJsonResponse(200, settingsBody); + throw new Error(`unexpected fetch ${url}`); + }) as unknown as typeof fetch; + + const { default: PersonaCatalogPage } = await import("../app/(authed)/persona-catalog/page"); + const node = await PersonaCatalogPage(); + + const clientEl = findElementByType(node, PersonaCatalogClient); + // Every choice carrying provenance rides through — a shipped default (no importedFrom) does + // not, mirroring the theme-provenance-badge.spec.tsx precedent's own "shipped default never + // gets a row" rule one layer up. + expect(clientEl?.props["installedThemeProvenance"]).toEqual([ + { slug: "midnight-drive", importedFrom: "midnight-drive-catalog-entry", importedAt: "2026-07-21T09:05:00Z" }, + { slug: "aurora-glow", importedFrom: "file", importedAt: "2026-07-20T14:32:00Z" }, + ]); + }); + + it("degrades to an empty list — never crashing the page — when GET /api/settings fails", async () => { + global.fetch = jest.fn().mockImplementation(async (input) => { + const url = String(input); + if (url.endsWith("/api/catalog/index")) return makeJsonResponse(200, indexBody); + if (url.endsWith("/api/fonts")) return makeJsonResponse(200, []); + if (url.endsWith("/api/settings")) return makeJsonResponse(500, {}); + throw new Error(`unexpected fetch ${url}`); + }) as unknown as typeof fetch; + + const { default: PersonaCatalogPage } = await import("../app/(authed)/persona-catalog/page"); + const node = await PersonaCatalogPage(); + + const clientEl = findElementByType(node, PersonaCatalogClient); + expect(clientEl?.props["installedThemeProvenance"]).toEqual([]); + }); + }); + describe("Scenario: disabled surface is a bare 404 (SPEC F90.1, sad path)", () => { it("renders an inline 'Not found' page when GET /api/catalog/index 404s", async () => { global.fetch = jest diff --git a/admin-ui/__specs__/theme-catalog-preview-install.spec.tsx b/admin-ui/__specs__/theme-catalog-preview-install.spec.tsx index 8f29a051..d28a3beb 100644 --- a/admin-ui/__specs__/theme-catalog-preview-install.spec.tsx +++ b/admin-ui/__specs__/theme-catalog-preview-install.spec.tsx @@ -136,7 +136,12 @@ function themeFlowFetchMock(overrides: { if (url === IMPORT_URL) { return ( overrides.importResponse ?? - makeJsonResponse(200, { slug: "golden-frequency", name: "Golden Frequency", importedFrom: "golden-frequency" }) + makeJsonResponse(200, { + slug: "golden-frequency", + name: "Golden Frequency", + importedFrom: "golden-frequency", + importedAt: "2026-08-06T00:00:00Z", + }) ); } throw new Error(`unexpected fetch ${url}`); @@ -151,12 +156,22 @@ function cardFor(name: string): HTMLElement { return card; } -/** Opens Golden Frequency's detail panel and waits for the live composed preview to render. */ -async function openGoldenFrequencyPreview(fetchMock: jest.MockedFunction): Promise { +/** Opens Golden Frequency's detail panel and waits for the live composed preview to render. + * `installedThemeProvenance` (gh-#375) defaults to `[]`, the same "not installed" default + * `PersonaCatalogClient`'s own prop carries — pass a row naming this entry's own slug to exercise + * the already-installed path. */ +async function openGoldenFrequencyPreview( + fetchMock: jest.MockedFunction, + installedThemeProvenance: { slug: string; importedFrom: string; importedAt: string }[] = [] +): Promise { global.fetch = fetchMock; render( <> - + ); @@ -299,6 +314,52 @@ describe("Feature: previewing and installing a catalog theme", () => { await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); expect(await screen.findByText('"Golden Frequency" installed.')).toBeInTheDocument(); }); + + it("flips the detail panel to Installed/Re-install with the real provenance locally, no reload (gh-#375)", async () => { + const fetchMock = themeFlowFetchMock(); + // Starts NOT installed — the default `installedThemeProvenance=[]` — so the button starts + // "Install" and no provenance line renders yet. + await openInstallDialog(fetchMock); + + const dialog = within(screen.getByRole("dialog")); + await act(async () => { + fireEvent.click(dialog.getByRole("button", { name: "Confirm install" })); + await Promise.resolve(); + }); + await waitFor(() => expect(screen.queryByRole("dialog")).not.toBeInTheDocument()); + + // The detail panel itself (still open — only the confirm dialog closed) now reads installed, + // with the REAL provenance the import response carried (never a fabricated "just now") and + // no second fetch: PersonaCatalogClient.handleThemeInstalled flips its own local state on the + // toast, the same cheap path the font half's own T204 spec calls for. + expect(screen.getByText("Installed")).toBeInTheDocument(); + expect(screen.getByText("Imported · golden-frequency · Aug 6, 2026")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Re-install" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Install" })).not.toBeInTheDocument(); + }); + }); + + describe("Scenario: installed-state awareness (gh-#375 — the theme half of Dean's demo feedback)", () => { + it("shows Install and no provenance line when the theme is not installed", async () => { + const fetchMock = themeFlowFetchMock(); + await openGoldenFrequencyPreview(fetchMock); + + expect(screen.getByRole("button", { name: "Install" })).toBeInTheDocument(); + expect(screen.queryByText("Installed")).not.toBeInTheDocument(); + expect(screen.queryByText(/^Imported ·/)).not.toBeInTheDocument(); + }); + + it('shows an Installed chip, "Imported · · ", and Re-install when the theme is already installed', async () => { + const fetchMock = themeFlowFetchMock(); + await openGoldenFrequencyPreview(fetchMock, [ + { slug: "golden-frequency", importedFrom: "golden-frequency", importedAt: "2026-07-21T09:05:00Z" }, + ]); + + expect(screen.getByText("Installed")).toBeInTheDocument(); + expect(screen.getByText("Imported · golden-frequency · Jul 21, 2026")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Re-install" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Install" })).not.toBeInTheDocument(); + }); }); // ── SAD PATH ──────────────────────────────────────────────────────────── @@ -362,5 +423,27 @@ describe("Feature: previewing and installing a catalog theme", () => { // The dialog stays open — a failed confirm is not a crash, and the operator can still cancel. expect(screen.getByRole("dialog")).toBeInTheDocument(); }); + + it("flips nothing locally — the detail panel behind the dialog still reads Install, not Installed (gh-#375)", async () => { + const fetchMock = themeFlowFetchMock({ + importResponse: makeJsonResponse(409, { detail: '"golden-frequency" is a shipped theme\'s slug and cannot be overwritten by an import (SPEC F103.8).' }), + }); + await openInstallDialog(fetchMock); + + const dialog = within(screen.getByRole("dialog")); + await act(async () => { + fireEvent.click(dialog.getByRole("button", { name: "Confirm install" })); + await Promise.resolve(); + }); + await screen.findByRole("alert"); + + // `onInstalled` (PersonaCatalogClient.handleThemeInstalled) only ever fires on + // ThemeInstallModal's own 2xx branch — a 409 never reaches it, so the detail panel's own + // Install button, still present behind the open dialog, never flips to Re-install. + // `getByText`, not `getByRole` (Radix marks the background `aria-hidden` while the dialog is + // open, which `*ByRole` correctly excludes but a plain text query does not). + expect(screen.getByText("Install")).toBeInTheDocument(); + expect(screen.queryByText("Installed")).not.toBeInTheDocument(); + }); }); }); diff --git a/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx b/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx index 9ab3e975..c159b896 100644 --- a/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx +++ b/admin-ui/app/(authed)/persona-catalog/FontDetailPanel.tsx @@ -2,6 +2,7 @@ import type { ReactNode } from "react"; import { Button } from "@/components/ui/button"; +import { Chip } from "@/components/ui/chip"; import { formatFontByteTotal, licenceLine } from "./font-format"; import { prettifySlug } from "./format-slug"; import { SpecimenBlock } from "./SpecimenBlock"; @@ -54,7 +55,7 @@ export interface FontDetailPanelProps { * preview, never the pack itself), so that caption is now state-neutral in BOTH states (see * `SpecimenBlock`'s own remarks) and the installed signal moved here instead. `isInstalled` (sourced * by `PersonaCatalogClient` from `GET /api/fonts`, see its own remarks) drives an "Installed" chip — - * the same quiet bordered-pill treatment the Wardrobe page's own provenance chip uses — and the + * the shared `Chip` component (`components/ui/chip.tsx`, gh-#375 extraction) — and the * button's own label: "Re-install" when a pack under this slug is already installed * (`FontPackController.Install` upserts, PLAN T199, so a re-install is a genuinely supported, * non-destructive action), "Install" otherwise. @@ -65,7 +66,10 @@ export function FontDetailPanel({ slug, detail, isInstalled, onInstallClick }: F

{prettifySlug(slug)}

- {isInstalled && } + {/* Bare status word, not a provenance stamp (no slug/date — this panel already names the + slug in its own heading) — mirrors WardrobeClient's own ProvenanceChip shape one level + up, just narrower content. */} + {isInstalled && Installed}
{/* Install/Re-install (scope addition, see this component's own remarks) opens FontInstallModal's confirm step — this click itself issues no request; the modal POSTs @@ -99,17 +103,3 @@ export function FontDetailPanel({ slug, detail, isInstalled, onInstallClick }: F
); } - -/** "Installed" chip (PLAN T204) — the SAME quiet bordered-pill treatment the Wardrobe page's own - * `ProvenanceChip` uses (`app/(authed)/wardrobe/WardrobeClient.tsx`), reused here as a plain status - * marker rather than a provenance stamp (no slug/date — this panel already names the slug in its own - * heading): a genuine shared component would need editing both files for a shape that already - * differs (provenance text vs a bare status word), the same reasoning that chip's own remarks give - * for not sharing with the persona/theme chips either. */ -function InstalledChip(): ReactNode { - return ( - - Installed - - ); -} diff --git a/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx b/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx index 4007e60e..d4392e56 100644 --- a/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx +++ b/admin-ui/app/(authed)/persona-catalog/PersonaCatalogClient.tsx @@ -3,9 +3,11 @@ import { useRouter } from "next/navigation"; import { useRef, useState, type ReactNode } from "react"; import { Button } from "@/components/ui/button"; +import { Chip } from "@/components/ui/chip"; import { EmptyState } from "@/components/ui/empty-state"; import { Skeleton } from "@/components/ui/skeleton"; import { toast } from "@/components/ui/toast"; +import { formatDateStamp } from "@/lib/format-clock"; import { readErrorMessage } from "@/lib/problem-details"; import { cn } from "@/lib/utils"; import { PersonaCardReviewModal, type PersonaCardReviewImportResult } from "../_components/PersonaCardReviewModal"; @@ -22,6 +24,7 @@ import type { CatalogShelfEntryDto, CatalogThemePreview, CatalogThemeSwatchSet, + ThemeCatalogProvenanceDto, } from "./types"; interface PersonaCatalogClientProps { @@ -38,6 +41,19 @@ interface PersonaCatalogClientProps { * evidence for. */ installedFontSlugs?: string[]; + /** + * Every catalog-imported theme's provenance, per `GET /api/settings`'s own `Station:Theme` choices + * (gh-#375 — the theme half of the same reopening-shows-no-installed-state complaint the + * font half above already closed). Mirrors `installedFontSlugs`'s own shape and defaults ([]) — + * fail closed, so an isolated render with no live signal never CLAIMS a theme is installed that it + * has no evidence for — see `ThemeCatalogProvenanceDto`'s own remarks for why this rides + * `/api/settings` rather than a new backend route. + */ + installedThemeProvenance?: ThemeCatalogProvenanceDto[]; + /** Test-only injection point for the theme provenance line's `formatDateStamp` call (gh-#375); + * production omits this and gets the browser's local zone — the same SettingsForm/WardrobeClient/ + * PersonasClient idiom, not a bespoke one. */ + timeZone?: string; } type DetailState = @@ -79,7 +95,12 @@ type DetailState = * dedicated install-button task for M1, and T204's exit-check checklist has no other UI surface to * install a pack from. */ -export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: PersonaCatalogClientProps): ReactNode { +export function PersonaCatalogClient({ + initialIndex, + installedFontSlugs = [], + installedThemeProvenance = [], + timeZone, +}: PersonaCatalogClientProps): ReactNode { const router = useRouter(); const [detail, setDetail] = useState({ kind: "idle" }); const [reviewing, setReviewing] = useState(false); @@ -90,6 +111,12 @@ export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: // `useState(() => ...)` (lazy initializer): this only needs to run once, not re-derive the Set on // every render. const [installedSlugs, setInstalledSlugs] = useState>(() => new Set(installedFontSlugs)); + // Same lazy-initializer/local-flip shape as `installedSlugs` above, keyed by slug — a Map, not a + // Set, because the theme detail panel's provenance line needs the WHOLE row + // (importedFrom/importedAt), not just a boolean. + const [installedThemes, setInstalledThemes] = useState>( + () => new Map(installedThemeProvenance.map((provenance) => [provenance.slug, provenance])) + ); // Request token (T102 review, HIGH): loadDetail's fetch is not the only thing that can change // `detail` between when a request starts and when it resolves — the operator can also collapse @@ -179,10 +206,19 @@ export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: /** SPEC F103.6's success path: no `/themes` list page exists to land on (unlike Personas' own * `router.push` above) — `Station:Theme`'s choice list widening is a server-side fact the next - * `GET /api/settings` read already reflects (PLAN T183/T184), nothing this component needs to - * fetch or thread. Closing the modal and toasting is the whole client-side job. */ - function handleThemeInstalled(result: ThemeInstallResult): void { + * `GET /api/settings` read already reflects (PLAN T183/T184). Closing the modal, toasting, AND + * (gh-#375 — mirrors `handleFontInstalled`'s own local flip) marking `slug` installed + * in local state — so `ThemeDetailPanel` flips to "Installed"/"Re-install" with the real + * provenance line immediately, no reload — is the whole client-side job. A failed install never + * reaches this function at all (`ThemeInstallModal` only calls `onInstalled` on its own 2xx + * branch), so a rejected confirm flips nothing here, same as the font half. */ + function handleThemeInstalled(slug: string, result: ThemeInstallResult): void { setInstallingTheme(false); + setInstalledThemes((prev) => { + const next = new Map(prev); + next.set(slug, { slug, importedFrom: result.importedFrom, importedAt: result.importedAt }); + return next; + }); toast.success(`"${result.name}" installed.`); } @@ -258,6 +294,8 @@ export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: setInstallingTheme(true)} /> ); @@ -322,7 +360,7 @@ export function PersonaCatalogClient({ initialIndex, installedFontSlugs = [] }: slug={detail.slug} manifestText={detail.detail.card} onCancel={() => setInstallingTheme(false)} - onInstalled={handleThemeInstalled} + onInstalled={(result) => handleThemeInstalled(detail.slug, result)} /> )} @@ -360,26 +398,54 @@ function detailSectionAriaLabel(kind: CatalogEntryKind | undefined): string { } } +/** + * A theme entry's detail panel (SPEC F103.5, F103.6, PLAN T186; installed-state awareness gh-#375 + * — the theme half of Dean's demo feedback, mirroring `FontDetailPanel`'s own + * `isInstalled`/Re-install treatment). `provenance` is `null` for a theme with no `station.theme` + * row under this catalog slug (never installed, or the shipped default it happens to share a slug + * with — see `ThemeCatalogProvenanceDto`'s own remarks); non-null drives the SAME "Installed" chip + * `FontDetailPanel` uses (the shared `Chip` component) plus an "Imported · ⟨source⟩ · ⟨date⟩" + * provenance line — the T187 copy verbatim, minus the leading label `SettingsForm`'s own + * `ThemeProvenanceBadge` folds in (this panel already names the theme in its own heading, exactly + * the same reasoning `FontDetailPanel`'s own bare-word chip gives) — and the Install→Re-install + * button label. `importedFrom` renders VERBATIM, same provenance rule every other chip in this + * codebase follows. + */ function ThemeDetailPanel({ slug, manifestText, + provenance, + timeZone, onInstallClick, }: { slug: string; manifestText: string; + provenance: ThemeCatalogProvenanceDto | null; + timeZone?: string; onInstallClick: () => void; }): ReactNode { return (
-

{prettifySlug(slug)}

- {/* Install (SPEC F103.6) opens ThemeInstallModal's confirm/cancel step — this click itself - issues no request; the modal POSTs the SAME manifestText already reviewed here. */} +
+

{prettifySlug(slug)}

+ {provenance !== null && Installed} +
+ {/* Install/Re-install (SPEC F103.6; label gh-#375) opens ThemeInstallModal's + confirm/cancel step — this click itself issues no request; the modal POSTs the SAME + manifestText already reviewed here. Re-install is a genuinely supported, non-destructive + action — ThemesImportController.Import upserts by slug (SPEC F103.7). */}
+ {provenance !== null && ( +

+ {`Imported · ${provenance.importedFrom} · ${formatDateStamp(provenance.importedAt, { timeZone })}`} +

+ )} +
); diff --git a/admin-ui/app/(authed)/persona-catalog/ThemeInstallModal.tsx b/admin-ui/app/(authed)/persona-catalog/ThemeInstallModal.tsx index 7700ce85..484baefa 100644 --- a/admin-ui/app/(authed)/persona-catalog/ThemeInstallModal.tsx +++ b/admin-ui/app/(authed)/persona-catalog/ThemeInstallModal.tsx @@ -8,12 +8,22 @@ import { prettifySlug } from "./format-slug"; export interface ThemeInstallResult { name: string; + /** The provenance stamp the import route actually wrote (SPEC F103.11) — always this modal's own + * `slug` in practice (see `ThemeCatalogProvenanceDto`'s own remarks), read off the response + * rather than assumed, mirroring `ThemeImportSuccessBody`'s own already-present field. */ + importedFrom: string; + /** When {@link importedFrom} was stamped (gh-#375) — a server read-back + * (`ThemesImportController`'s own remarks), never a client-side `Date.now()` guess, so + * `PersonaCatalogClient`'s post-install local flip can show the SAME provenance line a fresh + * `GET /api/settings` read would. */ + importedAt: string; } interface ThemeImportSuccessBody { slug: string; name: string; importedFrom: string; + importedAt: string; } export interface ThemeInstallModalProps { @@ -67,7 +77,7 @@ export function ThemeInstallModal({ slug, manifestText, onCancel, onInstalled }: if (resp.ok) { const body = (await resp.json()) as ThemeImportSuccessBody; - onInstalled({ name: body.name }); + onInstalled({ name: body.name, importedFrom: body.importedFrom, importedAt: body.importedAt }); return; } diff --git a/admin-ui/app/(authed)/persona-catalog/page.tsx b/admin-ui/app/(authed)/persona-catalog/page.tsx index ff677baa..3d108e56 100644 --- a/admin-ui/app/(authed)/persona-catalog/page.tsx +++ b/admin-ui/app/(authed)/persona-catalog/page.tsx @@ -2,7 +2,7 @@ import type { ReactNode } from "react"; import { cookies } from "next/headers"; import { apiGet } from "@/lib/api"; import { PersonaCatalogClient } from "./PersonaCatalogClient"; -import type { CatalogIndexResponseDto } from "./types"; +import type { CatalogIndexResponseDto, ThemeCatalogProvenanceDto } from "./types"; // The underlying Community:CatalogIndexUrl setting is live-editable from Settings (SPEC F90.1), // and the shelf itself can flip disabled<->enabled or gain/lose entries between visits — always @@ -41,12 +41,72 @@ async function fetchInstalledFontSlugs(cookieHeader: string): Promise } } +/** Wire shape of one `Station:Theme` choice, off `GET /api/settings` (SPEC F103.11, PLAN T187) — + * only the fields this page reads; mirrors `InstalledFontPackRow`'s own narrow-cast idiom above + * rather than importing `settings/settings-types.ts`'s full `SettingChoice` for three fields. */ +interface StationThemeChoiceRow { + value: string; + importedFrom?: string | null; + importedAt?: string | null; +} + +/** Wire shape of one `GET /api/settings` row — only the one field this page reads (mirrors + * `wardrobe/page.tsx`'s own local `SettingRow` idiom for the SAME endpoint, a different key). */ +interface SettingRow { + key: string; + choices?: StationThemeChoiceRow[]; +} + +const STATION_THEME_KEY = "Station:Theme"; + +/** + * Every catalog-imported theme's provenance (gh-#375, Dean's demo feedback — the theme + * half of the font half's own T204 "reopening an installed pack shows no installed state" fix). + * + * Route choice (this task's own dispatch note): `GET /api/settings`, not a new `GET /api/themes` + * route. `Station:Theme`'s choices already widen to shipped ∪ owner themes with + * `importedFrom`/`importedAt` per choice (SPEC F103.7/F103.11, PLAN T183/T187, + * `StationSettingsAllowlist.ThemeChoices`/`SettingChoice`) — the exact data this page needs already + * rides an existing, generic endpoint, so a dedicated `GET /api/themes` listing (the font half's own + * `/api/fonts` shape) would duplicate that projection for zero new capability. The one thing this + * costs over a dedicated route: this page reads the WHOLE settings document (every allowlisted key, + * not only `Station:Theme`) to reach one field. That document is small (one row per allowlisted + * key, no large payloads — SPEC F55.3's full-coverage allowlist is still a few dozen rows) and + * already fetched wholesale by other authed pages for a single key each (`wardrobe/page.tsx`'s own + * `fetchCatalogEnabled`, `layout.tsx`'s own `Station:Theme` read for the header's ThemeSwitcher) — + * this is that same established shape, not a new pattern. + * + * Fetched ALONGSIDE the index and `installedFontSlugs`, in the SAME server component (one more + * `Promise.all` leg, the smaller diff over a lazy per-open client fetch). Any failure (network + * error, non-200, an unexpected shape) degrades to `[]` — fail closed, matching + * `fetchInstalledFontSlugs`'s own posture above: no live signal means no theme gets FALSELY claimed + * installed. + */ +async function fetchInstalledThemeProvenance(cookieHeader: string): Promise { + try { + const response = await apiGet("/api/settings", { cookies: cookieHeader }); + if (!response.ok) return []; + const settings = (await response.json()) as SettingRow[]; + const themeSetting = settings.find((row) => row.key === STATION_THEME_KEY); + const choices = themeSetting?.choices ?? []; + return choices + .filter( + (choice): choice is StationThemeChoiceRow & { importedFrom: string; importedAt: string } => + choice.importedFrom != null && choice.importedAt != null + ) + .map((choice) => ({ slug: choice.value, importedFrom: choice.importedFrom, importedAt: choice.importedAt })); + } catch { + return []; + } +} + export default async function PersonaCatalogPage(): Promise { const cookieStore = await cookies(); const cookieHeader = cookieStore.toString(); - const [response, installedFontSlugs] = await Promise.all([ + const [response, installedFontSlugs, installedThemeProvenance] = await Promise.all([ apiGet("/api/catalog/index", { cookies: cookieHeader }), fetchInstalledFontSlugs(cookieHeader), + fetchInstalledThemeProvenance(cookieHeader), ]); // Disabled (SPEC F90.1): CatalogController serves a bare, zero-byte 404 here — the same @@ -78,7 +138,11 @@ export default async function PersonaCatalogPage(): Promise {

Community Catalog

- +
); diff --git a/admin-ui/app/(authed)/persona-catalog/types.ts b/admin-ui/app/(authed)/persona-catalog/types.ts index 9739e4c4..7012c0ba 100644 --- a/admin-ui/app/(authed)/persona-catalog/types.ts +++ b/admin-ui/app/(authed)/persona-catalog/types.ts @@ -85,3 +85,25 @@ export interface CatalogEntryDetailDto { fontVersion: string | null; fontSubset: string | null; } + +/** + * One catalog-imported theme's provenance (gh-#375, Dean's demo feedback — the theme half + * of the v3.1.1 polish, mirroring the font half's `installedFontSlugs` — see + * `PersonaCatalogClient`'s own remarks). Sourced from `GET /api/settings`'s own `Station:Theme` + * choices (SPEC F103.11, PLAN T187's `SettingChoice.importedFrom`/`importedAt`), never a new + * backend route: `persona-catalog/page.tsx` extracts every choice carrying provenance and hands the + * list straight through — the smaller diff over adding a dedicated `GET /api/themes` listing (this + * task's own dispatch weighed both; see that file's own remarks for the full reasoning). + * + * `slug` is the catalog entry's own slug, kept as its own field distinct from `importedFrom` even + * though the two are always equal today (`ThemeInstallModal` always installs a theme under its own + * catalog slug, threading that SAME value as both the import route's target slug and its + * `?catalogSlug=`) — a caller keyed on `slug` never has to assume that equality holds, the same + * "read the real field, don't infer it" discipline `WardrobeClient`'s own `ProvenanceChip` remarks + * state for its own always-equal `importedFrom`/`slug` pair. + */ +export interface ThemeCatalogProvenanceDto { + slug: string; + importedFrom: string; + importedAt: string; +} diff --git a/admin-ui/app/(authed)/personas/PersonasClient.tsx b/admin-ui/app/(authed)/personas/PersonasClient.tsx index 7111dea7..7b18c81b 100644 --- a/admin-ui/app/(authed)/personas/PersonasClient.tsx +++ b/admin-ui/app/(authed)/personas/PersonasClient.tsx @@ -2,6 +2,7 @@ import { Fragment, useRef, useState, type FormEvent, type ReactNode } from "react"; import { Button } from "@/components/ui/button"; +import { Chip } from "@/components/ui/chip"; import { EmptyState } from "@/components/ui/empty-state"; import { toast } from "@/components/ui/toast"; import { formatDateStamp } from "@/lib/format-clock"; @@ -107,7 +108,14 @@ function displayVoice(voice: string): string { * `lib/format-clock.ts`'s bare-calendar-date formatter — NOT `formatUpSince` (that one folds its * own `HH:MM · Mon D` pair, no year, into the string, which would both break the badge's literal * three-field shape and silently collide two imports a year apart). `timeZone` is a plain - * pass-through from the page prop, the house test-injection idiom. */ + * pass-through from the page prop, the house test-injection idiom. + * + * The shared `Chip` component (`components/ui/chip.tsx`, gh-#375 extraction — adjudicated + * compatible: this badge's own className differed from the other four sites only by `ml-2` in + * place of `w-fit`, and `inline-flex` already sizes to its content with no explicit width utility + * either way, so adding `Chip`'s own `w-fit` here changes nothing rendered) with an `ml-2` override + * — this badge sits inline immediately after the persona's name text (unlike every other chip + * site's standalone placement), so it alone needs the extra left margin. */ function ProvenanceBadge({ importedFrom, importedAt, @@ -117,11 +125,7 @@ function ProvenanceBadge({ importedAt: string; timeZone?: string; }): ReactNode { - return ( - - {`Hired · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`} - - ); + return {`Hired · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`}; } /** On The Air badge (SPEC F94.1, STORY-246, PLAN T127) — the roster's replacement for the retired diff --git a/admin-ui/app/(authed)/settings/SettingsForm.tsx b/admin-ui/app/(authed)/settings/SettingsForm.tsx index f51dba38..a90de3fa 100644 --- a/admin-ui/app/(authed)/settings/SettingsForm.tsx +++ b/admin-ui/app/(authed)/settings/SettingsForm.tsx @@ -9,6 +9,7 @@ import { type ReactNode, } from "react"; import { Button } from "@/components/ui/button"; +import { Chip } from "@/components/ui/chip"; import { useConfirm } from "@/components/ui/confirm-dialog"; import { toast } from "@/components/ui/toast"; import { formatDateStamp } from "@/lib/format-clock"; @@ -1161,9 +1162,9 @@ function ThemeProvenanceList({ * * Kept as its own small component in this file rather than merged with `PersonasClient`'s * `ProvenanceBadge` into one shared component (PLAN T187 review F1's "cheap, do it" note): the - * shapes differ (this one folds the label into the same chip; persona's leaves the name outside - * it) and a genuine merge would mean editing `PersonasClient.tsx` too, which sits outside this - * task's file partition — noted rather than done silently. + * TEXT shapes differ (this one folds the label into the same chip; persona's leaves the name + * outside it) — only the visual chip styling was ever duplicated, and `Chip` + * (`components/ui/chip.tsx`, gh-#375 extraction) now owns that once, shared by both. */ function ThemeProvenanceBadge({ label, @@ -1176,23 +1177,17 @@ function ThemeProvenanceBadge({ importedAt: string; timeZone?: string; }): ReactNode { - return ( - - {`${label} — Imported · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`} - - ); + return {`${label} — Imported · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`}; } -/** 3px-radius bordered chip for the source tag, per design-aesthetic chip conventions. */ +/** 3px-radius bordered chip for the source tag, per design-aesthetic chip conventions — the shared + * `Chip` component, with `aria-label`/`data-source` passed straight through via its own `...props` + * spread. */ function SourceChip({ source }: { source: SettingDto["source"] }): ReactNode { return ( - + [{sourceLabel(source)}] - + ); } diff --git a/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx b/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx index cd22cbf6..0cfdf3d3 100644 --- a/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx +++ b/admin-ui/app/(authed)/wardrobe/WardrobeClient.tsx @@ -1,6 +1,7 @@ "use client"; import type { ReactNode } from "react"; +import { Chip } from "@/components/ui/chip"; import { EmptyState } from "@/components/ui/empty-state"; import { formatDateStamp } from "@/lib/format-clock"; import { formatFontByteTotal, licenceLine } from "../persona-catalog/font-format"; @@ -27,14 +28,15 @@ export interface WardrobeClientProps { } /** Provenance chip — "Installed · <slug> · <date>" (SPEC F104.7 AC1, the db/25 pattern) - * — mirrors `PersonasClient`'s own `ProvenanceBadge`/`SettingsForm`'s own `ThemeProvenanceBadge` - * treatment (quiet bordered chip, T105/T187) rather than importing either: this page sits outside - * both files' own partitions, and the shape here ("Installed", no leading label) differs from both - * ("Hired"/"<label> — Imported") enough that a genuine shared component would need editing - * either file anyway. `importedFrom` renders VERBATIM — this is provenance, not decoration, same - * rule the persona/theme chips already follow — even though it is always equal to the pack's own - * `slug` today (a pack has no authored-in-place path); reading it off its own field rather than - * `pack.slug` keeps this chip honest about which column IS the provenance stamp. */ + * — the shared `Chip` component (`components/ui/chip.tsx`, gh-#375 extraction) rather than + * `PersonasClient`'s own `ProvenanceBadge`/`SettingsForm`'s own `ThemeProvenanceBadge` directly: + * this page sits outside both files' own partitions, and the TEXT shape here ("Installed", no + * leading label) differs from both ("Hired"/"<label> — Imported") — only the visual chip + * styling itself was ever duplicated, which `Chip` now owns once. `importedFrom` renders VERBATIM — + * this is provenance, not decoration, same rule the persona/theme chips already follow — even + * though it is always equal to the pack's own `slug` today (a pack has no authored-in-place path); + * reading it off its own field rather than `pack.slug` keeps this chip honest about which column IS + * the provenance stamp. */ function ProvenanceChip({ importedFrom, importedAt, @@ -44,11 +46,7 @@ function ProvenanceChip({ importedAt: string; timeZone?: string; }): ReactNode { - return ( - - {`Installed · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`} - - ); + return {`Installed · ${importedFrom} · ${formatDateStamp(importedAt, { timeZone })}`}; } /** diff --git a/admin-ui/components/ui/chip.tsx b/admin-ui/components/ui/chip.tsx new file mode 100644 index 00000000..58c8d5b9 --- /dev/null +++ b/admin-ui/components/ui/chip.tsx @@ -0,0 +1,37 @@ +import type { HTMLAttributes, ReactNode } from "react"; +import { cn } from "@/lib/utils"; + +export interface ChipProps extends HTMLAttributes { + children?: ReactNode; +} + +/** + * The quiet bordered-pill chip (design-aesthetic skill) — 3px radius, `--line` border, `--mute` + * text — used across the admin UI for a status word or a provenance stamp: source tags + * (`SettingsForm`'s `SourceChip`), imported-theme provenance (`SettingsForm`'s + * `ThemeProvenanceBadge`, `PersonaCatalogClient`'s theme detail panel), imported-persona provenance + * (`PersonasClient`'s `ProvenanceBadge`), an installed font pack's provenance + * (`WardrobeClient`'s `ProvenanceChip`), and a catalog font pack's bare "Installed" status + * (`FontDetailPanel`). Every one of those five pre-existing sites carried its OWN copy of the same + * className string (gh-#375 review carry-forward, N4) — this is the one extraction, children and + * an optional `className` override are the only thing that ever varied. `className` merges via + * `cn()` (the `Button`/`EmptyState` precedent) rather than replacing the base styling outright, so a + * caller can ADD a layout concern (e.g. `PersonasClient`'s own `ml-2` — it sits inline right after a + * name, unlike every other site's standalone placement) without repeating the visual treatment + * itself. Every other native `` attribute (`aria-label`, `data-source`, `data-testid`, …) + * passes straight through via `...props`, the same `Button` idiom — `SourceChip`'s own + * `aria-label`/`data-source` pair needs both. + */ +export function Chip({ children, className, ...props }: ChipProps): ReactNode { + return ( + + {children} + + ); +} diff --git a/src/GenWave.Host/Api/CatalogController.cs b/src/GenWave.Host/Api/CatalogController.cs index 0d039216..80b7dc63 100644 --- a/src/GenWave.Host/Api/CatalogController.cs +++ b/src/GenWave.Host/Api/CatalogController.cs @@ -128,8 +128,7 @@ public async Task Entry(string slug, CancellationToken ct) { CatalogEntryFetchResult.Ok ok => Ok(ToEntryResponse(ok)), CatalogEntryFetchResult.NotFound => NotFound(UnknownEntryProblem(slug)), - CatalogEntryFetchResult.Unreachable => Ok(new CatalogEntryResponse( - null, null, null, Unreachable: true, null, null, null, null, null, null, null, null, null, null, null, null)), + CatalogEntryFetchResult.Unreachable => Ok(CatalogEntryResponse.UnreachableCatalog()), CatalogEntryFetchResult.HashMismatch => StatusCode(StatusCodes.Status502BadGateway, WithheldProblem("failed its integrity check")), CatalogEntryFetchResult.Oversize => diff --git a/src/GenWave.Host/Api/CatalogEntryResponse.cs b/src/GenWave.Host/Api/CatalogEntryResponse.cs index e2ecc165..dee111d5 100644 --- a/src/GenWave.Host/Api/CatalogEntryResponse.cs +++ b/src/GenWave.Host/Api/CatalogEntryResponse.cs @@ -94,4 +94,40 @@ public sealed record CatalogEntryResponse( string? FontSpecimenFile, string? FontLicense, string? FontVersion, - string? FontSubset); + string? FontSubset) +{ + /// + /// The graceful "catalog currently unreachable" shape (SPEC F90.4, = + /// , every other field ) — a named factory (gh-#375, + /// inherited from the font half's own review, finding N1) replacing the 15-positional-null literal + /// used to construct inline. That literal predates this record's own by-name discipline (PLAN + /// T204's already names every argument it + /// passes) and had grown a silent trap: every widening of this record (three fields added at + /// T204 alone) meant one more null slotted into that same literal with nothing to catch a + /// miscount or a misordered pair of same-typed fields (two adjacent ? + /// positions swap silently). A one-time factory means the NEXT widening only ever touches this + /// method's own body, never a call site that has to be found and recounted. Named + /// UnreachableCatalog, not the record's own property name — C# + /// forbids a method and a property sharing one identifier on the same type (CS0102), so the + /// dispatch's literal CatalogEntryResponse.Unreachable() naming could not compile as + /// written; this is the closest unambiguous name that still reads as "the unreachable shape" at + /// the call site. + /// + public static CatalogEntryResponse UnreachableCatalog() => new( + Card: null, + Meta: null, + FetchedAt: null, + Unreachable: true, + Kind: null, + Audience: null, + BestFor: null, + Author: null, + Description: null, + SamplePatter: null, + FontFamily: null, + FontByteTotal: null, + FontSpecimenFile: null, + FontLicense: null, + FontVersion: null, + FontSubset: null); +} diff --git a/src/GenWave.Host/Api/ThemeImportResponse.cs b/src/GenWave.Host/Api/ThemeImportResponse.cs index dfa757a6..275d720a 100644 --- a/src/GenWave.Host/Api/ThemeImportResponse.cs +++ b/src/GenWave.Host/Api/ThemeImportResponse.cs @@ -15,4 +15,14 @@ namespace GenWave.Host.Api; /// The manifest's own display name. /// The provenance stamp actually written: the catalogSlug query /// value, or "file" for a direct upload (SPEC F103.6/F103.11). -public sealed record ThemeImportResponse(string Slug, string Name, string ImportedFrom); +/// +/// The moment was stamped (gh-#375, Dean's gh-#375 demo feedback — the +/// theme half of the v3.1.1 polish, mirroring the font half's own FontPackInstallResponse +/// shape) — read back from straight after the upsert +/// commits, the same value GET /api/settings's own +/// Station:Theme choices will report from the next read, never a client-approximated +/// guess: the admin UI's catalog detail panel flips to "Installed" +/// entirely from THIS response (no second fetch, PersonaCatalogClient's own local-flip precedent), +/// so a fabricated timestamp here would be a lie the very next settings read could contradict. +/// +public sealed record ThemeImportResponse(string Slug, string Name, string ImportedFrom, DateTime ImportedAt); diff --git a/src/GenWave.Host/Api/ThemesImportController.cs b/src/GenWave.Host/Api/ThemesImportController.cs index 3fcaf81f..3463ef5f 100644 --- a/src/GenWave.Host/Api/ThemesImportController.cs +++ b/src/GenWave.Host/Api/ThemesImportController.cs @@ -240,7 +240,19 @@ public async Task Import(string slug, [FromQuery] string? catalog LogSafeText.Sanitize(slug), LogSafeText.Sanitize(importedFrom)); - return Ok(new ThemeImportResponse(normalized.Slug, normalized.Name, importedFrom)); + // gh-#375 (gh-#375, ThemeImportResponse's own remarks) — a read-back, not a client-side + // DateTime.UtcNow guess: the store just stamped imported_at unconditionally (both the + // insert and the update ON CONFLICT branch, see IThemeStore.UpsertAsync's own remarks) with + // an importedFrom that is NEVER null on this path, so OwnerTheme's own "ImportedAt is null + // exactly when ImportedFrom is" invariant guarantees a value here — a null read-back would + // mean the write this request just awaited never actually committed, worth surfacing loudly + // rather than papering over with a fabricated timestamp the admin UI would show as fact. + var stored = await themeStore.GetBySlugAsync(slug, ct) + ?? throw new InvalidOperationException($"Theme '{slug}' was upserted but could not be read back."); + var importedAt = stored.ImportedAt + ?? throw new InvalidOperationException($"Theme '{slug}' was imported but carries no imported_at stamp."); + + return Ok(new ThemeImportResponse(normalized.Slug, normalized.Name, importedFrom, importedAt)); } // ── Helpers ────────────────────────────────────────────────────────────── diff --git a/tests/GenWave.Host.Tests/Specs/Story272_ThemeImport.cs b/tests/GenWave.Host.Tests/Specs/Story272_ThemeImport.cs index ac476558..32d63d75 100644 --- a/tests/GenWave.Host.Tests/Specs/Story272_ThemeImport.cs +++ b/tests/GenWave.Host.Tests/Specs/Story272_ThemeImport.cs @@ -353,6 +353,60 @@ public async Task TheImportedSlugIsListedAndAcceptedAsStationTheme() } } + public sealed class ScenarioTheResponseCarriesImportedAt + { + [Fact] + public async Task TheImportResponseImportedAtMatchesTheStoredRow() + { + // Given a freshly imported catalog theme (gh-#375: the admin UI's catalog + // detail panel flips to "Installed" straight off THIS response, no second fetch — see + // ThemeImportResponse's own remarks — so it needs a real, store-sourced imported_at, + // never a client-side DateTime.UtcNow approximation), + var themeStore = new FakeThemeStore(); + await using var factory = new ThemeImportWebFactory(themeStore); + var client = await LoggedInClientAsync(factory); + + // When it is imported, + var response = await PostManifestAsync( + client, "midnight-drive", ThemeImportFixture.ValidManifestJson("midnight-drive"), + catalogSlug: "midnight-drive-catalog-entry"); + var body = await response.Content.ReadFromJsonAsync(); + + // Then the response's own ImportedAt is the SAME value the store actually persisted — + // a read-back, not a guess. + Assert.True(response.IsSuccessStatusCode, await response.Content.ReadAsStringAsync()); + var stored = await themeStore.GetBySlugAsync("midnight-drive", CancellationToken.None); + Assert.Equal(stored?.ImportedAt, body?.ImportedAt); + Assert.NotEqual(default, body?.ImportedAt); + } + + [Fact] + public async Task TheSettingsSurfaceReportsTheSameImportedAtForTheChoice() + { + // Given a freshly imported catalog theme, + var settingsStore = new FakeThemeSettingsStore(); + await using var factory = new ThemeImportWebFactory(settingsStore: settingsStore); + var client = await LoggedInClientAsync(factory); + var importResponse = await PostManifestAsync( + client, "aurora-glow", ThemeImportFixture.ValidManifestJson("aurora-glow"), + catalogSlug: "aurora-glow-catalog-entry"); + var importBody = await importResponse.Content.ReadFromJsonAsync(); + Assert.True(importResponse.IsSuccessStatusCode, await importResponse.Content.ReadAsStringAsync()); + + // When Station:Theme's own choices are read back — the SAME seam gh-#375's own admin-ui + // catalog page reads to derive its installed-provenance list, no new backend route, + var getResponse = await client.GetAsync("/api/settings"); + var settings = await getResponse.Content.ReadFromJsonAsync>(); + var themeSetting = settings!.Single(s => s.Key.Equals("Station:Theme", StringComparison.OrdinalIgnoreCase)); + var choice = themeSetting.Choices!.Single(c => c.Value == "aurora-glow"); + + // Then the choice's own importedFrom/importedAt agree with the import response — the + // two reads can never silently disagree about when this theme was imported. + Assert.Equal("aurora-glow-catalog-entry", choice.ImportedFrom); + Assert.Equal(importBody?.ImportedAt, choice.ImportedAt); + } + } + // ── SAD PATH ──────────────────────────────────────────────────────────── public sealed class ScenarioRejectingBadImports