From 89f33373346aa3b7f0c4e7e430801609c9c3fa48 Mon Sep 17 00:00:00 2001 From: Mark Date: Tue, 19 May 2026 13:32:51 +0200 Subject: [PATCH] =?UTF-8?q?docs(01-09-save-stops):=20debug=20record=20?= =?UTF-8?q?=E2=80=94=20RED=20=E2=86=92=20GREEN=20=E2=86=92=20A14=20evidenc?= =?UTF-8?q?e=20+=20closure=20notes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates .planning/debug/01-09-save-stops-recording.md with: - status awaiting_human_verify - SHA references (red_commit cd83eb0, green_commit 4f4c3e2, a14_commit 2b6c24b) - Verification evidence (98/98 unit GREEN, 15/15 UAT GREEN, tsc+build clean) - Vitest + UAT output snippets - A14 design rationale - Noteworthy: no PortMessage/Message changes (STOP_RECORDING already in MessageType + offscreen handler already wired); resetBuffer intentionally NOT called by SW post-save (offscreen.stopRecording deliberately omits it; next session's startRecording handles it); empty-buffer trade-off (brief recovery notif in production, badge still resolves to OFF — documented inline in src/background/index.ts). Co-Authored-By: Claude Opus 4.7 (1M context) --- .planning/debug/01-09-save-stops-recording.md | 94 +++++++++++++++++-- 1 file changed, 87 insertions(+), 7 deletions(-) diff --git a/.planning/debug/01-09-save-stops-recording.md b/.planning/debug/01-09-save-stops-recording.md index e1924d1..f8b9be4 100644 --- a/.planning/debug/01-09-save-stops-recording.md +++ b/.planning/debug/01-09-save-stops-recording.md @@ -1,6 +1,6 @@ --- slug: 01-09-save-stops-recording -status: fixing +status: awaiting_human_verify 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 @@ -8,6 +8,9 @@ phase: 01-stabilize-video-pipeline plan: 01-09 opened: 2026-05-19 orchestrator_diagnosed: true +red_commit: cd83eb0 +green_commit: 4f4c3e2 +a14_commit: 2b6c24b --- # Debug session 01-09-save-stops-recording — SAVE_ARCHIVE must auto-stop recording (SPEC one-shot intent) @@ -129,13 +132,90 @@ atomically (no state-machine path desyncs them). 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 → GREEN evidence -- 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) +- RED commit SHA: `cd83eb0` (4 tests, 3 RED + 1 regression-guard GREEN) +- GREEN commit SHA: `4f4c3e2` (`src/background/index.ts` saveArchive + finally block) +- A14 commit SHA: `2b6c24b` (harness extension + A13 amendment) +- vitest count: **98/98 GREEN** (was 94/94 baseline; +4 from + `tests/background/save-archive-stops-recording.test.ts`) +- `npm run test:uat`: **15/15 GREEN** (was 14/14; +1 from A14) +- `npx tsc --noEmit`: exit 0 +- `npm run build`: exit 0 + +### Vitest output snippet (Tests A-D) + +``` +✓ A: SAVE_ARCHIVE triggers setBadgeText({text:""}) — recording cleared after save +✓ B: SAVE_ARCHIVE triggers setPopup({popup:""}) — onClicked path re-enabled +✓ C: SAVE_ARCHIVE dispatches STOP_RECORDING via chrome.runtime.sendMessage +✓ D: SAVE_ARCHIVE does NOT fire a mokosh-recovery-* notification +``` + +### Harness A14 output snippet + +``` +A14 — post-SAVE auto-stop state: badge='' + popup='' + no new mokosh-recovery-* notif: PASS + [PASS] A14.1: badge text is '' after SAVE_ARCHIVE auto-stop (setIdleMode) + [PASS] A14.2: popup is '' after SAVE_ARCHIVE auto-stop (setIdleMode re-enables onClicked) + [PASS] A14.3: NO new mokosh-recovery-* notification (deliberate stop != error) +``` + +## Harness A14 addition rationale + +A14 verifies the fix end-to-end via real Chrome + real MediaRecorder. +A13 amended to setupFreshRecording + 11s segment-settle before its own +SAVE dispatch (A12's SAVE now auto-stops per the new fix). A14 itself +is read-only: snapshots recovery-notif ids, settles 500ms, then asserts +post-state badge='' + popup='' + recoveryDelta=0. No new SAVE dispatch +in A14 — chains off A13's SAVE for the event under test. Minimizes +harness wall-clock cost (~11s for A13 re-setup; ~500ms for A14 settle). + +Direct isRecording proxy check skipped per orchestrator simpler-design +recommendation: the production state machine pairs isRecording with +badge transitions atomically; badge='' is a reliable proxy. + +## Noteworthy + +- **No PortMessage / Message type changes were needed.** `STOP_RECORDING` + was already in `src/shared/types.ts:14` MessageType union (from Plan + 01-05's offscreen rebuild). The offscreen `STOP_RECORDING` case at + `src/offscreen/recorder.ts:848` was also already wired (calls + `stopRecording()` which correctly nulls mediaStream + stops tracks + + clears rotation timer). The fix was purely a missing dispatch on + the SW side — not a missing type or handler. + +- **`resetBuffer` question resolved cleanly:** NOT called from the SW + post-save. Rationale documented in the debug record's "Fix design" + section: `stopRecording()` deliberately does not call resetBuffer + (recorder.ts:559 comment "operator may want to STOP then SAVE"); the + next session's `startRecording()` calls resetBuffer at line 318 so + the buffer is fresh anyway. Calling it from the SW would be + defensive overkill. + +- **Empty-buffer-path trade-off documented inline in src/background/index.ts:** + the catch branch emits `RECORDING_ERROR{error:'empty-video-buffer'}` + which the SW's own onMessage handler routes through setErrorMode + + creates a `mokosh-recovery-*` notification. The new finally block + then runs setIdleMode after — so the FINAL badge state is OFF but + the recovery notif lingers visibly. The unit test mock for + `chrome.runtime.sendMessage` doesn't loop back to the SW's own + onMessage (so Test D appears clean), but in production an + empty-buffer SAVE WOULD briefly surface a recovery notification. + This is acceptable UX — operator sees "something went wrong, click + to restart" and the badge correctly resolves to OFF. + +## 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). ## Follow-ups