# TEST_STRENGTH_AUDIT.md — RobCo U.O.S. v2.6.0 > **FILED UNDER 2.8.0 — PRODUCED-BY, NOT MEASURED-VERSION.** This document *measures* the earlier v2.6.0/v2.7.0 > codebase, but it was *produced* as a stage of the PARKED-CHAIN audit sequence — the analysis phase that became > the 2.8.0 overhaul. Planning artifacts are filed by the release whose work produced them, never by the version > they discuss. The version in the title is the code it audited, not the folder it belongs in. > **[ARCHIVE-class snapshot — not current truth.]** Per the 2.8.5 U-B1 doc-maintenance model (see `CLAUDE.md`'s > Reference Pointer Index), this doc is frozen as of the date below and is never updated to track current code. It > audited a codebase roughly 30% smaller than today's (pre-`test-console.js`, pre-OCR, pre-Diagnostic-Shell, > pre-CHASSIS/Overseer). Reuse its structural reasoning as historical input only — re-verify every measurement and > file-split proposal against current code before acting on it. > Stage A6 of the PARKED-CHAIN audit sequence. PLAN-ONLY. Nothing here is implemented or committed. > Companion docs: HOUSE_STANDARD.md (A0), DIEGETIC_AUDIT.md (A0), PERFORMANCE_AUDIT.md (A1), > ACCESSIBILITY_AUDIT.md (A2), CONTENT_AUDIT.md (A3), MOBILE_AUDIT.md (A4), CODE_QUALITY_AUDIT.md (A5). > Target: bc8c2eb (v2.6.0-r1, 1078 tests / 89 suites). > > Test files: check-persistence.js (8,791 lines) · check-persistence.ps1 (5,209 lines) · gate.js (133 lines) --- ## Executive summary The suite is **architecturally well-structured** — it auto-discovers state keys, has real VM-sandbox behavioral tests in Suites 2b, 12, 44, 51, and 84, and the gate enforces runner parity at every push. The primary weakness class is **presence-only guards on data-safety and recovery paths**: the functions exist, but the transformations they perform are never exercised with real inputs and the outputs never asserted. The three most dangerous gaps: 1. **Kill-switch fail-open (Suite 48)** — Protocol 33's core guarantee ("if the config is unreachable, the app must remain fully usable") is backed by zero behavioral tests. A catch block that silently disabled all features would pass all 11 assertions. 2. **Save integrity recovery paths (Suite 51)** — `restoreRollingBackup` and the `checksum_mismatch` consumer path in `loadFromSlot` / `handleFileUpload` are presence-checked but never executed. A regression where the backup is fetched but never applied would pass every test. 3. **Suite 23 explicit carve-out for api.js** — The prohibited-pattern suite explicitly excludes `api.js` from the `innerHTML +=` guard with a comment calling it "intentional." A5 confirmed it IS in a loop and IS the O(n²) banned pattern. The escape valve that lets this through is documented in the test file itself. --- ## Summary counts | Severity | Count | |---|---| | **CRITICAL** | 2 | | **HIGH** | 12 | | **MEDIUM** | 9 | | **LOW** | 5 | | **Gate holes (escape-ratchet candidates)** | 3 | | **Total findings** | **31** | --- ## Section 1 — GATE STRUCTURE The gate runs in two modes (`gate:fast` at commit, full `gate` at push/CI): | Step | Mode | Tool | |---|---|---| | ESLint `--max-warnings 0` | Both | `npx eslint .` | | Prettier format check | Both | `npx prettier --check .` | | Node persistence audit | Both | `node tests/check-persistence.js` | | PowerShell persistence audit + parity check | Both | `pwsh/powershell tests/check-persistence.ps1` | | Playwright Chromium availability | Push only | inline script | | Boot smoke (HTTP) | Push only | `tests/boot-smoke.mjs` | | Render check (360px & 412px) | Push only | `tests/render-check.mjs` | | A11y check (axe serious/critical) | Push only | `tests/a11y-check.mjs` | **Parity check:** `gate.js` runs both runners and compares the `ALL N TESTS` total. A mismatch blocks the push. This correctly enforces Protocol 15 at the gate boundary. ✓ **PS fallback:** When neither `pwsh` nor `powershell` is found on a dev machine, the gate logs a warning and skips PS (non-CI). In CI, missing `pwsh` is a hard failure. This is correct — CI always enforces parity. ✓ --- ## Section 2 — GATE HOLES (Escape-Ratchet Candidates) These are cases where a real violation slipped through the gate — exactly the Protocol 36(b) ratchet trigger. --- ### GH-1 · [CRITICAL] · Suite 23 explicitly carves api.js out of the `innerHTML +=` guard **check-persistence.js:1341–1344:** ```js // Note: api.js has a known, intentional innerHTML+= in the model-fetch <select> builder // (not a render hot-path), so the check is scoped to ui.js only. assert(!/innerHTML\s*\+=/.test(uiSource), 'ui.js has no innerHTML += (O(n²) re-parse guard)'); ``` This is the exact escape valve that let **A5 QA-PROHIB-1** (api.js:257, `innerHTML +=` inside `.forEach`) pass all 1078 tests. The comment labels the violation "intentional" and "not a render hot-path" — but A5 confirmed it IS inside a `.forEach` loop and IS the O(n²) banned pattern. **The ratchet fix:** Remove the carve-out comment and extend the check to all served JS files: ```js // No innerHTML += anywhere in served JS (O(n²) DOM re-parse) const allSrc = uiSource + '\n' + apiSource + '\n' + cloudSource; assert(!/innerHTML\s*\+=/.test(allSrc), 'No served JS file has innerHTML += (O(n²) guard)'); ``` When A5 QA-PROHIB-1 is fixed (replace with single `innerHTML = arr.join('')`), this extended guard will stay green and prevent it from coming back. **Verdict:** BUILD-NOW (fix paired with A5 QA-PROHIB-1) · **CRITICAL** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### GH-2 · [HIGH] · No guard for unescaped `localStorage`/Firestore values in `innerHTML` assignments **A5 QA-PROHIB-2** (ui-core.js:158) and **QA-PROHIB-3** (cloud.js:586) both assign user-controlled strings directly into `innerHTML` without `escapeHtml()`. Suite 21 guards three specific XSS regressions (XSS-1/XSS-2/XSS-3) with targeted grep patterns, but there is no general guard that would catch a NEW unescaped innerHTML assignment. The existing XSS guards are: - XSS-1: `eval(`, `new Function(` absent - XSS-2: `document.write(` absent - XSS-3: `JSON.parse` result is never used as `innerHTML` without escaping None of these would catch `innerHTML = userString` where `userString` came from localStorage or Firestore. **Ratchet fix:** Add a guard that scans all `.innerHTML =` assignments in all served JS files and asserts the RHS is one of: (a) a string literal, (b) `escapeHtml(...)`, (c) a `.join('')` result from a `.map()` of known-safe template literals. This requires code-analysis, not just regex. A pragmatic version: scan for `.innerHTML\s*=\s*[^`]` patterns where the RHS is NOT `''`, NOT a literal string, and NOT immediately preceded by `escapeHtml(` — then assert count is 0. **Verdict:** BUILD-NOW (add after A5 QA-PROHIB-2/3 are fixed) · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### GH-3 · [HIGH] · No guard for `localStorage.getItem()` in `getSystemDirective()` / `transmitMessage()` **A5 QA-PROHIB-4/5** found `localStorage.getItem()` calls inside both `getSystemDirective()` (called on every AI transmit) and `transmitMessage()`. Suite 23.2 guards against `localStorage.getItem` in audio functions only — not in the AI transmission hot path. **Ratchet fix:** Add a check that the extracted bodies of `getSystemDirective()` and `transmitMessage()` contain no `localStorage.getItem` calls. **Verdict:** BUILD-NOW (add after A5 PROHIB-4/5 are fixed) · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ## Section 3 — PRESENCE-VS-BEHAVIORAL GAPS These are tests that verify a function exists or a string appears in source, but never exercise the function with inputs and check the output. A behavioral failure would pass all of them. --- ### TS-GAP-1 · [CRITICAL] · Suite 48 kill-switch fail-open is never behaviorally tested **Suite 48** (11 tests, all PRESENCE): Every test greps source for strings — `loadRemoteConfig`, `Promise.race`, `setTimeout`, `try.*catch`, `!== false`. Not one test actually calls any of these functions. Protocol 33 says: "reading the remote kill-switch on boot must never disable features or black-screen the app if the config is unreachable." This is the most safety-critical invariant in the entire kill-switch system — and it is backed by zero behavioral checks. A developer could change the catch block from `return` to `disableAllFeatures()` and all 11 tests would still pass. **Proposed behavioral tests (3 new tests, both runners):** ```js // 48-B1: loadRemoteConfig with rejected Firestore call returns, features remain enabled { const vm = require('vm'); const ctx = { /* mock cloud.js deps */ }; // Mock getDoc to reject with network error ctx.getDoc = () => Promise.reject(new Error('Network failure')); ctx.isFeatureEnabled = /* loaded from cloud.js */; // After failed config load, aiChat must still be enabled (fail-open) await loadRemoteConfig.call(ctx); assert(ctx.isFeatureEnabled('aiChat') !== false, 'fail-open: aiChat enabled after config fetch failure'); } // 48-B2: isFeatureEnabled with never-set key returns true (fail-open default) assert(isFeatureEnabled('unknownFeatureKey2025') !== false, 'isFeatureEnabled returns true for unknown key (fail-open default)'); // 48-B3: loadRemoteConfig resolves when getDoc never settles (timeout path) // Timeout race: mock getDoc that never resolves; loadRemoteConfig must settle within 5s ``` **Verdict:** BUILD-NOW · Both runners · **CRITICAL** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-2 · [HIGH] · Suite 51 `restoreRollingBackup` is never called in the VM sandbox Suite 51 has excellent behavioral tests for `snapRollingBackup`, `getRollingBackups`, `computeSaveChecksum`, and `verifySaveEnvelope` — but `restoreRollingBackup` is only presence-checked (tests 51.16, 51.36 grep the function body for string fragments). The actual execution path — "user confirms → backup data fetched from ring → sanitized → migrated → applied to live state" — is never exercised. A regression where `sanitizeImportedContainer` is called but its return value is silently ignored (state not updated) would pass all assertions. **Proposed behavioral test:** ```js // 51-B1: restoreRollingBackup actually applies backup state to live storage // Seed a backup with a specific state; mock confirm() to return true; call restoreRollingBackup; // assert localStorage('robco_v8') now contains the backup's robco_v8 payload ``` **Verdict:** BUILD-NOW · Both runners · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-3 · [HIGH] · Suite 51 `checksum_mismatch` consumer path in `loadFromSlot` never tested Suite 51.23 correctly behavioral-tests `verifySaveEnvelope` returning `'checksum_mismatch'` when the envelope is tampered. But test 51.10 only checks that `verifySaveEnvelope` is *called* inside `loadFromSlot` (presence). The question "what does `loadFromSlot` actually DO when it gets `'checksum_mismatch'` back?" is never answered by any test. If `loadFromSlot` ignored the return value and applied state anyway on mismatch, test 51.23 (which only tests the helper) and test 51.10 (which only checks the call exists) would both still pass. **Proposed behavioral test:** ```js // 51-B2: loadFromSlot with tampered checksum does NOT apply state without confirmation // Seed slot 1 with a tampered envelope (flip one char in checksum); // call loadFromSlot(1) with confirm() returning false; // assert state is unchanged (no overwrite without confirmation) ``` **Verdict:** BUILD-NOW · Both runners · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-4 · [HIGH] · Suite 4/5 exportSaveFile and handleFileUpload never executed **Suite 4** (4 tests) and **Suite 5** (4 tests): All 8 tests grep extracted function bodies for string fragments. Neither function is ever called with real inputs; no save file is ever built; no file is ever "imported." For Suite 4 specifically: if `computeSaveChecksum` were passed incorrect arguments (e.g., missing `playstyle`), the exported checksum would be wrong and the file would fail `verifySaveEnvelope` on re-import. No test catches this. **Proposed behavioral test:** ```js // 4-B1: export→re-import round-trip returns verifySaveEnvelope 'ok' // In the Suite 51 VM: simulate exportSaveFile for a known state, parse the JSON, // call verifySaveEnvelope on it, assert status === 'ok' ``` **Verdict:** BUILD-NOW · Both runners · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-5 · [HIGH] · Suite 46 `syncLocalSavesToCloud` dedup never executed Suite 46.7–46.9 parse the function body and check for structural patterns (no `setDoc`, `contentHash` present, dedup loop present). But the dedup invariant — "a second sync with the same content hash is a no-op, never calls `addDoc`" — is never actually exercised. The content-hash dedup is the entire guarantee against duplicate cloud saves accumulating indefinitely. It is never tested at runtime. **Proposed behavioral test:** ```js // 46-B1: syncLocalSavesToCloud with duplicate contentHash does not call addDoc // Mock getDocs to return one existing doc with matching contentHash; // call syncLocalSavesToCloud; assert addDoc was NOT called ``` **Verdict:** BUILD-NOW · Both runners · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-6 · [HIGH] · Suite 46 `loadCloudSave` sanitize+migrate round-trip never executed Suite 46.10–46.12 check that `confirm(`, `sanitizeImportedContainer`, and `migrateState` appear in the `loadCloudSave` function body. The function is never called with a mock cloud document. A cloud document containing a malicious payload in a quest name (e.g., `<script>alert(1)</script>`) would never be caught by the test suite — the XSS guard only checks the storage layer, not the load path. **Proposed behavioral test:** ```js // 46-B2: loadCloudSave with XSS payload in quest name sanitizes before applying // Mock Firestore doc with state.quests[0].name = '<img src=x onerror=alert(1)>'; // call loadCloudSave with confirm() returning true; // assert state.quests[0].name does NOT contain '<' or '>' // (sanitizeImportedContainer must have run) ``` **Verdict:** BUILD-NOW · Both runners · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-7 · [HIGH] · Suite 1 autoImportState never called with a test payload Suite 1 is a `\bkey\b` word-boundary regex scan over the extracted `autoImportState()` function body. It verifies each state key string *appears* in the function — but the function is never instantiated, never called, and no output is ever asserted. A field like `collectibles` could appear in the body as part of `_collectNames.has(c)` (a validation set) while the actual line that writes to `state.collectibles` was accidentally deleted. Suite 1 passes. Suite 76 (autoImportState hardening) addresses a few specific fields with more structural checks, but it is not a general field-mapping validator. **Proposed behavioral test:** ```js // 1-B1: autoImportState with a synthetic full payload populates all top-level state fields // Build payload = { lvl: 7, skills: { barter: 55 }, perks: ['Confirmed Bachelor'], // quests: [{ name: 'Ain\'t That a Kick in the Head' }], collectibles: ['Goodsprings Snowglobe'], // traits: ['Wild Wasteland'], ... }; // Call autoImportState(JSON.stringify({ narrative:'', state: payload, modal: null }), state); // Assert state.lvl === 7, state.skills.barter === 55, state.perks includes 'Confirmed Bachelor', etc. ``` **Note:** This is the upgrade to Suite 12's approach — extend the VM sandbox to cover `autoImportState` round-trips. The closest existing behavioral test is Suite 76 which tests hardening (field rejection), not happy-path field mapping. **Verdict:** SPEC-FIRST (requires VM sandbox for api.js + its dependencies) · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-8 · [HIGH] · Suite 48 `isFeatureEnabled` with unknown key never called Test 48.6 checks that the string `!== false` appears in the `isFeatureEnabled` function body — this is what the fail-open default looks like in source. But the function is never called with an unknown key and the return value never asserted. **Proposed behavioral test:** ```js // 48-B2: isFeatureEnabled('unknownKey9999') returns true (not false/undefined) // Eval isFeatureEnabled from cloud.js in a vm with empty _cachedFlags = {}; // assert(isFeatureEnabled('unknownKey9999') !== false) ``` **Verdict:** BUILD-NOW (simpler than TS-GAP-1; just needs cloud.js function eval) · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ### TS-GAP-9 · [HIGH] · Suite 45 `signOutAccount` re-anon path never executed Suite 45.6 checks that `signInAnonymously` appears in the `signOutAccount` function body. The function is never run in a sandbox. The "sign out → mint new anonymous user" path is the critical behavior that Protocol 31's guard is designed to protect — but the guard itself only verifies the string appears, not that the function is correctly gated. **Proposed behavioral test:** ```js // 45-B1: signOutAccount re-mints anonymous session (structural VM test) // Extract signOutAccount body; check it: (a) calls signOut before signInAnonymously, // (b) signInAnonymously is inside a try/catch (not called unconditionally before try) // This is stronger than 45.6's grep and catches the Protocol 31 unconditional-call regression. ``` **Verdict:** BUILD-NOW (structural body analysis, not full auth flow simulation) · **HIGH** **Files:** `tests/check-persistence.js`, `tests/check-persistence.ps1` --- ## Section 4 — WEAK ASSERTIONS Tests that would pass even if the guarded behavior broke. --- ### TS-WEAK-1 · [HIGH] · Suite 23 `innerHTML +=` guard doesn't cover api.js — see GH-1 Documented in Section 2. The carve-out comment is the weakness. Severity elevated to HIGH because it's a documented known exception for a confirmed active violation. --- ### TS-WEAK-2 · [MEDIUM] · Suite 11 migration presence grep allows wrong default values Suite 11 searches for `s\.${key}` in the `migrateState` body. This passes for `s.collectibles = s.collectibles || null` (wrong default — should be `{}`) just as well as it passes for the correct `s.collectibles = s.collectibles || {}`. The regex only confirms the field is *referenced*, not that the default value is correct. **Fix:** Suite 12's VM approach covers this for the fields it checks. Extend Suite 12's v1→current migration test to assert every v2.x field (traits, skillBooks, magazines, lincolnItems, craftedItems) is not `null` or `undefined` after migration. **Verdict:** BUILD-NOW (extend Suite 12 assertions) · **MEDIUM** --- ### TS-WEAK-3 · [MEDIUM] · Suite 12 does not assert v2.x fields after migration from v1.0.0 Suite 12 tests that `migrateState('1.0.0', payload)` runs successfully and produces certain fields (`factions.ncr`, `factions.legion`, `perks[]`, `quests[]`, `equipped.weapon`). None of the fields added in v2.x (traits, skillBooks, magazines, lincolnItems, craftedItems, skillMagazines, playthroughType, campaignMode) are asserted post-migration. A migration guard that silently dropped these new fields by returning early would pass Suite 12. **Fix:** Add to Suite 12's VM sandbox: `assert(migrated.traits !== undefined, 'traits field present after v1→current migration')` etc. for every v2.x field. **Verdict:** BUILD-NOW · **MEDIUM** --- ### TS-WEAK-4 · [MEDIUM] · Suite 7 backward-compat grep likely matches a comment Test 7.2: `assert(/legacy.*flat.*key|flat.*key.*fallback/i.test(apiSource), ...)` — this pattern is likely matching a comment in api.js that describes the legacy fallback, not the actual conditional branch. If the comment were preserved but the fallback branch deleted, the test passes. **Fix:** Extract the `autoImportState` function body and check for an actual conditional: `if (typeof parsed.lvl === 'undefined' && typeof parsed.s !== 'undefined')` or similar structural marker. **Verdict:** BUILD-NOW · **MEDIUM** --- ### TS-WEAK-5 · [MEDIUM] · Suite 51 51.21–51.34 are presence-only — JS runner is stronger than PS1 for save integrity The JS runner runs `verifySaveEnvelope`, `snapRollingBackup`, `getRollingBackups` in a real vm sandbox for tests 51.21–51.34. The PS1 runner replaces these 14 behavioral assertions with static pattern-matching against the source text. This is a **documented intentional trade-off** (PS1 cannot run vm.runInContext). However, it means logic bugs in checksum/semver/ring functions would pass the PS1 gate and fail the JS gate. The parity check at the gate catches the count mismatch if test counts diverge, but it does NOT catch the case where PS1 matches a pattern while the real behavior is wrong — the count stays equal. **Fix:** Maintain the current approach (JS is authoritative for behavioral tests; PS1 has structural equivalents). Document this explicitly in a comment block in both files so future developers know which runner is authoritative for which class of test. Identify the 14 tests that have structural-only equivalents in PS1 and add a comment: `# PS1: structural equivalent of JS behavioral test (vm.runInContext not available in PS)`. **Verdict:** SPEC-FIRST (requires PS1 to call Node subprocess for behavioral tests — already done for Suite 2b, 12, 18) · **MEDIUM** --- ### TS-WEAK-6 · [LOW] · Suite 87.9 partial-match weakening for PS1 encoding Test 87.9 in PS1 matches `'Programmer'` rather than the full `"Programmer's Digest"` due to PS1 encoding fragility with smart-quote characters. The JS runner matches the full string. If the magazine title were changed to something not starting with `'Programmer'`, PS1 would still pass. **Fix:** In PS1, use `.Replace("'", "'")` to normalize smart quotes before the match, then use the full title. **Verdict:** BUILD-NOW · **LOW** --- ### TS-WEAK-7 · [LOW] · Suite 51 QuotaExceededError recovery path never triggered Test 51.19 checks `QuotaExceededError`, `removeItem`, and adjacency of the error handler in the function body. The mock localStorage in the VM sandbox never throws `QuotaExceededError`, so the drop-oldest-retry path is never exercised. **Fix:** Add a variant of the `_snap()` behavioral test where `setItem` throws `QuotaExceededError` on the first call; assert the ring still has 3 entries (recovery succeeded). **Verdict:** BUILD-NOW · **LOW** --- ### TS-WEAK-8 · [LOW] · Suite 84 `craftSetMax` output never asserted `craftSetMax` is defined, called in the sandbox, but the resulting input `value` is never checked. A bug that set max quantity to `Infinity` or `NaN` would pass. **Fix:** Call `craftSetMax(0)` with a state where the scarce ingredient has qty=2; assert `document.getElementById('craftQty_0').value === '2'`. **Verdict:** BUILD-NOW · **LOW** --- ### TS-WEAK-9 · [LOW] · Suite 84 `_craftGetHave` ammo-first path never exercised All behavioral craft tests supply ingredients via `state.inventory`. The `_craftGetHave` function is documented to check `state.ammo` first, then fall back to `state.inventory`. The ammo-first branch is never exercised. **Fix:** In one craft test, place the ingredient in `state.ammo` (not `state.inventory`); assert the craft succeeds. **Verdict:** BUILD-NOW · **LOW** --- ## Section 5 — COVERAGE GAPS (no guard at all) --- ### TS-COV-1 · [HIGH] · No test verifies `autoImportState` rejects out-of-range values Suite 76 (autoImportState hardening) tests field-shape validation: equipped slot keys exist, BUFF/DEBUFF/NEUTRAL whitelist, string type normalization. But no test asserts that out-of-range numeric values are clamped or rejected — e.g., `state.lvl = 999`, `state.s = -5`, `state.karma = 10000`. If the AI sent `{ "lvl": 999 }` and it bypassed clamping, the UI would show LVL 999 until the user manually corrected it. **Proposed test:** ```js // TS-COV-1: autoImportState with out-of-range lvl does not set state.lvl to 999 // The test should verify either: (a) value is clamped, or (b) the field is guarded by a schema check. // If no clamping exists in autoImportState, document it as a known gap. ``` **Verdict:** SPEC-FIRST (requires decision on whether autoImportState should clamp, or whether clamping belongs in the save layer) · **HIGH** --- ### TS-COV-2 · [HIGH] · No behavioral test for `onGameContextChange` triggering UI re-render Suite 69 (FO3 game-switch regression) checks: `onGameContextChange` exists, `state.gameContext` is set, the `_contextSwitching` guard is wired, the `beforeunload` early-exit fires. All 4 tests are static source-text checks. The actual UI consequence — "switching game context re-renders the skills panel with the correct skill set for the new game" — is never verified. A regression where `renderSkills()` is not called after a game switch would pass all 4 tests. **Proposed test:** ```js // TS-COV-2: After onGameContextChange('FO3'), renderSkills re-renders with FO3 skill keys // In the JS vm sandbox: set state.gameContext = 'FNV'; call onGameContextChange('FO3'); // assert state.gameContext === 'FO3'; assert skillsGrid innerHTML contains 'Big Guns' // (FO3-specific skill), NOT 'Guns' (FNV-specific skill) ``` **Verdict:** BUILD-NOW (behavioral, no DOM dependency — just check the function calls renderSkills) · **HIGH** --- ### TS-COV-3 · [HIGH] · No test for never-negative inventory quantities Protocol behavior: deleting or consuming items must never produce negative quantities. There is no test that calls `removeItem()` on a quantity of 1 and asserts the item is removed (not driven to -1), or that `_craftConsume` correctly clamps to 0. Suite 84 tests the clamp (`_craftConsume` checks `≥0`) but only through the craft flow. Direct `removeItem` with qty=1 on a qty=1 item is never tested. **Proposed test:** ```js // TS-COV-3: removeItem on qty=1 removes the entry entirely (never drives to qty=0 or negative) // state.inventory = [{name: 'Stimpak', qty: 1, type: 'AID', ...}]; // call removeItem('Stimpak'); assert state.inventory.length === 0 ``` **Verdict:** BUILD-NOW · **HIGH** --- ### TS-COV-4 · [MEDIUM] · No test for `migrateState` on a mid-version payload (v2.x partial state) Suite 12 tests v1.0.0 → current migration. No test covers v2.3.0 → current, where some newer fields exist but others (added in v2.4.0+) are absent. A migration guard that was conditional on a version string could skip a required default for users upgrading from v2.3.x. **Proposed test:** ```js // TS-COV-4: migrateState on v2.3.0 partial state adds all v2.4.0+ fields with correct defaults // Payload: { ...v1Payload, traits: undefined, skillBooks: undefined, magazines: undefined }; // After migration: assert migrated.traits is an array, migrated.skillBooks is an array, etc. ``` **Verdict:** BUILD-NOW · **MEDIUM** --- ### TS-COV-5 · [MEDIUM] · No test for the `syncStateFromDom` SPECIAL stat clamp boundary (s=10 edge case) Suite 64 tests `commitStat` (1–10 clamp on blur). Suite 10 tests DOM binding. But the case where a user types "11" and the `capStatMax` function fires (preventing values >10 via `oninput`) is not behavioral-tested — only the presence of `capStatMax` in the handler is checked. A regression where `capStatMax` allowed setting to 11 (e.g., a `>` vs `>=` typo) would pass Suite 64 and Suite 10. **Proposed test:** ```js // TS-COV-5: capStatMax with value=11 reduces to 10 (upper-only clamp, no lower force) // Eval capStatMax in a vm with a mock input element; set input.value = '11'; // call capStatMax(input); assert input.value === '10' ``` **Verdict:** BUILD-NOW · **MEDIUM** --- ### TS-COV-6 · [MEDIUM] · No test verifies `_updateContextPanels` shows/hides the correct panels on game switch `_updateContextPanels` hides FO3-only panels on FNV context and vice versa. There is no test that sets game context to FO3 and asserts the FO3-specific panels are visible and the FNV-specific panels are hidden. A regression where all panels remained visible regardless of game context would pass the entire suite. **Proposed test:** ```js // TS-COV-6: _updateContextPanels('FO3') hides FNV-only panels, shows FO3-only panels // Set state.gameContext = 'FO3'; call _updateContextPanels('FO3'); // assert #traitsSection.style.display === 'none' (FNV-only) // assert #karmaCenter.style.display !== 'none' (FO3-only) ``` **Verdict:** BUILD-NOW · **MEDIUM** --- ### TS-COV-7 · [LOW] · No test for `addItem` dedup behavior (adding an existing item increases qty) The `addItem` CRUD helper has dedup logic: adding an item that already exists should increase its `qty`, not create a duplicate entry. No test exercises this — tests only verify `addItem` creates entries, not that it correctly updates existing ones. **Proposed test:** ```js // TS-COV-7: addItem on existing item name+type increases qty, does not create duplicate // state.inventory = [{name: 'Stimpak', type: 'AID', qty: 3, ...}]; // addItem with name='Stimpak', type='AID', qty=2; // assert state.inventory.length === 1; assert state.inventory[0].qty === 5 ``` **Verdict:** BUILD-NOW · **LOW** --- ## Section 6 — RUNNER PARITY GAPS --- ### TS-PAR-1 · [MEDIUM] · Suite 51 behavioral tests (51.21–51.56) are static pattern-matches in PS1 **JS:** `vm.runInContext` evaluates `_fnv1a32`, `computeSaveChecksum`, `verifySaveEnvelope`, `snapRollingBackup`, `getRollingBackups` against real inputs and asserts runtime return values. **PS1:** 36 behavioral assertions replaced with static source-text pattern matches. A logic inversion in `_semverGt` (e.g., `>` changed to `<`) would pass PS1 and fail JS. The gate parity check compares test COUNTS, not test QUALITY. Both runners report 56 tests for Suite 51; both pass; the parity check is satisfied. Only the JS runner actually tests the logic. **Fix:** PS1 should call Node for these behavioral tests via the subprocess pattern already used in Suites 2b, 12, and 18. Until that is done, add a comment in both files: `// JS-AUTHORITATIVE: PS1 uses structural equivalents for tests 51.21–51.56; JS runner is the correctness gate for save integrity logic`. **Verdict:** SPEC-FIRST (PS1 Node subprocess approach is proven — reuse the pattern) · **MEDIUM** --- ### TS-PAR-2 · [LOW] · Suite 87.9 uses partial match in PS1 due to smart-quote encoding See TS-WEAK-6. The fix is in PS1 only: normalize smart quotes before matching. **Verdict:** BUILD-NOW · **LOW** --- ### TS-PAR-3 · [LOW] · Suites 42–48 are absent from the `GATE_SUITES` presence check in Suite 28 Suite 28 (`Meta / Runner Parity`) asserts both runners contain suite strings `22–41` and `49–87`. Suites 42–48 were authored after the GATE_SUITES list was frozen and were never added to it. If a future change accidentally deleted one of these suites, the meta-parity check would not catch it. **Fix:** Update the `GATE_SUITES` array to include `42` through `48`. **Verdict:** BUILD-NOW · **LOW** --- ## Section 7 — FRAGILE TESTS Tests that break on benign formatting or refactor changes. --- ### TS-FRAG-1 · [LOW] · Suite 64.1 uses 550-char window around DOM IDs — Prettier expansion can break it Test 64.1 looks for `onchange="commitStat(this)"` within a 550-character window around the `id="${id}"` position. The comment notes this accommodates Prettier multiline expansion. If Prettier's line-wrapping of the input attributes changes (e.g., a new attribute is added before `onchange`), the 550-char window might exclude the `onchange` attribute. **Fix:** Increase the window to 800 characters, or better: search for the `id="${id}"` tag boundary and scan to the closing `>` of the element. **Verdict:** BUILD-NOW · **LOW** --- ### TS-FRAG-2 · [LOW] · Suite 59 (inline handler integrity) uses definition-anchored resolution — correctly avoids fragility Suite 59 is the model for non-fragile presence testing: it extracts ALL `on*` attribute values from HTML, then resolves each as a function definition by testing `function\s+NAME\b`, `NAME\s*=`, or `window\.NAME\s*=` against all served JS. This is source-position independent and would survive any amount of Prettier reformatting. ✓ No finding here — noting as the **correct pattern to follow for future inline handler checks**. --- ## Section 8 — GATE COMPLETENESS ASSESSMENT | Ratchet | Status | |---|---| | ESLint zero-warning | ✓ passing | | Prettier format check | ✓ passing | | Both runners at parity | ✓ passing | | Boot smoke (HTTP) | ✓ at push gate | | Render check 360/412px | ✓ at push gate | | A11y check (axe serious/critical baseline) | ✓ at push gate | | `innerHTML +=` in api.js | ✗ **GH-1: explicit carve-out** | | Unescaped innerHTML in ui-core.js/cloud.js | ✗ **GH-2: no guard** | | `localStorage.getItem` in AI hot paths | ✗ **GH-3: no guard** | | Kill-switch fail-open behavior | ✗ **TS-GAP-1: zero behavioral tests** | | Save/load round-trip with corrupted data | ✗ **TS-GAP-3: presence only** | | Backup restore actually applied to state | ✗ **TS-GAP-2: presence only** | The gate is strong for FORMAT, PRESENCE, and STRUCTURAL invariants. The systemic gap is **behavioral correctness for data-safety paths**: recovery, fail-open, and round-trip transformations. These are exactly the paths where silent regressions cause data loss or security failures. --- ## Prioritized Build-Now Backlog ### Tier 1 — Gate holes + Critical data safety (fix these in the same commit as the A5 violations they guard) | ID | Fix | Files | Tests added | |---|---|---|---| | GH-1 | Remove api.js carve-out from Suite 23; extend `innerHTML +=` check to all served JS | Both runners | 1 | | GH-2 | Add unescaped-innerHTML guard covering `apiModelInput` and Firestore model option | Both runners | 2 | | GH-3 | Assert `getSystemDirective()` and `transmitMessage()` bodies contain no `localStorage.getItem` | Both runners | 2 | | TS-GAP-1 | Suite 48 behavioral: `isFeatureEnabled('unknownKey')` returns true; fail-open on rejected getDoc | Both runners | 2–3 | | TS-GAP-8 | `isFeatureEnabled` with unknown key actually called and asserted | Both runners | 1 | ### Tier 2 — Data-safety behavioral gaps | ID | Fix | Files | Tests added | |---|---|---|---| | TS-GAP-2 | Suite 51: call `restoreRollingBackup` in VM, assert state applied | Both runners | 2 | | TS-GAP-3 | Suite 51: call `loadFromSlot` with tampered slot, assert state not overwritten on reject | Both runners | 2 | | TS-GAP-4 | Suite 4/5: export→re-import round-trip in VM, assert `verifySaveEnvelope` returns `'ok'` | Both runners | 1 | | TS-GAP-5 | Suite 46: run `syncLocalSavesToCloud` dedup with mock Firestore, assert `addDoc` called once | Both runners | 2 | | TS-GAP-6 | Suite 46: run `loadCloudSave` with XSS payload, assert sanitized output | Both runners | 2 | | TS-GAP-9 | Suite 45: structural body-analysis of `signOutAccount` for correct sign-out→re-anon order | Both runners | 1 | | TS-COV-2 | Suite 69: call `onGameContextChange`, assert skills panel re-renders with correct skill set | Both runners | 1 | | TS-COV-3 | `removeItem` on qty=1 removes entry entirely | Both runners | 1 | ### Tier 3 — Coverage and weakness upgrades | ID | Fix | Files | Tests added | |---|---|---|---| | TS-WEAK-2 | Suite 11: extend migration presence check with default-value assertions | Both runners | ~5 | | TS-WEAK-3 | Suite 12: assert v2.x fields (traits, skillBooks, etc.) after migration | Both runners | ~8 | | TS-GAP-7 | Suite 48 timeout race: `loadRemoteConfig` resolves even when `getDoc` never settles | Both runners | 1 | | TS-WEAK-7 | Suite 51: trigger `QuotaExceededError` in VM, assert ring recovery | Both runners | 1 | | TS-WEAK-8 | Suite 84: `craftSetMax` output assertion | Both runners | 1 | | TS-WEAK-9 | Suite 84: `_craftGetHave` ammo-first path | Both runners | 1 | | TS-COV-4 | Suite 12: mid-version (v2.3.0) migration payload | Both runners | 1 | | TS-COV-5 | `capStatMax` boundary: value=11 → 10 | Both runners | 1 | | TS-COV-6 | `_updateContextPanels` panel visibility on game switch | Both runners | 2 | | TS-COV-7 | `addItem` dedup: adding existing item increases qty | Both runners | 1 | | TS-PAR-2 | Suite 87.9 PS1: normalize smart quotes before match | PS1 only | 0 (fix existing) | | TS-PAR-3 | Suite 28 `GATE_SUITES`: add suites 42–48 | Both runners | 0 (fix existing) | | TS-FRAG-1 | Suite 64: increase DOM window or use element-boundary scan | Both runners | 0 (fix existing) | --- ## Constraint Checklist - **[GATE]** 1078 tests must pass · never `--no-verify` · current count must be stable before adding new tests. - **[2A]** Every test added increments the count in BOTH runners + all 7 Protocol 2a sync locations simultaneously. - **[PARITY]** Every test added to JS runner must have an exact-equivalent in PS1 at the same count (Protocol 15). The gate's parity check will block any push where counts diverge. - **[DETERMINISTIC]** All new behavioral tests must mock Gemini, Firestore, and Auth — no live network calls in the gate. Use the vm-sandbox + mock pattern already established in Suites 2b, 12, 51. - **[BATCH]** GH-1 must be added in the same commit that fixes A5 QA-PROHIB-1 (the `innerHTML +=` in api.js) — the guard and the fix must land together, per Protocol 36(b) escape-ratchet. - **[NEVER]** Never bump APP_VERSION for test-only commits. --- *End of TEST_STRENGTH_AUDIT.md*