c100c5f192
Adds a workflow_dispatch-triggered job on the self-hosted axiom runner
that captures fresh `tests/visual/__screenshots__/*.png` against a
caller-provided URL and opens a `chore(visual): refresh baselines from
<url>` PR via peter-evans/create-pull-request@v6.
This is brief 1 of 2 of the harden-visual-diff-gate convoy. The
workflow exists but is not invoked by this PR — operator dispatches via
the GitHub UI or `gh workflow run seed-visual-baselines.yml` once they
want a fresh baseline against post-glass-redesign main.
Brief 2 (flip continue-on-error: true off visual-diff.yml, add 9th
forbidden-patterns check) is unblocked once a fresh baseline lands via
this workflow's auto-PR.
Convoy decision D1 ratified: Option B (workflow_dispatch + auto-PR)
chosen over Option A (ad-hoc SSH-into-CT-111 + manual commit). The
workflow gives the baseline regeneration a reviewable Git-native shape;
the auto-PR carries a checklist for visual sanity-check before merge.
Workflow shape:
- Inputs: base_url (required, no default to avoid wrong-target
accidents), reason (optional, used in PR body).
- Permissions: contents: write + pull-requests: write — sufficient for
default GITHUB_TOKEN; no PAT needed.
- Caches: shares the node_modules + Playwright browser caches with
visual-diff.yml so the byte-equivalence guarantee holds without
cache miss overhead.
- Idempotent: peter-evans/create-pull-request short-circuits to a
::notice:: annotation if the captures match the existing committed
baselines (no PR opened).
Metrics: logged role-conductor + role-architect + role-implementer
events for this convoy in .convoys/.metrics.jsonl. Satisfies the
convoy-metrics-gate (PR #134, 9eef8d9) that fires on convoy:-titled
PRs requiring at least one new metrics row.
Co-authored-by: Cursor <cursoragent@cursor.com>
118 lines
5.8 KiB
Markdown
118 lines
5.8 KiB
Markdown
---
|
|
slug: harden-visual-diff-gate
|
|
status: in-progress
|
|
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 <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
|
|
|
|
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.** *(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.
|
|
|
|
## 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.
|