Files
mokosh/.planning/debug/resolved/01-09-recovery-flow.md
Mark 0cd50fde94 docs(debug): import Bug B recovery-flow debug record from prior session
Untracked file present at session spawn (per orchestrator pre-flight
intelligence). The Bug B debug investigation that produced commit
b9eeeeb (the conditional-routing fix in src/background/index.ts
RECORDING_ERROR handler that Plan 01-11 assertion 6 verifies) was
recorded under .planning/debug/resolved/01-09-recovery-flow.md but
never committed. Importing it now so the debug provenance is
preserved alongside Plan 01-11's harness coverage of the bug class.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-17 21:46:09 +02:00

197 lines
8.6 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
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.