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

---
phase: 01-stabilize-video-pipeline
plan: 05
subsystem: service-worker-coordinator
tags: [chrome-extension, mv3, service-worker, long-lived-port, offscreen-document, t-1-04, security-mitigation]

# Dependency graph
requires:
  - phase: 01-stabilize-video-pipeline
    provides: "Plan 03 ring-buffer ownership moved to offscreen; Plan 03 inline SW cleanup of VIDEO_CHUNK / VIDEO_CHUNK_SAVED / openIndexedDB / loadChunkFromIndexedDB; Plan 04 offscreen-side port keepalive + OFFSCREEN_READY handshake"
provides:
  - "Shrunk src/background/index.ts (coordinator only): offscreen lifecycle, port host, export-time buffer fetch — zero local buffer state"
  - "SW-side chrome.runtime.onConnect listener bound to 'video-keepalive' port (Plan 04 counterparty) with T-1-04 sender-id check"
  - "Async getVideoBufferFromOffscreen(): REQUEST_BUFFER → BUFFER round-trip with 2s timeout; powers GET_VIDEO_BUFFER and SAVE_ARCHIVE handlers"
  - "OFFSCREEN_READY handshake handler resolving a module-level Promise; startVideoCapture awaits it before sending START_RECORDING (audit P1 #12 fix)"
  - "Idempotent indexedDB.deleteDatabase('VideoRecorderDB') in onInstalled (T-1-NEW-05-02; RESEARCH.md Runtime State Inventory)"
  - "chrome.offscreen.hasDocument() check on SW init for robust offscreenCreated state across SW respawns (audit P1 #8)"
  - "T-1-NEW-05-01 onMessage sender.id guard at handler top"
affects: [01-06-vite-config, 01-07-ffprobe-gate]

# Tech tracking
tech-stack:
  added: []
  patterns:
    - "SW-as-pure-coordinator: zero local state for chunks; offscreen is the single source of truth (D-16)"
    - "Promise-based handshake (offscreenReady) so popup's transient user-activation isn't lost to bootstrap race"
    - "Port-based async RPC via REQUEST_BUFFER → BUFFER with per-call onMessage listener + 2 s timeout fallback"
    - "Defense-in-depth on both onConnect (port name + port.sender?.id) and onMessage (sender.id) — T-1-04 mitigations on both surfaces"
    - "instanceof Error pattern in catches instead of '(error as any).message' — eliminates audit P1 #13 instances in this file"

key-files:
  created: []
  modified:
    - "src/background/index.ts (388 → 491 lines; ~ +130 / -100 net; structural shrink — buffer + alarms + IDB + tabCapture paths went away; port host + handshake + IDB cleanup + hasDocument check + sender guards came in)"

key-decisions:
  - "checkPermissions / requestPermissions deleted entirely (Rule 1 deviation): they referenced 'tabCapture' permission which was swapped to 'desktopCapture' in manifest (D-A6). Under getDisplayMedia (D-01) no runtime-permission check is meaningful — the browser prompts via picker on user gesture. REQUEST_PERMISSIONS now just calls startVideoCapture() and returns granted:true. Plan only mandated removing the getMediaStreamId call; the broken permissions code was logically downstream."
  - "Synchronous getVideoBuffer() kept as an empty-array stub in Task 1 (to keep tsc clean while videoBuffer module state was deleted), then replaced with async getVideoBufferFromOffscreen() in Task 2. The plan acknowledges this two-step in Task 2 step (4)."
  - "Added chrome.offscreen.hasDocument() check inside initialize() (Rule 2 robustness — orchestrator-flagged audit P1 #8). The check is guarded with `typeof chrome.offscreen?.hasDocument === 'function'` so it stays safe across versions and partial stubs. Plan did not explicitly require this, but the orchestrator's instructions did."
  - "Fixed two `as any` casts in the existing code that the plan didn't strictly require but CLAUDE.md mandates: (a) the catch block in ensureOffscreen now uses `error instanceof Error` instead of `(error as any).message`; (b) chrome.tabs.sendMessage cast moved from `as any` to an explicit `{ events?, userEvents? } | undefined` type. Both are pre-existing audit P1 #13 instances; cleaning them now closes a deviation surface for Phase 5."
  - "Did NOT mark REQ-video-ring-buffer complete: Plan 07 owns the ffprobe gate that proves end-to-end requirement satisfaction."

patterns-established:
  - "Port lifecycle on SW side: onConnect → port-name filter → sender.id filter → store in videoPort → register onDisconnect (null out reference) → done. Each request installs a one-shot onMessage listener via getVideoBufferFromOffscreen and removes it on completion or timeout."
  - "Handshake pattern: module-level Promise + resolve closure captured at module load. OFFSCREEN_READY case calls resolve() and nulls out the closure (one-shot semantics). startVideoCapture awaits it after ensureOffscreen()."
  - "T-1-04 enforcement: when SW is the trusting party of a port opened by offscreen, validate `port.sender?.id === chrome.runtime.id`. When SW is the trusting party of a runtime message, validate `sender.id === chrome.runtime.id`. Both checks are now in place."

requirements-completed: []  # REQ-video-ring-buffer still pending Plan 07 ffprobe gate; this plan is structural plumbing only.

# Metrics
duration: 8min
completed: 2026-05-15
---

# Phase 01 Plan 05: SW Shrink + Port Host Summary

**Shrunk `src/background/index.ts` to a pure coordinator: deleted legacy buffer / alarms / IndexedDB / tabCapture code paths (Task 1) and wired the SW-side counterparty of the long-lived `'video-keepalive'` port (Task 2) — with T-1-04 sender-id guards on both onConnect and onMessage, an `OFFSCREEN_READY` handshake, `chrome.offscreen.hasDocument()` re-sync on SW respawn, and an idempotent `indexedDB.deleteDatabase('VideoRecorderDB')` cleanup on install.**

## Performance

- **Duration:** ~8 min
- **Started:** 2026-05-15T15:53:30Z (immediately after Plan 04 completed)
- **Completed:** 2026-05-15T16:03:00Z
- **Tasks:** 2 (DELETE pass + ADD pass)
- **Commits:** 2 (one per task)
- **Files modified:** 1 (`src/background/index.ts`)

## Before / After Line Count

| Snapshot | Lines | Notes |
|----------|-------|-------|
| Pre-Plan 05 (post-Plan 03 inline cleanup) | **436** | Plan 03's executor had already removed VIDEO_CHUNK / VIDEO_CHUNK_SAVED / openIndexedDB / loadChunkFromIndexedDB inline as Rule-3 blocking-fix deps. |
| Post-Task 1 (DELETE pass) | 387 | -49 lines: deletions of videoBuffer, setupKeepalive, chrome.tabCapture.getMediaStreamId, checkPermissions, requestPermissions, USER_MEDIA cast, two `as any` casts. |
| Post-Task 2 (ADD pass) | **491** | +104 lines: onConnect handler, getVideoBufferFromOffscreen, offscreenReady Promise, OFFSCREEN_READY case, sender-id guards, indexedDB.deleteDatabase, hasDocument check, comments documenting each mitigation. |

The plan estimated 380-440 lines post-Task 2. The actual count is 491 because (a) I added the orchestrator-requested `chrome.offscreen.hasDocument()` block (~15 lines including comments) and (b) I expanded the comments around each security mitigation to make T-1-04 / T-1-NEW-05-01 / T-1-NEW-05-02 / audit P1 #8 / audit P1 #12 references explicit for future auditors. The structural shrink (zero buffer state, zero alarms, zero IDB plumbing, zero tabCapture) is intact — the line-count overshoot is documentation, not code.

## Task 1: Deletions

**Commit:** `886376e refactor(01-05): delete legacy SW buffer, alarms, IndexedDB, tabCapture paths`

| Deleted Symbol / Path | Reason |
|-----------------------|--------|
| `let videoBuffer: VideoChunk[] = []` | D-16 — buffer ownership moved to offscreen |
| `function setupKeepalive` + `chrome.alarms.create('keepalive', ...)` + `chrome.alarms.onAlarm.addListener` | D-18 / audit P1 #8 — alarms never reset SW idle timer; port does |
| Call site `setupKeepalive()` inside `initialize` | Paired with the function delete |
| `chrome.tabCapture.getMediaStreamId({...})` block inside `startVideoCapture` (and its `as any` cast) | D-01 — getDisplayMedia now runs inside the offscreen document |
| `async function checkPermissions` | Referenced `'tabCapture'` (removed from manifest by D-A6); under getDisplayMedia no runtime perm check is meaningful |
| `async function requestPermissions` | Same reason. The popup user-gesture flows directly into startVideoCapture now |
| `reasons: ['USER_MEDIA'] as any` in createDocument | Replaced by canonical `[chrome.offscreen.Reason.DISPLAY_MEDIA]` (D-02; @types/chrome 0.0.268 exposes the enum) |
| `(error as any).message?.includes(...)` in createDocument catch | Replaced by `error instanceof Error ? error.message : String(error)` (CLAUDE.md no-`as any` rule) |
| `as any` cast on `chrome.tabs.sendMessage(...)` response | Replaced by an explicit `{ events?: unknown[]; userEvents?: unknown[] } | undefined` type |
| Justification string `'Need to record video from tab for error reporting'` | Replaced with `'Continuous screen recording for operator session diagnostics'` to match the new capture semantics (D-04 — NOT silent) |

**Note (already done by Plan 03):** `addVideoChunkFromBlob`, `cleanupVideoBuffer`, `firstChunkSaved`, `VIDEO_BUFFER_DURATION_MS`, the `VIDEO_CHUNK` and `VIDEO_CHUNK_SAVED` case branches, `loadChunkFromIndexedDB`, `openIndexedDB`, and the chrome.tabs.onActivated handler were ALREADY removed by Plan 03's Rule-3 inline cleanup (logged in STATE.md decisions). Plan 05 verified these via grep gates and removed only the residual placeholder comments.

## Task 2: Additions

**Commit:** `5cd1519 feat(01-05): wire SW-side port host and port-based buffer fetch`

### onMessage switch — final shape

```typescript
chrome.runtime.onMessage.addListener((message: Message, sender, sendResponse) => {
  // T-1-NEW-05-01 mitigation: only accept onMessage traffic from this extension
  if (sender.id !== chrome.runtime.id) {
    logger.warn('Rejecting message with mismatched sender:', sender.id);
    return false;
  }
  logger.log('Received message:', message.type, message);

  switch (message.type) {
    case 'REQUEST_PERMISSIONS':  // → startVideoCapture(), respond {granted: true|false}
    case 'GET_VIDEO_BUFFER':     // → getVideoBufferFromOffscreen() → sendResponse(resp)
    case 'SAVE_ARCHIVE':         // → saveArchive() → sendResponse(result)
    case 'OFFSCREEN_READY':      // → offscreenReadyResolve?.(); offscreenReadyResolve = null
    default:                     // → logger.warn('Unknown message type:', ...), return false
  }
});
```

### chrome.offscreen.createDocument — as committed

```typescript
await chrome.offscreen.createDocument({
  url: url,
  reasons: [chrome.offscreen.Reason.DISPLAY_MEDIA],
  justification: 'Continuous screen recording for operator session diagnostics'
});
```

(Plans 06 / 07 can grep against `chrome.offscreen.Reason.DISPLAY_MEDIA` to confirm D-02 wiring.)

### onConnect — port host

```typescript
chrome.runtime.onConnect.addListener((port) => {
  if (port.name !== 'video-keepalive') return;
  if (port.sender?.id !== chrome.runtime.id) {  // T-1-04
    port.disconnect();
    return;
  }
  videoPort = port;
  port.onDisconnect.addListener(() => { videoPort = null; });
});
```

### getVideoBufferFromOffscreen — port-based RPC

```typescript
async function getVideoBufferFromOffscreen(): Promise<VideoBufferResponse> {
  if (videoPort === null) return { chunks: [] };
  const port = videoPort;
  return new Promise<VideoBufferResponse>((resolve) => {
    const timer = setTimeout(() => {
      port.onMessage.removeListener(handler);
      resolve({ chunks: [] });   // 2 s timeout fallback
    }, BUFFER_FETCH_TIMEOUT_MS);
    const handler = (msg: unknown) => {
      if (typeof msg === 'object' && msg !== null && (msg as { type?: unknown }).type === 'BUFFER') {
        clearTimeout(timer);
        port.onMessage.removeListener(handler);
        const chunks = (msg as { chunks?: VideoChunk[] }).chunks ?? [];
        resolve({ chunks });
      }
    };
    port.onMessage.addListener(handler);
    port.postMessage({ type: 'REQUEST_BUFFER' });
  });
}
```

### onInstalled — orphan IDB cleanup

```typescript
chrome.runtime.onInstalled.addListener((details) => {
  // T-1-NEW-05-02 / RESEARCH.md Runtime State Inventory
  try {
    indexedDB.deleteDatabase('VideoRecorderDB');
  } catch (e) {
    logger.warn('IDB cleanup failed:', e);
  }
  initialize();
});
```

### initialize() — hasDocument re-sync (P1 #8)

```typescript
async function initialize() {
  // After SW respawn, offscreenCreated resets to false but the offscreen
  // document may still exist. Ask Chrome the ground truth.
  try {
    if (typeof chrome.offscreen?.hasDocument === 'function') {
      const exists = await chrome.offscreen.hasDocument();
      if (exists) offscreenCreated = true;
    }
  } catch (err) { logger.warn(...); }
}
```

## Verification

### tsc + vitest (both green)

```
$ npx tsc --noEmit
(exit 0, no output)

$ npx vitest run
 Test Files  4 passed (4)
      Tests  9 passed (9)
   Start at  18:03:02
   Duration  319ms
```

All 9 tests passing across 4 files in `tests/offscreen/` — Plan 04's offscreen-side stayed untouched, so port + handshake + ring-buffer + codec-check tests all green as expected.

### Grep gates (all pass)

| Gate | Expected | Actual |
|------|----------|--------|
| `chrome.alarms` in src/background/ | 0 | 0 |
| `VideoRecorderDB` / `openIndexedDB` / `loadChunkFromIndexedDB` in src/ (excluding cleanup call) | 0 outside cleanup | 2 (both the cleanup call + its log message) — only the cleanup path remains |
| `setupKeepalive` / `addVideoChunkFromBlob` / `cleanupVideoBuffer` / `tabCapture` / `getMediaStreamId` in src/background/ | 0 | 0 |
| `chrome.tabs.onActivated` / `chrome.tabs.onUpdated` in src/background/ | 0 | 0 (D-14 / D-15 satisfied) |
| `chrome.runtime.onConnect.addListener` in src/background/index.ts | 1 | 1 |
| `'video-keepalive'` in src/background/index.ts | ≥1 | 1 |
| `port.sender?.id !== chrome.runtime.id` (T-1-04) | 1 | 1 |
| `sender.id !== chrome.runtime.id` (T-1-NEW-05-01) | 1 | 1 |
| `indexedDB.deleteDatabase('VideoRecorderDB')` | 1 | 1 |
| `chrome.offscreen.hasDocument` (audit P1 #8) | ≥1 | 1 (the typeof + call) |
| `function getVideoBufferFromOffscreen` | 1 | 1 |
| `OFFSCREEN_READY` mentions | ≥1 | 3 (Promise comment + case label + log message) |
| `offscreenReady` mentions | ≥2 | 6 (Promise var + resolve closure + await + case label + log + comment) |
| `as any` in src/background/ | 0 | 0 |
| `@ts-ignore` in src/background/ | 0 | 0 |

## Deviations from Plan

### Rule 1 — Bug fixes (auto-applied)

**1. [Rule 1 - Bug] Deleted broken `checkPermissions` / `requestPermissions` flow**
- **Found during:** Task 1.
- **Issue:** `chrome.permissions.contains({ permissions: ['tabCapture'] })` references a permission that was removed from `manifest.json` by D-A6 (replaced with `desktopCapture`). The check would always return `false`, sending `REQUEST_PERMISSIONS` into the never-granted branch, which itself calls `chrome.permissions.request({ permissions: ['tabCapture'] })` — same problem. Recording could not start cleanly.
- **Fix:** Deleted both functions entirely. `REQUEST_PERMISSIONS` now just calls `startVideoCapture()` (which goes through ensureOffscreen → DISPLAY_MEDIA reason → offscreen → getDisplayMedia picker → user gesture).
- **Files modified:** `src/background/index.ts`.
- **Commit:** `886376e`.

**2. [Rule 1 - Bug] Replaced two `(error as any).message` patterns with `error instanceof Error`**
- **Found during:** Task 1 (the `as any` grep gate was at 2 instead of 0 because of a pre-existing instance in the ensureOffscreen catch).
- **Issue:** Audit P1 #13 — `as any` violates CLAUDE.md "no `as any`" rule. The catch block in `ensureOffscreen` accessed `.message` via `(error as any).message`.
- **Fix:** `const msg = error instanceof Error ? error.message : String(error); if (msg.includes(...)) { ... }`.
- **Files modified:** `src/background/index.ts`.
- **Commit:** `886376e`.

**3. [Rule 1 - Bug] Replaced `chrome.tabs.sendMessage(...) as any` with an explicit response type**
- **Found during:** Task 1 (same `as any` grep gate).
- **Issue:** Same audit P1 #13 / CLAUDE.md violation; the response was typed as `any` to access `.events` and `.userEvents`.
- **Fix:** Explicit `{ events?: unknown[]; userEvents?: unknown[] } | undefined` annotation on the response variable; nullish coalescing for the two extracts.
- **Files modified:** `src/background/index.ts`.
- **Commit:** `886376e`.

### Rule 2 — Missing critical functionality (auto-applied)

**4. [Rule 2 - Robustness] Added `chrome.offscreen.hasDocument()` check inside `initialize()`**
- **Found during:** Task 2 (orchestrator-flagged audit P1 #8).
- **Issue:** Across SW respawns, the in-memory `offscreenCreated` flag resets to `false`, but the offscreen document may still be alive (it survives SW idle unload because it holds the DISPLAY_MEDIA-reason capture). The next `ensureOffscreen()` would then call `createDocument` over an existing one. The catch block handles "already exists" so it's not strictly broken — but the hasDocument check makes it idempotent and is the canonical Chrome MV3 pattern (RESEARCH.md A7).
- **Fix:** `initialize()` is now `async` and calls `await chrome.offscreen.hasDocument()` to set `offscreenCreated = true` if a document is already there. Guarded with `typeof chrome.offscreen?.hasDocument === 'function'` so it stays safe across @types/chrome versions and partial stubs.
- **Files modified:** `src/background/index.ts`.
- **Commit:** `5cd1519`.

### Plan / orchestrator reconciliation note

The plan's `must_haves` and Task 1 instructions referenced symbols (`addVideoChunkFromBlob`, `cleanupVideoBuffer`, IDB helpers, VIDEO_CHUNK case, etc.) that Plan 03's executor had already inline-deleted as a Rule-3 blocking-fix dependency (documented in STATE.md). Plan 05 verified those via grep gates and only had to handle the residual `videoBuffer` array, the keepalive function, the tabCapture call site, and the comments. This matches the orchestrator's pre-flight note in the prompt.

## TDD Gate Compliance

This plan was `type: execute` (not `type: tdd`), so the RED/GREEN/REFACTOR gate sequence does not apply. The test suite remained at 9/9 throughout — Plan 04's port + handshake tests stayed green because Plan 05 only touches the SW side, and the offscreen-side port contract is unchanged.

## Authentication Gates

None — this plan is pure refactor + integration plumbing.

## Known Stubs

None. All paths in the modified file have a real implementation. `getVideoBufferFromOffscreen` returns `{ chunks: [] }` when `videoPort === null`, which is an intentional fallback (offscreen not yet connected, e.g. during SW cold start before the offscreen has finished bootstrapping). This is the documented contract per `<interfaces>` step 4 of the plan, not a stub.

## Self-Check

Verified after writing this summary:
- ✓ `886376e` exists in git log (Task 1 commit).
- ✓ `5cd1519` exists in git log (Task 2 commit).
- ✓ `src/background/index.ts` exists (491 lines).
- ✓ `npx tsc --noEmit` exits 0.
- ✓ `npx vitest run` reports 9/9 PASS across 4 test files.
- ✓ All Task 1 deletion grep gates return 0.
- ✓ All Task 2 addition grep gates return their expected counts.
- ✓ `grep -c "as any"` and `grep -c "@ts-ignore"` in `src/background/` both return 0.

## Self-Check: PASSED