Convoy file:
- status: queued -> in-progress
- New § Decisions (post-IA round): 6 decisions investigated.
Architect-self-ratifiable: D2 (CI fail-loud predicate mirrors
rate-limit.js convention), D3 (two projects is a structural
workflow requirement), D5 (no eslint change — verified `npx
eslint tests/smoke/app.smoke.spec.ts` exits 0). Operator-
ratifiable at gate 1: D1 (recommend keep .ts; convention call),
D4 (recommend defer baseline commit to Linux-Docker follow-up;
process call), D6 (recommend simple test:smoke/test:visual
scripts, no auto-boot wrapper; DX call).
- New § Architecture: file plan (5 files), API surface (n/a),
schema diff (n/a), test plan (3 smoke + 1 visual; vitest 21/21
+ lint baseline 128 held), risk list R1-R9 (request fixture
header propagation, snapshotPathTemplate alignment, cross-
platform mismatch, --update-snapshots=none semantics, CI
predicate variants, bypass trace leak, npm ci lockfile sync,
eslint future drift, Playwright 1.60.0 freshness).
- New § Decomposition: 1 brief, ~120 LOC, slice_dependencies
YAML block. Justified single brief because total LOC is
small and all files are semantically coupled (visual spec
depends on visual project in config which depends on the
installed dep).
Brief file (.convoys/adopt-playwright-smoke/brief-1-*.md):
- Goal/Files/Conventions/Acceptance/Manual-verification/
Boot-the-brief findings/Out-of-scope sections matching the
fix-vercel-deployment-protection-in-ci/brief-1 shape.
- 8 Boot-the-brief findings: dep resolution clean, .ts lints
clean, baselines dir MUST NOT be gitignored, extraHTTPHeaders
propagates to request fixture per docs, /api/health is
anonymous, ESM shape required, snapshotPathTemplate token set
has no {platform}, --update-snapshots=none never auto-creates.
- Verbatim playwright.config.js shape with CI fail-loud / dev
warn predicate, snapshotPathTemplate aligned with workflow's
artifact upload path, two projects with testMatch partitioning.
No code changes (read-only architect pass). No push.
Co-authored-by: Cursor <cursoragent@cursor.com>
659 lines
32 KiB
Markdown
659 lines
32 KiB
Markdown
---
|
||
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: in-progress
|
||
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: <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.
|