diff --git a/.planning/ROADMAP.md b/.planning/ROADMAP.md index 29cc63d..0f6ae5f 100644 --- a/.planning/ROADMAP.md +++ b/.planning/ROADMAP.md @@ -70,7 +70,7 @@ directory + `vite.config.ts` inline string + `src/background/`. - [x] 01-01-PLAN.md — Doc cascade: amend DEC-003 / DEC-010 / RETIRE constraints / swap manifest permissions (D-A1..D-A6) - [x] 01-02-PLAN.md — Wave-0 test infrastructure: Vitest install + 4 RED test files + fixtures placeholder - [x] 01-03-PLAN.md — Offscreen recorder TDD: ring buffer + codec strict-mode + getDisplayMedia + track-ended cleanup; D-13 fallback skeleton pre-staged -- [ ] 01-04-PLAN.md — Port keepalive + OFFSCREEN_READY handshake (TDD): replaces alarms keepalive on offscreen side +- [x] 01-04-PLAN.md — Port keepalive + OFFSCREEN_READY handshake (TDD): replaces alarms keepalive on offscreen side - [ ] 01-05-PLAN.md — SW shrink: delete legacy buffer + alarms + IndexedDB + tabCapture paths; wire SW-side onConnect host - [ ] 01-06-PLAN.md — Build pipeline collapse: delete vite.config.ts inline plugin + top-level offscreen/ dir; declare rollupOptions.input - [ ] 01-07-PLAN.md — Manual smoke + ffprobe D-12 acceptance gate; commit regression fixture @@ -224,7 +224,7 @@ Phases execute in numeric order: 1 → 2 → 3 → 4 → 5. | Phase | Plans Complete | Status | Completed | |-------|----------------|--------|-----------| -| 1. Stabilize video pipeline | 3/7 | In Progress| | +| 1. Stabilize video pipeline | 4/7 | In Progress| | | 2. Stabilize DOM + event capture privacy | 0/TBD | Not started | - | | 3. Stabilize export pipeline | 0/TBD | Not started | - | | 4. SPEC §10 smoke verification | 0/TBD | Not started | - | diff --git a/.planning/STATE.md b/.planning/STATE.md index 71e9817..627e504 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -3,15 +3,15 @@ gsd_state_version: 1.0 milestone: v2.0.0 milestone_name: milestone status: executing -stopped_at: Completed Plan 01-03 — recorder.ts ring-buffer + codec strict-mode GREEN; Plan 04 next (port keepalive + REQUEST_BUFFER wiring) -last_updated: "2026-05-15T15:41:54.994Z" +stopped_at: Completed Plan 01-04 — port keepalive + OFFSCREEN_READY handshake GREEN (9/9 tests pass); Plan 05 next (SW shrink + onConnect host with T-1-04 sender check) +last_updated: "2026-05-15T15:53:24.264Z" last_activity: 2026-05-15 progress: total_phases: 5 completed_phases: 0 total_plans: 7 - completed_plans: 3 - percent: 43 + completed_plans: 4 + percent: 57 --- # Project State @@ -28,12 +28,12 @@ no server, no password leaks. ## Current Position Phase: 1 (Stabilize Video Pipeline) — EXECUTING -Plan: 4 of 7 +Plan: 5 of 7 Status: Ready to execute Last activity: 2026-05-15 REQUIREMENTS.md, ROADMAP.md, STATE.md written) -Progress: [████░░░░░░] 43% +Progress: [██████░░░░] 57% ## Performance Metrics @@ -62,6 +62,7 @@ Progress: [████░░░░░░] 43% | Phase 01 P01 | 4min | 6 tasks | 6 files | | Phase 01 P02 | 4min | 5 tasks | 8 files | | Phase 1 P03 | 8min | 3 tasks | 5 files | +| Phase 01 P04 | 4min | 3 tasks | 1 files | ## Accumulated Context @@ -88,6 +89,9 @@ current work: - [Phase 01-03]: Bundled OffscreenLogger into Task 2 commit (Rule 3 blocking dependency — recorder.ts cannot typecheck without the import) - [Phase 01-03]: Defensive bootstrap guard (typeof chrome check) lets pure ring-buffer test import recorder module without chrome stub - [Phase 01-03]: Removed SW-side VIDEO_CHUNK/VIDEO_CHUNK_SAVED branches + IndexedDB helpers inline (tsc-clean requires; Plan 05 owns remaining SW shrink) +- [Phase 01-04]: Kept Plan 03's defensive bootstrap guard (typeof chrome / per-API existence checks) instead of Plan 04's verbatim unguarded block — Plan 04's verbatim block regressed ring-buffer and codec-check tests (they don't stub full chrome surface); restored guard preserves Plan 02 RED contract while satisfying Plan 04's new GREEN contract. Rule 1 deviation. +- [Phase 01-04]: T-1-04 SW-side sender check documented redundantly (4 places in recorder.ts) for Plan 05 executor visibility — Offscreen is trusting party; SW is validating party. Documenting in module header, port-name constant, threat-mitigation comment near bootstrap, and inline at connectPort makes the contract impossible to miss when grepping for T-1-04 during Plan 05. +- [Phase 01-04]: REFACTOR pass NOT skipped: stale 'Plan 04 wires this' comments replaced with actual D-17/Pattern 5 citations — Forward-pointing TODO-style comments became misleading after the work landed; minimal correctness-preserving comment update with all 9 tests still GREEN. ### Pending Todos @@ -110,7 +114,7 @@ Items acknowledged and carried forward from previous milestone close: ## Session Continuity -Last session: 2026-05-15T15:41:54.976Z -Stopped at: Completed Plan 01-03 — recorder.ts ring-buffer + codec strict-mode GREEN; Plan 04 next (port keepalive + REQUEST_BUFFER wiring) +Last session: 2026-05-15T15:53:12.593Z +Stopped at: Completed Plan 01-04 — port keepalive + OFFSCREEN_READY handshake GREEN (9/9 tests pass); Plan 05 next (SW shrink + onConnect host with T-1-04 sender check) intel synthesis. Coverage validated: 11/11 v1 REQs mapped. -Resume file: .planning/phases/01-stabilize-video-pipeline/01-04-PLAN.md +Resume file: .planning/phases/01-stabilize-video-pipeline/01-05-PLAN.md diff --git a/.planning/phases/01-stabilize-video-pipeline/01-04-SUMMARY.md b/.planning/phases/01-stabilize-video-pipeline/01-04-SUMMARY.md new file mode 100644 index 0000000..c7c62f7 --- /dev/null +++ b/.planning/phases/01-stabilize-video-pipeline/01-04-SUMMARY.md @@ -0,0 +1,218 @@ +--- +phase: 01-stabilize-video-pipeline +plan: 04 +subsystem: offscreen-recorder +tags: [chrome-extension, mv3, service-worker-keepalive, long-lived-port, offscreen-document, handshake, tdd] + +# Dependency graph +requires: + - phase: 01-stabilize-video-pipeline + provides: "Plan 02 RED tests for OFFSCREEN_READY handshake + port reconnect contracts; Plan 03 stub bootstrap + PortMessage types in shared/types.ts" +provides: + - "Full long-lived port lifecycle in src/offscreen/recorder.ts — connect → ping (25 s) → REQUEST_BUFFER handler → pre-emptive reconnect (290 s) → onDisconnect synchronous reconnect" + - "OFFSCREEN_READY handshake confirmed to fire exactly once after listener registration (Pattern 4 ordering)" + - "T-1-04 mitigation: explicit message-shape switch in onPortMessage (defense-in-depth); sender.id === chrome.runtime.id contract documented for Plan 05's SW-side onConnect listener" +affects: [01-05-sw-shrink, 01-06-vite-config, 01-07-ffprobe-gate] + +# Tech tracking +tech-stack: + added: [] + patterns: + - "Long-lived port keepalive with synchronous reconnect-on-disconnect (Pitfall 4 mitigation against Chrome's ~5 min port lifetime cap)" + - "Idempotent connectPort(): teardown timers → fresh connect → re-attach all listeners. Reentrant via onDisconnect → connectPort()" + - "Defense-in-depth on inbound port messages: typeof-check, type-switch on known PortMessageType union, silent drop of unknown shapes" + +key-files: + created: [] + modified: + - "src/offscreen/recorder.ts" + +key-decisions: + - "Kept Plan 03's defensive bootstrap() guard (typeof chrome / per-API checks) so the pure ring-buffer + codec-check tests can import the module without a full chrome stub. Plan 04's verbatim block in the PLAN.md assumed chrome was always present — applying it as-is regressed ring-buffer + codec-check (Rule 1)." + - "REFACTOR pass: trimmed three stale 'Plan 04 wires this' comments and replaced them with the actual D-17 / Pattern 5 citations now that Plan 04 has landed." + - "T-1-04 sender-id check is documented in 4 places in recorder.ts but the SW-side enforcement is Plan 05's responsibility (the offscreen INITIATES the port; the SW is the trusting party that must validate sender)." + +patterns-established: + - "connectPort() is the single re-entry point for the port lifecycle. teardownPortTimers() runs first, fresh connect second, listeners third, timers fourth. The onDisconnect handler calls connectPort() recursively — synchronous reconnect path the Plan 02 test pins." + - "PORT_PING_MS = 25_000 keeps SW idle timer alive (< 30 s threshold). PORT_RECONNECT_MS = 290_000 pre-empts the ~5 min port lifetime cap before Chrome closes it on us." + +requirements-completed: [] # REQ-video-ring-buffer is STILL pending Plan 07 ffprobe gate; do NOT mark complete. + +# Metrics +duration: 4min +completed: 2026-05-15 +--- + +# Phase 01 Plan 04: Port Keepalive + Handshake TDD GREEN Summary + +**Long-lived `chrome.runtime.connect` port keepalive with 25 s PING loop, 290 s pre-emptive reconnect, synchronous reconnect-on-disconnect, and REQUEST_BUFFER → BUFFER response handler — flipped Plan 02 port reconnect test from RED to GREEN.** + +## Performance + +- **Duration:** ~4 min +- **Started:** 2026-05-15T15:44:22Z +- **Completed:** 2026-05-15T15:47:56Z +- **Tasks:** 3 (Task 1 verify-only, Task 2 GREEN commit, Task 3 REFACTOR commit) +- **Files modified:** 1 + +## Accomplishments + +- **GREEN gate cleared:** `tests/offscreen/port.test.ts > reconnects when port disconnects` now passes. All 4 test files in `tests/offscreen/` are GREEN (9 tests total: 4 ring-buffer + 2 codec-check + 1 handshake + 2 port). +- **Full port lifecycle:** `connectPort()` opens the port, registers `onMessage` (REQUEST_BUFFER → BUFFER) and `onDisconnect` (synchronous reconnect), schedules a 25 s PING postMessage interval, and arms a 290 s pre-emptive reconnect timer. Idempotent: reentry via `onDisconnect` tears down all timers cleanly before fresh connect. +- **D-17 contract fulfilled** at the offscreen side: long-lived port now serves as the SW keepalive (replacing the deleted `chrome.alarms` from D-18). +- **T-1-04 defense-in-depth in place:** `onPortMessage` does an explicit `typeof message === 'object'` + type-switch before destructuring. Any unknown port message shape is silently dropped. The SW-side `sender.id === chrome.runtime.id` enforcement contract is documented inline at 4 locations in `recorder.ts` for Plan 05's executor. +- **Strict-type hygiene preserved:** zero `as any`, zero `@ts-ignore`, zero fallback codec strings. `npx tsc --noEmit` exits 0. + +## TDD Gate Sequence + +| Gate | Command | Result | +|------|---------|--------| +| **RED** (pre-implementation) | `npx vitest run tests/offscreen/handshake.test.ts tests/offscreen/port.test.ts` | 1 failed / 2 passed (3 tests) — `reconnects when port disconnects` FAILS as expected. Captured to `/tmp/01-04-red.log`. | +| **GREEN** (post-implementation) | `npx vitest run` (full suite) | exit 0, 9/9 PASS across 4 test files. Captured to `/tmp/01-04-green.log`. | +| **REFACTOR** | comment cleanup in `recorder.ts` (header + PORT_NAME + keepalivePort) | 9/9 still PASS, `tsc --noEmit` clean — committed as `refactor(01-04): ...` | + +### RED log excerpt + +``` +FAIL tests/offscreen/port.test.ts > port reconnect > reconnects when port disconnects +AssertionError: expected 1 to be greater than or equal to 2 + ❯ tests/offscreen/port.test.ts:87:26 + 86| disconnectListeners.forEach((fn) => fn()); + 87| expect(connectCount).toBeGreaterThanOrEqual(2); + | ^ + + Test Files 1 failed | 1 passed (2) + Tests 1 failed | 2 passed (3) +``` + +### GREEN test output (9/9 PASS) + +``` + RUN v4.1.6 /home/parf/projects/work/repremium + + Test Files 4 passed (4) + Tests 9 passed (9) + Start at 17:47:46 + Duration 294ms +``` + +Test names now all 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` +7. `OFFSCREEN_READY handshake > sends OFFSCREEN_READY after listener registration` +8. `port reconnect > connects on module load` +9. `port reconnect > reconnects when port disconnects` ← flipped to GREEN by Plan 04 + +## Task Commits + +| # | Task | Type | Commit | +|---|------|------|--------| +| 1 | RED-verify (no commit per plan — verify-only) | — | — | +| 2 | GREEN — wire offscreen port keepalive + OFFSCREEN_READY handshake | feat | **b064a21** | +| 3 | REFACTOR — remove stale 'Plan 04 wires this' comments | refactor | **b0f4adc** | + +Final metadata commit will follow after STATE.md / ROADMAP.md updates. + +## Final `src/offscreen/recorder.ts` Shape + +- **Total lines:** 270 (was 215 after Plan 03 → +55 lines net from Plan 04) +- **Bootstrap structure:** `bootstrap()` defensive-guards each chrome.runtime sub-API, registers `onMessage` listener, calls `connectPort()`, sends `OFFSCREEN_READY`. Order matches Pattern 4 (listener-then-ready) so the SW can safely send START_RECORDING the moment OFFSCREEN_READY arrives. +- **Port lifecycle exports** (none — all internal): `connectPort`, `onPortMessage`, `teardownPortTimers`, plus the inline `onDisconnect` handler. Plan 05 grep-tests against the wire contract (PORT_NAME = 'video-keepalive', PortMessage types from `shared/types.ts`), not against this module's internal symbols. + +## Plan Acceptance Grep Gates (all PASS) + +| Gate | Expected | Actual | +|------|----------|--------| +| `function connectPort` | 1 | 1 ✓ | +| `PORT_PING_MS = 25_000` | 1 | 1 ✓ | +| `PORT_RECONNECT_MS = 290_000` | 1 | 1 ✓ | +| `REQUEST_BUFFER` | ≥1 | 1 ✓ | +| `'BUFFER'` | ≥1 | 1 ✓ | +| `isFromOwnExtension` | ≥1 | 2 ✓ (definition + call) | +| `as any` | 0 | 0 ✓ | +| `@ts-ignore` | 0 | 0 ✓ | +| `void keepalivePort` | 0 | 0 ✓ (no-unused-locals workaround removed) | +| `T-1-04` or `sender.id === chrome.runtime.id` | ≥1 | 4 ✓ | +| `chrome.runtime.connect` | ≥1 | 2 ✓ | +| `onDisconnect` | ≥1 | 2 ✓ | + +## Threat Mitigations Verified + +- **T-1-04 (port hijack from another extension):** Offscreen INITIATES the port, so the offscreen is the trusting party and the SW (Plan 05) is the listening / validating party. Plan 04 lays down: (a) an explicit `typeof message === 'object'` check before any destructure inside `onPortMessage`; (b) a type-switch on inbound `PortMessageType` that silently drops any unknown shape; (c) inline-comment documentation at 4 locations of `recorder.ts` flagging the SW-side `sender.id === chrome.runtime.id` requirement for Plan 05. +- **T-1-NEW-04-01 (port reconnect storm):** The reconnect path is idempotent: `teardownPortTimers()` clears interval + timeout, then `chrome.runtime.connect()` opens a fresh port wrapped in a `try / catch` so a transient connect failure becomes a logged warning instead of a thrown exception. The 290 s pre-emptive timer continues to retry on schedule even if a single reconnect attempt failed. + +## Decisions Made + +1. **Kept Plan 03's defensive bootstrap guard.** Plan 04's verbatim PLAN.md replacement assumed `chrome.runtime.{onMessage,connect,sendMessage}` were always present and called them unconditionally at top-level. Applying that as written regressed `tests/offscreen/ring-buffer.test.ts` (no chrome stub at all) and `tests/offscreen/codec-check.test.ts` (only `sendMessage` stubbed). Plan 04's `` explicitly requires all 4 test files to remain GREEN, so the fix was to wrap the bootstrap in the same `bootstrap()` function Plan 03 used, keeping the per-API existence checks. Production behavior is unchanged (Chrome always populates the full surface). See Deviations §1 (Rule 1). +2. **REFACTOR pass NOT skipped.** Three stale `// Plan 04 ...` comments on `PORT_NAME`, `keepalivePort`, and the module header were now misleading (they pointed forward to work that has landed). Replaced them with the actual citations (D-17 / Pattern 5 / Pattern 4). This satisfies the plan's "Comments that became stale after the bootstrap refactor" target. Plan 03's REFACTOR was skipped; Plan 04's is a meaningful (if small) tidy. +3. **T-1-04 SW-side contract documented redundantly.** The offscreen-side mitigation is defense-in-depth only; the actual enforcement lives in Plan 05. Documenting in 4 places (module header comment, port-name constant, threat-mitigation comment near `bootstrap()`, and inline at `connectPort()`) creates redundant signals so Plan 05's executor cannot miss the requirement when grepping for `T-1-04`. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Plan 04's verbatim bootstrap block regressed ring-buffer + codec-check tests** + +- **Found during:** Task 2 — first GREEN run after applying the plan's verbatim block as written +- **Issue:** Plan 04's verbatim replacement (lines 219-304 of `01-04-PLAN.md`) puts `chrome.runtime.onMessage.addListener(...)`, `connectPort()`, and `chrome.runtime.sendMessage(...)` at top-level (no guard). The pure ring-buffer test (`tests/offscreen/ring-buffer.test.ts`) imports the module without providing any `chrome` stub at all, so `chrome.runtime.onMessage.addListener(...)` at top-level throws `ReferenceError: chrome is not defined`. The codec-check test stubs only `chrome.runtime.sendMessage`, so it throws `TypeError: Cannot read properties of undefined (reading 'addListener')` on the same line. Plan 04's acceptance gate explicitly requires "ALL test files in tests/offscreen/ passing (8 tests total across 4 files)" — applying the verbatim block as written fails 4 of those 9 tests (2 in ring-buffer, 2 in codec-check). +- **Fix:** Wrapped the same bootstrap content in the `bootstrap()` function Plan 03 used, preserving (a) the `typeof chrome === 'undefined'` top-level guard and (b) the `typeof chrome.runtime.X === 'function'` per-API checks. Production behavior is unchanged because production always has the full chrome surface. The full port lifecycle (`connectPort`, `onPortMessage`, `teardownPortTimers`, ping interval, pre-emptive reconnect timer, REQUEST_BUFFER handler) is identical to the plan's verbatim block — only the outer guarding shell differs. +- **Files modified:** `src/offscreen/recorder.ts` (in Task 2 commit b064a21) +- **Verification:** `npx vitest run` → 9/9 PASS across 4 test files; `npx tsc --noEmit` exits 0; grep gates all return their expected values. + +--- + +**Total deviations:** 1 auto-fixed (1× Rule 1 bug — plan's verbatim block was inconsistent with prior test stubs) +**Impact on plan:** Single localized fix; no architectural change; no scope creep. The plan's `` `` (all 4 test files green, T-1-04 mitigations in place, no `as any` / `@ts-ignore`) are all satisfied. The verbatim block was authored without re-checking the stubs in the existing passing tests (`ring-buffer.test.ts` provides no chrome stub at all; `codec-check.test.ts` provides only `sendMessage`); restoring the guard preserves both Plan 02's RED contract and Plan 04's new GREEN contract. + +## Issues Encountered + +- **None beyond the Deviation §1 case above.** Tests, tsc, and grep gates all behaved as the plan predicted on the second attempt (after applying the guard fix). + +## Threat Flags + +None — no new security-relevant surface beyond what the plan's `` already enumerated (T-1-04 and T-1-NEW-04-01). T-1-04 enforcement on the SW side is Plan 05's explicit responsibility and is documented at 4 places in `recorder.ts`. + +## Plan 05 Handoff (CRITICAL — do not skip the sender check) + +**Plan 05 (`src/background/index.ts`) MUST implement:** + +1. **`chrome.runtime.onConnect.addListener` filter:** + ```typescript + chrome.runtime.onConnect.addListener((port) => { + if (port.name !== 'video-keepalive') return; + // T-1-04 sender check (REQUIRED — offscreen documents this contract): + if (port.sender?.id !== chrome.runtime.id) { + port.disconnect(); + return; + } + // ... attach onMessage / onDisconnect ... + }); + ``` +2. **On `SAVE_ARCHIVE`:** send `{ type: 'REQUEST_BUFFER' }` over the kept-open port; resolve the `saveArchive()` Promise inside the `port.onMessage` handler when `{ type: 'BUFFER', chunks: [...] }` arrives. +3. **Handle SW unload window:** the offscreen reconnects on disconnect; SW should NOT cache the port reference across unloads — re-bind in `onConnect` each time. +4. **Delete `setupKeepalive` + alarms code** (audit P1 #8 / D-18) — Plan 03 left it intact; Plan 05 owns its removal. + +## Next Phase Readiness + +- REQ-video-ring-buffer remains incomplete pending **Plan 07 ffprobe gate** (a fresh-export sample must pass `ffprobe -v error -f matroska -i last_30sec.webm` exit 0 — D-12). Do NOT mark the requirement complete. +- Plan 05 (SW-side port host + SW shrink) can now proceed against a stable offscreen-side contract: `PORT_NAME = 'video-keepalive'`, inbound `PortMessage` = REQUEST_BUFFER, outbound `PortMessage` = PING / BUFFER. + +## Self-Check: PASSED + +- `[x] src/offscreen/recorder.ts` exists at expected path (`test -f` exits 0; 270 lines) +- `[x] Commit b064a21` exists in git log (`git log --oneline | grep b064a21` returns 1) +- `[x] Commit b0f4adc` exists in git log (`git log --oneline | grep b0f4adc` returns 1) +- `[x] All 4 test files pass` (verified by `/tmp/01-04-green.log` + live re-run) +- `[x] tsc --noEmit exits 0` +- `[x] T-1-04 SW-side contract` documented in 4 places (grep count = 4) +- `[x] void keepalivePort` no-unused-locals workaround removed (grep count = 0) +- `[x] No as any / @ts-ignore` introduced (both counts = 0) + +--- +*Phase: 01-stabilize-video-pipeline* +*Completed: 2026-05-15*