Removes `continue-on-error: true` from `.github/workflows/visual-diff.yml`'s `Capture screenshots (PR)` step. Visual drift is now a real merge gate on UI-touching PRs. Brief 2/2 of the `harden-visual-diff-gate` convoy. PR #138 shipped the seed workflow (Brief 1); PR #139 (`54495fe`) landed the fresh Linux baseline regenerated against post-glass-redesign main on CT 111. With a known-good baseline committed, the gate can flip without false-failing every UI-touching PR. ## What changes - `.github/workflows/visual-diff.yml` — drop the `continue-on-error: true` flag; add an inline rationale block linking to the convoy + the operator runbook for both intentional changes (dispatch seed workflow → manually open PR → merge → re-run) and unintentional regressions (inspect artifact diff → fix → push). - `.github/workflows/ci.yml` — add 9th `forbidden-patterns` check that greps `visual-diff.yml` for `^\s*continue-on-error:\s*true` and fails the build if it returns. Risk #3 of the convoy made concrete: prevents silent re-introduction via template revert. Scoped narrowly to that one file; other workflows (`seed-visual-baselines.yml`'s PR-open step, etc.) legitimately use the flag. Job name bumped from "Forbidden patterns (8 checks)" → "(9 checks)". All `Check N/8` group labels renumbered to `N/9`. - `AGENTS.md` — § Testing § Visual baselines rewritten to drop the "Known staleness as of 2026-06-12" callout (resolved by PR #139); § Testing § Screenshot diff rewritten to lead with "hard merge gate", document the intentional-change runbook, reference the new ci.yml check, and explicitly mention the org-setting caveat for the seed workflow's auto-PR step. - `tests/visual/homepage.spec.ts` — module docblock rewritten to match the AGENTS.md runbook: drops the "advisory, not gating" language; promotes the seed-visual-baselines workflow as the primary re-seeding path; demotes the Playwright Docker image to the offline fallback. - `.github/workflows/seed-visual-baselines.yml` — patches the `peter-evans/create-pull-request@v6` PR-open failure case discovered during Brief 1's first dispatch (run 27454132468). The PR-open step is now `continue-on-error: true` (narrowly scoped, with an inline rationale callout distinguishing it from the just-removed `visual-diff.yml` flag — that one silently hid real UI regressions; this one fronts a known org-level "Allow GitHub Actions to create and approve pull requests" limitation with a loud failure notice). New steps disambiguate the three possible outcomes (no-changes / pr-opened / branch-pushed-pr-blocked) via a `git ls-remote` check on the bot branch and exit non-zero on the blocked-PR case so the workflow run shows red and the operator gets the exact `gh pr create` command in the run logs. - `.convoys/harden-visual-diff-gate.md` — status: shipping; Step 2 marked SHIPPED; Decision D4 ratified (chose option C: accept org setting, document manual `gh pr create` fallback). Inline links to PR #139 + PR #140. ## Test plan - [x] `npm run lint` — clean (1 pre-existing unrelated warning) - [x] `npm run test:run` — 24 files / 118 tests pass - [ ] CI on this PR: 9th forbidden-patterns check passes; visual-diff job passes against the fresh baseline; convoy-metrics-gate passes (2 new rows added by this commit) - [ ] After merge: smoke test the 9th check by opening a throwaway PR that re-adds `continue-on-error: true` to `visual-diff.yml`; confirm it red-X's. (Skip if confident in the grep.) ## Convoy state - Brief 1: SHIPPED (PR #138, `c100c5f`, 2026-06-13) - Baseline refresh: SHIPPED (PR #139, `54495fe`, 2026-06-13) - Brief 2 (this PR): shipping - Convoy closeout: this PR's merge Co-authored-by: Cursor <cursoragent@cursor.com>
7.8 KiB
slug: harden-visual-diff-gate status: shipping opened: 2026-06-12 owner: rstillw prerequisites:
- Stable
mainpost-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-smokeDecision 4:continue-on-errorend state tests/visual/homepage.spec.tsmodule docblock.github/workflows/visual-diff.ymlline ~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 <url> 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=<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: truefrom.github/workflows/visual-diff.yml'sCapture screenshots (PR)step. - Added a 9th
forbidden-patternscheck in.github/workflows/ci.ymlthat grepsvisual-diff.ymlfor^\s*continue-on-error:\s*trueand 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.tsto 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.ymlworkflow) 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 usespeter-evans/create-pull-request@v6so the baseline lands as a reviewable PR rather than a direct push tomain. -
D2.
maxDiffPixelRatiovalue. 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 (nocontinue-on-error); no change needed. This convoy is scoped tovisual-diff.ymlonly. -
D4. (emerged during Step 1 dispatch, 2026-06-13) How to handle the
peter-evans/create-pull-request@v6PR-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:writeseeded assecrets.HOMELAB_CI_PAT— sidesteps the org setting but adds another rotating secret. - C. Accept the limitation; document the manual
gh pr createstep 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 exactgh pr createcommand.
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-removedvisual-diff.ymlflag), (b) disambiguate the three possible outcomes (no-changes / pr-opened / branch-pushed-pr-blocked) via agit ls-remotecheck 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.ymlgates 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.tsdocblock no longer describe the gate as advisory.- Optional 9th forbidden-patterns check locks the gate in place.