convoy: flip visual-diff to a hard merge gate (harden-visual-diff-gate brief 2/2) (#140)
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>
This commit is contained in:
varutasu
GitHub
parent
54495fea28
commit
c2ebd18cf0
7 changed files with 198 additions and 54 deletions
|
|
@ -66,3 +66,5 @@
|
|||
{"ts": "2026-06-13T02:21:43Z", "role": "role-conductor", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "classification": "ci", "duration_s": 120, "outcome": "routed-to-architect"}
|
||||
{"ts": "2026-06-13T02:21:43Z", "role": "role-architect", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "classification": "ci", "duration_s": 420, "outcome": "architecture-only"}
|
||||
{"ts": "2026-06-13T02:21:43Z", "role": "role-implementer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 1, "classification": "ci", "duration_s": 900, "outcome": "pr-open"}
|
||||
{"ts": "2026-06-13T02:57:31Z", "role": "role-implementer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 2, "classification": "ci", "duration_s": 1200, "outcome": "pr-open"}
|
||||
{"ts": "2026-06-13T02:57:31Z", "role": "role-reviewer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 2, "classification": "ci", "duration_s": 300, "outcome": "approved"}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
---
|
||||
slug: harden-visual-diff-gate
|
||||
status: in-progress
|
||||
status: shipping
|
||||
opened: 2026-06-12
|
||||
owner: rstillw
|
||||
prerequisites:
|
||||
|
|
@ -58,22 +58,31 @@ 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
|
||||
### Step 2 — Flip the gate *(SHIPPED via PR #140)*
|
||||
|
||||
After Step 1's PR merges and a follow-up PR confirms `Screenshot diff`
|
||||
passes cleanly against the new baseline:
|
||||
PR #139 (`54495fe`) landed the fresh baseline. PR #140 then:
|
||||
|
||||
- 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.
|
||||
- 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
|
||||
|
||||
|
|
@ -91,6 +100,32 @@ passes cleanly against the new baseline:
|
|||
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 |
|
||||
|
|
|
|||
40
.github/workflows/ci.yml
vendored
40
.github/workflows/ci.yml
vendored
|
|
@ -116,7 +116,7 @@ jobs:
|
|||
# (matches the previous independent-jobs behavior; nicer than
|
||||
# stop-at-first-failure).
|
||||
forbidden-patterns:
|
||||
name: Forbidden patterns (8 checks)
|
||||
name: Forbidden patterns (9 checks)
|
||||
runs-on: [self-hosted, axiom]
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
|
|
@ -125,7 +125,7 @@ jobs:
|
|||
FAIL=0
|
||||
|
||||
# ---------- (1) No dev endpoints in pages/api ----------
|
||||
echo "::group::Check 1/8: No dev endpoints in pages/api"
|
||||
echo "::group::Check 1/9: No dev endpoints in pages/api"
|
||||
BAD_PATHS=(
|
||||
"pages/api/simple.js"
|
||||
"pages/api/test-auth.js"
|
||||
|
|
@ -153,7 +153,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (2) No wildcard CORS in pages/api ----------
|
||||
echo "::group::Check 2/8: No wildcard CORS in pages/api"
|
||||
echo "::group::Check 2/9: No wildcard CORS in pages/api"
|
||||
MATCHES=$(grep -rEn 'Access-Control-Allow-(Origin|Methods|Headers)' pages/api/ 2>/dev/null || true)
|
||||
if [ -n "$MATCHES" ]; then
|
||||
echo "::error::Forbidden CORS headers present under pages/api/. Remove them — same-origin Vercel deployment does not need CORS."
|
||||
|
|
@ -169,7 +169,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (3) No client-side LLM key leakage ----------
|
||||
echo "::group::Check 3/8: No client-side LLM key leakage"
|
||||
echo "::group::Check 3/9: No client-side LLM key leakage"
|
||||
if [ -f pages/api/config/gemini.js ]; then
|
||||
echo "::error file=pages/api/config/gemini.js::Forbidden config endpoint — do not return API keys to browsers."
|
||||
FAIL=1
|
||||
|
|
@ -225,7 +225,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (4) No legacy modal shells ----------
|
||||
echo "::group::Check 4/8: No new modal shells without <Modal> primitive"
|
||||
echo "::group::Check 4/9: No new modal shells without <Modal> primitive"
|
||||
MODAL_FOUND=()
|
||||
while IFS= read -r file; do
|
||||
MODAL_FOUND+=("$file")
|
||||
|
|
@ -244,7 +244,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (5) No deprecated color aliases ----------
|
||||
echo "::group::Check 5/8: No use of deprecated color aliases"
|
||||
echo "::group::Check 5/9: No use of deprecated color aliases"
|
||||
PATTERNS=(
|
||||
'gradient-text-purple'
|
||||
'gradient-text-pink'
|
||||
|
|
@ -278,7 +278,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (6) No stale ownership/collection copy ----------
|
||||
echo "::group::Check 6/8: No stale ownership/collection copy"
|
||||
echo "::group::Check 6/9: No stale ownership/collection copy"
|
||||
STRING_PATTERNS=(
|
||||
'Mark Owned'
|
||||
'Owned Cards'
|
||||
|
|
@ -307,7 +307,7 @@ jobs:
|
|||
echo "::endgroup::"
|
||||
|
||||
# ---------- (7) No bespoke var(--glass-surface-*) inline styles ----------
|
||||
echo "::group::Check 7/8: No bespoke var(--glass-surface-*) inline styles"
|
||||
echo "::group::Check 7/9: No bespoke var(--glass-surface-*) inline styles"
|
||||
# unify-glass-panel-surfaces convoy, Brief 7 (PR sequence after #126).
|
||||
# After Briefs 1-6 land, every panel-shaped surface in the app
|
||||
# composes via .glass-panel / .glass-panel-strong /
|
||||
|
|
@ -375,7 +375,7 @@ jobs:
|
|||
# Operator override: if you DO need to add a fourth always-billed
|
||||
# workflow (rare — discuss it in a convoy first), append the file
|
||||
# to RUNS_ON_ALLOWLIST below in the same PR.
|
||||
echo "::group::Check 8/8: No drift back to runs-on: ubuntu-latest outside agent-context-drift.yml"
|
||||
echo "::group::Check 8/9: No drift back to runs-on: ubuntu-latest outside agent-context-drift.yml"
|
||||
RUNS_ON_ALLOWLIST=(
|
||||
".github/workflows/agent-context-drift.yml"
|
||||
)
|
||||
|
|
@ -407,12 +407,32 @@ jobs:
|
|||
fi
|
||||
echo "::endgroup::"
|
||||
|
||||
# ---------- (9) No regression of visual-diff merge gate ----------
|
||||
# `harden-visual-diff-gate` brief 2 (PR #140) removed
|
||||
# `continue-on-error: true` from `visual-diff.yml`'s screenshot
|
||||
# capture step, making visual diff a real merge gate. This check
|
||||
# guards against the flag being re-introduced (intentionally or
|
||||
# by template-revert) and silently downgrading the gate to
|
||||
# advisory again.
|
||||
#
|
||||
# Scoped narrowly to visual-diff.yml — other workflows
|
||||
# legitimately use `continue-on-error: true` (e.g. preview-smoke
|
||||
# could choose to in a future advisory mode).
|
||||
echo "::group::Check 9/9: No continue-on-error in visual-diff.yml"
|
||||
if grep -qE '^\s*continue-on-error:\s*true' .github/workflows/visual-diff.yml 2>/dev/null; then
|
||||
echo "::error file=.github/workflows/visual-diff.yml::Forbidden \`continue-on-error: true\` re-introduced to visual-diff.yml. The screenshot-capture step is a hard merge gate post-harden-visual-diff-gate brief 2. To intentionally downgrade the gate, open a convoy and document the rollback in AGENTS.md § Testing § Screenshot diff."
|
||||
FAIL=1
|
||||
else
|
||||
echo "OK: visual-diff.yml has no continue-on-error: true."
|
||||
fi
|
||||
echo "::endgroup::"
|
||||
|
||||
# ---------- Final exit ----------
|
||||
if [ "$FAIL" -ne 0 ]; then
|
||||
echo "::error::One or more forbidden-pattern checks failed. See annotations above."
|
||||
exit 1
|
||||
fi
|
||||
echo "All 8 forbidden-pattern checks passed."
|
||||
echo "All 9 forbidden-pattern checks passed."
|
||||
|
||||
migrate:
|
||||
name: Migrations apply (node-pg-migrate)
|
||||
|
|
|
|||
68
.github/workflows/seed-visual-baselines.yml
vendored
68
.github/workflows/seed-visual-baselines.yml
vendored
|
|
@ -99,12 +99,34 @@ jobs:
|
|||
# `peter-evans/create-pull-request@v6` handles branch creation,
|
||||
# commit, push, and PR open/update in a single idempotent step. If
|
||||
# there are no changes to `tests/visual/__screenshots__/`, it
|
||||
# short-circuits and leaves no branch/PR behind. Uses the default
|
||||
# GITHUB_TOKEN (no PAT needed) — `permissions:` block above grants
|
||||
# `contents: write` + `pull-requests: write` which are sufficient.
|
||||
# short-circuits and leaves no branch/PR behind.
|
||||
#
|
||||
# KNOWN LIMITATION (encountered in workflow run 27454132468, first
|
||||
# invocation 2026-06-13): the PR-open sub-step fails with
|
||||
# "GitHub Actions is not permitted to create or approve pull requests"
|
||||
# because stwl-labs has the org-level "Allow GitHub Actions to
|
||||
# create and approve pull requests" setting disabled (Settings →
|
||||
# Actions → General → Workflow permissions). The branch IS pushed
|
||||
# successfully even when this fails — so the operator can open the
|
||||
# PR manually:
|
||||
#
|
||||
# gh pr create --base main \
|
||||
# --head bot/visual-baselines-<run_id> \
|
||||
# --title "chore(visual): refresh baselines from <url>" \
|
||||
# --body "..."
|
||||
#
|
||||
# `continue-on-error: true` on this step is intentional and scoped:
|
||||
# it lets the workflow as a whole succeed when the branch push
|
||||
# works but the PR-open step is blocked by the org setting. The
|
||||
# follow-up step below tells the operator exactly what to run.
|
||||
# This is NOT the same `continue-on-error` as the one we just
|
||||
# removed from `visual-diff.yml`'s screenshot capture (that one
|
||||
# silently hid real UI regressions; this one fronts a known org
|
||||
# limitation with a loud notice).
|
||||
- name: Open baseline-refresh PR
|
||||
id: cpr
|
||||
uses: peter-evans/create-pull-request@v6
|
||||
continue-on-error: true
|
||||
with:
|
||||
token: ${{ secrets.GITHUB_TOKEN }}
|
||||
branch: bot/visual-baselines-${{ github.run_id }}
|
||||
|
|
@ -151,12 +173,46 @@ jobs:
|
|||
add-paths: |
|
||||
tests/visual/__screenshots__/
|
||||
|
||||
- name: Report no changes
|
||||
if: steps.cpr.outputs.pull-request-number == ''
|
||||
# Three possible outcomes from the cpr step:
|
||||
# 1. No baseline changes → cpr.outputs.pull-request-number == '' AND no bot/ branch on origin
|
||||
# 2. PR opened successfully → cpr.outputs.pull-request-number != ''
|
||||
# 3. Branch pushed, PR-open blocked by org setting → cpr.outputs.pull-request-number == '' BUT bot/ branch exists on origin
|
||||
# The branch-exists query disambiguates outcomes 1 and 3.
|
||||
- name: Disambiguate outcome
|
||||
id: outcome
|
||||
if: always() && steps.cpr.outputs.pull-request-number == ''
|
||||
run: |
|
||||
BRANCH="bot/visual-baselines-${GITHUB_RUN_ID}"
|
||||
if git ls-remote --exit-code --heads origin "$BRANCH" >/dev/null 2>&1; then
|
||||
echo "kind=branch-pushed-pr-blocked" >> "$GITHUB_OUTPUT"
|
||||
echo "branch=$BRANCH" >> "$GITHUB_OUTPUT"
|
||||
else
|
||||
echo "kind=no-changes" >> "$GITHUB_OUTPUT"
|
||||
fi
|
||||
|
||||
- name: Report — no changes
|
||||
if: steps.outcome.outputs.kind == 'no-changes'
|
||||
run: |
|
||||
echo "::notice::No baseline changes detected. The captures from ${{ inputs.base_url }} match the committed baselines — no PR opened."
|
||||
|
||||
- name: Report opened PR
|
||||
- name: Report — PR opened
|
||||
if: steps.cpr.outputs.pull-request-number != ''
|
||||
run: |
|
||||
echo "::notice::Opened PR #${{ steps.cpr.outputs.pull-request-number }} with refreshed baselines: ${{ steps.cpr.outputs.pull-request-url }}"
|
||||
|
||||
- name: Report — branch pushed, manual PR-open required
|
||||
if: steps.outcome.outputs.kind == 'branch-pushed-pr-blocked'
|
||||
run: |
|
||||
BRANCH="${{ steps.outcome.outputs.branch }}"
|
||||
BASE_URL="${{ inputs.base_url }}"
|
||||
REASON="${{ inputs.reason }}"
|
||||
echo "::warning::Branch \`$BRANCH\` pushed with refreshed baselines, but PR-open was blocked by stwl-labs org setting (\"Allow GitHub Actions to create and approve pull requests\" is OFF)."
|
||||
echo ""
|
||||
echo "Operator: run this from your local checkout to open the baseline PR:"
|
||||
echo ""
|
||||
echo " gh pr create --base main --head $BRANCH \\\\"
|
||||
echo " --title \"chore(visual): refresh baselines from $BASE_URL\" \\\\"
|
||||
echo " --body \"Refresh baselines (run $GITHUB_RUN_ID). Reason: $REASON\""
|
||||
echo ""
|
||||
echo "See AGENTS.md § Testing § Screenshot diff for the full operator runbook."
|
||||
exit 1
|
||||
|
|
|
|||
16
.github/workflows/visual-diff.yml
vendored
16
.github/workflows/visual-diff.yml
vendored
|
|
@ -118,12 +118,26 @@ jobs:
|
|||
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 }}
|
||||
continue-on-error: true
|
||||
|
||||
- name: Upload screenshots + diffs
|
||||
if: always()
|
||||
|
|
|
|||
|
|
@ -166,11 +166,14 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
|
|||
|
||||
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`.
|
||||
**Current baseline:** refreshed against post-glass-redesign `main` via PR #139 (`54495fe`, 2026-06-13), generated on CT 111 against the `c100c5f` production deployment. Diff is a hard merge gate post-`harden-visual-diff-gate` brief 2 — see the next bullet.
|
||||
- **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` (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.
|
||||
- **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`. **It is now a hard merge gate** post-`harden-visual-diff-gate` brief 2 (PR #140, 2026-06-13) — `continue-on-error: true` was removed. A failed diff blocks merge.
|
||||
- **Intentional UI change?** Dispatch the seeding workflow first: `gh workflow run seed-visual-baselines.yml -f base_url=<preview-url> -f reason="..."`. It pushes a `bot/visual-baselines-<run_id>` branch with a refreshed `home.png`. The auto-PR-open step **fails** because stwl-labs has "Allow GitHub Actions to create and approve pull requests" disabled at the org level (Settings → Actions → General → Workflow permissions); the operator runs `gh pr create --base main --head bot/visual-baselines-<run_id> ...` manually. Merge the baseline PR, then re-run the UI-touching PR's visual diff.
|
||||
- **Unintentional regression?** Open the run's artifact bundle, inspect the diff PNG, fix the regression in source, push.
|
||||
- **Forbidden re-introduction:** `ci.yml`'s `forbidden-patterns` job's 9th check fails any PR that re-adds `continue-on-error: true` to `visual-diff.yml`.
|
||||
- **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.
|
||||
|
|
|
|||
|
|
@ -11,31 +11,45 @@ import { test, expect } from '@playwright/test';
|
|||
* `tests/visual/__screenshots__/home.png` (seeded by PR #58 `83a358b`,
|
||||
* 2026-06-02).
|
||||
*
|
||||
* NOTE: as of this writing, `visual-diff.yml`'s screenshot capture step
|
||||
* still carries `continue-on-error: true` — failures surface as
|
||||
* artifacts + PR comments but do not gate merge. Flipping that to a
|
||||
* 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).
|
||||
* Visual diff is a HARD MERGE GATE post-`harden-visual-diff-gate` brief 2
|
||||
* (PR #140). A failed diff blocks merge; there is no `continue-on-error`
|
||||
* escape valve. If the failure is intentional (real UI change), the
|
||||
* operator must dispatch the baseline-refresh workflow before merging
|
||||
* — see the section below.
|
||||
*
|
||||
* 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:
|
||||
* Always run on Linux — Mac-generated PNGs do not match CT 111 Linux
|
||||
* output because `playwright.config.js`'s `snapshotPathTemplate` has
|
||||
* no `{platform}` token. Two paths:
|
||||
*
|
||||
* docker run --rm -v "$PWD":/work -w /work \
|
||||
* mcr.microsoft.com/playwright:v1.60.0-noble \
|
||||
* sh -c "npm ci && BASE_URL=https://<preview>.vercel.app \
|
||||
* VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
|
||||
* npm run test:visual:update"
|
||||
* 1. **Dispatch the seed-visual-baselines workflow (recommended).**
|
||||
*
|
||||
* …or dispatch the (queued) `seed-visual-baselines` workflow on
|
||||
* CT 111 directly, which uses the same Linux toolchain as the
|
||||
* diff workflow so byte-equivalence is guaranteed.
|
||||
* gh workflow run seed-visual-baselines.yml \
|
||||
* -f base_url=<production-or-preview-vercel-url> \
|
||||
* -f reason="<short reason>"
|
||||
*
|
||||
* Commit the regenerated `tests/visual/__screenshots__/home.png`.
|
||||
* The workflow runs on the same CT 111 axiom runner as
|
||||
* visual-diff.yml, so the captured PNG is byte-equivalent. It
|
||||
* pushes the new baseline to a `bot/visual-baselines-<run_id>`
|
||||
* branch. The auto-PR-open step requires the org-level "Allow
|
||||
* GitHub Actions to create and approve pull requests" setting to
|
||||
* be enabled (currently OFF for stwl-labs); if it fails, push
|
||||
* the branch is fine and the operator opens the PR manually via:
|
||||
*
|
||||
* gh pr create --base main \\
|
||||
* --head bot/visual-baselines-<run_id> \\
|
||||
* --title "chore(visual): refresh baselines ..."
|
||||
*
|
||||
* 2. **Playwright Docker image (host-independent).** Slower; useful
|
||||
* if CT 111 is offline:
|
||||
*
|
||||
* docker run --rm -v "$PWD":/work -w /work \\
|
||||
* mcr.microsoft.com/playwright:v1.60.0-noble \\
|
||||
* sh -c "npm ci && BASE_URL=https://<preview>.vercel.app \\
|
||||
* VERCEL_AUTOMATION_BYPASS_SECRET=<value> \\
|
||||
* npm run test:visual:update"
|
||||
*
|
||||
* Then commit the regenerated `tests/visual/__screenshots__/home.png`.
|
||||
*/
|
||||
const BASE = process.env.BASE_URL ?? 'http://localhost:3000';
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue