deckhearth/.convoys/adopt-playwright-smoke.md
Randall Stillwell eefc390636 docs: post-convoy cleanup for adopt-playwright-smoke
Reflects the merged adopt-playwright-smoke convoy (PR #18, squash
commit 7b6f751) in repo documentation. Closes the test-infrastructure
side of P1 #10 step 2 (launch sequence step 10). One commit in the
convoy: Brief 1, with two small lint-baseline-preserving deviations
from the brief's verbatim shape that the implementer report flagged.

.convoys/adopt-playwright-smoke.md:
  - frontmatter status: in-progress -> shipped (added shipped:
    2026-05-24)
  - new ## As-shipped section: operator-ratified Decisions (D1 keep
    .ts, D4 defer baselines, D6 simple scripts); two implementer
    deviations (removed unused eslint-disable-no-console directive
    that would have regressed lint 128 -> 129; placed @playwright/test
    first in devDeps for alphabetical correctness); cross-validation
    that smoke test 2 ("sign-in page renders") locks in PR #15's
    logged-out CTA work in components/Layout.js; empirical metrics
    from post-merge run 26376162598 (59s workflow, 3/3 in 2.9s,
    0-leak); operator-action-required note pointing at the queued
    seed-visual-baselines-on-linux follow-up; What did NOT change
    audit trail.

.convoys/ship-readiness.md:
  - Queued convoys: new entry seed-visual-baselines-on-linux (Linux-
    Docker baseline generation per Decision 4 + Boot-the-brief
    Finding 7; Mac-generated baselines would silently overwrite Linux
    CI baselines because the custom snapshotPathTemplate has no
    {platform} token).
  - Queued convoys: new RESOLVED block for adopt-playwright-smoke
    (PR #18, 7b6f751) — as-shipped surface, implementer deviations,
    empirical metrics (59s workflow / 3/3 in 2.9s / 0 secret leaks),
    the PR #15 cross-validation finding, operator-action-required
    going forward (the seed-visual-baselines-on-linux follow-up),
    flagged-but-deferred items, and ownership trail (3 architect-
    self-ratified decisions + 3 operator-ratified).
  - Launch sequence step 10: marked RESOLVED 2026-05-24 with the
    commit + metrics inline.
  - P1 #10 No tests Fix sequence: step 2 marked RESOLVED with the
    convoy + metrics ref; step 3 (re-enable test: job in
    ci.yml) called out as the next remaining task; step 5 (wire
    preview-smoke.yml to the Vercel preview URL) marked RESOLVED
    across PR #17 + PR #18 since both contributed.

AGENTS.md:
  - Section 6 Testing: rewritten end-to-end. Was "E2E/smoke runner
    still pending"; is now "@playwright/test@^1.60.0 wired, two
    projects (smoke + visual), npm run test:smoke / test:visual /
    test:visual:update". Documents the local-run convention (boot
    next dev separately, then BASE_URL=... npm run test:smoke);
    the one-time npx playwright install --with-deps chromium step;
    the no-baselines-yet state + the Linux-Docker seed command +
    the cross-platform mismatch reason (no {platform} token in
    snapshotPathTemplate); the CI behavior split (vitest blocking,
    smoke on every PR with pipeline:skip-smoke escape hatch,
    Screenshot diff path-filtered with the first-red-on-missing-
    baseline state documented). Updates vitest coverage count
    16 -> 21 (the +5 Layout regression-lock tests from PR #15).
    Notes TESTING_GUIDE.md is being eclipsed and will be renamed
    to docs/MANUAL_QA.md in a future cleanup convoy.
  - Section 7 Deployment: rewrites the Vercel-bypass paragraph from
    a single "query param now / header reserved for future" bullet
    into a two-shape audit ((1) query param on the wait-action's
    path: input per PR #17; (2) HTTP header in playwright.config.js's
    use.extraHTTPHeaders per PR #18). Documents the Decision-2
    fail-loud-in-CI / warn-in-dev predicate and references Gotcha #12
    as the established precedent (lib/rate-limit.js). Picked Section
    7 over a new Gotcha because the bypass plumbing is operationally
    a deployment concern, not an app pitfall.

No changes to: package.json, package-lock.json, playwright.config.js,
eslint.config.mjs, lib/**, pages/**, components/**, scripts/**,
.github/workflows/**, .cursor/rules/**, README.md,
tests/visual/homepage.spec.ts (JSDoc is already neutral-tense, no
future-tense references to clean up).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-24 19:33:43 -05:00

40 KiB
Raw Permalink Blame History

name classification success_metric skip status created shipped parent addresses depends_on
adopt-playwright-smoke convoy `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.
role-design-system-auditor
role-a11y-auditor
role-ux-reviewer
role-ia-architect
shipped 2026-05-24 2026-05-24 ship-readiness P1
fix-vercel-deployment-protection-in-ci (shipped — PR
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:

  • smoketestMatch: 'smoke/**/*.spec.@(ts|js)'
  • visualtestMatch: '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 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:

# 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, 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)

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/reactautoprefixer → ... 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 testssuccess (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).