.planning/debug/resolved/01-09-recovery-flow.md

@@ -0,0 +1,196 @@
+---
+slug: 01-09-recovery-flow
+status: RESOLVED
+goal: find_and_fix
+trigger: Plan 01-09 Task 5 operator empirical UAT surfaced operator-lockout after Chrome "Stop sharing" mid-recording
+tdd_mode: true
+phase: 01-stabilize-video-pipeline
+plan: 01-09
+opened: 2026-05-17
+resolved: 2026-05-17
+red_commit: 91b4475
+green_commit: b9eeeeb
+specialist_review: typescript-expert (not invoked — straight TDD fix, no design ambiguity)
+---
+
+# Debug session 01-09-recovery-flow — Bug B operator-lockout
+
+## Problem statement
+
+During Plan 01-09 Task 5 operator empirical UAT, after the operator clicks
+Chrome's "Stop sharing" banner mid-recording, the extension enters a
+locked-out state: the toolbar badge stays on ERROR (yellow `ERR`), the
+popup remains in SAVE-only mode pointing at `src/popup/index.html`, and
+the operator has no path to restart recording. Root mechanism:
+`chrome.action.onClicked` only fires when `chrome.action.setPopup({popup: ''})`
+is active (per the MV3 spec: a non-empty `default_popup` causes the toolbar
+click to open the popup rather than fire `onClicked`). The `RECORDING_ERROR`
+handler in `src/background/index.ts:725-744` unconditionally called
+`setErrorMode()` regardless of the incoming `message.error` code, including
+for `user-stopped-sharing` — which is not actually an error but a deliberate
+end-of-session signal from the operator. With the popup pinned, the
+toolbar-click restart path was gone. Operator perception: "recording never
+stops, and I can't start it again." The accompanying recovery notification
+was also failing for a separate reason (Bug A — icon files were placeholder
+stubs below Chrome `imageUtil` minimums; resolved at a881bf0). Bug B is the
+state-routing fix that remained.
+
+## Hypotheses verified by prior session (preserved, not re-verified)
+
+The prior gsd-debug session-manager completed empirical verification on
+three hypotheses before context-anxiety pause; this session preserves the
+diagnostic record without re-running.
+
+### H1: `chrome.action.onClicked` is gated by popup presence
+
+**Status:** VERIFIED. Per the W3C MV3 spec and the Chromium implementation
+of `chrome.action`, the `onClicked` event fires for the toolbar icon click
+only when `default_popup` is unset (empty string OR not specified in the
+manifest). The Plan 01-09 design uses Option B (`setPopup('')` for idle,
+`setPopup('src/popup/index.html')` for recording) precisely because clicking
+when popup is set opens the popup instead of firing onClicked — pinning the
+state machine forces the dispatch outcome. Confirmed by inspecting
+`src/background/index.ts` `setIdleMode/setRecordingMode/setErrorMode`
+(lines 93-108) and cross-referencing the existing
+`tests/background/badge-state-machine.test.ts` Test D dispatch behavior.
+Implication for Bug B: `setErrorMode()` flips popup to
+`src/popup/index.html`, so any subsequent toolbar click opens the (empty,
+SAVE-only) popup instead of firing onClicked. Operator restart path closed.
+
+### H2: Offscreen recorder emits `RECORDING_ERROR{error:'user-stopped-sharing'}` after `resetBuffer`
+
+**Status:** VERIFIED. `src/offscreen/recorder.ts:451-480`
+(`onUserStoppedSharing`) is registered on every captured track via
+`addEventListener('ended', onUserStoppedSharing, {once: true})`. The
+handler is idempotent-guarded by `teardownInProgress`. On fire, it calls
+`resetBuffer()` (line 458), stops the MediaRecorder, releases all tracks,
+and emits `chrome.runtime.sendMessage({type:'RECORDING_ERROR', error:
+'user-stopped-sharing'})` (line 471). The buffer is therefore EMPTY by
+the time the SW receives the message — SAVE-only popup mode would have
+nothing to save. Implication for Bug B: the IDLE landing state is
+architecturally correct; SAVE-only mode (the current routing's outcome)
+is meaningless because no segments remain.
+
+### H3: No alternative restart path exists when popup is pinned
+
+**Status:** VERIFIED. Reviewed the chrome.* surfaces the extension
+exposes: `chrome.action.onClicked` (gated by popup), `chrome.notifications
+.onClicked` (would work, but Bug A — corrupt icon files — was suppressing
+the recovery notification; even after Bug A's a881bf0 fix, an unreliable
+notification is not an acceptable primary restart UX), and `chrome.runtime
+.onStartup` (only fires on browser session start, not mid-session). The
+toolbar click is the canonical restart gesture per the Plan 01-09 charter
+("per-session clicks drop from 3 to 2"). No backup path exists; the
+toolbar click MUST be made operative after stop-sharing.
+
+## Fix design
+
+The patch applies conditional routing to the existing `RECORDING_ERROR`
+case in `src/background/index.ts`:
+
+```typescript
+case 'RECORDING_ERROR': {
+  const errorCode = (message as unknown as { error?: unknown }).error;
+  if (errorCode === 'user-stopped-sharing') {
+    isRecording = false;
+    setIdleMode();
+  } else {
+    setErrorMode();
+    // ... existing recovery-notification block preserved
+  }
+  return false;
+}
+```
+
+Three sub-decisions:
+
+1. **`isRecording = false`** alongside `setIdleMode()` for the
+   stop-sharing branch. The offscreen recorder has already released
+   tracks and reset the buffer; the SW state must mirror that so a
+   subsequent `chrome.action.onClicked` doesn't trip the
+   `if (isRecording) return` guard at the top of the click handler
+   (`src/background/index.ts:811-815`).
+
+2. **Recovery notification suppressed** for stop-sharing. The operator
+   performed the action deliberately; surfacing a "Recording stopped"
+   notification would be UX noise. Genuine errors (codec-unsupported,
+   wrong-display-surface, etc.) keep the notification — those are
+   unexpected and the operator benefits from an explicit click-to-restart
+   surface.
+
+3. **Type-safe narrowing** via `(message as unknown as { error?: unknown }).error`.
+   The canonical `Message` interface (`src/shared/types.ts:25-29`) only
+   types `type/data/tabId`; the offscreen recorder emits the extra
+   `error` field as part of the `RECORDING_ERROR` wire shape. The
+   double-cast keeps us off `as any` per project type discipline.
+
+No refactor warranted — the conditional adds one narrow + one if/else.
+Extracting a `routeRecordingError(code)` helper would just move the
+same 2-arm switch one level down without complexity reduction.
+
+## RED → GREEN evidence
+
+### RED commit: `91b4475`
+
+Extended `tests/background/badge-state-machine.test.ts` with Tests E + F:
+
+- **Test E** (RED today): asserts `RECORDING_ERROR{error:'user-stopped-sharing'}`
+  triggers `setBadgeText({text:''})` + `setBadgeBackgroundColor({color:'#D32F2F'})`
+  + `setPopup({popup:''})`, AND does NOT trigger ERROR-state side effects
+  (no ERR text, no yellow color, no popup html re-set). Snapshot baseline
+  after init isolates the dispatch delta.
+- **Test F** (GREEN today, preservation guard): asserts that any other
+  error code (representative: `'codec-unsupported'`) continues to route
+  through `setErrorMode` — badge ERR + yellow + popup html. Regression
+  pin against the fix over-rotating to IDLE for all codes.
+
+Vitest output at RED:
+```
+ FAIL  tests/background/badge-state-machine.test.ts > Test E
+AssertionError: expected 0 to be greater than or equal to 1
+  → expect(offTextAfter - offTextBefore).toBeGreaterThanOrEqual(1);
+
+ Test Files  1 failed (1)
+      Tests  1 failed | 5 passed (6)
+```
+
+### GREEN commit: `b9eeeeb`
+
+Patched `src/background/index.ts` RECORDING_ERROR case with conditional
+routing per the fix design above.
+
+Vitest output at GREEN:
+```
+ Test Files  18 passed (18)
+      Tests  83 passed (83)
+```
+
+(Was 81/81 GREEN before this debug session; +2 from new Tests E+F.
+Bug B test Tests E + F both GREEN.)
+
+Toolchain:
+- `npx tsc --noEmit` → exit 0
+- `npm run build` → exit 0; `dist/manifest.json` and bundle assets
+  emitted cleanly.
+
+## Files modified
+
+- `tests/background/badge-state-machine.test.ts` (+157 lines: Tests E+F,
+  plus 2 helpers `badgeColorCallsFor` + `setPopupCallsFor`, plus header
+  comment expansion).
+- `src/background/index.ts` (case-block patch only, lines 725-744 →
+  rewritten as conditional routing; +48 / −14).
+
+## Follow-ups for orchestrator
+
+- Pre-checkpoint bundle gates (SW CSP, Node-globals, Tier-1
+  SW-bundle-import gate, manifest validation) — NOT performed by this
+  debug session per scope instruction. Orchestrator runs these before
+  any operator UAT re-surface.
+- Operator UAT re-run validates both Bug A (recovery notification fires
+  via working icons) AND Bug B (badge transitions to OFF on stop-sharing
+  + toolbar click restarts recording). The recovery notification is now
+  ONLY emitted for genuine errors (not for stop-sharing) — UAT step 11
+  expectation needs adjustment: after operator clicks "Stop sharing",
+  badge transitions to OFF (not ERROR), and NO recovery notification
+  fires. To restart, operator clicks the toolbar icon directly.