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>
* convoy: migrate CI to self-hosted axiom runners (briefs 1+2)
Moves 4 of 5 GitHub Actions workflows from `ubuntu-latest` to the new
`stwl-labs` org-level self-hosted pool (CT 111 axiom-runner-1..4) and
rewires the `migrate` job to use CT 102's shared Postgres via per-run
databases.
Changes:
- ci.yml: lint, schema-map-fresh, forbidden-patterns, migrate, test ->
`[self-hosted, axiom]`. migrate job drops `services.postgres` (saved
~30s/run of image pull) and switches to `HOMELAB_CI_POSTGRES_BASE_URL`
secret + per-run DB (`ci_run_<run_id>_<run_attempt>`) with `always()`
cleanup so failed migrations don't leak DBs.
- preview-smoke.yml: gate + smoke -> self-hosted. Playwright browser
cache lives under /opt/appdata/gha-runner/shared-cache/playwright on
the host bind mount; first PR primes it, subsequent runs reuse.
- visual-diff.yml: gate + visual -> self-hosted (same Playwright cache).
- pr-health-rollup.yml: rollup -> self-hosted.
- agent-context-drift.yml: deliberately LEFT on ubuntu-latest (D4 in
convoy doc). Weekly cron stays GitHub-hosted so it runs even when
axiom is down.
Why on this side and not the runner side:
- migrate adds an explicit `sudo apt-get install -y postgresql-client`
step (~10s, amortized via apt-cache survival). The runner image
doesn't ship psql; baking it in would require a custom image and
doesn't earn its keep for one job.
Repo prereqs (set before this PR opens):
- `HOMELAB_CI_POSTGRES_BASE_URL` repo secret set (value pattern:
`postgres://deckhearth_ci:<pw>@192.168.68.102:5432`)
- `deckhearth_ci` Postgres user created on CT 102 with CREATEDB,
no superuser
- stwl-labs org Actions settings: "Require approval for all outside
collaborators" + runner group rejects public repos
- 4 runners online: `axiom-runner-1..4`, status Idle
Follow-ups (per convoy):
- Brief 3: forbidden-pattern gate to catch `runs-on: ubuntu-latest`
re-introduction outside the agent-context-drift allowlist
- Brief 4: AGENTS.md updates + 1-line revert path (D5)
- Weekly cron on CT 102 to GC any `ci_run_*` DBs older than 7d
(Risk #4 mitigation)
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(migrate): use PGHOST/PGUSER/PGPASSWORD instead of URL secret
First Brief 1+2 validation run failed on the migrate job with
`psql: invalid option -- '/'` despite the secret being set correctly
and a direct CT-111 → CT-102 psql connection working fine. The
URL-parse path in `psql "$PGBASE/postgres"` was the fragile bit.
Splitting the connection into discrete `PG*` env vars (which psql
picks up automatically) sidesteps URL parsing entirely. The
`HOMELAB_CI_POSTGRES_BASE_URL` repo secret is now
`HOMELAB_CI_POSTGRES_PASSWORD` — password only — and the workflow
hardcodes the (non-sensitive) host/port/user. `node-pg-migrate`
still reads `POSTGRES_URL` from `.env.local`, so we assemble that
URL inline for it; the runner is ephemeral so the leaked-to-disk
password is bounded to one job.
Convoy doc updated to reflect the shipped approach + lesson learned
in prerequisites.
Co-authored-by: Cursor <cursoragent@cursor.com>
* ci: trigger vercel preview after stwl-labs reauth
Co-authored-by: Cursor <cursoragent@cursor.com>
* ci: re-test playwright after vercel project rebind
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Standalone infrastructure PR (no convoy ceremony needed — single-file
scope). Triggered by the GitHub Actions billing block that gated
PRs #124 + #125 today.
Three layers of savings applied per the user's max-savings option:
1. paths-ignore on ci.yml + preview-smoke.yml
- Doc-only PRs (.convoys/**, **/*.md, docs/**, AGENTS.md,
.cursor/**, README.md) now trigger ZERO Actions jobs.
- Vercel still builds (it's not on the Actions billing).
- visual-diff.yml unchanged — it was already cost-conscious via a
positive paths: allowlist (pages/**, components/**, styles/**,
etc.).
2. Consolidate 6 grep-only forbidden-* jobs into 1
- Previously 6 independent jobs each ran their own
actions/checkout (~3s × 6 = 18s of redundant checkout).
- Merged into a single forbidden-patterns job with 6 sequential
::group:: sections, one FAIL flag at the bottom — preserves
"see all violations in one run" diagnostic behavior. Per-file
::error file=...::msg annotations work the same way.
- Removed jobs: forbidden-endpoints, forbidden-cors-headers,
forbidden-client-side-llm-keys,
forbidden-modal-shell-without-primitive,
forbidden-deprecated-color-aliases, forbidden-stale-strings.
- pr-health-rollup.yml only looks up "Lint" and "Schema map up to
date" by name — unaffected.
3. Cache node_modules + Playwright browsers
- actions/cache@v4 for node_modules keyed by package-lock.json
hash, applied to lint / test / migrate / preview-smoke /
visual-diff. Cuts npm ci from ~30-45s to ~3-5s on cache hit.
setup-node@v4's built-in cache: npm stays (caches ~/.npm) —
both layered.
- actions/cache@v4 for ~/.cache/ms-playwright keyed by the
resolved @playwright/test version. Cache invalidates on any
Playwright version bump. On cache hit, only system deps install
runs (npx playwright install-deps chromium) — saves ~15-25s/run.
AGENTS.md updates:
- § 6 Testing § CI behavior: appended "CI minute optimizations" subsection
documenting all three layers.
- § Product vocabulary table caption: updated "CI job
forbidden-stale-strings" reference to "CI check Forbidden patterns
(6 checks) → Check 6/6" with a historical pointer.
- Gotcha #5: updated the standalone forbidden-endpoints reference
similarly.
Estimated savings per typical convoy mix (~30% doc PRs based on
repo history):
- Doc-only PRs: 100% reduction (was ~6-7 min, now 0 Actions min).
- Code-touching PRs: ~30-50% reduction (cache hits for npm + Playwright +
no redundant 6× checkout).
- Weighted average: ~50-60% reduction.
This is short of the 70-80% I floated in chat — the real ceiling is
limited by lint / vitest / migrate / Playwright runtime itself, all of
which are kept on code-touching PRs (they're high-signal).
Verification:
- All 3 workflow YAMLs parse (python3 -c "yaml.safe_load(...)").
- npm run lint passes (1 pre-existing unrelated warning).
- npm run test:run: 118/118 tests pass.
- forbidden-patterns logic is byte-equivalent to the 6 original jobs'
bash bodies — the differences are: per-check ::group::/::endgroup::
framing, a shared FAIL flag instead of per-job exit 1, and renamed
local arrays (FOUND → LLM_FOUND / MODAL_FOUND / STRING_FOUND) to
avoid clobbering across the single job's scope.
Co-authored-by: Cursor <cursoragent@cursor.com>
The `Screenshot diff` workflow's `paths:` filter included `pages/**`
which matches `pages/api/**` too, so API-only PRs triggered the
visual-diff workflow even though they can't possibly move a single
rendered pixel. PR #19 (cors-tighten) and PR #20 (add-rate-limiting)
both empirically hit this — each was an API-only sweep, and each
burned ~55s of CI runtime on a `Screenshot diff` job that
`continue-on-error: true` then swallowed. Documented as a queued
follow-up in `.convoys/ship-readiness.md` § Queued convoys, ratified
for fix in this convoy.
The fix is a single negated-glob entry inserted immediately after
`pages/**` in the `paths:` list. GitHub Actions evaluates `paths:`
with minimatch and supports `!`-prefixed exclusions per the
published path-filter cheatsheet, but the order matters: a `!pattern`
only takes effect if it appears AFTER an include that already
matched the path. Keeping `!pages/api/**` second in the list (right
after `pages/**`, before all the other includes) is the canonical
shape. All five existing entries are preserved verbatim; only the
one exclusion entry plus an inline comment explaining the ordering
rule and the empirical motivation is added.
`preview-smoke.yml` is intentionally untouched — verified its `on:`
block has no `paths:` filter at all (it triggers on every PR
targeting main, with skip-via-PR-body-directive in the gate job),
so there's no false-positive shape to fix there. Smoke SHOULD run on
every PR including API-only ones because changes to `pages/api/**`
can break the home redirect + sign-in + `/api/health` endpoints the
smoke spec exercises.
Co-authored-by: Cursor <cursoragent@cursor.com>
* architect: plan fix-vercel-deployment-protection-in-ci convoy (1 brief)
Flip convoy status queued -> in-progress and append the architect
output per the role-architect contract:
- Decisions (post-IA round) A/B/C/D with recommendations + rationale,
routed back to operator for gate-1 ratification (A and D especially).
- Architecture section: file plan, risk list (R1-R8), test plan,
decomposition (1 brief, justified), slice_dependencies block.
- brief-1: inject VERCEL_AUTOMATION_BYPASS_SECRET into both workflows
via query-param-on-path (Decision A), tighten max_timeout 600 -> 120
(Decision B), extend gate to skip fork PRs (Decision D), and forward
the secret as an env var to the Playwright/visual step for forward-
compat with adopt-playwright-smoke.
Boot-the-brief findings preempted in the brief:
- wait-for-vercel-preview@v1.3.2 source confirms only `targetUrl` (the
bare deployment URL) is ever logged or emitted as outputs.url; the
bypass query in `path:` is structurally invisible to logs and to
downstream consumers of ${{ steps.vercel.outputs.url }}.
- The action exposes no custom-header input -- Option B is mechanically
impossible for the wait step without forking the action.
- `path:` MUST begin with a leading `/` because the action parses it
via `new URL(path, url)`.
- PR #16 run logs confirm the 401 timeout failure mode the convoy targets.
- Playwright config doesn't exist yet (adopt-playwright-smoke owns it);
this brief only plumbs the env var.
No workflow YAML, no Playwright config, no AGENTS.md changes in this
commit -- those land in the implementer phase per the architect contract.
Awaiting human gate 1 (Decisions A + B + D ratification + brief approval)
before implementer dispatch.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): plumb VERCEL_AUTOMATION_BYPASS_SECRET into preview-smoke + visual-diff (Brief 1 of fix-vercel-deployment-protection-in-ci)
Closes the CI-infra half of P0 #7's follow-up. PR #16 (squash commit
7e97254) added scoped permissions to both workflows but exposed that
Vercel Deployment Protection 401s anonymous GitHub-runner requests,
causing both Playwright smoke and Screenshot diff to time out at 10
minutes on every PR. This brief plumbs the bypass secret end-to-end
so the wait-action's healthcheck reaches 200.
Per architect Decision A (.convoys/fix-vercel-deployment-protection-in-ci.md):
- preview-smoke.yml + visual-diff.yml: wait-for-vercel-preview's
`path:` input now carries the bypass as a query parameter
`?x-vercel-protection-bypass=${{ secrets.* }}&x-vercel-set-bypass-cookie=true`.
The action only logs the bare targetUrl (verified in action.js:357,360,363)
so the secret stays out of workflow logs.
Per Decision B:
- max_timeout: 600 -> 120. PR #16 evidence shows Vercel previews are up
within seconds of job start; 120s gives ample headroom and surfaces
misconfigurations in ~2 minutes instead of ~10.
Per Decision D (NEW -- surfaced by Boot-the-brief):
- gate: job's Decide step now checks github.event.pull_request.head.repo.fork
FIRST. Forks lack repo secrets, so they would otherwise burn ~4 minutes
per PR on a misleading 401. The fork-check emits `::notice::` and short-
circuits before the existing skip-via-PR-body directive runs.
Forward-compat for adopt-playwright-smoke:
- Both workflows' Playwright/screenshot-capture step now exports
VERCEL_AUTOMATION_BYPASS_SECRET as env. The actual Playwright config
consumes it via extraHTTPHeaders in adopt-playwright-smoke's brief.
BASE_URL stays as the bare \${{ steps.vercel.outputs.url }} (no query
string) so it remains safe to echo.
Verification:
- YAML parses (js-yaml load on both files: preview-smoke jobs [gate, smoke];
visual-diff jobs [gate, visual])
- actionlint not run (binary not installed locally); recommend installing
for future PRs. Future adopt-actionlint convoy could add it to CI.
- No `set -x`, `echo`, `cat`, or `printf` of the secret or bypass URL
in any modified step
- permissions: and concurrency: blocks unchanged (PR #16 contracts preserved)
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): route github.event.pull_request.body through env: to avoid shell injection
Pre-existing latent bug surfaced by PR #17's CI run. The Decide step's
inline `${{ github.event.pull_request.body }}` interpolation pastes
arbitrary PR-body text directly into a bash script. When the body
contains shell metacharacters (parens, pipes, backticks, redirections,
etc.) the resulting script either errors out at YAML-load time OR —
worse — executes attacker-controlled shell.
This bit PR #17 with a real syntax error ("unexpected token `('")
because the PR body contains parenthesized phrases like
"(was: 10-minute timeout)". Every Decide-step run in this repo has
been one badly-formatted PR body away from breaking the gate.
Fix: forward `github.event.pull_request.body` and
`github.event.pull_request.head.repo.fork` through the step's `env:`
block as `PR_BODY` and `PR_IS_FORK`, then quote them in shell
(`"$PR_BODY"`, `"$PR_IS_FORK"`). The env-var path leaves the values
as plain strings rather than syntactically embedded code, which is the
standard GitHub Actions hardening pattern (see GitHub's "Security
hardening for GitHub Actions" → "Using a third-party action").
Same change in both workflows; ~9 LOC each.
This fix is technically beyond Brief 1's scope (which targeted only
Vercel-bypass plumbing) but is added in this convoy because the bug
actively blocks Brief 1's success criterion from being validated on
PR #17. Documented in the convoy file's "Anything flagged but not
acted on" follow-up pass.
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(ci): drop x-vercel-set-bypass-cookie=true from wait-action path
The wait-action's healthcheck was still 401ing despite the bypass query
being correct. Root cause: with `x-vercel-set-bypass-cookie=true`, Vercel
returns 307 + Set-Cookie (`_vercel_jwt`), but axios in Node has no cookie
jar — it follows the redirect to the bare URL without the cookie, which
then 401s.
Local verification (run by operator):
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>" | head -1
→ HTTP/2 200 (works, no cookie needed)
curl -sI "https://<preview>/?x-vercel-protection-bypass=<secret>&x-vercel-set-bypass-cookie=true" | head -1
→ HTTP/2 307 (the redirect-without-cookie path that breaks axios)
For a one-shot healthcheck, the per-request bypass query is enough.
The cookie variant stays reserved for the future Playwright config
(adopt-playwright-smoke) where a real browser cookie jar exists.
Added an inline comment in preview-smoke.yml explaining this so the
next agent doesn't accidentally re-add the cookie param.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Cursor <cursoragent@cursor.com>
Both Playwright-on-Vercel workflows fail at the very first action
(`patrickedqvist/wait-for-vercel-preview@v1.3.2`) with 403 "Resource
not accessible by integration". The cause is the repo-default workflow
token being read-only-by-default with no scopes declared by the workflow.
Add minimal scoped permission blocks per the GitHub Actions least-privilege
guidance:
preview-smoke.yml:
contents: read
deployments: read # wait-for-vercel-preview queries GitHub Deployments
pull-requests: read # correlate deployment with this PR
statuses: read # some Vercel deployments use commit statuses
visual-diff.yml:
contents: read
deployments: read
pull-requests: write # final step posts "Visual Diff" comment via Issues API
statuses: read
Verified against the failure on PR #15:
- Playwright smoke: "Resource not accessible by integration" on
wait-for-vercel-preview → fixed by deployments+statuses+pull-requests:read
- Screenshot diff: 403 from POST /repos/.../issues/15/comments with
`x-accepted-github-permissions: issues=write; pull_requests=write`
in the response → fixed by pull-requests:write (covers Issues API
for PR comments; issues:write would also work but pull-requests:write
is the idiomatic scope)
Unblocks visual-regression signal on every future PR. No code changes,
no test changes — workflow YAML only.
Co-authored-by: Cursor <cursoragent@cursor.com>
Randall Stillwell