From ac91d73a342ca2c022b9fcc484fc72ee079320d4 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Fri, 12 Jun 2026 21:58:54 -0500 Subject: [PATCH] convoy: flip visual-diff to a hard merge gate (harden-visual-diff-gate brief 2/2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .convoys/.metrics.jsonl | 2 + .convoys/harden-visual-diff-gate.md | 65 +++++++++++++++----- .github/workflows/ci.yml | 40 +++++++++--- .github/workflows/seed-visual-baselines.yml | 68 +++++++++++++++++++-- .github/workflows/visual-diff.yml | 16 ++++- AGENTS.md | 7 ++- tests/visual/homepage.spec.ts | 54 ++++++++++------ 7 files changed, 198 insertions(+), 54 deletions(-) diff --git a/.convoys/.metrics.jsonl b/.convoys/.metrics.jsonl index 7c0a0e2..5f32388 100644 --- a/.convoys/.metrics.jsonl +++ b/.convoys/.metrics.jsonl @@ -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"} diff --git a/.convoys/harden-visual-diff-gate.md b/.convoys/harden-visual-diff-gate.md index 9a4ce00..6c550c3 100644 --- a/.convoys/harden-visual-diff-gate.md +++ b/.convoys/harden-visual-diff-gate.md @@ -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 | diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3ac5391..270ed7f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 primitive" + echo "::group::Check 4/9: No new modal shells without 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) diff --git a/.github/workflows/seed-visual-baselines.yml b/.github/workflows/seed-visual-baselines.yml index 9c4bdc8..2532a25 100644 --- a/.github/workflows/seed-visual-baselines.yml +++ b/.github/workflows/seed-visual-baselines.yml @@ -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- \ + # --title "chore(visual): refresh baselines from " \ + # --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 diff --git a/.github/workflows/visual-diff.yml b/.github/workflows/visual-diff.yml index 40f7f4c..d86923d 100644 --- a/.github/workflows/visual-diff.yml +++ b/.github/workflows/visual-diff.yml @@ -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() diff --git a/AGENTS.md b/AGENTS.md index fa04cb4..ada31e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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= -f reason="..."`. It pushes a `bot/visual-baselines-` 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- ...` 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. diff --git a/tests/visual/homepage.spec.ts b/tests/visual/homepage.spec.ts index 8d6d220..177b6c7 100644 --- a/tests/visual/homepage.spec.ts +++ b/tests/visual/homepage.spec.ts @@ -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://.vercel.app \ - * VERCEL_AUTOMATION_BYPASS_SECRET= \ - * 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= \ + * -f 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-` + * 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- \\ + * --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://.vercel.app \\ + * VERCEL_AUTOMATION_BYPASS_SECRET= \\ + * npm run test:visual:update" + * + * Then commit the regenerated `tests/visual/__screenshots__/home.png`. */ const BASE = process.env.BASE_URL ?? 'http://localhost:3000'; -- 2.45.2