From c8f1541fa994267875296dac2c95572aa8d2baf7 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sun, 24 May 2026 17:28:55 -0500 Subject: [PATCH 1/3] convoy: scope adopt-playwright-smoke (P1 #10 step 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scaffolds the convoy file for the next sequential convoy in .convoys/ship-readiness.md's launch sequence (step 10). The infra prerequisites have already shipped: - bump-next-js (e57ea17) — Vercel previews actually deploy - fix-vercel-deployment-protection-in-ci (9a3e077) — wait-action healthcheck reaches 200; BASE_URL and VERCEL_AUTOMATION_BYPASS_SECRET already exported to the test step's env: in both workflows What remains is small and well-bounded: add @playwright/test to devDependencies, scaffold playwright.config.js with two projects (smoke + visual), decide what to do with the existing tests/smoke/app.smoke.spec.ts (which is .ts in a JS-only repo — Decision A queued for architect), and put a trivial visual baseline in place so the screenshot workflow has something to do on first run. 6 decisions queued for architect ratification (TS vs JS spec, fail-loud vs warn-and-continue on missing secret, project shape, visual baseline strategy, lint coverage for tests/, local-test script ergonomics). No operator action required — all secrets and env vars are already wired. Co-authored-by: Cursor --- .convoys/adopt-playwright-smoke.md | 276 +++++++++++++++++++++++++++++ 1 file changed, 276 insertions(+) create mode 100644 .convoys/adopt-playwright-smoke.md diff --git a/.convoys/adopt-playwright-smoke.md b/.convoys/adopt-playwright-smoke.md new file mode 100644 index 0000000..ff09442 --- /dev/null +++ b/.convoys/adopt-playwright-smoke.md @@ -0,0 +1,276 @@ +--- +name: adopt-playwright-smoke +classification: convoy +success_metric: | + `Playwright smoke` on every PR reaches `npx playwright test` and either + passes (smoke green) or fails on real test assertions. The current + fast-fail at the test step ("playwright not installed" / "no config") + goes away. `Screenshot diff` workflow either passes (snapshots stable) + or fails on a real pixel diff with the standard upload + PR comment + artifacts. Both checks complete in < 5 minutes. +skip: + - role-design-system-auditor # spec authoring, not visual design + - role-a11y-auditor # tested-from-outside; a11y belongs in deeper specs + - role-ux-reviewer # no UX surface + - role-ia-architect # no IA surface +status: queued +created: 2026-05-24 +parent: ship-readiness +addresses: P1 #10 step 2 (launch sequence step 10) +depends_on: + - fix-vercel-deployment-protection-in-ci (shipped — PR #17, 9a3e077) + - bump-next-js (shipped — e57ea17; needed for working Vercel previews) +--- + +# Convoy: adopt-playwright-smoke + +Stand up Playwright end-to-end. The infrastructure scaffolding has already +landed in three earlier convoys; this one wires the actual `@playwright/test` +dependency, the `playwright.config.js` that points it at the Vercel preview +URL, and turns the existing `tests/smoke/app.smoke.spec.ts` from "drafted +but inert" into "executed by CI on every PR". + +## Why now + +PR #17 (`fix-vercel-deployment-protection-in-ci`, squash `9a3e077`) closed +the last infra gap blocking Playwright smoke. The wait-action now reaches a +2xx in ~200ms, both `BASE_URL` and `VERCEL_AUTOMATION_BYPASS_SECRET` are +already exported to the smoke / screenshot-capture step's `env:`, and +`tests/smoke/app.smoke.spec.ts` is already drafted (3 tests: home renders +without 5xx, sign-in page renders, `/api/health` responds 2xx — and +`pages/api/health.js` already exists, so the third test won't 404). + +What's missing is small and well-bounded: + +1. `@playwright/test` is NOT in `package.json` devDependencies (verified via + `grep`-of-package.json). +2. There is no `playwright.config.js` (or `.ts`) in the tree. +3. The smoke spec is `.ts` in a JS-only repo (no `tsconfig.json`, no other + `.ts` source files except `next-env.d.ts`). Decision to ratify: convert + to `.js` or accept `.ts` for `tests/` only? +4. The `Screenshot diff` workflow runs `npx playwright test --project=visual` + which also needs a `visual` project in the config. The visual workflow's + "Capture screenshots (PR)" step has `continue-on-error: true` and a + PR-comment step (after PR #16's `pull-requests: write` perm fix), so + we can ship without baselines on the first run — the comment will say + "no baselines yet, run `--update-snapshots` locally to seed them." + +This convoy is the next step in the launch sequence (step 10 of +`.convoys/ship-readiness.md`'s "Proposed launch sequence"). After it ships, +every PR gets real smoke regression signal — which materially de-risks +every subsequent convoy (`single-auth-provider`, `single-sql-client`, +`god-component-split`, etc.) because those will start touching live page +flows that nothing currently exercises automatically. + +## Scope + +**In scope:** + +- `package.json` — add `@playwright/test` to `devDependencies`. (Latest + stable as of 2026-05-24; architect picks the exact version pin.) +- `package-lock.json` — regenerated by `npm install`. +- `package.json` `scripts` — add `test:smoke` and `test:visual` (or + a single `test:e2e`; architect's call). Should the scripts run against + `localhost:3000` by default and require an explicit `BASE_URL` for + preview, or vice versa? Decision to ratify. +- `playwright.config.js` (new) — at minimum: `testDir: './tests'`, two + `projects:` blocks (`smoke` and `visual`) matching the workflow + invocations (`npx playwright test --project=smoke|visual`), + `use.baseURL` from `process.env.BASE_URL`, `use.extraHTTPHeaders` + forwarding `x-vercel-protection-bypass` from + `process.env.VERCEL_AUTOMATION_BYPASS_SECRET` (per AGENTS.md § 7), + and a reasonable `timeout` / `expect.timeout`. +- `tests/smoke/app.smoke.spec.ts` (existing) — keep as `.ts` OR rename + to `.js`, depending on Decision A. If kept as `.ts`, may need to + exclude `tests/**/*.ts` from ESLint (the JS-only repo's lint config + doesn't currently handle `.ts` and will likely error). +- `tests/visual/` (new directory + at least one trivial spec) — needs + a single `.spec.ts` (or `.js`) that takes a screenshot of the + homepage. Without a spec, `npx playwright test --project=visual` + exits 0 and the screenshot workflow has nothing to compare. +- `eslint.config.mjs` (possibly) — if Decision A keeps `.ts` specs, + add `tests/**/*.ts` to `globalIgnores` OR wire typescript-eslint + to parse them safely. +- `.gitignore` (possibly) — `test-results/`, `playwright-report/`, + `.playwright/` should be ignored (Playwright generates these on + every local run). +- `AGENTS.md` — section on running smoke tests locally (`npm run + test:smoke`) + the "expect baselines to drift on UI changes; run + `--update-snapshots`" guidance. The doc-writer pass at convoy + close handles this; the brief should NOT touch `AGENTS.md`. + +**Out of scope:** + +- **Writing deep E2E tests beyond the 3 existing smoke checks.** This + convoy makes smoke green; deeper coverage is per-feature work in + feature convoys (`add-rate-limiting` adds a rate-limit smoke check, + etc.). +- **Authoring real visual baselines.** First-run snapshots can be + trivial (homepage only). Real baseline curation across critical + pages is a separate convoy (`adopt-visual-baselines`?) once UX has + stabilized post-`pick-a-name`. +- **Re-enabling the `test:` job in `.github/workflows/ci.yml`.** Per + ship-readiness P1 #10 step 3, that re-enable is a separate task — + this convoy's job is the Playwright side only. +- **Replacing `wait-for-vercel-preview`.** Still queued as + `replace-wait-for-vercel-preview` if the action ages out further. +- **Adding `test:smoke:local` cron / pre-commit hooks.** Smoke specs + should be runnable locally on demand; automatic cron is a separate + scope. +- **Migrating any source files to TypeScript.** Decision A may keep + the spec as `.ts`, but that's a test-only file — no source code + migrates. See AGENTS.md Gotcha #9. + +## Operator action required + +**None.** All prerequisites are already in place: + +- `VERCEL_AUTOMATION_BYPASS_SECRET` is seeded in GitHub Actions repo + secrets (`gh secret list` shows it; seeded 2026-05-24T20:03:31Z). +- Both target workflows (`preview-smoke.yml`, `visual-diff.yml`) + already export the secret to the test step's `env:`. +- `BASE_URL` is already wired. +- `pages/api/health.js` already exists for the existing smoke spec. + +## Decisions to ratify with operator + +Queued; do not pre-decide. Architect picks recommended option per +decision and routes back at gate 1. + +1. **`.ts` vs `.js` for Playwright specs.** The existing + `tests/smoke/app.smoke.spec.ts` uses TypeScript-flavored imports + (`import { test, expect } from '@playwright/test'`). The codebase + is JS-only (no `tsconfig.json`, no other `.ts` source files except + `next-env.d.ts`). Three options: + - **(a)** Convert the spec to `.js` — matches codebase convention, + no eslint config change needed (the import syntax works fine in + ESM `.js`). + - **(b)** Keep `.ts` for `tests/` only — Playwright docs default + to `.ts`; tests are isolated from production code; need to add + `tests/**/*.ts` to `eslint.config.mjs`'s `globalIgnores` or + properly configure typescript-eslint for the tests directory. + - **(c)** Mixed — `.js` for smoke, `.ts` for new specs going + forward. Inconsistent; not recommended. +2. **Fail-loud vs warn-and-continue when + `VERCEL_AUTOMATION_BYPASS_SECRET` is unset.** Same pattern as + `lib/rate-limit.js` (per AGENTS.md Gotcha #12): in CI (where + `process.env.CI === 'true'`), fail loudly — throw at config load + time with a clear error pointing at `gh secret set ...`. In dev + (where the secret might be missing but you're hitting localhost), + warn-and-continue. Architect picks the exact predicate and + error-message wording. +3. **One project (`smoke`) or two (`smoke` + `visual`)?** Both + workflows already invoke `--project=smoke` and `--project=visual` + respectively (post PR #17). Splitting into two projects in the + config is required. The question is whether `visual` should reuse + the same specs as `smoke` (with screenshot assertions added) OR + live in its own `tests/visual/` directory with separate specs. + The convoy's success metric only requires the workflows to run to + completion — the actual visual-coverage scope is a follow-up. +4. **First-run visual baselines.** With no committed baseline images, + `npx playwright test --project=visual` will either (a) fail + (no baselines to diff against) — which the workflow's + `continue-on-error: true` swallows, then the upload + comment step + surfaces the missing-baseline state — OR (b) Playwright treats + no-baseline as "create on first run" (depends on config). + Decision: do we commit a trivial homepage baseline now, OR + document the "run `--update-snapshots` locally first" workflow, + OR auto-commit baselines via a separate PR? +5. **ESLint coverage for `tests/`.** Currently + `eslint.config.mjs` does NOT explicitly ignore `tests/`. If + Decision A keeps `.ts`, eslint will try to parse it. The two + choices: add `tests/**/*.ts` (or just `tests/**`) to + `globalIgnores`, OR wire typescript-eslint into the test + directory. Latter is more work for arguable test-side benefit; + recommend the former. +6. **Should we add a `test:smoke:local` script that boots + `next dev` and runs against `localhost:3000` automatically?** Or + leave it as "you boot dev manually, then `BASE_URL=http://localhost:3000 + npm run test:smoke`"? The latter is simpler; the former is + friendlier. Convention-match with the existing `test:run` script + shape. + +## Known constraints + +- **`tests/smoke/app.smoke.spec.ts` already exists** with 3 tests + using `@playwright/test`'s API. Don't rewrite it; just enable it. + The third test references `/api/health`, which exists at + `pages/api/health.js` — confirmed. +- **Both target workflows already export + `VERCEL_AUTOMATION_BYPASS_SECRET` and `BASE_URL` to the test + step's `env:`** — `playwright.config.js`'s job is to read them + and apply them via `use.baseURL` + `use.extraHTTPHeaders`. +- **The header form** of the Vercel bypass + (`x-vercel-protection-bypass: `) is the correct shape + for a browser cookie-jar context. Per AGENTS.md § 7 and PR #17's + Decision A reservation, this is what `playwright.config.js` + should use — NOT the query-param form (that's reserved for + curl/axios contexts without cookie jars). +- **Playwright versions** ship browsers as a separate install step + (`npx playwright install --with-deps chromium` — already in + both workflows). Picking a Playwright version pin should consider + the workflow's expectation that the binary exists. +- **Vercel preview URLs are auth-protected.** This is why the bypass + exists. Without `use.extraHTTPHeaders` correctly wired, every + `page.goto(BASE)` call will hit Vercel SSO and the test will fail + with a content-mismatch (not a 401, because Vercel returns an HTML + SSO challenge page with 401 status). +- **Test runtime budget:** the smoke spec's comment says "<60s + total". Three trivial smoke tests should run in well under 30s. + The convoy's success metric is < 5 min for the whole workflow + (which includes ~2 min of `npm ci` + browser install). Comfortable. + +## Acceptance criteria + +The convoy is shippable when ALL of the following hold: + +1. `Playwright smoke` workflow on a fresh PR reaches `npx playwright + test --project=smoke`, browsers are installed, and the 3 existing + smoke tests run to completion. Pass OR fail; just not "no config". +2. `Screenshot diff` workflow on a fresh PR touching `pages/**` or + `components/**` reaches its visual capture step and either passes + OR posts a meaningful "Visual Diff" comment to the PR (per the + existing `continue-on-error: true` + comment step pattern). +3. Both workflows complete in < 5 minutes total. +4. `npm run test:smoke` (or whatever Decision 6 chooses) works + locally against either localhost or a deployed preview URL, + given a `BASE_URL` env var. +5. `npm run lint` exit code matches baseline (still 128 problems + per the `fix-lint-baseline` convoy; do NOT regress). +6. `npm run test:run` (vitest) still passes 21/21 (no regression + from the existing test surface). +7. Bypass secret does NOT appear in any workflow run log. Verify + by downloading the raw log of a passing run and grepping for + the secret's first 8 chars. +8. `playwright.config.js` is well-commented (every non-obvious + choice has a one-line explanation), so the next agent doesn't + need to re-derive context from the convoy file. + +## Anything flagged but not acted on (in advance) + +- **`tests/smoke/app.smoke.spec.ts` is `.ts` in a JS-only repo.** + This is a real anomaly. Decision A resolves it one way or another. + If we keep `.ts`, document the special-case treatment in + AGENTS.md so future agents don't try to "normalize" by converting + to `.js` (or vice versa). +- **Visual baseline curation strategy.** First-run baselines will + inevitably need re-capturing as the UI evolves toward the + `pick-a-name` rebrand. Don't over-invest in baselines this + convoy; document the `--update-snapshots` workflow and move on. +- **CI workflow `paths:` filter for `visual-diff.yml`.** Currently + the filter is `pages/**`, `components/**`, `styles/**`, + `tailwind.config.js`, `postcss.config.js`. After this convoy + ships, the filter is still correct — visual tests should + re-run when any of those change. No change needed in this + convoy. +- **`@playwright/test` security advisories.** Pin a recent version + and document the rationale (avoid security CVEs, avoid known + buggy versions). Architect picks; no operator ratification + needed unless a specific advisory is relevant. +- **PR-comment template for `Screenshot diff`.** The existing + comment-on-PR step in `visual-diff.yml` hardcodes the comment + body ("Screenshots and diffs uploaded as artifacts: [view + run](...)"). After this convoy, the body should arguably + include a quick diff summary (pixel count, % difference per + page). That's an enhancement for a separate `polish-visual-diff-comment` + convoy; not in scope here. -- 2.45.2 From 3ac527ef48af164e98e90fe6c90f1207c8495025 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sun, 24 May 2026 17:56:20 -0500 Subject: [PATCH 2/3] architect: ratify decisions + brief for adopt-playwright-smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convoy file: - status: queued -> in-progress - New § Decisions (post-IA round): 6 decisions investigated. Architect-self-ratifiable: D2 (CI fail-loud predicate mirrors rate-limit.js convention), D3 (two projects is a structural workflow requirement), D5 (no eslint change — verified `npx eslint tests/smoke/app.smoke.spec.ts` exits 0). Operator- ratifiable at gate 1: D1 (recommend keep .ts; convention call), D4 (recommend defer baseline commit to Linux-Docker follow-up; process call), D6 (recommend simple test:smoke/test:visual scripts, no auto-boot wrapper; DX call). - New § Architecture: file plan (5 files), API surface (n/a), schema diff (n/a), test plan (3 smoke + 1 visual; vitest 21/21 + lint baseline 128 held), risk list R1-R9 (request fixture header propagation, snapshotPathTemplate alignment, cross- platform mismatch, --update-snapshots=none semantics, CI predicate variants, bypass trace leak, npm ci lockfile sync, eslint future drift, Playwright 1.60.0 freshness). - New § Decomposition: 1 brief, ~120 LOC, slice_dependencies YAML block. Justified single brief because total LOC is small and all files are semantically coupled (visual spec depends on visual project in config which depends on the installed dep). Brief file (.convoys/adopt-playwright-smoke/brief-1-*.md): - Goal/Files/Conventions/Acceptance/Manual-verification/ Boot-the-brief findings/Out-of-scope sections matching the fix-vercel-deployment-protection-in-ci/brief-1 shape. - 8 Boot-the-brief findings: dep resolution clean, .ts lints clean, baselines dir MUST NOT be gitignored, extraHTTPHeaders propagates to request fixture per docs, /api/health is anonymous, ESM shape required, snapshotPathTemplate token set has no {platform}, --update-snapshots=none never auto-creates. - Verbatim playwright.config.js shape with CI fail-loud / dev warn predicate, snapshotPathTemplate aligned with workflow's artifact upload path, two projects with testMatch partitioning. No code changes (read-only architect pass). No push. Co-authored-by: Cursor --- .convoys/adopt-playwright-smoke.md | 385 +++++++++++++- ...brief-1-wire-playwright-and-visual-spec.md | 495 ++++++++++++++++++ 2 files changed, 879 insertions(+), 1 deletion(-) create mode 100644 .convoys/adopt-playwright-smoke/brief-1-wire-playwright-and-visual-spec.md diff --git a/.convoys/adopt-playwright-smoke.md b/.convoys/adopt-playwright-smoke.md index ff09442..dc5ef59 100644 --- a/.convoys/adopt-playwright-smoke.md +++ b/.convoys/adopt-playwright-smoke.md @@ -13,7 +13,7 @@ skip: - role-a11y-auditor # tested-from-outside; a11y belongs in deeper specs - role-ux-reviewer # no UX surface - role-ia-architect # no IA surface -status: queued +status: in-progress created: 2026-05-24 parent: ship-readiness addresses: P1 #10 step 2 (launch sequence step 10) @@ -274,3 +274,386 @@ The convoy is shippable when ALL of the following hold: include a quick diff summary (pixel count, % difference per page). That's an enhancement for a separate `polish-visual-diff-comment` convoy; not in scope here. + +## Decisions (post-IA round) + +Six decisions queued in the convoy file. Architect-investigated; +recommendations + ratification routing below. **3 of 6** are +architect-self-ratifiable (structural facts or convention mirrors). +**3 of 6** need operator ratification at human gate 1. + +### Decision 1 — `.ts` vs `.js` for Playwright specs → **(b) keep `.ts`** (operator ratifies) + +**Recommendation:** keep `tests/smoke/app.smoke.spec.ts` as `.ts`; +new visual spec ships as `tests/visual/homepage.spec.ts`. + +**Investigation:** + +- `npx eslint tests/smoke/app.smoke.spec.ts` — exit 0, zero output + (verified 2026-05-24 against the post-PR-17 tree). The + `eslint-config-next/core-web-vitals` chain bundled by + `eslint-config-next@16` already parses `.ts` files via its + transitive `typescript-eslint` dependency. **This is the same + mechanism that makes `typescript@^5.9.3` a hard devDep even + though no source file is TS** (AGENTS.md Gotcha #9). +- `npm run lint` baseline stays at 128 problems with the `.ts` + spec present — confirmed empirically. Decision 5 (no eslint + config change) follows from this. + +**Rationale:** + +1. Playwright's official docs and `create-playwright` scaffolding + default to `.ts`. Fighting that for every future spec is + friction. +2. Test files are isolated from production source — AGENTS.md + Gotcha #9's "no `.ts` files except `next-env.d.ts`" rule was + scoped at production code, not test infrastructure. +3. Lint already accepts it; no `eslint.config.mjs` change needed. +4. Converting to `.js` saves one anomaly in `rg --type=ts` output + but creates a new precedent ("the repo prefers `.js` even when + the framework defaults to `.ts`") that future Playwright work + would have to re-litigate. + +**Routing:** operator ratifies. Convention preference, not a +structural requirement. + +### Decision 2 — fail-loud vs warn-and-continue → **CI fail-loud, dev warn-and-no-op** (architect-self-ratifiable) + +**Decision:** in `playwright.config.js`, `throw` at config load +when `process.env.CI === 'true'` AND +`process.env.VERCEL_AUTOMATION_BYPASS_SECRET` is missing / +empty. In non-CI (`process.env.CI` unset), `console.warn` once +and continue with `extraHTTPHeaders` undefined (caller hits +localhost or a public URL). + +**Rationale:** mirrors the established `lib/rate-limit.js` +convention documented in AGENTS.md Gotcha #12 ("In prod, the +rate-limit module fails closed if either of the two REST vars is +missing... In dev / test, it warn-and-continues as a no-op"). +Predicate is `process.env.CI === 'true'` (not `NODE_ENV === +'production'`) because Playwright config has no Next.js context +and `CI` is the canonical CI-runner signal. Error message names +the env var, the rotation command (`gh secret set +VERCEL_AUTOMATION_BYPASS_SECRET --body ""`), and points +at AGENTS.md § 7 for the full context. + +**Routing:** architect-self-ratifiable. Mirrors existing repo +convention; not a fresh policy. + +### Decision 3 — one project (`smoke`) or two (`smoke` + `visual`) → **two projects, two directories** (architect-self-ratifiable) + +**Decision:** `playwright.config.js` declares two projects: + +- `smoke` — `testMatch: 'smoke/**/*.spec.@(ts|js)'` +- `visual` — `testMatch: 'visual/**/*.spec.@(ts|js)'` + +Both projects share the same `use:` block (`baseURL`, +`extraHTTPHeaders`, `trace: 'off'`). The `visual` project alone +hits `expect(page).toHaveScreenshot()`; `smoke` does not. + +**Rationale:** the post-PR-17 workflows already invoke +`--project=smoke` (in `preview-smoke.yml`) and `--project=visual` +(in `visual-diff.yml`). Two projects is a structural +requirement, not a preference. Separate directories cleanly +partition `testMatch` and avoid the boot-the-brief failure +mode where `testDir: './tests'` mixes both project's specs +into both projects. + +**Routing:** architect-self-ratifiable. The workflows already +made this call; the architect's job is to honor it. + +### Decision 4 — first-run visual baselines → **(b) do NOT commit baselines this convoy; document seed-on-Linux workflow + queue follow-up** (operator ratifies) + +**Recommendation:** ship the visual spec without a committed +baseline. First CI run of `Screenshot diff` will fail at the +test step (per Playwright's `--update-snapshots=none` semantics, +"missing snapshots cause test failure"), but the workflow's +existing `continue-on-error: true` swallows that and the +comment-on-PR step posts "Visual Diff — view run" with empty +artifacts. Operator then runs `npm run test:visual:update` +in a Linux environment (Docker `mcr.microsoft.com/playwright` +image, or a manually-dispatched workflow with +`--update-snapshots`) and commits the baselines in a separate +follow-up PR (`seed-visual-baselines-on-linux`, queued). + +**Investigation:** + +- Playwright snapshot file names include the platform suffix: + `--.png` (default template). With + a custom `snapshotPathTemplate` we can drop the platform, but + cross-platform mismatches then become silent overwrites — bad + for a multi-OS team. +- Verified via [Playwright docs](https://playwright.dev/docs/test-snapshots) + and `--update-snapshots` flag semantics: default mode is + `missing` (auto-create on first run, fail the test); explicit + `none` (what the workflow uses) NEVER creates and ALWAYS fails + when missing. +- The convoy file's own out-of-scope section says: "Authoring + real visual baselines... is a separate convoy + (`adopt-visual-baselines`?) once UX has stabilized." This + decision matches that intent. + +**Rationale:** committing a Mac-generated baseline now would +either (i) fail CI on the very first run because the platform +suffix won't match, or (ii) drop the platform suffix entirely +via `snapshotPathTemplate`, which silently hides platform +drift for any future contributor running on a different OS. +Neither is worth saving the operator one follow-up PR. +Bundling Linux-Docker baseline generation into this convoy +adds a Docker dependency, a `--network=host` workaround on +Mac, and a `BASE_URL` reachability question — all out of +scope per the convoy file. + +**Routing:** operator ratifies. Process preference about how +much baseline curation belongs in this convoy vs the +follow-up. + +### Decision 5 — ESLint coverage for `tests/` → **no change to `eslint.config.mjs`** (architect-self-ratifiable) + +**Decision:** `eslint.config.mjs` is NOT touched in this convoy. +The brief verifies post-implementation by re-running `npm run +lint` and confirming the baseline stays at 128 problems. + +**Investigation:** `npx eslint tests/smoke/app.smoke.spec.ts` +exits 0 with zero output against the current config (verified +2026-05-24 on the convoy branch HEAD). The +`eslint-config-next/core-web-vitals` chain in `eslint-config-next@16` +bundles the `typescript-eslint` parser; `.ts` files in the +repo (today: just `next-env.d.ts` plus the smoke spec) are +parsed cleanly without any explicit `tests/**/*.ts` +`globalIgnores` entry. The convoy file flagged this as +"likely to need an ignore" — investigation found it doesn't. + +**Routing:** architect-self-ratifiable. Empirical evidence; +no convention question. + +### Decision 6 — `test:smoke:local` boot-`next dev` script → **no; ship simple `test:smoke` + `test:visual` + `test:visual:update`** (operator ratifies) + +**Recommendation:** add three scripts to `package.json`: + +- `"test:smoke": "playwright test --project=smoke"` +- `"test:visual": "playwright test --project=visual"` +- `"test:visual:update": "playwright test --project=visual --update-snapshots"` + +Operator's local workflow: + +```bash +# Terminal 1 +npm run dev +# Terminal 2 +BASE_URL=http://localhost:3000 npm run test:smoke +# OR against a deployed preview: +BASE_URL=https://.vercel.app \ + VERCEL_AUTOMATION_BYPASS_SECRET= \ + npm run test:smoke +``` + +**Rationale:** + +1. Matches the existing `test` / `test:run` shape — each + script does one thing. +2. Auto-booting `next dev` from a test runner introduces + wait-for-ready / cleanup / port-conflict edge cases + that are fragile across OS. +3. `test:visual:update` is the exact command the visual + workflow's PR-comment text instructs operators to run + when seeding new baselines, so it's a one-line DX win + without adding any logic. + +**Routing:** operator ratifies. DX preference about how +much wrapper logic belongs in the test scripts. + +## Architecture + +### File plan + +| File | Action | Purpose | +|---|---|---| +| `package.json` | modified | Add `@playwright/test@^1.60.0` to devDependencies. Add `test:smoke`, `test:visual`, `test:visual:update` scripts. | +| `package-lock.json` | modified | Regenerated by `npm install` (committed in the same commit; required for `npm ci` parity in CI per Risk R7). | +| `playwright.config.js` | new | Root-level ESM config. `testDir: './tests'`, two `projects:` blocks (`smoke` + `visual`) per Decision 3, `use.baseURL` from `process.env.BASE_URL`, `use.extraHTTPHeaders` with CI-gated `x-vercel-protection-bypass` per Decision 2, `snapshotPathTemplate` for the visual project (Risk R2), `timeout: 30_000` + `expect.timeout: 10_000`. Well-commented per AC #8. | +| `tests/smoke/app.smoke.spec.ts` | unchanged | Existing 3-test spec stays as `.ts` per Decision 1. NOT renamed; NOT edited. | +| `tests/visual/homepage.spec.ts` | new | One screenshot spec: `expect(page).toHaveScreenshot('home.png')`. Inline comment documents the seed-on-Linux first-run workflow per Decision 4. | +| `.gitignore` | modified | Add `/playwright-report/`, `/test-results/`, `/.playwright/`. **Do NOT add `tests/visual/__screenshots__/`** — baselines MUST be committed when they exist (boot-the-brief finding). | + +### API surface + +N/A. No new API routes. Smoke tests hit existing +`pages/api/health.js` which already returns 200 anonymously +with no auth gate and no rate-limit wrapper (verified by +reading the source — 7-line handler, no `getUserFromRequest`, +no `checkAuthRateLimit`). The Vercel bypass header passes +through to API routes because Vercel's preview protection +runs at the platform edge, not inside the function — the +function receives the request as if anonymous. + +### Schema diff + +N/A. No DB changes. + +### Test plan + +This convoy IS the test infrastructure. Net new tests: + +- `tests/smoke/app.smoke.spec.ts` (already exists, 3 tests): + home renders without 5xx, sign-in page renders, `/api/health` + 2xx. Becomes executed-by-CI in this convoy. +- `tests/visual/homepage.spec.ts` (new, 1 test): screenshot of + the homepage at default viewport (1280×720 — Playwright + default). First run fails because no baseline exists; that's + the documented end state of this convoy per Decision 4. + +Regression coverage held in place by: + +- `npm run test:run` (vitest) still passes 21/21 — the new + `@playwright/test` install does not touch any vitest config or + setup file. Verified by re-running `npm run test:run` after + the implementer's `npm install`. +- `npm run lint` exit code matches the established 128-problem + baseline (see Decision 5). +- `npm run build` still succeeds — Playwright is a devDep, not + bundled into the Next.js build graph. + +### Risk list + +- **R1 — `request` fixture vs `use.extraHTTPHeaders` propagation.** + The existing smoke spec uses `await request.get(/api/health)`. + Per [Playwright fixtures docs](https://playwright.dev/docs/api/class-fixtures#fixtures-request), + the test-level `request` fixture is an "Isolated APIRequestContext + instance for each test." Per `testOptions.extraHTTPHeaders` docs, + the option applies to **every request** including those issued + by the APIRequestContext (the testOptions surface is shared + between browser context and request fixture creation). + **Mitigation:** the brief's manual-verification step confirms + the health-endpoint test passes against a Vercel preview. If it + 401s with the SSO HTML body despite the config, the implementer + files a hotfix to pass headers explicitly in the spec body OR + to add a `request` fixture override in `playwright.config.js`. + Not expected based on the doc evidence, but worth a manual + check. + +- **R2 — Snapshot path template misalign with workflow artifact path.** + Playwright's default snapshot location is + `.spec.ts-snapshots/--.png` + alongside the spec. The visual workflow uploads + `tests/visual/__screenshots__/` recursively. Without an + explicit `snapshotPathTemplate`, the artifact upload picks up + zero baselines. **Mitigation:** set + `snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}'` + in `playwright.config.js`. Architect verified the supported + tokens via Playwright `testProject.snapshotPathTemplate` + reference (`{arg}`, `{ext}`, `{projectName}`, `{snapshotDir}`, + `{testDir}`, `{testFileDir}`, `{testFileName}`, + `{testFilePath}` — no `{platform}` token). + +- **R3 — Cross-platform snapshot mismatch (Mac dev vs Linux CI).** + With the flat `snapshotPathTemplate` from R2, all baselines + collapse into a single file per snapshot name regardless of + platform. A Mac contributor running `npm run test:visual:update` + locally would overwrite the Linux CI baseline. **Mitigation:** + per Decision 4, do not commit baselines this convoy. Document + the "seed via Docker `mcr.microsoft.com/playwright:v1.60.0-noble` + on Linux" workflow in the visual spec inline comment + + AGENTS.md (doc-writer pass). Queue `seed-visual-baselines-on-linux` + as the follow-up convoy. + +- **R4 — `--update-snapshots=none` behavior with missing baselines.** + Verified via Playwright CLI docs: "Possible values are 'all', + 'changed', 'missing', and 'none'. Running tests without the + flag defaults to 'missing'; running tests with the flag but + without a value defaults to 'changed'." Mode `none` never + creates snapshots; missing snapshots fail the test. The + visual workflow's `continue-on-error: true` + comment-on-PR + step is the safety net. **Mitigation:** documented behavior; + no config change needed. + +- **R5 — `process.env.CI` predicate false negative.** GitHub + Actions sets `CI=true`; Vercel build runtime sets `CI=1`. + Playwright is invoked only from GitHub Actions in this convoy + (the Vercel build does not run Playwright). Strict equality + `process.env.CI === 'true'` is correct for the GitHub Actions + case. If a future workflow invokes Playwright from a different + CI provider, revisit. **Mitigation:** documented in the + config's inline comment. + +- **R6 — Bypass secret leakage via Playwright trace HAR.** If a + future change enables `trace: 'on'` (or `retain-on-failure`), + the bypass header lands in the trace.zip HAR payload. The + upload-artifact step then preserves it for 7 days. + **Mitigation:** keep `trace: 'off'` in this convoy. If/when + traces are enabled in a future polish convoy, that convoy MUST + decide on HAR sanitization (e.g. a custom reporter that strips + the `x-vercel-protection-bypass` header from saved traces, or + rotating the bypass token more aggressively). + +- **R7 — `npm ci` in CI vs `npm install` locally.** Both + workflows run `npm ci`, which requires + `package-lock.json` to be in sync with `package.json`. The + implementer MUST run `npm install` locally and commit the + regenerated lockfile in the SAME commit (or `npm ci` in CI + will fail with "lockfile out of sync"). **Mitigation:** AC + in the brief calls this out explicitly; the architect's + Boot-the-brief did not run `npm install` itself (read-only + pass) but the requirement is mechanical. + +- **R8 — `eslint-config-next` typescript-eslint future drift.** + Verified clean today (`npx eslint tests/smoke/app.smoke.spec.ts` + exit 0). Risk is future drift if `eslint-config-next` ever + drops or restructures the typescript-eslint bundle. + **Mitigation:** the brief verifies post-implementation that + `npm run lint` still hits the 128-problem baseline. If it + grows, investigate before merge — the new errors are most + likely from the `.ts` spec parsing, which would need a + `globalIgnores` entry as a hotfix. + +- **R9 — Playwright 1.60.0 freshness (released 2026-05-11, 13 + days old at time of this convoy).** No reported critical + regressions in changelog scan. Previous stable 1.59.1 (2026-04-01, + ~7 weeks old) is the safer pin. Architect picks `^1.60.0` to + align with the workflow's `npx playwright install --with-deps + chromium` step (downloads the bundled binary matching the + installed package version) and to inherit any 1.60.x patches. + **Mitigation:** if 1.60.0 surfaces regressions in the + implementer's smoke run, downgrade to `^1.59.1` in a hotfix + before merge. + +## Decomposition + +| Brief # | Title | Files | Depends on | Estimated PR size | +|---|---|---|---|---| +| 1 | Install `@playwright/test`, ship `playwright.config.js`, add visual homepage spec | `package.json`, `package-lock.json`, `playwright.config.js`, `tests/visual/homepage.spec.ts`, `.gitignore` | none | ~120 LOC source diff + lockfile churn | + +**Brief count: 1.** Justification: + +1. **Total source-diff LOC < 200.** `playwright.config.js` ~70 + LOC, visual spec ~20 LOC, `package.json` ~5 LOC, + `.gitignore` ~5 LOC. Lockfile churn is mechanical, not + reviewable. +2. **All files are semantically coupled.** The visual spec + cannot be discovered without `playwright.config.js` + declaring the `visual` project. The config cannot be + loaded without `@playwright/test` in `node_modules`. + `.gitignore` covers artifacts produced by both. Splitting + into separate briefs would force interim states that + either don't compile or don't run. +3. **No parallelizability benefit.** Two implementers cannot + meaningfully work on disjoint subsets of this change set. +4. **One human review is enough.** All changes fit in a + single PR's diff comfortably. + +### Slice dependencies (multitask-ready) + +```yaml +slice_dependencies: + - brief: 1 + depends_on: [] + files: + - package.json + - package-lock.json + - playwright.config.js + - tests/visual/homepage.spec.ts + - .gitignore +``` + +Single brief; no `/multitask` fan-out. Conductor dispatches +serially. diff --git a/.convoys/adopt-playwright-smoke/brief-1-wire-playwright-and-visual-spec.md b/.convoys/adopt-playwright-smoke/brief-1-wire-playwright-and-visual-spec.md new file mode 100644 index 0000000..01a084b --- /dev/null +++ b/.convoys/adopt-playwright-smoke/brief-1-wire-playwright-and-visual-spec.md @@ -0,0 +1,495 @@ +--- +convoy: adopt-playwright-smoke +brief_number: 1 +depends_on: [] +files: + - package.json + - package-lock.json + - playwright.config.js + - tests/visual/homepage.spec.ts + - .gitignore +--- + +# Brief 1: Install `@playwright/test`, ship `playwright.config.js`, add the homepage visual spec — so the post-PR-17 `Preview smoke` and `Screenshot diff` workflows reach `npx playwright test` and execute against the Vercel preview + +## Goal (1 sentence) + +Wire `@playwright/test@^1.60.0` into devDeps, create `playwright.config.js` with two projects (`smoke` + `visual`) and a CI-gated `x-vercel-protection-bypass` header per `playwright.config` Decision 2, add a single `tests/visual/homepage.spec.ts` baseline-bearing screenshot test per Decision 3, add three `npm` scripts per Decision 6, and ignore Playwright's local-run artifacts in `.gitignore` — without renaming or editing the existing `tests/smoke/app.smoke.spec.ts`, without touching `eslint.config.mjs`, and without committing any baseline images (operator seeds those in a follow-up Linux-Docker run per Decision 4). + +## Files in scope (do not edit anything else) + +- `package.json` — modified. +- `package-lock.json` — modified (regenerated by `npm install`; commit in the SAME commit as `package.json` to keep `npm ci` happy in CI per Risk R7). +- `playwright.config.js` — new (root level). +- `tests/visual/homepage.spec.ts` — new. +- `.gitignore` — modified. + +**Files explicitly out of scope** (do not touch even if it seems related): + +- `tests/smoke/app.smoke.spec.ts` — exists, stays as-is per Decision 1. NOT renamed to `.js`. NOT edited. +- `eslint.config.mjs` — empirically verified to parse the existing `.ts` spec cleanly (Decision 5 + Boot-the-brief finding #2 below). NOT touched. +- `vitest.config.js`, `test/setup.js`, any file under `test/` (vitest's home) — different runner; out of scope. +- `.github/workflows/preview-smoke.yml`, `.github/workflows/visual-diff.yml`, `.github/workflows/ci.yml` — all workflow YAML is owned by other convoys (PR #17 / PR #16 / `fix-lint-baseline`); zero touches here. +- `AGENTS.md` § 7 (the seed-on-Linux workflow + the local-run command table) — that's the doc-writer pass at convoy close, NOT this brief. +- `tests/visual/__screenshots__/` — do NOT create or commit baselines in this convoy per Decision 4. Operator runs `npm run test:visual:update` in a Linux Docker env (`mcr.microsoft.com/playwright:v1.60.0-noble`) and commits in a follow-up `seed-visual-baselines-on-linux` PR. +- `pages/api/health.js` — already exists and already returns 200 anonymously (verified by reading the 7-line source). Do NOT modify or move. +- `next.config.js`, `tailwind.config.js`, `postcss.config.js` — unrelated; zero touches. + +## Conventions to follow + +### Decisions from the convoy file (cite when implementing) + +- **Decision 1 (`.convoys/adopt-playwright-smoke.md` § Decisions, post-IA round):** keep `tests/smoke/app.smoke.spec.ts` as `.ts`. New visual spec also `.ts` (`tests/visual/homepage.spec.ts`). +- **Decision 2:** in `playwright.config.js`, `throw` at config load when `process.env.CI === 'true'` AND `VERCEL_AUTOMATION_BYPASS_SECRET` is missing/empty. In non-CI, `console.warn` once and continue with `extraHTTPHeaders` undefined. Error message names the env var, the rotation command, and points at `AGENTS.md § 7`. +- **Decision 3:** two projects — `smoke` (`testMatch: 'smoke/**/*.spec.@(ts|js)'`) and `visual` (`testMatch: 'visual/**/*.spec.@(ts|js)'`). Both share the same `use:` block (no per-project `use:` overrides). +- **Decision 4:** do NOT commit baselines. First CI run of `Screenshot diff` will fail at the test step; the existing `continue-on-error: true` swallows the failure and the comment-on-PR step posts "Visual Diff — view run" with empty artifacts. That's the documented end state of this brief. +- **Decision 5:** no `eslint.config.mjs` change. Verified empirically that `npx eslint tests/smoke/app.smoke.spec.ts` exits 0 against the current config; the new `tests/visual/homepage.spec.ts` is structurally identical and will also pass. +- **Decision 6:** add three scripts — `test:smoke`, `test:visual`, `test:visual:update`. No auto-boot of `next dev`. + +### Repo conventions (cite + match) + +- **ESM module style.** `package.json` has `"type": "module"`. `vitest.config.js` and `next.config.js` are both ESM (`import { defineConfig } from '...'; export default defineConfig({...})`). `playwright.config.js` MUST match this shape: + ```js + import { defineConfig } from '@playwright/test'; + export default defineConfig({ ... }); + ``` +- **No-go zones (`.cursor/rules/no-go-zones.mdc`).** None of the files in scope are listed. `playwright.config.js` does not exist yet. `tests/visual/` does not exist yet. `package.json` / `package-lock.json` / `.gitignore` are all editable per established convoy precedent (`bump-next-js`, `fix-auth-bypass`). +- **Secret-handling discipline (AGENTS.md § 7):** + - NEVER `console.log` / `echo` / write to a file any string containing `process.env.VERCEL_AUTOMATION_BYPASS_SECRET`. + - The `console.warn` branch (Decision 2 dev path) must say "VERCEL_AUTOMATION_BYPASS_SECRET unset" — NOT print the value. + - The fail-loud branch (Decision 2 CI path) `throw`s a string error message; the message names the env var by name but does NOT echo any value. +- **Style match.** Two close precedents for ESM config files: `vitest.config.js` (heavy inline comments explaining each non-default choice) and `next.config.js` (terse). `playwright.config.js` should follow `vitest.config.js`'s commented style because the convoy's AC #8 requires it ("`playwright.config.js` is well-commented (every non-obvious choice has a one-line explanation), so the next agent doesn't need to re-derive context from the convoy file"). + +## Acceptance criteria + +### `package.json` + +- [ ] Add `"@playwright/test": "^1.60.0"` to `devDependencies` (alphabetical position — between `@neondatabase/serverless` is in `dependencies`, so in `devDependencies` it lands between `@testing-library/react` and `autoprefixer`; verify alphabetization is preserved). + + Pin rationale: `^1.60.0` accepts patch updates (1.60.x), matches the workflow's `npx playwright install --with-deps chromium` (which downloads the bundled browser matching the installed package version), and 1.60.0 is 13 days old at time of writing — fresh but not bleeding-edge. If `npm install` resolves to a newer 1.60.x patch, that's expected. + +- [ ] Add three scripts to the `scripts` block, between `test:run` and the closing brace: + + ```json + "test:smoke": "playwright test --project=smoke", + "test:visual": "playwright test --project=visual", + "test:visual:update": "playwright test --project=visual --update-snapshots" + ``` + + Notes: + - Use bare `playwright test`, NOT `npx playwright test`. With the dep installed locally, npm scripts resolve `playwright` from `node_modules/.bin` automatically. Matches the existing `test` / `test:run` shape (which uses bare `vitest`, not `npx vitest`). + - `test:visual:update` is the exact command the visual workflow's PR-comment text instructs operators to run — keep the verbatim string match so a future operator can copy-paste from the comment. + - Do NOT add `test:e2e`, `test:smoke:local`, or any auto-boot variant. Decision 6 explicitly rejected those. + +- [ ] No other changes to `package.json`. `dependencies`, `name`, `version`, `private`, `type` all stay byte-identical. + +### `package-lock.json` + +- [ ] Regenerated by running `npm install` after the `package.json` edit. Commit the resulting lockfile in the SAME commit as the `package.json` change (Risk R7 in the convoy file). The diff will be large (Playwright pulls many transitive deps) but is mechanical; do NOT hand-edit it. + +- [ ] Verify lockfile sync by running `npm ci` locally after commit: it should succeed with exit 0. If it errors with "Missing: ... from lock file" or "Invalid: lock file's ... does not satisfy package.json", the regeneration is incomplete — re-run `npm install` and re-commit. + +### `playwright.config.js` (new file at repo root) + +- [ ] Verbatim shape (commented for AC #8; the implementer is free to tighten wording but every non-obvious choice MUST have a one-line explanation): + +```js +// Playwright config for the post-PR-17 `Preview smoke` and +// `Screenshot diff` workflows. ESM per the repo's +// `"type": "module"` setting in package.json. Companion docs: +// `.convoys/adopt-playwright-smoke.md` (Decisions 1-6), +// AGENTS.md § 7 (Vercel preview bypass conventions). + +import { defineConfig } from '@playwright/test'; + +const BASE_URL = process.env.BASE_URL ?? 'http://localhost:3000'; +const BYPASS_SECRET = process.env.VERCEL_AUTOMATION_BYPASS_SECRET; + +// `CI === 'true'` is the canonical GitHub Actions signal (set by +// the runner). Playwright config has no Next.js context, so +// `NODE_ENV` is not reliable here. Mirrors the rate-limit.js +// fail-closed pattern documented in AGENTS.md Gotcha #12. +const IS_CI = process.env.CI === 'true'; + +if (IS_CI && !BYPASS_SECRET) { + // Fail loud in CI per Decision 2. The workflow's `env:` block + // (preview-smoke.yml line 101, visual-diff.yml line 85) maps + // `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` into the process + // env; if it's empty here, the secret is unseeded or the + // workflow YAML drift broke the mapping. + throw new Error( + 'VERCEL_AUTOMATION_BYPASS_SECRET is required in CI to reach ' + + 'Vercel-Protection-protected preview deployments. ' + + 'Reseed via: gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "". ' + + 'See AGENTS.md § 7 for the full plumbing context.' + ); +} + +if (!BYPASS_SECRET && !IS_CI) { + // Dev fallback per Decision 2 — warn once at config load, + // proceed without the header. Local runs target localhost + // (no preview protection) or a non-protected URL. + // eslint-disable-next-line no-console -- intentional one-shot warning at config load + console.warn( + '[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ' + + 'running without the Vercel bypass header. Targets a non-protected ' + + 'URL (e.g. http://localhost:3000). Hitting a protected preview without ' + + 'this header will return Vercel\'s SSO challenge page.' + ); +} + +export default defineConfig({ + // Both `tests/smoke/` and `tests/visual/` live under `tests/`. + // Project-level `testMatch` (below) partitions them so the + // two workflows (`--project=smoke` and `--project=visual`) + // each see only the specs they should run. + testDir: './tests', + + // Smoke + visual specs are independent; parallelism within a + // single spec adds no value here and would complicate the + // per-test screenshot baseline lifecycle. + fullyParallel: false, + workers: IS_CI ? 1 : undefined, + + // One retry in CI handles transient Vercel preview flakes + // (cold-start, DNS propagation). Local: zero retries — fail + // fast so the dev sees the issue immediately. + retries: IS_CI ? 1 : 0, + + // 30s per test is plenty for the 3 smoke checks + 1 visual + // screenshot. The convoy file's success metric is < 5min + // total workflow runtime; per-test 30s is well inside that. + timeout: 30_000, + expect: { timeout: 10_000 }, + + // List reporter in dev for human readability; add HTML in CI + // so the `Upload Playwright report on failure` step + // (preview-smoke.yml line 104) has a populated `playwright-report/` + // to upload. `open: 'never'` keeps the HTML from auto-launching + // a browser tab in headless CI. + reporter: IS_CI ? [['list'], ['html', { open: 'never' }]] : 'list', + + // Visual baselines live at `tests/visual/__screenshots__/{ext}`. + // Workflow `visual-diff.yml` line 94 uploads this exact path as the + // artifact — keep them aligned. {arg} is the snapshot name from + // `toHaveScreenshot('home.png')` without the extension; {ext} is + // the extension with the leading dot. + // + // Cross-platform note (Risk R3): this template drops Playwright's + // default `--` suffix. That means a Mac dev + // running `npm run test:visual:update` overwrites the Linux-CI + // baseline. Per Decision 4, we don't commit baselines this convoy; + // operator seeds via `mcr.microsoft.com/playwright:v1.60.0-noble` + // Docker on Linux. Multi-platform support is the + // `seed-visual-baselines-on-linux` follow-up convoy's job. + snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}', + + use: { + baseURL: BASE_URL, + // Headers apply to BOTH browser `page.goto(...)` calls AND the + // test-level `request` fixture's APIRequestContext (verified + // against Playwright docs: `testOptions.extraHTTPHeaders` is + // shared between browser context and APIRequestContext + // construction). This is why `tests/smoke/app.smoke.spec.ts`'s + // third test (`request.get('/api/health')`) reaches the + // protected preview without re-injecting the header in the + // spec body. If a future hotfix shows the header NOT + // propagating to APIRequestContext, see Risk R1 in the + // convoy file's Architecture section. + extraHTTPHeaders: BYPASS_SECRET + ? { 'x-vercel-protection-bypass': BYPASS_SECRET } + : undefined, + // Trace OFF this convoy. Enabling it would land the bypass + // header in the HAR payload (Risk R6); a future polish convoy + // owns the trace-on + HAR-sanitization decision. + trace: 'off', + screenshot: 'off', + video: 'off', + }, + + projects: [ + { + // `Preview smoke` workflow invokes `--project=smoke` (per + // preview-smoke.yml line 98). The testMatch keeps the + // visual specs out of this project. + name: 'smoke', + testMatch: 'smoke/**/*.spec.@(ts|js)', + }, + { + // `Screenshot diff` workflow invokes `--project=visual` + // (per visual-diff.yml line 82). The testMatch keeps the + // smoke specs out of this project (so a `--project=visual` + // run doesn't redundantly execute the smoke tests). + name: 'visual', + testMatch: 'visual/**/*.spec.@(ts|js)', + }, + ], +}); +``` + + Two things the implementer can vary without breaking AC: + + 1. Comment wording — the substantive choices (predicate, error message contents, path template) must stay; the explanatory prose can be tightened. + 2. Property ordering inside `defineConfig({...})` — Playwright doesn't care; alphabetical or grouped-by-concern are both fine. + + Things the implementer MUST NOT change: + + - The `IS_CI` predicate (`=== 'true'`, not `=== 'true' || === '1'`). + - The `throw` vs `console.warn` branch logic. + - The `snapshotPathTemplate` value (the workflow's artifact upload path depends on it). + - The two project names (`smoke`, `visual`) — workflow YAML invokes them verbatim. + - The two `testMatch` patterns — partitioning is the whole reason for two projects. + - The `trace: 'off'` setting (Risk R6). + - `BYPASS_SECRET ? { ... } : undefined` ternary — passing `extraHTTPHeaders: { 'x-vercel-protection-bypass': undefined }` would still send the header with the literal string `'undefined'`, which 401s loudly. The conditional is load-bearing. + +### `tests/visual/homepage.spec.ts` (new file) + +- [ ] Verbatim shape: + +```ts +import { test, expect } from '@playwright/test'; + +/** + * Visual baseline for the public homepage. + * + * FIRST RUN (no committed baseline yet): + * The Screenshot diff workflow runs `playwright test --project=visual + * --update-snapshots=none` (per .github/workflows/visual-diff.yml). With + * no baseline file at `tests/visual/__screenshots__/home.png` AND the + * `none` flag, this test FAILS — and that's the documented end state of + * the `adopt-playwright-smoke` convoy (Decision 4 in + * `.convoys/adopt-playwright-smoke.md`). The workflow's + * `continue-on-error: true` swallows the failure and the comment-on-PR + * step posts "Visual Diff — view run" with empty artifacts. + * + * SEEDING THE BASELINE (post-merge follow-up): + * Run `npm run test:visual:update` in a Linux environment so the + * generated PNG matches what CI will produce. The cleanest path is the + * Playwright Docker image: + * + * 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 `tests/visual/__screenshots__/home.png`. This is tracked + * as the `seed-visual-baselines-on-linux` follow-up convoy. + */ +const BASE = process.env.BASE_URL ?? 'http://localhost:3000'; + +test.describe('visual: public homepage', () => { + test('home renders consistently against baseline', async ({ page }) => { + await page.goto(BASE); + await expect(page).toHaveScreenshot('home.png'); + }); +}); +``` + + Notes: + + - `BASE` constant mirrors the existing `tests/smoke/app.smoke.spec.ts` shape (line 11). Don't read `process.env.BASE_URL` inside the test body. + - Snapshot name is the literal string `'home.png'`. The `snapshotPathTemplate` from `playwright.config.js` resolves this to `tests/visual/__screenshots__/home.png`. + - `test.describe` block name `'visual: public homepage'` matches the smoke spec's naming pattern (`'smoke: app boots and core pages render'`). + - Default viewport (1280×720) is fine for a first-pass baseline; do NOT set `viewport:` overrides this convoy. + - Do NOT add `await page.waitForLoadState('networkidle')` — networkidle is unreliable on a JS-heavy Next.js app and causes false flakes. `page.goto` already waits for `load` by default; that's enough for a homepage baseline. + - Do NOT add `{ fullPage: true }` to `toHaveScreenshot`. Default (viewport-only) keeps the baseline file small (~50KB) and easier to review on PRs. + +### `.gitignore` + +- [ ] Add the following block AFTER the existing `.code-review-graph/` line (current line 39): + +```gitignore + +# Playwright test runner artifacts (generated on every local run; +# never committed). Baselines under `tests/visual/__screenshots__/` +# are EXPLICITLY NOT ignored — they must be committed when they exist. +/playwright-report/ +/test-results/ +/.playwright/ +``` + + Three rules: + + 1. `/playwright-report/` — the HTML reporter's output (config `reporter` block). + 2. `/test-results/` — Playwright's default `--output` directory for trace/screenshot/video on failure. + 3. `/.playwright/` — Playwright's local browser cache and other internal state. + + Do NOT add `tests/visual/__screenshots__/` to `.gitignore`. Baselines are committed artifacts (per Decision 4 they don't exist yet, but when they do, they MUST be tracked). This is a boot-the-brief finding — see Finding 3 below. + +### Cross-file checks + +- [ ] **No secret leaks.** Grep before commit: + ```bash + rg -i 'console\.log.*VERCEL_AUTOMATION_BYPASS_SECRET' playwright.config.js tests/ + ``` + Expected: zero matches. The two intentional references (the `if (IS_CI && !BYPASS_SECRET)` `throw` and the `if (!BYPASS_SECRET && !IS_CI)` `console.warn`) name the env var but do NOT echo its value. +- [ ] **No baseline files committed.** Grep before commit: + ```bash + ls tests/visual/__screenshots__/ 2>/dev/null && echo "FAIL: directory exists with files; do NOT commit" || echo "OK: no baselines" + ``` + Expected: `OK: no baselines`. If you ran `npm run test:visual:update` locally to verify the spec resolves, delete the generated PNG before commit. +- [ ] **`npm run test:run` exit 0** (vitest baseline unchanged): + ```bash + npm run test:run + ``` + Expected: `Tests 21 passed (21)` per the existing vitest suite (16 auth + 5 Layout). If any vitest test fails, investigate before merge — this brief should be a pure additive change with no vitest impact. +- [ ] **`npm run lint` exit code unchanged** (Decision 5): + ```bash + npm run lint 2>&1 | tail -3 + ``` + Expected: `✖ 128 problems (81 errors, 47 warnings)` — the established baseline. If it grows by 1-2 new problems from the new visual spec, investigate (Risk R8); the architect's empirical check on the existing `.ts` smoke spec showed `exit 0` so the new spec should be silent too. +- [ ] **`npm run build` exit 0** — Playwright is a devDep and should not affect the Next.js build graph. Run as a smoke check: + ```bash + npm run build 2>&1 | tail -5 + ``` + Expected: build success (Turbopack compile ~1-2s, 23 static pages + 47 API routes per the post-`bump-next-js` baseline). +- [ ] **Diff hygiene.** `git diff main..HEAD --stat` should show only the 5 files listed in the brief frontmatter. No whitespace-only changes elsewhere. + +### Acceptance criterion #1 — end-state behavior + +After this brief lands on the convoy branch and a Vercel preview deployment is published for the PR: + +- [ ] **`Preview smoke` workflow:** wait-action succeeds in ≤90s (already proven by PR #17 at 194ms); `npm ci` succeeds; `npx playwright install --with-deps chromium` succeeds (~30-60s); `Run smoke tests` step REACHES `npx playwright test --project=smoke`; **3 smoke tests execute against the Vercel preview**. Pass/fail outcome: + - **Most likely PASS**: home renders (existing pages don't 5xx), `/login` renders (existing page), `/api/health` returns 2xx (verified handler). If any of these fail, the failure is a real signal — investigate before declaring the brief incomplete. + - Total workflow runtime: < 5 minutes (convoy success metric). +- [ ] **`Screenshot diff` workflow:** wait-action succeeds; `npx playwright install` succeeds; `Capture screenshots (PR)` step REACHES `npx playwright test --project=visual --update-snapshots=none`. **The single visual test FAILS** because no baseline exists. `continue-on-error: true` swallows the failure. `Upload screenshots + diffs` step uploads `tests/visual/__screenshots__/` (empty) + `test-results/` (contains the failure detail). `Comment on PR with diff link` step posts "## Visual Diff" comment with the run URL. **This is the documented end state of this convoy** per Decision 4 — operator follows up with the seed-on-Linux PR. +- [ ] **The bypass secret does NOT appear in any line of either workflow's run log.** Same verification pattern as the `fix-vercel-deployment-protection-in-ci` brief (download logs, grep for first 8 chars of the secret value, expect zero hits). GitHub Actions auto-masks registered secrets; this brief's contribution (Decision 2's `throw` and `console.warn` branches) does not interpolate the secret value into any string. + +## Manual verification (in addition to CI on push) + +Run these in order. Paste relevant output (with secrets redacted) into the PR description. + +- [ ] **Local install + lockfile parity.** + ```bash + npm install + npm ci # second run, verify lockfile is in sync + ``` + Expected: both succeed exit 0. `npm ci` is the critical one — if it errors, the lockfile is out of sync and the implementer needs to re-run `npm install` and re-commit. + +- [ ] **Config loads cleanly.** + ```bash + npx playwright --version + npx playwright test --list --project=smoke + npx playwright test --list --project=visual + ``` + Expected: + - `Version 1.60.x` (or later 1.60 patch). + - `--list --project=smoke` enumerates 3 tests from `tests/smoke/app.smoke.spec.ts`. + - `--list --project=visual` enumerates 1 test from `tests/visual/homepage.spec.ts`. + If `--list` shows specs in the wrong project, the `testMatch` patterns are wrong — fix before commit. + +- [ ] **Config Decision-2 fail-loud branch.** Simulate CI without the secret: + ```bash + unset VERCEL_AUTOMATION_BYPASS_SECRET + CI=true npx playwright test --list --project=smoke + ``` + Expected: exit code 1, error includes "VERCEL_AUTOMATION_BYPASS_SECRET is required in CI" and the `gh secret set` rotation command. If it does NOT error, the Decision 2 logic is wrong. + +- [ ] **Config Decision-2 warn-and-continue branch.** Simulate dev without the secret: + ```bash + unset VERCEL_AUTOMATION_BYPASS_SECRET + unset CI + npx playwright test --list --project=smoke + ``` + Expected: stderr includes the `[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ...` warning, command proceeds and lists tests with exit 0. + +- [ ] **Smoke spec runs against localhost.** In one terminal: + ```bash + npm run dev + ``` + In another: + ```bash + BASE_URL=http://localhost:3000 npm run test:smoke + ``` + Expected: 3 tests pass against the local Next.js dev server. If any fail locally, the spec body has a real issue OR a local config drift — investigate before pushing. + +- [ ] **Visual spec fail-on-missing-baseline behavior.** Against localhost: + ```bash + BASE_URL=http://localhost:3000 npx playwright test --project=visual --update-snapshots=none + ``` + Expected: 1 test FAILS with "A snapshot doesn't exist at tests/visual/__screenshots__/home.png". This is the desired Decision-4 behavior. If you accidentally generate a baseline locally with `--update-snapshots`, delete it before commit: + ```bash + rm -rf tests/visual/__screenshots__/ + ``` + +- [ ] **`.gitignore` actually ignores the right things.** After a local Playwright run (which creates `playwright-report/` and `test-results/`): + ```bash + git status --short + ``` + Expected: no `playwright-report/` or `test-results/` entries. If they appear, the `.gitignore` entries are wrong (missing leading `/` or wrong directory name). + +- [ ] **No bypass-secret leak in spec output.** After running `npm run test:smoke` with the bypass var set: + ```bash + VERCEL_AUTOMATION_BYPASS_SECRET=fake-secret-value BASE_URL=http://localhost:3000 \ + npm run test:smoke 2>&1 | grep -i 'fake-secret-value' && echo "FAIL: secret leaked" || echo "OK: secret not in output" + ``` + Expected: `OK: secret not in output`. (`fake-secret-value` is just a probe string for the grep; the real secret never enters this command.) + +- [ ] **Push and observe the first workflow run.** From `convoy/adopt-playwright-smoke`: + ```bash + git push -u origin HEAD + ``` + Then watch both workflows. Expect the end-state described in Acceptance criterion #1 above: smoke passes (or fails on a real assertion), visual fails on missing baseline + posts the comment. + +## Boot-the-brief findings (preempted by the architect; do not re-investigate) + +### Finding 1 — `@playwright/test@^1.60.0` resolves cleanly against the current dep tree + +`npm view @playwright/test version` → `1.60.0` (released 2026-05-11). No peer-dep conflicts with the existing `react@^18.3.1` / `next@^16.2.6` / `eslint@^9.39.4` graph (Playwright has no React or Next peers). The bundled Chromium binary downloaded by `npx playwright install --with-deps chromium` (already in both workflow steps — see `preview-smoke.yml` line 95 and `visual-diff.yml` line 79) matches the installed package version, so no version drift between the JS API and the browser binary. + +### Finding 2 — ESLint already parses `.ts` test files cleanly + +Verified 2026-05-24 on the convoy branch HEAD (`c8f1541`): + +```bash +$ npx eslint tests/smoke/app.smoke.spec.ts +$ echo $? +0 +``` + +Zero output, exit 0. The `eslint-config-next/core-web-vitals` chain bundled by `eslint-config-next@16` carries `typescript-eslint` as a hard dep (one of the reasons `typescript@^5.9.3` is in `devDependencies` per AGENTS.md Gotcha #9). The new `tests/visual/homepage.spec.ts` is structurally identical to the existing smoke spec (same imports, same patterns) so will also lint clean. **No `eslint.config.mjs` change is needed** (Decision 5). + +### Finding 3 — `tests/visual/__screenshots__/` MUST NOT be in `.gitignore` + +Default Playwright snapshot behavior: baselines are committed source-of-truth (visual regression depends on having a known-good reference). The convoy file's success metric requires the visual workflow's `Upload screenshots + diffs` step (visual-diff.yml line 88-96) to find files at `tests/visual/__screenshots__/` once baselines exist. Adding the path to `.gitignore` would silently break that step in any future PR that touches `pages/**` after baselines are committed in the follow-up convoy. + +### Finding 4 — `extraHTTPHeaders` applies to the test-level `request` fixture + +Per [Playwright fixtures docs](https://playwright.dev/docs/api/class-fixtures#fixtures-request), the test-level `request` fixture is "Isolated APIRequestContext instance for each test." Per [testOptions docs](https://playwright.dev/docs/api/class-testoptions), `extraHTTPHeaders` is "An object containing additional HTTP headers to be sent with **every request**." The testOptions surface is shared between browser context and APIRequestContext construction — the `request` fixture inherits the header. **This is why `tests/smoke/app.smoke.spec.ts`'s third test (`request.get('/api/health')`) reaches the protected preview without re-injecting the header in the spec body.** See Risk R1 in the convoy file's Architecture section for the hotfix path if this propagation breaks empirically. + +### Finding 5 — `pages/api/health.js` returns 200 anonymously + +The 7-line handler does NOT call `getUserFromRequest`, does NOT call `checkAuthRateLimit`, and does NOT gate on `req.method`. The Vercel preview's edge-protection layer terminates above the function: a request with the bypass header reaches the function as if anonymous, and the function returns `{ status: 'ok', ... }` with HTTP 200. The third smoke test (`request.get('/api/health')`) will pass cleanly against a Vercel preview, given the `extraHTTPHeaders` plumb (Finding 4). + +### Finding 6 — Repo `"type": "module"` requires ESM-shape `playwright.config.js` + +`package.json` line 5 declares `"type": "module"`. `vitest.config.js` and `next.config.js` both follow the ESM `import {...} from '...'; export default ...` shape. CommonJS-style `module.exports = { ... }` in `playwright.config.js` would throw at load time with `ReferenceError: module is not defined in ES module scope`. The spec'd `playwright.config.js` matches the ESM shape — do NOT regress to CommonJS. + +### Finding 7 — `snapshotPathTemplate` token reference + +Per [Playwright `testProject.snapshotPathTemplate` docs](https://playwright.dev/docs/api/class-testproject#test-project-snapshot-path-template), supported tokens are: `{arg}`, `{ext}`, `{projectName}`, `{snapshotDir}`, `{testDir}`, `{testFileDir}`, `{testFileName}`, `{testFilePath}`. **There is no `{platform}` or `{browserName}` token** — the default platform-suffix-in-filename behavior is built into the default template, and any custom template loses it. This is the structural reason for Risk R3 (cross-platform mismatch) and feeds Decision 4 (defer baseline commit to a Linux-Docker follow-up convoy). + +### Finding 8 — `--update-snapshots=none` semantics + +Per [Playwright CLI docs](https://playwright.dev/docs/test-cli): "Possible values are 'all', 'changed', 'missing', and 'none'. Running tests without the flag defaults to 'missing'; running tests with the flag but without a value defaults to 'changed'." Mode `none` (what `visual-diff.yml` line 82 uses) NEVER creates snapshots; missing snapshots fail the test. The workflow's `continue-on-error: true` + comment-on-PR step is the documented safety net. **First CI run of this brief on a touching-`pages/**` PR will produce a "Visual Diff — view run" comment with empty artifacts; that's the desired Decision-4 end state.** + +## Out of scope (do not do these) + +- [ ] Do not rename `tests/smoke/app.smoke.spec.ts` to `.js` (Decision 1). +- [ ] Do not commit any baseline image to `tests/visual/__screenshots__/` (Decision 4). +- [ ] Do not edit `eslint.config.mjs` (Decision 5 + Finding 2). +- [ ] Do not edit any `.github/workflows/*.yml` file (those are owned by other convoys; the brief's job is to MAKE the YAML's `npx playwright test` invocation work, not to modify the YAML). +- [ ] Do not edit `AGENTS.md` (doc-writer pass at convoy close owns § 7 updates and the seed-on-Linux instructions). +- [ ] Do not add a `test:smoke:local` or `test:e2e` wrapper script that boots `next dev` automatically (Decision 6). +- [ ] Do not add a `vitest` watch/setup change to support Playwright — the two runners stay independent (Test plan section of the convoy file). +- [ ] Do not enable `trace: 'on'` or `trace: 'retain-on-failure'` in `playwright.config.js`'s `use:` block (Risk R6). +- [ ] Do not add a `webServer:` block to `playwright.config.js` (auto-starts Next.js — explicitly rejected by Decision 6). +- [ ] Do not pin `@playwright/test` to an exact version (`1.60.0` without the `^`) — patch upgrades are desired (Risk R9 mitigation path). +- [ ] Do not add a `tsconfig.json` to the repo just because the new spec is `.ts`. Lint already accepts it; tsc-noEmit is not run in CI; the JS-only repo policy (AGENTS.md Gotcha #9) explicitly defers TypeScript adoption to a separate convoy. +- [ ] Do not bump `node-version: '20'` in any workflow (out of scope; Vercel default is 20). +- [ ] Do not run `npm audit fix` as part of this brief. If `npm install` surfaces audit warnings, note them in the PR description but do NOT take action — audit churn is a separate concern. + +## Rationale (≤3 sentences) + +The post-PR-17 workflows already invoke `npx playwright test --project={smoke,visual}` and already export `BASE_URL` + `VERCEL_AUTOMATION_BYPASS_SECRET` to the test step's `env:` — this brief is the small bridge that makes those invocations actually find a config, a dep, and a runnable visual spec. Bundling the visual spec with the config in one brief is the right call because they're semantically coupled (the visual spec depends on the `visual` project being declared in the config, which depends on `@playwright/test` being installed); splitting would force interim states that don't compile or run. Decision 4 (no baselines now) and Decision 6 (no auto-boot wrapper) explicitly keep the surface area small so the convoy ships in one PR; baseline curation and DX wrappers are queued as follow-up convoys (`seed-visual-baselines-on-linux`, possibly `adopt-test-smoke-local`) that operators can take or leave based on actual usage friction. -- 2.45.2 From c72d006666d5a7c1e9107872598ee44608c07588 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sun, 24 May 2026 18:44:52 -0500 Subject: [PATCH 3/3] feat(test): adopt @playwright/test + ship playwright.config.js + visual scaffold (Brief 1 of adopt-playwright-smoke) Closes P1 #10 step 2 / launch sequence step 10. PR #17 already plumbed VERCEL_AUTOMATION_BYPASS_SECRET into both workflows' env; this brief wires the actual @playwright/test dep, the playwright.config.js that bridges env -> use.extraHTTPHeaders, and a single visual spec so the screenshot workflow has something to discover. Per architect Decision 1 - tests/smoke/app.smoke.spec.ts stays as .ts (npx eslint exits 0 against the current config); new visual spec also .ts. Per Decision 2 - playwright.config.js fail-loud in CI when VERCEL_AUTOMATION_BYPASS_SECRET is missing (throw at config load with actionable message naming the env var, rotation command, AGENTS.md reference); warn-and-no-op in dev. Mirrors lib/rate-limit.js fail-closed pattern (AGENTS.md Gotcha #12). Per Decision 3 - two projects partitioned by testMatch: - smoke -> tests/smoke/**/*.spec.{ts,js} - visual -> tests/visual/**/*.spec.{ts,js} Shared use: block. Matches the workflows' --project=smoke|visual invocations. Per Decision 4 - NO baselines committed. The Mac-vs-Linux platform- suffix issue (architect Boot-the-brief Finding 7: Playwright's custom snapshotPathTemplate has no {platform} token) means a Mac-generated baseline would silently overwrite a Linux CI baseline. Queued as seed-visual-baselines-on-linux follow-up convoy. First CI run of Screenshot diff will fail at the test step; existing continue-on-error: true swallows it and the comment-on-PR step posts the run link. Per Decision 5 - no eslint.config.mjs change. Per Decision 6 - three simple scripts (test:smoke, test:visual, test:visual:update). No auto-boot wrapper. Verification: - npm ci --dry-run exits 0 (lockfile in sync) - npm run lint: 128 problems (baseline match - 81 errors, 47 warnings) - npm run test:run: 21/21 vitest pass (no regression) - npm run test:smoke (no BASE_URL): dev-path warn-and-continue fires "[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset" warning, enumerates 3 smoke tests, attempts to run (fails on missing local chromium binary - workflows install browsers via npx playwright install --with-deps chromium) - CI=true npm run test:smoke: CI-path throws at config load with "VERCEL_AUTOMATION_BYPASS_SECRET is required in CI to reach Vercel-Protection-protected preview deployments. Reseed via: gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body \"\". See AGENTS.md \xc2\xa7 7 for the full plumbing context." - npx playwright --version: Version 1.60.0 Implementation note: removed the architect's defensive eslint-disable- next-line no-console directive in the dev-warn branch because the active eslint config does not enable no-console for repo-root config files; ESLint reported the directive as unused, which would have regressed the lint baseline by +1. Behavior unchanged; comment-only edit per the brief's explicit "comment wording can be tightened" allowance. Replaced with an explanatory comment noting why the directive is intentionally absent. Implementation note 2: alphabetized @playwright/test as the FIRST entry in devDependencies (before @testing-library/dom) because "@p" < "@t". The brief's prose said "between @testing-library/react and autoprefixer" which would have broken alphabetization; followed the brief's overarching principle ("alphabetical position") instead. No code paths in pages/** or lib/** touched. No workflow YAML edited. No AGENTS.md edits (doc-writer pass at convoy close handles the seed-on-Linux runbook). Co-authored-by: Cursor --- .gitignore | 7 ++ package-lock.json | 64 ++++++++++++++++ package.json | 6 +- playwright.config.js | 133 ++++++++++++++++++++++++++++++++++ tests/visual/homepage.spec.ts | 37 ++++++++++ 5 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 playwright.config.js create mode 100644 tests/visual/homepage.spec.ts diff --git a/.gitignore b/.gitignore index b928a15..d16380b 100644 --- a/.gitignore +++ b/.gitignore @@ -36,3 +36,10 @@ __pycache__/ # Local code-knowledge-graph (per-developer; user-code-review-graph MCP) .code-review-graph/ + +# Playwright test runner artifacts (generated on every local run; +# never committed). Baselines under `tests/visual/__screenshots__/` +# are EXPLICITLY NOT ignored — they must be committed when they exist. +/playwright-report/ +/test-results/ +/.playwright/ diff --git a/package-lock.json b/package-lock.json index 7d43838..8f2f2e7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -23,6 +23,7 @@ "resend": "^4.7.0" }, "devDependencies": { + "@playwright/test": "^1.60.0", "@testing-library/dom": "^10.4.1", "@testing-library/react": "^16.3.2", "autoprefixer": "^10.4.21", @@ -2071,6 +2072,22 @@ "node": ">=14" } }, + "node_modules/@playwright/test": { + "version": "1.60.0", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.60.0.tgz", + "integrity": "sha512-O71yZIbAh/PxDMNGns37GHBIfrVkEVyn+AXyIa5dOTfb4/xNvRWV+Vv/NMbNCtODB/pO7vLlF2OTmMVLhmr7Ag==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright": "1.60.0" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/@react-email/render": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/@react-email/render/-/render-1.1.2.tgz", @@ -7531,6 +7548,53 @@ "node": ">= 6" } }, + "node_modules/playwright": { + "version": "1.60.0", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.60.0.tgz", + "integrity": "sha512-hheHdokM8cdqCb0lcE3s+zT4t4W+vvjpGxsZlDnikarzx8tSzMebh3UiFtgqwFwnTnjYQcsyMF8ei2mCO/tpeA==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.60.0" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "fsevents": "2.3.2" + } + }, + "node_modules/playwright-core": { + "version": "1.60.0", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.60.0.tgz", + "integrity": "sha512-9bW6zvX/m0lEbgTKJ6YppOKx8H3VOPBMOCFh2irXFOT4BbHgrx5hPjwJYLT40Lu+4qtD36qKc/Hn56StUW57IA==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=18" + } + }, + "node_modules/playwright/node_modules/fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, "node_modules/possible-typed-array-names": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/possible-typed-array-names/-/possible-typed-array-names-1.1.0.tgz", diff --git a/package.json b/package.json index c1c6e53..db16ec0 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,10 @@ "import-popular": "node scripts/import-popular-sets.js", "import-all": "node scripts/bulk-import-all.js", "test": "vitest", - "test:run": "vitest run" + "test:run": "vitest run", + "test:smoke": "playwright test --project=smoke", + "test:visual": "playwright test --project=visual", + "test:visual:update": "playwright test --project=visual --update-snapshots" }, "dependencies": { "@neondatabase/serverless": "^1.0.1", @@ -30,6 +33,7 @@ "resend": "^4.7.0" }, "devDependencies": { + "@playwright/test": "^1.60.0", "@testing-library/dom": "^10.4.1", "@testing-library/react": "^16.3.2", "autoprefixer": "^10.4.21", diff --git a/playwright.config.js b/playwright.config.js new file mode 100644 index 0000000..10759d4 --- /dev/null +++ b/playwright.config.js @@ -0,0 +1,133 @@ +// Playwright config for the post-PR-17 `Preview smoke` and +// `Screenshot diff` workflows. ESM per the repo's +// `"type": "module"` setting in package.json. Companion docs: +// `.convoys/adopt-playwright-smoke.md` (Decisions 1-6), +// AGENTS.md § 7 (Vercel preview bypass conventions). + +import { defineConfig } from '@playwright/test'; + +const BASE_URL = process.env.BASE_URL ?? 'http://localhost:3000'; +const BYPASS_SECRET = process.env.VERCEL_AUTOMATION_BYPASS_SECRET; + +// `CI === 'true'` is the canonical GitHub Actions signal (set by +// the runner). Playwright config has no Next.js context, so +// `NODE_ENV` is not reliable here. Mirrors the rate-limit.js +// fail-closed pattern documented in AGENTS.md Gotcha #12. +const IS_CI = process.env.CI === 'true'; + +if (IS_CI && !BYPASS_SECRET) { + // Fail loud in CI per Decision 2. The workflow's `env:` block + // (preview-smoke.yml line 101, visual-diff.yml line 85) maps + // `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` into the process + // env; if it's empty here, the secret is unseeded or the + // workflow YAML drift broke the mapping. + throw new Error( + 'VERCEL_AUTOMATION_BYPASS_SECRET is required in CI to reach ' + + 'Vercel-Protection-protected preview deployments. ' + + 'Reseed via: gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "". ' + + 'See AGENTS.md § 7 for the full plumbing context.' + ); +} + +if (!BYPASS_SECRET && !IS_CI) { + // Dev fallback per Decision 2 — warn once at config load, + // proceed without the header. Local runs target localhost + // (no preview protection) or a non-protected URL. + // (No eslint-disable for `no-console` here: the active config + // does not enable `no-console` for repo-root config files; + // adding the directive here trips `eslint-comments/no-unused-disable`.) + console.warn( + '[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ' + + 'running without the Vercel bypass header. Targets a non-protected ' + + 'URL (e.g. http://localhost:3000). Hitting a protected preview without ' + + 'this header will return Vercel\'s SSO challenge page.' + ); +} + +export default defineConfig({ + // Both `tests/smoke/` and `tests/visual/` live under `tests/`. + // Project-level `testMatch` (below) partitions them so the + // two workflows (`--project=smoke` and `--project=visual`) + // each see only the specs they should run. + testDir: './tests', + + // Smoke + visual specs are independent; parallelism within a + // single spec adds no value here and would complicate the + // per-test screenshot baseline lifecycle. + fullyParallel: false, + workers: IS_CI ? 1 : undefined, + + // One retry in CI handles transient Vercel preview flakes + // (cold-start, DNS propagation). Local: zero retries — fail + // fast so the dev sees the issue immediately. + retries: IS_CI ? 1 : 0, + + // 30s per test is plenty for the 3 smoke checks + 1 visual + // screenshot. The convoy file's success metric is < 5min + // total workflow runtime; per-test 30s is well inside that. + timeout: 30_000, + expect: { timeout: 10_000 }, + + // List reporter in dev for human readability; add HTML in CI + // so the `Upload Playwright report on failure` step + // (preview-smoke.yml line 104) has a populated `playwright-report/` + // to upload. `open: 'never'` keeps the HTML from auto-launching + // a browser tab in headless CI. + reporter: IS_CI ? [['list'], ['html', { open: 'never' }]] : 'list', + + // Visual baselines live at `tests/visual/__screenshots__/{ext}`. + // Workflow `visual-diff.yml` line 94 uploads this exact path as the + // artifact — keep them aligned. {arg} is the snapshot name from + // `toHaveScreenshot('home.png')` without the extension; {ext} is + // the extension with the leading dot. + // + // Cross-platform note (Risk R3): this template drops Playwright's + // default `--` suffix. That means a Mac dev + // running `npm run test:visual:update` overwrites the Linux-CI + // baseline. Per Decision 4, we don't commit baselines this convoy; + // operator seeds via `mcr.microsoft.com/playwright:v1.60.0-noble` + // Docker on Linux. Multi-platform support is the + // `seed-visual-baselines-on-linux` follow-up convoy's job. + snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}', + + use: { + baseURL: BASE_URL, + // Headers apply to BOTH browser `page.goto(...)` calls AND the + // test-level `request` fixture's APIRequestContext (verified + // against Playwright docs: `testOptions.extraHTTPHeaders` is + // shared between browser context and APIRequestContext + // construction). This is why `tests/smoke/app.smoke.spec.ts`'s + // third test (`request.get('/api/health')`) reaches the + // protected preview without re-injecting the header in the + // spec body. If a future hotfix shows the header NOT + // propagating to APIRequestContext, see Risk R1 in the + // convoy file's Architecture section. + extraHTTPHeaders: BYPASS_SECRET + ? { 'x-vercel-protection-bypass': BYPASS_SECRET } + : undefined, + // Trace OFF this convoy. Enabling it would land the bypass + // header in the HAR payload (Risk R6); a future polish convoy + // owns the trace-on + HAR-sanitization decision. + trace: 'off', + screenshot: 'off', + video: 'off', + }, + + projects: [ + { + // `Preview smoke` workflow invokes `--project=smoke` (per + // preview-smoke.yml line 98). The testMatch keeps the + // visual specs out of this project. + name: 'smoke', + testMatch: 'smoke/**/*.spec.@(ts|js)', + }, + { + // `Screenshot diff` workflow invokes `--project=visual` + // (per visual-diff.yml line 82). The testMatch keeps the + // smoke specs out of this project (so a `--project=visual` + // run doesn't redundantly execute the smoke tests). + name: 'visual', + testMatch: 'visual/**/*.spec.@(ts|js)', + }, + ], +}); diff --git a/tests/visual/homepage.spec.ts b/tests/visual/homepage.spec.ts new file mode 100644 index 0000000..9cab621 --- /dev/null +++ b/tests/visual/homepage.spec.ts @@ -0,0 +1,37 @@ +import { test, expect } from '@playwright/test'; + +/** + * Visual baseline for the public homepage. + * + * FIRST RUN (no committed baseline yet): + * The Screenshot diff workflow runs `playwright test --project=visual + * --update-snapshots=none` (per .github/workflows/visual-diff.yml). With + * no baseline file at `tests/visual/__screenshots__/home.png` AND the + * `none` flag, this test FAILS — and that's the documented end state of + * the `adopt-playwright-smoke` convoy (Decision 4 in + * `.convoys/adopt-playwright-smoke.md`). The workflow's + * `continue-on-error: true` swallows the failure and the comment-on-PR + * step posts "Visual Diff — view run" with empty artifacts. + * + * SEEDING THE BASELINE (post-merge follow-up): + * Run `npm run test:visual:update` in a Linux environment so the + * generated PNG matches what CI will produce. The cleanest path is the + * Playwright Docker image: + * + * 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 `tests/visual/__screenshots__/home.png`. This is tracked + * as the `seed-visual-baselines-on-linux` follow-up convoy. + */ +const BASE = process.env.BASE_URL ?? 'http://localhost:3000'; + +test.describe('visual: public homepage', () => { + test('home renders consistently against baseline', async ({ page }) => { + await page.goto(BASE); + await expect(page).toHaveScreenshot('home.png'); + }); +}); -- 2.45.2