Mark
cd83eb0498
Plan 01-13 Task 9 operator UAT closure. Operator 2026-05-19 empirical
session: SAVE click downloaded zip but recording stayed live (badge=REC,
sharing banner persisted, subsequent toolbar press re-opened SAVE-only
popup). Operator pressed 4×, got 2 zips + confusion.
Root cause: src/background/index.ts saveArchive() returns success after
chrome.downloads.download without signaling offscreen to stop or
transitioning the SW state machine — SPEC `Тз расширение фаза1.md`
"one click MUST produce a self-contained archive" was over-extended to
"always-on" framing by the implementation.
Fix contract (RED today; GREEN after src/background/index.ts patch):
A: setBadgeText({text:''}) called post-save (setIdleMode side effect)
B: setPopup({popup:''}) called post-save (re-enables chrome.action.onClicked
restart path per MV3 contract)
C: chrome.runtime.sendMessage({type:'STOP_RECORDING'}) dispatched
(offscreen recorder.ts:848 STOP_RECORDING case already wired —
no offscreen-side change needed)
D: NO mokosh-recovery-* notification fires (deliberate stop ≠ error;
mirrors Bug B `user-stopped-sharing` suppression branch from
.planning/debug/resolved/01-09-recovery-flow.md)
Tests A/B/C RED (assertion errors `expected 0 >= 1`); Test D GREEN today
as the regression guard against fix over-rotating to setErrorMode.
Test architecture mirrors tests/background/request-id-protocol.test.ts:
synthetic BUFFER response delivered via port.onMessage listeners to drive
saveArchive's request-id'd buffer fetch to completion. Empty-segments
BUFFER causes createArchive → EmptyVideoBufferError → catch branch; the
fix's STOP+IDLE dispatch MUST happen on both success and empty-buffer
paths (operator UI contract: SAVE click = stop, success or empty alike).
Debug record: .planning/debug/01-09-save-stops-recording.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
150 lines
6.8 KiB
Markdown
150 lines
6.8 KiB
Markdown
---
|
||
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).
|