repremium/mokosh
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

docs(04-08): plan-checker iter-2 — PASSED (0 BLOCKER + 1 WARNING + 4 advisory)

Browse Source 
Verifies iter-2 plan revision at 1f2eb2e against iter-1 findings (051813e):

BLOCKER 1 (Vite ?url asset-emission path) — RESOLVED via explicit
web_accessible_resources entry for assets/*.webm in manifest.json
(Option B from iter-1 remediation; pre-decided + grep-gated; inert in
production because dist/ has zero *.webm assets).

BLOCKER 2 (eager-install contract preservation) — RESOLVED via SYNC
install + LAZY first-frame closure (Option A from iter-1 remediation).
installFakeDisplayMedia() remains synchronous; canplay wait + .play()
deferred into fakeGetDisplayMedia closure. Three grep gates codify the
contract (sync signature present + NOT async + no await callers).

All 5 iter-1 WARNINGs addressed concretely with grep-gated remediations.
All 3 iter-1 cosmetic-advisories addressed.

New iter-2 findings: 1 WARNING (displaySurface sub-gate scope ambiguity;
alternative documented; non-blocking) + 4 cosmetic-advisories (symbol
name lookup, SUMMARY-write practice, vitest math, duration rationale).
Below PASSED threshold.

Recommendation: proceed to execute Plan 04-08 Wave 5.5.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Mark
2026-05-22 09:44:11 +02:00
parent 1f2eb2e818
commit 9c334b77e8
1 changed files with 329 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

329
.planning/phases/04-harden-clean-up-optional/04-08-CHECKER-iter-2.md Normal file
View File

@@ -0,0 +1,329 @@
---
phase: 04-harden-clean-up-optional
plan: 08
checker_iteration: 2
checked_at: 2026-05-22T15:00:00Z
plan_commit: 1f2eb2e
parent_iter1_commit: 051813e
verdict: PASSED
severity_summary:
blocker: 0
warning: 1
cosmetic-advisory: 4
goal_backward_check: "Plan 04-08 iter-2 addresses both BLOCKERs from iter-1 with concrete, codified solutions: BLOCKER 1 (Vite ?url WAR strategy) is resolved by pre-deciding an explicit web_accessible_resources entry for assets/*.webm in manifest.json (Option B from iter-1 remediation), with grep gates enforcing both the presence of the entry AND the inert-production invariant. BLOCKER 2 (eager-install contract) is resolved by preserving installFakeDisplayMedia() as SYNCHRONOUS with the SYNC phase doing DOM creation + monkey-patch + module-level Promise kickoff, deferring the lazy first-frame wait (canplay + .play()) into the fakeGetDisplayMedia closure (Option A from iter-1 remediation). The five WARNINGs all have codified, grep-gated remediations in the verify blocks. The three cosmetic advisories are addressed in the plan body. One new WARNING surfaced during iter-2 review (the optional --check-display-surface-only sub-gate references a spike script mode that does not exist yet — labeled as 'create one or skip' but the create path is poorly scoped); four new cosmetic-advisories were noted. None of these rise to the level of blocking the plan."
recommendation: "PROCEED to execution. Plan 04-08 iter-2 is execution-ready. The single iter-2 WARNING is a low-severity scope-clarity issue on an OPTIONAL sub-gate (executor can take the documented alternative — rely on full spike re-run); it does not prevent the plan from achieving SC #1 closure. Total iter-2 findings (1 WARNING + 4 cosmetic-advisories) are below the PASSED threshold of '≤3 WARNINGs of low severity'."
---
# Plan 04-08 Pre-Execution Validation — Iter-2
**Plan under review:** `.planning/phases/04-harden-clean-up-optional/04-08-PLAN.md` @ commit `1f2eb2e`
**Parent iteration:** iter-1 at commit `051813e` (verdict: ITERATE-NEEDED; 2 BLOCKER + 5 WARNING + 3 cosmetic-advisory)
**Diff size (iter-1 → iter-2):** +740 / -236 lines (planner re-drafted Task 1 Step 3 substantially + expanded both verify blocks + added revision_history + must_haves entries + STRIDE threats)
**Authority artifacts re-read for iter-2:**
- `.planning/phases/04-harden-clean-up-optional/04-08-PLAN.md` @ 1f2eb2e (1307 lines)
- `.planning/phases/04-harden-clean-up-optional/04-08-CHECKER-iter-1.md` @ 051813e
- `.planning/phases/04-harden-clean-up-optional/04-04-PLAN.md` (476 lines)
- `.planning/phases/04-harden-clean-up-optional/04-04-SUMMARY.md` (267 lines + amendment)
- `.planning/debug/sw-offscreen-persistence-investigation-session-2.md` (97 lines)
- `.planning/phases/04-harden-clean-up-optional/04-CONTEXT.md`
- `.planning/PROJECT.md` + `.planning/ROADMAP.md` (lines 240-292)
- `src/test-hooks/offscreen-hooks.ts` (full 541 lines)
- `src/offscreen/recorder.ts:40-100` (top-level await + segments declaration)
- `tests/uat/spike-a33-sw-persistence.ts` (full 337 lines)
- `tests/uat/lib/harness-page-driver.ts:40-130` (Browser type + stopServiceWorker + interfaces)
- `tests/uat/extension-page-harness.ts` (assertA2 + setupFreshRecording location)
- `tests/uat/harness.test.ts:95-138, 340-487` (imports + drivers-array + wrapped-driver block)
- `globals.d.ts` (37 lines)
- `manifest.json` (current source) + `dist/manifest.json` (current build output — KEY EMPIRICAL: shows crxjs DID auto-WAR JS chunks `assets/logger-WoCoXGpL.js`, `assets/index-CgqXENQe.js`, `assets/index.ts-D0uUn23q.js` — refining the iter-1 picture)
- `vite.config.ts` (170 lines) + `vite.test.config.ts` (101 lines)
- `tests/background/no-test-hooks-in-prod-bundle.test.ts` (297 lines — full Tier-1 inventory + collectDistFiles helper structure)
- `dist/` + `dist-test/` file enumeration (verified zero `*.webm` in either currently; dist-test/assets/offscreen-hooks-BvgUCNzF.js present)
- `tests/fixtures/last_30sec.webm` (1888636 bytes; codec=vp9; width=1142; height=1044; duration=N/A per ffprobe — same as iter-1)
- `tests/uat/fixtures/` directory: DOES NOT EXIST YET — confirms Task 1 Step 1 still needs to land the directory + file
---
## Goal-Backward Verification — Iter-2 Still Closes SC #1?
**Architectural claim (unchanged from iter-1):** HTMLVideoElement.captureStream bypasses canvas-throttling per debug session-2; segments architecture preserved.
**Iter-2 reinforcement:** the SYNC-install + LAZY-first-frame contract is now codified with three independent grep gates in Task 1 verify:
1. `grep -c "installFakeDisplayMedia(): void"` ≥ 1 (sync signature)
2. `grep -c "export function installFakeDisplayMedia(): Promise"` == 0 (NOT async — negation gate)
3. `grep -c "await installFakeDisplayMedia"` == 0 (no callers turned async — bridge + eager-install preserved)
These three gates collectively make BLOCKER 2's race condition unreintroducible without tripping the build. The verification is mechanical, not judgment-based.
**Verdict:** YES — Plan 04-08 iter-2 IS execution-ready. Both blockers from iter-1 have concrete, codified, grep-gated remediations. The single new WARNING (below) is a scope-clarity issue on an optional sub-gate, not a blocker.
---
## Iter-1 BLOCKER Resolution Verification
### BLOCKER 1 (Vite `?url` asset-emission path) — ADDRESSED
**Iter-1 finding:** the `?url` import for a 1.9 MB WebM is the *extracted-asset* code path (different from Plan 01-10 mokosh-mark.svg's 877-byte inlined SVG path); the `@crxjs/vite-plugin` auto-WAR behavior for extracted-media-assets in offscreen-document context was unverified; the plan's contingency block delegated the decision to executor improvisation.
**Iter-2 resolution (verified):**
- **Strategy chosen:** Option B from iter-1 remediation (explicit `web_accessible_resources` entry for `assets/*.webm` in `manifest.json`). Plan lines 86-92, 407-419, 894-913 + must_haves.artifacts entry for manifest.json + key_links entry + STRIDE T-04-08-09 (regression catch).
- **Pre-decided, not delegated:** the plan explicitly states (line 908) "Rationale (locked in iter-2; no executor decision)". The pre-edit manifest excerpt + post-edit JSON are both shown verbatim in the `<interfaces>` block (lines 397-419). No "executor decides at run time" ambiguity remains.
- **Empirical justification:** Plan correctly states (line 911) that the WAR entry "is inert in production" because `find dist -name '*.webm' | wc -l == 0` (verified live: confirmed; the current production `dist/` has zero `*.webm` files).
- **Grep gates:**
- Task 1 verify: `grep -c '"assets/\*\.webm"' manifest.json | awk '$1 >= 1 {exit 0} {exit 1}'`
- Task 1 verify: `find dist -name '*.webm' | wc -l | awk '$1 == 0 {exit 0} {exit 1}'` (production inert)
- Task 1 verify: `find dist-test -name '*.webm' | wc -l | awk '$1 >= 1 {exit 0} {exit 1}'` (test bundle authoritative)
- Task 1 verify: `grep -r "synthetic-display-source" dist/ 2>/dev/null | wc -l | awk '$1 == 0 {exit 0} {exit 1}'` (no production leak)
**Iter-2 verification observation refining iter-1's picture:** examining the current `dist/manifest.json` empirically shows that `@crxjs/vite-plugin` DID auto-WAR three JS chunks (`assets/logger-WoCoXGpL.js`, `assets/index-CgqXENQe.js`, `assets/index.ts-D0uUn23q.js`). So crxjs's auto-WAR behavior IS active for extracted JS assets transitively reachable from extension-page entrypoints. Whether it would auto-WAR a WebM asset transitively-reachable-from-the-offscreen-document is a slightly different question (the iter-2 plan correctly identifies offscreen documents as a different code path than typical extension pages). The defensive explicit entry remains the right call — it's cheap, idempotent with whatever crxjs may or may not auto-WAR, and it's empirically inert in production.
**Verdict on BLOCKER 1:** RESOLVED.
### BLOCKER 2 (`installFakeDisplayMedia()` async conversion → race) — ADDRESSED
**Iter-1 finding:** the original plan's recommendation to convert `installFakeDisplayMedia()` to async + use `void installFakeDisplayMedia().catch(...)` at module load would create a race window where `recorder.startRecording`'s `await navigator.mediaDevices.getDisplayMedia(...)` could see the REAL (un-patched) navigator.mediaDevices.getDisplayMedia and hang on the Chrome screen-share picker.
**Iter-2 resolution (verified):**
- **Strategy chosen:** Option A from iter-1 remediation (SYNC install + LAZY first-frame closure). Plan lines 220-335 in `<interfaces>` + lines 618-892 in Task 1 Step 3.
- **SYNC contract preserved verbatim:** `installFakeDisplayMedia(): void` — function signature unchanged from current code. The plan's Sub-step 3c explicitly says (line 620): "**Critical contract:** `installFakeDisplayMedia()` MUST remain SYNCHRONOUS (no `async`/`await` in the function declaration). ... DO NOT convert to async. DO NOT change the function's return type."
- **SYNC phase work:**
- `document.createElement('video')` + setting all videoEl attrs (sync)
- `document.body.appendChild(videoEl)` (sync)
- `fakeVideoEl = videoEl` (sync module-level cell assignment)
- `fakeVideoReadyPromise = new Promise<void>((resolve, reject) => { ... })` (sync — Promise constructor returns immediately; the canplay listener is registered sync but does not yet fire)
- `videoEl.addEventListener('canplay', onCanPlay)` + `videoEl.addEventListener('error', onError)` (sync registration)
- `(navigator.mediaDevices as ...).getDisplayMedia = fakeGetDisplayMedia` (sync monkey-patch assignment — KEY GATE)
- **LAZY phase work** (inside `fakeGetDisplayMedia` closure):
- `await fakeVideoReadyPromise` (waits for canplay + .play() resolution; first call may block ~50-500ms; subsequent calls observe resolved Promise immediately)
- `mintStream()` → `videoEl.captureStream(30)` + `patchDisplaySurface(stream)` → return
- **Eager-install at lines 528-537 UNCHANGED:** plan's Sub-step 3f explicitly verifies the existing `try { installFakeDisplayMedia(); } catch (e) {...}` still works because the function remains sync void.
- **Bridge handler at lines 408-418 UNCHANGED:** plan's Sub-step 3g explicitly verifies the existing `if (op === 'install-fake-display-media') { ... return false; }` (sync `sendResponse + return false`) still works because the install itself stays sync.
- **Three grep gates codify the contract** (Task 1 verify block):
- `grep -c "installFakeDisplayMedia(): void"` ≥ 1 (sync signature present)
- `grep -c "export function installFakeDisplayMedia(): Promise"` == 0 (negation gate — explicitly verifies NOT async)
- `grep -c "await installFakeDisplayMedia"` == 0 (negation gate — neither bridge handler nor eager-install await it)
- **STRIDE threat T-04-08-08** explicitly codifies the regression-catch contract: "future refactor converts installFakeDisplayMedia() to async (breaking the eager-install contract — recurrence of iter-2 BLOCKER 2). Mitigate: Grep gate codified in Task 1 verify... Any future PR that flips the signature trips the build-test pipeline."
**Race-window analysis:** the iter-2 plan correctly identifies the head-start window — `fakeVideoReadyPromise` is kicked off at install time (sync), so by the time `recorder.startRecording` reaches `await navigator.mediaDevices.getDisplayMedia(...)` (which happens AFTER recorder.ts:46-48 top-level await resolves AND after recorder.bootstrap() AND after the production START_RECORDING handler dispatches), the video element has typically had hundreds of ms head start on its canplay event. The first invocation MAY block briefly inside the closure (~50-500ms per plan estimate); subsequent calls fast-path. This matches Option A from iter-1 remediation verbatim.
**Verdict on BLOCKER 2:** RESOLVED.
---
## Iter-1 WARNING Resolution Verification
### WARNING 1 (headless autoplay fallback) — ADDRESSED
**Iter-1 finding:** no fallback for `videoEl.play()` rejecting with NotAllowedError.
**Iter-2 resolution:** plan lines 289-295 + 711-728 — the Promise rejection path produces an explicit error class identifier:
```typescript
reject(new Error(
`synthetic-display-source.webm play() rejected: ${...} (autoplay-blocked or codec-unsupported in headless context)`
));
```
The error message includes the explicit class string `(autoplay-blocked or codec-unsupported in headless context)` so a downstream spike failure surfaces the precise root cause via the offscreen console capture (the existing harness diagnostic plumbing already captures offscreen.console). NOT a true "Plan B fallback" (no synthetic-user-gesture path; no canvas-fallback), but an EXPLICIT diagnostic signal so the failure mode is unambiguous.
**Verdict:** PARTIAL but acceptable. The iter-2 approach trades full fallback for explicit failure-mode signaling. Acceptable because (a) muted-autoplay in headless Chrome is well-established practice (numerous Chromium test suites rely on it); (b) the explicit error classification means a hostile failure surfaces in the spike re-run as an unambiguous signal rather than as a mysterious 0-frames result. NEW cosmetic-advisory below mentions one improvement.
### WARNING 2 (displaySurface monkey-patch portability) — ADDRESSED
**Iter-1 finding:** `patchDisplaySurface()` was assumed to work with HTMLVideoElement.captureStream tracks; not verified.
**Iter-2 resolution:** plan Sub-step 3 Step 7 (lines 966-967) adds an explicit `--check-display-surface-only` sub-gate:
```bash
HEADLESS=1 SKIP_PROD_REBUILD=1 npx tsx tests/uat/spike-a33-sw-persistence.ts --check-display-surface-only 2>&1 | tee /tmp/04-08-display-surface-check.log
```
with documented expected log line `DISPLAY-SURFACE-CHECK: PASSED`. Implementation noted as "append to the existing tests/uat/spike-a33-sw-persistence.ts script a --check-display-surface-only mode that runs only Steps 1-2 (prime + 5s post-prime probe), invokes __mokoshOffscreenQuery op='get-display-surface', asserts result is 'monitor', exits."
**Verdict:** ADDRESSED but with a NEW concern — see "New iter-2 WARNING" below regarding the optional-vs-required ambiguity of this sub-gate.
### WARNING 3 (spike probe-value grep gates) — ADDRESSED
**Iter-1 finding:** the three probe-value checks (POST-PRIME=0, PRE-KILL≥3, POST-KILL≥3) were buried in prose, not surfaced as automated grep gates.
**Iter-2 resolution:** Task 2 verify block contains explicit grep gates (line 1173):
```bash
grep -cE 'SPIKE PROBE \[POST-PRIME\]: segments\.length=0' /tmp/04-08-spike-rerun.log
grep -cE 'SPIKE PROBE \[PRE-KILL\]: segments\.length=([3-9]|[1-9][0-9]+)' /tmp/04-08-spike-rerun.log
grep -cE 'SPIKE PROBE \[POST-KILL\]: segments\.length=([3-9]|[1-9][0-9]+)' /tmp/04-08-spike-rerun.log
```
Cross-checked with the spike script (`tests/uat/spike-a33-sw-persistence.ts:139-144` + 252): the script emits the exact label strings `SPIKE PROBE [POST-PRIME]`, `SPIKE PROBE [PRE-KILL]`, `SPIKE PROBE [POST-KILL]` (when canonical mode; `POST-SKIP-KILL` only when SPIKE_SKIP_SW_KILL=1, which the canonical re-run does NOT set). Regex patterns match the actual emit format.
**Verdict:** RESOLVED.
### WARNING 4 (ROADMAP.md edit pre-spec + grep gate) — ADDRESSED
**Iter-1 finding:** the ROADMAP SC #1 flip was described inline without a grep gate to enforce the edit landed correctly.
**Iter-2 resolution:** plan Step 7 (lines 1118-1166) pre-specifies the EXACT pre-edit prose block + EXACT post-edit prose block (both shown verbatim) + adds 3 grep gates:
```bash
grep -c 'CLOSED via Plan 04-08' .planning/ROADMAP.md # >= 1
grep -c 'STATUS 2026-05-21: OPEN' .planning/ROADMAP.md # == 0 (old removed)
grep -c 'STATUS 2026-05-22: CLOSED' .planning/ROADMAP.md # >= 1
```
Cross-checked with current ROADMAP.md (line 254): `STATUS 2026-05-21: OPEN` is indeed present verbatim. The pre-edit text shown in the plan's <interfaces>-equivalent block matches the actual ROADMAP.md text. Plan also pre-specifies the Plan tracker line update at ~line 277 + the phase tracker table row at ~line 292.
**Verdict:** RESOLVED.
### WARNING 5 (Tier-2 filename-leak gate) — ADDRESSED
**Iter-1 finding:** the test-only `synthetic-display-source.webm` filename was not gated against accidental production-bundle leak.
**Iter-2 resolution:** plan Task 1 Step 6 (lines 919-948) adds a NEW Tier-2 sub-test in `tests/background/no-test-hooks-in-prod-bundle.test.ts`:
```typescript
test('Tier-2: synthetic-display-source filename does not leak into production dist/', () => {
const distFiles = collectDistFiles();
const offendingFiles: string[] = [];
for (const filePath of distFiles) {
const content = readFileSync(filePath, 'utf-8');
if (content.includes('synthetic-display-source')) {
offendingFiles.push(filePath);
}
}
expect(offendingFiles).toEqual([]);
});
```
Plan also notes (line 950): "the executor should READ the existing test file first to identify the exact name of the helper that walks dist/ files (the snippet above calls it `collectDistFiles` — verify the actual name and adapt)."
**Cross-check with actual test file:** the actual helper at `tests/background/no-test-hooks-in-prod-bundle.test.ts:152-171` is named `listAllFilesRecursive(DIST_DIR)`, NOT `collectDistFiles`. The plan acknowledges this ambiguity with its "verify the actual name and adapt" guidance. This is acceptable scope-of-discretion handoff — the executor reading the file first will substitute the correct symbol. NEW cosmetic-advisory below notes the executor could have been spared this lookup if the plan had stated the exact symbol name.
**Verdict:** RESOLVED (with minor advisory). Tier-1 inventory at 12 unchanged confirmed; Tier-2 is a new orthogonal axis (asset filenames) not in the lockstep.
---
## Iter-1 Cosmetic-Advisory Resolution Verification
### advisory 1 (Task 3 reference in commit message) — ADDRESSED
Plan Step 8 (line 1168): "Single atomic commit for Task 2 with all Task 2 artifacts + SUMMARY + STATE/ROADMAP markers: `feat(04-08): A33 SW state persistence harness assertion — methodology reframe (34/34 GREEN; ROADMAP SC #1 CLOSED)`. Task 1 was committed separately at the close of Task 1 per its own done line: `feat(04-08): video-file MediaStream + sync-install/lazy-first-frame + explicit WAR — methodology reframe per debug session-2 + iter-2 BLOCKER fixes`." — correctly references Task 1 + Task 2 only (no Task 3).
**Verdict:** RESOLVED.
### advisory 2 (segments invariant grep) — ADDRESSED
Plan Task 1 Step 7 + verify (line 968): `grep -cE 'let segments: Blob\[\] = \[\];' src/offscreen/recorder.ts` returns 1.
**Verdict:** RESOLVED.
### advisory 3 (dual-fixture-location note) — ADDRESSED
Plan Task 1 Step 1 (line 601): "**advisory 3 note**: `tests/fixtures/last_30sec.webm` remains in place (Plan 01-07 regression fixture; not affected). The new path at `tests/uat/fixtures/synthetic-display-source.webm` is a SECOND copy under the UAT subtree..." + further rationale.
**Verdict:** RESOLVED.
---
## NEW Iter-2 Findings
### NEW WARNING (single, low-severity) — displaySurface sub-gate scope ambiguity
**Dimension:** task_completeness
**Plan claim** (lines 966-967, Task 1 Step 7):
The plan introduces an explicit `--check-display-surface-only` mode for the spike script as the WARNING 2 remediation. The mode does not exist yet in `tests/uat/spike-a33-sw-persistence.ts` (verified empirically — grep returns 0 hits on `--check-display-surface-only` or `DISPLAY-SURFACE-CHECK`). The plan delegates the implementation to "append to the existing tests/uat/spike-a33-sw-persistence.ts script a --check-display-surface-only mode that runs only Steps 1-2 (prime + 5s post-prime probe)" without specifying the exact entry-point change, the exact log emit format, or whether the spike script's existing `process.exit(code)` at line 336 is short-circuited in this mode.
Plan ALSO offers an explicit alternative (line 967): "Alternative if the executor finds the spike-script-augmentation overhead too high: skip the dedicated sub-gate and rely on the spike re-run in Task 2 Step 1 (the displaySurface failure would manifest as 'wrong-display-surface' Error in the offscreen console capture; the spike's gating condition would still fail). The dedicated sub-gate is the LOW-LATENCY path; the spike re-run is the HIGH-LATENCY path; both gate the same invariant."
**Why this is a WARNING, not a BLOCKER:**
- The alternative path is documented and well-reasoned — the spike re-run in Task 2 Step 1 DOES catch the same failure, just with 6+ min latency instead of <10s.
- Task 1's `<verify>` block does NOT currently include a `grep 'DISPLAY-SURFACE-CHECK: PASSED' /tmp/04-08-display-surface-check.log` gate (line 971 verify command does NOT include this grep), so the sub-gate is genuinely optional — Task 1 verification passes without it.
- The plan's STRIDE threat T-04-08-08 + the WARNING 2 remediation are not weakened by skipping the sub-gate, because the spike re-run is the canonical catch.
**Why it's still a WARNING:**
- The "create a new spike-script mode" path is under-specified relative to the rest of the plan. An executor who chooses the LOW-LATENCY path would be making 5-10 LOC of implementation decisions (where to insert the env-var check; what to log; whether to wire it through `main()` or a new `mainCheckDisplaySurface()`; what `process.exit` code to use) that the plan does not pre-specify.
- The plan would be tighter if the displaySurface sub-gate were either (a) fully spec'd (5-10 lines of the exact code to add to the spike script) OR (b) explicitly retired in favor of the high-latency alternative (with a note in the SUMMARY documenting the WARNING 2 closure path was the high-latency one).
**Remediation for iter-3 (NOT NEEDED — does not block execution):**
Either pre-spec the exact spike-script edit (concrete 5-10 LOC patch), OR explicitly retire the sub-gate option (single sentence: "executor takes the HIGH-LATENCY path via Task 2 Step 1 spike re-run; the dedicated sub-gate is dropped"). Both fixes are <5 lines of plan text.
**Severity:** WARNING (not BLOCKER). The alternative path is documented; SC #1 closure is not impacted.
### NEW cosmetic-advisory 1 — collectDistFiles symbol-name mismatch
**Plan line 939:** `const distFiles = collectDistFiles(); // existing helper from the Tier-1 test`
**Actual code:** the helper at `tests/background/no-test-hooks-in-prod-bundle.test.ts:152` is named `listAllFilesRecursive(DIST_DIR)`.
The plan's own note at line 950 says "verify the actual name and adapt" — so the executor's lookup is anticipated and acceptable, but the plan would be tighter if the correct symbol name (`listAllFilesRecursive`) were used directly in the snippet. Cosmetic; ~30 seconds of executor lookup time saved.
### NEW cosmetic-advisory 2 — WARNING 1 documentation could be slightly stronger
The plan's WARNING 1 resolution (lines 711-728) trades a true fallback for explicit error classification. The SUMMARY (per Step 6's expected content list at lines 1109) does not explicitly call out "WARNING 1 closure path: no fallback; explicit error classification chosen instead" — the executor writing the SUMMARY should be careful to acknowledge the trade-off. This is a meta-note about SUMMARY-writing practice, not a plan-edit need.
### NEW cosmetic-advisory 3 — vitest baseline numbers minor inconsistency
Plan line 1100: "vitest baseline preserved: `npm test 2>&1 | tail -5` — expect 184/184 GREEN (was 183 baseline + 1 new Tier-2 test from Plan 04-08)"
Plan line 1261: "vitest baseline 184 preserved (183 prior + 1 new Tier-2; or 181/184 with 3 documented pre-existing flakes; pass in isolation)"
Both correct, but the math 183 + 1 = 184 should be flagged: 1 new Tier-2 test adds 1 test to the inventory, but the prior baseline (per 04-04-SUMMARY) was 183. Verify: `npm test` baseline is currently 183/183 GREEN absent the new Tier-2 test; after Plan 04-08 it becomes 184/184. Math checks out; cosmetic.
### NEW cosmetic-advisory 4 — duration-N/A acknowledged but not load-bearing
Plan Task 1 Step 1 (line 600): "duration is `N/A` per ffprobe (live-capture WebM lacks a duration tag), BUT the 1.9 MB VP9 at 400 kbps yields ~38s of decoded timeline — sufficient for the 5-min spike via `videoEl.loop = true`."
Empirically verified: `ffprobe ... tests/fixtures/last_30sec.webm` reports `duration=N/A`, codec=vp9, 1142x1044. The plan's claim about ~38s decoded timeline is reasonable (1888636 bytes / 400kbit/s ≈ ~38s). The `videoEl.loop = true` attr handles indefinite playback. This is a minor reasoning chain that's worth pinning explicitly in the SUMMARY's "Bundled WebM fixture decision" section so future maintainers understand why the N/A duration is acceptable. Cosmetic.
---
## Dimensional Pass/Fail Summary
| Dimension | Status | Notes |
|-----------|--------|-------|
| 1. Requirement Coverage | PASS | Phase requirements honor D-P4-01 (Full Phase 4 — ROADMAP SC #1); tasks address SC #1 directly. |
| 2. Task Completeness | PASS | Both tasks have all 4 elements; iter-2 added codified grep gates in both verify blocks; only the displaySurface sub-gate has the scope ambiguity noted above. |
| 3. Dependency Correctness | PASS | depends_on [01,02,03,04,05,06] is acyclic + correct (Wave 5.5 after Wave 5). |
| 4. Key Links Planned | PASS | iter-2 codifies the WAR entry + the sync-monkey-patch chain. key_links section is explicit about the chrome-extension://-scheme route + the lazy-first-frame closure. |
| 5. Scope Sanity | PASS | 2 tasks; 10 files_modified (was 7 in iter-1; iter-2 added globals.d.ts, manifest.json, tests/background/no-test-hooks-in-prod-bundle.test.ts). Within the 2-3 task / 5-8 file target's flexibility budget; the additions are small and well-scoped. |
| 6. Verification Derivation | PASS | must_haves.truths are user-observable (`videoSize > 100_000`, `34/34 GREEN`, `ROADMAP SC #1 CLOSED`); iter-2 added 4 new truths (lines 61-62, 65, 73, 76 — sync-install/lazy-closure, WAR entry, Tier-2 gate, patchDisplaySurface compatibility). |
| 7. Context Compliance | PASS | Honors D-P4-01 (full scope); does not contradict locked decisions; deferred IDB persistence work correctly excluded per debug session-2 verdict. |
| 7b. Scope Reduction Detection | PASS | No "v1"/"static for now"/"future enhancement" language; plan delivers SC #1 fully. iter-2 actually EXPANDS scope (adds Tier-2 gate, WAR entry, displaySurface sub-gate, 4 new must_haves truths) — opposite of reduction. |
| 7c. Architectural Tier Compliance | PASS | Test-only hook (test-hooks tier) + bundled test asset (fixtures tier) + Tier-2 gate (test build verification tier) are all correctly tier-located. Production code untouched. |
| 8. Nyquist Compliance | PASS | Both tasks have `<automated>` verify commands; Task 1 build-test verify + 14 separate grep gates + 1 vitest sub-test; Task 2 UAT skip-mode verify + 5 separate grep gates including the WARNING 3 probe values. No watch-mode; sampling adequate. |
| 9. Cross-Plan Data Contracts | PASS | No competing transforms on shared data; offscreen RAM buffer pipeline unchanged. |
| 10. CLAUDE.md Compliance | N/A | No `./CLAUDE.md` exists in working directory. Skip per protocol. |
| 11. Research Resolution | PASS | 04-RESEARCH.md Q2 was resolved + superseded by debug session-2; plan explicitly references the supersession + amends Plan 04-04 SUMMARY. |
| 12. Pattern Compliance | PASS | iter-2 explicitly clarifies that Plan 01-10 mokosh-mark.svg is the SVG-side analog (different code path due to inline limit) + that the explicit WAR entry is the iter-2 BLOCKER 1 remediation. The pattern analog is appropriately scoped. |
---
## Recommendation for Orchestrator
**Verdict: PASSED.**
Plan 04-08 iter-2 is execution-ready. Both iter-1 BLOCKERs have concrete, codified, grep-gated remediations. All five WARNINGs are addressed. All three cosmetic-advisories are addressed. One new WARNING and four new cosmetic-advisories were surfaced during iter-2 review, but none rise to the level of blocking execution:
- **NEW WARNING (displaySurface sub-gate scope):** the optional `--check-display-surface-only` mode is under-spec'd, but the plan documents an explicit alternative (high-latency catch via spike re-run). Executor can take either path; SC #1 closure is not impacted.
- **NEW cosmetic-advisories (4):** symbol-name mismatch, SUMMARY-write practice note, vitest math note, duration-N/A rationale. All minor.
Iter-2 findings (1 WARNING + 4 cosmetic-advisories) are below the PASSED threshold of "≤3 WARNINGs of low severity". The single WARNING is genuinely low-severity (alternative documented; not a blocker).
**Proceed to execute Plan 04-08.** Wave 5.5 starts with Task 1 (bundle + WAR entry + sync-install/lazy-closure rewrite + Tier-2 gate); Task 2 follows (spike re-run + A33 land + ROADMAP flip).
**Architectural alignment confirmed (carried forward from iter-1 + reinforced in iter-2):** the plan's core thesis (HTMLVideoElement.captureStream bypasses canvas-throttling per debug session-2) is correct. The iter-2 revision tightens both the bundling-strategy decision AND the sync-monkey-patch contract with explicit codified gates. Execution-ready.
---
*Plan-checker iter-2 completed 2026-05-22 by gsd-plan-checker.*
*Output committed atomically; orchestrator decides routing (execute Plan 04-08 OR optional iter-3 to address the displaySurface sub-gate scope WARNING).*