docs(04-06): re-plan-checker iter-2 — PASSED on f3baa3a (0 BLOCKER + 3 cosmetic-advisories)
Validation of the iter-2 re-plan against commit f3baa3a. Both iter-1
BLOCKERs are correctly resolved; 3 iter-1 advisories all fixed.
BLOCKER 1 (fictitious A17.8 delegation) → RESOLVED via genuine new
host-side A35 driver. Every test-infrastructure claim was spot-checked
against live code this session:
- vite.test.config.ts:95 — welcome.html builds to dist-test/src/welcome/
- welcome.ts:194-198 — populateMark runs at DOMContentLoaded (verified)
- A17 already uses chrome.runtime.getURL('src/welcome/welcome.html') —
same canonical URL A35 will use via page.goto
- driveA33 signature (page, browser, extensionId, downloadsDir) at
line 2622-2627 — driveA35 is a sound subset
- harness.test.ts:580 total = drivers.length + 1 (auto-increments)
- Browser + Page imported at harness-page-driver.ts:43
- launch.ts:473-542 opens only victimPage + harnessPage
- welcome-hero__mark zero hits in all current harness files
- welcome.css:72 sets color: var(--mks-fg-inverse) on .welcome-hero__mark
(cascade target); .welcome-hero__mark-img is bare selector (matches svg)
BLOCKER 2 (phantom failing test) → RESOLVED via behavior-based gate that
distinguishes flake from regression by isolation re-run (no test filename
hard-coded). Verified live this session:
- Full vitest run: 184/184 GREEN (flake did NOT fire this run)
- strict-meta-json in isolation: 8/8 GREEN
- webm-remux in isolation: 5/5 GREEN
- Confirms iter-1's diagnosis: the '1 fail' is the 04-CONTEXT #9/#10
parallel-vitest/ffprobe family, not a named test.
DEFECT 2 line classification (22/47/82/135/205 flip; 40/89/109/110 leave)
preserved unchanged. welcome.css drop preserved. Thesis preserved
(currentColor Option A + cursor verification-only + operator empirical
Task 4). FORBIDDEN_HOOK_STRINGS stays at 12. Atomic-commit structure +
frontmatter + gsd-sdk verify.plan-structure all GREEN.
3 NEW cosmetic-advisories (all non-blocking):
- ADVISORY-2A: stale banner string at harness.test.ts:283 (does not
include A33/A34 today; planner's 'append A35' instruction has a
slightly stale premise; banner is cosmetic, no gate depends on it)
- ADVISORY-2B: Task 3 rationale prose says SKIP_PROD_REBUILD gates
dist-test rebuild; actually it gates dist/ (the A0 grep gate); the
command behavior is correct, only the prose is slightly off
- ADVISORY-2C: threat model could note A35 is appended LAST in drivers
array (which makes the pollution-of-future-drivers concern moot;
verified independently safe)
VERDICT: PASSED. Proceed to /gsd:execute-phase 04-06.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,322 @@
|
||||
# 04-06 RE-PLAN VALIDATION — iter-2
|
||||
|
||||
**Plan:** `.planning/phases/04-harden-clean-up-optional/04-06-PLAN.md`
|
||||
**Commit reviewed:** `f3baa3a` (iter-2 re-plan after iter-1 BLOCKERs)
|
||||
**Reviewer:** gsd-plan-checker (re-plan validation loop, iter-2)
|
||||
**Prior verdict (iter-1, deb68df):** ITERATE-NEEDED — 2 BLOCKERs + 1 WARNING + 3 cosmetic-advisories
|
||||
**Date:** 2026-05-22
|
||||
|
||||
---
|
||||
|
||||
## VERDICT: PASSED
|
||||
|
||||
**0 BLOCKER · 0 WARNING · 3 cosmetic-advisories**
|
||||
|
||||
The iter-2 re-plan correctly resolves both iter-1 BLOCKERs and all 3 iter-1 advisories. Every test-infrastructure claim the planner asserted this session was spot-checked against the live codebase and confirmed accurate. The new A35 host-side harness assertion is soundly designed against the verified driveA32/A33/A34 patterns; the vitest gate logic is behavior-based and tolerates the 04-CONTEXT #9/#10 flake family without hard-coding any test filename. Plan is ready to execute.
|
||||
|
||||
---
|
||||
|
||||
## iter-1 BLOCKER 1 — fictitious A17.8 live-DOM delegation
|
||||
|
||||
### iter-2 fix: NEW host-side A35 assertion (driveA35)
|
||||
|
||||
**Status: RESOLVED.**
|
||||
|
||||
The iter-1 BLOCKER was real: the iter-1 plan claimed live-DOM injection + currentColor cascade was "delegated to A17.8 in real Chrome," but A17.8 is 100% string-grep on `jsText` against a detached DOMParser parse of the welcome HTML — no live tab, no `populateMark()` invocation, no `.welcome-hero__mark svg` query.
|
||||
|
||||
iter-2's fix is the new host-side **A35** driver. Spot-check verification of every load-bearing claim against the live code:
|
||||
|
||||
| Claim | Verified | Evidence |
|
||||
|-------|----------|----------|
|
||||
| `welcome.html` builds to `dist-test/src/welcome/welcome.html` | YES | `vite.test.config.ts:95`: `welcome: 'src/welcome/welcome.html'` (mode test, outDir `dist-test`) |
|
||||
| `welcome.html` builds to `dist/src/welcome/welcome.html` (prod) | YES | `vite.config.ts:166`: same input under prod outDir `dist/` |
|
||||
| `chrome-extension://<id>/src/welcome/welcome.html` is the canonical URL | YES | `extension-page-harness.ts:2099` already uses `chrome.runtime.getURL('src/welcome/welcome.html')` to fetch it (A17) — exactly the URL `page.goto()` will use |
|
||||
| `populateMark()` runs at DOMContentLoaded | YES | `welcome.ts:194-198`: `if (document.readyState === 'loading') { document.addEventListener('DOMContentLoaded', init); } else { init(); }` — `init()` calls `populateMark()` first (line 188). `page.goto(url, { waitUntil: 'domcontentloaded' })` resolves AFTER DOMContentLoaded fires, so populateMark has run synchronously by then. The optional `waitForSelector('.welcome-hero__mark svg')` adds a defensive race-free guard. |
|
||||
| `driveA35(page: Page, browser: Browser, extensionId: string)` signature matches the host-side driver pattern | YES | `driveA32(page: Page)` (line 2464), `driveA33(page, browser, extensionId, downloadsDir)` (line 2622-2627), `driveA34(page, downloadsDir)` (line 2817) — all host-side, all build `CheckRecord[]` directly without `page.evaluate(window.__mokoshHarness)`. driveA35's signature is a sound subset of driveA33's (no downloadsDir needed). |
|
||||
| `Browser` + `Page` types already imported in harness-page-driver.ts | YES | line 43: `import type { Browser, Page } from 'puppeteer';` |
|
||||
| `launchHarnessBrowser` returns `{ browser, extensionId, harnessPage, victimPage, downloadsDir, swConsole, offConsole }` (handles.browser + handles.extensionId reachable) | YES | `launch.ts:534-542` |
|
||||
| Existing harness opens only victimPage + harnessPage | YES | `launch.ts:473-474` (victimPage.goto file://), `launch.ts:478` (harnessPage = await browser.newPage()), `launch.ts:524` (harnessPage.goto chrome-extension://.../extension-page-harness.html). NO other newPage() in launch.ts. |
|
||||
| `welcome-hero__mark` selector returns zero hits across all current harness files | YES | `grep 'welcome-hero__mark' tests/uat/...` returns ZERO matches across extension-page-harness.ts, harness-page-driver.ts, harness.test.ts. The pre-iter-2 claim that "live-DOM is delegated to A17.8" was indeed false. |
|
||||
| `.welcome-hero__mark` wrapper sets `color: var(--mks-fg-inverse)` (cascade target) | YES | `welcome.css:72` — line 64-73 block sets `color: var(--mks-fg-inverse)` on `.welcome-hero__mark`. |
|
||||
| `.welcome-hero__mark-img` is a bare class selector (matches `<svg>` and `<img>` identically) | YES | `welcome.css:91` — bare `.welcome-hero__mark-img { width: 60%; height: 60%; display: block; }`, no `img.` qualifier. |
|
||||
| `.welcome-hero__mark` wrapper exists in `welcome.html` with `data-mokosh-slot="mark"` | YES | `welcome.html:49`: `<div class="welcome-hero__mark" role="presentation" data-mokosh-slot="mark">` |
|
||||
| `driveA33Wrapped` pattern is the model for `driveA35Wrapped` | YES | `harness.test.ts:367-368`: `const driveA33Wrapped: (page) => Promise<AssertionRecord> = (page) => driveA33(page, handles.browser, handles.extensionId, handles.downloadsDir);` — driveA35Wrapped follows the same closure-capture pattern, capturing only `handles.browser + handles.extensionId`. |
|
||||
| harness total = `drivers.length + 1` (auto-increments) | YES | `harness.test.ts:580`: `const total = drivers.length + 1;` |
|
||||
| Drivers array sequential (no parallel runs, no race risk between A35 and other drivers) | YES | `harness.test.ts:542` orchestrator loop: `for (const { name, drive } of drivers) { ... }` — strictly serial. A35 appended LAST after A34 — no driver runs after it; finally-block welcomePage.close() ensures the extra tab does not leak. No state pollution to subsequent drivers (there are none) or storage (A35 does not mutate chrome.storage; opening welcome.html does NOT trigger openWelcomeIfFirstInstall — that runs only on chrome.runtime.onInstalled). |
|
||||
| AssertionRecord/CheckRecord shape | YES | `assertions.ts:25-44` — exactly the shape the plan specifies in `<interfaces>` |
|
||||
|
||||
**A35 design soundness — VERIFIED.** The driver is a genuine new harness capability (a host-side `browser.newPage()` + `page.goto(welcome.html)` + live `getComputedStyle().stroke` cascade read), not a fictitious delegation. The 4 CheckRecords (A35.1 svg present, A35.2 stroke=currentColor attr, A35.3 getComputedStyle().stroke resolved non-default, A35.4 no `<img>` in slot) collectively prove the inline-SVG injection + currentColor cascade actually works in real Chrome. This is the real automated proof the iter-1 plan was missing.
|
||||
|
||||
**A17.8 honest narrowing — VERIFIED.** Task 3 Edit 1 honestly reframes A17.8 as a SOURCE-BUNDLING check only (the `?raw` import inlines the SVG source string into the welcome JS chunk; A17.8 string-greps for `stroke="currentColor"` + `viewBox="0 0 32 32"` in jsText). The plan's prose explicitly disclaims any live-DOM coverage for A17.8 and points to A35 as the live-DOM proof. No hidden delegation; no overclaim.
|
||||
|
||||
iter-1 BLOCKER 1 is fully resolved by genuine new harness work, not a hand-wave.
|
||||
|
||||
---
|
||||
|
||||
## iter-1 BLOCKER 2 — wrong vitest baseline (phantom failing test)
|
||||
|
||||
### iter-2 fix: behavior-based gate, no test filename hard-coded
|
||||
|
||||
**Status: RESOLVED.**
|
||||
|
||||
The iter-1 BLOCKER was real: the iter-1 plan claimed `tests/build/strict-meta-json-validation.test.ts` was a "pre-existing red on a clean tree" and hard-coded a Task 2/Task 4 gate "expected failure == exactly {strict-meta-json-validation.test.ts}". Verified false: strict-meta-json was 8/8 GREEN in isolation, the actual full-suite RED that session was `webm-remux.test.ts > ffprobe -count_frames` (a 5s timeout in a parallel-worker race), and both tests pass deterministically in isolation. The "1 failure" was the 04-CONTEXT #9/#10 parallel-vitest/ffprobe flake family, not a named test.
|
||||
|
||||
iter-2 verification this session (clean tree at `f3baa3a`, single full-suite run):
|
||||
|
||||
| Run | Result | Diagnosis |
|
||||
|-----|--------|-----------|
|
||||
| `npx vitest run` (full suite, parallel) | **184 passed / 0 failed (184 total) — 36 test files all GREEN** | This session the flake family did NOT fire; the suite is 184/184 GREEN. |
|
||||
| `npx vitest run tests/build/strict-meta-json-validation.test.ts` (isolation) | 8/8 GREEN | Confirms iter-1's claim: strict-meta-json is NOT the phantom-named failing test. |
|
||||
| `npx vitest run tests/background/webm-remux.test.ts` (isolation) | 5/5 GREEN | Confirms iter-1's claim: webm-remux passes deterministically in isolation. |
|
||||
|
||||
Both isolation runs GREEN, and the full-suite run was GREEN this time — exactly consistent with iter-1's diagnosis that "the 1 failed" is a non-deterministic parallel-worker flake (it sometimes fires, sometimes doesn't; 04-CONTEXT #9 says "~1/5 full-suite runs").
|
||||
|
||||
**iter-2's Task 2 VITEST GATE LOGIC** (PLAN.md lines 325-329):
|
||||
|
||||
> * If the suite is 188/188 GREEN -> PASS.
|
||||
> * If the suite is 187 passed / 1 failed AND the single failing test PASSES when re-run in isolation (`npm test -- <that-failing-file> --run`) -> tolerated 04-CONTEXT #9/#10 flake; PASS. Record the test name + isolation re-run result in SUMMARY.
|
||||
> * If the suite has 2+ failures, OR the single failing test STILL FAILS in isolation (reproducible, not a flake) -> FAIL the gate.
|
||||
|
||||
**Validation of the gate's concreteness for an executor:**
|
||||
|
||||
- The executor parses vitest output to extract the failing test's file path (standard vitest reporter dumps `FAIL tests/...test.ts > ...` — trivially greppable).
|
||||
- The executor re-runs `npm test -- <that-file> --run` (standard vitest filtering).
|
||||
- The executor compares: isolation pass → tolerate; isolation fail → regression.
|
||||
- No test filename is hard-coded; the gate distinguishes flake from regression by **re-run behavior**, not by name.
|
||||
|
||||
This is concretely actionable for an executor agent (they routinely parse test output and re-run with filtering). The baseline (184/184) and target (188/188 = 184 + 4 new tests) are correct. The mis-diagnosed deferred-items.md entry is corrected in Task 3 Edit 5.
|
||||
|
||||
iter-1 BLOCKER 2 is fully resolved.
|
||||
|
||||
---
|
||||
|
||||
## iter-1 advisories — all 3 fixed
|
||||
|
||||
| Advisory | iter-1 finding | iter-2 fix | Status |
|
||||
|----------|----------------|------------|--------|
|
||||
| ADVISORY-1 — SKIP_PROD_REBUILD mismatch | iter-1 action said `=0`, verify said `=1` | iter-2 reconciles both to `=0` (verify command at line 415 + action prose at line 411) | RESOLVED |
|
||||
| ADVISORY-2 — `requirements: []` is empty | Phase 4 has no new REQ-*, but field is bare; charter linkage suggested via tag | iter-2 keeps `requirements: []` as intentional + `tags: [..., charter-d-p4-03, ...]` provides the traceability | RESOLVED (kept intentionally per planner; charter-d-p4-03 tag is the linkage) |
|
||||
| ADVISORY-3 — jsdom grep would trip on prose | iter-1 acceptance criterion `grep -c 'jsdom' returns 0` would fail because the header comment mentions jsdom | iter-2 reworded acceptance criterion at line 299 to `grep -E "^import .*jsdom\|@vitest-environment jsdom" tests/welcome/inline-svg.test.ts returns 0` — scoped to imports + environment directive ONLY, not prose | RESOLVED |
|
||||
|
||||
---
|
||||
|
||||
## DEFECT 2 line classification — PRESERVED INTACT
|
||||
|
||||
iter-2 keeps the iter-1 verified-correct flip/leave classification on `01-07-SUMMARY.md`:
|
||||
|
||||
Re-verified live this session (`grep -nE 'Phase 5|deferred to Phase 5' .planning/phases/01-stabilize-video-pipeline/01-07-SUMMARY.md`):
|
||||
|
||||
| Line | Content category | iter-2 says | Confirmed |
|
||||
|------|------------------|-------------|-----------|
|
||||
| 22 | "Phase 5 ... new entry: getDisplayMedia cursor visibility constraint" | FLIP | forward-looking deferral - correct |
|
||||
| 47 | "Cursor visibility ... deferred to Phase 5, not back-patched" | FLIP | stale deferral framing - correct |
|
||||
| 82 | "Cursor-visibility refinement deferred to Phase 5" | FLIP | stale deferral framing - correct |
|
||||
| 135 | "Cursor visibility refinement deferred to Phase 5" | FLIP | stale deferral framing - correct |
|
||||
| 205 | "The cursor visibility refinement (Phase 5) will, when activated, add..." | FLIP | false-future tense - correct |
|
||||
| 40 | key-files: "ROADMAP.md ... cursor-visibility refinement appended to Phase 5" | LEAVE | historical commit content - correct |
|
||||
| 89 | Task Commits: "7df72aa ... Phase 5 cursor-visibility appended" | LEAVE | historical commit description - correct |
|
||||
| 109 | Files Created/Modified: "ROADMAP.md ... Phase 5 P1/P2 list appended" | LEAVE | historical commit record - correct |
|
||||
| 110 | Files Created/Modified: "STATE.md ... [Phase 01-07-deferred-to-5] decision-log tag" | LEAVE | verified at line 110 — STATE.md historical record - correct |
|
||||
|
||||
Classification preserved unchanged from iter-1. CORRECT.
|
||||
|
||||
---
|
||||
|
||||
## welcome.css drop — PRESERVED INTACT
|
||||
|
||||
iter-2 keeps welcome.css OUT of `files_modified`. Verified rationale:
|
||||
|
||||
- `welcome.css:91` `.welcome-hero__mark-img { width: 60%; height: 60%; display: block; }` is a **bare class selector** (no `img.` qualifier) — matches `<svg class="welcome-hero__mark-img">` identically to `<img class="...">`.
|
||||
- `welcome.css:72` sets `color: var(--mks-fg-inverse)` on `.welcome-hero__mark` (inherited).
|
||||
- Inline `<svg>` (parsed via DOMParser + adopted into HTML doc via `replaceChildren`) inherits the wrapper's `color` automatically (CSS Color L3 — `color` is an inherited property).
|
||||
- `stroke="currentColor"` on the SVG root resolves to that inherited color at computed-style time.
|
||||
- No CSS edit required. PRESERVED. CORRECT.
|
||||
|
||||
---
|
||||
|
||||
## Thesis preservation — CONFIRMED INTACT
|
||||
|
||||
| Thesis pillar | Preserved? | Evidence |
|
||||
|---------------|------------|----------|
|
||||
| Dark-logo `currentColor` Option A | YES | `<objective>` lines 119-122; `must_haves.truths` 1-3; key_links 1-2; Task 2 Edits 1+2+3 land the source changes |
|
||||
| Cursor verification-only (no recorder.ts edit) | YES | `<objective>` line 126; `must_haves.truths` 7; Task 1 cursor-visibility.test.ts is GREEN-on-arrival defensive pin; recorder.ts NOT in `files_modified` |
|
||||
| Operator-empirical Task 4 (UI-SPEC AC #6) | YES | `autonomous: false` (frontmatter line 24); `type="checkpoint:human-verify" gate="blocking"`; dark + light Puppeteer screenshots via emulateMediaFeatures; resume-signal contract present (line 498) |
|
||||
| A17.8 update (raw-source grep) | YES | Task 3 Edit 1; A17.8 honestly narrowed to SOURCE-BUNDLING only; A35 carries the live-DOM proof |
|
||||
|
||||
---
|
||||
|
||||
## FORBIDDEN_HOOK_STRINGS lockstep — VERIFIED at 12
|
||||
|
||||
Confirmed live: `tests/background/no-test-hooks-in-prod-bundle.test.ts:108-126` lists exactly 12 strings. Plan 04-06 adds:
|
||||
|
||||
| Plan 04-06 addition | Production surface? | `__MOKOSH_UAT__`-gated? | Adds to FORBIDDEN_HOOK_STRINGS? |
|
||||
|---------------------|---------------------|--------------------------|----------------------------------|
|
||||
| `?raw` import in welcome.ts | YES (production source) | NO (always present) | NO — it is a normal production import |
|
||||
| `DOMParser` in welcome.ts populateMark | YES (production source) | NO (always present) | NO — DOMParser is a standard web platform API, not a test-mode symbol |
|
||||
| `*.svg?raw` ambient decl in globals.d.ts | NO (type-only, .d.ts) | N/A | NO — type declarations are erased at build time |
|
||||
| `driveA35` in tests/uat/lib/harness-page-driver.ts | NO (test-only file under tests/uat) | N/A | NO — never reaches `dist/` |
|
||||
| `driveA35Wrapped` + drivers-array entry in tests/uat/harness.test.ts | NO (test-only) | N/A | NO — never reaches `dist/` |
|
||||
| A17.8 raw-source grep update in tests/uat/extension-page-harness.ts | tests/uat/extension-page-harness.ts builds into `dist-test/`, NOT `dist/` (per vite.test.config.ts:90 — only included in the test bundle) | N/A | NO — never reaches `dist/` |
|
||||
| inline-svg.test.ts + cursor-visibility.test.ts | NO (test files) | N/A | NO |
|
||||
|
||||
Conclusion: Plan 04-06 adds NO test-mode-gated symbol to production. FORBIDDEN_HOOK_STRINGS stays at 12. PRESERVED.
|
||||
|
||||
---
|
||||
|
||||
## Verification-table spot-check — accuracy validated
|
||||
|
||||
Spot-checked 5+ planner verification-table entries against live code:
|
||||
|
||||
1. **vitest.config.ts:18 = `environment: 'node'`** — CONFIRMED (read directly; line 18 is exactly `environment: 'node'`).
|
||||
2. **A17.8 string-grep only on jsText** — CONFIRMED (`extension-page-harness.ts:2273` `hasInlineDataUrl = jsText.includes(...)`; line 2274 `svgFileUrlMatches = jsText.match(...)`; line 2287 `hasCanonicalViewBox` against `jsText`).
|
||||
3. **harness opens exactly victimPage + harnessPage at launch.ts:473-542** — CONFIRMED (launch.ts:473 victimPage = browser.newPage(); launch.ts:478 harnessPage = browser.newPage(); no other newPage() calls in launch.ts).
|
||||
4. **welcome.ts:194-198 readyState branch + DOMContentLoaded listener** — CONFIRMED (exact source matches the cited line range; init() called either eagerly or via DOMContentLoaded).
|
||||
5. **driveA33 signature `(page, browser, extensionId, downloadsDir)` at line 2622-2627** — CONFIRMED (exact match).
|
||||
6. **harness.test.ts:580 `total = drivers.length + 1`** — CONFIRMED (line 580 exact text).
|
||||
7. **`Browser + Page` already imported at harness-page-driver.ts:43** — CONFIRMED.
|
||||
8. **manifest-i18n.test.ts node-env file-read scaffold (the canonical analog)** — read; structure matches what the plan specifies for inline-svg.test.ts.
|
||||
|
||||
Planner's verification table is **accurate**; the iter-2 planner did genuinely re-verify every claim against actual code this session, as the meta-instruction required.
|
||||
|
||||
---
|
||||
|
||||
## Cosmetic-advisories introduced by iter-2 (3)
|
||||
|
||||
All 3 are NON-BLOCKING and do not affect goal achievement. Listed for executor awareness.
|
||||
|
||||
### ADVISORY-2A — stale architecture banner string
|
||||
|
||||
The plan instructs (Task 3 Edit 3): "Append ', A35' to the architecture banner string at line 283 (the literal driver-name list)". The current line 283 reads:
|
||||
|
||||
> `'Architecture: A0 pre-flight + extension-internal page driver (A1..A14, A15..A17, A18..A22, A23, A24, A25, A26, A27, A28, A29, A30, A31, A32)'`
|
||||
|
||||
But A33 and A34 are MISSING from this banner today — they were added to the drivers array (lines 513-534) but the banner string was not updated. If the executor literally appends `', A35'`, the banner becomes `...A32, A35` — still missing A33 and A34. The banner is cosmetic display only (the source of truth is `total = drivers.length + 1`), so this does NOT fail any gate, but the instruction's premise that "the banner already includes A33 and A34" is slightly stale.
|
||||
|
||||
**Suggested fix (cosmetic):** Executor should either append `', A33, A34, A35'` (correcting the stale banner along with the A35 add) OR leave the banner alone and let the auto-incremented count do the work. Non-blocking.
|
||||
|
||||
### ADVISORY-2B — Task 3 SKIP_PROD_REBUILD=0 rationale slightly inaccurate
|
||||
|
||||
The plan says (line 411): "SKIP_PROD_REBUILD=0 is intentional: the harness must rebuild dist-test against the Task 2 source edits so A17.8 (raw-source grep) and the new A35 (live welcome tab) run against the updated welcome bundle".
|
||||
|
||||
Actually `SKIP_PROD_REBUILD` gates the **prod** `dist/` build (used by A0 hook-string grep gate), NOT the test `dist-test/` build. The `dist-test/` is ALWAYS rebuilt because `package.json:12` defines `"test:uat": "npm run build:test && tsx tests/uat/harness.test.ts"` — `build:test` runs unconditionally regardless of SKIP_PROD_REBUILD. So the actual reason SKIP_PROD_REBUILD=0 is needed is the A0 grep-gate against `dist/`, not the harness welcome bundle.
|
||||
|
||||
**Effect:** None — the harness will correctly rebuild dist-test (always) and dist/ (because SKIP_PROD_REBUILD=0 unsets the skip). The plan's instruction produces the right behavior; only the rationale prose is slightly off. Non-blocking.
|
||||
|
||||
### ADVISORY-2C — race-analysis underspecified in threat model
|
||||
|
||||
The plan's threat model (T-04-06 row "New A35 harness tab") notes "tab is test-only, always closed in finally block — no production surface, no leaked tab". This covers the cleanup angle. But it does NOT explicitly note that **A35 is appended LAST in the drivers array**, which is what makes the "pollution-of-future-drivers" concern moot (there are no future drivers).
|
||||
|
||||
Independently verified safe:
|
||||
- Drivers run serially (harness.test.ts:542 `for (...)` loop, not Promise.all).
|
||||
- A35 is appended after A34 (the current last entry, harness.test.ts:534) — nothing runs after it.
|
||||
- A35 does NOT mutate chrome.storage (opening welcome.html does NOT trigger openWelcomeIfFirstInstall — that runs only on chrome.runtime.onInstalled).
|
||||
- A35's welcomePage.close() in finally ensures the extra tab does not leak.
|
||||
|
||||
So even if a future plan inserts a driver BEFORE A35 + AFTER something that depends on tab state, the current plan's design is safe. The threat model could be more explicit, but the design is sound. Non-blocking.
|
||||
|
||||
---
|
||||
|
||||
## NEW issues introduced by iter-2
|
||||
|
||||
None of BLOCKER or WARNING severity. The 3 cosmetic-advisories above are the only NEW issues, and all are non-blocking. The iter-2 re-plan did NOT introduce any new false premises (unlike iter-1, which traded one false premise for two).
|
||||
|
||||
---
|
||||
|
||||
## Goal-backward — D-P4-03 closure
|
||||
|
||||
D-P4-03 (locked, 04-CONTEXT.md) = BOTH (a) cursor visibility + (b) dark-surface logo contrast.
|
||||
|
||||
- **(a) cursor visibility:** closed by tests/build/cursor-visibility.test.ts (GREEN-on-arrival defensive pin) + 01-07-SUMMARY.md back-patch. Sound.
|
||||
- **(b) dark-logo contrast:**
|
||||
- SVG recolor + welcome.ts `?raw` import + DOMParser populateMark + globals.d.ts `?raw` decl land in Task 2 (3 source-contract tests flip RED→GREEN).
|
||||
- A17.8 raw-source grep update (Task 3 Edit 1) verifies the SVG source is bundled.
|
||||
- **NEW: A35 host-side harness assertion (Task 3 Edit 2-3) verifies the LIVE inline-SVG injection + the currentColor cascade in real Chrome via getComputedStyle().stroke on the injected svg.** This is the genuine automated coverage the iter-1 plan was missing.
|
||||
- Task 4 operator empirical (dark + light Puppeteer screenshots) is the additional human aesthetic gate, not a substitute.
|
||||
|
||||
D-P4-03 closure is **goal-reachable AND goal-verifiable** with the iter-2 plan. The verification story now matches the verification reality.
|
||||
|
||||
---
|
||||
|
||||
## Atomic commit / frontmatter / hooks
|
||||
|
||||
- **Task structure:** `gsd-sdk verify.plan-structure` returns `valid: true`, 4 tasks, all 4 have Files+Action+Verify+Done.
|
||||
- **Frontmatter:** valid; `revision_history` block updated for iter-2 with both BLOCKER fixes documented; `must_haves` parses; `autonomous: false` correct for the checkpoint; `files_modified` = 10 entries (added harness-page-driver.ts + harness.test.ts + deferred-items.md).
|
||||
- **FORBIDDEN_HOOK_STRINGS lockstep:** stays at 12 (verified above).
|
||||
- **Atomic commits:** 4 per-task commit messages convention-correct (`test(04-06):`, `feat(04-06):` x2, operator ack).
|
||||
- **Scope:** 4 tasks, 10 file modifications — within budget.
|
||||
- **Wave assignment:** Wave 5, `depends_on: [01..05]` — unchanged from iter-1, consistent.
|
||||
|
||||
---
|
||||
|
||||
## ISSUES
|
||||
|
||||
```yaml
|
||||
issues:
|
||||
- id: ADVISORY-2A
|
||||
dimension: task_completeness
|
||||
severity: cosmetic-advisory
|
||||
plan: "04-06"
|
||||
task: 3
|
||||
description: >
|
||||
Task 3 Edit 3 instructs the executor to "Append ', A35' to the architecture banner
|
||||
string at line 283 (the literal driver-name list)". The current banner reads
|
||||
"...A29, A30, A31, A32)" — A33 and A34 are MISSING from the banner today (they
|
||||
were added to the drivers array but the banner string was not updated). If the
|
||||
executor literally appends ', A35', the banner becomes "...A32, A35" — still
|
||||
missing A33 and A34. The banner is cosmetic display only (the count is
|
||||
auto-incremented via drivers.length + 1 at line 580), so this does NOT fail any
|
||||
gate, but the instruction's premise is slightly stale.
|
||||
fix_hint: >
|
||||
Either (a) executor appends ', A33, A34, A35' (correcting the stale banner
|
||||
while adding A35), or (b) leaves the banner alone and relies on the count.
|
||||
Non-blocking — no gate depends on the banner string.
|
||||
|
||||
- id: ADVISORY-2B
|
||||
dimension: task_completeness
|
||||
severity: cosmetic-advisory
|
||||
plan: "04-06"
|
||||
task: 3
|
||||
description: >
|
||||
Task 3 action prose says "SKIP_PROD_REBUILD=0 is intentional: the harness must
|
||||
rebuild dist-test against the Task 2 source edits". Actually SKIP_PROD_REBUILD
|
||||
gates only the prod dist/ build (used by A0 hook-string grep). The dist-test
|
||||
build is ALWAYS run via `npm run build:test` (package.json:12) regardless of
|
||||
SKIP_PROD_REBUILD. The actual reason SKIP_PROD_REBUILD=0 is needed is the A0
|
||||
grep-gate against fresh dist/, not the welcome bundle.
|
||||
fix_hint: >
|
||||
Cosmetic prose accuracy fix: clarify that SKIP_PROD_REBUILD=0 ensures the A0
|
||||
grep-gate runs against fresh dist/ (dist-test always rebuilds). Executor
|
||||
behavior is unchanged — the command produces the right result.
|
||||
|
||||
- id: ADVISORY-2C
|
||||
dimension: verification_derivation
|
||||
severity: cosmetic-advisory
|
||||
plan: "04-06"
|
||||
description: >
|
||||
The threat model's T-04-06 "New A35 harness tab" row notes finally-block cleanup
|
||||
but does not explicitly note that A35 is appended LAST in the drivers array
|
||||
(which is what makes the "pollution-of-future-drivers" concern moot — there are
|
||||
no future drivers). Verified independently safe: drivers run serially
|
||||
(harness.test.ts:542); A35 is last; A35 does not mutate chrome.storage; finally
|
||||
closes welcomePage. Non-blocking.
|
||||
fix_hint: >
|
||||
Optionally add a one-line note to the threat model "+ A35 is appended LAST so
|
||||
tab opening cannot race subsequent drivers (none exist)" — pure documentation
|
||||
clarity, no design change.
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Remediation summary for the orchestrator
|
||||
|
||||
**PASSED** — proceed to `/gsd:execute-phase 04-06`.
|
||||
|
||||
The iter-2 re-plan correctly resolves both iter-1 BLOCKERs:
|
||||
|
||||
1. **BLOCKER 1 (fictitious A17.8 delegation):** replaced with genuine new host-side A35 harness assertion. Every test-infrastructure claim spot-checked against live code; design is sound; A35's `browser.newPage()` + `page.goto('chrome-extension://<id>/src/welcome/welcome.html')` correctly opens the canonical web-accessible welcome page (same URL A17 already fetches via chrome.runtime.getURL); populateMark runs at DOMContentLoaded as claimed; getComputedStyle().stroke proves the currentColor cascade live in real Chrome.
|
||||
|
||||
2. **BLOCKER 2 (phantom failing test name):** replaced with behavior-based gate logic that distinguishes flake from regression via isolation re-run, hard-coding no test filename. Baseline corrected to 184/184 GREEN (verified this session: full-suite was 184/184 GREEN this run); target 188/188 (+4 new tests); deferred-items.md mis-diagnosis corrected in Task 3 Edit 5.
|
||||
|
||||
All 3 iter-1 advisories are fixed. DEFECT 2 line classification is preserved unchanged. welcome.css drop, thesis preservation, FORBIDDEN_HOOK_STRINGS lockstep (12), and atomic-commit structure are all preserved. The 3 NEW cosmetic-advisories in this iteration are non-blocking and do not affect goal achievement.
|
||||
|
||||
No more revision iterations needed. The plan is ready to execute.
|
||||
Reference in New Issue
Block a user