mokosh/.planning/phases/01-stabilize-video-pipeline/01-04-SUMMARY.md

---
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 `<acceptance_criteria>` 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 `<verification>` `<success_criteria>` (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 `<threat_model>` 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*