> For the complete documentation index, see [llms.txt](https://faction-os.gitbook.io/faction-os-docs/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://faction-os.gitbook.io/faction-os-docs/.spec_system/specs/phase24-session04-combat-playback-effects-layer/code-review.md). # Code Review and Repair Report **Session ID**: `phase24-session04-combat-playback-effects-layer` **Package**: apps/web **Reviewed**: 2026-07-08 **Base Commit**: 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c **Scope**: All changes since the base commit (uncommitted work plus mid-session commits) **Result**: RESOLVED ## Review Surface **Files reviewed** (all changes since the base commit): * `.spec_system/PRD/PRD.md` - tracked-modified * `.spec_system/PRD/phase_24/PRD_phase_24.md` - tracked-modified * `.spec_system/PRD/phase_24/session_08_validation_and_documentation.md` - tracked-deleted * `.spec_system/PRD/phase_24/session_08_generated_asset_implementation_handoff.md` - untracked * `.spec_system/PRD/phase_24/session_09_validation_and_documentation.md` - untracked * `.spec_system/state.json` - tracked-modified * `.spec_system/specs/phase24-session04-combat-playback-effects-layer/spec.md` - untracked * `.spec_system/specs/phase24-session04-combat-playback-effects-layer/tasks.md` - untracked * `.spec_system/specs/phase24-session04-combat-playback-effects-layer/implementation-notes.md` - untracked * `.spec_system/specs/phase24-session04-combat-playback-effects-layer/code-review.md` - untracked review artifact * `apps/web/src/components/battlefield/Battlefield.tsx` - tracked-modified * `apps/web/src/components/battlefield/CampLayer.tsx` - tracked-modified * `apps/web/src/components/battlefield/CombatPlaybackLayer.tsx` - untracked * `apps/web/src/lib/combatPlaybackEffects.ts` - untracked * `apps/web/src/index.css` - tracked-modified * `apps/web/tests/Battlefield.test.tsx` - tracked-modified * `apps/web/tests/CombatPlaybackLayer.test.tsx` - untracked * `apps/web/tests/combatPlaybackEffects.test.ts` - untracked * `docs/game-design/02-core-gameplay-and-player-verbs.md` - tracked-modified * `docs/game-design/03-game-loops-and-onboarding.md` - tracked-modified * `docs/game-design/04-world-map-and-combat-systems.md` - tracked-modified * `docs/game-design/06-social-and-scoring-systems.md` - tracked-modified * `docs/game-design/07-content-audio-and-feature-inventory.md` - tracked-modified * `docs/game-design/08-production-roadmap-and-phase-a-spec.md` - tracked-modified * `docs/game-design/09-risk-boredom-and-integrity-audit.md` - tracked-modified * `docs/game-design/11-open-questions-and-change-log.md` - tracked-modified * `docs/game-design/12-utility-admin-holdout-inventory.md` - tracked-modified * `docs/game-design/13-implementation-phase-outline.md` - tracked-modified * `docs/game-design/15-phases-yet-to-be-implemented.md` - tracked-modified **Inventory commands**: `git status`, `git log --oneline "$BASE"..HEAD`, `git diff "$BASE"`, `git diff --cached "$BASE"`, `git ls-files --others --exclude-standard` ## Findings by Severity ### Critical * None. ### High * None. ### Medium * `apps/web/src/lib/combatPlaybackEffects.ts:478` - Combat playback effect keys were built from raw mission and tool event identifiers, and `CombatPlaybackLayer` used those keys in DOM `data-testid` attributes. A malicious or malformed producer could place sensitive text in a tool id and leak it through the rendered DOM. Fix: hash the raw mission/tool reference into a stable `ref-*` value before constructing effect keys, and extend the privacy regression to cover hostile `tool_use.id`, `tool_result.toolUseId`, and result preview text. Status: FIXED. ### Low * None. ## Assumptions and Deliberate Non-Fixes * Existing non-ASCII characters in historical game-design docs were left unchanged. Added diff lines were checked separately and contain no non-ASCII bytes; rewriting unrelated historical prose would exceed this review surface. * Phase PRD status for Session 04 remains pre-validation because `validate` and `updateprd` own workflow status advancement. `creview` records review/repair only. ## Behavior Changes * Combat playback effect keys and derived DOM test ids no longer include raw mission/tool event references. Product-facing copy, effect selection, timers, caps, and presentation behavior are unchanged. ## Evidence Ledger | Check | Command or Inspection | Result | Evidence / Blocker | | ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | Deterministic state | `if [ -d ".spec_system/scripts" ]; then bash .spec_system/scripts/analyze-project.sh --json; else bash /home/aiwithapex/.codex/plugins/cache/apexdev/apex-spec/2.2.19-codex/skills/apex-spec/scripts/analyze-project.sh --json; fi` | PASS | Current session resolved to `phase24-session04-combat-playback-effects-layer`; monorepo true; package scope `apps/web`. | | Base commit | `git rev-parse --verify --quiet "6e5af583288dbe9dfd9ca58c55ac0f5d8865831c^{commit}"` | PASS | Base commit exists and was used for the review surface. | | Inventory | `git status`; `git log --oneline 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c..HEAD`; `git diff --name-status 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c`; `git diff --cached 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c`; `git ls-files --others --exclude-standard` | PASS | No mid-session commits; no staged diff; tracked and untracked files inventoried. | | Source context | Targeted inspections of `.spec_system/CONVENTIONS.md`, `.spec_system/CONSIDERATIONS.md`, session `spec.md`, `tasks.md`, `implementation-notes.md`, `apps/web/README_web.md`, `docs/game-design/README_game-design.md`, `docs/battlefield.md`, `package.json`, `biome.jsonc`, `vitest.config.ts`, and `apps/web/tsconfig.json` | PASS | Conventions, session scope, privacy boundaries, battlefield contracts, and configured commands identified. | | Asset paths | `ls assets/generated/game-design/phase03/live-enemies/live-enemy-effects-sheet.png assets/generated/game-design/phase03/strike-vocabulary/strike-vocabulary-effects-sheet.png assets/generated/game-design/phase03/reduced-motion/reduced-motion-static-badges-sheet.png assets/generated/game-design/phase10/alliance-war/alliance-war-visuals-sheet.png assets/generated/game-design/phase11/seasons-saga/seasons-saga-visuals-sheet.png assets/generated/game-design/phase12/war-games/war-games-demo-presentation-sheet.png` | PASS | All newly referenced generated asset paths exist. | | Finding fix test | `npm test -- apps/web/tests/combatPlaybackEffects.test.ts` | PASS | 1 file passed, 9 tests passed. | | Focused tests | `npm test -- apps/web/tests/combatPlaybackEffects.test.ts apps/web/tests/CombatPlaybackLayer.test.tsx apps/web/tests/Battlefield.test.tsx` | PASS | 3 files passed, 49 tests passed. | | Full tests | `npm test` | PASS | 284 files passed, 1 skipped; 3342 tests passed, 1 skipped. | | Web typecheck | `npm --workspace @factionos/web run typecheck` | PASS | `tsc -b --noEmit` exited 0. | | Workspace typecheck | `npm run typecheck --workspaces --if-present` | PASS | Adapters, server, warroom, web, protocol, and public website typechecks passed; Astro check reported 0 errors, 0 warnings, 0 hints. | | Formatter write | `npx biome format --write apps/web/src/lib/combatPlaybackEffects.ts apps/web/src/components/battlefield/CombatPlaybackLayer.tsx apps/web/src/components/battlefield/CampLayer.tsx apps/web/src/components/battlefield/Battlefield.tsx apps/web/src/index.css apps/web/tests/combatPlaybackEffects.test.ts apps/web/tests/CombatPlaybackLayer.test.tsx apps/web/tests/Battlefield.test.tsx` | PASS | Formatted 8 files; no fixes applied after the review patch. | | Formatter check | `npm run format:check` | PASS | Biome checked 784 files; no fixes applied. | | Linter | `npm run lint` | PASS | Biome checked 786 files; no fixes applied. | | Security | `npm run security:secrets`; targeted inspection of `scripts/scan-secrets.mjs`; privacy regression in `apps/web/tests/combatPlaybackEffects.test.ts` | PASS | Secret scan passed for tracked text files; scanner patterns do not flag the new synthetic `raw-secret` fixture; untracked privacy-sensitive fields are covered by the focused regression. | | Diff whitespace | `git diff --check 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c` | PASS | No whitespace errors. | | Added-line ASCII | \`git diff -U0 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c -- . | LC\_ALL=C grep '^+' | grep -v '^+++' | | LF endings | \`grep -n $'\r' \[reviewed text files] | | true\` | | Final diff re-read | `git diff 6e5af583288dbe9dfd9ca58c55ac0f5d8865831c` plus full reads of untracked text files and final `git status --short` | PASS | No remaining review findings after the privacy fix. | ## Summary 1. Reviewed 29 files in the final review surface: app code/tests/CSS, session specs and notes, phase PRD/state updates, generated-asset session split docs, and game-design documentation references. 2. Findings: 0 critical, 0 high, 1 medium, 0 low. The medium privacy finding was fixed by hashing raw event references before effect keys reach DOM attributes and by adding regression coverage. 3. Deliberately not fixed: historical non-ASCII already present in game-design docs, because added lines are ASCII and the broad rewrite is unrelated to this session. 4. Evidence: focused tests, full tests, web and workspace typecheck, formatter, linter, secret scan, diff whitespace, added-line ASCII, LF, asset path, and final diff checks all passed. --- # Agent Instructions This documentation is published with GitBook. GitBook is the documentation platform designed so that both humans and AI agents can read, navigate, and reason over technical content effectively. Learn more at gitbook.com. ## Querying This Documentation If you need additional information that is not directly available in this page, you can query the documentation dynamically by asking a question. Perform an HTTP GET request on the current page URL with the `ask` query parameter, and the optional `goal` query parameter: ``` GET https://faction-os.gitbook.io/faction-os-docs/.spec_system/specs/phase24-session04-combat-playback-effects-layer/code-review.md?ask=&goal= ``` `ask` is the immediate question: it should be specific, self-contained, and written in natural language. `goal` is optional and describes the broader end goal you are ultimately trying to accomplish on behalf of the user. GitBook uses it to tailor the answer towards what is most useful for that goal. The response will contain a direct answer to the question and relevant excerpts and sources from the documentation. Use this mechanism when the answer is not explicitly present in the current page, you need clarification or additional context, or you want to retrieve related documentation sections.