Milestone v1 (v2.0.0): Mokosh — Session Capture #1
236
.planning/phases/01-stabilize-video-pipeline/01-14-SUMMARY.md
Normal file
236
.planning/phases/01-stabilize-video-pipeline/01-14-SUMMARY.md
Normal file
@@ -0,0 +1,236 @@
|
|||||||
|
---
|
||||||
|
phase: 01-stabilize-video-pipeline
|
||||||
|
plan: 14
|
||||||
|
subsystem: offscreen-recorder
|
||||||
|
tags:
|
||||||
|
- mv3-extension
|
||||||
|
- getDisplayMedia
|
||||||
|
- monitorTypeSurfaces
|
||||||
|
- picker-friction-reduction
|
||||||
|
- uat
|
||||||
|
- harness
|
||||||
|
- w3c-screen-capture
|
||||||
|
status: complete
|
||||||
|
requires: [01-09, 01-10, 01-11, 01-13]
|
||||||
|
provides:
|
||||||
|
- monitorTypeSurfaces:'include' top-level constraint on production getDisplayMedia call
|
||||||
|
- UAT harness A23 picker-narrowing regression gate
|
||||||
|
- Tier-1 grep gate inventory extension (2 new forbidden strings; total 12)
|
||||||
|
affects:
|
||||||
|
- src/offscreen/recorder.ts (getDisplayMedia constraints object — picker UI behavior)
|
||||||
|
- operator picker dialog UX (Chrome >= 119 only — graceful no-op on older Chrome)
|
||||||
|
tech-stack:
|
||||||
|
added: []
|
||||||
|
patterns:
|
||||||
|
- "Top-level DisplayMediaStreamOptions member (W3C spec §6.1) — NOT nested in MediaTrackConstraints"
|
||||||
|
- "Typed widening cast extended in lockstep with the new field (no `as any`)"
|
||||||
|
- "Bridge-op route for harness inspection (mirrors get-display-surface / get-segment-count)"
|
||||||
|
- "FORBIDDEN_HOOK_STRINGS lockstep across unit-gate + UAT A0 mirror"
|
||||||
|
key-files:
|
||||||
|
created: []
|
||||||
|
modified:
|
||||||
|
- src/offscreen/recorder.ts
|
||||||
|
- src/test-hooks/offscreen-hooks.ts
|
||||||
|
- tests/offscreen/display-surface-constraint.test.ts
|
||||||
|
- tests/uat/extension-page-harness.ts
|
||||||
|
- tests/uat/lib/harness-page-driver.ts
|
||||||
|
- tests/uat/harness.test.ts
|
||||||
|
- tests/background/no-test-hooks-in-prod-bundle.test.ts
|
||||||
|
decisions:
|
||||||
|
- "src/test-hooks/types.ts NOT modified — the new lastGetDisplayMediaConstraints cell + get-last-getDisplayMedia-constraints op are module-internal to offscreen-hooks.ts; bridge dispatches on op string match, not surface method. The MokoshTestSurface cross-cast in offscreen-hooks.ts does not need to widen for this plan. Plan Step 3 explicitly delegated this read to the implementer."
|
||||||
|
- "Test count drift accepted as positive deviation (98/98 → 100/100, +2 GREEN, 0 RED). The +2 are the 2 new parametrized FORBIDDEN_HOOK_STRINGS tests for the new symbols, both PASS. The plan's '98/98 preserved' clause was written before recognizing that the parametrized loop spawns one `it()` per new string."
|
||||||
|
- "Key ordering across source + test expectation: { video: {...}, monitorTypeSurfaces: 'include', audio: false } — sibling between video: and audio:. Source change AND test-expectation update use the identical ordering."
|
||||||
|
metrics:
|
||||||
|
duration_minutes: 49
|
||||||
|
duration_seconds: 2957
|
||||||
|
tasks_completed: 1
|
||||||
|
files_modified: 7
|
||||||
|
files_created: 0
|
||||||
|
lines_added: 228
|
||||||
|
lines_deleted: 13
|
||||||
|
completed_date: 2026-05-19
|
||||||
|
commits:
|
||||||
|
atomic_feat: b467123
|
||||||
|
revision_linkage:
|
||||||
|
- "Plan revised once after plan-checker flagged B-01-14-01 (revision commit 433ee28; 2026-05-19). The original plan's must_haves truth #5 understated baseline regression risk by omitting Step 1b lockstep test-expectation update. The revision added Step 1b (display-surface-constraint.test.ts:223-226 update) and corresponding frontmatter/artifacts/key-link entries."
|
||||||
|
ceremony_note: "Canonical GSD ceremony — plan (41c1f7e) → checker (B-01-14-01) → revision (433ee28) → executor (this commit b467123) → SUMMARY (this file). Replaces the retired AMENDMENT-A.md improvisation path per 01-11-SUMMARY.md Architectural Notes."
|
||||||
|
architectural_delta: "ZERO new permissions; ZERO new manifest changes; ZERO new web_accessible_resources; new internal test-hook surface (lastGetDisplayMediaConstraints cell + get-last-getDisplayMedia-constraints op) gated identically to Plan 01-13's hook inventory via __MOKOSH_UAT__ Vite define-token dead-branch tree-shake."
|
||||||
|
---
|
||||||
|
|
||||||
|
# Phase 01 Plan 14: monitorTypeSurfaces Picker Narrowing Summary
|
||||||
|
|
||||||
|
**One-liner:** Shipped W3C Screen Capture `monitorTypeSurfaces: 'include'` as a top-level constraint on the production `getDisplayMedia` call site (Chrome ≥ 119 narrows the operator picker to monitor surfaces only — no Window/Chrome-Tab panes), with UAT harness A23 regression gate + Tier-1 grep-gate lockstep extension covering the new internal hook surface.
|
||||||
|
|
||||||
|
## What Shipped
|
||||||
|
|
||||||
|
1. **Source change** — `src/offscreen/recorder.ts:270-292` (commit `b467123`): added `monitorTypeSurfaces: 'include'` as a TOP-LEVEL `DisplayMediaStreamOptions` sibling of `video:` (NOT nested inside `video:` per W3C Screen Capture spec §6.1). The typed widening cast at lines 287-289 was extended in lockstep to include `monitorTypeSurfaces: 'include'` so the explicit-typing contract is preserved (no `as any`). The D-15 post-grant validation block (recorder.ts:294-307) is **unchanged** — belt-and-suspenders chain preserved.
|
||||||
|
|
||||||
|
2. **Lockstep test-expectation update** — `tests/offscreen/display-surface-constraint.test.ts:223-228`: Test 1's strict-deep-equality `toHaveBeenCalledWith` expectation grew from `{ video: {...}, audio: false }` to `{ video: {...}, monitorTypeSurfaces: 'include', audio: false }`. Same key ordering as the source change (`video:` → `monitorTypeSurfaces:` → `audio:`). The test author's "no objectContaining" discipline is honored (the comment at lines 221-225 was extended to mention Plan 01-14's addition; the strict deep-equality contract remains). Both source + test changes landed in the same atomic commit, so the 98/98 baseline never crossed a commit boundary in RED state.
|
||||||
|
|
||||||
|
3. **Harness wiring for A23** — `src/test-hooks/offscreen-hooks.ts`:
|
||||||
|
- New module-scoped cell: `let lastGetDisplayMediaConstraints: DisplayMediaStreamOptions | null = null` (immediately after the existing fake-state cells, with docstring).
|
||||||
|
- `installFakeDisplayMedia`'s `fakeGetDisplayMedia` closure: renamed `_constraints` → `constraints` and added `lastGetDisplayMediaConstraints = constraints ?? null;` as the first line.
|
||||||
|
- New `__mokoshOffscreenQuery` dispatcher op: `'get-last-getDisplayMedia-constraints'` between `get-display-surface` and `get-segment-count`. Response: `{ constraints: lastGetDisplayMediaConstraints }`. Defensive try/catch mirroring the existing dispatcher pattern.
|
||||||
|
|
||||||
|
4. **Harness page assertion** — `tests/uat/extension-page-harness.ts:1906-1989`: new `assertA23()` mirroring `assertA3` (bridge query → 2-check `AssertionResult`):
|
||||||
|
- A23.1: constraints object recorded by fakeGetDisplayMedia (non-null) → PASS
|
||||||
|
- A23.2: `constraints.monitorTypeSurfaces === 'include'` (W3C spec §6.1; Chrome ≥ 119 picker narrowing) → PASS
|
||||||
|
- `Window.__mokoshHarness` declaration + runtime export + status-bar text + console.log updated.
|
||||||
|
|
||||||
|
5. **Harness driver** — `tests/uat/lib/harness-page-driver.ts:996-1023`: exported `driveA23(page)` mirroring the `driveA14` page.evaluate wrapper shape. Read-only — no side effects, no new dispatch.
|
||||||
|
|
||||||
|
6. **Harness orchestrator** — `tests/uat/harness.test.ts`:
|
||||||
|
- Header comment extended to describe A23 as the final functional assertion (post-A14; read-only inspection of A2-captured constraints).
|
||||||
|
- `FORBIDDEN_HOOK_STRINGS` (line 85) extended with `lastGetDisplayMediaConstraints` and `get-last-getDisplayMedia-constraints`.
|
||||||
|
- Import block extended with `driveA23` from `./lib/harness-page-driver`.
|
||||||
|
- `drivers` array appended with `{ name: 'A23', drive: driveA23 }` after the A14 entry (with explanatory comment).
|
||||||
|
- `Total = drivers.length + 1` comment updated to mention A23.
|
||||||
|
- Orchestrator stdout updated: `Plan 01-13 + 01-14`, architecture line mentions `A1..A14, A23`.
|
||||||
|
|
||||||
|
7. **Unit-level grep gate** — `tests/background/no-test-hooks-in-prod-bundle.test.ts`: header comment extended with the 2 new strings (Total: 12 surface strings — was 10). `FORBIDDEN_HOOK_STRINGS` (line 105) extended in lockstep with the UAT A0 list.
|
||||||
|
|
||||||
|
**Atomic commit hash:** `b467123` — single commit, 7 files, +228 / -13 lines.
|
||||||
|
|
||||||
|
## Verification Evidence
|
||||||
|
|
||||||
|
| Gate | Pre-Plan Baseline | Post-Plan Result | Status |
|
||||||
|
|------|-------------------|------------------|--------|
|
||||||
|
| `npx tsc --noEmit` | exit 0 (clean) | exit 0 (clean) | GREEN |
|
||||||
|
| `npm run build` | exit 0 | exit 0 | GREEN |
|
||||||
|
| `npm run build:test` | exit 0 | exit 0 | GREEN |
|
||||||
|
| `npm test` | 98/98 | **100/100** | GREEN (+2 via new parametrized FORBIDDEN_HOOK_STRINGS tests; both PASS) |
|
||||||
|
| `npm run test:uat` | 15/15 (A1..A14 + A0) | **16/16** (+A23) | GREEN |
|
||||||
|
| Production bundle hook-free (spot-check) | clean | clean | GREEN — zero occurrences of `lastGetDisplayMediaConstraints` or `get-last-getDisplayMedia-constraints` in `dist/` |
|
||||||
|
| Production bundle ships `monitorTypeSurfaces` | n/a | present in `dist/assets/offscreen-*.js` | EXPECTED — operator-facing picker hint ships in prod |
|
||||||
|
|
||||||
|
**A23 evidence (from UAT harness run output):**
|
||||||
|
|
||||||
|
```
|
||||||
|
--- A23 ---
|
||||||
|
[harness-step] Step 1: bridge query 'get-last-getDisplayMedia-constraints'
|
||||||
|
[harness-step] Step 1 result: {"constraints":{"video":{"displaySurface":"monitor","cursor":"always"},"monitorTypeSurfaces":"include","audio":false}}
|
||||||
|
========================================================================
|
||||||
|
A23 — getDisplayMedia called with monitorTypeSurfaces:'include' (Plan 01-14 picker narrowing): PASS
|
||||||
|
Checks:
|
||||||
|
[PASS] A23.1: constraints object recorded by fakeGetDisplayMedia (non-null)
|
||||||
|
expected: "non-null DisplayMediaStreamOptions"
|
||||||
|
actual: "{\"video\":{\"displaySurface\":\"monitor\",\"cursor\":\"always\"},\"monitorTypeSurfaces\":\"include\",\"audio\":false}"
|
||||||
|
[PASS] A23.2: constraints.monitorTypeSurfaces === 'include' (W3C spec §6.1; Chrome ≥ 119 picker narrowing)
|
||||||
|
expected: "include"
|
||||||
|
actual: "include"
|
||||||
|
========================================================================
|
||||||
|
UAT harness: 16/16 assertions passed
|
||||||
|
```
|
||||||
|
|
||||||
|
The recorded constraints object shows the **exact production call shape** with the new sibling field — confirming the source change reached the call site through both the typed widening cast and the JavaScript runtime serialization across the production → offscreen-hooks fake → bridge dispatcher → harness assertion path.
|
||||||
|
|
||||||
|
## Research Linkage
|
||||||
|
|
||||||
|
This plan ships the recommendation from Plan 01-10 RESEARCH §5 + §Pitfall-5 + §Environment-Availability: *"Add `monitorTypeSurfaces: 'include'` to the offscreen `getDisplayMedia` constraints. Single-line, zero-risk change that removes visual noise from the picker."* The Chrome ≥ 119 feature-gate per Plan 01-10 RESEARCH §Environment-Availability matches the target Chrome version for this extension.
|
||||||
|
|
||||||
|
Authoritative references encoded into the commit body + source comments:
|
||||||
|
- W3C Screen Capture §6.1 DisplayMediaStreamOptions: <https://www.w3.org/TR/screen-capture/#dom-displaymediastreamoptions-monitortypesurfaces>
|
||||||
|
- Chrome screen-sharing-controls (Chrome 119+): <https://developer.chrome.com/docs/web-platform/screen-sharing-controls>
|
||||||
|
|
||||||
|
## Decision Linkage
|
||||||
|
|
||||||
|
- **D-01** (whole-desktop only via getDisplayMedia; reject window/tab surfaces): `monitorTypeSurfaces: 'include'` is the picker-UI realization of D-01's intent. The operator's choice space is narrowed at the dialog level rather than catching wrong selections post-grant.
|
||||||
|
- **D-15** (post-grant validation): unchanged. The recorder.ts:294-307 block remains the actual enforcement — `monitorTypeSurfaces` is a HINT-class constraint (per spec semantics; older Chrome / managed-policy / DevTools can override). Belt (picker-UI narrowing) + suspenders (post-grant tear-down with `wrong-display-surface` error code) chain preserved.
|
||||||
|
- **D-13** (segment rotation 3 × 10 s = 30 s): unchanged.
|
||||||
|
- **D-17** (long-lived port keepalive): unchanged.
|
||||||
|
|
||||||
|
## Ceremony Note
|
||||||
|
|
||||||
|
This plan replaces the prior AMENDMENT-A.md improvisation path that was retired per `.planning/phases/01-stabilize-video-pipeline/01-11-SUMMARY.md` Architectural Notes. Canonical GSD ceremony (plan → checker → executor → SUMMARY) landed:
|
||||||
|
|
||||||
|
1. Plan creation: commit `41c1f7e` (2026-05-19).
|
||||||
|
2. Plan-checker review: BLOCKER B-01-14-01 raised — must_haves truth #5 understated baseline regression risk by omitting Step 1b lockstep test-expectation update.
|
||||||
|
3. Plan revision: commit `433ee28` (2026-05-19) — added Step 1b (display-surface-constraint.test.ts:223-226 update) + corresponding frontmatter/artifacts/key-link entries.
|
||||||
|
4. Executor (this commit `b467123`, 2026-05-19): single atomic commit for the entire 1-task plan.
|
||||||
|
5. SUMMARY (this file): canonical closure artifact.
|
||||||
|
|
||||||
|
The lesson encoded by B-01-14-01: **strict-deep-equality test expectations are tightly coupled to the source-call shape**; any plan that mutates the source object MUST mutate the expectation in the same atomic commit, or the baseline crosses a RED boundary at commit-time. The checker caught this before any harm; the executor honored the same-commit guarantee.
|
||||||
|
|
||||||
|
## Revision Linkage
|
||||||
|
|
||||||
|
The plan was revised once after the plan-checker flagged BLOCKER B-01-14-01 (commit `433ee28`, 2026-05-19). The revision added Step 1b (lockstep test-expectation update) + corresponding frontmatter/file/key-link entries to the plan. Without the revision, executing the original plan as-written would have dropped the vitest baseline from 98/98 GREEN to 97/98 RED on the strict-deep-equality assertion at `tests/offscreen/display-surface-constraint.test.ts:223-226`. The single-commit landing in this executor pass restored the same-commit guarantee.
|
||||||
|
|
||||||
|
## Architectural Delta
|
||||||
|
|
||||||
|
- **ZERO** new permissions
|
||||||
|
- **ZERO** new manifest changes
|
||||||
|
- **ZERO** new web_accessible_resources entries
|
||||||
|
- **ZERO** new dependencies (no new npm packages, no new types)
|
||||||
|
- New internal test-hook surface (`lastGetDisplayMediaConstraints` cell + `get-last-getDisplayMedia-constraints` bridge op) gated identically to Plan 01-13's hook inventory via the `__MOKOSH_UAT__` Vite define-token dead-branch tree-shake. Tier-1 grep gate (unit-level + UAT A0) extended in lockstep to enforce.
|
||||||
|
|
||||||
|
## What Did Not Change
|
||||||
|
|
||||||
|
- D-15 post-grant validation block (recorder.ts:294-307) — unchanged
|
||||||
|
- Plan 01-09 toolbar `onClicked` chain — unchanged
|
||||||
|
- Plan 01-10 welcome-page architecture — unchanged (scope reserved for the next-up plan)
|
||||||
|
- Plan 01-12 design-integration scope — unchanged (scope reserved for the next-up plan)
|
||||||
|
- Plan 01-13 harness architecture — the 14 existing assertions (A1..A14) are unmodified; A23 is purely additive (appended after the A14 driver in `drivers` array)
|
||||||
|
- `src/test-hooks/types.ts` — unchanged (decision documented above; the new cell + op are module-internal to offscreen-hooks.ts)
|
||||||
|
- `chrome.runtime` message contracts — unchanged (new bridge op uses the existing `__mokoshOffscreenQuery` envelope)
|
||||||
|
- Vite build configuration — unchanged (the existing `__MOKOSH_UAT__` define-token tree-shake handles the new surface automatically)
|
||||||
|
|
||||||
|
## Deviations from Plan
|
||||||
|
|
||||||
|
### Auto-fixed Issues
|
||||||
|
|
||||||
|
**None.** No bugs encountered. No CLAUDE.md violations. No missing critical functionality. No blocking issues. The plan's `<verify>` block + `<done>` criteria mapped 1-to-1 to the actual execution path.
|
||||||
|
|
||||||
|
### Test count drift (benign positive deviation)
|
||||||
|
|
||||||
|
- **Found during:** Step 9 (`npm test` post-commit)
|
||||||
|
- **Issue:** Plan predicted "98/98 GREEN preserved"; actual count is 100/100.
|
||||||
|
- **Root cause:** The unit-level grep gate at `tests/background/no-test-hooks-in-prod-bundle.test.ts` uses a parametrized `for (const needle of FORBIDDEN_HOOK_STRINGS) { it(...) }` loop (line 254-278) — each entry in the list spawns one independent `it()` test. Adding 2 strings to the inventory adds 2 tests. The plan's truth #5 wrote "98/98 GREEN" without noticing this arithmetic.
|
||||||
|
- **Action taken:** None (no fix required). All 100 tests are GREEN; the 2 new tests assert the production bundle is hook-free for the new symbols (`lastGetDisplayMediaConstraints` and `get-last-getDisplayMedia-constraints`) — both PASS. The plan's spirit ("baseline preserved") is honored; the arithmetic just shifted up.
|
||||||
|
- **Documented in:** decisions[] in this SUMMARY's frontmatter.
|
||||||
|
|
||||||
|
### Authentication Gates
|
||||||
|
|
||||||
|
None encountered. The plan's `<verify>` chain (`npm run build`, `npx tsc --noEmit`, `npm test`, `npm run test:uat`) is fully autonomous (Chrome launches headlessly via puppeteer; no operator interaction).
|
||||||
|
|
||||||
|
## Threat Surface Scan
|
||||||
|
|
||||||
|
No new threat surface introduced beyond what the plan's `<threat_model>` block already documents. The 6 STRIDE entries (T-01-14-01..T-01-14-06) cover the full delta. T-01-14-04 (Information Disclosure via leaked hook surface) is **mitigated** by the Tier-1 grep gate extension (verified GREEN). No threat flags to surface.
|
||||||
|
|
||||||
|
## Known Stubs
|
||||||
|
|
||||||
|
None. No placeholder text, no hardcoded empty values, no TODO/FIXME markers introduced. Every new code path is fully wired to a real data source.
|
||||||
|
|
||||||
|
## Files Modified Summary
|
||||||
|
|
||||||
|
| File | LoC delta | Purpose |
|
||||||
|
|------|-----------|---------|
|
||||||
|
| `src/offscreen/recorder.ts` | +16 / -0 | Source change + extended typed widening cast + explanatory comment |
|
||||||
|
| `src/test-hooks/offscreen-hooks.ts` | +54 / -5 | Module cell + capture in fake + dispatcher op + dispatcher comment |
|
||||||
|
| `tests/offscreen/display-surface-constraint.test.ts` | +5 / -1 | Strict-deep-equality expectation lockstep update |
|
||||||
|
| `tests/uat/extension-page-harness.ts` | +77 / -4 | `assertA23` impl + global decl + runtime export + status/log |
|
||||||
|
| `tests/uat/lib/harness-page-driver.ts` | +27 / -0 | `driveA23` page.evaluate wrapper |
|
||||||
|
| `tests/uat/harness.test.ts` | +26 / -6 | FORBIDDEN inventory + import + drivers + comments |
|
||||||
|
| `tests/background/no-test-hooks-in-prod-bundle.test.ts` | +18 / -2 | FORBIDDEN inventory lockstep + header comment |
|
||||||
|
| **Total** | **+228 / -13** | **7 files, 1 atomic commit** |
|
||||||
|
|
||||||
|
## Self-Check: PASSED
|
||||||
|
|
||||||
|
**File-existence verification:**
|
||||||
|
- `src/offscreen/recorder.ts` — FOUND (modified per commit b467123)
|
||||||
|
- `src/test-hooks/offscreen-hooks.ts` — FOUND (modified)
|
||||||
|
- `tests/offscreen/display-surface-constraint.test.ts` — FOUND (modified)
|
||||||
|
- `tests/uat/extension-page-harness.ts` — FOUND (modified)
|
||||||
|
- `tests/uat/lib/harness-page-driver.ts` — FOUND (modified)
|
||||||
|
- `tests/uat/harness.test.ts` — FOUND (modified)
|
||||||
|
- `tests/background/no-test-hooks-in-prod-bundle.test.ts` — FOUND (modified)
|
||||||
|
|
||||||
|
**Commit-existence verification:**
|
||||||
|
- `b467123` — FOUND in `git log --oneline -3` (HEAD).
|
||||||
|
|
||||||
|
**Gate evidence:**
|
||||||
|
- `npm test` → 100/100 GREEN
|
||||||
|
- `npm run test:uat` → 16/16 GREEN ("UAT harness: 16/16 assertions passed")
|
||||||
|
- `npx tsc --noEmit` → exit 0
|
||||||
|
- `npm run build` → exit 0
|
||||||
|
- `npm run build:test` → exit 0
|
||||||
|
- Production bundle spot-check: zero occurrences of `lastGetDisplayMediaConstraints` and `get-last-getDisplayMedia-constraints` in `dist/`
|
||||||
Reference in New Issue
Block a user