--- slug: 01-09-save-stops-recording status: fixing goal: find_and_fix trigger: Plan 01-13 Task 9 operator empirical UAT — clicking SAVE downloads archive but recording continues; toolbar press re-opens SAVE-only popup; operator confusion (4× toolbar presses → 2 zips downloaded, 1 misclick) tdd_mode: true phase: 01-stabilize-video-pipeline plan: 01-09 opened: 2026-05-19 orchestrator_diagnosed: true --- # Debug session 01-09-save-stops-recording — SAVE_ARCHIVE must auto-stop recording (SPEC one-shot intent) ## Problem statement During the Plan 01-13 Task 9 operator empirical UAT 2026-05-19, after the operator clicked SAVE in the popup during an active recording session, the extension produced the requested archive zip (downloaded to `~/Downloads`) but the recording **did not stop**: the toolbar badge stayed `REC`, Chrome's "Sharing your screen" banner remained visible, and subsequent toolbar presses re-opened the SAVE-only popup with no feedback indicating the prior SAVE had succeeded. The operator performed 4 toolbar presses across ~94 seconds; 2 zips downloaded (`session_report_2026-05-19_10-59-44.zip` + `session_report_2026-05-19_11-01-18.zip`); the SW console captured 2 `SAVE_ARCHIVE` receives + 2 `Archive download started` events, with the recording stream alive throughout. ## Root cause (orchestrator-diagnosed) `src/background/index.ts:744-748` SAVE_ARCHIVE handler invokes `saveArchive()`, which produces the zip + calls `chrome.downloads.download`, then returns `{success: true}` — but the handler **never signals offscreen to stop the recording**. The recorder's mediaStream + MediaRecorder + rotation timer all stay live. The operator's perception ("nothing happened") was correct: the popup did not transition state after SAVE; the badge did not transition; no notification fired. This violates the SPEC `Тз расширение фаза1.md` "one click MUST produce a self-contained archive" — which implies one-shot save-and-done semantics. The original design's "always-on" framing was over-extended by engineering interpretation; the SPEC actually calls for a deliberate stop after each save. ## Fix design Two-file patch matched to the existing parallel Bug B (user-stopped-sharing) pattern in `.planning/debug/resolved/01-09-recovery-flow.md`: ### `src/background/index.ts` SAVE_ARCHIVE handler After `downloadArchive` returns (i.e. inside `saveArchive()` after the download dispatch succeeds, before the function returns `{success: true}`): ```typescript // Plan 01-13 Task 9 operator UAT closure: SAVE = one-shot per SPEC intent. // Send STOP_RECORDING to offscreen + transition state to IDLE so the // operator sees a clean reset (badge clears, popup empties, sharing // banner closes via track.stop()). Mirrors the Bug B user-stopped-sharing // path: deliberate stop ≠ error; no recovery notification. try { await chrome.runtime.sendMessage({ type: 'STOP_RECORDING' }); } catch (sendErr) { // Offscreen may be unreachable mid-teardown; non-fatal — SW-side // state still transitions to IDLE so the operator regains the // restart path. logger.warn('STOP_RECORDING dispatch after save failed:', sendErr); } isRecording = false; setIdleMode(); ``` ### `src/offscreen/recorder.ts` STOP_RECORDING handler ALREADY EXISTS at recorder.ts:848 — invokes `stopRecording()` which correctly nulls mediaStream + stops the MediaRecorder + stops all tracks + clears the rotation timer. No edit needed on the offscreen side. **Edge case decided:** `resetBuffer` is intentionally NOT called by the SW post-save because: 1. `stopRecording()` in recorder.ts does NOT call resetBuffer (line 559 comment: "we do NOT call resetBuffer() here — the operator may want to STOP and then SAVE the buffered footage"). 2. After SAVE_ARCHIVE the buffer's contents have already been consumed into the zip; the operator's next session will start fresh because `startRecording()` calls resetBuffer at line 318. 3. Calling resetBuffer here would be defensive overkill — the segments are about to be overwritten on the next START anyway, and a no-op resetBuffer on an already-cleared-at-next-start buffer wastes nothing. ## TDD plan ### RED unit tests (NEW file: `tests/background/save-archive-stops-recording.test.ts`) 1. After `SAVE_ARCHIVE` receive completes → `chrome.action.setBadgeText` called with `text=''` (setIdleMode side effect) 2. After `SAVE_ARCHIVE` receive → `chrome.action.setPopup` called with `popup=''` (setIdleMode side effect — the popup-empties signal that re-enables onClicked restart path) 3. After `SAVE_ARCHIVE` receive → `chrome.runtime.sendMessage` called with `{type:'STOP_RECORDING'}` (the SW → offscreen STOP signal) 4. After `SAVE_ARCHIVE` receive → NO recovery notification fired (`chrome.notifications.create` not called with `mokosh-recovery-*` id) ### A14 harness extension `tests/uat/extension-page-harness.ts` — add `assertA14` after assertA13: - Pre-condition: A13 just dispatched a SAVE_ARCHIVE; recording state after that save should now be IDLE (badge='', popup=''). - Reads `chrome.action.getBadgeText({})` → expects `''` - Reads `chrome.action.getPopup({})` → expects `''` (relative path empty; Chrome returns empty string when setPopup was called with `''`) - Snapshots `chrome.notifications.getAll()` keys → expects NO id with `mokosh-recovery-` prefix added since A13 (verifies no recovery notif fired on the SAVE auto-stop) Recommended simplification per spec: skip direct `isRecording` query (no bridge op exposes it). The badge proxy is sufficient — production state machine pairs `isRecording` updates with badge transitions atomically (no state-machine path desyncs them). ### Files touched - `tests/background/save-archive-stops-recording.test.ts` (NEW) - `src/background/index.ts` (saveArchive() function — append STOP signal + setIdleMode + isRecording=false) - `tests/uat/extension-page-harness.ts` (assertA14 + global surface) - `tests/uat/lib/harness-page-driver.ts` (driveA14) - `tests/uat/harness.test.ts` (orchestrator — add A14 in sequence) Note: STOP_RECORDING is already in `MessageType` (src/shared/types.ts:14) — no type-system surface change needed. ## RED → GREEN evidence (to be filled in) - RED commit SHA: TBD - GREEN commit SHA: TBD - A14 commit SHA: TBD - vitest count (target): >= 94 + 4 (new tests) = >= 98 GREEN - `npm run test:uat` (target): 15/15 GREEN (was 14/14) ## Follow-ups - Orchestrator runs pre-checkpoint bundle gates (SW CSP-safety, Node-globals in SW chunk, DOM-globals not in SW chunk, Tier-1 hook-string grep over dist/) BEFORE re-surfacing operator UAT. Per `feedback-pre-checkpoint-bundle-gates.md` operator time = automation cannot verify. - Operator UAT after this lands should confirm: SAVE click → zip lands + badge clears + sharing banner closes + popup empties + subsequent toolbar click starts a NEW recording session (clean state machine).