---
phase: 04-harden-clean-up-optional
plan: 01
subsystem: content-script
tags: [audit-p1-polish, p1-11-fetch-url, p1-14-nav-url, p1-15-rrweb-timestamps, tdd, content-script, charter-d-p4-02]

# Dependency graph
requires:
  - phase: 03-spec-10-smoke-verification-dom-event-log-verification
    provides: REQ-user-event-log validated (A30 harness; 5 UserEvent types — click + input + navigation + js_error + network_error captured under canonical assertion shape)
  - phase: 01-stabilize-video-pipeline
    provides: tests/background/start-video-capture-no-tab.test.ts scaffold pattern (Plan 01-09 — pure-function-with-vi.fn-stub mirror for tests/content/)
provides:
  - Audit P1 #11 fetch URL extraction fix — fetch(new Request(url)) now captures Request.url in events.json instead of literal '[object Request]'
  - Audit P1 #14 navigation URL tracking fix — module-level `let previousUrl` tracker replaces stale `history.state?.url || 'unknown'` emit so meta.previousUrl carries the operator's prior URL verbatim
  - Audit P1 #15 rrweb timestamp normalization — rrweb emit-side timestamps normalized to Date.now() Unix-epoch ms so cleanupOldEvents (now - event.timestamp) arithmetic is meaningful
  - tests/content/ NEW directory mirroring src/content/ — establishes the content-script unit-test placement convention for Phase 4+
  - 9 new vitest tests (171 baseline → 180 GREEN) pinning the three contracts above
affects: [04-02 (CSP/dead-code hardening; same Phase 4 wave), 04-04 (A34 fetch+XHR empirical harness will exercise the P1 #11 fix end-to-end), 04-07 (Phase 4 closure aggregator)]

# Tech tracking
tech-stack:
  added: []
  patterns:
    - "Content-script unit-test scaffold: vi.mock('rrweb') + globalThis stubs for window/document/history/Request before await import('../../src/content/index'); userEvents+rrwebEvents read back via GET_RRWEB_EVENTS chrome.runtime.onMessage handler. Established at tests/content/ for Phase 4+ content-script work."
    - "Type-narrow over implicit-coercion at trust boundaries: `args[0] instanceof Request ? args[0].url : String(args[0])` replaces `args[0]?.toString()` at the fetch wrapper sites. Discipline applied at both ok-branch (line ~190) AND catch-branch (line ~210) — twin sites carry the same narrow."
    - "Module-level state for closure-time inter-event tracking: `let previousUrl` mutated swap-then-emit inside handleNavigation. Mirrors the existing rrwebEvents + userEvents module-level pattern at lines 21+24."
    - "Wrapper-side timestamp normalization at emit time: `event.timestamp = Date.now()` prepended in rrweb record() emit callback. Same Unix-epoch convention as the existing addUserEvent normalization at line 54."

key-files:
  created:
    - "tests/content/fetch-interception.test.ts"
    - "tests/content/navigation-tracking.test.ts"
    - "tests/content/rrweb-timestamps.test.ts"
  modified:
    - "src/content/index.ts (3 surgical edits — module-level previousUrl declaration at line 29; handleNavigation rewrite at lines 106-122; fetch URL narrow at lines 190 + 210; rrweb emit timestamp normalization at line 315)"
    - ".planning/config.json (use_worktrees: false — orchestrator-staged flip from earlier in this session; folded into Task 1 commit)"

key-decisions:
  - "Task 1 (RED) added 4 fetch tests instead of the plan's nominal 3: Tests A+C are paired with new Tests B+D so both fetch interception sites (ok-branch line ~190 + catch-branch line ~210) get an explicit Request-arg test against the same canonical narrow. Plan acceptance allows ≥3 it() in fetch-interception.test.ts (min_lines: 60); 4 strengthens the regression net."
  - "Used `(typeof window !== 'undefined')` guard in the module-level previousUrl initializer so the content script remains importable under Node-env vitest (vitest.config.ts environment: 'node'). Production content-script realm always has window; the guard is a one-line defensive bootstrap mirroring the project's import-time discipline (same guard pattern Plan 01-03 established for chrome.* in src/offscreen/recorder.ts)."
  - "Did NOT introduce a helper function for the Request-or-string narrow. The inline form `(args[0] instanceof Request ? args[0].url : String(args[0])) || 'unknown'` reads clearly at the two call sites and avoids a new module-scope name. Optionally extractable in a future plan if a third call site appears."

patterns-established:
  - "tests/content/ as the canonical content-script unit-test directory (parallel to existing tests/background/, tests/i18n/, tests/offscreen/, tests/build/)"
  - "Reading content-script module state via the production GET_RRWEB_EVENTS chrome.runtime.onMessage path doubles as a contract pin for the archive export surface"
  - "Audit P1 polish in src/content/index.ts is now closed end-to-end via a TDD pair (RED scaffolds + GREEN edits in cohesive sibling commits)"

requirements-completed: []

# Metrics
duration: ~30 min
completed: 2026-05-21
---

# Phase 4 Plan 01: audit-p1-polish-content-script Summary

**Three audit P1 correctness fixes in src/content/index.ts — fetch(new Request(url)) URL extraction at both fetch-wrapper sites, module-level previousUrl tracker replacing the always-"unknown" navigation emit, and Unix-epoch rrweb emit timestamp normalization — gated by 9 new RED→GREEN unit tests under tests/content/.**

## Performance

- **Duration:** ~30 min (RED scaffold + GREEN edits + bundle gates + SUMMARY)
- **Started:** 2026-05-21T11:55:00Z (after orchestrator respawn in foreground mode)
- **Completed:** 2026-05-21T12:25:00Z
- **Tasks:** 2 (Wave 0 RED + Wave 1 GREEN per plan's tdd type)
- **Files modified:** 4 (3 new test files + 1 source file edit + 1 config flip)

## Accomplishments

- Audit P1 #11 (fetch URL extraction) — `fetch(new Request(url))` now records the canonical `request.url` in events.json instead of the literal string `'[object Request]'`. Twin fix at both ok-branch (line ~190) and catch-branch (line ~210).
- Audit P1 #14 (navigation URL tracking) — module-level `let previousUrl` declared at line 29; handleNavigation rewritten to swap-then-emit so `meta.previousUrl` carries the operator's actual prior URL instead of the literal `'unknown'` produced by the stale `history.state?.url || 'unknown'` lookup.
- Audit P1 #15 (rrweb emit timestamps) — `event.timestamp = Date.now()` prepended in the rrweb `record({ emit })` callback at line 315; cleanupOldEvents `(now - event.timestamp) < RRWEB_RETENTION_MS` arithmetic at line 33 is now meaningful (both sides Unix-epoch ms).
- 9 new vitest tests across `tests/content/` pin all three contracts; baseline preserved (171 → 180/180 GREEN). tsc-clean preserved.

## Task Commits

Each task was committed atomically per the plan's TDD cycle:

1. **Task 1: Wave 0 RED — content-script test scaffolds** — `3dbc51c` (test)
   - 7 of 9 tests RED at commit (B+D fetch, A+B+C navigation, A+B rrweb); 2 tests pass-as-baseline (A+C fetch — String(string)===string identity).
2. **Task 2: Wave 1 GREEN — 3 surgical edits in src/content/index.ts** — `7da30af` (feat)
   - All 9 tests flip GREEN; full vitest baseline preserved at 180/180.

**Plan metadata commit:** to follow this SUMMARY landing.

## Files Created/Modified

- `tests/content/fetch-interception.test.ts` (NEW; 4 tests pinning P1 #11 — fetch(stringUrl) ok-branch + fetch(new Request(url)) ok-branch + fetch(stringUrl) catch-branch + fetch(new Request(url)) catch-branch)
- `tests/content/navigation-tracking.test.ts` (NEW; 3 tests pinning P1 #14 — popstate + hashchange + history.pushState wrap all read meta.previousUrl from module-level state)
- `tests/content/rrweb-timestamps.test.ts` (NEW; 2 tests pinning P1 #15 — Test A normalization to >1e12 + Test B cleanupOldEvents arithmetic regression)
- `src/content/index.ts` (3 surgical edits — see Decisions Made for line-level details)
- `.planning/config.json` (use_worktrees flipped to false earlier in this session by the orchestrator; folded into Task 1 commit per orchestrator instruction)

## Decisions Made

**Inline narrow over extracted helper for P1 #11:** Considered factoring out `function extractFetchUrl(arg: unknown): string` at module scope; chose to keep the narrow inline at both call sites for the same reason the project consistently inlines `event.timestamp = Date.now();` in the existing addUserEvent path — the discipline reads at the call site, the call sites are two-and-only-two, and a future plan can refactor if a third caller appears.

**typeof window guard for module-level previousUrl:** Production content-script realm always has `window`; the `(typeof window !== 'undefined')` guard at line 29 keeps the module importable under vitest's Node environment, mirroring the defensive bootstrap pattern Plan 01-03 established for chrome.* in src/offscreen/recorder.ts.

**Test count bumped 8 → 9:** Plan nominal was 3+3+2=8 tests. Added a 4th fetch test (catch-branch + new Request) so both fetch-wrapper sites have an explicit Request-arg pin. Plan acceptance permits ≥3 it() per file; this strengthens the regression net for the catch-branch fix at line ~210 which would otherwise have a single regression-guard test (Test C with a string arg).

## Deviations from Plan

### Auto-fixed Issues

**1. [Rule 3 - Blocking] vi.mock('rrweb') in fetch + navigation tests**
- **Found during:** Task 1 (first npm test run)
- **Issue:** Initial RED-test scaffold for fetch-interception + navigation-tracking did not stub rrweb. The content script's `import { record } from 'rrweb'` pulled rrweb's lib/*.js through vitest's CJS transform pipeline; rrweb 2.0.0-alpha.4 ships ESM-only, so the import crashed with "exports is not defined in ES module scope" before any test body ran.
- **Fix:** Added `vi.mock('rrweb', () => ({ record: (_opts) => () => {} }))` at the top of both fetch-interception.test.ts and navigation-tracking.test.ts. The rrweb-timestamps test already mocked rrweb (it needs the captured emit callback), so this completes the pattern across all 3 files.
- **Files modified:** tests/content/fetch-interception.test.ts, tests/content/navigation-tracking.test.ts
- **Verification:** All 9 content tests imported cleanly post-fix; 7 RED + 2 baseline-pass as expected for Task 1's RED gate.
- **Committed in:** `3dbc51c` (Task 1 commit; deviation absorbed into the same Wave 0 RED commit because the fix was in the test files themselves, pre-RED-gate).

**2. [Rule 1 - Bug] Removed `static prototype: unknown` from FakeXHR stubs**
- **Found during:** Task 1 (first npm test run — vite:oxc parse error)
- **Issue:** Initial scaffold included `static prototype: unknown = { open: () => {}, send: () => {} };` on the FakeXHR class stub. The TC39 class fields proposal reserves `prototype` as a non-writable class property; oxc (Vite's parser) correctly rejects it with `Classes may not have a static property named 'prototype'`.
- **Fix:** Removed the `static prototype` line from all 3 test files. The instance-level open/send/addEventListener methods on FakeXHR suffice — the content script only assigns to `XMLHttpRequest.prototype.open` and `XMLHttpRequest.prototype.send` (production prototype chain), which is satisfied by the auto-generated class prototype.
- **Files modified:** tests/content/fetch-interception.test.ts, tests/content/navigation-tracking.test.ts, tests/content/rrweb-timestamps.test.ts
- **Verification:** Files parsed clean post-fix.
- **Committed in:** `3dbc51c` (Task 1 commit).

---

**Total deviations:** 2 auto-fixed (1 blocking, 1 bug — both in the new test files, both resolved pre-commit during the Task 1 RED iteration).
**Impact on plan:** Zero scope creep; deviations were test-side mechanics that surfaced during the first RED-gate run and got rolled into the same Task 1 commit before the RED gate was claimed. Source-side edits (Task 2) landed exactly as RESEARCH §"Specifics" and 04-PATTERNS.md spelled out.

## Issues Encountered

**Pre-existing vitest flakes** — 3 unrelated tests intermittently fail across baseline runs (`tests/background/blob-url-download.test.ts` 5000ms timeout race, `tests/background/webm-remux.test.ts` ffprobe frame count, `tests/offscreen/webm-playback.test.ts` ffmpeg dry-run). These are documented Phase 4 deferred items per `03-VERIFICATION.md` "Forward-Looking Deferred Items" + 04-CONTEXT.md §"Flake stabilization" — owned by Plan 04-03 (A29 race rewrite + parallel-vitest race + 2 ffprobe flakes). Confirmed pre-existing via `git stash` baseline runs (171/171 only on lucky runs; 168/171 or 165/171 on flaky runs). Plan 04-01 introduces no new flakes — final clean baseline run was 180/180 GREEN.

## Pre-Checkpoint Bundle Gates

Per saved memory `feedback-pre-checkpoint-bundle-gates.md` (6/6 standard inventory):

1. **Tier-1 FORBIDDEN_HOOK_STRINGS** — `tests/background/no-test-hooks-in-prod-bundle.test.ts` 13/13 GREEN; inventory unchanged at 12 strings (Plan 04-01 added no harness hooks; pure source-side polish).
2. **SW CSP-safety grep** — 1 `new Function("")` in SW chunk `dist/assets/index.ts-8LkXuqac.js`. Pre-existing `vite-plugin-node-polyfills` setimmediate polyfill; documented at `.planning/phases/01-stabilize-video-pipeline/deferred-items.md`; Plan 04-02 owns the polyfill replacement. NOT a Plan 04-01 regression.
3. **Node-globals grep** — `Buffer.copy / .isView / .length / .push / .shift` in SW chunk — all from JSZip internals (lowercase-b `buffer` field names + JSZip stream pipeline). Pre-existing across all Phase 1+2+3 builds.
4. **DOM-globals grep** — `document.createElement / .createTextNode / .documentElement / .F` + `window.Math / .console / .localStorage / .process` in SW chunk — pre-existing shimmed-DOM references inside JSZip's text encoder fallback paths.
5. **manifest.json** — present at `dist/manifest.json`; `name: "__MSG_extName__"` (chrome.i18n message resolution intact). Plan 04-01 did NOT touch `_locales/` so en↔ru parity is untouched.
6. **Build itself** — `npm run build` exits 0 in 4.66s; all chunks ship.

## Self-Check

- **Files created:**
  - tests/content/fetch-interception.test.ts → FOUND
  - tests/content/navigation-tracking.test.ts → FOUND
  - tests/content/rrweb-timestamps.test.ts → FOUND
- **Files modified:**
  - src/content/index.ts → FOUND (diff shows 3 surgical edits)
  - .planning/config.json → FOUND (use_worktrees: false)
- **Commits:**
  - 3dbc51c → FOUND (test(04-01): Wave 0 RED)
  - 7da30af → FOUND (feat(04-01): Wave 1 GREEN)
- **Verification commands all green:**
  - npm test -- tests/content/ --run → 9 passed (9)
  - npm test -- --run → 180 passed (180) [confirmed twice; once on flaky run with 1 pre-existing failure, once 180/180 clean]
  - npx tsc --noEmit → exit 0
  - All 7 acceptance grep checks pass per Task 2 commit body

## Self-Check: PASSED

## Next Phase Readiness

- Plan 04-02 (build hygiene: vite.config.ts polyfill replacement + generate-icons ESM/CJS + dead-code grep + deferred-items.md update) is **unblocked** and queued NEXT in Wave 1. It modifies `vite.config.ts` + `src/background/index.ts` + `generate-icons.{js,cjs}` + 2 build tests + the Phase 1 deferred-items.md; zero overlap with Plan 04-01's `src/content/index.ts` surface.
- Plan 04-04 (A34 fetch+XHR network_error empirical harness; ROADMAP SC #2) will validate Plan 04-01's P1 #11 fix end-to-end against a real probe page in Wave 3.
- Audit P1 polish backlog: **CLOSED 3/3** (items #11 + #14 + #15 all GREEN with regression coverage).

---
*Phase: 04-harden-clean-up-optional*
*Completed: 2026-05-21*