# Independent Audit — commit `d95e5d7` (equipped-reconcile + Karma companions) **Auditor:** Opus (Protocol 8 stage 3, independent). Formed from the diff first; commit message read last. **Base:** `97188ee` → `d95e5d7` on `dev`. Tests 2980 → **2997** (+17), suites 221 → **222**, cache r10 → r11. **Both runners run green at 2997 (Node + PowerShell verified locally).** ## TOP LINE: **ACCEPT WITH FOLLOW-UPS** The core bug is genuinely and correctly fixed for the six enumerated paths. `reconcileEquipped()` is correct, cannot wrongly clear legitimately-held gear, and the on-load self-heal is **safe** (it never wipes on the boot path — see F2). The 17 new tests are **real behavioral guards**, not static filler. No gate weakening; the `test.html` fixture change is legitimate. Two honest follow-ups below — both NOTE/SHOULD-FIX, neither a blocker. --- ## FINDINGS ### F1 — MISSED REMOVAL PATH: `nativeUseItem()` does not call `reconcileEquipped()` · SHOULD-FIX (low practical risk) - **What:** The implementer's list of six removal paths omits `nativeUseItem()` (`js/ui/ui-render-inventory.js:420`). It splices an item out of inventory when a consumable's qty drains to 0, and does **not** call `reconcileEquipped()`. The commit body confirms the implementer did not know about it (lists only delItem / adjItemQty / _craftConsume / doSell / autoImportState). - **Why it (mostly) doesn't bite:** `nativeUseItem` hard-gates on `item.type === 'aid'` (line 331), and `toggleEquipItem` only accepts `weapon`/`armor` types — so a consumed item can normally never be an equipped item. `reconcileEquipped` only inspects the `weapon`/`armor` slots. The only way to produce a stale reference here is a contrived exact name-collision: a weapon "X" equipped, a separate aid item *also* named "X", the real weapon removed earlier (kept alive by the collision), then the aid "X" used to depletion. Essentially never happens in real play. - **Why still fix it:** The whole point of stage-3 is enumeration completeness. `reconcileEquipped` is idempotent and cheap; one line in the `remainingQty <= 0` branch closes the gap and makes the "every removal path" claim literally true. - **Correction:** In `nativeUseItem`, after `state.inventory.splice(idx, 1);` (line 420), add `if (reconcileEquipped(state)) renderEquipped();`, and add a behavioral test to Suite 221 (both runners, +1 each → 2999). ### F2 — OVERSTATED CLAIM: "self-heals on every load" is FALSE on the primary boot path · SHOULD-FIX (narration + coverage) - **What:** The code comment, commit body, and **user-facing CHANGELOG** all say a pre-existing stale save "self-heals the first time it's loaded" via `migrateState()`. But the normal boot path for an existing save is the **v8 fast-path** (`js/ui/ui-core.js:358-368`), which **explicitly skips `migrateState`** — its own comment says so ("the v8 fast-path skips migrateState"). It re-runs one targeted migration there (`_migrateEventLog`, line 367) but **not** `reconcileEquipped`. So a plain reload of an existing v8 save does **not** self-heal a stale equipped slot. - **Where the self-heal *does* run:** v7→v8 one-time migration, fresh boot (no data), cloud pull, file import, save-slot load — all real, but none is "just reload the app." - **Impact:** Cosmetic only, and self-limiting: the runtime removal reconcilers clear the stale slot the next time the user deletes/sells/scraps anything. No safety issue — skipping reconcile never wipes gear. The problem is the claim (especially in the CHANGELOG, which users read) is inaccurate about *when* healing happens. - **This mirrors audit #4:** an implementer overstating its own finding. Flagging per the honesty mandate. - **Correction (clean):** Add `reconcileEquipped(state)` to the v8 fast-path right beside the existing `_migrateEventLog` call (line 367) — the fast-path already runs targeted migrations there precisely because it bypasses `migrateState`, so this is the same pattern. That makes the "self-heals on load" claim true. Add a Suite guard that the fast-path reconciles. (Alternative, weaker: soften the CHANGELOG/comment wording.) ### F3 — `reconcileEquipped()` correctness · CLEAN - Item still held elsewhere with qty>0 / duplicate-name stacks: `inv.some(it => it && it.name === name)` keeps the slot if *any* entry matches — no false clear. ✓ - Empty/absent `equipped`: guarded `if (!s || !s.equipped) return false`. ✓ - Malformed inventory entry (null): `it && it.name` guard. ✓ - `headgear` deliberately excluded (AI-write-only, no backing item type) — documented and test-covered. ✓ - Case sensitivity: exact `===`. `renderEquipped()` prints the stored name verbatim with **no** inventory lookup, so the display can never desync from the reconcile check. `toggleEquipItem` stores the exact inventory name, so user-equipped items always match exactly. The only mismatch edge is pathological AI data (AI sends `equipped.weapon` in different case than the inventory name it also sent) — already-inconsistent data; clearing it is defensible. NOTE only, not a defect. - **Verdict: cannot wrongly clear gear the player legitimately holds.** ### F4 — `migrateState()` on-load self-heal SAFETY · CLEAN (no legit-gear-wipe path) - `reconcileEquipped(s)` is the last statement before `return s`. `migrateState` defaults absent `equipped` to all-null (line 23) *before* reconcile, so a legacy save with no equipped → no-op. - A save can only carry a **non-null** `equipped` if it is v1.6.4+ (when equipped was introduced); `inventory` predates equipped and is a base field, so any such save also has an `inventory` sibling. There is no secondary item store. Therefore reconcile can only clear a slot whose named item is genuinely absent from inventory — the intended heal, never legit gear. - The dangerous "runs before inventory is populated" scenario does **not** exist: every `migrateState` caller passes a fully-parsed campaign container with inventory present. And the boot path that could be most sensitive (v8 fast-path) skips `migrateState` entirely (see F2) — so it also carries zero wipe risk. ### F5 — 17 new tests are REAL · CLEAN - **Suite 221 (11 tests):** 221.1–221.5 static call-site guards; **221.6–221.9 behavioral** — they `vm.runInContext(stateSource…)` and execute the REAL `delItem` / `adjItemQty` / `_craftConsume` / `autoImportState` bodies, then assert `state.equipped` actually cleared. 221.7 specifically asserts the **false-positive guard** (qty 2→1 survivor keeps its equipped reference). 221.10 single-definition (Protocol 22); 221.11 migrate call-site. - **Suite 12 (+3):** behaviorally runs `migrateState` on a stale payload (cleared) and a valid payload (survives) — real execution. - **Suite 187 (+3, 20→23):** 187.9d static (no literals in render body), 187.9e data-present, **187.9f behavioral** — runs `renderKarmaCenter()` at each karma tier against the REAL `GAME_DEFS.FO3.karmaCompanions`. - **PowerShell parity:** the `.ps1` Suite 221 shells out to `node` and runs the identical VM behavioral tests — genuine parity, not a static stub. Both runners: 2997, 222 suites. ### F6 — `test.html` fixture change · LEGITIMATE (not a bent test) - Original Suite-4 fixture had `equipped.armor: 'Combat Armor'` with **no** Combat Armor in inventory — a genuine, incidental inconsistency. That assertion's real intent is "equipped preserved + null-headgear key-in-obj logic," **not** reconciliation. - The implementer added the backing `Combat Armor` inventory item (keeping the preservation assertion's intent intact) **and** added a separate, explicit reconcile test (Ghost Rifle/Armor cleared, Ghost Hat headgear untouched). The reconcile behavior is proven by a *new* assertion, not by weakening the old one. This is the correct way to resolve a fixture the new code exposed. ✓ ### F7 — Bug 2 / Protocol 38 · CLEAN - `karmaCompanions` lives only in `GAME_DEFS.FO3` (a per-game definition entry) — not in `let state`, `autoImportState`, `migrateState`, or the sanitizer. Protocol 4 correctly does not apply. - On-screen text is **byte-identical** for all three tiers (`COMPANIONS: <roster> available`). The new `if (companions)` wrap is a strict improvement (a future karma game with no roster omits the line rather than mis-showing FO3's). - No feature code hardcodes the companion names anymore — the only remaining occurrences are `GAME_DEFS.FO3` and `reg_fo3.js` (both per-game data files, the correct homes). ✓ ### F8 — Protocol 2a count sync · CLEAN - 2980→2997 and 221→222 synced across CLAUDE.md (all 4 sites + quick-ref), RULES.md, README.md, ARCHITECTURE.md, CHANGELOG `[Unreleased]` header, per-suite comments (187 → "23 tests" both runners, Suite 221 → "11 tests"). No stale `2980` / `221 suites` anywhere. Cache r10→r11 (served js/ changed), eslint global added, CHANGELOG entries plain-English. ✓ --- ## Answers to the orchestrator's questions - **Missed removal path?** Yes — `nativeUseItem()` (F1). Real gap in the enumeration, but benign in practice (aid-only removal can't normally touch a weapon/armor equip slot). SHOULD-FIX for completeness. - **Could it wrongly CLEAR legit gear, esp. via on-load self-heal?** No. The reconcile logic keeps a slot if any inventory entry matches; the on-load path only runs where inventory is present, and the primary boot path skips it entirely. No legit-gear-wipe path exists (F3, F4). - **`test.html` fixture: genuinely wrong, or bent to fit?** Genuinely-wrong fixture (equipped armor with no backing item), fixed the right way + covered by a separate new test (F6). - **17 tests real or filler?** Real — multiple execute the actual source in a VM (both runners), including a false-positive guard (F5). - **Overstated narration?** Yes, one: "self-heals on every load" is false on the v8 fast-path, and that claim reached the user-facing CHANGELOG (F2).