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>
197 lines
8.6 KiB
Markdown
197 lines
8.6 KiB
Markdown
---
|
||
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.
|