Portable campaign format: runtime-loaded JSON pipeline for all 8 campaigns - #224
Conversation
…igns Resolved the composition.ts conflict in favor of the spike's async runtime-JSON loader over main's compiled-in/positional-array approach. Since the spike had only converted 2 of 8 campaigns, extended PortableCatalog/PortableMigration to the remaining 6 (including main's new WhatWouldLuciferDo campaign), re-exported all 8, and updated test fetch stubs accordingly. Manually verified in-browser.
…, fix save/load Loading a campaign no longer requires clicking through a blocking notice dialog; any content advisory now shows inline in the briefing instead. Each campaign gets a real ?campaign= permalink that loads it directly, including on first page load. Also fixes local save/load, which was silently broken: saves were written keyed by campaignId but read back keyed by saveId, so loadGame could never find one after a reload. The UI now surfaces this with a working "Resume saved run" button. Along the way, fixed a jsdom test-environment bug where Node's own global localStorage was shadowing jsdom's with a non-functional stub, which had been masking persistence bugs in the unit test suite.
…ures Prettier had drifted on 10 files (7 exported campaign JSON files plus PlayApp.tsx, composition.ts, browser-client.test.ts) since the last `spike:export` run. Three browser-mode test files (fixtures.tsx, accessibility.browser.test.tsx, viewport.browser.test.tsx) queried for the catalog UI with a synchronous getByRole immediately after mount, which predates PlayApp's async createBrowserDemo() catalog fetch and raced the "Loading catalog..." state -- switched those to findByRole.
|
/review |
Qodo FixerNo findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR. |
| localStorage.setItem(saveKey(record.saveId), JSON.stringify(record)); | ||
| localStorage.setItem( | ||
| `subzerodev.play.save.v1.${record.campaignId}`, | ||
| JSON.stringify(record), | ||
| campaignSaveIndexKey(record.campaignId), | ||
| record.saveId, |
There was a problem hiding this comment.
2. Autosaves accumulate without bound 🐞 Bug ☼ Reliability
saves.put/localPersistence.put writes each new autosave under a fresh saveId-keyed localStorage entry and only repoints the per-campaign index, so the previously indexed full save record is never removed and accumulates forever. Because the UI saves at startup and after each successful choice (and campaigns may be restarted under new saveIds), repeated play can eventually exhaust localStorage and prevent further progress from being saved.
Agent Prompt
## Issue description
Autosaves are stored under `saveKey(saveId)` with a fresh `saveId` each time, and `campaignSaveIndexKey(campaignId)` is repointed to the newest `saveId` without removing the previously indexed save record. This leaves orphaned save entries in localStorage that accumulate over repeated play/restart cycles and frequent in-run saves, eventually exhausting localStorage and breaking progress saving.
## Issue Context
- `saveKey(saveId)` holds the full `StoredSaveRecord`, while `campaignSaveIndexKey(campaignId)` holds only the most recent `saveId` used by `findLocalSave` to show a Resume option.
- `put()` writes the new record and updates the index but does not delete the old `saveKey(oldSaveId)` record, so the old record becomes unreachable once the index moves.
- `delete(id)` only clears the index if it currently points at `id`, so it cannot remove older records after a subsequent `put()` has repointed the index.
- The session/store layer generates a new id and serialized envelope on each save call, and the UI triggers saves at startup and after each successful choice, so the leak happens quickly in normal usage.
- Implement the update safely: write the new record and index, then delete the superseded record after the replacement succeeds, while handling partial storage failures so an existing valid checkpoint is not lost.
## Fix Focus Areas
- site/src/play/composition.ts[53-72]
- site/src/play/composition.ts[57-63]
- site/src/play/composition.ts[64-72]
- site/src/play/PlayApp.tsx[193-271]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| "lint": "eslint src scripts", | ||
| "demo": "tsx scripts/demo-cli.ts" | ||
| "demo": "tsx scripts/demo-cli.ts", | ||
| "spike:export": "tsx scripts/spike-export-campaigns.ts" |
There was a problem hiding this comment.
3. Campaign artifacts can drift 🐞 Bug ⚙ Maintainability
The exporter is added only as a standalone command; the repository's cited site build/check scripts neither run it nor compare regenerated output with the checked-in JSON. A TypeScript campaign or migration edit can therefore pass those gates while production continues serving stale runtime artifacts.
Agent Prompt
## Issue description
Runtime campaign JSON can diverge from the TypeScript campaign builders because export is not enforced by the build or checks. Add a deterministic generation/verification gate.
## Issue Context
Either generate before site build or regenerate in CI and fail when the working tree differs. Keep the checked-in-artifact strategy if desired, but make staleness detectable automatically.
## Fix Focus Areas
- src/engine/package.json[19-27]
- src/engine/scripts/spike-export-campaigns.ts[48-71]
- site/package.json[6-18]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const response = await fetch(`${base}campaigns/${path}`); | ||
| if (!response.ok) | ||
| throw new Error(`Failed to load ${path}: ${response.status}`); | ||
| return (await response.json()) as T; |
There was a problem hiding this comment.
4. Format versions are ignored 🐞 Bug ≡ Correctness
fetchJson only casts network JSON to the versioned portable types, and neither loader checks the manifest or campaign formatVersion. Incompatible artifacts are therefore passed into current v1 hydration and fail later unpredictably or may be accidentally accepted instead of receiving a deterministic compatibility rejection.
Agent Prompt
## Issue description
Runtime JSON is asserted to versioned TypeScript types without validating the version stamps. Reject unsupported manifest and campaign versions explicitly before dereferencing or hydrating their fields.
## Issue Context
TypeScript literal types do not validate fetched JSON. Add runtime shape/version validation with a clear incompatible-format error for both artifact levels.
## Fix Focus Areas
- site/src/play/composition.ts[104-130]
- src/engine/src/spike/portable.ts[54-80]
- src/engine/src/spike/portable.ts[186-213]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| async function loadPortableCampaigns(): Promise<readonly PortableCampaign[]> { | ||
| const manifest = await fetchJson<PortableManifest>("manifest.json"); | ||
| return Promise.all( | ||
| manifest.campaigns.map((fileName) => fetchJson<PortableCampaign>(fileName)), | ||
| ); | ||
| } |
There was a problem hiding this comment.
5. Manifest fetch failure fails entire catalog 🐞 Bug ☼ Reliability
loadPortableCampaigns() fetches all campaign JSON files in parallel via Promise.all, so if any one campaign 404s, has a non-OK response, is malformed, or is temporarily unavailable, the entire catalog load rejects and PlayApp renders a full-page error. This turns a single independent asset failure into a total outage, removing access to otherwise valid campaigns and offering no in-app retry or per-campaign failure handling.
Agent Prompt
## Issue description
`loadPortableCampaigns` uses `Promise.all` to fetch all campaign JSON files listed in `manifest.json`, so a failure in any single campaign fetch (404, network error, non-OK response, malformed JSON) rejects the entire catalog load. Update the loading pipeline to preserve and hydrate the successfully loaded campaigns, isolate/report per-campaign failures, and/or provide a retry path, instead of turning one campaign file problem into a global outage.
## Issue Context
`createBrowserDemo()` calls `loadPortableCampaigns()` and hydrates each returned `PortableCampaign` via `fromPortable`. `PlayApp.tsx` calls `createBrowserDemo()` inside a `useEffect` and currently shows a blocking full-page error UI when the promise rejects. While a manifest fetch failure may remain a global failure, individual campaign-file failures can be isolated using settled results (e.g., all-settled style handling) with explicit reporting, and the registry/demo construction should only receive successfully hydrated campaigns.
## Fix Focus Areas
- site/src/play/composition.ts[104-140]
- site/src/play/PlayApp.tsx[101-129]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const bulgariaBureaucracyMigration: PortableMigration = { | ||
| fromVersion: "1.0.0", | ||
| nodeMap: { | ||
| clerk_review: "registry_route_event_1", | ||
| expired: "registry_route_1", | ||
| room_14: "registry_route_event_2", | ||
| room_6: "registry_route_3", | ||
| reward: "ending_ultimate_reward", | ||
| }, | ||
| }; | ||
| export { bulgariaBureaucracyMigration }; |
There was a problem hiding this comment.
6. Adventure campaigns migration duplicated, drift risk 🐞 Bug ⚙ Maintainability
Each Bulgaria adventure campaign (e.g. bulgaria-bureaucracy.ts) now defines its nodeMap/endingMap twice — once as an exported PortableMigration constant for the export script, and once inline inside buildBulgariaBureaucracyCampaign's migrateState closure — so the two copies can silently diverge if one is edited without the other, defeating the single-source-of-truth intent the PR states for PortableMigration.
Agent Prompt
## Issue description
In each Bulgaria adventure campaign file (e.g. `bulgaria-bureaucracy.ts`, and similarly `bulgaria-return.ts`, `bulgaria-driving.ts`, `bulgaria-inheritance.ts`, `bulgaria-enterprise.ts`), the `nodeMap`/`endingMap` id-remap tables used for v1→v2 state migration are now declared twice: once as an exported `PortableMigration` object consumed by `spike-export-campaigns.ts`, and once more as an inline object literal inside `buildXxxCampaign`'s `migrateState` assignment. These two copies must be kept in sync by hand, and nothing enforces that; a future edit to one and not the other silently changes behavior only for players on one of the two loading paths (compiled-in vs. portable JSON).
## Issue Context
`PortableMigration`'s stated purpose (`src/engine/src/spike/portable.ts`) is to be the single place these remap tables live. The inline duplicate in each campaign's `buildXxxCampaign` function undermines that.
## Fix Focus Areas
- src/engine/src/campaigns/bulgaria-bureaucracy.ts[19-29]
- src/engine/src/campaigns/bulgaria-bureaucracy.ts[100-112]
- src/engine/src/campaigns/bulgaria-return.ts
- src/engine/src/campaigns/bulgaria-driving.ts
- src/engine/src/campaigns/bulgaria-inheritance.ts
- src/engine/src/campaigns/bulgaria-enterprise.ts
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…shipped code; track W41-W60 Four reconciliation findings against built code the specs had not caught up to: RecordIdSource (S1, commit 42f34f6) had no design/ entry at all; the portable campaign format (PR #224) shipped as a load-bearing runtime-fetch mechanism while docs still claimed no runtime network request; BASE_REASON_CODES and ContentRegistry.resolution disagreed with registry/packs.ts on three points; and four statements across 07-replay.md/10-design.md/30-slices.md described shipped machinery (createSessionLayer, ExperimentSource) as unbuilt. Each is recorded in 90-decisions.md with why the code stayed and the documents moved. Also completes /track's slice-ledger sync: W66 ticked to match its closed issue (#209); W41-W49 converted from bullet items to enumerable ### headings, matching every other delivered unit, so they are no longer invisible to issue-enumeration tooling (#214); W70 added as the missing gate the portable-format finding names.
…equence (#243) * Export the W59 experiment surface, and fix `__proto__` as an experimentId The `ExperimentSource` port and the three gate functions were unreachable: nothing in the session layer reads `SessionHost.experiments`, and none of them was exported. `10-design.md` §5.5 puts the composition they serve — one registry per distinct assignment combination — above the session seam, so the host is the intended caller and could not name them. `applyExperimentGates` needed both an `Object.hasOwn` guard and a null-prototype assignment map, not either alone: on an object literal `assignments["__proto__"] = variant` hits the inherited accessor and stores nothing, so a real assignment vanished between being resolved and being read. Fail-closed, but a gate that silently ignores a real assignment is still a wrong answer. `EmittedRecord.experiments` stays unbuilt, and 90-decisions.md now records why rather than leaving it to be rediscovered: §5.5 resolves packs host-side while 05 §6 has the store stamping the map, and the store receives a resolved registry, never the candidate packs. Settling that changes `10-design.md` §4 and is not an implementation PR's call. * Pin §5a's gated-dependsOn consequence, and mark W56-W60 done 11 §5a spends a paragraph on what filtering *before* dependency resolution means: a pack excluded by its gate is simply absent, so a `dependsOn` naming it fails as an ordinary §7 Tier 1 `pack_dependency_missing` at session creation rather than at authoring time. That was the one behavioural claim §5a makes with nothing holding it, and it is the claim most likely to be "fixed" later by someone adding a gate-aware dependency check the design deliberately does not want. The test asserts both directions — the same pair resolves cleanly when the gate selects the dependency. The ledger had four merged units still reading `[ ]` / Not started: W56 (#236), W58 (#238), W59 (#239) and W60 (#240). Fixing only the newest would have made the other three look deliberate, so all four are marked together. W57 stays open — it is genuinely blocked on the `week_limit_reached` precedence decision. * Reconcile session-id, portable-format, and content-pack docs against shipped code; track W41-W60 Four reconciliation findings against built code the specs had not caught up to: RecordIdSource (S1, commit 42f34f6) had no design/ entry at all; the portable campaign format (PR #224) shipped as a load-bearing runtime-fetch mechanism while docs still claimed no runtime network request; BASE_REASON_CODES and ContentRegistry.resolution disagreed with registry/packs.ts on three points; and four statements across 07-replay.md/10-design.md/30-slices.md described shipped machinery (createSessionLayer, ExperimentSource) as unbuilt. Each is recorded in 90-decisions.md with why the code stayed and the documents moved. Also completes /track's slice-ledger sync: W66 ticked to match its closed issue (#209); W41-W49 converted from bullet items to enumerable ### headings, matching every other delivered unit, so they are no longer invisible to issue-enumeration tooling (#214); W70 added as the missing gate the portable-format finding names. * Regenerate stale TODO.md pages after the W41-W60 ledger edit design/30-slices.md changed (W66 ticked, W41-W49 converted to headings) without re-running the generator, leaving engine/TODO.md and its design/TODO.md compatibility pointer stale. build/Test-Documentation.ps1 catches exactly this. * Record the null-prototype assignments map as a known consumer-facing edge Review found the shape resolveExperimentAssignments returns is honest but sharp: no Object.prototype, so hasOwnProperty or string coercion on it throws where an ordinary Record would not. Retained rather than fixed — the null prototype is the mechanism that makes __proto__ a plain key — with the spread-copy alternative named in place should a consumer ever need an ordinary object.
…ments edge * Export the W59 experiment surface, and fix `__proto__` as an experimentId The `ExperimentSource` port and the three gate functions were unreachable: nothing in the session layer reads `SessionHost.experiments`, and none of them was exported. `10-design.md` §5.5 puts the composition they serve — one registry per distinct assignment combination — above the session seam, so the host is the intended caller and could not name them. `applyExperimentGates` needed both an `Object.hasOwn` guard and a null-prototype assignment map, not either alone: on an object literal `assignments["__proto__"] = variant` hits the inherited accessor and stores nothing, so a real assignment vanished between being resolved and being read. Fail-closed, but a gate that silently ignores a real assignment is still a wrong answer. `EmittedRecord.experiments` stays unbuilt, and 90-decisions.md now records why rather than leaving it to be rediscovered: §5.5 resolves packs host-side while 05 §6 has the store stamping the map, and the store receives a resolved registry, never the candidate packs. Settling that changes `10-design.md` §4 and is not an implementation PR's call. * Pin §5a's gated-dependsOn consequence, and mark W56-W60 done 11 §5a spends a paragraph on what filtering *before* dependency resolution means: a pack excluded by its gate is simply absent, so a `dependsOn` naming it fails as an ordinary §7 Tier 1 `pack_dependency_missing` at session creation rather than at authoring time. That was the one behavioural claim §5a makes with nothing holding it, and it is the claim most likely to be "fixed" later by someone adding a gate-aware dependency check the design deliberately does not want. The test asserts both directions — the same pair resolves cleanly when the gate selects the dependency. The ledger had four merged units still reading `[ ]` / Not started: W56 (#236), W58 (#238), W59 (#239) and W60 (#240). Fixing only the newest would have made the other three look deliberate, so all four are marked together. W57 stays open — it is genuinely blocked on the `week_limit_reached` precedence decision. * Reconcile session-id, portable-format, and content-pack docs against shipped code; track W41-W60 Four reconciliation findings against built code the specs had not caught up to: RecordIdSource (S1, commit 42f34f6) had no design/ entry at all; the portable campaign format (PR #224) shipped as a load-bearing runtime-fetch mechanism while docs still claimed no runtime network request; BASE_REASON_CODES and ContentRegistry.resolution disagreed with registry/packs.ts on three points; and four statements across 07-replay.md/10-design.md/30-slices.md described shipped machinery (createSessionLayer, ExperimentSource) as unbuilt. Each is recorded in 90-decisions.md with why the code stayed and the documents moved. Also completes /track's slice-ledger sync: W66 ticked to match its closed issue (#209); W41-W49 converted from bullet items to enumerable ### headings, matching every other delivered unit, so they are no longer invisible to issue-enumeration tooling (#214); W70 added as the missing gate the portable-format finding names. * Regenerate stale TODO.md pages after the W41-W60 ledger edit design/30-slices.md changed (W66 ticked, W41-W49 converted to headings) without re-running the generator, leaving engine/TODO.md and its design/TODO.md compatibility pointer stale. build/Test-Documentation.ps1 catches exactly this. * Record the null-prototype assignments map as a known consumer-facing edge Review found the shape resolveExperimentAssignments returns is honest but sharp: no Object.prototype, so hasOwnProperty or string coercion on it throws where an ordinary Record would not. Retained rather than fixed — the null prototype is the mechanism that makes __proto__ a plain key — with the spread-copy alternative named in place should a consumer ever need an ordinary object. * Remove reason-code open item now tracked as issue #245 /track found the bullet still in 90-decisions.md's Open register even though it was already moved to a GitHub issue in an earlier pass — the completion step (remove the bullet once tracked) never ran. Regenerated OPEN-QUESTIONS.md to match.
What changed, and why. Graduates the
spike/authoring-and-playwork (plans/spike-notes.md)onto this branch: campaigns are now runtime-loaded JSON under
site/public/campaigns/, generatedby
spike-export-campaigns.ts, instead of compiled into the engine package via a positionaldescriptions[]array that was silently index-coupled tobuilt[]. All 8 story campaigns nowexport through the same
toPortable/fromPortablepipeline. This commit also removes thecontent-notice click-through, adds permanent per-campaign links, and fixes save/load. On top of
that work, this PR fixes: a stale
src/engine/dist/build that predatedfromPortable's export(caused a typecheck failure), Prettier drift across 10 files, and a real race in three browser-mode
test fixtures that queried the catalog UI synchronously right after mount --
PlayApp's catalogload is now async (
createBrowserDemo()fetchesmanifest.json), so those fixtures nowfindByRoleinstead ofgetByRole.Verified
cd src/engine && npm run typecheck && npm run lint && npm test-- passed (730/730)cd site && npm run check(format, lint, typecheck, unit tests, real-Chromium browser tests, production build) -- passed (43 unit + 52 browser tests)./build/Test-Documentation.ps1-- passed (18 generated pages, 124 Markdown files)git diff --check-- cleanAgent detail
Not a
/slice-tracked work unit -- this is a spike graduation (plans/spike-notes.md), mergedonto
mainand extended to all 8 campaigns across several prior commits on this branch. Nodesign/files were touched; the portable-JSON loader lands on the existingcore/registry/build.tsauthoring -> registry boundary. Open questions the spike notes flaggedfor reconcile (whether
PortableMigration's two-table shape generalizes, wheretoPortableultimately lives, wiring
spike:exportinto the build) are not resolved by this PR.PR Summary by Qodo
Runtime-loaded JSON campaigns via portable pipeline (all 8 campaigns)
✨ Enhancement🐞 Bug fix🧪 Tests📝 Documentation⚙️ Configuration changes🕐 40+ MinutesAI Description
Diagram
graph TD A["TS campaign authoring"] --> B["spike-export-campaigns.ts"] --> C["Campaign JSON + manifest"] --> D["createBrowserDemo() loader"] --> E["fromPortable()"] --> F["Validated registry + engine"] D --> G[("LocalStorage saves")]High-Level Assessment
1. Bundle-time JSON via Vite `import.meta.glob` (no runtime fetch)
2. Generated TypeScript manifest (build step emits `campaignManifest.ts`)
manifest.json) if inlined into code.3. Add schema/version validation before `fromPortable` hydration
formatVersion.Files changed (31)
+18896 / -383Enhancement (21)
+18427 / -283bulgaria-bureaucracy.json
Add exported portable JSON for Bulgaria: Bureaucracy+1119/-0bulgaria-driving.json
Add exported portable JSON for Bulgaria: Driving+1108/-0bulgaria-enterprise.json
Add exported portable JSON for Bulgaria: Enterprise+1114/-0bulgaria-inheritance.json
Add exported portable JSON for Bulgaria: Inheritance+1125/-0bulgaria-return.json
Add exported portable JSON for Bulgaria: Return+1089/-0lucifer-chronicles.json
Add exported portable JSON for Lucifer Chronicles+6067/-0saki-quest-for-redemption.json
Add exported portable JSON for Saki: Quest for Redemption+2624/-0what-would-lucifer-do.json
Add exported portable JSON for What Would Lucifer Do?+3581/-0PlayApp.tsx
Make PlayApp async-load the catalog; add permalinks and resume flow+114/-126composition.ts
Replace compiled-in campaign build with runtime portable JSON loader; fix save keying+87/-115play.css
Style inline advisory, briefing actions, and permalink; remove notice dialog styles+24/-34bulgaria-bureaucracy.ts
Embed portable catalog metadata and migration tables for Bureaucracy+23/-0bulgaria-driving.ts
Embed portable catalog metadata and migration tables for Driving+21/-0bulgaria-enterprise.ts
Embed portable catalog metadata and migration tables for Enterprise+20/-0bulgaria-inheritance.ts
Embed portable catalog metadata and migration tables for Inheritance+27/-0bulgaria-return.ts
Embed portable catalog metadata and migration tables for Return+16/-0lucifer-chronicles.ts
Embed portable catalog metadata and migration tables for Lucifer Chronicles+26/-8saki-quest-for-redemption.ts
Embed portable catalog metadata for Saki (hidden campaign)+12/-0what-would-lucifer-do.ts
Embed portable catalog metadata for featured campaign+12/-0index.ts
Export 'fromPortable' and portable types from engine package+4/-0portable.ts
Introduce portable campaign format with 'toPortable'/'fromPortable' and migration reattachment+214/-0Tests (6)
+213 / -99PlayApp.test.tsx
Update PlayApp tests for async catalog fetch and no notice dialog+106/-61browser-client.test.ts
Stub runtime campaign fetch and await async demo creation in client tests+62/-20accessibility.browser.test.tsx
Adjust accessibility browser test for async shelf rendering and notice removal+5/-14fixtures.tsx
Fix browser test fixtures to wait for async campaign catalog mount+3/-2viewport.browser.test.tsx
Wait for async catalog load in viewport browser test+1/-1setup.ts
Install working jsdom localStorage to avoid Node 22 global stub+36/-1Documentation (1)
+164 / -0spike-notes.md
Document portable campaign spike rationale and pipeline+164/-0Other (3)
+92 / -1manifest.json
Add campaign manifest for runtime catalog loading+13/-0package.json
Add 'spike:export' script for portable campaign JSON generation+2/-1spike-export-campaigns.ts
Add author-time exporter for portable campaign JSON + manifest+77/-0