From 0f2b3399a00277fca4b34b421f9190f2d63d0059 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:25:46 +0000 Subject: [PATCH 01/10] fix: dedupe entities by declared id on the index-driven loader path loadEntitiesFromDirectory deduped file names via dedupeIds but returned the loaded entities without dedupeEntitiesById, so two distinct index entries whose files declare the same inner id produced duplicate card ids: duplicate React keys, linked flips, cross-card edit bleed, and a reachable throw in CardGrid's selection guard. Route the directory path's return through the same dedupe the single-file paths use, and correct the dedupeEntitiesById docblock, which claimed dedupeIds already gave the index path this guarantee (it only dedupes fetch paths). Regression test observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/loaders/collectionLoader.ts | 10 +++++++--- tests/loaders/collectionLoader.test.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/loaders/collectionLoader.ts b/src/loaders/collectionLoader.ts index f6995fe..29043e8 100644 --- a/src/loaders/collectionLoader.ts +++ b/src/loaders/collectionLoader.ts @@ -106,8 +106,10 @@ function dedupeIds(ids: string[]): string[] { /** * Remove entities repeating an id already seen, keeping the first. * - * The single-file array formats bypass the index path, so they need the same - * uniqueness guarantee as dedupeIds gives the index. + * dedupeIds only removes repeated fetch paths, so it cannot stop two distinct + * index entries (or single-file array rows) from declaring the same id inside. + * Every path that produces entities needs this final check to hold the + * uniqueness the render layer depends on. */ function dedupeEntitiesById(entities: Entity[]): Entity[] { if (entities.length < 2) { @@ -432,7 +434,9 @@ async function loadEntitiesFromDirectory( const workerCount = Math.min(ENTITY_FETCH_CONCURRENCY, ids.length); await Promise.all(Array.from({ length: workerCount }, () => worker())); - return results.filter((entity): entity is Entity => entity !== null); + return dedupeEntitiesById( + results.filter((entity): entity is Entity => entity !== null) + ); } /** diff --git a/tests/loaders/collectionLoader.test.ts b/tests/loaders/collectionLoader.test.ts index 9e667f9..ad8087a 100644 --- a/tests/loaders/collectionLoader.test.ts +++ b/tests/loaders/collectionLoader.test.ts @@ -344,6 +344,22 @@ describe("loadEntities duplicate id handling", () => { expect(entityFetches).toHaveLength(1); }); + it("keeps one entity per id when two index files declare the same id", async () => { + // Deduping the index's file names is not enough: distinct file names can + // each declare the same id inside, so the collision survives to render. + const base = "/data/collections/demo"; + stubFetch({ + [`${base}/adverts/index.json`]: ["a", "b"], + [`${base}/adverts/a.json`]: { id: "dup", title: "First" }, + [`${base}/adverts/b.json`]: { id: "dup", title: "Impostor" }, + }); + + const entities = await loadEntities(base, "advert"); + + expect(entities.map((e) => e.id)).toEqual(["dup"]); + expect(entities[0]?.title).toBe("First"); + }); + it("keeps one entity per id when a single array file repeats one", async () => { const base = "/data/collections/demo"; stubFetch({ From 044bfc7e60a04703fa0308abd6b3646b40190d64 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:28:18 +0000 Subject: [PATCH 02/10] fix: coerce untrusted myVerdict so EditForm save cannot silently no-op Entity schemas are loose and unknown keys are copied verbatim onto the display card, so a collection shipping a non-string myVerdict reached the edit form through an unchecked cast: the textarea showed '[object Object]' and save validation failed on a field whose error is never rendered, leaving the Save button silently dead. Coerce the incoming value (string passes, number becomes its string form, anything else is treated as absent) and render the missing summary/verdict error spans so a future validation failure is always visible. Reproduction tests observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/components/EditForm/EditForm.tsx | 31 +++++++++- tests/components/EditForm.test.tsx | 87 ++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 tests/components/EditForm.test.tsx diff --git a/src/components/EditForm/EditForm.tsx b/src/components/EditForm/EditForm.tsx index f125316..b677c0b 100644 --- a/src/components/EditForm/EditForm.tsx +++ b/src/components/EditForm/EditForm.tsx @@ -55,6 +55,21 @@ function getContextEditId(contextId: string): string { return `context:${contextId}`; } +/** + * Coerce an untrusted entity field to an editable string. + * + * The v2 entity schema is `.loose()`, so text fields may hold arbitrary JSON. + * An object left as-is renders as "[object Object]" in the textarea and then + * fails `cardFieldsSchema` on Save, leaving the modal open with no explanation. + * Mirror the collection display coercion: keep strings, stringify numbers, + * drop everything else. + */ +function toEditableString(value: unknown): string | undefined { + if (typeof value === "string") return value; + if (typeof value === "number") return String(value); + return undefined; +} + /** * Edit form modal for modifying entity fields. */ @@ -82,7 +97,7 @@ export function EditForm({ card, onClose }: EditFormProps) { const editSummary = existingCardEdit?.fields.summary as string | undefined; const editVerdict = existingCardEdit?.fields.myVerdict as string | undefined; const cardSummary = card.summary; - const cardVerdict = card.myVerdict as string | undefined; + const cardVerdict = toEditableString(card.myVerdict); return { title: editTitle ?? card.title, @@ -331,7 +346,14 @@ export function EditForm({ card, onClose }: EditFormProps) { }} placeholder="Brief description..." rows={3} + aria-invalid={!!cardErrors.summary} + aria-describedby={cardErrors.summary ? "edit-card-summary-error" : undefined} /> + {cardErrors.summary && ( + + {cardErrors.summary} + + )} {/* My Verdict */} @@ -348,7 +370,14 @@ export function EditForm({ card, onClose }: EditFormProps) { }} placeholder="Your personal opinion..." rows={3} + aria-invalid={!!cardErrors.myVerdict} + aria-describedby={cardErrors.myVerdict ? "edit-card-verdict-error" : undefined} /> + {cardErrors.myVerdict && ( + + {cardErrors.myVerdict} + + )} ) : ( diff --git a/tests/components/EditForm.test.tsx b/tests/components/EditForm.test.tsx new file mode 100644 index 0000000..4ac3fd0 --- /dev/null +++ b/tests/components/EditForm.test.tsx @@ -0,0 +1,87 @@ +/** + * Tests for EditForm: untrusted `myVerdict` values. + * + * Entity schemas are `.loose()`, so a collection can ship a non-string + * `myVerdict`. The form must coerce it the same way the collection display + * layer does, otherwise the value fails local validation on Save and the + * modal appears dead. + */ + +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import { EditForm } from "@/components/EditForm/EditForm"; +import { useEditsStore } from "@/stores/editsStore"; +import type { DisplayCard } from "@/hooks/useCollection"; + +function makeCard(overrides: Partial = {}): DisplayCard { + return { + id: "card-1", + title: "Test Card", + imageUrl: "https://example.com/image.jpg", + imageUrls: ["https://example.com/image.jpg"], + order: null, + ...overrides, + } as DisplayCard; +} + +/** The "My Verdict" textarea. */ +function getVerdictField(): HTMLTextAreaElement { + return screen.getByLabelText("My Verdict") as HTMLTextAreaElement; +} + +describe("EditForm untrusted myVerdict", () => { + beforeEach(() => { + useEditsStore.setState({ edits: {} }); + }); + + it("saves and closes when the source myVerdict is an object", () => { + const onClose = vi.fn(); + + render( + + ); + + // The object must never reach the textarea as "[object Object]" + expect(getVerdictField().value).toBe(""); + + fireEvent.click(screen.getByText("Save Changes")); + + expect(onClose).toHaveBeenCalled(); + }); + + it("round-trips a numeric myVerdict as its string form", () => { + const onClose = vi.fn(); + + render( + + ); + + expect(getVerdictField().value).toBe("42"); + + fireEvent.click(screen.getByText("Save Changes")); + + expect(onClose).toHaveBeenCalled(); + expect(useEditsStore.getState().edits["card-1"]?.fields.myVerdict).toBe( + "42" + ); + }); + + it("shows an error message when the verdict fails validation", () => { + const onClose = vi.fn(); + + render(); + + // Title is required, so an empty title surfaces its error span + fireEvent.change(screen.getByLabelText(/Title/), { target: { value: "" } }); + fireEvent.click(screen.getByText("Save Changes")); + + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByText("Title is required")).toBeInTheDocument(); + }); +}); From ba840a03079cac7a73ae3372c368b5aaa7a9fe30 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:28:52 +0000 Subject: [PATCH 03/10] fix: honour the Competing auto-advance setting in RoundResultOverlay The Auto-Advance Rounds toggle was stored and surfaced in settings but never read: RoundResultOverlay unconditionally scheduled the next round two seconds after the result appeared, so turning the setting off changed nothing. Gate only the timer on the setting; click and keypress dismissal stay unconditional, so disabling auto-advance cannot soft-lock the game. Regression test observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/mechanics/competing/components.tsx | 7 +- tests/mechanics/competing/components.test.tsx | 160 ++++++++++++++++++ 2 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 tests/mechanics/competing/components.test.tsx diff --git a/src/mechanics/competing/components.tsx b/src/mechanics/competing/components.tsx index 64712b8..379411e 100644 --- a/src/mechanics/competing/components.tsx +++ b/src/mechanics/competing/components.tsx @@ -292,6 +292,7 @@ function RoundResultOverlay() { const phase = useCompetingStore((s) => s.phase); const numericFields = useCompetingStore((s) => s.numericFields); const nextRound = useCompetingStore((s) => s.nextRound); + const autoAdvance = useCompetingStore((s) => s.autoAdvance); const handleDismiss = useCallback(() => { if (phase === "round_end") { @@ -299,11 +300,13 @@ function RoundResultOverlay() { } }, [phase, nextRound]); + // Only the timer is gated by the setting; manual dismissal (click or key + // press) always works, so the overlay can never soft-lock the game. useEffect(() => { - if (phase !== "round_end") return; + if (phase !== "round_end" || !autoAdvance) return; const timer = setTimeout(() => { handleDismiss(); }, 2000); return () => { clearTimeout(timer); }; - }, [phase, handleDismiss]); + }, [phase, handleDismiss, autoAdvance]); useEffect(() => { if (phase !== "round_end") return; diff --git a/tests/mechanics/competing/components.test.tsx b/tests/mechanics/competing/components.test.tsx new file mode 100644 index 0000000..1e1fc29 --- /dev/null +++ b/tests/mechanics/competing/components.test.tsx @@ -0,0 +1,160 @@ +/** + * Tests for Competing mechanic components. + * + * Covers the round result overlay's advance behaviour: the "Auto-Advance + * Rounds" setting must gate the 2 second timer, while manual dismissal + * (click or key press) must keep working regardless of the setting. + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { + render, + screen, + fireEvent, + act, + cleanup, +} from "@testing-library/react"; +import { CompetingGridOverlay } from "@/mechanics/competing/components"; +import { useCompetingStore } from "@/mechanics/competing/store"; +import type { NumericFieldInfo } from "@/mechanics/competing/types"; + +// Mechanic components read collection data from context; the round result +// overlay does not use it, so a minimal stub keeps the test focused. +vi.mock("@/context/CollectionDataContext", () => ({ + useCollectionData: () => ({ + cards: [], + isLoading: false, + error: null, + }), +})); + +// Exit/play-again handlers require a MechanicProvider; stub the context hook. +vi.mock("@/mechanics/context", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useMechanicContext: () => ({ + deactivateMechanic: () => undefined, + openMechanicPanel: () => undefined, + }), + }; +}); + +const sampleFields: NumericFieldInfo[] = [ + { key: "attack", label: "Attack", min: 50, max: 100, higherIsBetter: true }, + { key: "defence", label: "Defence", min: 30, max: 90, higherIsBetter: true }, +]; + +const sampleCardData = { + card1: { id: "card1", title: "Hero A", attack: 80, defence: 60 }, + card2: { id: "card2", title: "Hero B", attack: 70, defence: 75 }, + card3: { id: "card3", title: "Hero C", attack: 90, defence: 50 }, + card4: { id: "card4", title: "Hero D", attack: 65, defence: 85 }, +}; + +/** + * Seed the store at the end of a round the player has won, ready for the + * round result overlay to advance. + */ +function seedRoundEnd(autoAdvance: boolean): void { + useCompetingStore.setState({ + isActive: true, + phase: "round_end", + difficulty: "medium", + roundLimit: 0, + showCpuThinking: true, + autoAdvance, + playerDeck: ["card3"], + cpuDeck: ["card4"], + tiePile: [], + currentRound: 1, + currentTurn: "player", + playerCard: "card1", + cpuCard: "card2", + selectedStat: "attack", + roundResult: { + winner: "player", + playerValue: 80, + cpuValue: 70, + stat: "attack", + cardsWon: 2, + }, + roundsWon: { player: 1, cpu: 0 }, + cardsWon: { player: 2, cpu: 0 }, + gameStartedAt: 0, + gameEndedAt: null, + numericFields: sampleFields, + cardData: sampleCardData, + playerSelectionHistory: ["attack"], + errorMessage: null, + }); +} + +describe("RoundResultOverlay auto-advance", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + cleanup(); + vi.clearAllTimers(); + vi.useRealTimers(); + }); + + it("should not advance the round on the timer when auto-advance is disabled", () => { + seedRoundEnd(false); + + render(); + expect(screen.getByText("You Win!")).toBeInTheDocument(); + + act(() => { + vi.advanceTimersByTime(3000); + }); + + const state = useCompetingStore.getState(); + expect(state.phase).toBe("round_end"); + expect(state.currentRound).toBe(1); + }); + + it("should advance the round on the timer when auto-advance is enabled", () => { + seedRoundEnd(true); + + render(); + + act(() => { + vi.advanceTimersByTime(2000); + }); + + const state = useCompetingStore.getState(); + expect(state.phase).toBe("player_select"); + expect(state.currentRound).toBe(2); + }); + + it("should still advance on click when auto-advance is disabled", () => { + seedRoundEnd(false); + + render(); + + act(() => { + fireEvent.click(screen.getByText("You Win!")); + }); + + const state = useCompetingStore.getState(); + expect(state.phase).toBe("player_select"); + expect(state.currentRound).toBe(2); + }); + + it("should still advance on key press when auto-advance is disabled", () => { + seedRoundEnd(false); + + render(); + + act(() => { + fireEvent.keyDown(window, { key: "a" }); + }); + + const state = useCompetingStore.getState(); + expect(state.phase).toBe("player_select"); + expect(state.currentRound).toBe(2); + }); +}); From ae4dcff2d1622f26b85d96e9aa8a28ad586a0a9f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:29:15 +0000 Subject: [PATCH 04/10] fix: cancel superseded discovery scans and pair sources with their scanned username A collection scan for one username could finish after a newer scan for another and overwrite its results (or wipe them with a stale error), because no state update in useMyPlausibleMeDiscovery was guarded against staleness and the effect cleanup only cleared the debounce timer. Worst case, CollectionPicker paired the current username with a stale entry's folder and persisted an activated source that can never resolve. Add a generation token bumped on each run and on effect cleanup, guard every state update and the metadata worker loop, carry the scanned username on each discovered entry, and have the picker add sources under the entry's own username. Reproduction tests observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- .../CollectionPicker/CollectionPicker.tsx | 9 +- src/hooks/useMyPlausibleMeDiscovery.ts | 42 +++- .../CollectionPicker.staleUsername.test.tsx | 69 ++++++ .../useMyPlausibleMeDiscovery.race.test.ts | 198 ++++++++++++++++++ 4 files changed, 314 insertions(+), 4 deletions(-) create mode 100644 tests/components/CollectionPicker.staleUsername.test.tsx create mode 100644 tests/hooks/useMyPlausibleMeDiscovery.race.test.ts diff --git a/src/components/CollectionPicker/CollectionPicker.tsx b/src/components/CollectionPicker/CollectionPicker.tsx index c1ea771..a635066 100644 --- a/src/components/CollectionPicker/CollectionPicker.tsx +++ b/src/components/CollectionPicker/CollectionPicker.tsx @@ -67,15 +67,18 @@ export function CollectionPicker({ onSelect, initialUsername, notice }: Collecti }, [inputValue, username]); const handleCollectionSelect = useCallback((collection: CollectionEntry) => { - // Add the source and set it as active + // Pair the folder with the username the entry was scanned from, not the + // current input state: a scan is asynchronous, so the entry on screen may + // predate a username change. Using the current username would persist and + // activate a source that can never resolve. const sourceId = addMyPlausibleMeSource( - username, + collection.username, collection.folder, collection.name ); setActiveSource(sourceId); onSelect(sourceId); - }, [username, addMyPlausibleMeSource, setActiveSource, onSelect]); + }, [addMyPlausibleMeSource, setActiveSource, onSelect]); // Handle example collection selection (F-112) const handleExampleSelect = useCallback((example: ExampleCollection) => { diff --git a/src/hooks/useMyPlausibleMeDiscovery.ts b/src/hooks/useMyPlausibleMeDiscovery.ts index b1a044b..7a2bdad 100644 --- a/src/hooks/useMyPlausibleMeDiscovery.ts +++ b/src/hooks/useMyPlausibleMeDiscovery.ts @@ -8,7 +8,7 @@ * @see F-087: Collection Discovery & Startup Picker */ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { isCollectionCached } from "@/lib/cardCache"; import { useSourceStore } from "@/stores/sourceStore"; @@ -16,6 +16,15 @@ import { useSourceStore } from "@/stores/sourceStore"; * Collection entry discovered from repository. */ export interface CollectionEntry { + /** + * GitHub username this collection was scanned from. + * + * Carried on the entry rather than read from the caller's current username + * state: a scan is asynchronous, so the entry on screen may belong to an + * earlier username. Pairing that folder with the newer username would build + * (and persist) a source that can never resolve. + */ + username: string; /** Collection folder name */ folder: string; /** Display name (from collection.json or folder name) */ @@ -176,10 +185,25 @@ export function useMyPlausibleMeDiscovery( const [isLoading, setIsLoading] = useState(false); const [error, setError] = useState(null); + /** + * Generation counter identifying the newest discovery run. + * + * A scan is one tree fetch plus up to MAX_DISCOVERED_COLLECTIONS metadata + * fetches, so it can take seconds. Without a generation guard a superseded + * run finishing late would overwrite the newer run's collections (or wipe + * them with a stale error), leaving entries on screen that belong to a + * different username. + */ + const runIdRef = useRef(0); + const discover = useCallback(async () => { + const runId = ++runIdRef.current; + const isStale = () => runIdRef.current !== runId; + if (!username.trim()) { setCollections([]); setError(null); + setIsLoading(false); return; } @@ -198,6 +222,8 @@ export function useMyPlausibleMeDiscovery( }, }); + if (isStale()) return; + if (!response.ok) { if (response.status === 404) { setError("Repository not found. Check the username."); @@ -215,6 +241,8 @@ export function useMyPlausibleMeDiscovery( const treeData: unknown = await response.json(); + if (isStale()) return; + if (!treeData || typeof treeData !== "object" || !("tree" in treeData)) { setError("Invalid repository structure"); setCollections([]); @@ -261,6 +289,8 @@ export function useMyPlausibleMeDiscovery( let fileCursor = 0; const metadataWorker = async (): Promise => { while (fileCursor < collectionJsonFiles.length) { + // A superseded run keeps no results, so stop spending fetches on it. + if (isStale()) return; const index = fileCursor; fileCursor += 1; const file = collectionJsonFiles[index]; @@ -306,6 +336,7 @@ export function useMyPlausibleMeDiscovery( pathSegments[pathSegments.length - 1] ?? collectionPath; validCollections.push({ + username: trimmedUsername, folder: collectionPath, name: metadata.name ?? folderName, description: metadata.description, @@ -324,6 +355,8 @@ export function useMyPlausibleMeDiscovery( Array.from({ length: metadataWorkerCount }, () => metadataWorker()) ); + if (isStale()) return; + // Sort: cached collections first, then alphabetically by name validCollections.sort((a, b) => { // Cached collections come first @@ -342,6 +375,7 @@ export function useMyPlausibleMeDiscovery( setIsLoading(false); } catch (err) { + if (isStale()) return; setError(err instanceof Error ? err.message : "Discovery failed"); setCollections([]); setIsLoading(false); @@ -353,6 +387,7 @@ export function useMyPlausibleMeDiscovery( if (!enabled) { setCollections([]); setError(null); + setIsLoading(false); return; } @@ -363,6 +398,11 @@ export function useMyPlausibleMeDiscovery( return () => { clearTimeout(timeoutId); + // Invalidate any run already in flight for the previous username. The + // next run only starts after the debounce, so without this bump a scan + // resolving inside that window would still be the newest generation and + // would publish results for a username the caller has moved on from. + runIdRef.current += 1; }; }, [username, enabled, discover]); diff --git a/tests/components/CollectionPicker.staleUsername.test.tsx b/tests/components/CollectionPicker.staleUsername.test.tsx new file mode 100644 index 0000000..f863858 --- /dev/null +++ b/tests/components/CollectionPicker.staleUsername.test.tsx @@ -0,0 +1,69 @@ +/** + * Tests for CollectionPicker source pairing. + * + * Regression: the picker paired the CURRENT username state with whatever + * collection entry was on screen. A scan result that arrived from an earlier + * username therefore persisted (and activated) a source built from the new + * username and the old folder — a permanently-404ing source, added with no + * validation. The picker must use the username the entry was scanned from. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; + +vi.mock("@/hooks/useMyPlausibleMeDiscovery", () => ({ + useMyPlausibleMeDiscovery: vi.fn(), +})); + +import { CollectionPicker } from "@/components/CollectionPicker/CollectionPicker"; +import { useMyPlausibleMeDiscovery } from "@/hooks/useMyPlausibleMeDiscovery"; +import { useSourceStore } from "@/stores/sourceStore"; + +const SCANNED_USER = "scanneduser"; +const CURRENT_USER = "currentuser"; + +const addMyPlausibleMeSource = vi.fn(() => "src_test_1"); +const setActiveSource = vi.fn(); + +describe("CollectionPicker source pairing", () => { + beforeEach(() => { + vi.clearAllMocks(); + + vi.mocked(useMyPlausibleMeDiscovery).mockReturnValue({ + collections: [ + { + folder: "retro/games", + name: "Retro Games", + username: SCANNED_USER, + }, + ], + isLoading: false, + error: null, + refresh: vi.fn(), + }); + + useSourceStore.setState({ + addMyPlausibleMeSource, + setActiveSource, + }); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("adds the source under the username the collection was scanned from", () => { + render( + + ); + + fireEvent.click(screen.getByRole("button", { name: /Retro Games/ })); + + expect(addMyPlausibleMeSource).toHaveBeenCalledWith( + SCANNED_USER, + "retro/games", + "Retro Games" + ); + expect(setActiveSource).toHaveBeenCalledWith("src_test_1"); + }); +}); diff --git a/tests/hooks/useMyPlausibleMeDiscovery.race.test.ts b/tests/hooks/useMyPlausibleMeDiscovery.race.test.ts new file mode 100644 index 0000000..f9e330a --- /dev/null +++ b/tests/hooks/useMyPlausibleMeDiscovery.race.test.ts @@ -0,0 +1,198 @@ +/** + * Tests for useMyPlausibleMeDiscovery run cancellation. + * + * Regression: discovery had no generation guard, so a slow scan for username A + * (one GitHub tree fetch plus up to 200 jsDelivr metadata fetches) could land + * after a newer scan for username B and replace B's results with A's — or wipe + * them with a stale error. The picker then paired the CURRENT username with a + * STALE collection folder and persisted a permanently-404ing source. + * + * Each discovery run must be tagged, and every state write from a superseded + * run must be dropped — including during the sub-debounce window, where the + * newer run has not started yet. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act, waitFor } from "@testing-library/react"; + +vi.mock("@/lib/cardCache", () => ({ + isCollectionCached: vi.fn(() => Promise.resolve(false)), +})); + +import { useMyPlausibleMeDiscovery } from "@/hooks/useMyPlausibleMeDiscovery"; +import { isCollectionCached } from "@/lib/cardCache"; +import { useSourceStore } from "@/stores/sourceStore"; + +const SLOW_USER = "slowuser"; +const FAST_USER = "fastuser"; +const OTHER_USER = "otheruser"; + +/** Debounce window inside the hook. */ +const DEBOUNCE_MS = 500; + +/** Build a single-collection tree response for a username. */ +function treeFor(folder: string) { + return { + sha: "sha-tree", + truncated: false, + tree: [ + { + path: `data/collections/${folder}/collection.json`, + type: "blob" as const, + sha: "sha-blob", + }, + ], + }; +} + +/** Extract the repository owner from either the API or the CDN URL. */ +function usernameFrom(url: string): string { + const apiMatch = /api\.github\.com\/repos\/([^/]+)\//.exec(url); + if (apiMatch?.[1]) return apiMatch[1]; + return /\/gh\/([^/]+)\//.exec(url)?.[1] ?? ""; +} + +/** Metadata fetches issued for the slow username. */ +let slowMetadataRequests = 0; +/** Releases the slow username's pending metadata fetches. */ +let releaseSlowMetadata: () => void = () => undefined; + +function mockFetch(): void { + slowMetadataRequests = 0; + const gate = new Promise((resolve) => { + releaseSlowMetadata = resolve; + }); + + vi.stubGlobal( + "fetch", + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + const user = usernameFrom(url); + + if (url.includes("api.github.com")) { + return new Response(JSON.stringify(treeFor(`${user}-collection`)), { + status: 200, + }); + } + + if (url.endsWith("/collection.json")) { + if (user === SLOW_USER) { + slowMetadataRequests += 1; + await gate; + } + return new Response(JSON.stringify({ name: `${user} collection` }), { + status: 200, + }); + } + + return new Response("not found", { status: 404 }); + }) + ); +} + +/** Let pending microtasks and timers settle inside React's act() scope. */ +async function settle(ms: number): Promise { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, ms)); + }); +} + +describe("useMyPlausibleMeDiscovery cancels superseded runs", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockFetch(); + useSourceStore.setState({ + sources: [], + activeSourceId: null, + defaultSourceId: null, + }); + vi.mocked(isCollectionCached).mockResolvedValue(false); + }); + + afterEach(() => { + releaseSlowMetadata(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it("keeps the newest username's results when an older scan finishes last", async () => { + const { result, rerender } = renderHook( + ({ user }: { user: string }) => useMyPlausibleMeDiscovery(user), + { initialProps: { user: SLOW_USER } } + ); + + // The slow scan has started and is parked on its metadata fetch. + await waitFor( + () => { + expect(slowMetadataRequests).toBe(1); + }, + { timeout: 3000 } + ); + + rerender({ user: FAST_USER }); + + // The newer scan completes end to end. + await waitFor( + () => { + expect(result.current.collections.map((c) => c.folder)).toEqual([ + `${FAST_USER}-collection`, + ]); + }, + { timeout: 3000 } + ); + + // Now the superseded scan finally resolves; it must not write anything. + releaseSlowMetadata(); + await settle(200); + + expect(result.current.collections.map((c) => c.folder)).toEqual([ + `${FAST_USER}-collection`, + ]); + expect(result.current.error).toBeNull(); + }); + + it("drops a superseded scan that resolves inside the debounce window", async () => { + const { result, rerender } = renderHook( + ({ user }: { user: string }) => useMyPlausibleMeDiscovery(user), + { initialProps: { user: SLOW_USER } } + ); + + await waitFor( + () => { + expect(slowMetadataRequests).toBe(1); + }, + { timeout: 3000 } + ); + + // Switch usernames, then let the old scan finish before the new scan's + // debounce has even elapsed — nothing has superseded it yet by arrival. + rerender({ user: OTHER_USER }); + releaseSlowMetadata(); + await settle(DEBOUNCE_MS / 5); + + expect(result.current.collections).toEqual([]); + + // The new scan still lands normally. + await waitFor( + () => { + expect(result.current.collections.map((c) => c.folder)).toEqual([ + `${OTHER_USER}-collection`, + ]); + }, + { timeout: 3000 } + ); + }); + + it("tags each discovered collection with the username it was scanned from", async () => { + const { result } = renderHook(() => useMyPlausibleMeDiscovery(FAST_USER)); + + await waitFor( + () => { + expect(result.current.collections).toHaveLength(1); + }, + { timeout: 3000 } + ); + + expect(result.current.collections[0]?.username).toBe(FAST_USER); + }); +}); From a10985b99a3487cb3d5600fccb8d22fa6888819e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:31:04 +0000 Subject: [PATCH 05/10] fix: remember every source whose collection defaults were applied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The applied-defaults marker was a single slot holding only the last source, so alternating between two collections that both ship a settings.json defaults block re-applied each one's defaults over the user's manual choices on every return visit — contradicting the documented one-shot-per-source intent. Track applied source ids as a bounded FIFO array (mirroring the cache-consent pattern), gate on membership, migrate the persisted scalar (version 27 to 28), normalise tampered persisted values, and keep the round-6 rule that restoring forced settings never clears defaults tracking. Reproduction tests observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/stores/settingsStore.ts | 70 +++++- src/utils/settingsExport.ts | 10 +- .../settingsStore.collectionDefaults.test.ts | 214 ++++++++++++++++++ .../settingsStore.collectionForced.test.ts | 6 +- tests/utils/settingsExport.test.ts | 4 +- 5 files changed, 282 insertions(+), 22 deletions(-) create mode 100644 tests/stores/settingsStore.collectionDefaults.test.ts diff --git a/src/stores/settingsStore.ts b/src/stores/settingsStore.ts index 626da97..aeb1415 100644 --- a/src/stores/settingsStore.ts +++ b/src/stores/settingsStore.ts @@ -542,8 +542,15 @@ interface SettingsState { /** Source ID whose forced settings are currently applied (null = none) */ collectionForcedSourceId: string | null; - /** Source ID of the collection whose defaults were applied (prevents re-applying) */ - appliedCollectionDefaultsSourceId: string | null; + /** + * Source IDs whose settings.json `defaults` have already been applied. + * A collection seeds its defaults the first time it is seen and never again, + * so this has to remember every source, not just the most recent one: + * alternating between two collections that both ship defaults would + * otherwise let each re-apply its defaults over the user's later choices. + * Capped at MAX_APPLIED_COLLECTION_DEFAULTS_SOURCES (oldest evicted first). + */ + appliedCollectionDefaultsSourceIds: string[]; // ============================================================================ // v0.14.0: Draft State Management (F-090) @@ -595,7 +602,7 @@ interface SettingsState { setShowDragIcon: (show: boolean) => void; setCustomThemeUrl: (url: string | null) => void; setHasAppliedCollectionDefaults: (applied: boolean) => void; - setAppliedCollectionDefaultsSourceId: (sourceId: string | null) => void; + setAppliedCollectionDefaultsSourceIds: (sourceIds: string[]) => void; applyCollectionDefaults: (config: CollectionConfigForDefaults) => void; setRandomSelectionEnabled: (enabled: boolean) => void; setRandomSelectionCount: (count: number) => void; @@ -684,6 +691,13 @@ interface SettingsState { hasCacheConsent: (sourceId: string) => boolean; } +/** + * Maximum number of source IDs remembered as having had their collection + * `defaults` applied. Bounded so a long browsing history cannot grow the + * persisted state without limit; the oldest entry is evicted first. + */ +const MAX_APPLIED_COLLECTION_DEFAULTS_SOURCES = 50; + /** * Default settings values. */ @@ -753,7 +767,7 @@ const DEFAULT_SETTINGS = { collectionForcedSettings: null as ForcedSettings | null, _collectionForcedBackup: null as CollectionForcedBackup | null, collectionForcedSourceId: null as string | null, - appliedCollectionDefaultsSourceId: null as string | null, + appliedCollectionDefaultsSourceIds: [] as string[], // v0.14.0: Draft State defaults (F-090) _draft: null as DraftSettings | null, isDirty: false, @@ -886,8 +900,10 @@ export const useSettingsStore = create()( set({ hasAppliedCollectionDefaults }); }, - setAppliedCollectionDefaultsSourceId: (appliedCollectionDefaultsSourceId) => { - set({ appliedCollectionDefaultsSourceId }); + setAppliedCollectionDefaultsSourceIds: ( + appliedCollectionDefaultsSourceIds + ) => { + set({ appliedCollectionDefaultsSourceIds }); }, setRandomSelectionEnabled: (randomSelectionEnabled) => { @@ -1203,9 +1219,18 @@ export const useSettingsStore = create()( } } - // Only apply defaults if not already applied for this source - if (settings.defaults && state.appliedCollectionDefaultsSourceId !== sourceId) { - updates.appliedCollectionDefaultsSourceId = sourceId; + // Only apply defaults if they have never been applied for this + // source. Tracking every source (not just the most recent) is what + // makes this one-shot: alternating between two collections that both + // ship defaults must not let either re-seed over the user's choices. + if ( + settings.defaults && + !state.appliedCollectionDefaultsSourceIds.includes(sourceId) + ) { + updates.appliedCollectionDefaultsSourceIds = [ + ...state.appliedCollectionDefaultsSourceIds, + sourceId, + ].slice(-MAX_APPLIED_COLLECTION_DEFAULTS_SOURCES); // Apply default visual settings if (settings.defaults.visualTheme !== undefined) { @@ -1263,7 +1288,7 @@ export const useSettingsStore = create()( collectionForcedSettings: null, _collectionForcedBackup: null, collectionForcedSourceId: null, - // appliedCollectionDefaultsSourceId is intentionally untouched: + // appliedCollectionDefaultsSourceIds is intentionally untouched: // clearing it would let the same source re-apply its one-shot // `defaults` over the user's later choices. }; @@ -1467,13 +1492,19 @@ export const useSettingsStore = create()( }), { name: "itemdeck-settings", - version: 27, + version: 28, storage: createJSONStorage(() => localStorage), // Force-clear activeMechanicId after rehydration - games should never auto-start onRehydrateStorage: () => (state) => { if (state?.activeMechanicId) { state.activeMechanicId = null; } + // localStorage is attacker-adjacent and migrate only runs on a version + // change, so a same-version payload can still carry a non-array here. + // The applied-defaults gate calls .includes() on every collection load. + if (state && !Array.isArray(state.appliedCollectionDefaultsSourceIds)) { + state.appliedCollectionDefaultsSourceIds = []; + } // Restore settings from a mechanic override backup that survived a // crash or tab-kill. No mechanic session is ever active after // rehydration (activeMechanicId is never persisted), so a stored @@ -1559,7 +1590,8 @@ export const useSettingsStore = create()( // rolls them back from the backup. _collectionForcedBackup: state._collectionForcedBackup, collectionForcedSourceId: state.collectionForcedSourceId, - appliedCollectionDefaultsSourceId: state.appliedCollectionDefaultsSourceId, + appliedCollectionDefaultsSourceIds: + state.appliedCollectionDefaultsSourceIds, // v0.14.0: Draft state is intentionally NOT persisted // _draft and isDirty are excluded - editing session is transient // v0.15.5: Mechanic override backup IS persisted so that a crash or @@ -1848,6 +1880,20 @@ export const useSettingsStore = create()( }; } + // Handle migration from version 27 to 28 (track EVERY source whose + // collection defaults were applied, not just the most recent one). + // Carry the single remembered source forward; anything else (including + // a tampered localStorage value) normalises to an empty list. + if (version < 28) { + const { appliedCollectionDefaultsSourceId: previous, ...rest } = + state; + state = { + ...rest, + appliedCollectionDefaultsSourceIds: + typeof previous === "string" ? [previous] : [], + }; + } + return state as unknown as SettingsState; }, } diff --git a/src/utils/settingsExport.ts b/src/utils/settingsExport.ts index 1dabe4f..584ca37 100644 --- a/src/utils/settingsExport.ts +++ b/src/utils/settingsExport.ts @@ -201,14 +201,14 @@ function applySettings(settings: ExportableSettings, mode: ImportMode): void { // CollectionDataContext effects re-fire and re-apply the active collection's // defaults over the values being imported (silently discarding the user's // backup for those fields). `hasAppliedCollectionDefaults` gates the config - // defaults; `appliedCollectionDefaultsSourceId` (compared against the active + // defaults; `appliedCollectionDefaultsSourceIds` (checked for the active // source in applyCollectionSettings) gates the settings.json defaults — // restoring only the former still let the active collection's settings.json // clobber the import on the next load. Both are internal state, not // user-facing settings, so importing settings must not disturb them. const hadAppliedCollectionDefaults = store.hasAppliedCollectionDefaults; - const hadAppliedCollectionDefaultsSourceId = - store.appliedCollectionDefaultsSourceId; + const hadAppliedCollectionDefaultsSourceIds = + store.appliedCollectionDefaultsSourceIds; // Reset to defaults first if replace mode if (mode === "replace") { @@ -301,8 +301,8 @@ function applySettings(settings: ExportableSettings, mode: ImportMode): void { // via its config defaults or its settings.json defaults). if (mode === "replace") { store.setHasAppliedCollectionDefaults(hadAppliedCollectionDefaults); - store.setAppliedCollectionDefaultsSourceId( - hadAppliedCollectionDefaultsSourceId + store.setAppliedCollectionDefaultsSourceIds( + hadAppliedCollectionDefaultsSourceIds ); } } diff --git a/tests/stores/settingsStore.collectionDefaults.test.ts b/tests/stores/settingsStore.collectionDefaults.test.ts new file mode 100644 index 0000000..fcc1f48 --- /dev/null +++ b/tests/stores/settingsStore.collectionDefaults.test.ts @@ -0,0 +1,214 @@ +/** + * Tests for the one-shot application of a collection's settings.json + * `defaults`. + * + * A collection's `defaults` are a courtesy: they seed the user's settings the + * FIRST time that source is seen and must never be re-applied afterwards, or a + * source could silently undo the user's own later choices. Remembering only + * the most recent source made that guarantee collapse as soon as two sources + * that both ship defaults were alternated between, so the applied set is + * tracked per source. + */ + +import { describe, it, expect, beforeEach, vi } from "vitest"; +import { useSettingsStore } from "@/stores/settingsStore"; +import type { CollectionSettings } from "@/types/collectionSettings"; + +const STORAGE_KEY = "itemdeck-settings"; + +function getSetItemMock() { + return vi.mocked(window.localStorage.setItem); +} + +function getGetItemMock() { + return vi.mocked(window.localStorage.getItem); +} + +function lastPersistedState(): Record { + const writes = getSetItemMock().mock.calls.filter( + ([key]) => key === STORAGE_KEY + ); + const lastWrite = writes[writes.length - 1]; + if (!lastWrite) throw new Error("expected a persisted write"); + const parsed = JSON.parse(lastWrite[1]) as { + state: Record; + }; + return parsed.state; +} + +const SOURCE_A = "https://cdn.jsdelivr.net/gh/a/MyPlausibleMe@main/x"; +const SOURCE_B = "https://cdn.jsdelivr.net/gh/b/MyPlausibleMe@main/y"; + +const A_DEFAULTS: CollectionSettings = { + defaults: { visualTheme: "retro", cardSizePreset: "small" }, +}; +const B_DEFAULTS: CollectionSettings = { + defaults: { visualTheme: "minimal", cardSizePreset: "large" }, +}; + +describe("settingsStore - collection defaults are applied once per source", () => { + beforeEach(() => { + useSettingsStore.getState().resetToDefaults(); + getSetItemMock().mockClear(); + getGetItemMock().mockReset(); + }); + + it("does not re-apply a source's defaults when alternating A → B → A", () => { + // First visit to A seeds A's defaults. + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + expect(useSettingsStore.getState().visualTheme).toBe("retro"); + + // The user visits B, which seeds its own defaults. + useSettingsStore.getState().applyCollectionSettings(SOURCE_B, B_DEFAULTS); + expect(useSettingsStore.getState().visualTheme).toBe("minimal"); + + // The user then makes deliberate choices of their own. + useSettingsStore.getState().setVisualTheme("modern"); + useSettingsStore.getState().setCardSizePreset("medium"); + + // Returning to A must NOT re-apply A's one-shot defaults. + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + + const state = useSettingsStore.getState(); + expect(state.visualTheme).toBe("modern"); + expect(state.cardSizePreset).toBe("medium"); + }); + + it("survives repeated alternation between two sources that ship defaults", () => { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + useSettingsStore.getState().applyCollectionSettings(SOURCE_B, B_DEFAULTS); + + useSettingsStore.getState().setVisualTheme("modern"); + useSettingsStore.getState().setMaxVisibleCards(7); + + for (let i = 0; i < 5; i++) { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + useSettingsStore.getState().applyCollectionSettings(SOURCE_B, B_DEFAULTS); + } + + const state = useSettingsStore.getState(); + expect(state.visualTheme).toBe("modern"); + expect(state.maxVisibleCards).toBe(7); + }); + + it("still skips a re-apply of the same source's defaults", () => { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + useSettingsStore.getState().setVisualTheme("modern"); + + // A refetch of the same source (settings.json is not pinned). + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + + expect(useSettingsStore.getState().visualTheme).toBe("modern"); + expect( + useSettingsStore.getState().appliedCollectionDefaultsSourceIds + ).toEqual([SOURCE_A]); + }); + + it("tracks each source that has had its defaults applied", () => { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + useSettingsStore.getState().applyCollectionSettings(SOURCE_B, B_DEFAULTS); + + expect( + useSettingsStore.getState().appliedCollectionDefaultsSourceIds + ).toEqual([SOURCE_A, SOURCE_B]); + }); + + it("caps the tracked source ids, evicting the oldest first", () => { + for (let i = 0; i < 55; i++) { + useSettingsStore + .getState() + .applyCollectionSettings(`https://example.com/s${i}`, A_DEFAULTS); + } + + const tracked = + useSettingsStore.getState().appliedCollectionDefaultsSourceIds; + expect(tracked).toHaveLength(50); + expect(tracked).not.toContain("https://example.com/s0"); + expect(tracked[tracked.length - 1]).toBe("https://example.com/s54"); + }); + + it("is not cleared by restoreCollectionForcedSettings", () => { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, { + ...A_DEFAULTS, + forced: { cardBackDisplay: "none" }, + }); + + useSettingsStore.getState().restoreCollectionForcedSettings(); + + expect( + useSettingsStore.getState().appliedCollectionDefaultsSourceIds + ).toEqual([SOURCE_A]); + + // And the restored source still cannot re-seed its defaults. + useSettingsStore.getState().setVisualTheme("modern"); + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + expect(useSettingsStore.getState().visualTheme).toBe("modern"); + }); + + it("persists the tracked source ids", () => { + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + + expect(lastPersistedState().appliedCollectionDefaultsSourceIds).toEqual([ + SOURCE_A, + ]); + }); + + it("migrates a version 27 scalar source id to a one-element array", async () => { + getGetItemMock().mockReturnValue( + JSON.stringify({ + state: { appliedCollectionDefaultsSourceId: SOURCE_A }, + version: 27, + }) + ); + + await useSettingsStore.persist.rehydrate(); + + const state = useSettingsStore.getState() as unknown as Record< + string, + unknown + >; + expect(state.appliedCollectionDefaultsSourceIds).toEqual([SOURCE_A]); + expect(state.appliedCollectionDefaultsSourceId).toBeUndefined(); + + // The migrated source keeps its one-shot guarantee. + useSettingsStore.getState().setVisualTheme("modern"); + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + expect(useSettingsStore.getState().visualTheme).toBe("modern"); + }); + + it("migrates a version 27 null source id to an empty array", async () => { + getGetItemMock().mockReturnValue( + JSON.stringify({ + state: { appliedCollectionDefaultsSourceId: null }, + version: 27, + }) + ); + + await useSettingsStore.persist.rehydrate(); + + expect( + useSettingsStore.getState().appliedCollectionDefaultsSourceIds + ).toEqual([]); + }); + + it("normalises a tampered persisted value to an empty array", async () => { + // localStorage is attacker-adjacent: a non-array value must not survive + // into the store, where `.includes` would throw on every collection load. + getGetItemMock().mockReturnValue( + JSON.stringify({ + state: { appliedCollectionDefaultsSourceIds: { evil: true } }, + version: 28, + }) + ); + + await useSettingsStore.persist.rehydrate(); + + expect( + useSettingsStore.getState().appliedCollectionDefaultsSourceIds + ).toEqual([]); + + // The store is still functional afterwards. + useSettingsStore.getState().applyCollectionSettings(SOURCE_A, A_DEFAULTS); + expect(useSettingsStore.getState().visualTheme).toBe("retro"); + }); +}); diff --git a/tests/stores/settingsStore.collectionForced.test.ts b/tests/stores/settingsStore.collectionForced.test.ts index abbcd1d..abbe023 100644 --- a/tests/stores/settingsStore.collectionForced.test.ts +++ b/tests/stores/settingsStore.collectionForced.test.ts @@ -137,7 +137,7 @@ describe("settingsStore - collection forced-settings backup/restore", () => { it("restores the user's own values and clears backup state on restore", () => { useSettingsStore.setState({ cardBackDisplay: "logo", - appliedCollectionDefaultsSourceId: SOURCE_A, + appliedCollectionDefaultsSourceIds: [SOURCE_A], }); useSettingsStore.getState().applyCollectionSettings(SOURCE_A, { forced: { cardBackDisplay: "none" }, @@ -150,8 +150,8 @@ describe("settingsStore - collection forced-settings backup/restore", () => { expect(state.collectionForcedSettings).toBeNull(); expect(state._collectionForcedBackup).toBeNull(); expect(state.collectionForcedSourceId).toBeNull(); - // appliedCollectionDefaultsSourceId must NOT be cleared by a forced restore. - expect(state.appliedCollectionDefaultsSourceId).toBe(SOURCE_A); + // appliedCollectionDefaultsSourceIds must NOT be cleared by a forced restore. + expect(state.appliedCollectionDefaultsSourceIds).toEqual([SOURCE_A]); }); it("captures the user's originals (not source A's forced values) when chaining A → B", () => { diff --git a/tests/utils/settingsExport.test.ts b/tests/utils/settingsExport.test.ts index 6dfa1cb..e5cc37c 100644 --- a/tests/utils/settingsExport.test.ts +++ b/tests/utils/settingsExport.test.ts @@ -150,7 +150,7 @@ describe("settingsExport", () => { // already been applied for the active source. const store = useSettingsStore.getState(); store.setHasAppliedCollectionDefaults(true); - store.setAppliedCollectionDefaultsSourceId("src_active"); + store.setAppliedCollectionDefaultsSourceIds(["src_active"]); const importData = { version: SETTINGS_EXPORT_VERSION, @@ -166,7 +166,7 @@ describe("settingsExport", () => { // marker to null would re-arm the active collection's settings.json // defaults to clobber the just-imported values on the next load. expect(state.hasAppliedCollectionDefaults).toBe(true); - expect(state.appliedCollectionDefaultsSourceId).toBe("src_active"); + expect(state.appliedCollectionDefaultsSourceIds).toEqual(["src_active"]); }); it("preserves unspecified settings in merge mode", async () => { From ed5c4cc9f19503efc49c59bc63b4bb354e0fe4c6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:31:28 +0000 Subject: [PATCH 06/10] fix: validate source health against the real collection definition schema checkSourceHealth parsed fetched collection.json with the legacy items/categories schema and matched schema versions against strings the version detector never returns, so every healthy source showed a permanent red Invalid indicator in the sources settings tab. Parse with the v2 collection definition schema, take the name from the definition, infer the schema version via detectSchemaVersion, and stop deriving itemCount (entity counts live in per-type index files). The test fixture now imports the canonical example collection so it cannot go vacuous again, and the legacy payload is asserted to be rejected. Regression test observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/services/sourceHealthCheck.ts | 31 +++++++---- tests/services/sourceHealthCheck.test.ts | 70 +++++++++++++++++++++--- 2 files changed, 82 insertions(+), 19 deletions(-) diff --git a/src/services/sourceHealthCheck.ts b/src/services/sourceHealthCheck.ts index 3eb38ab..0fff3b7 100644 --- a/src/services/sourceHealthCheck.ts +++ b/src/services/sourceHealthCheck.ts @@ -5,7 +5,8 @@ * Performs accessibility checks, schema validation, and latency measurement. */ -import { collectionSchema } from "@/schemas/collection.schema"; +import { collectionDefinitionSchema } from "@/schemas/v2/collection.schema"; +import { detectSchemaVersion, type CollectionDefinition } from "@/types/schema"; /** * Health check result status. @@ -50,7 +51,13 @@ export interface HealthCheckResult { latency: number; /** Collection name if discovered */ collectionName?: string; - /** Number of items in collection */ + /** + * Number of items in collection. + * + * Not derivable from collection.json: the definition describes entity types, + * while entity counts live in the per-entity index files. Left undefined + * unless a future check fetches those indexes. + */ itemCount?: number; /** Schema version detected */ schemaVersion?: string; @@ -72,8 +79,11 @@ const HIGH_LATENCY_THRESHOLD_MS = 2000; /** * Supported schema versions. + * + * Values match those returned by {@link detectSchemaVersion}; collections may + * omit `schemaVersion` entirely, in which case the version is inferred. */ -const SUPPORTED_SCHEMA_VERSIONS = ["2.0", "2"]; +const SUPPORTED_SCHEMA_VERSIONS = ["v1", "v2"]; /** * Check health of a remote source. @@ -90,7 +100,6 @@ export async function checkSourceHealth(url: string): Promise const startTime = performance.now(); const issues: HealthIssue[] = []; let collectionName: string | undefined; - let itemCount: number | undefined; let schemaVersion: string | undefined; let schemaCompatible: boolean | undefined; @@ -172,13 +181,16 @@ export async function checkSourceHealth(url: string): Promise } // Phase 3: Schema validation - const parseResult = collectionSchema.safeParse(data); + const parseResult = collectionDefinitionSchema.safeParse(data); if (parseResult.success) { - const collection = parseResult.data; - collectionName = collection.meta?.name; - itemCount = collection.items.length; - const version = collection.meta?.schemaVersion ?? "2.0"; + // Cast as in the collection loader: the inferred schema type is a looser + // structural match for the hand-written definition type. + const collection = parseResult.data as CollectionDefinition; + collectionName = collection.name; + // Collections may omit schemaVersion, so infer it from the definition + // rather than assuming a default. + const version = detectSchemaVersion(collection); schemaVersion = version; schemaCompatible = SUPPORTED_SCHEMA_VERSIONS.includes(version); @@ -221,7 +233,6 @@ export async function checkSourceHealth(url: string): Promise status, latency, collectionName, - itemCount, schemaVersion, schemaCompatible, lastChecked: new Date(), diff --git a/tests/services/sourceHealthCheck.test.ts b/tests/services/sourceHealthCheck.test.ts index 67c3862..ff7d565 100644 --- a/tests/services/sourceHealthCheck.test.ts +++ b/tests/services/sourceHealthCheck.test.ts @@ -16,8 +16,15 @@ globalThis.fetch = mockFetch; // Note: We don't mock performance.now as it complicates async tests // Instead we'll just check that latency is a reasonable number -// Valid collection data that matches the collection schema -const validCollection = { +// The canonical collection definition shipped as the reference example. +// Imported rather than hand-written so the fixture always matches the shape +// real collections publish at collection.json, and cannot drift into a +// synthetic shape that no source actually serves. +import validCollection from "../../docs/reference/schemas/examples/retro-games/collection.json"; + +// The legacy v1 payload shape (items/categories), which no real collection +// serves — it is mutually exclusive with the entity-type format above. +const legacyItemsCollection = { meta: { name: "Test Collection", schemaVersion: "2.0", @@ -27,7 +34,7 @@ const validCollection = { { id: "2", title: "Item 2" }, { id: "3", title: "Item 3" }, ], - categories: [], // Required by schema + categories: [], }; describe("checkSourceHealth", () => { @@ -40,7 +47,7 @@ describe("checkSourceHealth", () => { }); describe("healthy status", () => { - it("should return healthy for valid source", async () => { + it("should return healthy for the canonical collection definition", async () => { mockFetch .mockResolvedValueOnce({ ok: true }) // HEAD request .mockResolvedValueOnce({ @@ -51,13 +58,24 @@ describe("checkSourceHealth", () => { const result = await checkSourceHealth("https://example.com/data"); expect(result.status).toBe("healthy"); - expect(result.collectionName).toBe("Test Collection"); - expect(result.itemCount).toBe(3); - expect(result.schemaVersion).toBe("2.0"); + expect(result.collectionName).toBe("My Top Computer & Video Games"); expect(result.schemaCompatible).toBe(true); expect(result.issues).toHaveLength(0); }); + it("should leave itemCount undefined (counts live in entity indexes)", async () => { + mockFetch + .mockResolvedValueOnce({ ok: true }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve(validCollection), + }); + + const result = await checkSourceHealth("https://example.com/data"); + + expect(result.itemCount).toBeUndefined(); + }); + it("should normalise URL without trailing slash", async () => { mockFetch .mockResolvedValueOnce({ ok: true }) @@ -199,7 +217,22 @@ describe("checkSourceHealth", () => { expect(result.error).toBe("Schema validation failed"); }); - it("should detect supported schema version 2.0", async () => { + it("should return invalid for a legacy items/categories payload", async () => { + mockFetch + .mockResolvedValueOnce({ ok: true }) + .mockResolvedValueOnce({ + ok: true, + json: () => Promise.resolve(legacyItemsCollection), + }); + + const result = await checkSourceHealth("https://example.com/data"); + + expect(result.status).toBe("invalid"); + expect(result.issues.some((i) => i.code === "SCHEMA_INCOMPATIBLE")).toBe(true); + expect(result.error).toBe("Schema validation failed"); + }); + + it("should infer the schema version when the collection omits it", async () => { mockFetch .mockResolvedValueOnce({ ok: true }) .mockResolvedValueOnce({ @@ -209,7 +242,26 @@ describe("checkSourceHealth", () => { const result = await checkSourceHealth("https://example.com/data"); - expect(result.schemaVersion).toBe("2.0"); + // The canonical example declares no schemaVersion and uses no v2-only + // field types, so it is detected as v1 — and v1 is supported. + expect(validCollection).not.toHaveProperty("schemaVersion"); + expect(result.schemaVersion).toBe("v1"); + expect(result.schemaCompatible).toBe(true); + }); + + it("should detect an explicit v2 schema version", async () => { + mockFetch + .mockResolvedValueOnce({ ok: true }) + .mockResolvedValueOnce({ + ok: true, + json: () => + Promise.resolve({ ...validCollection, schemaVersion: "v2" }), + }); + + const result = await checkSourceHealth("https://example.com/data"); + + expect(result.status).toBe("healthy"); + expect(result.schemaVersion).toBe("v2"); expect(result.schemaCompatible).toBe(true); }); }); From cc6301ac4dc1ace40fd80a84746d270fe07bda7a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:32:03 +0000 Subject: [PATCH 07/10] fix: unbias quiz answer order and make a flawless run score 100% The answer shuffle seeded a position formula with a sum of the question id's char codes; real ids cluster into few residues and the hand-rolled swap loop is a pure function of seed mod 1000, so the correct answer landed at option B about 36% of the time and only 13 of 24 orderings ever occurred. Hash the id (djb2) and delegate to the existing seeded Fisher-Yates in utils/shuffle, keeping per-question determinism. Separately, getResults assumed the full streak bonus on every question, but scoring uses the pre-answer streak, so question i can earn at most min(i * streakBonus, maxStreakBonus) and a flawless five-question untimed run displayed 70%. Compute the achievable maximum per index; flawless runs now score exactly 100% at every length in both modes. Reproduction tests observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/mechanics/quiz/store.ts | 46 +++--- tests/mechanics/quiz/store.test.ts | 222 +++++++++++++++++++++++++++++ 2 files changed, 251 insertions(+), 17 deletions(-) create mode 100644 tests/mechanics/quiz/store.test.ts diff --git a/src/mechanics/quiz/store.ts b/src/mechanics/quiz/store.ts index 112fae6..1accda1 100644 --- a/src/mechanics/quiz/store.ts +++ b/src/mechanics/quiz/store.ts @@ -5,6 +5,7 @@ */ import { create } from "zustand"; +import { shuffleWithSeed } from "@/utils/shuffle"; import { generateQuestions, canGenerateQuiz } from "./generators"; import type { GeneratorCardData } from "./generators"; import type { @@ -51,6 +52,21 @@ interface QuizStore extends QuizState, QuizSettings { getShuffledAnswers: () => Answer[]; } +/** + * Hash a string to a 32-bit seed (djb2). + * + * Question IDs share a long common prefix and differ only in a short suffix, + * so summing character codes collapses them onto a handful of seeds and biases + * the answer order. Mixing every character keeps the seeds spread out. + */ +function hashString(value: string): number { + let hash = 5381; + for (let i = 0; i < value.length; i++) { + hash = ((hash << 5) + hash + value.charCodeAt(i)) | 0; + } + return hash >>> 0; +} + /** * Initial state. */ @@ -344,9 +360,19 @@ export const useQuizStore = create((set, get) => ({ const { answers, maxStreak, quizStartedAt, quizEndedAt, questions, timerMode } = get(); const totalScore = answers.reduce((sum, a) => sum + a.pointsEarned, 0); - // Max score includes timer bonus if timer mode is enabled + // Max score includes timer bonus if timer mode is enabled. + // The streak bonus is scored from the streak held *before* the answer, so + // question i can earn at most i streak levels: the full streak bonus is + // unreachable on the opening questions and must not inflate the maximum. const maxTimerBonus = timerMode ? SCORING.timerBonus.fast.points : 0; - const maxScore = questions.length * (SCORING.basePoints + SCORING.maxStreakBonus + maxTimerBonus); + const maxScore = questions.reduce( + (sum, _question, i) => + sum + + SCORING.basePoints + + Math.min(i * SCORING.streakBonus, SCORING.maxStreakBonus) + + maxTimerBonus, + 0 + ); const correctCount = answers.filter((a) => a.isCorrect).length; const incorrectCount = answers.filter((a) => !a.isCorrect && a.selectedAnswerId !== null).length; const skippedCount = answers.filter((a) => a.selectedAnswerId === null).length; @@ -377,20 +403,6 @@ export const useQuizStore = create((set, get) => ({ // Use a seeded shuffle based on question ID for consistency // during re-renders (answers stay in same position) - const seed = question.id.split("").reduce((acc, char) => acc + char.charCodeAt(0), 0); - const shuffled = [...allAnswers]; - - // Simple seeded shuffle - for (let i = shuffled.length - 1; i > 0; i--) { - const j = Math.floor(((seed * (i + 1)) % 1000) / 1000 * (i + 1)); - const temp = shuffled[i]; - const swapItem = shuffled[j]; - if (temp !== undefined && swapItem !== undefined) { - shuffled[i] = swapItem; - shuffled[j] = temp; - } - } - - return shuffled; + return shuffleWithSeed(allAnswers, hashString(question.id)); }, })); diff --git a/tests/mechanics/quiz/store.test.ts b/tests/mechanics/quiz/store.test.ts new file mode 100644 index 0000000..be3c642 --- /dev/null +++ b/tests/mechanics/quiz/store.test.ts @@ -0,0 +1,222 @@ +/** + * Quiz store answer placement and results scoring. + * + * The answer order shown to the player is derived from the question ID so it + * stays stable across re-renders, and the results screen compares the player's + * score against the best score that run could have produced. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import { useQuizStore } from "@/mechanics/quiz/store"; +import type { Answer, Question } from "@/mechanics/quiz/types"; +import { generateQuestionId } from "@/mechanics/quiz/generators/utils"; + +/** Build a question whose four options are identifiable by label. */ +function questionWithId(id: string): Question { + const answer = (label: string): Answer => ({ id: `a-${label}`, label }); + return { + id, + type: "imageToName", + prompt: "Which card is this?", + correctAnswer: answer("correct"), + wrongAnswers: [answer("wrong1"), answer("wrong2"), answer("wrong3")], + relatedCardId: "card-1", + }; +} + +/** + * Question IDs as the generator really produces them: a millisecond timestamp + * plus seven base-36 characters. + */ +function realisticQuestionIds(count: number): string[] { + const ids: string[] = []; + const base = Date.UTC(2026, 0, 1); + for (let i = 0; i < count; i++) { + // Space the timestamps over a few hours, as separate quiz runs would be. + vi.setSystemTime(base + i * 6151); + ids.push(generateQuestionId()); + } + return ids; +} + +/** Place a single question in front of an active quiz. */ +function showQuestion(question: Question): void { + useQuizStore.setState({ + isActive: true, + questions: [question], + currentIndex: 0, + }); +} + +describe("getShuffledAnswers", () => { + beforeEach(() => { + useQuizStore.getState().deactivate(); + }); + + afterEach(() => { + vi.useRealTimers(); + useQuizStore.getState().deactivate(); + }); + + it("spreads the correct answer evenly across the four positions", () => { + vi.useFakeTimers(); + const ids = realisticQuestionIds(2000); + vi.useRealTimers(); + + const positionCounts = [0, 0, 0, 0]; + for (const id of ids) { + showQuestion(questionWithId(id)); + const shuffled = useQuizStore.getState().getShuffledAnswers(); + const position = shuffled.findIndex((a) => a.label === "correct"); + expect(position).toBeGreaterThanOrEqual(0); + positionCounts[position] = (positionCounts[position] ?? 0) + 1; + } + + for (const count of positionCounts) { + const share = count / ids.length; + expect(share).toBeGreaterThan(0.15); + expect(share).toBeLessThan(0.35); + } + }); + + it("reaches every ordering of the four options", () => { + vi.useFakeTimers(); + const ids = realisticQuestionIds(2000); + vi.useRealTimers(); + + const orderings = new Set(); + for (const id of ids) { + showQuestion(questionWithId(id)); + orderings.add( + useQuizStore + .getState() + .getShuffledAnswers() + .map((a) => a.label) + .join(",") + ); + } + + expect(orderings.size).toBe(24); + }); + + it("keeps the same order for a question across re-renders", () => { + const question = questionWithId("q-1767225600000-abc1234"); + showQuestion(question); + + const first = useQuizStore.getState().getShuffledAnswers(); + const second = useQuizStore.getState().getShuffledAnswers(); + + expect(second.map((a) => a.id)).toEqual(first.map((a) => a.id)); + + // A fresh store instance for the same question ID must agree too. + useQuizStore.getState().deactivate(); + showQuestion(questionWithId(question.id)); + expect( + useQuizStore + .getState() + .getShuffledAnswers() + .map((a) => a.id) + ).toEqual(first.map((a) => a.id)); + }); + + it("returns every option exactly once", () => { + showQuestion(questionWithId("q-1767225600000-zzz0000")); + const labels = useQuizStore + .getState() + .getShuffledAnswers() + .map((a) => a.label) + .sort(); + + expect(labels).toEqual(["correct", "wrong1", "wrong2", "wrong3"]); + }); + + it("returns nothing when no question is showing", () => { + expect(useQuizStore.getState().getShuffledAnswers()).toEqual([]); + }); +}); + +describe("getResults", () => { + beforeEach(() => { + useQuizStore.getState().deactivate(); + }); + + afterEach(() => { + vi.useRealTimers(); + useQuizStore.getState().deactivate(); + }); + + /** Answer every question correctly, as fast as the scoring allows. */ + function playFlawlessRun(questionCount: number, timerMode: boolean): void { + const questions = Array.from({ length: questionCount }, (_, i) => + questionWithId(`q-1767225600000-run${String(i).padStart(4, "0")}`) + ); + + useQuizStore.setState({ + isActive: true, + timerMode, + questions, + currentIndex: 0, + answers: [], + score: 0, + streak: 0, + maxStreak: 0, + quizStartedAt: Date.now(), + quizEndedAt: null, + questionStartedAt: Date.now(), + feedbackVisible: false, + }); + + for (let i = 0; i < questionCount; i++) { + const question = useQuizStore.getState().getCurrentQuestion(); + expect(question).not.toBeNull(); + useQuizStore.getState().submitAnswer(question?.correctAnswer.id ?? ""); + useQuizStore.getState().nextQuestion(); + } + } + + for (const timerMode of [false, true]) { + for (const questionCount of [5, 10, 15, 20]) { + it(`scores a flawless ${String(questionCount)}-question run at 100% (timer ${ + timerMode ? "on" : "off" + })`, () => { + vi.useFakeTimers(); + vi.setSystemTime(Date.UTC(2026, 0, 1)); + + playFlawlessRun(questionCount, timerMode); + + const results = useQuizStore.getState().getResults(); + expect(results.correctCount).toBe(questionCount); + expect(results.totalScore).toBe(results.maxScore); + expect(results.percentage).toBe(100); + }); + } + } + + it("keeps an imperfect run below 100%", () => { + vi.useFakeTimers(); + vi.setSystemTime(Date.UTC(2026, 0, 1)); + + playFlawlessRun(10, false); + // Replace the last answer with a miss. + const answers = [...useQuizStore.getState().answers]; + const last = answers[answers.length - 1]; + if (last) { + answers[answers.length - 1] = { + ...last, + isCorrect: false, + pointsEarned: 0, + }; + } + useQuizStore.setState({ answers }); + + const results = useQuizStore.getState().getResults(); + expect(results.percentage).toBeLessThan(100); + expect(results.percentage).toBeGreaterThan(0); + }); + + it("reports zero rather than dividing by an empty quiz", () => { + const results = useQuizStore.getState().getResults(); + expect(results.maxScore).toBe(0); + expect(results.percentage).toBe(0); + }); +}); From 67897f6439862f28e863232f4f1e4f1ffa1f7984 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:32:03 +0000 Subject: [PATCH 08/10] fix: bound the quiz similarity scan over untrusted card data Expert/Extreme distractor selection scored every available card and walked every key of the correct card; both dimensions are controlled by untrusted collection data, so a wide hostile collection froze the main thread for seconds per quiz start. Sample the scored candidate pool at MAX_SIMILARITY_CANDIDATES and cap the per-card key walk at MAX_SIMILARITY_KEYS, leaving behaviour for ordinary collections unchanged. Reproduction tests observed failing before the change and passing after. Assisted-by: Claude:claude-fable-5 --- src/mechanics/quiz/generators/utils.ts | 33 +++- .../quiz/selectWrongAnswerCards.test.ts | 173 ++++++++++++++++++ 2 files changed, 202 insertions(+), 4 deletions(-) create mode 100644 tests/mechanics/quiz/selectWrongAnswerCards.test.ts diff --git a/src/mechanics/quiz/generators/utils.ts b/src/mechanics/quiz/generators/utils.ts index 217a966..d978577 100644 --- a/src/mechanics/quiz/generators/utils.ts +++ b/src/mechanics/quiz/generators/utils.ts @@ -16,6 +16,21 @@ export const MIN_CARDS_FOR_QUIZ = 4; */ export const WRONG_ANSWER_COUNT = 3; +/** + * Maximum cards scored for similarity when picking distractors. + * + * Similarity scoring is O(cards x fields) and both come from the loaded + * collection, so larger collections are sampled down to this many candidates. + */ +export const MAX_SIMILARITY_CANDIDATES = 200; + +/** + * Maximum entity fields compared by a single similarity score. + * + * Mirrors the MAX_DISPLAYABLE_FIELDS cap used for card details. + */ +export const MAX_SIMILARITY_KEYS = 100; + /** * Generate a unique question ID. */ @@ -73,7 +88,11 @@ export function calculateSimilarity( // Check other shared string fields (excluding standard fields) const excludeFields = new Set(["id", "title", "imageUrl", "year", "categoryShort", "categoryTitle"]); - for (const key of Object.keys(card1)) { + let keysVisited = 0; + for (const key in card1) { + if (!Object.hasOwn(card1, key)) continue; + if (keysVisited >= MAX_SIMILARITY_KEYS) break; + keysVisited++; if (excludeFields.has(key)) continue; const val1 = card1[key]; const val2 = card2[key]; @@ -111,8 +130,14 @@ export function selectWrongAnswerCards( return shuffled.slice(0, count); } - // Calculate similarity for each available card - const withSimilarity = available.map((card) => ({ + // Score a bounded pool: large collections are sampled first so the scan + // stays cheap regardless of how many cards the collection holds + const scorable = available.length > MAX_SIMILARITY_CANDIDATES + ? shuffle(available).slice(0, MAX_SIMILARITY_CANDIDATES) + : available; + + // Calculate similarity for each scorable card + const withSimilarity = scorable.map((card) => ({ card, similarity: calculateSimilarity(correctCard, card), })); @@ -122,7 +147,7 @@ export function selectWrongAnswerCards( // Take the most similar cards, but add some randomness // Take top 2*count similar cards, then shuffle and pick count - const candidatePool = withSimilarity.slice(0, Math.min(count * 2, available.length)); + const candidatePool = withSimilarity.slice(0, Math.min(count * 2, withSimilarity.length)); const shuffledCandidates = shuffle(candidatePool); return shuffledCandidates.slice(0, count).map((c) => c.card); } diff --git a/tests/mechanics/quiz/selectWrongAnswerCards.test.ts b/tests/mechanics/quiz/selectWrongAnswerCards.test.ts new file mode 100644 index 0000000..93f0b39 --- /dev/null +++ b/tests/mechanics/quiz/selectWrongAnswerCards.test.ts @@ -0,0 +1,173 @@ +/** + * Distractor selection over untrusted card data. + * + * On the expert and extreme difficulties every available card is scored for + * similarity against the correct card, and the scoring walks the correct card's + * keys. Both the collection size and the per-entity key count come from the + * loaded collection, so the scan has to be bounded on each axis. + */ + +import { describe, it, expect } from "vitest"; +import { + calculateSimilarity, + selectWrongAnswerCards, + MAX_SIMILARITY_KEYS, +} from "@/mechanics/quiz/generators/utils"; +import type { GeneratorCardData } from "@/mechanics/quiz/generators/types"; + +/** Cards carrying `keyCount` extra string fields each. */ +function wideCards(count: number, keyCount: number): GeneratorCardData[] { + return Array.from({ length: count }, (_, i) => { + const card: GeneratorCardData = { + id: `c${String(i)}`, + title: `Card ${String(i)}`, + imageUrl: `https://example.com/${String(i)}.png`, + year: String(1970 + (i % 50)), + categoryShort: `cat${String(i % 8)}`, + }; + for (let k = 0; k < keyCount; k++) { + card[`field${String(k)}`] = `value${String(k % 11)}`; + } + return card; + }); +} + +describe("selectWrongAnswerCards with untrusted card data", () => { + it("selects similar distractors for a large wide collection without stalling", () => { + // A quiz asks for distractors once per question; a full run is 20. + const cards = wideCards(6000, 400); + + const started = Date.now(); + for (let i = 0; i < 20; i++) { + const distractors = selectWrongAnswerCards( + cards, + cards[i] as GeneratorCardData, + 3, + true + ); + expect(distractors).toHaveLength(3); + for (const distractor of distractors) { + expect(distractor.id).not.toBe(cards[i]?.id); + } + } + const elapsed = Date.now() - started; + + expect(elapsed).toBeLessThan(1500); + }); + + it("still prefers similar cards for a normal collection", () => { + // Twelve cards: three share the correct card's category, the rest do not. + const correct: GeneratorCardData = { + id: "target", + title: "Target", + imageUrl: "https://example.com/target.png", + year: "1990", + categoryShort: "ARC", + }; + const similar = Array.from({ length: 3 }, (_, i) => ({ + id: `sim${String(i)}`, + title: `Similar ${String(i)}`, + imageUrl: `https://example.com/sim${String(i)}.png`, + year: "1990", + categoryShort: "ARC", + })); + const dissimilar = Array.from({ length: 8 }, (_, i) => ({ + id: `dis${String(i)}`, + title: `Different ${String(i)}`, + imageUrl: `https://example.com/dis${String(i)}.png`, + year: "2020", + categoryShort: "NES", + })); + + const selected = selectWrongAnswerCards( + [correct, ...similar, ...dissimilar], + correct, + 3, + true + ); + + expect(selected).toHaveLength(3); + // The pool of the six most similar cards is all three similar cards plus + // three others, so at least one similar card must survive the pick. + expect(selected.some((c) => c.id.startsWith("sim"))).toBe(true); + expect(selected.every((c) => c.id !== "target")).toBe(true); + }); + + it("never returns the correct card and honours the requested count", () => { + const cards = wideCards(40, 5); + const selected = selectWrongAnswerCards( + cards, + cards[0] as GeneratorCardData, + 3, + true + ); + + expect(selected).toHaveLength(3); + expect(new Set(selected.map((c) => c.id)).size).toBe(3); + expect(selected.every((c) => c.id !== cards[0]?.id)).toBe(true); + }); + + it("falls back to random selection when there are barely enough cards", () => { + const cards = wideCards(4, 3); + const selected = selectWrongAnswerCards( + cards, + cards[0] as GeneratorCardData, + 3, + true + ); + + expect(selected.map((c) => c.id).sort()).toEqual(["c1", "c2", "c3"]); + }); +}); + +describe("calculateSimilarity with untrusted card data", () => { + it("bounds the keys it walks on a card with a huge field count", () => { + const fieldCount = 20000; + let reads = 0; + + // The correct card drives the key loop, so count how many of its fields + // a single similarity score actually reads. + const wide: GeneratorCardData = { + id: "wide", + title: "Wide", + imageUrl: "https://example.com/wide.png", + }; + for (let k = 0; k < fieldCount; k++) { + Object.defineProperty(wide, `field${String(k)}`, { + enumerable: true, + get: () => { + reads++; + return `value${String(k % 11)}`; + }, + }); + } + const other = wideCards(1, fieldCount)[0] as GeneratorCardData; + + calculateSimilarity(wide, other); + + expect(reads).toBeGreaterThan(0); + expect(reads).toBeLessThanOrEqual(MAX_SIMILARITY_KEYS); + }); + + it("scores ordinary cards unchanged", () => { + const a: GeneratorCardData = { + id: "a", + title: "A", + imageUrl: "https://example.com/a.png", + year: "1990", + categoryShort: "ARC", + publisher: "Acme", + }; + const b: GeneratorCardData = { + id: "b", + title: "B", + imageUrl: "https://example.com/b.png", + year: "1990", + categoryShort: "ARC", + publisher: "Acme", + }; + + // 40 (category) + 30 (same year) + 10 (shared publisher) + expect(calculateSimilarity(a, b)).toBe(80); + }); +}); From 83038974911221ed989e11fe84feb55d3bc19bd0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 16 Aug 2026 03:32:41 +0000 Subject: [PATCH 09/10] docs: record round 11 decisions Assisted-by: Claude:claude-fable-5 --- .abcd/work/DECISIONS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index c96bd8c..911d3c7 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -12,3 +12,4 @@ Dated, one-line summaries of each autonomous security-hardening round. - 2026-08-13 — Round 8: fixed 5 confirmed defects — (1) the startup collection picker rendered discovered `collection.json` metadata (`name`/`description`/`itemCount`) straight into JSX as React children while `fetchCollectionMetadata` only checked `typeof data === "object"`, and the picker sits above every error boundary, so an attacker `/gh//` link (discovery auto-runs with no click) serving a non-primitive `name`/`description`/`itemCount` threw "Objects are not valid as a React child" and blanked the whole app, with the poisoned `name` also persisting into the source store; the discovery boundary now coerces all three to primitives. (2) `getDisplayableFields` and the platform `additionalFields` copy emitted one DOM row per entity key with no ceiling (the entity schema is `.loose()`), so a single entity carrying ~100k scalar keys mounted ~300k DOM nodes in one synchronous commit when its "More" overlay opened — both paths are now capped at 100, matching `MAX_MEDIA_PER_CARD`. (3) `validateForcedSettings` guarded `cardBackStyle` and `titleDisplayMode` against inverted allowlists (`plain|pattern|gradient` and `always|hover|never`) that matched neither the real `CardBackStyle` (`bitmap|svg|colour`) nor `TitleDisplayMode` (`truncate|wrap`) enums, so every honest author's forced value was silently dropped while out-of-enum values were accepted, persisted globally, and then rejected wholesale by the settings-export schema on reimport — bricking the user's own backup; the allowlists now match the real enums (this fixes the round-6/7 mismatch on its genuine grounds — the correctness drop plus the export-brick self-DoS — not the previously-refuted CSS-clamping claim, which remains inert). (4) a "replace"-mode settings import called `resetToDefaults()`, which cleared the one-time `hasAppliedCollectionDefaults` marker to false and re-armed the `CollectionDataContext` effect, so the active untrusted collection's `defaults` (including `fieldMapping`) immediately overwrote seven just-imported fields while the UI reported success; the marker is now preserved across the replace reset. (5) imported edits typed field values as `z.unknown()` and merged them raw over the source card, which is rendered as a React child, so an "edits backup" with an object/array-valued `title` threw on every grid render and persisted across reloads and collections; the import boundary now restricts edit field values to JSON primitives (the only shape the edit form produces). 9 reproduction tests added (985 → 994). Refuted: the `?reset=1` settings wipe (a documented, in-app-surfaced self-service reset that clears only the `itemdeck-settings` key — edits/themes/sources/plugins survive — and whose substring match has no realistic colliding URL) and the unbounded `collection.json` `maxVisibleCards` (`z.number().int().positive()` already blocks the harmful non-finite/`<1`/float inputs, and the sole consumer uses the value only as a downward cap min'd against the real card count, so a large value is inert rather than a DoS). Nitpicks/defence-in-depth noted (not fixed): the `?reset=1` substring test and the missing `.max()` on the v2 `maxVisibleCards` schema are minor hardening items; plus the previously-logged raw `edits[card.id]` inherited-key lookup, dead discovery/theme-loader code, and the dev-only `migrate-collection.ts` `parseInt`→`null` / hardcoded-licence correctness bugs. Dependency review: `npm audit --omit=dev` reports 0 advisories in the production tree; no dependency was changed. - 2026-08-13 — Round 9: fixed 9 confirmed defects — (1) an untrusted `collection.json` `relationships` record is unbounded and `type.field` keyed, so a record whose keys all share the primary type's prefix collapsed into one bucket and made both the resolve pass and the per-card rank pass `O(relationships × entities)` again (the round-7 grouping bounded only the per-entity rebuild); capped at `MAX_RELATIONSHIPS`, and the single-file entity-array paths (`{type}s.json`/`{type}.json`) now share the `MAX_ENTITY_IDS` ceiling the index path already enforced, removing the entity-count multiplier. (2) `useAvailableFields` walked every key of the sampled cards and turned each into an `