docs(01-03): complete offscreen recorder TDD plan
- Add 01-03-SUMMARY.md documenting RED -> GREEN gate (Plan 02 tests now pass), 3 Rule-3 auto-fixes (OffscreenLogger inline, defensive bootstrap, SW dead-code removal), and Plan 04 / 05 handoff notes. - Update STATE.md: advance plan counter to 4 of 7 (43%), append metrics + 3 execution decisions, record session. - Update ROADMAP.md: mark Plan 01-03 [x] complete. REQ-video-ring-buffer remains NOT complete — still pending Plans 04 (port keepalive) and 07 (ffprobe acceptance gate). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
234
.planning/phases/01-stabilize-video-pipeline/01-03-SUMMARY.md
Normal file
234
.planning/phases/01-stabilize-video-pipeline/01-03-SUMMARY.md
Normal file
@@ -0,0 +1,234 @@
|
||||
---
|
||||
phase: 01-stabilize-video-pipeline
|
||||
plan: 03
|
||||
subsystem: offscreen-recorder
|
||||
tags: [mediarecorder, getDisplayMedia, vp9, ring-buffer, tdd, chrome-extension, mv3]
|
||||
|
||||
# Dependency graph
|
||||
requires:
|
||||
- phase: 01-stabilize-video-pipeline
|
||||
provides: "Plan 02 RED tests pinning ring-buffer + codec contracts"
|
||||
provides:
|
||||
- "src/offscreen/recorder.ts — canonical offscreen recorder module (ring buffer + getDisplayMedia + codec strict-mode + track-ended cleanup)"
|
||||
- "src/offscreen/index.html — crxjs-managed bundle entry"
|
||||
- "OffscreenLogger class in src/shared/logger.ts (uses unknown[] for strict-mode hygiene)"
|
||||
- "PortMessage / PortMessageType types in src/shared/types.ts"
|
||||
- "Removed broken VIDEO_CHUNK / VIDEO_CHUNK_SAVED message variants (audit P0 #2 dead path)"
|
||||
- "D-13 restart-segments fallback skeleton pre-staged (no re-plan needed if Plan 07 ffprobe fails)"
|
||||
affects: [01-04-port-keepalive, 01-05-sw-shrink, 01-06-vite-config, 01-07-ffprobe-gate]
|
||||
|
||||
# Tech tracking
|
||||
tech-stack:
|
||||
added: []
|
||||
patterns:
|
||||
- "Pure ring-buffer functions exported for Node-testable unit tests"
|
||||
- "Defensive bootstrap with typeof chrome guard (allows pure-function tests to import the module)"
|
||||
- "Codec strict-mode (assertCodecSupported throws + emits RECORDING_ERROR before throw)"
|
||||
- "OffscreenLogger uses ...args: unknown[] (strict-mode hygiene divergence from legacy Logger/ContentLogger which keep any[])"
|
||||
|
||||
key-files:
|
||||
created:
|
||||
- "src/offscreen/recorder.ts"
|
||||
- "src/offscreen/index.html"
|
||||
modified:
|
||||
- "src/shared/logger.ts"
|
||||
- "src/shared/types.ts"
|
||||
- "src/background/index.ts" # Rule-3 inline cleanup; Plan 05 owns the broader shrink
|
||||
|
||||
key-decisions:
|
||||
- "Bootstrap section guarded with typeof chrome check (Rule 3) so the pure ring-buffer test can import the module without a chrome stub"
|
||||
- "OffscreenLogger added in Task 2 commit (recorder.ts cannot typecheck without the import — Rule 3 blocking dependency)"
|
||||
- "VIDEO_CHUNK / VIDEO_CHUNK_SAVED SW-side handlers and IndexedDB plumbing deleted inline (Rule 3) — removing types without removing referencing branches would break tsc-clean acceptance"
|
||||
- "Task 4 refactor pass: no obvious improvements — SKIPPED"
|
||||
|
||||
patterns-established:
|
||||
- "Pure functions (addChunk / trimAged / getBuffer / resetBuffer / assertCodecSupported) exported separately from impure side-effects (getDisplayMedia / MediaRecorder lifecycle) — keeps Node-only test discipline"
|
||||
- "Defensive chrome.runtime guards in bootstrap so the module is import-safe under partial test stubs (the handshake + port tests stub the full chrome surface; the ring-buffer test stubs nothing)"
|
||||
- "Russian section header comments preserved per project provenance (CONTEXT.md 'Established patterns')"
|
||||
|
||||
requirements-completed: [] # REQ-video-ring-buffer is NOT yet complete — still pending Plans 04 (port keepalive) + 07 (ffprobe gate)
|
||||
|
||||
# Metrics
|
||||
duration: 8min
|
||||
completed: 2026-05-15
|
||||
---
|
||||
|
||||
# Phase 01 Plan 03: Offscreen Recorder TDD GREEN Summary
|
||||
|
||||
**vp9-strict offscreen recorder module with header-pinned 30 s ring buffer, getDisplayMedia capture, MediaRecorder lifecycle, and track-ended cleanup — flipped Plan 02 ring-buffer + codec-check tests from RED to GREEN.**
|
||||
|
||||
## Performance
|
||||
|
||||
- **Duration:** 8 min
|
||||
- **Started:** 2026-05-15T15:30:29Z
|
||||
- **Completed:** 2026-05-15T15:38:42Z
|
||||
- **Tasks:** 4 (3 executed, 1 SKIPPED per Task 4 rules)
|
||||
- **Files modified:** 5 (2 created, 3 modified)
|
||||
|
||||
## Accomplishments
|
||||
|
||||
- **GREEN gate cleared:** `tests/offscreen/ring-buffer.test.ts` (4 tests) and `tests/offscreen/codec-check.test.ts` (2 tests) — all 6 now pass.
|
||||
- **Canonical recorder module:** `src/offscreen/recorder.ts` (214 lines) owns the ring buffer + getDisplayMedia + MediaRecorder lifecycle + codec strict-mode (D-20) + track.onended cleanup (D-03).
|
||||
- **Codec strict-mode enforced:** `assertCodecSupported()` calls `MediaRecorder.isTypeSupported('video/webm;codecs=vp9')` and on failure emits RECORDING_ERROR to SW *before* throwing — T-1-01 mitigation, no fallback chain.
|
||||
- **Header retention + 30 s age trim:** ring-buffer pure functions implement D-10 (first chunk pinned indefinitely) and D-11 (drop later chunks older than 30 s by arrival timestamp).
|
||||
- **D-13 fallback skeleton pre-staged** as a commented restart-segments block at the bottom of recorder.ts so Plan 07's potential fallback path does not require a re-plan.
|
||||
- **Strict-type hygiene:** zero `as any`, zero `@ts-ignore`, zero fallback codec strings in `src/offscreen/`.
|
||||
|
||||
## TDD Gate Sequence
|
||||
|
||||
| Gate | Command | Result |
|
||||
|------|---------|--------|
|
||||
| **RED** (pre-implementation) | `npx vitest run tests/offscreen/ring-buffer.test.ts tests/offscreen/codec-check.test.ts` | exit 1, `Cannot find module '../../src/offscreen/recorder'` (captured to `/tmp/01-03-red.log`) |
|
||||
| **GREEN** (post-implementation) | `npx vitest run tests/offscreen/ring-buffer.test.ts tests/offscreen/codec-check.test.ts` | exit 0, 6 tests pass (captured to `/tmp/01-03-green.log`) |
|
||||
| **REFACTOR** | inspection of `src/offscreen/recorder.ts` | no obvious improvements — SKIPPED per Task 4 rules |
|
||||
|
||||
### RED log excerpt
|
||||
```
|
||||
FAIL tests/offscreen/ring-buffer.test.ts [ tests/offscreen/ring-buffer.test.ts ]
|
||||
Error: Cannot find module '../../src/offscreen/recorder' imported from
|
||||
/home/parf/projects/work/repremium/tests/offscreen/ring-buffer.test.ts
|
||||
```
|
||||
|
||||
### GREEN test output (6/6 PASS)
|
||||
```
|
||||
RUN v4.1.6 /home/parf/projects/work/repremium
|
||||
······
|
||||
Test Files 2 passed (2)
|
||||
Tests 6 passed (6)
|
||||
```
|
||||
|
||||
Test names now passing:
|
||||
1. `ring buffer > first chunk is header`
|
||||
2. `ring buffer > second chunk is NOT header`
|
||||
3. `ring buffer > trim 30s — keeps header, evicts aged tail`
|
||||
4. `ring buffer > trim with empty buffer does not throw`
|
||||
5. `codec strict mode > throws on unsupported vp9 and emits RECORDING_ERROR`
|
||||
6. `codec strict mode > does not throw when vp9 IS supported`
|
||||
|
||||
## Task Commits
|
||||
|
||||
| # | Task | Type | Commit |
|
||||
|---|------|------|--------|
|
||||
| 1 | RED-verify (no commit per plan — verify-only) | — | — |
|
||||
| 2 | GREEN — write recorder.ts + index.html (bundled OffscreenLogger inline due to Rule-3 dependency) | feat | **fff1aea** |
|
||||
| 3 | OffscreenLogger + types cleanup (bundled SW dead-code removal due to Rule-3 dependency) | feat | **c5828d3** |
|
||||
| 4 | REFACTOR — SKIPPED (no obvious improvements) | — | — |
|
||||
|
||||
Final metadata commit will follow after STATE.md / ROADMAP.md updates.
|
||||
|
||||
## Export Surface of `src/offscreen/recorder.ts`
|
||||
|
||||
For Plans 04 / 05 to grep against without re-reading:
|
||||
|
||||
```typescript
|
||||
// constants
|
||||
export const VIDEO_BUFFER_DURATION_MS: number; // 30_000
|
||||
|
||||
// pure ring-buffer functions
|
||||
export function addChunk(blob: Blob, timestamp: number): void;
|
||||
export function trimAged(now: number): void;
|
||||
export function getBuffer(): VideoChunk[];
|
||||
export function resetBuffer(): void;
|
||||
|
||||
// codec strict-mode (throws on unsupported vp9; emits RECORDING_ERROR before throw)
|
||||
export function assertCodecSupported(): void;
|
||||
```
|
||||
|
||||
Import-time side effects (from `bootstrap()` — guarded by `typeof chrome !== 'undefined'`):
|
||||
- `chrome.runtime.onMessage.addListener(...)` — registered exactly once
|
||||
- `chrome.runtime.connect({ name: 'video-keepalive' })` — called exactly once
|
||||
- `chrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' })` — called exactly once
|
||||
|
||||
## Files Created / Modified
|
||||
|
||||
| File | Status | Lines | Purpose |
|
||||
|------|--------|-------|---------|
|
||||
| `src/offscreen/recorder.ts` | created | 214 | Canonical offscreen recorder (D-01..D-13, D-20) |
|
||||
| `src/offscreen/index.html` | created | 9 | crxjs bundle entry referencing `./recorder.ts` |
|
||||
| `src/shared/logger.ts` | modified | +28 | Added `OffscreenLogger` class (uses `unknown[]` per plan style note) |
|
||||
| `src/shared/types.ts` | modified | -2 / +9 | Removed `VIDEO_CHUNK`/`VIDEO_CHUNK_SAVED` from `MessageType`; added `PortMessageType` + `PortMessage` |
|
||||
| `src/background/index.ts` | modified | -91 | Removed dead VIDEO_CHUNK + VIDEO_CHUNK_SAVED case branches and unreachable helper functions (Rule 3 — see Deviations) |
|
||||
|
||||
## Threat Mitigations Verified
|
||||
|
||||
- **T-1-01 (codec downgrade tampering):** `assertCodecSupported()` calls `MediaRecorder.isTypeSupported('video/webm;codecs=vp9')` strict; on failure emits `RECORDING_ERROR` to SW BEFORE throwing. Grep gate: `grep -v '^#' src/offscreen/recorder.ts | grep -cE 'codecs=(vp8|h264)'` returns 0. Unit test mocking `isTypeSupported -> false` confirms throw + sendMessage call.
|
||||
- **T-1-03 (stream content leakage):** Accepted residual risk per CONTEXT.md D-04. Code-level mitigation deferred: the `getDisplayMedia` call captures whatever the user picks; logging `track.label` for support visibility is Plan 04 work (the bootstrap defers to Plan 04 for the port handler that surfaces this).
|
||||
- **T-1-NEW-03-01 (unbounded buffer):** `trimAged` is a pure filter over arrival timestamps; defensive grep + tests confirm trim is idempotent on empty buffer and never grows past `header + chunks newer than now - 30 000 ms`.
|
||||
|
||||
## Decisions Made
|
||||
|
||||
1. **OffscreenLogger style divergence:** uses `...args: unknown[]` per plan style_divergence_note; legacy `Logger` / `ContentLogger` stay on `any[]`. Plan 03 does NOT refactor the legacy classes — they remain per project provenance.
|
||||
2. **Bootstrap defensive guard:** the `bootstrap()` function checks `typeof chrome !== 'undefined'` before registering any side effects. This was the minimal change needed for the pure ring-buffer test (which stubs nothing) to import the module without crashing. Plan 04 will replace this stub with the full reconnect + ping loop.
|
||||
3. **Task 4 refactor SKIPPED:** inspection found no obvious improvements (no constant duplication, no unused imports, no mis-placed comments). Per Task 4 rules, do not commit if no changes.
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
### Auto-fixed Issues
|
||||
|
||||
**1. [Rule 3 - Blocking] OffscreenLogger bundled into Task 2 commit instead of Task 3**
|
||||
- **Found during:** Task 2 GREEN implementation
|
||||
- **Issue:** `src/offscreen/recorder.ts` imports `OffscreenLogger` from `../shared/logger` (per plan verbatim code), but Task 3 was scheduled to ADD that class. The Task 2 acceptance gate `npx tsc --noEmit` cannot pass without the import resolving.
|
||||
- **Fix:** Added the `OffscreenLogger` class to `src/shared/logger.ts` as part of the Task 2 commit. Task 3 commit then handled only the `src/shared/types.ts` cleanup + SW dead-code removal.
|
||||
- **Files modified:** `src/shared/logger.ts` (in Task 2 commit fff1aea)
|
||||
- **Verification:** `grep -c "export class OffscreenLogger" src/shared/logger.ts` returns 1; `npx tsc --noEmit` exits 0.
|
||||
|
||||
**2. [Rule 3 - Blocking] Defensive bootstrap guard added — pure ring-buffer test does not stub chrome**
|
||||
- **Found during:** Task 2 — first GREEN run after writing the verbatim plan code
|
||||
- **Issue:** The plan's verbatim bootstrap code unconditionally accesses `chrome.runtime.onMessage.addListener`, `chrome.runtime.connect`, and `chrome.runtime.sendMessage`. The Plan 02 ring-buffer test imports the module but does NOT stub `chrome` (intentionally — it tests pure functions). Result: `ReferenceError: chrome is not defined` on every import. The codec-check test stubs `chrome.runtime.sendMessage` only, so it also crashed on the missing `onMessage.addListener`.
|
||||
- **Fix:** Wrapped the bootstrap in a function with `typeof chrome === 'undefined'` / per-API-existence guards. The handshake + port tests provide the full chrome stub, so they still observe the side effects; the pure-function tests no-op the bootstrap. Plan 04 will replace this stub with the full reconnect + ping loop on top of the guarded structure.
|
||||
- **Files modified:** `src/offscreen/recorder.ts` (in Task 2 commit fff1aea)
|
||||
- **Verification:** `npx vitest run tests/offscreen/ring-buffer.test.ts tests/offscreen/codec-check.test.ts` → 6/6 PASS; the handshake test still passes (single OFFSCREEN_READY emission observed).
|
||||
|
||||
**3. [Rule 3 - Blocking] Removed SW-side VIDEO_CHUNK + VIDEO_CHUNK_SAVED branches and unreachable IndexedDB helpers**
|
||||
- **Found during:** Task 3 — first tsc run after removing the union members
|
||||
- **Issue:** Removing `VIDEO_CHUNK` / `VIDEO_CHUNK_SAVED` from `MessageType` (Task 3 acceptance criterion) broke two `case` branches in `src/background/index.ts:457-473`. Leaving them would have caused TS2678 errors; tsc-clean is a Task 3 acceptance gate. The branches referenced `addVideoChunkFromBlob` and `loadChunkFromIndexedDB`. After deleting the case branches, `loadChunkFromIndexedDB` had no callers (TS6133 unused). Cascading: `openIndexedDB`, `addVideoChunkFromBlob`, `cleanupVideoBuffer`, `firstChunkSaved`, the SW-side `VIDEO_BUFFER_DURATION_MS` constant all became unused.
|
||||
- **Fix:** Deleted the case branches and the now-unreachable helper functions and state variables. Kept `videoBuffer: VideoChunk[] = []` (still referenced by `getVideoBuffer` and `saveArchive`) as an empty placeholder; Plan 04 wires it to fetch from offscreen over the keepalive port. CONTEXT.md "Files to DELETE in this phase" explicitly lists these functions as Phase 1 delete targets; the attribution between plans 03 and 05 was not strictly enumerated. Plan 05 still owns the broader SW shrink.
|
||||
- **Files modified:** `src/background/index.ts` (in Task 3 commit c5828d3) — net `-91 lines` (115 removed, 24 added)
|
||||
- **Verification:** `npx tsc --noEmit` exits 0; ring-buffer + codec tests still pass; SW module compiles cleanly.
|
||||
|
||||
---
|
||||
|
||||
**Total deviations:** 3 auto-fixed (3× Rule 3 blocking dependencies)
|
||||
**Impact on plan:** All three were inevitable consequences of the plan's TDD ordering — Tasks 2 and 3 had cross-dependencies the plan author glossed over. None introduced new architecture; all stayed within Phase 1's CONTEXT.md authorization. No scope creep beyond what CONTEXT.md "Files to DELETE in this phase" already sanctioned.
|
||||
|
||||
## Issues Encountered
|
||||
|
||||
- **Plan 04 port-reconnect test stays RED** (per plan `<verification>` line 646 — this is intentional). The handshake test passes (single OFFSCREEN_READY emission), so wave-2 of the TDD choreography is partially complete; Plan 04 will flip the remaining reconnect test to GREEN.
|
||||
|
||||
## Threat Flags
|
||||
|
||||
None — no new security-relevant surface was introduced beyond what the plan's `<threat_model>` already enumerated (T-1-01, T-1-03, T-1-NEW-03-01).
|
||||
|
||||
## Plan 04 / 05 Handoff
|
||||
|
||||
**Plan 04 needs to:**
|
||||
1. Replace the import-time stub `chrome.runtime.connect({ name: 'video-keepalive' })` with the full ping-loop + reconnect-on-disconnect from RESEARCH.md Pattern 5.
|
||||
2. Wire the `REQUEST_BUFFER` handler so the SW can pull chunks on export (uses the `PortMessage` types added in this plan).
|
||||
3. Confirm the existing `OFFSCREEN_READY` send is still emitted exactly once (the handshake test should remain green after Plan 04's refactor pass).
|
||||
4. Once Plan 04 lands, the `void keepalivePort;` line near the bottom of `recorder.ts` becomes dead and should be removed in Plan 04's refactor.
|
||||
|
||||
**Plan 05 needs to:**
|
||||
- Update SW side: replace the empty `videoBuffer` placeholder with a port-driven fetch; collapse the remaining SW shell (`getVideoBuffer`, `saveArchive` integration with offscreen).
|
||||
- Delete `setupKeepalive` + alarms code (audit P1 #8 / D-18) — this plan left it intact.
|
||||
|
||||
## Next Phase Readiness
|
||||
|
||||
REQ-video-ring-buffer is NOT yet complete — it remains tied to Plans 04 (port keepalive) and 07 (ffprobe gate). Do NOT mark the requirement complete; STATE.md will reflect Plan 03 done but the requirement still pending.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- `[x] src/offscreen/recorder.ts` exists (214 lines, exit 0 from `test -f`)
|
||||
- `[x] src/offscreen/index.html` exists (9 lines)
|
||||
- `[x] src/shared/logger.ts` updated (OffscreenLogger class found via grep)
|
||||
- `[x] src/shared/types.ts` updated (VIDEO_CHUNK removed, PortMessage added)
|
||||
- `[x] src/background/index.ts` updated (dead code removed)
|
||||
- `[x] Commit fff1aea` exists (`git log --oneline | grep fff1aea` returns 1)
|
||||
- `[x] Commit c5828d3` exists (`git log --oneline | grep c5828d3` returns 1)
|
||||
- `[x] GREEN tests pass` (verified by `/tmp/01-03-green.log` and live re-run)
|
||||
- `[x] tsc --noEmit` exits 0
|
||||
- `[x] T-1-01 grep gate` returns 0 (no fallback codec strings)
|
||||
- `[x] D-13 marker` present (`// FALLBACK D-13: restart-segments` grep returns 1)
|
||||
|
||||
---
|
||||
*Phase: 01-stabilize-video-pipeline*
|
||||
*Completed: 2026-05-15*
|
||||
Reference in New Issue
Block a user