deckhearth/.convoys/adopt-playwright-smoke.md

---
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: <secret>`) 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 "<value>"`), 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:
  `<arg>-<browserName>-<platform>.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://<preview>.vercel.app \
  VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
  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>.spec.ts-snapshots/<arg>-<browser>-<platform>.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 `<Link href="/login">Sign in</Link>`
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 "<new value>"`).
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`).