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

8.6 KiB
Raw Permalink Blame History

slug, status, goal, trigger, tdd_mode, phase, plan, opened, resolved, red_commit, green_commit, specialist_review
slug status goal trigger tdd_mode phase plan opened resolved red_commit green_commit specialist_review
01-09-recovery-flow RESOLVED find_and_fix Plan 01-09 Task 5 operator empirical UAT surfaced operator-lockout after Chrome "Stop sharing" mid-recording true 01-stabilize-video-pipeline 01-09 2026-05-17 2026-05-17 91b4475 b9eeeeb 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:

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.