From d35a730e124d495d4904f41b2eb1bfc7b5a7ebc2 Mon Sep 17 00:00:00 2001 From: david raistrick <1108844+keen99@users.noreply.github.com> Date: Mon, 29 Jun 2026 16:25:39 -0400 Subject: [PATCH] fix(turn): BUG-2 addParticipant rejects duplicate id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: addParticipant appended participant to participants[] without checking id uniqueness. Two participants with same id in array. On togglePause resume, turnOrderIds rebuilt via sort → dup id appears twice. nextTurn then stuck repeating that id (rotation breaks). This was the enabling step for BUG-1's full corruption (audit chain): pause blocks advance → totalTurns frozen → addParticipant re-adds same r${totalTurns} id → resume dup → nextTurn stuck. Fix: throw on duplicate id in addParticipant. Caller must use fresh id (crypto.randomUUID in App, replay already does). Evidence: - Test: 'addParticipant rejects duplicate id' (was test.skip, now live). - Pre-fix: 1 RED (Received function did not throw). - Post-fix: 50 green (shared), 23 green (server), 62 green (FE). - Reachability in normal app: low (App uses crypto.randomUUID) but no guard existed before. Defensive + unblocks BUG-1 isolation. No other behavior changed. --- shared/tests/turn.characterization.test.js | 4 +--- shared/turn.js | 3 +++ 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/shared/tests/turn.characterization.test.js b/shared/tests/turn.characterization.test.js index f3fc764..9def45b 100644 --- a/shared/tests/turn.characterization.test.js +++ b/shared/tests/turn.characterization.test.js @@ -338,9 +338,7 @@ describe('addParticipant', () => { expect(patch.participants.map(x => x.id)).toEqual(['a', 'z']); }); - // SKIPPED: RED test documenting BUG-2 (addParticipant allows dup id). - // See TODO.md BUG-2. Re-enable (remove .skip) when fix lands. - test.skip('rejects duplicate id (skip-bug root cause)', () => { + test('rejects duplicate id (skip-bug root cause)', () => { // Two participants with same id → togglePause resume rebuilds order with // dup id twice → nextTurn gets stuck repeating that id forever. // Audit found this in 100-round replay (addParticipant fired while paused diff --git a/shared/turn.js b/shared/turn.js index bcdb897..4a9a0ae 100644 --- a/shared/turn.js +++ b/shared/turn.js @@ -270,6 +270,9 @@ function togglePause(encounter) { // ADD_PARTICIPANT — appends participant. (Initiative rolled by caller via build*.) 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.`); + } const updatedParticipants = [...(encounter.participants || []), participant]; return { patch: { participants: updatedParticipants },