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>
163 lines
6.3 KiB
YAML
163 lines
6.3 KiB
YAML
name: Visual diff
|
|
|
|
# Same as preview-smoke — waits for Vercel's Preview deployment, then captures
|
|
# Playwright screenshots against it. UI-paths-only trigger to keep cost down.
|
|
# Paths are tcg-vault-specific (pages router, JS).
|
|
|
|
on:
|
|
pull_request:
|
|
branches: [main]
|
|
paths:
|
|
- 'pages/**'
|
|
# Exclude API-only edits — they don't render UI, so they can't move
|
|
# any visual-diff pixels. Order matters: GitHub Actions evaluates the
|
|
# `paths:` list with minimatch and applies `!`-prefixed exclusions
|
|
# only after they've already matched a prior include. Keep this entry
|
|
# immediately AFTER `pages/**`.
|
|
# Surfaced by `tighten-visual-diff-path-filter` after PR #19
|
|
# (cors-tighten) and PR #20 (add-rate-limiting) both falsely
|
|
# triggered Screenshot diff at ~55s/PR.
|
|
- '!pages/api/**'
|
|
- 'components/**'
|
|
- 'styles/**'
|
|
- 'tailwind.config.js'
|
|
- 'postcss.config.js'
|
|
# Baseline-only PRs (specs + committed snapshots) should still run
|
|
# visual diff now that home.png is in tests/visual/__screenshots__/.
|
|
- 'tests/visual/**'
|
|
|
|
concurrency:
|
|
group: visual-diff-${{ github.event.pull_request.number }}
|
|
cancel-in-progress: true
|
|
|
|
# `deployments: read` + `statuses: read` are required by wait-for-vercel-preview
|
|
# (see preview-smoke.yml for context). `pull-requests: write` is required by the
|
|
# final github-script step that posts the "Visual Diff" comment back to the PR;
|
|
# without it the API returns 403 even though the screenshots upload fine.
|
|
permissions:
|
|
contents: read
|
|
deployments: read
|
|
pull-requests: write
|
|
statuses: read
|
|
|
|
jobs:
|
|
gate:
|
|
name: Should run?
|
|
runs-on: [self-hosted, axiom]
|
|
outputs:
|
|
should_run: ${{ steps.check.outputs.should_run }}
|
|
steps:
|
|
- id: check
|
|
env:
|
|
# See preview-smoke.yml for the rationale: ${{ github.event.* }}
|
|
# inlined into shell is a syntax-error + injection vector.
|
|
PR_BODY: ${{ github.event.pull_request.body }}
|
|
PR_IS_FORK: ${{ github.event.pull_request.head.repo.fork }}
|
|
run: |
|
|
if [[ "$PR_IS_FORK" == "true" ]]; then
|
|
echo "should_run=false" >> $GITHUB_OUTPUT
|
|
echo "::notice::Visual diff skipped on fork PR (bypass secret unavailable to forks)"
|
|
elif echo "$PR_BODY" | grep -qE 'pipeline:.*skip.*\bvisual\b'; then
|
|
echo "should_run=false" >> $GITHUB_OUTPUT
|
|
echo "::notice::Visual diff skipped via pipeline directive"
|
|
else
|
|
echo "should_run=true" >> $GITHUB_OUTPUT
|
|
fi
|
|
|
|
visual:
|
|
name: Screenshot diff
|
|
needs: gate
|
|
if: needs.gate.outputs.should_run == 'true'
|
|
runs-on: [self-hosted, axiom]
|
|
timeout-minutes: 20
|
|
steps:
|
|
- uses: actions/checkout@v4
|
|
|
|
- name: Wait for Vercel Preview deployment
|
|
id: vercel
|
|
uses: patrickedqvist/wait-for-vercel-preview@v1.3.2
|
|
with:
|
|
token: ${{ secrets.GITHUB_TOKEN }}
|
|
max_timeout: 120
|
|
# See preview-smoke.yml for the no-`set-bypass-cookie` rationale.
|
|
path: /?x-vercel-protection-bypass=${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
|
|
|
|
- uses: actions/setup-node@v4
|
|
with:
|
|
node-version: '20'
|
|
cache: npm
|
|
|
|
- name: Cache node_modules
|
|
id: cache-node-modules
|
|
uses: actions/cache@v4
|
|
with:
|
|
path: node_modules
|
|
key: node-modules-${{ runner.os }}-node20-${{ hashFiles('package-lock.json') }}
|
|
|
|
- run: npm ci
|
|
if: steps.cache-node-modules.outputs.cache-hit != 'true'
|
|
|
|
# Cache Playwright browsers — see preview-smoke.yml for rationale.
|
|
- name: Resolve Playwright version
|
|
id: pw-version
|
|
run: |
|
|
VERSION=$(node -p "require('./package-lock.json').packages['node_modules/@playwright/test'].version")
|
|
echo "version=$VERSION" >> "$GITHUB_OUTPUT"
|
|
- name: Cache Playwright browsers
|
|
id: cache-playwright
|
|
uses: actions/cache@v4
|
|
with:
|
|
path: ~/.cache/ms-playwright
|
|
key: playwright-${{ runner.os }}-${{ steps.pw-version.outputs.version }}-chromium
|
|
|
|
- name: Install Playwright browsers
|
|
run: |
|
|
if [ "${{ steps.cache-playwright.outputs.cache-hit }}" = "true" ]; then
|
|
npx playwright install-deps chromium
|
|
else
|
|
npx playwright install --with-deps chromium
|
|
fi
|
|
|
|
# Hard-gating after `harden-visual-diff-gate` brief 2 (PR #140).
|
|
# The prior `continue-on-error: true` was a documented end state of
|
|
# `adopt-playwright-smoke` (Decision 4) — necessary while no Linux
|
|
# baseline existed. PR #58 seeded the first baseline, and PR #139
|
|
# refreshed it against post-glass-redesign main. Visual diff is now
|
|
# a real merge gate.
|
|
#
|
|
# If this step fails on a UI-touching PR, two paths:
|
|
# (a) Intentional change: dispatch
|
|
# `.github/workflows/seed-visual-baselines.yml` against this
|
|
# PR's preview URL, merge the resulting baseline-refresh PR,
|
|
# then re-run this workflow.
|
|
# (b) Unintentional regression: open the artifact bundle from
|
|
# this run, inspect the diff PNG, fix the regression in
|
|
# source, push.
|
|
- name: Capture screenshots (PR)
|
|
run: npx playwright test --project=visual --update-snapshots=none
|
|
env:
|
|
BASE_URL: ${{ steps.vercel.outputs.url }}
|
|
VERCEL_AUTOMATION_BYPASS_SECRET: ${{ secrets.VERCEL_AUTOMATION_BYPASS_SECRET }}
|
|
|
|
- name: Upload screenshots + diffs
|
|
if: always()
|
|
uses: actions/upload-artifact@v4
|
|
with:
|
|
name: visual-diff
|
|
path: |
|
|
tests/visual/__screenshots__/
|
|
test-results/
|
|
retention-days: 7
|
|
|
|
- name: Comment on PR with diff link
|
|
if: always()
|
|
uses: actions/github-script@v7
|
|
with:
|
|
script: |
|
|
const run = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
|
|
github.rest.issues.createComment({
|
|
issue_number: context.issue.number,
|
|
owner: context.repo.owner,
|
|
repo: context.repo.repo,
|
|
body: `## Visual Diff\n\nScreenshots and diffs uploaded as artifacts: [view run](${run})\n\nIf intentional changes: update snapshots locally with \`npx playwright test --project=visual --update-snapshots\` and commit.`
|
|
});
|