deckhearth/.convoys/tighten-visual-diff-path-filter.md

tighten-visual-diff-path-filter (P3 polish — single-file YAML tweak)

Status: OPEN 2026-05-26 (this PR) Priority: P3 polish (CI-cost / signal-noise; not a security or correctness issue — Screenshot diff already swallows its own "snapshot doesn't exist" failure via continue-on-error: true, so the false-trigger is wasted runtime + a slightly chatty checks tab, nothing more) Convoy owner: parent (no architect — single-file workflow YAML tweak following published GitHub Actions path-filter semantics) Opened: 2026-05-26 Branch: convoy/tighten-visual-diff-path-filter

Background

.github/workflows/visual-diff.yml is supposed to fire only on UI changes. Its current paths: filter:

paths:
  - 'pages/**'
  - 'components/**'
  - 'styles/**'
  - 'tailwind.config.js'
  - 'postcss.config.js'

But pages/** matches pages/api/** too, and the tcg-vault repo's API surface lives entirely under pages/api/ (Next.js Pages router). That means API-only PRs trigger the visual-diff workflow even though they can't possibly move a single rendered pixel. The empirical false-positives:

• PR #19 cors-tighten (squash da50d78, 2026-05-24) — touched 24 files all under pages/api/**. Triggered Screenshot diff, documented in .convoys/ship-readiness.md § P0 #5 As-shipped metrics: "Screenshot diff — workflow exited 0 because of continue-on-error: true, but the actual visual test failed with the documented 'snapshot doesn't exist' error… Triggered on PR #19 despite this being API-only because its paths: filter is pages/** which matches pages/api/** too — minor false-positive queued as tighten-visual-diff-path-filter."
• PR #20 add-rate-limiting (squash 708ef45, 2026-05-24) — touched 6 routes under pages/api/ (plus lib/rate-limit.js and pages/admin/card-import.js). Same false-trigger, same swallow. Documented in .convoys/ship-readiness.md § P0 #6 As-shipped metrics.

Cost per false-trigger: ~55s of CI runtime (the wait-for-vercel-preview step hits its 120s budget against the deployed preview, then Playwright npm ci + npx playwright install + the visual project runs — even though the test ultimately can't compare against a non-existent baseline). The Screenshot diff job exits 0 because of continue-on-error: true (Decision-4 end state of adopt-playwright-smoke, until seed-visual-baselines-on-linux lands), but it still posts a "Visual Diff — view run" comment and clutters the PR Checks tab with a green-but-meaningless run.

Design decision — negated-glob !pages/api/**

GitHub Actions evaluates paths: with minimatch and supports !-prefixed exclusion patterns per the official path-filter cheatsheet. Order matters: a !pattern only takes effect if it comes AFTER an include that already matched the path. So the canonical shape is:

paths:
  - 'pages/**'
  - '!pages/api/**'   # must follow 'pages/**' to subtract from it
  - 'components/**'
  ...

Considered alternatives:

1. Per-feature paths — replace pages/** with explicit subdirectory globs (pages/!(api)/** or pages/dashboard.js, pages/cards/**, pages/decks/**, …). Rejected: too verbose, needs to be touched every time a top-level page is added, defeats the "trigger on UI changes" intent.
2. Extglob pages/!(api)/** — would work in bash with extglob enabled, but minimatch's default options used by GitHub Actions do NOT enable extglob without a flag we can't set from YAML. The queue entry explicitly flagged this risk; the negated-glob shape is the safer documented path.
3. Move the gate into the gate: job — add a step that diffs pages/api/** and sets should_run=false if every changed file is API-only. Rejected: more code, more surface, doesn't actually fire faster (the gate job itself spins up a runner). The native paths: filter short-circuits BEFORE any runner spins up, which is the cheapest possible exclusion.

The simple negation is sufficient and matches GitHub's published guidance.

The fix

Single edit in .github/workflows/visual-diff.yml. Insert !pages/api/** immediately after pages/**, with an inline comment explaining the ordering rule and the empirical motivation:

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'

All five existing entries are preserved; only the one exclusion entry is added.

.github/workflows/preview-smoke.yml — left untouched

Verified the sibling workflow's shape:

on:
  pull_request:
    branches: [main]
    types: [labeled, opened, synchronize, reopened]

preview-smoke.yml has no paths: filter at all — it triggers on every PR targeting main (modulo the in-job gate: skip via pipeline: skip smoke in the PR body). This is intentional: a smoke test that hits the home redirect, the sign-in page, and /api/health SHOULD run on every PR including API-only ones, because changes to pages/api/** can break those routes too. There is no false-positive shape to fix here. Leaving preview-smoke.yml strictly out of scope.

Verification plan

1. YAML parse — python3 -c "import yaml; ..." confirms the paths: list deserializes to the expected 6-entry list with '!pages/api/**' at index 1 (immediately after 'pages/**'). Done at gate time, see § Acceptance criteria.

2. npm run lint — exits 1 with 128 problems (baseline preserved, no regression). YAML files don't go through ESLint; verification here is just that we didn't accidentally edit a .js source file.

3. npm run test:run — 21/21 pass. YAML changes don't touch any test surface; verification only.

4. Post-merge CI behavior verification — DEFERRED. The only true verification that the !pages/api/** exclusion actually fires the way we expect is observing the next API-only PR after this merges and confirming Screenshot diff does NOT appear in its Checks tab. We document this explicitly here so the doc-writer pass that closes the convoy can record the next API-only PR's number + a "Screenshot diff: not triggered" line as the as-shipped success criterion (mirroring the fix-reset-db-script convoy's Screenshot-diff-not-triggered line in its as-shipped block).

   We do NOT attempt to live-verify the path filter at convoy time (e.g. by pushing a throwaway API-only commit to a sacrificial branch and watching CI). That'd be theater — GitHub's path-filter semantics are documented and stable, and the YAML parse + the syntax match against the published cheatsheet is enough pre-merge confidence for a P3 polish convoy.

Acceptance criteria

• python3 -c "import yaml; d=yaml.safe_load(open('.github/workflows/visual-diff.yml')); print(d[True]['pull_request']['paths'])" → ['pages/**', '!pages/api/**', 'components/**', 'styles/**', 'tailwind.config.js', 'postcss.config.js'] (order-sensitive)
• npm run lint → exit 1 with 128 problems (baseline preserved)
• npm run test:run → 21/21 pass
• .github/workflows/preview-smoke.yml unchanged in this PR's diff
• No other workflow files touched

Risks

• R1 — minimatch syntax compatibility. GitHub Actions uses minimatch internally; the ! prefix at the start of a pattern is documented as the canonical exclusion syntax. If for any reason Actions rejects this shape on the next workflow load (unlikely — this exact shape is shown in the published cheatsheet), the workflow would either fail to register OR silently treat the ! pattern as a literal include. Fallback: restructure to per-feature path globs (pages/dashboard.js, pages/cards/**, pages/decks/**, pages/deck/**, pages/deck-builder.js, pages/scanner.js, pages/profile.js, pages/settings.js, pages/login.js, pages/register.js, pages/admin/**, pages/collection/**, pages/collections.js, pages/community/**, pages/invite/**, pages/my-cards.js, pages/card/**, pages/_app.js, pages/_document.js, pages/_error.js, pages/index.js). More verbose, but unambiguously valid. Track as a follow-up convoy ONLY if the post-merge verification step (next API-only PR) shows the exclusion didn't fire.
• R2 — future pages/<non-api>/<api-like> subdir. If someone later adds a directory like pages/server/** that contains both API-style endpoints AND visual UI pages, the simple !pages/api/** exclusion would not catch it, and visual-diff would fire on changes to that directory. Documented but accepted: the repo convention for the foreseeable future is "all backend lives under pages/api/**", and the only realistic alternative ("server components" or similar) would warrant its own paths-filter revisit at that point. R2 is a "watch this space" risk, not a blocker.

Scope

• In scope: .github/workflows/visual-diff.yml only (plus this convoy planning doc).
• Out of scope: any other workflow file. Verified preview-smoke.yml has no paths: filter and intentionally fires on every PR, so no mirror-fix is needed there.

Why no architect

This is a single-file YAML tweak following published vendor documentation. No new precedents; no new decisions; the queue entry in .convoys/ship-readiness.md § Queued convoys already ratified the negated-glob direction. Parent applies the fix, runs the bounded checks (YAML parse + lint baseline + vitest), opens the PR. If anything surprising surfaces (the YAML doesn't parse, minimatch rejects the syntax), the parent stops and dispatches an architect mid-execution.

Out of scope (queued follow-ups)

• seed-visual-baselines-on-linux (priority: P3 polish; was already queued by adopt-playwright-smoke) — once visual baselines are seeded under tests/visual/__screenshots__/ from a Linux runner (or the documented Playwright Docker container), the Screenshot diff job will start posting real visual-diff comparisons and continue-on-error: true can be removed. This convoy's path-filter tightening is orthogonal to baseline seeding — both are needed eventually, but neither blocks the other. Surfaced in AGENTS.md § 6 Testing.

As-shipped

Single squash commit ba95462 (PR #26, merged 2026-05-27T03:51:22Z UTC / local 2026-05-26). Parent-owned end-to-end per the convoy spec — no architect, no implementer subagent dispatched. Mirror-the-pattern fix exactly as planned; no mid-execution surprises that would have forced an architect bounce.

Diff: 2 files, +279 / -0. .github/workflows/visual-diff.yml +N for the one !pages/api/** entry + the inline comment block explaining the ordering rule and naming the empirical PRs that motivated the fix (#19 cors-tighten, #20 add-rate-limiting); .convoys/tighten-visual-diff-path-filter.md +M for the full planning document, committed atomically with the YAML edit.

The change shipped exactly as designed. The paths: list now deserializes to ['pages/**', '!pages/api/**', 'components/**', 'styles/**', 'tailwind.config.js', 'postcss.config.js'] (order-sensitive — GitHub Actions evaluates paths: with minimatch and !-prefixed exclusions only fire after a prior include matches; keeping !pages/api/** immediately after pages/** is what makes the exclusion work). All five existing entries preserved verbatim; preview-smoke.yml intentionally untouched (no paths: filter; fires on every PR by design — see § The fix § preview-smoke.yml left untouched).

Verification at merge (all gates green):

• YAML parse: paths: deserializes to the expected 6-entry list with '!pages/api/**' at index 1 (immediately after 'pages/**')
• npm run lint → 128 problems (baseline at convoy time; later improved to 125 by PR #31 single-auth-provider)
• npm run test:run → 21/21 pass
• All pre-existing CI gates green at merge

Post-merge success criterion — CONFIRMED. The deferred verification from § Verification plan: the next API-only PR after this merges should NOT show Screenshot diff in its Checks tab. PR #30 (single-sql-client, squash c403ea4, merged 2026-05-27T03:54:01Z UTC) was the first API-only PR post-merge. Its diff touched pages/api/auth-utils.js + test/api/auth-utils.test.js + lib/database.js (deletion) + .convoys/single-sql-client.md — the pages/api/** portion matched pages/** but was correctly subtracted by the new !pages/api/** exclusion. Screenshot diff did NOT appear in PR #30's Checks tab — the empirical confirmation that the exclusion fires as documented. This is the as-shipped success line the convoy file's § Verification plan asked the doc-writer pass to record, mirroring .convoys/fix-reset-db-script.md's "Screenshot diff: not triggered (script-only PR — paths: filter excludes scripts/**…)" line.

Operator action required going forward: none. Path-filter behavior is self-defending; no env vars, no secrets, no infra changes.

Spec deviation: none.

No follow-up surfaced. The orthogonal seed-visual-baselines-on-linux follow-up was already queued by adopt-playwright-smoke; this convoy didn't add or change anything around it. The R1 fallback if exclusion didn't fire → restructure to per-feature path globs contingency from the convoy file was NOT exercised because the post-merge verification on PR #30 confirmed the negated-glob fired correctly.

Owns

Parent (single-file proven-pattern fix; no architect or implementer subagent required).