diff --git a/shared/tests/static.no-sort.test.js b/shared/tests/static.no-sort.test.js new file mode 100644 index 0000000..a3d7239 --- /dev/null +++ b/shared/tests/static.no-sort.test.js @@ -0,0 +1,70 @@ +// STATIC GUARD: no `.sort(` introduced in shared/turn.js outside the ONE +// allowed function (sortParticipantsByInitiative, used by startEncounter to +// freeze the list once). Slot-not-sort design (docs/INITIATIVE_ORDERING.md): +// mutations = insert/move, never wholesale re-sort. A stray `.sort(` after +// start destroys drag tie-break order. +// +// This test errs the moment someone adds `.sort(` anywhere but the allowlist. +// Maintenance: add a function to ALLOWED only if it runs at startEncounter +// (one-time freeze), NOT in add/update/reorder/nextTurn paths. + +'use strict'; + +const fs = require('fs'); +const path = require('path'); + +const SRC = fs.readFileSync(path.join(__dirname, '..', 'turn.js'), 'utf8'); + +// Functions permitted to call .sort(. Listed so intent is explicit + reviewed. +const ALLOWED_SORT_FUNCTIONS = new Set([ + 'sortParticipantsByInitiative', +]); + +// Collect top-level function bodies by brace counting. +// Matches `function NAME(` decls AND `const NAME = ... =>` arrow fns. +// Returns [{ name, body }]. +function collectBody(src, fromIdx) { + let i = fromIdx; + while (i < src.length && src[i] !== '{') i++; + let depth = 0; + const start = i; + for (; i < src.length; i++) { + if (src[i] === '{') depth++; + else if (src[i] === '}') { depth--; if (depth === 0) break; } + } + return src.slice(start, i + 1); +} + +function extractFunctions(src) { + const out = []; + const fnRe = /\bfunction\s+([A-Za-z0-9_$]+)\s*\(/g; + const arrowRe = /(?:const|let|var)\s+([A-Za-z0-9_$]+)\s*=\s*[^=;]*?=>/g; + let m; + while ((m = fnRe.exec(src)) !== null) { + out.push({ name: m[1], body: collectBody(src, m.index + m[0].length) }); + } + while ((m = arrowRe.exec(src)) !== null) { + out.push({ name: m[1], body: collectBody(src, m.index + m[0].length) }); + } + return out; +} + +describe('STATIC: no .sort( outside allowlist (slot-not-sort design)', () => { + test('every .sort( call lives in an allowed function', () => { + const fns = extractFunctions(SRC); + const offenders = []; + for (const { name, body } of fns) { + if (!ALLOWED_SORT_FUNCTIONS.has(name) && /\.sort\(/.test(body)) { + offenders.push(name); + } + } + expect(offenders).toEqual([]); + }); + + test('allowed sort function is declared (no silent allowlist drift)', () => { + const declared = new Set(extractFunctions(SRC).map(f => f.name)); + for (const name of ALLOWED_SORT_FUNCTIONS) { + expect(declared.has(name)).toBe(true); + } + }); +}); diff --git a/shared/tests/turn.slot-not-sort.test.js b/shared/tests/turn.slot-not-sort.test.js new file mode 100644 index 0000000..4d3ec40 --- /dev/null +++ b/shared/tests/turn.slot-not-sort.test.js @@ -0,0 +1,77 @@ +// SLOT-NOT-SORT invariant: addParticipant + updateParticipant must PRESERVE +// manually-dragged tie order (same-initiative participants). +// +// Design doc (docs/INITIATIVE_ORDERING.md): +// "Re-slotting on add/edit must preserve drag-established tie order." +// "Tie-break = original add order. Later additions slot AFTER existing +// same-init participants. Stable insertion." +// +// Current code uses sortParticipantsByInitiative (stable sort, tie-break = +// original array index). That is NOT stable insertion: it re-sorts the whole +// list, destroying drag order when a drag moved a same-init pair. +// +// RED now: drag tie order survives neither add nor edit. + +'use strict'; + +const shared = require('@ttrpg/shared'); +const { + makeParticipant, + startEncounter, addParticipant, updateParticipant, reorderParticipants, +} = shared; + +function p(id, init, extra = {}) { + return makeParticipant({ id, name: id, type: 'monster', + initiative: init, maxHp: 100, currentHp: 100, ...extra }); +} +function enc(ps, extra = {}) { + return { name:'t', participants:ps, isStarted:false, isPaused:false, + round:0, currentTurnParticipantId:null, turnOrderIds:[], ...extra }; +} +const apply = (e, r) => r && r.patch ? { ...e, ...r.patch } : e; + +describe('SLOT-NOT-SORT: drag tie order survives add/edit', () => { + test('addParticipant preserves existing drag tie order', () => { + // Two same-init participants. DM drags b BEFORE a (tie override). + let e = enc([p('a', 10), p('b', 10)]); + e = apply(e, reorderParticipants(e, 'b', 'a')); // drag b ahead of a → [b,a] + expect(e.participants.map(x => x.id)).toEqual(['b', 'a']); + + // Add unrelated participant (different init). Tie order [b,a] MUST survive. + e = apply(e, addParticipant(e, p('c', 5))); + const ids = e.participants.map(x => x.id); + // c slots last (init 5 < 10). b,a tie order preserved. + expect(ids).toEqual(['b', 'a', 'c']); + }); + + test('addParticipant with same init as dragged pair slots AFTER tie group', () => { + // DM drags b before a (both init 10). Then add d also init 10. + // d must slot AFTER existing same-init pair (stable insertion), not re-sort. + let e = enc([p('a', 10), p('b', 10)]); + e = apply(e, reorderParticipants(e, 'b', 'a')); // [b,a] + e = apply(e, addParticipant(e, p('d', 10))); // init 10, after tie group + const ids = e.participants.map(x => x.id); + expect(ids).toEqual(['b', 'a', 'd']); + }); + + test('updateParticipant preserves drag tie order on unrelated field edit', () => { + let e = enc([p('a', 10), p('b', 10)]); + e = apply(e, reorderParticipants(e, 'b', 'a')); // [b,a] + // Edit a's name (not initiative). Tie order MUST survive. + e = apply(e, updateParticipant(e, 'a', { name: 'A2' })); + expect(e.participants.map(x => x.id)).toEqual(['b', 'a']); + }); + + test('updateParticipant init change slots, preserves OTHER tie group order', () => { + // Two tie groups: [a,b] init 10, [c,d] init 5. DM drags b before a. + let e = enc([p('a', 10), p('b', 10), p('c', 5), p('d', 5)]); + e = apply(e, reorderParticipants(e, 'b', 'a')); // [b,a,c,d] + // Edit c's initiative to 10 — slots into top group. [c] order within its + // own old group irrelevant (it leaves that group). But [b,a] tie MUST stay. + e = apply(e, updateParticipant(e, 'c', { initiative: 10 })); + const topTwo = e.participants.filter(x => x.initiative === 10).map(x => x.id); + // b before a preserved. c joins the 10-group (exact slot less critical + // than b,a order surviving). + expect(topTwo.indexOf('b')).toBeLessThan(topTwo.indexOf('a')); + }); +}); diff --git a/shared/turn.js b/shared/turn.js index 5c766dd..1888d0d 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -31,9 +31,18 @@ const formatInitMod = (mod) => { return mod >= 0 ? `+${mod}` : `${mod}`; }; -// Sort used ONLY at insert points (startEncounter, addParticipant) to position -// participants by initiative. Once positioned, turnOrderIds = participants.map(id) -// (1-list model). No re-sort after start — drag/edit are manual overrides. +// SLOT, NEVER SORT. 1-list model (docs/INITIATIVE_ORDERING.md). +// +// `sortParticipantsByInitiative` exists for ONE call site: startEncounter +// (freeze list once by init). After that, mutations = insert/move ops that +// PRESERVE existing array order. No wholesale re-sort ever — destroys drag +// tie-break order. +// +// slotIndexForInit: pure insert position. Scans existing list, returns index +// of first participant with init STRICTLY LESS than target. New participant +// splices there. Same-init participants stay ahead (stable). Existing tie +// order (incl. drag-established) untouched because scan reads current array, +// not original add-order. const sortParticipantsByInitiative = (participants, originalOrder) => { return [...participants].sort((a, b) => { if (a.initiative === b.initiative) { @@ -45,6 +54,17 @@ const sortParticipantsByInitiative = (participants, originalOrder) => { }); }; +// Find splice index for a single participant by initiative. List assumed +// already initiative-descending (startEncounter freezes it; add/edit maintain). +// Returns position to insert so list stays descending, ties go AFTER existing +// same-init (stable insertion). Current array order = source of truth. +function slotIndexForInit(list, init) { + for (let i = 0; i < list.length; i++) { + if (init > list[i].initiative) return i; + } + return list.length; +} + // 1-LIST SYNC: turnOrderIds always mirrors participants[].map(id). // Call after any participants[] mutation. Returns turnOrderIds patch. const syncTurnOrder = (participants) => ({ @@ -311,12 +331,12 @@ function addParticipant(encounter, participant) { if ((encounter.participants || []).some(p => p.id === participant.id)) { throw new Error(`Participant with id "${participant.id}" already exists in encounter.`); } - // FEAT-3: reslot by initiative on add (stable sort, tie-break original index). - // Pre-combat: list reflects init order immediately. Post-combat: slots into running order. - const updatedParticipants = sortParticipantsByInitiative( - [...(encounter.participants || []), participant], - encounter.participants || [] - ); + // SLOT (not sort): insert by initiative into current list. Preserves + // existing array order incl. drag-established tie order. + const base = [...(encounter.participants || [])]; + const idx = slotIndexForInit(base, participant.initiative); + base.splice(idx, 0, participant); + const updatedParticipants = base; return { patch: { participants: updatedParticipants, ...syncTurnOrder(updatedParticipants) }, log: { @@ -339,13 +359,21 @@ function addParticipants(encounter, newParticipants) { // UPDATE_PARTICIPANT — edit modal save (name/initiative/hp/isNpc). function updateParticipant(encounter, participantId, updatedData) { - const updatedParticipants = (encounter.participants || []).map(p => - p.id === participantId ? { ...p, ...updatedData } : p - ); - // FEAT-3: reslot by initiative so any init change reflects in list order - // immediately (stable sort, tie-break = original array index). - const reslotted = sortParticipantsByInitiative(updatedParticipants, encounter.participants || []); - return { patch: { participants: reslotted, ...syncTurnOrder(reslotted) }, log: null }; + const existing = encounter.participants || []; + const target = existing.find(p => p.id === participantId); + if (!target) throw new Error(`Participant "${participantId}" not found.`); + const merged = { ...target, ...updatedData }; + // SLOT only if initiative changed. Other edits (name/hp/notes/isNpc) = no + // position change (design doc). Re-slots on unrelated edits shuffle the + // list mid-round → rotation dupes. + if (!('initiative' in updatedData) || updatedData.initiative === target.initiative) { + const sameSlot = existing.map(p => p.id === participantId ? merged : p); + return { patch: { participants: sameSlot, ...syncTurnOrder(sameSlot) }, log: null }; + } + const without = existing.filter(p => p.id !== participantId); + const idx = slotIndexForInit(without, merged.initiative); + without.splice(idx, 0, merged); + return { patch: { participants: without, ...syncTurnOrder(without) }, log: null }; } // REMOVE_PARTICIPANT — verbatim from ParticipantManager.confirmDeleteParticipant