Files

Mark 526ac78046 docs(04): create phase plan — 7 plans for Phase 4 hardening (audit P1 polish + flake stabilization + SW persistence + visual polish + closure)

Wave structure:
- W1 (parallel): 04-01 (Audit P1 polish #11/#14/#15 TDD) + 04-02 (build/CSP hygiene: setimmediate polyfill + dead-code + generate-icons.cjs)
- W2: 04-03 (A29 cs-injection-world rewrite; closes flake)
- W3: 04-04 (A33 SW state persistence; spike-first + CDP worker.close())
- W4: 04-05 (A34 fetch+XHR network_error; ROADMAP SC #2 + validates Plan 04-01 P1 #11 end-to-end)
- W5: 04-06 (dark-logo currentColor + cursor verification + 01-07-SUMMARY back-patch; operator empirical)
- W6: 04-07 (04-VERIFICATION.md aggregator + ROADMAP backfill + v1 close prep)

Honors locked decisions D-P4-01..05 (full Phase 4 + all 3 P1 polish + both visual items + alpha-independent + ROADMAP backfill).
Implements RESEARCH Q1 (setimmediate option a), Q2 (spike-first SW persistence), Q3 (A29 cs-injection-world), Finding 4 (cursor already shipped — verification only).
UI-SPEC dark-logo currentColor strategy with inline-SVG injection landed per UI-SPEC §"Implementation amendment".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

2026-05-21 09:30:49 +02:00

22 KiB

Raw Blame History

phase, slug, plan, type, wave, depends_on, files_modified, autonomous, requirements, tags, user_setup, must_haves

phase

slug

plan

type

wave

depends_on

files_modified

autonomous

requirements

Source files — locus of P1 fixes (read once; extract patterns)

@src/content/index.ts @src/shared/types.ts

Analog test scaffold to mirror — pure-function-with-vi.fn-stub pattern

@tests/background/start-video-capture-no-tab.test.ts @tests/background/onboarding.test.ts

From src/content/index.ts (existing module-level state — additive pattern at lines 20-24):

let rrwebEvents: EventWithTime[] = [];     // line 21
let userEvents: UserEvent[] = [];          // line 24
// Phase 4 Plan 04-01 P1 #14 — ADD at line 25 (or adjacent to existing module-level state):
let previousUrl = '';                       // initialized in initOrSetUpInitialUrl() at content script init

From src/shared/types.ts (UserEvent shape — DO NOT MODIFY):

export interface UserEvent {
  timestamp: number;
  type: 'click' | 'input' | 'navigation' | 'js_error' | 'network_error';
  target: string;
  value: string;
  url: string;
  meta?: Record<string, unknown>;
}

From src/content/index.ts:174 + :190 (existing P1 #11 bug sites — TWO instances, both must be fixed):

// Line ~174 (ok-branch):
target: args[0]?.toString() || 'unknown',
// Line ~190 (catch-branch):
target: args[0]?.toString() || 'unknown',

After Phase 4 Plan 04-01:

// Helper at module scope OR inline in both branches:
target: (args[0] instanceof Request ? args[0].url : String(args[0])) || 'unknown',

From src/content/index.ts:107 (existing P1 #14 bug site):

meta: { previousUrl: history.state?.url || 'unknown' }

After Phase 4 Plan 04-01:

// Module-level state add at ~line 25:
let previousUrl = (typeof window !== 'undefined') ? window.location.href : '';

// In handleNavigation closure at line 100-109:
const fromUrl = previousUrl;
const toUrl = window.location.href;
previousUrl = toUrl;
addUserEvent({
  timestamp: Date.now(),
  type: 'navigation',
  target: 'window',
  value: toUrl,
  url: toUrl,
  meta: { previousUrl: fromUrl },
});

From src/content/index.ts:286-289 (existing P1 #15 bug site):

function initRrweb() {
  record({
    emit(event) {
      rrwebEvents.push(event);
    },

After Phase 4 Plan 04-01:

function initRrweb() {
  record({
    emit(event) {
      event.timestamp = Date.now();   // Plan 04-01 P1 #15: normalize to Unix epoch for cleanupOldEvents arithmetic
      rrwebEvents.push(event);
    },

Task 1: Wave 0 RED — three content-script unit-test scaffolds (one per audit P1 item) tests/content/fetch-interception.test.ts, tests/content/navigation-tracking.test.ts, tests/content/rrweb-timestamps.test.ts tests/background/start-video-capture-no-tab.test.ts, tests/background/onboarding.test.ts, src/content/index.ts, .planning/phases/04-harden-clean-up-optional/04-PATTERNS.md (sections "tests/content/fetch-interception.test.ts" through "tests/content/rrweb-timestamps.test.ts") fetch-interception.test.ts (3 tests): - Test A: `fetch(stringUrl)` where response.ok=false → `addUserEvent` captures `target === stringUrl` (NOT `'[object Object]'` or `'unknown'`). RED today (works for strings; PASS-LOOKING because String(string)===string is identity). - Test B: `fetch(new Request(url))` where response.ok=false → captures `target === request.url` (NOT `'[object Request]'`). RED today (bug — current code emits `'[object Request]'`). - Test C: regression — `fetch(stringUrl)` that throws (.catch path) → captures `target === stringUrl` AND `type === 'network_error'`. Pins the catch-branch fix too.

navigation-tracking.test.ts (3 tests):
- Test A: load content/index.ts; window.location starts at https://example.com/start; mutate to /next; dispatch popstate → captured UserEvent.meta.previousUrl === 'https://example.com/start' (NOT 'unknown'). RED today.
- Test B: same shape via hashchange (location.hash mutation). RED today.
- Test C: regression — history.pushState invocation still emits navigation with previousUrl honoring module state (not the just-pushed URL).

rrweb-timestamps.test.ts (2 tests):
- Test A: stub rrweb.record so its internal callback can be triggered with a synthetic event carrying timestamp=42 (page-load-relative class); after content script initRrweb runs, the captured rrwebEvents[0].timestamp > 1e12 (Date.now()-class). RED today.
- Test B: regression — cleanupOldEvents at lines 27-50 correctly filters; the (now - event.timestamp) arithmetic is meaningful because both sides are Unix-epoch. Build a synthetic rrwebEvents containing one event with timestamp=Date.now()-RRWEB_RETENTION_MS-1000 (older than retention) + one event with timestamp=Date.now() (fresh); after cleanup, only the fresh event remains.

Scaffold mirrors tests/background/start-video-capture-no-tab.test.ts:
- `beforeEach(() => { vi.resetModules(); });`
- Minimal `buildChromeStub()` providing `chrome.runtime.sendMessage` no-op + `chrome.runtime.onMessage.addListener` no-op (content script does not import chrome.tabs / chrome.storage directly).
- Dynamic `await import('../../src/content/index.ts')` after stubbing window.fetch + history + rrweb (use vi.mock or stub-via-global).
- For rrweb, mock the module: `vi.mock('rrweb', () => ({ record: vi.fn((opts) => { capturedEmit = opts.emit; }) }))`.

1. Create `tests/content/` directory (new — no existing tests at this path). 2. Create `tests/content/fetch-interception.test.ts` per the scaffold in 04-PATTERNS.md section "tests/content/fetch-interception.test.ts" (3-test contract). Use `vi.resetModules()` in `beforeEach`, `vi.mock('rrweb', ...)` to isolate from real rrweb (the rrweb module is irrelevant for fetch tests but the content script imports it at module top). The fetch wrapper is invoked by directly calling `window.fetch(...)` after the content script has run. 3. Create `tests/content/navigation-tracking.test.ts` per the same scaffold (3-test contract). Set up JSDOM-style window.location via vitest's environment: 'jsdom' in vitest.config.ts (already configured per the analog test). Dispatch popstate via `window.dispatchEvent(new PopStateEvent('popstate'))`. Read captured UserEvents via a global hook installed by the test (e.g., spy on `addUserEvent` via vi.spyOn after module import, OR capture via `chrome.runtime.onMessage` listener). 4. Create `tests/content/rrweb-timestamps.test.ts` per the same scaffold (2-test contract). Mock rrweb.record so the test can capture the `emit` callback handle, then synthesize an event with `timestamp: 42` (page-load-relative class) and assert that after `emit(synthetic)`, `rrwebEvents[0].timestamp > 1e12`.

All 3 files use TypeScript strict mode (project default); imports use absolute paths from project root per CLAUDE.md ("Always use absolute imports"); filter-pipeline form, no `continue` per CLAUDE.md Control Flow rules.

RED gate: run `npm test -- tests/content/ --run` — all 8 tests FAIL (Test A/C of fetch may pass on string args; Tests B + nav A/B + rrweb A guaranteed RED).

npm test -- tests/content/ --run 2>&1 | tee /tmp/04-01-task-1-red.log; grep -c 'FAIL' /tmp/04-01-task-1-red.log - File `tests/content/fetch-interception.test.ts` exists with ≥ 3 `it(...)` blocks in a `describe('Audit P1 #11 ...')` block. - File `tests/content/navigation-tracking.test.ts` exists with ≥ 3 `it(...)` blocks in a `describe('Audit P1 #14 ...')` block. - File `tests/content/rrweb-timestamps.test.ts` exists with ≥ 2 `it(...)` blocks in a `describe('Audit P1 #15 ...')` block. - At least 4 tests FAIL when run (the RED tests for B/nav A/B/rrweb A); the regression tests may PASS pre-fix. - `grep -c "instanceof Request" tests/content/fetch-interception.test.ts | grep -v '^#'` returns ≥ 2 (Test B assertion + post-fix expectation). - `grep -c "previousUrl" tests/content/navigation-tracking.test.ts | grep -v '^#'` returns ≥ 3 (Tests A/B/C assertions). - `grep -cE "Date\\.now|timestamp > 1e12" tests/content/rrweb-timestamps.test.ts | grep -v '^#'` returns ≥ 2. 3 RED test files committed (8 tests; ≥ 4 RED, ≤ 4 passing-as-regression-baseline). Atomic commit: `test(04-01): Wave 0 RED — audit P1 #11/#14/#15 content-script test scaffolds`. Task 2: Wave 1 GREEN — 3 surgical edits in src/content/index.ts (P1 #11 + #14 + #15) src/content/index.ts src/content/index.ts (full; ~310 lines), tests/content/fetch-interception.test.ts, tests/content/navigation-tracking.test.ts, tests/content/rrweb-timestamps.test.ts All 8 unit tests from Task 1 flip GREEN after these 3 surgical edits. - Edit 1 (P1 #11): both fetch interception sites (ok-branch line ~174 + catch-branch line ~190) use `args[0] instanceof Request ? args[0].url : String(args[0])` to extract URL. - Edit 2 (P1 #14): add `let previousUrl = (typeof window !== 'undefined') ? window.location.href : '';` at module scope (~line 25); rewrite `handleNavigation` closure to swap `previousUrl` and emit prior value. - Edit 3 (P1 #15): in `initRrweb` (~line 286), prepend `event.timestamp = Date.now();` before `rrwebEvents.push(event);`. Read `src/content/index.ts` in full once (~310 lines, well under context limit). Extract exact line numbers for the 3 edit sites (they may have drifted since the PATTERNS.md mapping if any prior plan touched them — confirm via grep). All edits use the Edit tool with exact strings (no heredoc; no sed):

Edit 1 — P1 #11 fetch URL extraction (TWO sites, identical pattern):
- At the ok-branch (currently ~line 174): replace `target: args[0]?.toString() || 'unknown',` with `target: (args[0] instanceof Request ? args[0].url : String(args[0])) || 'unknown',`
- At the catch-branch (currently ~line 190): same replacement (the two literal strings are identical so the Edit tool needs both replacements; use the `replace_all: false` discipline by using a slightly larger surrounding string per the Edit-tool spec, OR target each separately by including unique context).
- Optionally factor out to a helper `function extractFetchUrl(arg: unknown): string` at module scope; either is acceptable.

Edit 2 — P1 #14 navigation URL tracking:
- Add at module scope (insert after the existing `let userEvents: UserEvent[] = [];` line ~24): `let previousUrl: string = (typeof window !== 'undefined') ? window.location.href : '';  // Plan 04-01 P1 #14 — module-level previous-URL tracker`
- Rewrite the body of `handleNavigation` closure (currently lines 100-108):
  ```typescript
  const handleNavigation = () => {
    const fromUrl = previousUrl;
    const toUrl = window.location.href;
    previousUrl = toUrl;
    addUserEvent({
      timestamp: Date.now(),
      type: 'navigation',
      target: 'window',
      value: toUrl,
      url: toUrl,
      meta: { previousUrl: fromUrl },
    });
  };
  ```
- Preserve the `addEventListener('popstate', handleNavigation)` + `addEventListener('hashchange', handleNavigation)` + `history.pushState/replaceState` wrap exactly as-is (no behavior change to the dispatch mechanism).

Edit 3 — P1 #15 rrweb emit timestamp normalization:
- In the `record({ emit(event) { ... } })` block at ~line 286, prepend ONE line: `event.timestamp = Date.now();  // Plan 04-01 P1 #15 — normalize to Unix epoch for events.json downstream + cleanupOldEvents arithmetic correctness`. The existing `rrwebEvents.push(event);` stays unchanged.

TypeScript-strict expectations: tsc-clean (`npx tsc --noEmit`). The `EventWithTime = any;` alias at line 10 already opens `event.timestamp` for write; if strict-mode complains, use a typed widening cast (NEVER `as any`) per CLAUDE.md rule.

Run focused tests during edits: `npm test -- tests/content/ --run` should flip from 4-RED to 0-RED after all 3 edits.

npm test -- tests/content/ --run 2>&1 | tee /tmp/04-01-task-2-green.log; grep -c 'PASS\\|✓' /tmp/04-01-task-2-green.log; npx tsc --noEmit 2>&1 | tee /tmp/04-01-task-2-tsc.log; grep -c 'error TS' /tmp/04-01-task-2-tsc.log - `npm test -- tests/content/ --run` exits 0 with 8/8 GREEN (was 4-RED in Task 1). - `npx tsc --noEmit` exits 0 (no new type errors). - `grep -c "instanceof Request" src/content/index.ts` returns ≥ 2 (both sites) AND `grep -cE "args\\[0\\]\\?\\.toString" src/content/index.ts` returns 0 (old pattern removed). - `grep -cE "^let previousUrl" src/content/index.ts` returns 1 (the module-level state declaration). - `grep -cE "meta: \\{ previousUrl: fromUrl \\}" src/content/index.ts` returns 1 (the new closure emit). - `grep -cE "event\\.timestamp = Date\\.now\\(\\)" src/content/index.ts | grep -v '^#'` returns ≥ 1 (the rrweb emit normalization; addUserEvent at line 54 already has its own `event.timestamp = Date.now()` so total may be 2). - Full vitest suite passes: `npm test -- --run` exits 0 with previous baseline + 8 new tests = ≥ 179 GREEN (171 baseline + 8 new). P1 #11 + #14 + #15 GREEN; tsc-clean; full vitest GREEN with +8. Atomic commit: `feat(04-01): Wave 1 GREEN — fix audit P1 #11 fetch URL + #14 nav URL + #15 rrweb timestamps`.

<threat_model>

Trust Boundaries

Boundary	Description
page realm → content-script realm	`window.fetch` / `history.pushState` wrappers run in the content-script ISOLATED world after page realm dispatches; payloads (URLs, request objects) are page-controlled
content script → SW (events.json downstream)	UserEvents accumulate in module-level arrays; flushed via `chrome.runtime.sendMessage` at archive-assembly time; the `meta` object is JSON-serialized verbatim

STRIDE Threat Register

Threat ID	Category	Component	Disposition	Mitigation Plan
T-04-01-01	Information Disclosure	fetch wrapper line 174/190 — `args[0]?.toString()` masks the real URL with `'[object Request]'` when a page calls `fetch(new Request(url))`	mitigate	Replace with `args[0] instanceof Request ? args[0].url : String(args[0])` — explicit type-narrow before string conversion. This IS the V5 ASVS input-validation pattern.
T-04-01-02	Repudiation	navigation tracker always emits `previousUrl: 'unknown'` so support cannot determine where the operator was before the failing navigation; degrades the diagnostic value of saved archives	mitigate	Module-level `previousUrl` state; mutate after every emit; emit prior value. No third-party data introduced; same write surface as existing `rrwebEvents` / `userEvents` arrays.
T-04-01-03	Tampering	rrweb timestamps are page-load-relative; cleanupOldEvents' Unix-epoch-style arithmetic silently drops events incorrectly (`now - smallInt` is always >> retention)	mitigate	Normalize at emit: `event.timestamp = Date.now()`. rrweb's internal timestamps remain rrweb-internal (used by rrweb-player for replay timing); the wrapper writes Unix epoch only for events.json downstream. No external data introduced.
T-04-01-04	Information Disclosure	new previousUrl module state could leak across content-script reloads if the variable persisted across navigations	accept	content script is re-injected per Chrome's `<all_urls>` match-pattern on each top-level navigation; module state resets per content-script lifecycle. Same risk profile as existing `userEvents` / `rrwebEvents` arrays.
</threat_model>

- `npm test -- --run` exits 0 (vitest baseline ≥ 171 → ≥ 179 with new tests). - `npx tsc --noEmit` exits 0. - `grep -c "instanceof Request" src/content/index.ts` returns ≥ 2 (audit P1 #11 invariant pin). - `grep -c "previousUrl" src/content/index.ts` returns ≥ 4 (module-state declaration + closure body uses + emit). - `grep -cE "event\\.timestamp = Date\\.now\\(\\)" src/content/index.ts` returns ≥ 1 in the rrweb-emit region (lines 280-300 — verify with `grep -n` + line range check). - UAT harness 33/33 GREEN preserved (rrweb-emit timestamp change does NOT affect harness assertions which read EventType enum values, not timestamp ranges).

<success_criteria>

3 RED test files committed (Task 1; 8 tests; ≥ 4 RED).
3 surgical edits in src/content/index.ts committed (Task 2; flips 4 RED → 0 RED).
vitest baseline 171 → ≥ 179 GREEN (+8).
tsc-clean preserved.
UAT harness 33/33 GREEN preserved.
ROADMAP success criterion #2 (fetch + XHR network_error) PARTIAL — fetch URL extraction fixed; XHR variant is correct-by-construction (XHR.send uses _url captured at open-time, not args[0]); empirical XHR-404 verification deferred to Plan 04-04. </success_criteria>

After completion, create `.planning/phases/04-harden-clean-up-optional/04-01-SUMMARY.md` capturing: - 3 test files added with test counts (3 + 3 + 2 = 8) - 3 edits with before/after snippets (1-liner each) - vitest baseline before/after counts - tsc-clean confirmation - RED→GREEN cycle commit refs (Task 1 + Task 2) - Open items for downstream plans: Plan 04-04 (A34 fetch+XHR empirical harness extension exercises the fix end-to-end)

22 KiB Raw Blame History

Source files — locus of P1 fixes (read once; extract patterns)

Analog test scaffold to mirror — pure-function-with-vi.fn-stub pattern

Trust Boundaries

STRIDE Threat Register

22 KiB

Raw Blame History