Plan 01-09 Amendment 3 (2026-05-19) — atomic documentation pass for the save-does-not-stop-recording charter reversal. Changes: - .planning/phases/01-stabilize-video-pipeline/01-09-PLAN.md: Amendment 3 block added above <success_criteria> (mirrors Amendment 2 placement). Describes the reversed charter, references the new debug record, points at the inverted test file + harness A14. - .planning/phases/01-stabilize-video-pipeline/01-13-SUMMARY.md: "Subsequent Reversal (2026-05-19)" footer added. Notes that npm run test:uat still 15/15 GREEN under the inverted A14 contract; vitest baseline preserved at 98 GREEN. - .planning/STATE.md: Plan 01-13 closure block extended with CHARTER REVERSAL bullet citing the 4 commit SHAs (6ac23fdRED,7645765GREEN,1baaf45A14 invert, this commit docs). - .planning/debug/resolved/01-09-save-stops-recording.md: SUPERSEDED 2026-05-19 footer appended (audit trail; original fix was technically correct against its charter, reversal is UX iteration not technical defect). - .planning/debug/resolved/01-09-save-does-not-stop-recording.md: NEW debug record landed directly in resolved/ (no checkpoint cycle — orchestrator-diagnosed reversal). Documents symptom, charter clarification cycle, fix shape, RED→GREEN evidence with commit SHAs + vitest/harness output, anti-regression coverage at unit + E2E layers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
173 lines
6.6 KiB
Markdown
173 lines
6.6 KiB
Markdown
---
|
|
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
|