.convoys/harden-visual-diff-gate.md

@@ -0,0 +1,112 @@
+---
+slug: harden-visual-diff-gate
+status: queued
+opened: 2026-06-12
+owner: rstillw
+prerequisites:
+  - Stable `main` post-`unify-glass-panel-surfaces` /
+    `cleanup-card-item-list-and-share-modal-palette` /
+    `migrate-button-input-mobilenav-to-glass-primitive` (all merged)
+  - CT 111 self-hosted runner online (resolved by PR #132, 2026-06-07)
+related:
+  - PR #58 (`83a358b`) — initial Linux baseline seeded
+  - PR #18 (`7b6f751`) — `adopt-playwright-smoke` Decision 4: `continue-on-error` end state
+  - `tests/visual/homepage.spec.ts` module docblock
+  - `.github/workflows/visual-diff.yml` line ~126
+---
+
+# harden-visual-diff-gate
+
+## Problem
+
+`.github/workflows/visual-diff.yml` still carries `continue-on-error: true`
+on the "Capture screenshots (PR)" step. This was the documented end state
+of `adopt-playwright-smoke` (Decision 4) because no Linux baseline existed
+yet. PR #58 seeded the baseline on 2026-06-02, but the gate was never
+flipped — visual drift continues to ship as artifacts + PR comments only,
+not as a blocking check.
+
+Additionally, the committed baseline (`tests/visual/__screenshots__/home.png`)
+was generated against `main` at `83a358b`, which is several convoys behind
+the current homepage rendering (glass redesign briefs, corner-light tone
+adjustments, card-class retirement, etc.). Flipping `continue-on-error`
+today would fail every PR touching `pages/**` / `components/**` / `styles/**`
+against this stale reference.
+
+## Two-step shape
+
+This is one convoy with a strict ordering constraint:
+
+### Step 1 — Re-seed the baseline against current `main`
+
+Generate a fresh `home.png` from the current homepage rendering on Linux
+(byte-equivalent to CT 111's runner output). Two options:
+
+- **Option A: Ad-hoc on CT 111.** SSH the homelab, exec into a runner
+  container, run `BASE_URL=https://deckhearth.com VERCEL_AUTOMATION_BYPASS_SECRET=<value> npm run test:visual:update`,
+  scp the new PNG back, commit it. Most direct.
+- **Option B (recommended): New `seed-visual-baselines.yml` workflow.**
+  `workflow_dispatch`-only, runs on `[self-hosted, axiom]`, takes
+  `BASE_URL` as an input, runs `--update-snapshots`, uploads the
+  regenerated `tests/visual/__screenshots__/**` as an artifact for the
+  operator to download + commit. Repeatable; no SSH dance.
+
+Either option ships a PR with the new baseline PNG. The PR should also
+clear the "Known staleness" callout in `AGENTS.md` § Testing § Visual
+baselines.
+
+### Step 2 — Flip the gate
+
+After Step 1's PR merges and a follow-up PR confirms `Screenshot diff`
+passes cleanly against the new baseline:
+
+- Remove `continue-on-error: true` from
+  `.github/workflows/visual-diff.yml` (line ~126).
+- Update `AGENTS.md` § CI behavior § Screenshot diff to drop the
+  "advisory, not gating" language.
+- Update the module docblock in `tests/visual/homepage.spec.ts` to
+  drop the "still carries `continue-on-error: true`" callout.
+- Optional: bump `expect.toHaveScreenshot.maxDiffPixelRatio` in
+  `playwright.config.js` if some pixel jitter is unavoidable (e.g.
+  font subpixel rendering across runner image bumps). Default is 0
+  (any pixel diff fails); 0.01 (1%) is a common safe floor for
+  Chromium-vs-Chromium runs.
+
+## Decisions to ratify
+
+- **D1.** Option B (new `seed-visual-baselines.yml` workflow) vs Option A
+  (ad-hoc). Recommend B for repeatability — the homepage will continue
+  to evolve and re-seeding will be a recurring operation, not a
+  one-shot.
+- **D2.** `maxDiffPixelRatio` value. Defer to operator preference; 0 is
+  the strictest and what we currently use implicitly via Playwright
+  defaults. 0.001-0.01 is a reasonable cushion.
+- **D3.** Do we also harden `preview-smoke.yml`? It already runs as a
+  blocking gate (no `continue-on-error`); no change needed. This convoy
+  is scoped to `visual-diff.yml` only.
+
+## Risks
+
+| # | Risk | Mitigation |
+|---|---|---|
+| 1 | Re-seeded baseline drifts on next runner-image update | Watchtower's weekly update cycle (CT 111) could shift Chromium subpixel rendering. Mitigate via D2's `maxDiffPixelRatio` cushion |
+| 2 | Step 1's baseline PR fails its own `Screenshot diff` | Expected — the new baseline IS the visual change. Use `pipeline: skip visual` directive in the PR body OR open the baseline PR with the `skip-metrics` label equivalent for visual-diff (currently none — would need a new bypass mechanism). Cleaner: land the baseline PR via the `tests/visual/**` path filter, which DOES trigger visual-diff but the new screenshot vs new baseline should match by construction |
+| 3 | Step 2 lands but a third party reverts `continue-on-error` later | Add a 9th check to `forbidden-patterns` in `ci.yml`: `grep -n 'continue-on-error' .github/workflows/visual-diff.yml` should return zero |
+
+## Non-goals
+
+- Adding more visual baselines (login page, dashboard, etc.) — out of
+  scope. The single homepage baseline is the smoke test of the
+  visual-diff pipeline; deeper coverage is per-feature work.
+- Switching to a hosted visual-regression service (Percy, Chromatic,
+  Argos) — handles the platform problem cleanly but adds a paid
+  dependency. Local + axiom is free and works.
+
+## Acceptance
+
+- `visual-diff.yml` gates merge (failed diff = red required check).
+- Baseline regenerable via a reviewable Git-native workflow (no SSH
+  required, no Mac-overwrite-Linux footgun).
+- `AGENTS.md` + `tests/visual/homepage.spec.ts` docblock no longer
+  describe the gate as advisory.
+- Optional 9th forbidden-patterns check locks the gate in place.

AGENTS.md

@@ -150,21 +150,27 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
 
   Local-run convention: boot `next dev` in one terminal, then in another run `BASE_URL=http://localhost:3000 npm run test:smoke` (or against a deployed preview, `BASE_URL=https://<preview>.vercel.app VERCEL_AUTOMATION_BYPASS_SECRET=<value> npm run test:smoke`). No `next dev` auto-boot in the test scripts (Decision 6 of `adopt-playwright-smoke`).
 - **Browsers must be installed once locally:** `npx playwright install --with-deps chromium`. CI re-runs this on every workflow run (it's cached when possible).
-- **Visual baselines:** none committed yet. `tests/visual/__screenshots__/` is intentionally absent and intentionally NOT in `.gitignore` (baselines, when they exist, must be committed). First-run baseline generation MUST happen in a Linux environment so the PNG matches what CI produces. Recommended path is the Playwright Docker image:
+- **Visual baselines:** committed under `tests/visual/__screenshots__/`. The initial Linux baseline (`home.png`) was seeded by PR #58 (`83a358b`, 2026-06-02). Baselines are committed to git — they are not gitignored — so a `Screenshot diff` failure is reviewable from PR comments + artifacts without bouncing through a regeneration step. **Re-seeding** (when the homepage changes intentionally) MUST happen in a Linux environment so the PNG matches what CI produces. Recommended paths:
 
-  ```
+  1. **Playwright Docker image** (works from any host):
-  docker run --rm -v "$PWD":/work -w /work \
-    mcr.microsoft.com/playwright:v1.60.0-noble \
-    sh -c "npm ci && BASE_URL=<preview-url> \
-      VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
-      npm run test:visual:update"
-  ```
 
-  Mac-generated baselines will NOT match Linux CI — `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token, so a Mac update silently overwrites the canonical Linux baseline. Tracked as the queued `seed-visual-baselines-on-linux` convoy (see `.convoys/ship-readiness.md` § Queued convoys).
+     ```
+     docker run --rm -v "$PWD":/work -w /work \
+       mcr.microsoft.com/playwright:v1.60.0-noble \
+       sh -c "npm ci && BASE_URL=<preview-url> \
+         VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
+         npm run test:visual:update"
+     ```
+
+  2. **CT 111 directly** (preferred when iterating — same toolchain as the diff workflow, byte-equivalent output). Dispatched via the (queued) `seed-visual-baselines` workflow once it lands; until then, `pct exec 111 -- docker exec gha-runner-1 sh -c "...`" works ad-hoc.
+
+  Mac-generated baselines will NOT match Linux CI — `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token, so a Mac update silently overwrites the canonical Linux baseline. **Never run `npm run test:visual:update` on a Mac** unless you immediately throw the result away.
+
+  **Known staleness as of 2026-06-12:** the committed `home.png` was seeded against `main` at `83a358b` (pre-`unify-glass-panel-surfaces`, pre-`cleanup-card-item-list-and-share-modal-palette`, pre-`migrate-button-input-mobilenav-to-glass-primitive`). Until it is re-seeded, `Screenshot diff` is advisory only — see the next bullet for why `continue-on-error: true` is still in place on `visual-diff.yml`.
 - **CI behavior:**
   - **Vitest:** the `test:` job in `.github/workflows/ci.yml` runs `npm run test:run` on every PR and push to `main` and is **blocking** (no `|| true`, no `continue-on-error`). A red test job blocks merge.
   - **Playwright smoke:** runs on every PR via `preview-smoke.yml`. Gate skip via `pipeline: skip smoke` in the PR body (handled in the `gate:` job's Decide step via env-var routing — see § 7's shell-injection note). Pre-migration runtime: 59s end-to-end on `ubuntu-latest` (PR #18 post-merge run). Post-migration on the axiom pool: cold-cache first run ~6 min (Chromium download); warm cache thereafter ~1–2 min.
-  - **Screenshot diff:** runs only on PRs touching `pages/**` / `components/**` / `styles/**` / `tailwind.config.js` / `postcss.config.js` via `visual-diff.yml`. **First `Screenshot diff` run after `adopt-playwright-smoke` will fail at the test step** because no baseline exists yet; `continue-on-error: true` swallows the failure and the comment-on-PR step posts "Visual Diff — view run" with empty artifacts. That is the documented Decision-4 end state of `adopt-playwright-smoke`, not a regression — it stays that way until `seed-visual-baselines-on-linux` lands.
+  - **Screenshot diff:** runs only on PRs touching `pages/**` / `components/**` / `styles/**` / `tailwind.config.js` / `postcss.config.js` (and `tests/visual/**` for baseline updates) via `visual-diff.yml`. The committed Linux baseline at `tests/visual/__screenshots__/home.png` (PR #58, `83a358b`) means the test now exercises a real comparison rather than fast-failing on a missing file. The screenshot-capture step still carries `continue-on-error: true`, so the diff is **advisory, not gating**: artifacts + a "Visual Diff — view run" PR comment surface drift, but a red run does not block merge. Flipping that to a hard gate is the queued `harden-visual-diff-gate` follow-up — it requires re-seeding the baseline against post-`unify-glass-panel-surfaces` `main` first, otherwise every UI-touching PR fails the new gate against a stale reference image.
 - **CI minute optimizations (slash-ci-minutes convoy, 2026-06-04):**
   - **Doc-only PRs skip ALL of ci.yml + preview-smoke.yml.** Both workflows carry `paths-ignore` for `.convoys/**`, `**/*.md`, `docs/**`, `AGENTS.md`, `.cursor/**`, and `README.md`. A pure-docs PR triggers zero GitHub Actions jobs (Vercel still builds — it's not on the Actions billing). `visual-diff.yml` was already cost-conscious via a positive `paths:` allowlist and is unchanged.
   - **The 6 grep-only forbidden-* jobs collapsed into one.** They previously ran as 6 independent jobs (each with its own `actions/checkout`); the consolidated `forbidden-patterns` job runs all 6 checks as labeled `::group::` sections in a single bash step, with a FAIL flag at the bottom so every violation across all 6 checks still surfaces in one run (same diagnostic behavior, ~5/6 of the per-PR checkout overhead removed). The 6 original job names (`forbidden-endpoints`, `forbidden-cors-headers`, `forbidden-client-side-llm-keys`, `forbidden-modal-shell-without-primitive`, `forbidden-deprecated-color-aliases`, `forbidden-stale-strings`) no longer appear in the checks list — references in this file (e.g. CI job `forbidden-stale-strings` blocks ...) are now informational, not check-name lookups. `pr-health-rollup.yml` was unaffected because it only looks up `Lint` and `Schema map up to date` by name.

playwright.config.js

@@ -76,18 +76,20 @@ export default defineConfig({
   reporter: IS_CI ? [['list'], ['html', { open: 'never' }]] : 'list',
 
   // Visual baselines live at `tests/visual/__screenshots__/<arg>{ext}`.
-  // Workflow `visual-diff.yml` line 94 uploads this exact path as the
+  // Workflow `visual-diff.yml` uploads this exact path as the artifact —
-  // artifact — keep them aligned. {arg} is the snapshot name from
+  // keep them aligned. {arg} is the snapshot name from
   // `toHaveScreenshot('home.png')` without the extension; {ext} is
   // the extension with the leading dot.
   //
-  // Cross-platform note (Risk R3): this template drops Playwright's
+  // Cross-platform CAUTION (still live): this template drops Playwright's
-  // default `-<browser>-<platform>` suffix. That means a Mac dev
+  // default `-<browser>-<platform>` suffix. A Mac dev running
-  // running `npm run test:visual:update` overwrites the Linux-CI
+  // `npm run test:visual:update` silently overwrites the Linux-CI
-  // baseline. Per Decision 4, we don't commit baselines this convoy;
+  // baseline with a Mac-rendered PNG that the CT 111 runner will reject
-  // operator seeds via `mcr.microsoft.com/playwright:v1.60.0-noble`
+  // on the next diff. The seeded Linux baseline (PR #58 `83a358b`,
-  // Docker on Linux. Multi-platform support is the
+  // 2026-06-02) is the canonical artifact. Re-seed via the Linux Docker
-  // `seed-visual-baselines-on-linux` follow-up convoy's job.
+  // recipe in `tests/visual/homepage.spec.ts`'s module docblock OR
+  // dispatch the (queued) `seed-visual-baselines` workflow on CT 111.
+  // Never via `npm run test:visual:update` on a Mac.
   snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}',
 
   use: {

tests/visual/homepage.spec.ts

@@ -3,20 +3,27 @@ import { test, expect } from '@playwright/test';
 /**
  * Visual baseline for the public homepage.
  *
- * FIRST RUN (no committed baseline yet):
+ * The Screenshot diff workflow runs `playwright test --project=visual
- *   The Screenshot diff workflow runs `playwright test --project=visual
+ * --update-snapshots=none` (per .github/workflows/visual-diff.yml). The
- *   --update-snapshots=none` (per .github/workflows/visual-diff.yml). With
+ * `none` flag means "use the existing baseline; do not write a new one"
- *   no baseline file at `tests/visual/__screenshots__/home.png` AND the
+ * — drift between the rendered page and the committed PNG fails the
- *   `none` flag, this test FAILS — and that's the documented end state of
+ * test. The committed Linux baseline lives at
- *   the `adopt-playwright-smoke` convoy (Decision 4 in
+ * `tests/visual/__screenshots__/home.png` (seeded by PR #58 `83a358b`,
- *   `.convoys/adopt-playwright-smoke.md`). The workflow's
+ * 2026-06-02).
- *   `continue-on-error: true` swallows the failure and the comment-on-PR
- *   step posts "Visual Diff — view run" with empty artifacts.
  *
- * SEEDING THE BASELINE (post-merge follow-up):
+ * NOTE: as of this writing, `visual-diff.yml`'s screenshot capture step
- *   Run `npm run test:visual:update` in a Linux environment so the
+ * still carries `continue-on-error: true` — failures surface as
- *   generated PNG matches what CI will produce. The cleanest path is the
+ * artifacts + PR comments but do not gate merge. Flipping that to a
- *   Playwright Docker image:
+ * hard gate is the `harden-visual-diff-gate` follow-up; it depends on
+ * the baseline being re-seeded against the post-glass-redesign main
+ * (the seeded PR-#58 PNG predates the `unify-glass-panel-surfaces` /
+ * `cleanup-card-item-list-and-share-modal-palette` /
+ * `migrate-button-input-mobilenav-to-glass-primitive` work).
+ *
+ * RE-SEEDING THE BASELINE (when the homepage changes intentionally):
+ *   Run on Linux — Mac-generated PNGs do not match CT 111 Linux output
+ *   because `playwright.config.js`'s `snapshotPathTemplate` has no
+ *   `{platform}` token. Recommended path is the Playwright Docker image:
  *
  *     docker run --rm -v "$PWD":/work -w /work \
  *       mcr.microsoft.com/playwright:v1.60.0-noble \
@@ -24,8 +31,11 @@ import { test, expect } from '@playwright/test';
  *         VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
  *         npm run test:visual:update"
  *
- *   Then commit `tests/visual/__screenshots__/home.png`. This is tracked
+ *   …or dispatch the (queued) `seed-visual-baselines` workflow on
- *   as the `seed-visual-baselines-on-linux` follow-up convoy.
+ *   CT 111 directly, which uses the same Linux toolchain as the
+ *   diff workflow so byte-equivalence is guaranteed.
+ *
+ *   Commit the regenerated `tests/visual/__screenshots__/home.png`.
  */
 const BASE = process.env.BASE_URL ?? 'http://localhost:3000';