diff --git a/.planning/STATE.md b/.planning/STATE.md index 4a6ce27..59e8f3b 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -47,6 +47,13 @@ Progress: [█████████▌] 95% (Phase 1) — 0/5 phases fully co - Root cause: implementation 01-09 over-extended "always-on safety net" framing; SPEC intent is one-shot - Fix: SW SAVE_ARCHIVE handler dispatches STOP_RECORDING + setIdleMode in finally (4f4c3e2) - Harness regression coverage: A14 added (2b6c24b) — post-SAVE state check (badge='', popup='', no new recovery notif) +- **CHARTER REVERSAL 2026-05-19 — save-does-not-stop-recording** (`.planning/debug/resolved/01-09-save-does-not-stop-recording.md`): + - Operator UX iteration: prefers original "always-on safety net" framing (continuous recording; SAVE only creates a new zip) + - Revert: SW SAVE_ARCHIVE `finally` block REMOVED (commit 7645765) + - Test file inversions: `tests/background/save-archive-does-not-stop-recording.test.ts` (renamed via `git mv`, history preserved; commit 6ac23fd) + - Harness A14 inverted to assert continuous-recording post-SAVE: badge='REC', popup endsWith popup.html, no new recovery notif (commit 1baaf45) + - Plan 01-09 Amendment 3 landed documenting the reversed charter + - vitest preserved at 98 GREEN; `npm run test:uat` preserved at 15/15 GREEN under inverted contract - Plan 01-11 closed as spike-pivot (ba5474c SUMMARY); architecture lessons (no `await import(...)` in SW; `track.dispatchEvent('ended')` not `track.stop()`; `__MOKOSH_UAT__` Vite define-token) carried forward into Plan 01-13's Approach B harness - vitest: 83 → 98 GREEN across Plan 01-13 (+15: Tier-1 grep gate strings + hook contract tests + save-stops unit tests) diff --git a/.planning/debug/resolved/01-09-save-does-not-stop-recording.md b/.planning/debug/resolved/01-09-save-does-not-stop-recording.md new file mode 100644 index 0000000..933a639 --- /dev/null +++ b/.planning/debug/resolved/01-09-save-does-not-stop-recording.md @@ -0,0 +1,172 @@ +--- +slug: 01-09-save-does-not-stop-recording +status: resolved +goal: find_and_fix +trigger: Plan 01-09 charter reversal 2026-05-19 — operator UX iteration prefers original "always-on safety net" framing; the prior save-stops-recording fix (cd83eb0+4f4c3e2+2b6c24b+89f3337) is now considered wrong UX. Need to revert to "SAVE creates new zip but recording continues" semantics and lock the new (= original-original) charter via inverted tests. +tdd_mode: false +phase: 01-stabilize-video-pipeline +plan: 01-09 +opened: 2026-05-19 +resolved: 2026-05-19 +orchestrator_diagnosed: true +supersedes: 01-09-save-stops-recording +red_commit: 6ac23fd +green_commit: 7645765 +a14_invert_commit: 1baaf45 +docs_commit: pending +--- + +# Debug session 01-09-save-does-not-stop-recording — REVERSAL of save-stops contract + +## Symptom (operator UX reversal, 2026-05-19) + +After living with the save-stops fix (Amendment 2; commits cd83eb0 + +4f4c3e2 + 2b6c24b + 89f3337), the operator decided the previous +"always-on safety net" framing (the pre-fix Plan 01-09 charter) was +actually the correct UX. Quote: + +> "we need to change the ux so the thing never turns off, only saves new +> zip. ... It should never turn off, only if browser closed or extension +> uninstalled. Sorry, that's on me." + +This is a CHARTER REVERSAL, not a technical defect. The Amendment 2 fix +was correctly implemented against the (now-reversed) charter; the +reversal is an operator UX preference iteration. + +## Root cause (charter clarification cycle) + +1. Original Plan 01-09 implementation: SAVE creates zip; recording + continues ("always-on safety net" framing matching SPEC's + "continuous capture" intent). +2. 2026-05-19 operator empirical observation: "doesn't switch off after + save" — interpreted as a bug. +3. Prior `/gsd-debug` session: landed save-stops-recording fix at + commits cd83eb0 + 4f4c3e2 + 2b6c24b + 89f3337 (Amendment 2). +4. 2026-05-19 operator re-evaluation: prefers original always-on; + reversal needed. + +The fix is the inverse of the prior fix. + +## Fix shape + +### Code surgery (commit 7645765) + +`src/background/index.ts:saveArchive()` — removed the entire `finally` +block introduced by Amendment 2 (commit 4f4c3e2). SAVE_ARCHIVE flow +returns to: create zip → download → done. No state transitions; recording +continues. The function-level docstring was updated to reflect the +reversed charter and point at the new test file + A14 harness assertion. + +### Test inversions + +**`tests/background/save-archive-does-not-stop-recording.test.ts`** +(commit 6ac23fd — `git mv` from `save-archive-stops-recording.test.ts`, +history preserved; assertions inverted): + +- A (was: badge transitions to ''; now: no NEW '' transition — badge stays REC) +- B (was: setPopup('') call; now: no setPopup('') call — popup stays pinned to popup.html) +- C (was: STOP_RECORDING dispatch; now: no STOP_RECORDING dispatch) +- D (unchanged: no recovery notification — regression guard preserved) + +**Plan 01-13 harness A14** (commit 1baaf45) — inverted in both +`tests/uat/extension-page-harness.ts:assertA14` and `tests/uat/lib/harness-page-driver.ts:driveA14`: + +- A14.1 (was: badge === ''; now: badge === 'REC') +- A14.2 (was: popup === ''; now: popup endsWith 'src/popup/index.html') +- A14.3 (unchanged: no new mokosh-recovery-* notification) + +Also touched in commit 1baaf45: the A13 docstring + diag strings +clarifying that the setupFreshRecording call is now defensive +(orthogonal to A12 ordering) rather than a workaround for the +prior auto-stop; the 11s segment-settle wall-clock cost is preserved +identical to before Amendment 3. + +## RED → GREEN evidence + +### Commit 6ac23fd (RED) + +After rename + assertion inversion, with `src/background/index.ts` still +holding the Amendment 2 `finally` block: + +``` +FAIL tests/background/save-archive-does-not-stop-recording.test.ts + ✗ A: expected 1 to be +0 (1 NEW '' badge transition from setIdleMode) + ✗ B: expected 1 to be +0 (1 setPopup('') call from setIdleMode) + ✗ C: expected 1 to be +0 (1 STOP_RECORDING dispatch from finally) + ✓ D: 0 recovery notifications (sendMessage mock doesn't loop to handler) +``` + +3/4 RED — exactly as expected; the finally block is the load-bearing +source of all three deltas. + +### Commit 7645765 (GREEN) + +After removing the `finally` block: + +``` +PASS tests/background/save-archive-does-not-stop-recording.test.ts + ✓ A: no NEW '' badge transition + ✓ B: no setPopup('') call + ✓ C: no STOP_RECORDING dispatch + ✓ D: 0 recovery notifications +4/4 GREEN +``` + +Full vitest baseline (SKIP_BUILD=1): **98/98 GREEN** (preserved). +`npx tsc --noEmit`: clean. +`npm run build`: clean (SW chunk 374.91 kB; no test-hook leaks). + +### Commit 1baaf45 (A14 inversion, harness GREEN) + +``` +npm run test:uat +======================================================================== +UAT harness: 15/15 assertions passed + [PASS] A1..A13, A14 +======================================================================== + +A14 actual values: + badge='REC' + popup='chrome-extension://lbjhjpkiodafpaligjfngpfdglkfiflc/src/popup/index.html' + recoveryDelta=0 (before=1, after=1) +``` + +15/15 harness GREEN under the inverted contract. + +## Verification (post-revert) + +- Recording continues after SAVE: badge stays REC, popup stays + popup.html, isRecording stays true (transitively verified via badge + proxy in unit + harness). +- Bug B routing preserved: user-stopped-sharing still routes through + setIdleMode (no regression — only the SAVE_ARCHIVE finally was + removed). +- No recovery notification on SAVE: deliberate save is not an error + (D unchanged across both charters). +- vitest baseline preserved: 98/98 GREEN. +- Production bundle clean: tsc + build pass. + +## Anti-regression coverage + +The inverted test file + inverted A14 lock the new charter at two +levels: + +- **Unit** (`tests/background/save-archive-does-not-stop-recording.test.ts`): + fast feedback against any code change that would re-introduce a + STOP_RECORDING dispatch or setIdleMode call inside saveArchive. +- **E2E** (`tests/uat/extension-page-harness.ts:assertA14`): catches + any production-bundle-level regression by reading the actual + chrome.action state after a real SAVE_ARCHIVE. + +A future maintainer attempting to re-introduce save-stops semantics +will hit RED in both gates before merging. + +## Related artifacts + +- Superseded debug record: `.planning/debug/resolved/01-09-save-stops-recording.md` + (footer "SUPERSEDED 2026-05-19" added) +- Plan amendment: `.planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md` + Amendment 3 +- Plan 01-13 summary footer: `.planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md` + "Subsequent Reversal (2026-05-19)" +- STATE.md sync: Plan 01-13 closure block charter-reversal bullet diff --git a/.planning/debug/resolved/01-09-save-stops-recording.md b/.planning/debug/resolved/01-09-save-stops-recording.md index f8b9be4..cbd2a76 100644 --- a/.planning/debug/resolved/01-09-save-stops-recording.md +++ b/.planning/debug/resolved/01-09-save-stops-recording.md @@ -227,3 +227,20 @@ badge transitions atomically; badge='' is a reliable proxy. - 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). + +--- + +## SUPERSEDED 2026-05-19 — Charter REVERSED + +This fix was REVERSED on 2026-05-19 per operator UX iteration preferring +the original "always-on safety net" charter. The new (= original-original) +contract: SAVE creates a zip but does NOT stop the recorder; recording +is continuous until the operator clicks Chrome's "Stop sharing" banner, +the browser closes, or the extension is uninstalled. + +See the new debug record: `.planning/debug/resolved/01-09-save-does-not-stop-recording.md` +and Plan 01-09 Amendment 3. + +This record remains as audit trail for the charter cycle. The 4f4c3e2 fix +was technically correct against the (now-reversed) Amendment 2 charter; +the reversal is a UX preference, not a technical defect. diff --git a/.planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md b/.planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md index e9e906d..f05cef7 100644 --- a/.planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md +++ b/.planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md @@ -618,6 +618,31 @@ The harness cannot verify two things, both retained as operator-facing gates: The historical 15-step empirical checkpoint (lines 519-548 above) remains in this document as the reference for the original closure intent, and as a fallback if the harness toolchain becomes unavailable (e.g., Puppeteer breakage on a future Chrome major where the harness needs revision). +## Amendment 3 (2026-05-19) — Save-Does-Not-Stop-Recording (Reverses Save-Stops) + +Per operator UX preference iteration (2026-05-19), the prior save-stops +behavior (Amendment 2's harness-PASS-as-closure-gate validated save-stops +via A14) is REVERSED. Charter clarified: + +**Recording is continuous; SAVE creates a new zip but does NOT stop the +recorder.** The only termination paths are: +1. Operator clicks Chrome's "Stop sharing" banner button → + user-stopped-sharing path → setIdleMode (Bug B routing preserved) +2. Browser closes (SW terminates; offscreen tears down with it) +3. Extension uninstalled + +Matches SPEC's "continuous capture / always-on safety net" framing +(Тз расширение фаза1.md). Operator workflow: hit bug → save → continues +working → next bug also captured. + +Implementation: SAVE_ARCHIVE handler's `finally` block (Amendment 2's +STOP_RECORDING + setIdleMode dispatch) REMOVED. See debug session +`.planning/debug/resolved/01-09-save-does-not-stop-recording.md`. + +Test inversions lock the new charter: `tests/background/save-archive-does-not-stop-recording.test.ts` +(renamed via `git mv` + inverted, history preserved) and Plan 01-13 +harness A14 (inverted to assert continuous-recording post-SAVE). + Plan 01-09 is complete when: 1. The 4 displaySurface tests + 13 toolbar/badge/notification/popup-idle-race tests (17 new total) are all GREEN. diff --git a/.planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md b/.planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md index 594846a..42e1163 100644 --- a/.planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md +++ b/.planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md @@ -196,3 +196,13 @@ Remaining Phase 1 gates are operator/designer-facing only: - Plan 01-12 (design integration) — designer reply pending; branded surfaces; will require operator brand-fit ack post-integration Phase 2 (DOM + event-capture privacy) inherits the harness as its closure-gate template. + +## Subsequent Reversal (2026-05-19) + +The save-stops-recording fix landed at commit 89f3337 was REVERSED on +2026-05-19 per operator UX iteration preferring the original "always-on +safety net" charter. See `.planning/debug/resolved/01-09-save-does-not-stop-recording.md` ++ Plan 01-09 Amendment 3. Test files inverted to lock the new charter; +harness A14 inverted. `npm run test:uat` still 15/15 GREEN under the +new contract; vitest baseline preserved at 98 GREEN (the 4 save-archive +tests inverted — same count).