--- 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*