--- phase: 01-stabilize-video-pipeline plan: 04 type: tdd wave: 1 depends_on: ["02", "03"] files_modified: - src/offscreen/recorder.ts autonomous: true requirements: - REQ-video-ring-buffer requirements_addressed: - REQ-video-ring-buffer must_haves: truths: - "On module import, the offscreen opens exactly one port via `chrome.runtime.connect({ name: 'video-keepalive' })` AND emits exactly one `OFFSCREEN_READY` message via `chrome.runtime.sendMessage`" - "Both the port-connect and the OFFSCREEN_READY send happen AFTER `chrome.runtime.onMessage.addListener` registration (Pattern 4 ordering)" - "When the open port fires its registered `onDisconnect` listener, the module synchronously reconnects (Pitfall 4 mitigation)" - "The port emits a `{ type: 'PING' }` postMessage on a ≤ 25 s interval (RESEARCH.md Pattern 5)" - "Pre-emptive reconnect runs every 290 s (belt-and-braces against the ~5 min port-lifetime cap)" - "The port handles incoming `REQUEST_BUFFER` messages by responding with `{ type: 'BUFFER', chunks: }`" - "Running `npx vitest run tests/offscreen/handshake.test.ts tests/offscreen/port.test.ts` exits 0 (Plan 02 RED → GREEN)" artifacts: - path: "src/offscreen/recorder.ts" provides: "Full port keepalive + handshake + REQUEST_BUFFER handler" contains: "video-keepalive" key_links: - from: "src/offscreen/recorder.ts (port.onDisconnect)" to: "src/offscreen/recorder.ts (connectPort)" via: "synchronous reconnect call" pattern: "onDisconnect" - from: "src/offscreen/recorder.ts (REQUEST_BUFFER handler)" to: "getBuffer()" via: "port.postMessage({ type: 'BUFFER', chunks: getBuffer() })" pattern: "REQUEST_BUFFER" --- Replace the import-time STUB bootstrap in Plan 03's `src/offscreen/recorder.ts` with the full long-lived-port keepalive + OFFSCREEN_READY handshake + REQUEST_BUFFER handler. This flips the two remaining RED tests from Plan 02 (`tests/offscreen/handshake.test.ts` and `tests/offscreen/port.test.ts`) to GREEN. Purpose: REQ-video-ring-buffer also covers the SW-keepalive contract per D-17 (long-lived port replaces `chrome.alarms`) and the OFFSCREEN_READY handshake per RESEARCH.md Pattern 4. Both behaviors are observable from a pure-Node test that mocks `chrome.runtime` — exactly the TDD sweet spot for a contract that survives SW unloads, port 5-min cap, and offscreen bootstrap races. Output: Updated `src/offscreen/recorder.ts` — only this single file is touched. Plan 04 is an isolated unit-of-work; Plan 05 picks up the SW-side `onConnect` handler in parallel (Plan 04 runs alongside Plan 05 in Wave 1 with NO file overlap — Plan 04 owns offscreen-side, Plan 05 owns SW-side). @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md @.planning/phases/01-stabilize-video-pipeline/01-CONTEXT.md @.planning/phases/01-stabilize-video-pipeline/01-RESEARCH.md @.planning/phases/01-stabilize-video-pipeline/01-PATTERNS.md @tests/offscreen/handshake.test.ts @tests/offscreen/port.test.ts @src/offscreen/recorder.ts Module-import side-effects (from tests/offscreen/handshake.test.ts): - `chrome.runtime.onMessage.addListener` is called at least once - `chrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' })` is called exactly once Module-import side-effects (from tests/offscreen/port.test.ts): - `chrome.runtime.connect({ name: 'video-keepalive' })` is called exactly once (first test) - Firing the registered onDisconnect listener causes `chrome.runtime.connect` to be called a second time within the synchronous tick (second test) In-port message contract (per RESEARCH.md Pattern 5): - Outbound: `{ type: 'PING' }` every 25 s (configurable constant) - Outbound on REQUEST_BUFFER: `{ type: 'BUFFER', chunks: VideoChunk[] }` (the chunks are `getBuffer()`) - Inbound listener: handles `{ type: 'REQUEST_BUFFER' }` from SW side Existing exports unchanged: `addChunk`, `trimAged`, `getBuffer`, `resetBuffer`, `assertCodecSupported`, `VIDEO_BUFFER_DURATION_MS` (Plan 03's tests must continue to pass). ## Trust Boundaries | Boundary | Description | |----------|-------------| | port connection origin | Any context inside the extension can call `chrome.runtime.connect`; non-extension contexts cannot (Chrome enforces). The offscreen-side connect is initiated BY the offscreen, so the offscreen is the trusting party (it knows where the port goes). The SW-side `onConnect` (in Plan 05) is the listening party — that side validates `sender.id === chrome.runtime.id`. | | inbound port `onMessage` payload | The offscreen receives `REQUEST_BUFFER` over the port; the SW is the only caller (sender validated on SW side); inbound message validation here is defense-in-depth | ## STRIDE Threat Register | Threat ID | Category | Component | Disposition | Mitigation Plan | |-----------|----------|-----------|-------------|-----------------| | T-1-04 | Spoofing — port hijack | Offscreen-side `port.onMessage` listener | mitigate | The port name `'video-keepalive'` is paired with the SW-side `onConnect` filter `p.name === 'video-keepalive' && p.sender?.id === chrome.runtime.id` (added in Plan 05). On the offscreen side: the port is opened BY the offscreen, so the offscreen is the trusting party and the SW is the only legitimate counterparty. As defense in depth, the offscreen-side `port.onMessage` handler explicitly switches on `msg.type` and ignores any unknown shape. Grep gate (Plan 05 owns the sender check, Plan 04 owns the type-switch on inbound port traffic): `grep -v '^#' src/offscreen/recorder.ts \| grep -cE "REQUEST_BUFFER" ` returns at least 1, AND any branch outside the known PortMessageType union is treated as no-op. | | T-1-NEW-04-01 | DoS — port reconnect storm | offscreen `connectPort` → port.onDisconnect | mitigate | The reconnect path is idempotent: when `onDisconnect` fires, the existing port reference is cleared, a fresh port is opened, and the new port's listeners are attached. If reconnect throws (e.g., SW gone), the failure is caught and logged; the next pre-emptive 290 s reconnect cycle will retry. Grep gate: `grep -v '^#' src/offscreen/recorder.ts \| grep -cE "function connectPort"` returns 1. | Port keepalive + OFFSCREEN_READY handshake + REQUEST_BUFFER handler src/offscreen/recorder.ts RED already exists (Plan 02 handshake + port tests). GREEN must: Handshake (from tests/offscreen/handshake.test.ts): - On module import, register `chrome.runtime.onMessage.addListener` BEFORE calling `chrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' })` - `OFFSCREEN_READY` is sent EXACTLY ONCE during module bootstrap (not on every subsequent message) Port (from tests/offscreen/port.test.ts): - On module import, call `chrome.runtime.connect({ name: 'video-keepalive' })` exactly once - Register `onDisconnect` listener on the connected port - When `onDisconnect` fires, immediately call `chrome.runtime.connect({ name: 'video-keepalive' })` again (synchronous reconnect) - The reconnect attaches the same set of listeners (onMessage, onDisconnect, etc.) to the fresh port Production behavior (not directly tested in Plan 02, but verifiable): - Every 25 s while a port is open: `port.postMessage({ type: 'PING' })` (keepalive against SW idle) - Every 290 s: pre-emptively close + reopen the port (Pitfall 4 — beat the ~5 min lifetime cap) - On inbound port message `{ type: 'REQUEST_BUFFER' }`: respond with `{ type: 'BUFFER', chunks: getBuffer() }` (this is the SW's export-time data-pull path; Plan 05 wires the SW caller) See task list — three tasks: RED-verify the two port + handshake test files, GREEN refactor the bootstrap section of recorder.ts to add the port lifecycle, then REFACTOR (skipped or minimal). Task 1: RED-verify — confirm port + handshake tests fail on Plan 03's stub bootstrap (none modified) - tests/offscreen/handshake.test.ts (Plan 02 output) - tests/offscreen/port.test.ts (Plan 02 output) - src/offscreen/recorder.ts (Plan 03 stub bootstrap — lines 144-155 of the Task-2 verbatim module) Run the two test files this plan owns: ```bash npx vitest run tests/offscreen/handshake.test.ts tests/offscreen/port.test.ts 2>&1 | tee /tmp/01-04-red.log ``` Analyse the failure mode: - **Handshake test:** Plan 03's stub bootstrap already calls `chrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' })` once after the listener registration, so this test MAY ALREADY PASS as of Plan 03's GREEN landing. If it does — that is acceptable. The handshake contract is sufficiently met by Plan 03's stub. - **Port test (`connects on module load`):** Plan 03's stub calls `chrome.runtime.connect({ name: PORT_NAME })` once — this test MAY ALREADY PASS too. - **Port test (`reconnects when port disconnects`):** Plan 03's stub does NOT register `onDisconnect`, so when the test fires the registered listeners, `connectCount` stays at 1 — this test MUST FAIL. The intended RED gate for Plan 04 is the reconnect test. Confirm: ```bash grep -q "reconnects when port disconnects" /tmp/01-04-red.log && grep -q "FAIL" /tmp/01-04-red.log ``` If the reconnect test passes spuriously (e.g., because Plan 03 already added reconnect): STOP — the plan ordering may have shifted; reconcile with Plan 03 before continuing. If the handshake test or the `connects on module load` test fail: STOP — Plan 03 broke its own contract; revisit Plan 03 instead. Otherwise, proceed to Task 2. No commits in this task. npx vitest run tests/offscreen/port.test.ts -t "reconnects when port disconnects" 2>&1 | grep -qE "FAIL|✗" - `/tmp/01-04-red.log` exists and is non-empty - The reconnect test is in a FAIL state - The handshake test and the `connects on module load` port test outcomes are recorded in /tmp/01-04-red.log (whether they pass or fail — we don't gate on them here) RED gate confirmed: the reconnect test is failing because Plan 03's stub bootstrap doesn't reconnect on disconnect. Proceed to GREEN. Task 2: GREEN — replace stub bootstrap with full port lifecycle src/offscreen/recorder.ts - src/offscreen/recorder.ts (Plan 03 output — the bootstrap section starting at the `// ─── Bootstrap` comment through the `void keepalivePort;` line) - .planning/phases/01-stabilize-video-pipeline/01-RESEARCH.md §"Pattern 5: Long-lived port" (lines 590-648) and §"Pitfall 4" (lines 820-836) - .planning/phases/01-stabilize-video-pipeline/01-PATTERNS.md §`src/offscreen/recorder.ts` — port keepalive snippet (lines 185-198) - tests/offscreen/handshake.test.ts (the test that pins OFFSCREEN_READY ordering) - tests/offscreen/port.test.ts (the test that pins reconnect-on-disconnect) After Task 2 lands: ```bash npx vitest run tests/offscreen/handshake.test.ts tests/offscreen/port.test.ts # exit 0 — all 3 tests pass (1 in handshake, 2 in port) npx vitest run tests/offscreen/ring-buffer.test.ts tests/offscreen/codec-check.test.ts # exit 0 — ring-buffer and codec tests still green (NO regression from Plan 03) ``` Edit `src/offscreen/recorder.ts` to REPLACE the entire bootstrap section (currently starting at the `// ─── Bootstrap (Plan 04 wires the full port + handshake)` comment through the `void keepalivePort;` line) with the VERBATIM expanded bootstrap below. The pure-function exports above the bootstrap section and the D-13 fallback comment block below it are UNCHANGED. Also: at the top of the file, ADD a new constant block (after the existing `const PORT_NAME = ...` line and before the `const logger = new OffscreenLogger(...)` line): ```typescript const PORT_PING_MS = 25_000; // < 30 s SW idle threshold const PORT_RECONNECT_MS = 290_000; // pre-empt the ~5 min port cap (Pitfall 4) ``` The bootstrap section becomes VERBATIM: ```typescript // ─── Bootstrap: handshake + port lifecycle (D-17, RESEARCH.md Patterns 4 & 5) ── function isFromOwnExtension(sender: chrome.runtime.MessageSender | undefined): boolean { return sender?.id === chrome.runtime.id; } chrome.runtime.onMessage.addListener((message: Message, sender, sendResponse) => { if (!isFromOwnExtension(sender)) { return false; } switch (message.type) { case 'START_RECORDING': startRecording().then(() => sendResponse({ ok: true })).catch((err) => sendResponse({ ok: false, error: String(err) })); return true; case 'STOP_RECORDING': stopRecording(); sendResponse({ ok: true }); return false; default: return false; } }); // Stable handles for the ping interval and the pre-emptive reconnect timer, // so we can clear them on disconnect / re-init. let pingIntervalId: ReturnType | null = null; let preemptiveReconnectId: ReturnType | null = null; function teardownPortTimers(): void { if (pingIntervalId !== null) { clearInterval(pingIntervalId); pingIntervalId = null; } if (preemptiveReconnectId !== null) { clearTimeout(preemptiveReconnectId); preemptiveReconnectId = null; } } function onPortMessage(message: unknown): void { // Defense-in-depth: explicit shape check before destructuring if (typeof message !== 'object' || message === null) { return; } const type = (message as { type?: unknown }).type; if (type === 'REQUEST_BUFFER') { if (keepalivePort !== null) { keepalivePort.postMessage({ type: 'BUFFER', chunks: getBuffer() }); } } // Any unknown port message type is silently dropped (T-1-04 defense-in-depth). } function connectPort(): void { teardownPortTimers(); try { keepalivePort = chrome.runtime.connect({ name: PORT_NAME }); } catch (err) { logger.error('port connect failed:', err); keepalivePort = null; return; } keepalivePort.onMessage.addListener(onPortMessage); keepalivePort.onDisconnect.addListener(() => { logger.warn('port disconnected — reconnecting'); teardownPortTimers(); keepalivePort = null; // Synchronous reconnect — tests/offscreen/port.test.ts pins this connectPort(); }); pingIntervalId = setInterval(() => { keepalivePort?.postMessage({ type: 'PING' }); }, PORT_PING_MS); preemptiveReconnectId = setTimeout(() => { logger.log('pre-emptive port reconnect (290 s cap)'); keepalivePort?.disconnect(); // onDisconnect handler above triggers a fresh connectPort() call }, PORT_RECONNECT_MS); } // Open the first port and emit OFFSCREEN_READY. Order matters per Pattern 4: // the onMessage listener registration above already happened, so the SW can // safely send START_RECORDING the moment it sees OFFSCREEN_READY. connectPort(); chrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' }); ``` After this replacement, the previous `let keepalivePort: chrome.runtime.Port | null = null;` declaration in the module-level state block is UNCHANGED — it stays. The previous `void keepalivePort;` line is REMOVED (the variable is now used by `onPortMessage` and `connectPort`, so the no-unused-locals workaround is no longer needed). Run: ```bash npx vitest run npx tsc --noEmit ``` Both MUST exit 0. Notes: - `setInterval` / `setTimeout` in offscreen documents: offscreen runs in a real DOM context (NOT in the SW), so timers are reliable — unlike `setTimeout` inside a SW. RESEARCH.md "Don't Hand-Roll" table confirms this. - The `disconnect()` call inside the pre-emptive reconnect uses optional chaining; if the port is somehow already null, the call is a no-op. The `onDisconnect` listener fires synchronously in Chrome's port-disconnect path, which transitions us back through `connectPort()` cleanly. - `PortMessage` shape: outbound traffic uses the `PortMessage` type added to `src/shared/types.ts` in Plan 03. No `as any`. Inbound messages over the port are typed `unknown` and shape-checked. npx vitest run && npx tsc --noEmit - `npx vitest run` exits 0 with ALL test files in tests/offscreen/ passing (8 tests total across 4 files: 4 ring-buffer + 2 codec-check + 1 handshake + 2 port) - `npx tsc --noEmit` exits 0 - `grep -c "function connectPort" src/offscreen/recorder.ts` returns 1 - `grep -c "PORT_PING_MS = 25_000" src/offscreen/recorder.ts` returns 1 - `grep -c "PORT_RECONNECT_MS = 290_000" src/offscreen/recorder.ts` returns 1 - `grep -c "REQUEST_BUFFER" src/offscreen/recorder.ts` returns at least 1 (the inbound handler) - `grep -c "'BUFFER'" src/offscreen/recorder.ts` returns at least 1 (the outbound response shape) - `grep -c "isFromOwnExtension" src/offscreen/recorder.ts` returns at least 1 (sender check on onMessage — T-1-04 defense-in-depth) - `grep -c "as any" src/offscreen/recorder.ts` returns 0 - `grep -c "@ts-ignore" src/offscreen/recorder.ts` returns 0 - `grep -c "void keepalivePort" src/offscreen/recorder.ts` returns 0 (the no-unused-locals workaround is removed) All 4 test files from Plan 02 are now GREEN. The offscreen module owns the full port lifecycle. Plan 05 can wire the SW-side `onConnect` handler against the same port name with confidence. Task 3: REFACTOR — optional cleanup or no-op src/offscreen/recorder.ts (only if changed) - src/offscreen/recorder.ts (Task 2 output) Per the TDD REFACTOR phase rules: 1. Re-read `src/offscreen/recorder.ts` (the full module now that Plan 04 has landed). 2. Look ONLY for obvious improvements: - Constants that should be hoisted into a shared block - Type assertions that could be replaced with proper type narrowing - Comments that became stale after the bootstrap refactor 3. If no obvious improvements: SKIP THIS TASK (do not commit). Note in summary: "Refactor: no changes needed." 4. If improvements found: make them; run `npx vitest run && npx tsc --noEmit`; commit ONLY if both exit 0. Do NOT use this task to add features. Plan 05 picks up the SW-side wiring — do not anticipate it in this module. npx vitest run && npx tsc --noEmit - All 4 test files still pass after the refactor pass (or after the no-op decision) - `npx tsc --noEmit` still exits 0 - The SUMMARY.md documents whether refactor happened or was skipped REFACTOR phase complete (or explicitly skipped with rationale). After all three tasks land: 1. `npx vitest run` — exits 0 with ALL 8 tests passing across the 4 test files in tests/offscreen/. 2. `npx tsc --noEmit` — exits 0. 3. `grep -c "as any\|@ts-ignore" src/offscreen/recorder.ts` returns 0. 4. The grep gates listed in Task 2's `acceptance_criteria` all return their expected values. 5. The module's line count grew modestly (Task 2 adds ~40 lines net to the bootstrap section); the file remains well under 250 lines total. Commit cadence: - Task 1: no commit. - Task 2: ONE commit (`feat(01-04): wire offscreen port keepalive and OFFSCREEN_READY handshake`). - Task 3: zero or one commit (`refactor(01-04): {what was cleaned}`). Total: 1-2 commits. - All 4 Plan-02 test files green (ring-buffer, codec-check, handshake, port) — 8 tests total - `src/offscreen/recorder.ts` contains a single `connectPort()` function that handles connect, reconnect, REQUEST_BUFFER response, and pre-emptive reconnect - T-1-04 mitigations in place: sender-id check on onMessage; type-switch on inbound port messages - No `as any`, no `@ts-ignore` - D-17 contract (long-lived port replacing alarms) is implemented at the offscreen side; SW side lands in Plan 05 After completion, create `.planning/phases/01-stabilize-video-pipeline/01-04-SUMMARY.md` with: - RED: log excerpt from Task 1 showing the reconnect test failing - GREEN: full vitest output showing all 8 tests passing - REFACTOR: what changed (if anything) in Task 3 - Final line count for src/offscreen/recorder.ts - Note: "Plan 05 must add a corresponding `chrome.runtime.onConnect.addListener` in `src/background/index.ts` that (a) filters by `port.name === 'video-keepalive'`, (b) validates `port.sender?.id === chrome.runtime.id` (T-1-04), (c) sends `{ type: 'REQUEST_BUFFER' }` over the port when SAVE_ARCHIVE arrives, and (d) handles the incoming `BUFFER` response by resolving the saveArchive Promise." - Commit list (1-2 commits)