--- slug: harden-visual-diff-gate status: shipping 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` *(SHIPPED)* **Resolved via Option B** (see D1 below). New workflow `.github/workflows/seed-visual-baselines.yml` is `workflow_dispatch`-only, runs on `[self-hosted, axiom]`, takes `base_url` + `reason` as inputs, and auto-opens a `chore(visual): refresh baselines from ` PR with the regenerated PNGs using `peter-evans/create-pull-request@v6`. The workflow is byte-equivalent to `visual-diff.yml` (same Chromium version via shared `package-lock.json`, same `myoung34/github-runner` image on CT 111) — so the captured baseline will match the next diff run cleanly. If no baselines changed (rendering matches existing committed PNGs), the workflow emits a `::notice::` and opens nothing. **Operator action:** dispatch via the GitHub UI or `gh workflow run seed-visual-baselines.yml -f base_url= -f reason="..."`, then review + merge the resulting `chore(visual):` PR. The "Known staleness" callout in `AGENTS.md` § Testing § Visual baselines clears automatically once that PR merges (next PR touching `AGENTS.md` should sweep the line). ### Step 2 — Flip the gate *(SHIPPED via PR #140)* PR #139 (`54495fe`) landed the fresh baseline. PR #140 then: - Removed `continue-on-error: true` from `.github/workflows/visual-diff.yml`'s `Capture screenshots (PR)` step. - Added a 9th `forbidden-patterns` check in `.github/workflows/ci.yml` that greps `visual-diff.yml` for `^\s*continue-on-error:\s*true` and fails the build if it returns (Risk #3 mitigation made concrete). Scoped narrowly to that one file — other workflows (e.g. `seed-visual-baselines.yml`'s PR-open step, see D4 below) legitimately use the flag. - Rewrote `AGENTS.md` § Testing § Screenshot diff to lead with "hard merge gate", document the intentional-change runbook (dispatch seed workflow → manually open PR → merge → re-run), and reference the new ci.yml check. - Rewrote the module docblock in `tests/visual/homepage.spec.ts` to match the AGENTS.md runbook and drop the "advisory, not gating" language. `maxDiffPixelRatio` (D2 below) intentionally not touched — left at the Playwright default of 0 (any pixel diff fails). If subpixel jitter from a future runner-image bump becomes a problem, that becomes the trigger for a new convoy (`tune-visual-diff-tolerance`) rather than a quiet config bump. ## Decisions to ratify - **D1.** *(ratified)* Option B (new `seed-visual-baselines.yml` workflow) chosen over Option A (ad-hoc SSH). The homepage will continue to evolve and re-seeding is a recurring operation; the workflow shape removes the SSH dance and the Mac-overwrite footgun. Implementation uses `peter-evans/create-pull-request@v6` so the baseline lands as a reviewable PR rather than a direct push to `main`. - **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. - **D4.** *(emerged during Step 1 dispatch, 2026-06-13)* How to handle the `peter-evans/create-pull-request@v6` PR-open failure caused by the stwl-labs org-level "Allow GitHub Actions to create and approve pull requests" setting being OFF. Three options: - **A. Flip the org setting on** — fastest path; but a real surface increase (any future PR-creating workflow could push untrusted PRs). The org default is OFF for a reason. - **B. Use a fine-grained PAT scoped to `tcg-vault` + `pull-requests:write`** seeded as `secrets.HOMELAB_CI_PAT` — sidesteps the org setting but adds another rotating secret. - **C. Accept the limitation; document the manual `gh pr create` step as the runbook.** Operator dispatches the workflow, the branch pushes cleanly, the PR-open step fails-soft with a loud notice telling the operator the exact `gh pr create` command. **Chosen: C.** Baseline regen is a low-frequency operation (~once per major UI change), the manual step adds ~30s of operator time, and there's no new attack surface or secret to rotate. PR #140 updated the workflow to (a) flag the create-PR step with `continue-on-error: true` (narrowly scoped, with an inline rationale callout distinguishing it from the just-removed `visual-diff.yml` flag), (b) disambiguate the three possible outcomes (no-changes / pr-opened / branch-pushed-pr-blocked) via a `git ls-remote` check on the bot branch, and (c) exit non-zero on outcome C so the workflow run shows red and the operator can't miss the followup. ## 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.