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.