--- 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.