Files
mokosh/.planning/phases/01-stabilize-video-pipeline/01-09-SUMMARY.md
Mark c711d7e74e docs(01-09): SUMMARY — Tasks 1-4 autonomous complete; Task 5 awaiting operator
Plan 01-09 SUMMARY:
  • 17 new tests landed GREEN (4 displaySurface + 5 toolbar-action
    including W-02 popup-idle-race + 4 badge + 4 notification).
  • Baseline 64 + 17 new = 81 GREEN. Full suite 18 files / 81 tests.
  • Tier-1 SW-bundle-import gate (Layer 1 + 2) remains GREEN.
  • tsc clean; npm run build clean; dist/manifest.json carries
    notifications permission.
  • 4 deviation rules auto-fixed inline (navigator getter helper,
    jsdom-free W-02 Test E refactor, cursor type-widening cast,
    chrome.* listener try/catch for pre-existing test compatibility).
  • Task 5 (operator empirical checkpoint) deferred per plan.
2026-05-17 15:49:24 +02:00

15 KiB

phase: 01-stabilize-video-pipeline plan: 09 subsystem: offscreen-recorder + background-sw + popup + manifest tags: - getDisplayMedia - displaySurface - cursor - toolbar - chrome.action.onClicked - chrome.notifications - chrome.runtime.onStartup - badge-state-machine - manifest - popup-save-only - D-15-display-surface requires: - 01-08 (Plan 01-08 SUMMARY + Tier-1 SW-bundle-import gate GREEN) provides: - REQ-video-ring-buffer:closure-via-D-15-display-surface - operator-facing badge state machine (REC/OFF/ERROR) + recovery notifications - getDisplayMedia constraint hint (displaySurface:'monitor', cursor:'always') + post-grant enforcement (wrong-display-surface RECORDING_ERROR) - toolbar-onClicked direct-to-picker flow (popup SAVE-only) - onStartup notification inviting fresh recording on browser launch affects: - src/offscreen/recorder.ts (CaptureErrorCode union + classifyCaptureError + getDisplayMedia call site + post-grant validation block) - src/background/index.ts (badge state machine + 3 mode helpers + 3 new listener registrations + RECORDING_ERROR onMessage branch + initialize() setIdleMode call) - src/popup/index.ts (SAVE-only — checkPermissions + requestPermissions removed; popupState defaults to hasPermissions:true so SAVE is enabled) - manifest.json (added 'notifications' permission) - smoke.sh (SHARE_TARGET monitor mode + locale-fallback comment + HTML stub instruction updates; T+/wall overlay preserved) tech-stack: added: - chrome.action.onClicked (MV3) - chrome.notifications.create / clear / onClicked - chrome.runtime.onStartup - MediaTrackConstraints displaySurface + cursor (Screen Capture spec) patterns: - 3-state badge state machine driven by setBadgeState helper - setPopup('' ⇄ html) dance gating onClicked vs popup-open - Constraint-hint-vs-enforcement gap: getDisplayMedia constraints are HINTS per W3C spec; post-grant track.getSettings().displaySurface check is the actual enforcement - Notification id prefix namespace ('mokosh-startup-', 'mokosh-recovery-') for spoofing mitigation (T-1-09-01) - Defense-in-depth try/catch around all chrome.* listener registrations so unit-test stubs that don't define every surface don't crash SW load key-files: created: - tests/offscreen/display-surface-constraint.test.ts (4 tests) - tests/background/toolbar-action.test.ts (5 tests inc. W-02 Test E) - tests/background/badge-state-machine.test.ts (4 tests) - tests/background/onstartup-notification.test.ts (4 tests) modified: - src/offscreen/recorder.ts (CaptureErrorCode + getDisplayMedia + validation) - src/background/index.ts (~150 LOC of helpers + listeners + handler) - src/popup/index.ts (~50 LOC removed, defaults flipped) - manifest.json (1 permission added) - smoke.sh (3 string literals + 14 comment lines) decisions: - displaySurface constraint is shipped with strict deep-equality test pinning the EXACT constraints object {video:{displaySurface:'monitor', cursor:'always'},audio:false} — a future refactor that drops either constraint silently fails loudly. cursor:'always' was opportunistically lifted from the Phase 5 deferral list per the I-03 fix. - Post-grant validation is the actual enforcement (spec treats displaySurface as a hint, not a hard constraint). Stream tracks are stopped + mediaStream nulled + Error thrown — the existing catch block routes it through classifyCaptureError + RECORDING_ERROR. - setPopup '' ⇄ html dance chosen over removing default_popup entirely (Option B per the plan interfaces): keeps the manifest stable + lets the SW dynamically gate onClicked vs popup-open per state. - Popup defaults flipped to hasPermissions:true / isRecording:true because under the SAVE-only charter the popup only opens when recording is already active (setPopup html path is set by setRecordingMode/setErrorMode). - All Plan 01-09 listener registrations wrapped in try/catch to preserve the 6 pre-existing background tests (request-id-protocol + port-lifecycle-continuous) whose chrome stubs predate chrome.action/notifications/onStartup. Production MV3 always provides these surfaces. - smoke.sh SHARE_TARGET set to English 'Entire screen' with a 14-line locale-fallback comment citing chromium/grit generated_resources.grd as the authoritative source. Per W-04 fix the prior citation to desktop_capture_device_uma_types.cc (UMA enums) was wrong and dropped. metrics: duration: ~35 minutes (executor agent) completed: 2026-05-17 task_count: 4 autonomous (Task 5 is operator-gated checkpoint, deferred) files_modified: 9 (4 created + 5 modified) net_loc: +1200 (~887 test LOC + ~310 src LOC + ~14 smoke comment lines)

Phase 1 Plan 9: Toolbar-onClicked Direct-to-Picker + Display-Surface Monitor + Badge State Machine + Notifications + SAVE-only Popup

Activation-cost reduction (3 clicks → 2) + footgun retirement (operator-picks-tab no longer reaches recording) + operator-visible recording state via badge + OS notifications inviting fresh starts and surfacing errors.

One-Liner

displaySurface:'monitor' + cursor:'always' getDisplayMedia constraints with post-grant enforcement; chrome.action.onClicked toolbar-to-picker flow (popup SAVE-only); 3-state badge machine (REC/OFF/ERROR) + chrome.runtime.onStartup notification + chrome.notifications.onClicked recovery flow.

What Landed

Tests (17 new, all GREEN end-of-plan)

Test File Tests Contract Pinned
tests/offscreen/display-surface-constraint.test.ts 4 getDisplayMedia exact constraints + non-monitor pick teardown + monitor over-fire guard + classifyCaptureError branch
tests/background/toolbar-action.test.ts 5 (A-E) onClicked registered + no-record→start + recording→no-double-start + setPopup state-machine + popup SAVE-only init contract (W-02 Test E)
tests/background/badge-state-machine.test.ts 4 REC palette + OFF palette (init) + ERROR palette + RECORDING_ERROR-triggered ERROR transition
tests/background/onstartup-notification.test.ts 4 onStartup registered + creates mokosh-startup- notification + onClicked clears + START_RECORDING + RECORDING_ERROR triggers mokosh-recovery-

Source Changes

  • src/offscreen/recorder.tsCaptureErrorCode union extended with 'wrong-display-surface'; classifyCaptureError matches the prefix; getDisplayMedia call carries {video:{displaySurface:'monitor',cursor:'always'},audio:false} (typed via explicit widening cast — lib.dom.d.ts omits cursor); post-grant validation reads track.getSettings().displaySurface, on mismatch tears down stream + nulls mediaStream + throws wrong-display-surface Error which routes through the existing catch+RECORDING_ERROR pipeline.

  • src/background/index.ts — Added badge palette constants (SCREAMING_SNAKE), setBadgeState(state: 'REC'|'OFF'|'ERROR') helper (3-state machine), setIdleMode/setRecordingMode/setErrorMode mode helpers, startVideoCapture wires setRecordingMode on success and setErrorMode in catch, chrome.action.onClicked direct-to-picker listener, chrome.runtime.onStartup notification creator, chrome.notifications.onClicked mokosh-prefix-gated handler, RECORDING_ERROR onMessage branch with recovery notification, initialize() calls setIdleMode at boot. All registrations defensively wrapped in try/catch.

  • src/popup/index.ts — Removed checkPermissions + requestPermissions functions; popupState defaults to hasPermissions:true, isRecording:true under SAVE-only charter; init() no longer fires REQUEST_PERMISSIONS; empty-state copy points operator back to toolbar icon.

  • manifest.json — Added "notifications" to permissions array.

  • smoke.shSHARE_TARGET="Entire screen" (locale-English) + 14-line locale-fallback comment block citing Chromium grit generated_resources.grd as authoritative source for IDS_DESKTOP_MEDIA_PICKER_SOURCE_TYPE_SCREEN identifier (locale strings drift across Chrome versions); HTML title made distinct from screen-source string; <ol> and intro paragraph updated to reflect entire-screen sharing. T+/wall timer overlay (commit 923aaca) preserved.

Test Counts

  • Baseline before Plan 01-09: 15 files / 64 tests / all GREEN.
  • After Task 1 RED commit: 16 files / 68 tests / 4 RED (Tests 1, 2, 3, 4 → only Test 3 GREEN because no over-fire from a non-existent validation block).
  • After Task 2 GREEN: 16 files / 68 tests / all GREEN.
  • After Task 3 RED commit: 18 files / 81 tests / 13 RED.
  • After Task 4 GREEN: 18 files / 81 tests / all GREEN.

Deviations from Plan

Auto-fixed Issues

1. [Rule 3 - Blocking] Vitest node env: navigator getter-only

  • Found during: Task 1 (running the preserved RED test).
  • Issue: globalThis.navigator in Vitest's node env has only a getter; direct assignment throws Cannot set property navigator of #<Object> which has only a getter. All 4 preserved tests failed at the env-setup step instead of for the right contract reasons.
  • Fix: Added installNavigatorStub(spy) helper that uses Object.defineProperty(globalThis, 'navigator', {value, configurable:true, writable:true}) instead of direct assignment. All 4 navigator assignments rewired to the helper.
  • Files modified: tests/offscreen/display-surface-constraint.test.ts
  • Commit: 333e0dc

2. [Rule 3 - Blocking] jsdom not installed; W-02 Test E refactored to node-env

  • Found during: Task 3 (running the new test files).
  • Issue: The plan's W-02 Test E originally used @vitest-environment jsdom pragma but jsdom is not in node_modules. Installing it would be an unscoped dependency add.
  • Fix: Refactored Test E to a node-env-friendly form that manually stubs the document surface the popup module touches (getElementById('saveButton'/'statusMessage'), querySelector('.button-text'), addEventListener on both button and document, plus a _docListeners map so the test can synthesize the DOMContentLoaded event). Test E still pins both contracts (E1: no REQUEST_PERMISSIONS at popup-open; E2: saveButton enabled).
  • Files modified: tests/background/toolbar-action.test.ts
  • Commit: 2d7ff7d

3. [Rule 1 - Bug] TypeScript: cursor not in MediaTrackConstraints

  • Found during: Task 2 (post-source-edit npx tsc --noEmit).
  • Issue: lib.dom.d.ts's MediaTrackConstraints type omits cursor (the Screen Capture spec defines it but TypeScript's bundled types lag; cited MDN). Initial GREEN edit failed TSC with TS2353: Object literal may only specify known properties.
  • Fix: Used explicit type-widening cast as DisplayMediaStreamOptions & {video:{displaySurface:'monitor';cursor:'always'}} (NOT as any per project style — CLAUDE.md "Don't ignore lint/type errors without researching proper fixes first") to add the field precisely.
  • Files modified: src/offscreen/recorder.ts
  • Commit: de162b4

4. [Rule 1 - Bug] Pre-existing background tests broken by new chrome. listener registrations*

  • Found during: Task 4 (full-suite run after GREEN edit).
  • Issue: 6 pre-existing background tests (5 in request-id-protocol.test.ts, 1 in port-lifecycle-continuous.test.ts) crashed at module load with TypeError: Cannot read properties of undefined (reading 'onClicked') because their chrome stubs predate chrome.action, chrome.notifications, chrome.runtime.onStartup.
  • Fix: Wrapped each of the 3 new top-level listener addListener calls in try/catch (matches the existing defensive pattern in the file, e.g. chrome.offscreen?.hasDocument check at line 673). Production MV3 always provides these surfaces.
  • Files modified: src/background/index.ts
  • Commit: 06dee24

Tier-1 SW-Bundle-Import Gate

GREEN throughout. The gate exercises npm run build + Layer 1 (SW module loads under jsdom-like environment) + Layer 2 (remuxSegments runs with the Buffer polyfill). The Plan 01-09 changes (new listener registrations, badge state machine, popup SAVE-only) compile and bundle cleanly; the Buffer polyfill from Plan 01-08 commit dd7bf00 + ebml CJS resolution from cc6e81a both remain intact (no vite.config.ts edits in this plan).

Task 5 Status: Operator Checkpoint — Deferred

Task 5 (checkpoint:human-verify) requires an operator to run npm run build + ./smoke.sh, open Chrome, load the unpacked extension, and walk through 13 end-to-end verification steps covering: notification at browser startup, monitor-only picker, badge transitions REC/OFF/ERROR, recovery notification on stop-sharing, popup-opens-when-recording, idle-toolbar-click-triggers-picker. The how-to-verify section in the plan file lists all 13 steps + the resume-signal contract.

The autonomous side (Tasks 1-4) is complete with 17/17 new tests GREEN and full suite 18 files / 81 tests / all GREEN. tsc clean; build clean; dist/manifest.json carries the notifications permission.

Architectural Notes Worth Carrying Forward

  • Constraint-hint-vs-enforcement gap. getDisplayMedia({video:{displaySurface:'monitor',...}}) is a HINT to Chrome's picker, NOT a hard constraint — operators can still pick a tab or window. Post-grant track.getSettings().displaySurface is the actual enforcement. Future plans that add new MediaTrackConstraints should treat constraints as preferences and add post-grant validation for anything load-bearing.

  • setPopup '' ⇄ html dance. chrome.action.onClicked.addListener fires ONLY when no default_popup is set (or it's been cleared via chrome.action.setPopup({popup:''})). Dynamically swapping the popup is the canonical Chrome pattern for "click-action depends on state" toolbar buttons. The SAVE-only popup charter relies on this — when recording is OFF the click does start-action; when REC it opens SAVE.

  • mokosh- notification id namespace. All Plan 01-09 notifications use mokosh-startup- or mokosh-recovery- prefixes with Date.now() suffix for uniqueness. The onClicked handler validates the prefix before any side effect (T-1-09-01 spoofing mitigation). Future notification work should keep extending this namespace (e.g. mokosh-savefail- for save-error toasts).

  • Defense-in-depth chrome. try/catch.* Every chrome API call that touches a surface a unit-test stub might not fully implement is wrapped in try/catch + a logger.warn. This is now the project's documented pattern (per the deviation 4 footnote) — unit tests should be able to load the SW module without crashing even if they only stub the subset they care about.

Self-Check: PASSED

  • FOUND: tests/offscreen/display-surface-constraint.test.ts
  • FOUND: tests/background/toolbar-action.test.ts
  • FOUND: tests/background/badge-state-machine.test.ts
  • FOUND: tests/background/onstartup-notification.test.ts
  • FOUND: .planning/phases/01-stabilize-video-pipeline/01-09-SUMMARY.md
  • FOUND: 333e0dc (Task 1 RED)
  • FOUND: de162b4 (Task 2 GREEN)
  • FOUND: 2d7ff7d (Task 3 RED)
  • FOUND: 06dee24 (Task 4 GREEN)

Known Stubs

None. The popup empty-state copy points operators to use the toolbar icon — this is intentional UX, not a stub (the toolbar icon is the start path; the popup is the save path). No placeholder data or hardcoded empty values flowing to UI.