21 KiB
phase, plan, type, wave, depends_on, files_modified, autonomous, requirements, requirements_addressed, must_haves
| phase | plan | type | wave | depends_on | files_modified | autonomous | requirements | requirements_addressed | must_haves | |||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 01-stabilize-video-pipeline | 04 | tdd | 3 |
|
|
true |
|
|
|
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 (the SW-side
onConnect counterparty) follows in wave 4 — Plan 04 lands first in wave 3
so the SW-side host has a well-defined offscreen-side contract to bind
against. Plan 04 owns the offscreen-side; Plan 05 owns the SW-side; the two
files do not overlap (Plan 04 touches only src/offscreen/recorder.ts,
Plan 05 touches only src/background/index.ts).
<execution_context> @$HOME/.claude/get-shit-done/workflows/execute-plan.md @$HOME/.claude/get-shit-done/templates/summary.md </execution_context>
@.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.tsModule-import side-effects (from tests/offscreen/handshake.test.ts):
chrome.runtime.onMessage.addListeneris called at least oncechrome.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.connectto 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 aregetBuffer()) - 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).
<threat_model>
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. |
| </threat_model> |
Handshake (from tests/offscreen/handshake.test.ts):
- On module import, register
chrome.runtime.onMessage.addListenerBEFORE callingchrome.runtime.sendMessage({ type: 'OFFSCREEN_READY' }) OFFSCREEN_READYis 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
onDisconnectlistener on the connected port - When
onDisconnectfires, immediately callchrome.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).
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 callschrome.runtime.connect({ name: PORT_NAME })once — this test MAY ALREADY PASS too. - Port test (
reconnects when port disconnects): Plan 03's stub does NOT registeronDisconnect, so when the test fires the registered listeners,connectCountstays at 1 — this test MUST FAIL.
The intended RED gate for Plan 04 is the reconnect test. Confirm:
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|✗"
<acceptance_criteria>
- /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)
</acceptance_criteria>
RED gate confirmed: the reconnect test is failing because Plan 03's stub bootstrap doesn't reconnect on disconnect. Proceed to GREEN.
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)
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):
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:
// ─── 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<typeof setInterval> | null = null;
let preemptiveReconnectId: ReturnType<typeof setTimeout> | 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:
npx vitest run
npx tsc --noEmit
Both MUST exit 0.
Notes:
setInterval/setTimeoutin offscreen documents: offscreen runs in a real DOM context (NOT in the SW), so timers are reliable — unlikesetTimeoutinside 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. TheonDisconnectlistener fires synchronously in Chrome's port-disconnect path, which transitions us back throughconnectPort()cleanly. PortMessageshape: outbound traffic uses thePortMessagetype added tosrc/shared/types.tsin Plan 03. Noas any. Inbound messages over the port are typedunknownand shape-checked. npx vitest run && npx tsc --noEmit <acceptance_criteria>npx vitest runexits 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 --noEmitexits 0grep -c "function connectPort" src/offscreen/recorder.tsreturns 1grep -c "PORT_PING_MS = 25_000" src/offscreen/recorder.tsreturns 1grep -c "PORT_RECONNECT_MS = 290_000" src/offscreen/recorder.tsreturns 1grep -c "REQUEST_BUFFER" src/offscreen/recorder.tsreturns at least 1 (the inbound handler)grep -c "'BUFFER'" src/offscreen/recorder.tsreturns at least 1 (the outbound response shape)grep -c "isFromOwnExtension" src/offscreen/recorder.tsreturns at least 1 (sender check on onMessage — T-1-04 defense-in-depth)grep -c "as any" src/offscreen/recorder.tsreturns 0grep -c "@ts-ignore" src/offscreen/recorder.tsreturns 0grep -c "void keepalivePort" src/offscreen/recorder.tsreturns 0 (the no-unused-locals workaround is removed) </acceptance_criteria> All 4 test files from Plan 02 are now GREEN. The offscreen module owns the full port lifecycle. Plan 05 can wire the SW-sideonConnecthandler against the same port name with confidence.
- Re-read
src/offscreen/recorder.ts(the full module now that Plan 04 has landed). - 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
- If no obvious improvements: SKIP THIS TASK (do not commit). Note in summary: "Refactor: no changes needed."
- 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
<acceptance_criteria>
- 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
</acceptance_criteria>
REFACTOR phase complete (or explicitly skipped with rationale).
npx vitest run— exits 0 with ALL 8 tests passing across the 4 test files in tests/offscreen/.npx tsc --noEmit— exits 0.grep -c "as any\|@ts-ignore" src/offscreen/recorder.tsreturns 0.- The grep gates listed in Task 2's
acceptance_criteriaall return their expected values. - 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.
<success_criteria>
- All 4 Plan-02 test files green (ring-buffer, codec-check, handshake, port) — 8 tests total
src/offscreen/recorder.tscontains a singleconnectPort()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 </success_criteria>