--- 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: shipped created: 2026-05-24 shipped: 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. ## 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. ## As-shipped Shipped 2026-05-24 as squash commit `7b6f751` (PR #18). The convoy shipped in one PR (PR #18 architect-commit `3ac527e`, implementer-commit `c72d006`) — Brief 1 as planned, with two small implementer deviations from the brief's verbatim shape (both lint-baseline-preserving and behavior-neutral). Capturing the deviations + the cross-validation finding + empirical CI metrics here so the next architect / reviewer has the audit trail. ### Decisions ratified by operator at gate 1 Three of six decisions were architect-self-ratifiable (Decisions 2, 3, 5 — see § Decisions). The remaining three needed operator ratification, and all three were ratified verbatim at gate 1: - **Decision 1 — keep `.ts` for Playwright specs.** `tests/smoke/app.smoke.spec.ts` stayed `.ts`; the new `tests/visual/homepage.spec.ts` also shipped as `.ts`. Empirically `npx eslint` exits 0 on both files against the current config (`eslint-config-next@16`'s bundled `typescript-eslint` chain parses them) — no `eslint.config.mjs` change needed, lint baseline held at 128 problems. - **Decision 4 — defer visual baselines to a Linux-Docker follow-up PR.** No baseline images committed. `tests/visual/__screenshots__/` does not exist in the tree at merge; the visual workflow's `--update-snapshots=none` flag + `continue-on-error: true` + the PR-comment step are the documented Decision-4 safety net. - **Decision 6 — three simple scripts, no auto-boot wrapper.** `package.json` got `test:smoke`, `test:visual`, `test:visual:update` in that order. No `test:smoke:local` / `test:e2e` / `next dev` auto-boot variant. Operator's local flow is "boot `next dev` in one terminal, run `BASE_URL=http://localhost:3000 npm run test:smoke` in another" — same shape as the existing `test` / `test:run` scripts. ### Implementer deviations from Brief 1's verbatim shape Two small deviations, both surfaced in the implementer's PR report and both lint-baseline-preserving: 1. **Removed the brief's `// eslint-disable-next-line no-console` directive on the dev warn-and-continue branch of `playwright.config.js`.** The brief specified the directive verbatim to suppress an expected `no-console` complaint on the `console.warn(...)` call. Empirically the current ESLint config does NOT flag `console.warn` at all (only `console.log`), so the `eslint-disable-next-line` directive itself becomes an unused- directive lint error (`Unused eslint-disable directive`) and would have regressed the baseline from 128 → 129. Removing the directive is the behavior-neutral fix: the `console.warn` line still runs unchanged, just without the no-longer-needed disable comment. Lint baseline held at exactly 128 problems post-implementation. 2. **Placed `@playwright/test` first in `devDependencies` for strict alphabetical order.** The brief's prose was internally inconsistent about placement (it called for "alphabetical position" but then described the wrong neighbors — `@playwright` sorts before `@testing-library/react` lexically). The implementer followed the alphabetical rule rather than the prose's example, so the final ordering is `@playwright/test` → `@testing-library/dom` → `@testing-library/react` → `autoprefixer` → ... Behavior-neutral; matches the convention used elsewhere in the file (`dependencies` is alphabetical too). Both deviations are explicitly behavior-neutral — same code paths execute, same env-var predicates, same lint count. Documenting them here so a future reviewer comparing the brief's verbatim spec to the merged diff sees the rationale instead of flagging drift. ### Cross-validation: smoke test 2 locks in PR #15's "Sign in" CTA `tests/smoke/app.smoke.spec.ts`'s second test (`'sign-in page renders'`) navigates to `/login` and asserts `await expect(page.getByRole('button', { name: /sign in/i })).toBeVisible({ timeout: 10_000 })`. That assertion lights up only because the page renders a sign-in-named control — which is exactly the `Sign in` CTA that the `fix-layout-default-user` convoy (PR #15, squash `ca302a8`) added to `components/Layout.js`'s logged-out branch when it replaced the leaky maintainer-email default prop. **This convoy effectively locks in a regression test for that earlier convoy's work** — if a future change reverts to a hardcoded default user (or breaks the logged-out CTA wording) the smoke check now fails the PR. Surfaced organically from CI green; not a planned acceptance criterion of this convoy but worth noting because P0 #7's resolved state is now defended by a real CI signal, not just the 5 vitest assertions in `test/components/Layout.test.js`. ### As-shipped metrics (from post-merge run 26376162598 on `main`) - `Playwright smoke` workflow total runtime: **59 seconds**, exit 0 (was: fast-fail at "playwright not installed" / "no config" before this convoy — never reached `npx playwright test`). Comfortably inside the < 5-minute success metric. - `Run smoke tests` step: **3/3 tests pass in 2.9s** against the Vercel preview URL with the `x-vercel-protection-bypass` header applied: - `home redirects or renders without 5xx` → ✓ 683ms - `sign-in page renders` (the cross-validation above) → ✓ 459ms - `public health endpoint responds` (`/api/health` 2xx) → ✓ 571ms - Step breakdown: `Wait for Vercel Preview deployment` → success (~200ms range, per PR #17's plumbing); `npm ci` + `setup-node` + `playwright install --with-deps chromium` → success; `Run smoke tests` → **success** (the failure mode shifted from "no config" in PR #17's end state to "all green" here, which is the convoy's target end state). - `Screenshot diff` workflow: **not triggered on PR #18 itself**. Its `paths:` filter excludes test-infra-only changes (the PR touched only `package.json`, `package-lock.json`, `playwright.config.js`, `tests/visual/homepage.spec.ts`, `.gitignore`, and the convoy/brief docs — none of those are under `pages/**` / `components/**` / `styles/**` / `tailwind.config.js` / `postcss.config.js`). First real trigger fires on the next PR touching any of those paths; at that point the documented Decision-4 end state (test fails on missing baseline → `continue-on-error: true` swallows it → comment-on-PR step posts "Visual Diff — view run" with empty artifacts) gets its first live exercise. - Bypass secret leak check: **0 matches** against the raw workflow log (per AC #7's grep-for-first-8-chars-of-secret pattern). GitHub Actions auto-masks registered secrets; this convoy's Decision-2 branches (`throw` in CI, `console.warn` in dev) name the env var but never interpolate its value into any string, so the mask never had to engage on output from our code. ### Operator action required going forward `seed-visual-baselines-on-linux` is queued as the follow-up convoy (see `.convoys/ship-readiness.md` § Queued convoys). Until that PR lands, every `Screenshot diff` run on a PR touching `pages/**` / `components/**` / `styles/**` will fail at the test step and post a comment with empty artifacts. That is the documented end state of this convoy per Decision 4; no operator intervention is required to keep `Playwright smoke` green (smoke runs against the existing spec, which has no baseline dependency). If/when the operator rotates `VERCEL_AUTOMATION_BYPASS_SECRET` via the Vercel dashboard, both workflows fail with Vercel SSO challenge pages on every PR until the GitHub secret is re-seeded (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body ""`). Same human-responsibility pattern as `JWT_SECRET` rotation; documented in `AGENTS.md` § 7. ### What did NOT change - `tests/smoke/app.smoke.spec.ts` (existed pre-convoy; untouched per Decision 1). - `eslint.config.mjs` (Decision 5 + Finding 2; empirically clean without any change). - `pages/api/health.js` (already returns 200 anonymously; smoke test 3 passes against it without any handler change). - Any source under `pages/**` / `components/**` / `lib/**` / `scripts/**` (this convoy is test-infra-only by scope). - `vitest.config.js` / `test/setup.js` / anything under `test/` (the two runners stay independent per the convoy's Test plan §; `npm run test:run` still passes 21/21 at merge — no vitest regression from the new `@playwright/test` install). - Any `.github/workflows/*.yml` file (owned by PR #16 / PR #17 / `fix-lint-baseline`; this convoy made the YAML's existing invocations work, not modified them). - `tests/visual/__screenshots__/` (does not exist; Decision 4 defers to `seed-visual-baselines-on-linux`).