.planning/phases/04-harden-clean-up-optional/04-08-CHECKER-iter-3.md

@@ -0,0 +1,214 @@
+---
+phase: 04-harden-clean-up-optional
+plan: 08
+checker_iteration: 3
+checked_at: 2026-05-22T17:00:00Z
+plan_commit: 17e55dd
+parent_iter2_commit: 9c334b7
+parent_iter1_commit: 051813e
+verdict: PASSED-WITH-RESIDUAL
+severity_summary:
+  blocker: 0
+  warning: 0
+  cosmetic-advisory: 2
+goal_backward_check: "Plan 04-08 iter-3 polish revision delivers what was promised: the 1 iter-2 WARNING (displaySurface sub-gate scope ambiguity) is resolved by dropping the `--check-display-surface-only` mode entirely and locking in the HIGH-LATENCY catch path via the spike re-run's assertA2 fast-fail; the 4 iter-2 cosmetic-advisories (symbol-name mismatch, SUMMARY-write practice, vitest math, duration-N/A rationale) are addressed in the plan body with concrete edits. Two NEW low-severity cosmetic-advisories surfaced during iter-3 review (recorder.ts:294 mis-citation should be recorder.ts:319; duration=N/A rationale was preserved in-body rather than literally moved). Neither rises to BLOCKER or WARNING. The 73-line diff (+51/-22) is consistent with a pure polish pass — no thesis edits, no scope changes, BLOCKER fixes from iter-2 preserved verbatim. gsd-sdk verify.plan-structure returns valid=true; both tasks have files+action+verify+done; 14 frontmatter fields present including revision_history with all three iters logged."
+recommendation: "ORCHESTRATOR'S CALL between PASSED-WITH-RESIDUAL execution OR optional iter-4. Both residual advisories are documentation-cosmetic (line-number citation + framing-of-iter-3-polish-claim); neither blocks execution. Recommended: PROCEED to execute Plan 04-08. The iter-4 polish budget is better spent on Phase 5 work."
+---
+
+# Plan 04-08 Pre-Execution Validation — Iter-3
+
+**Plan under review:** `.planning/phases/04-harden-clean-up-optional/04-08-PLAN.md` @ commit `17e55dd`
+
+**Parent iteration:** iter-2 at commit `1f2eb2e` (verdict: PASSED; 1 WARNING + 4 cosmetic-advisories).
+
+**Iter-3 polish diff:** +51 / -22 lines on PLAN.md only (single-file change; pure polish pass — no thesis edits, no scope changes, no task additions).
+
+**Authority artifacts re-read for iter-3:**
+
+- `.planning/phases/04-harden-clean-up-optional/04-08-PLAN.md` @ 17e55dd (1336 lines)
+- `.planning/phases/04-harden-clean-up-optional/04-08-CHECKER-iter-2.md` @ 9c334b7
+- `.planning/phases/04-harden-clean-up-optional/04-08-CHECKER-iter-1.md` @ 051813e
+- `tests/background/no-test-hooks-in-prod-bundle.test.ts` (full 297 lines — verify symbol references)
+- `tests/uat/spike-a33-sw-persistence.ts` (full 337 lines — verify no dangling `--check-display-surface-only` references)
+- `src/offscreen/recorder.ts:280-322` (verify the actual location of the `wrong-display-surface` throw cited at recorder.ts:294)
+- gsd-sdk verify.plan-structure on the iter-3 plan (returns valid=true; both tasks complete; 14 frontmatter fields)
+
+---
+
+## Iter-2 WARNING Resolution Verification (1 WARNING)
+
+### iter-2 NEW WARNING — displaySurface sub-gate scope ambiguity — ADDRESSED
+
+**Iter-2 finding (from checker iter-2):** the optional `--check-display-surface-only` mode for the spike script was under-specified — the plan delegated 5-10 LOC of implementation decisions (where to insert the env-var check, what to log, how `process.exit` interacts) to the executor without pre-spec.
+
+**Iter-3 polish resolution (verified):**
+
+- **Strategy chosen:** retire the sub-gate option entirely; lock in the HIGH-LATENCY catch path via the spike re-run's assertA2 fast-fail.
+- **Concrete edits found:**
+  - Line 64 (revision_history iter-2 polish entry): "the dedicated `--check-display-surface-only` spike-script mode is dropped (it was under-specified and would have required 5-10 LOC of executor improvisation for no meaningful latency win)."
+  - Line 994 (Task 1 Step 7 verify body): the entire sub-step now reads "The displaySurface contract is gated by the spike re-run in Task 2 Step 1 — NOT by a dedicated low-latency sub-gate." with detailed rationale that the displaySurface failure surfaces in <30s of the spike (assertA2 prime catches the regression inside the production handler, not after 5-min idle).
+  - Line 1035 (Task 2 Step 0 GATING CONDITION): "displaySurface compatibility is gated by Step 1 below... No separate Task 1 sub-gate exists — the dedicated `--check-display-surface-only` mode was dropped in iter-3 polish per WARNING 1 closure".
+  - Line 1050 (Task 2 Step 2): "No spike-script edit is needed. The dedicated `--check-display-surface-only` mode was dropped in iter-3 polish..."
+  - Line 1138 (Step 6 SUMMARY content list): "WARNING 2 closure path (displaySurface compat) — document explicitly: HIGH-LATENCY catch via spike re-run's assertA2 fast-fail (NOT a dedicated sub-gate); cite the wall-clock-to-failure (<30s for displaySurface check inside assertA2 prime); the dedicated `--check-display-surface-only` mode was dropped in iter-3 polish per WARNING 1 closure".
+- **Empirical cross-check with spike script** (`tests/uat/spike-a33-sw-persistence.ts`): `grep -nE "(check-display|DISPLAY-SURFACE-CHECK)" tests/uat/spike-a33-sw-persistence.ts` returns 0 hits. The spike script has zero env hook or flag for the dropped mode — clean drop, no dangling references.
+- **Rationale-quality check on the new HIGH-LATENCY path:** the plan's claim that displaySurface failure surfaces in <30s (NOT after the 5-min idle) is correct — `src/offscreen/recorder.ts:313-321` enforces `if (observed !== 'monitor')` immediately after the `await navigator.mediaDevices.getDisplayMedia(...)` resolves inside `startRecording`, which is invoked during assertA2 prime (Step 1 of the spike), before the 5-min idle Step 2. The wall-clock-to-failure framing is accurate.
+
+**Verdict:** RESOLVED. Clean drop; the HIGH-LATENCY catch path is fully load-bearing; the spike re-run is the canonical catch (consistent with the iter-2 checker's own remediation guidance).
+
+---
+
+## Iter-2 cosmetic-advisory Resolution Verification (4 advisories)
+
+### iter-2 cosmetic-advisory 1 — symbol-name mismatch (`collectDistFiles` -> `listAllFilesRecursive`) — ADDRESSED
+
+**Iter-2 finding:** plan line 939 used `collectDistFiles()` as the existing dist/-walker helper, but the actual helper at `tests/background/no-test-hooks-in-prod-bundle.test.ts:152` is `listAllFilesRecursive(DIST_DIR)`.
+
+**Iter-3 polish resolution (verified):**
+
+- Plan line 961: `const distFiles = listAllFilesRecursive(DIST_DIR);  // existing helper at line ~152 of this file` — direct symbol replacement.
+- Plan also uses the existing `countOccurrencesInFile(filePath, 'synthetic-display-source')` helper at line 964 — matches the Tier-1 pattern from the live file at line 222.
+- Plan line 971-981 ("Note") was updated to cite both `listAllFilesRecursive(DIST_DIR)` at line ~152 AND `countOccurrencesInFile` at line ~185 AND the `DIST_DIR` constant at line ~133, sparing the executor the lookup time.
+- **Empirical cross-check with the live test file:**
+  - `tests/background/no-test-hooks-in-prod-bundle.test.ts:133`: `const DIST_DIR = resolvePath(process.cwd(), 'dist');` ✓
+  - `tests/background/no-test-hooks-in-prod-bundle.test.ts:152`: `function listAllFilesRecursive(root: string): ReadonlyArray<string>` ✓
+  - `tests/background/no-test-hooks-in-prod-bundle.test.ts:185`: `function countOccurrencesInFile(filePath: string, needle: string): number` ✓
+- All three symbols at the cited lines match the plan's iter-3 references verbatim.
+
+**Verdict:** RESOLVED.
+
+### iter-2 cosmetic-advisory 2 — WARNING 1 SUMMARY-write practice note — ADDRESSED
+
+**Iter-2 finding:** the plan's WARNING 1 resolution (autoplay reject path produces an explicit error class) traded a true fallback for explicit error classification, but the SUMMARY (per Step 6's expected content list) did not explicitly call out this trade-off — risk that the executor writing the SUMMARY would gloss over it.
+
+**Iter-3 polish resolution (verified):**
+
+- Plan lines 730-737 (inline comment block inside the `installFakeDisplayMedia` rewrite snippet): "WARNING 1 SUMMARY-write practice (iter-3 polish): the executor writing 04-08-SUMMARY.md MUST document the chosen failure path explicitly — 'no Plan B fallback; explicit error-class identifier on autoplay/codec reject is the chosen WARNING 1 closure path; downstream observability via the offscreen console capture is the diagnostic surface.' The error class is observable in the spike re-run's offscreen-console log capture, so the SUMMARY's evidence section should cite this path (cf. cosmetic-advisory 2 from checker iter-2)."
+- Plan line 1136 (Step 6 SUMMARY content list): "**WARNING 1 closure path** (autoplay reject fallback) — document explicitly: no Plan B fallback; explicit error-class identifier on autoplay/codec reject; the error class string is observable in the spike's offscreen-console capture; cite the wall-clock-to-failure surface (per iter-3 polish + checker iter-2 cosmetic-advisory 2)".
+- Both touchpoints are explicit: the executor writing SUMMARY now has both an in-code comment AND a dedicated SUMMARY content-list bullet directing them to document the WARNING 1 trade-off.
+
+**Verdict:** RESOLVED.
+
+### iter-2 cosmetic-advisory 3 — vitest math (183 -> 184) consistency — ADDRESSED
+
+**Iter-2 finding:** the planner's math was correct (183 + 1 = 184) but the +1 wasn't always anchored to the specific Tier-2 test block that adds it.
+
+**Iter-3 polish resolution (verified):** the +1 origin is now anchored consistently in 5 locations:
+
+| Line | Location | Content |
+|------|----------|---------|
+| 81 | must_haves.truths #10 | "vitest goes 183/183 -> 184/184 GREEN (Plan 04-08 adds exactly one new vitest `test(...)` block — the Tier-2 filename-leak gate in tests/background/no-test-hooks-in-prod-bundle.test.ts; cf. WARNING 5 remediation + iter-3 polish per checker iter-2 cosmetic-advisory 3)" |
+| 1128 | Task 2 Step 5 | "vitest baseline flips 183 -> 184 ... prior baseline 183 + exactly 1 new `test(...)` block added in Task 1 Step 6 — the Tier-2 'synthetic-display-source filename does not leak' gate" |
+| 1222 | Task 2 acceptance_criteria | "vitest baseline flips 183 -> 184 (prior 183 GREEN + exactly 1 new `test(...)` block — the Tier-2 ... gate)" |
+| 1290 | top-level verification | "vitest baseline flips 183 -> 184 GREEN (prior 183 + exactly 1 new `test(...)` block — the Tier-2 filename-leak gate" |
+| 1315 | top-level success_criteria | "vitest baseline flips 183 -> 184 (prior 183 + exactly 1 new `test(...)` block — the Tier-2 filename-leak gate added in Task 1 Step 6)" |
+
+All 5 locations anchor the +1 to the new Tier-2 test block in tests/background/no-test-hooks-in-prod-bundle.test.ts. Math is consistent across the plan.
+
+**Verdict:** RESOLVED.
+
+### iter-2 cosmetic-advisory 4 — duration=N/A rationale in SUMMARY (not PLAN body) — PARTIALLY ADDRESSED
+
+**Iter-2 finding:** the planner's claim was that the duration=N/A reasoning chain (1.9 MB / ~400 kbps / ~38s decoded timeline) belongs in SUMMARY-time documentation, not in the PLAN body's Task 1 Step 1.
+
+**Iter-3 polish resolution (verified):**
+
+- Plan line 1134 (Step 6 SUMMARY content list): "Bundled WebM fixture decision (copy vs regenerate; size + codec evidence; dual-location note; **duration=N/A rationale** — 1.9 MB / ~400 kbps VP9 = ~38s decoded timeline + `videoEl.loop = true` for indefinite playback across the 5-min spike; cf. iter-3 polish per checker iter-2 cosmetic-advisory 4)" — bullet added to SUMMARY content list ✓
+- Plan line 611 (Task 1 Step 1 body): "The full reasoning chain (ffprobe duration=N/A is acceptable because the 1.9 MB / ~400 kbps VP9 yields ~38s of decoded timeline; the `videoEl.loop = true` attr handles indefinite playback) is documented in 04-08-SUMMARY.md's 'Bundled WebM fixture decision' section so future maintainers understand the rationale at SUMMARY-time" — the rationale is PRESERVED in-body with an added cross-reference to SUMMARY (NOT removed).
+
+**Why "partially":** the planner's iter-3 polish claim was "moved out of the PLAN body into the SUMMARY content list (Step 6)" — but the in-body rationale at line 611 is still present, just now with a forward-pointer to SUMMARY. This is a documentation-clarity quibble: the prose was NOT moved (a strict reading of the claim); the prose was preserved AND a SUMMARY-list reference was added. The end-state behavior is fine (rationale is captured in both places + the SUMMARY-time writer is directed where to expand it). This becomes a new low-severity advisory below.
+
+**Verdict:** RESOLVED at end-state; the planner's iter-3 claim language slightly overstated the change (claim was "moved" but actual change was "preserved + forward-referenced"). Minor.
+
+---
+
+## NEW Iter-3 Findings
+
+### NEW cosmetic-advisory 1 — `src/offscreen/recorder.ts:294` line-number mis-citation
+
+**Dimension:** verification_derivation
+
+**Plan claim** (lines 64 + 994 + 1035):
+
+The plan cites `src/offscreen/recorder.ts:294` as the location of the production gate that "throws 'wrong-display-surface' within ~30s of spike start if the patchDisplaySurface helper is broken" (line 64 + repeated at line 994 + paraphrased at line 1035).
+
+**Actual code location** (verified during this check):
+
+- `src/offscreen/recorder.ts:293-301` is a COMMENT block + `if (__MOKOSH_UAT__)` test-hook wiring block (the comment text at line 294 reads: "surface so the harness can read displaySurface (assertion 3) and").
+- The actual displaySurface check is at lines 312-321:
+  - Line 312: `const observed = videoTrack.getSettings().displaySurface;`
+  - Line 313: `if (observed !== 'monitor') {`
+  - Lines 314-321: tear-down + `throw new Error(\`wrong-display-surface: got "${observed}", expected "monitor"\`);`
+
+**Severity rationale:** the plan's line citation is off by ~25 lines, but the function/conditional/throw it's referring to is unambiguous (there is only one `wrong-display-surface` throw in recorder.ts). The executor reading the plan + opening recorder.ts will land on line 294 (a comment), scroll down ~25 lines to find the actual gate, and proceed. The plan's underlying claim about the wall-clock-to-failure (<30s; fires inside assertA2 prime) is correct.
+
+**Severity:** cosmetic-advisory. Does not block execution. Plan would be tighter if the citation said `src/offscreen/recorder.ts:313-321` (the `if (observed !== 'monitor') { ... throw }` block) instead of line 294.
+
+**Remediation if desired:** 2 grep-replaces in PLAN.md:
+- `src/offscreen/recorder.ts:294 throws` -> `src/offscreen/recorder.ts:319 throws`
+- `src/offscreen/recorder.ts:294 tears down` -> `src/offscreen/recorder.ts:313-321 (tears down at 313; throws at 319)`
+
+### NEW cosmetic-advisory 2 — duration=N/A rationale: "moved" claim vs "preserved+forward-ref'd" reality
+
+**Dimension:** revision_history accuracy
+
+**Plan claim** (line 69, revision_history iter-2 polish entry): "duration=N/A rationale **moved out of the PLAN body into the SUMMARY content list (Step 6)** — the PLAN keeps only the load-bearing >=1 MB size gate + loop-attr behavioral assertion + spike-re-run empirical catch; the 1.9 MB / ~400 kbps / ~38s decoded-timeline reasoning chain is documented at SUMMARY-time per checker iter-2 advisory 4."
+
+**Actual change** (verified during this check):
+
+- The reasoning chain IS present in the SUMMARY content list at line 1134 — addition is correct.
+- BUT the in-body rationale at line 611 (Task 1 Step 1) ALSO retains the reasoning chain verbatim: "The full reasoning chain (ffprobe duration=N/A is acceptable because the 1.9 MB / ~400 kbps VP9 yields ~38s of decoded timeline; the `videoEl.loop = true` attr handles indefinite playback) is documented in 04-08-SUMMARY.md's 'Bundled WebM fixture decision' section..."
+
+So the prose was preserved in the PLAN body AND added to the SUMMARY content list (with a forward-pointer from PLAN body to SUMMARY). The "moved" claim language slightly overstates the change.
+
+**Severity rationale:** the executor reading the iter-2 polish entry might expect the in-body reasoning to be absent; on encountering it at line 611 they'd briefly wonder if the polish landed. The end-state behavior (reasoning captured + SUMMARY-time writer directed where to expand it) is fine.
+
+**Severity:** cosmetic-advisory. Does not block execution.
+
+**Remediation if desired (NOT NEEDED):** either (a) literally move the rationale from PLAN body (delete the in-body block at line 611's last sentence) — net -2 lines; OR (b) rephrase the revision_history claim from "moved out of the PLAN body into the SUMMARY content list" to "added to the SUMMARY content list (Step 6) with a forward-pointer from the PLAN body". Both are <5 lines of edit.
+
+---
+
+## Dimensional Pass/Fail Summary
+
+| Dimension | Status | Notes |
+|-----------|--------|-------|
+| 1. Requirement Coverage | PASS | Phase requirements (D-P4-01; ROADMAP SC #1) unchanged from iter-2; tasks address SC #1 directly. |
+| 2. Task Completeness | PASS | gsd-sdk verify.plan-structure returns valid=true; both tasks have files+action+verify+done. |
+| 3. Dependency Correctness | PASS | depends_on [01,02,03,04,05,06] unchanged; acyclic; Wave 5.5. |
+| 4. Key Links Planned | PASS | key_links section unchanged from iter-2 (WAR entry + sync-monkey-patch + lazy-closure + driveA33 wiring + JSZip video-size check all explicit). |
+| 5. Scope Sanity | PASS | 2 tasks; 11 files_modified (unchanged from iter-2); 1336 lines (iter-2: 1307; +29 net). Pure polish; no scope expansion. |
+| 6. Verification Derivation | PASS | must_haves.truths user-observable; iter-3 polish anchored the vitest +1 origin in 5 locations; HIGH-LATENCY displaySurface catch path locked in. One advisory on line-number citation accuracy (cosmetic-advisory 1 above). |
+| 7. Context Compliance | PASS | Honors D-P4-01; deferred IDB persistence work correctly excluded; iter-3 polish does NOT introduce scope reduction or unilateral decisions. |
+| 7b. Scope Reduction Detection | PASS | iter-3 polish actually CONTRACTS the scope (drops `--check-display-surface-only` option) but only at the level of an OPTIONAL sub-gate that was explicitly noted in iter-2 as "do or skip". Net effect: cleaner plan; no decision is lost (HIGH-LATENCY catch is the canonical path). Not a violation of feedback-no-unilateral-scope-reduction — the planner is honoring the iter-2 checker's own remediation guidance. |
+| 7c. Architectural Tier Compliance | PASS | Test-only hook + bundled test asset + Tier-2 gate all correctly tier-located; iter-3 polish unchanged from iter-2. |
+| 8. Nyquist Compliance | PASS | Both tasks have `<automated>` verify commands; iter-3 polish did not change the verify commands' Nyquist coverage. |
+| 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 resolved + superseded by debug session-2; plan explicitly references the supersession. |
+| 12. Pattern Compliance | PASS | Plan 01-10 mokosh-mark.svg precedent + the explicit WAR entry decision unchanged from iter-2. |
+
+---
+
+## Recommendation for Orchestrator
+
+**Verdict: PASSED-WITH-RESIDUAL.**
+
+Plan 04-08 iter-3 polish revision is execution-ready. The 1 iter-2 WARNING is RESOLVED via clean drop of the `--check-display-surface-only` mode + lock-in of the HIGH-LATENCY catch path. The 4 iter-2 cosmetic-advisories are addressed with concrete plan edits (symbol-name fix; SUMMARY-write practice note + content-list bullet; vitest math anchored in 5 locations; duration=N/A SUMMARY content-list bullet added with cross-reference from PLAN body).
+
+Two NEW low-severity cosmetic-advisories surfaced during iter-3 review:
+
+1. **recorder.ts:294 mis-citation** — the plan cites line 294 (a comment block) for the displaySurface throw that actually lives at lines 313-321. Off by ~25 lines but unambiguous; executor reading the file will land on the right gate without confusion.
+2. **duration=N/A "moved" framing** — the revision_history entry says "moved out of the PLAN body into the SUMMARY content list" but the actual change was "preserved in PLAN body + added to SUMMARY content list with forward-pointer". End-state is fine; framing is slightly aspirational.
+
+Neither residual advisory rises to BLOCKER or WARNING. Both are documentation-cosmetic; neither blocks Plan 04-08 execution.
+
+Iter-3 polish findings (0 BLOCKER + 0 WARNING + 2 cosmetic-advisories) are well within the PASSED threshold and meet the PASSED-WITH-RESIDUAL bar (orchestrator's choice between immediate execution OR optional iter-4).
+
+**Recommended path:** PROCEED to execute Plan 04-08. The iter-4 polish budget is better spent on Phase 5 work; both residual advisories are line-number / framing quibbles that a competent executor will absorb without friction.
+
+**Architectural alignment confirmed (carried forward from iter-1 + iter-2):** HTMLVideoElement.captureStream bypasses canvas-throttling per debug session-2. The iter-3 polish does not weaken any BLOCKER fix from iter-2; the SYNC install + LAZY first-frame contract + explicit WAR entry + grep gates are all preserved verbatim.
+
+---
+
+*Plan-checker iter-3 completed 2026-05-22 by gsd-plan-checker.*
+*Output committed atomically; orchestrator decides routing (execute Plan 04-08 OR optional iter-4 to address the recorder.ts:294 line-number citation + duration=N/A framing).*