diff --git a/CLAUDE.md b/CLAUDE.md index e061a91..12eb289 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,6 +66,17 @@ Integration — Install & Updates". loop mid-flight (FM-CAL-BACKCATALOG-FIX item 3). - System adapters implement `toChronicleFields()` / `fromChronicleFields()`. - All REST calls use Bearer token auth via `api-client.mjs`. +- **List responses come in two shapes.** Chronicle returns some list endpoints + as a bare JSON array and others wrapped in an envelope `{"data":[…],"total":N}` + (envelope: `/entities`, `/entity-types`, `/systems`, `/addons`, `/tags`, + `/relations/types`, `/calendar/events`; bare: `/maps`, `/maps/:id/*`, + `/members`, `/entities/:id/relations`, `/notes`). Every list-consuming caller + MUST unwrap defensively — accept a bare array AND `{data:[…]}` — via + `result?.data || result || []`, `_normalizeArray()`, `_coerceArray()`, or an + `Array.isArray(x) ? x : (x?.data ?? [])` guard, never assuming one shape. A + caller that consumes an envelope endpoint as a bare array is a silent no-op + (the #77 back-catalog bug). All call sites were audited under + FM-ENVELOPE-AUDIT (see `tools/test-envelope-audit.mjs`). - WebSocket messages are routed by type through `SyncManager`. - Chronicle-side serving rules live in `chronicle-package.json` at repo root; CI validates it against `module.json` via `tools/check-package-descriptor.mjs`. diff --git a/tools/test-envelope-audit.mjs b/tools/test-envelope-audit.mjs new file mode 100644 index 0000000..85cf519 --- /dev/null +++ b/tools/test-envelope-audit.mjs @@ -0,0 +1,166 @@ +#!/usr/bin/env node +/** + * Regression pins for FM-ENVELOPE-AUDIT — the follow-up sweep flagged by + * PR #77 (FM-CAL-BACKCATALOG-FIX) re-verify item 5. + * + * PR #77 fixed the calendar back-catalog silently importing nothing because + * Chronicle wraps list responses in `{"data":[…],"total":N}` while the module + * expected a bare array. This dispatch audited EVERY remaining list-consuming + * `get()` caller against the real Chronicle handler shape. The verdict: zero + * remaining mismatches — every list caller already unwraps defensively. See + * the PR body caller table for the full evidence. + * + * These tests are regression PINS, not fixes: they stub the REAL Chronicle + * response shape (envelope for `/entity-types`, `/systems`, `/addons`; bare + * array + `.data` envelope for map sub-resources) and assert that the two + * callers PR #77 explicitly flagged — plus the least-defensive envelope caller + * — still extract the list correctly. If a future refactor drops the `.data` + * unwrap at any of these sites, the corresponding test fails (verified by + * temporary revert during authoring). + * + * Consumer-verified handler shapes (Chronicle @ this session's checkout): + * /entity-types → ENVELOPE {data,total} internal/plugins/syncapi/api_handler.go:249-251 (ListEntityTypes) + * /systems → ENVELOPE {data,total} internal/plugins/syncapi/api_handler.go:1067-1069 (ListSystems) + * /addons → ENVELOPE {data,total} internal/plugins/syncapi/api_handler.go:1198-1200 (ListAddons) + * /maps/:id/* → BARE array internal/plugins/syncapi/map_api_handler.go (ListMarkers/… c.JSON of a slice) + * + * Run: `node --test tools/test-envelope-audit.mjs` + */ + +import test from 'node:test'; +import assert from 'node:assert/strict'; + +// --------------------------------------------------------------------------- +// Foundry global stubs (must be set before the first import of module code). +// Mirrors tools/test-pc-claiming.mjs so the modules import headlessly. +// --------------------------------------------------------------------------- + +globalThis.foundry = globalThis.foundry || { + applications: { + api: { ApplicationV2: class {}, HandlebarsApplicationMixin: (b) => b }, + sheets: { ActorSheetV2: class {} }, + ux: { TextEditor: { enrichHTML: async (x) => x } }, + }, + utils: { mergeObject: (a, b) => ({ ...a, ...b }) }, +}; +globalThis.CONST = globalThis.CONST || { + DOCUMENT_OWNERSHIP_LEVELS: { NONE: 0, LIMITED: 1, OBSERVER: 2, OWNER: 3 }, +}; +globalThis.Hooks = globalThis.Hooks || { on: () => {}, once: () => {}, off: () => {}, callAll: () => {} }; +globalThis.Actor = globalThis.Actor || { create: async () => ({ id: 'a1' }) }; +globalThis.ui = globalThis.ui || { + notifications: { info: () => {}, warn: () => {}, error: () => {} }, +}; +globalThis.game = globalThis.game || { + settings: { get: () => '', set: () => {}, register: () => {}, registerMenu: () => {} }, + i18n: { localize: (k) => k, format: (k) => k }, + modules: { get: () => null }, + user: { id: 'gm1', isGM: true }, + system: { id: 'dnd5e' }, + actors: { contents: [] }, + users: { get: () => null }, + journal: { find: () => null, contents: [] }, +}; + +const { ActorSync } = await import('../scripts/actor-sync.mjs'); +const { SyncManager } = await import('../scripts/sync-manager.mjs'); +const { MapSync } = await import('../scripts/map-sync.mjs'); + +/** The real Chronicle list envelope: {data:[…], total:N}. */ +const envelope = (arr) => ({ data: arr, total: arr.length }); + +// --------------------------------------------------------------------------- +// actor-sync.mjs:631 — GET /entity-types (ENVELOPE) [PR #77 flagged caller #1] +// _resolveCharacterTypeId reads `result?.data || result || []`. +// --------------------------------------------------------------------------- + +/** Build an ActorSync with a known character slug and a stubbed API. */ +function makeActorSync(getImpl) { + const s = new ActorSync(); + s._adapter = { characterTypeSlug: 'character' }; + s._api = { get: getImpl }; + return s; +} + +test('actor-sync /entity-types: REAL envelope {data,total} → character type resolves', async () => { + const s = makeActorSync(async (path) => { + assert.equal(path, '/entity-types'); + return envelope([ + { id: 7, slug: 'character', name: 'Character' }, + { id: 9, slug: 'npc', name: 'NPC' }, + ]); + }); + await s._resolveCharacterTypeId(); + assert.equal(s._characterTypeId, 7, 'must unwrap .data from the envelope and match the character type'); +}); + +test('actor-sync /entity-types: bare array (legacy shape) → still resolves', async () => { + const s = makeActorSync(async () => [{ id: 7, slug: 'character', name: 'Character' }]); + await s._resolveCharacterTypeId(); + assert.equal(s._characterTypeId, 7, 'bare-array fallback (result || []) must still work'); +}); + +test('actor-sync /entity-types: empty envelope {data:[],total:0} → no match, no throw', async () => { + const s = makeActorSync(async () => envelope([])); + await s._resolveCharacterTypeId(); + assert.ok(!s._characterTypeId, 'an empty list resolves to no character type (not a crash)'); +}); + +// --------------------------------------------------------------------------- +// sync-manager.mjs:456 — GET /systems (ENVELOPE) [least-defensive caller] +// _detectSystem reads `result.data || []` (envelope-only unwrap). +// --------------------------------------------------------------------------- + +test('sync-manager /systems: REAL envelope {data,total} → system matched by foundry_system_id', async () => { + const sm = new SyncManager(); + sm.logActivity = () => {}; + sm.api.get = async (path) => { + assert.equal(path, '/systems'); + return envelope([ + { id: 'dnd5e', name: 'D&D 5e', foundry_system_id: 'dnd5e', enabled: true }, + { id: 'pf2e', name: 'Pathfinder 2e', foundry_system_id: 'pf2e', enabled: true }, + ]); + }; + await sm._detectSystem(); + assert.equal(sm._matchedSystem, 'dnd5e', 'must unwrap .data and match on foundry_system_id'); +}); + +// --------------------------------------------------------------------------- +// sync-manager.mjs:423 — getAddons() → GET /addons (ENVELOPE) +// _fetchAddons reads `Array.isArray(addons) ? addons : (addons?.data ?? [])`. +// The existing test-pc-claiming.mjs only stubs the BARE-array shape; this pins +// the REAL envelope path. +// --------------------------------------------------------------------------- + +test('sync-manager /addons: REAL envelope {data,total} → PC-claiming addon detected', async () => { + const sm = new SyncManager(); + sm.logActivity = () => {}; + sm.api.getAddons = async () => envelope([ + { slug: 'player-character-claiming', enabled: true }, + { slug: 'calendar', enabled: true }, + ]); + await sm._fetchAddons(); + assert.ok(sm.isPcClaimingEnabled(), 'must unwrap .data from the addons envelope to see the addon'); +}); + +// --------------------------------------------------------------------------- +// map-sync.mjs:908 / :1052 — GET /maps/:id/{markers,drawings,tokens,layers} +// Sub-resources are BARE arrays; _coerceArray also tolerates a .data envelope. +// --------------------------------------------------------------------------- + +test('map-sync _coerceArray: bare array (real sub-resource shape) → passthrough', () => { + const ms = new MapSync(); + assert.deepEqual(ms._coerceArray([{ id: 'm1' }, { id: 'm2' }]), [{ id: 'm1' }, { id: 'm2' }]); +}); + +test('map-sync _coerceArray: {data:[…]} envelope → unwrapped', () => { + const ms = new MapSync(); + assert.deepEqual(ms._coerceArray(envelope([{ id: 'm1' }])), [{ id: 'm1' }]); +}); + +test('map-sync _coerceArray: null / non-array object → empty array (never throws)', () => { + const ms = new MapSync(); + assert.deepEqual(ms._coerceArray(null), []); + assert.deepEqual(ms._coerceArray(undefined), []); + assert.deepEqual(ms._coerceArray({ nope: true }), []); +});