deckhearth/.convoys/adopt-playwright-smoke/brief-1-wire-playwright-and-visual-spec.md
varutasu 7b6f7519b2
feat(test): adopt @playwright/test + ship playwright.config.js + visual scaffold (P1 #10 step 2)
Closes P1 #10 step 2. Adds @playwright/test@^1.60.0, playwright.config.js (CI fail-loud / dev warn-and-continue for missing VERCEL_AUTOMATION_BYPASS_SECRET, two projects partitioned by testMatch, snapshotPathTemplate aligned with workflow upload path), tests/visual/homepage.spec.ts (1 test, no baseline committed per architect Boot-the-brief Finding 7), 3 npm scripts, and 3 .gitignore entries. Smoke tests now run end-to-end against Vercel preview with x-vercel-protection-bypass header: 3/3 passed in 2.9s, total workflow 59s. Zero secret leaks in log. PR #18 architect-commit 3ac527e, implementer-commit c72d006.
2026-05-24 19:25:18 -05:00

33 KiB
Raw Permalink Blame History

convoy brief_number depends_on files
adopt-playwright-smoke 1
package.json
package-lock.json
playwright.config.js
tests/visual/homepage.spec.ts
.gitignore

Brief 1: Install @playwright/test, ship playwright.config.js, add the homepage visual spec — so the post-PR-17 Preview smoke and Screenshot diff workflows reach npx playwright test and execute against the Vercel preview

Goal (1 sentence)

Wire @playwright/test@^1.60.0 into devDeps, create playwright.config.js with two projects (smoke + visual) and a CI-gated x-vercel-protection-bypass header per playwright.config Decision 2, add a single tests/visual/homepage.spec.ts baseline-bearing screenshot test per Decision 3, add three npm scripts per Decision 6, and ignore Playwright's local-run artifacts in .gitignore — without renaming or editing the existing tests/smoke/app.smoke.spec.ts, without touching eslint.config.mjs, and without committing any baseline images (operator seeds those in a follow-up Linux-Docker run per Decision 4).

Files in scope (do not edit anything else)

  • package.json — modified.
  • package-lock.json — modified (regenerated by npm install; commit in the SAME commit as package.json to keep npm ci happy in CI per Risk R7).
  • playwright.config.js — new (root level).
  • tests/visual/homepage.spec.ts — new.
  • .gitignore — modified.

Files explicitly out of scope (do not touch even if it seems related):

  • tests/smoke/app.smoke.spec.ts — exists, stays as-is per Decision 1. NOT renamed to .js. NOT edited.
  • eslint.config.mjs — empirically verified to parse the existing .ts spec cleanly (Decision 5 + Boot-the-brief finding #2 below). NOT touched.
  • vitest.config.js, test/setup.js, any file under test/ (vitest's home) — different runner; out of scope.
  • .github/workflows/preview-smoke.yml, .github/workflows/visual-diff.yml, .github/workflows/ci.yml — all workflow YAML is owned by other convoys (PR #17 / PR #16 / fix-lint-baseline); zero touches here.
  • AGENTS.md § 7 (the seed-on-Linux workflow + the local-run command table) — that's the doc-writer pass at convoy close, NOT this brief.
  • tests/visual/__screenshots__/ — do NOT create or commit baselines in this convoy per Decision 4. Operator runs npm run test:visual:update in a Linux Docker env (mcr.microsoft.com/playwright:v1.60.0-noble) and commits in a follow-up seed-visual-baselines-on-linux PR.
  • pages/api/health.js — already exists and already returns 200 anonymously (verified by reading the 7-line source). Do NOT modify or move.
  • next.config.js, tailwind.config.js, postcss.config.js — unrelated; zero touches.

Conventions to follow

Decisions from the convoy file (cite when implementing)

  • Decision 1 (.convoys/adopt-playwright-smoke.md § Decisions, post-IA round): keep tests/smoke/app.smoke.spec.ts as .ts. New visual spec also .ts (tests/visual/homepage.spec.ts).
  • Decision 2: in playwright.config.js, throw at config load when process.env.CI === 'true' AND VERCEL_AUTOMATION_BYPASS_SECRET is missing/empty. In non-CI, console.warn once and continue with extraHTTPHeaders undefined. Error message names the env var, the rotation command, and points at AGENTS.md § 7.
  • Decision 3: two projects — smoke (testMatch: 'smoke/**/*.spec.@(ts|js)') and visual (testMatch: 'visual/**/*.spec.@(ts|js)'). Both share the same use: block (no per-project use: overrides).
  • Decision 4: do NOT commit baselines. First CI run of Screenshot diff will fail at the test step; the existing continue-on-error: true swallows the failure and the comment-on-PR step posts "Visual Diff — view run" with empty artifacts. That's the documented end state of this brief.
  • Decision 5: no eslint.config.mjs change. Verified empirically that npx eslint tests/smoke/app.smoke.spec.ts exits 0 against the current config; the new tests/visual/homepage.spec.ts is structurally identical and will also pass.
  • Decision 6: add three scripts — test:smoke, test:visual, test:visual:update. No auto-boot of next dev.

Repo conventions (cite + match)

  • ESM module style. package.json has "type": "module". vitest.config.js and next.config.js are both ESM (import { defineConfig } from '...'; export default defineConfig({...})). playwright.config.js MUST match this shape:
    import { defineConfig } from '@playwright/test';
    export default defineConfig({ ... });
    
  • No-go zones (.cursor/rules/no-go-zones.mdc). None of the files in scope are listed. playwright.config.js does not exist yet. tests/visual/ does not exist yet. package.json / package-lock.json / .gitignore are all editable per established convoy precedent (bump-next-js, fix-auth-bypass).
  • Secret-handling discipline (AGENTS.md § 7):
    • NEVER console.log / echo / write to a file any string containing process.env.VERCEL_AUTOMATION_BYPASS_SECRET.
    • The console.warn branch (Decision 2 dev path) must say "VERCEL_AUTOMATION_BYPASS_SECRET unset" — NOT print the value.
    • The fail-loud branch (Decision 2 CI path) throws a string error message; the message names the env var by name but does NOT echo any value.
  • Style match. Two close precedents for ESM config files: vitest.config.js (heavy inline comments explaining each non-default choice) and next.config.js (terse). playwright.config.js should follow vitest.config.js's commented style because the convoy's AC #8 requires it ("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").

Acceptance criteria

package.json

  • Add "@playwright/test": "^1.60.0" to devDependencies (alphabetical position — between @neondatabase/serverless is in dependencies, so in devDependencies it lands between @testing-library/react and autoprefixer; verify alphabetization is preserved).

    Pin rationale: ^1.60.0 accepts patch updates (1.60.x), matches the workflow's npx playwright install --with-deps chromium (which downloads the bundled browser matching the installed package version), and 1.60.0 is 13 days old at time of writing — fresh but not bleeding-edge. If npm install resolves to a newer 1.60.x patch, that's expected.

  • Add three scripts to the scripts block, between test:run and the closing brace:

        "test:smoke": "playwright test --project=smoke",
        "test:visual": "playwright test --project=visual",
        "test:visual:update": "playwright test --project=visual --update-snapshots"
    

    Notes:

    • Use bare playwright test, NOT npx playwright test. With the dep installed locally, npm scripts resolve playwright from node_modules/.bin automatically. Matches the existing test / test:run shape (which uses bare vitest, not npx vitest).
    • test:visual:update is the exact command the visual workflow's PR-comment text instructs operators to run — keep the verbatim string match so a future operator can copy-paste from the comment.
    • Do NOT add test:e2e, test:smoke:local, or any auto-boot variant. Decision 6 explicitly rejected those.
  • No other changes to package.json. dependencies, name, version, private, type all stay byte-identical.

package-lock.json

  • Regenerated by running npm install after the package.json edit. Commit the resulting lockfile in the SAME commit as the package.json change (Risk R7 in the convoy file). The diff will be large (Playwright pulls many transitive deps) but is mechanical; do NOT hand-edit it.

  • Verify lockfile sync by running npm ci locally after commit: it should succeed with exit 0. If it errors with "Missing: ... from lock file" or "Invalid: lock file's ... does not satisfy package.json", the regeneration is incomplete — re-run npm install and re-commit.

playwright.config.js (new file at repo root)

  • Verbatim shape (commented for AC #8; the implementer is free to tighten wording but every non-obvious choice MUST have a one-line explanation):
// Playwright config for the post-PR-17 `Preview smoke` and
// `Screenshot diff` workflows. ESM per the repo's
// `"type": "module"` setting in package.json. Companion docs:
// `.convoys/adopt-playwright-smoke.md` (Decisions 1-6),
// AGENTS.md § 7 (Vercel preview bypass conventions).

import { defineConfig } from '@playwright/test';

const BASE_URL = process.env.BASE_URL ?? 'http://localhost:3000';
const BYPASS_SECRET = process.env.VERCEL_AUTOMATION_BYPASS_SECRET;

// `CI === 'true'` is the canonical GitHub Actions signal (set by
// the runner). Playwright config has no Next.js context, so
// `NODE_ENV` is not reliable here. Mirrors the rate-limit.js
// fail-closed pattern documented in AGENTS.md Gotcha #12.
const IS_CI = process.env.CI === 'true';

if (IS_CI && !BYPASS_SECRET) {
  // Fail loud in CI per Decision 2. The workflow's `env:` block
  // (preview-smoke.yml line 101, visual-diff.yml line 85) maps
  // `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` into the process
  // env; if it's empty here, the secret is unseeded or the
  // workflow YAML drift broke the mapping.
  throw new Error(
    'VERCEL_AUTOMATION_BYPASS_SECRET is required in CI to reach ' +
    'Vercel-Protection-protected preview deployments. ' +
    'Reseed via: gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<value>". ' +
    'See AGENTS.md § 7 for the full plumbing context.'
  );
}

if (!BYPASS_SECRET && !IS_CI) {
  // Dev fallback per Decision 2 — warn once at config load,
  // proceed without the header. Local runs target localhost
  // (no preview protection) or a non-protected URL.
  // eslint-disable-next-line no-console -- intentional one-shot warning at config load
  console.warn(
    '[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ' +
    'running without the Vercel bypass header. Targets a non-protected ' +
    'URL (e.g. http://localhost:3000). Hitting a protected preview without ' +
    'this header will return Vercel\'s SSO challenge page.'
  );
}

export default defineConfig({
  // Both `tests/smoke/` and `tests/visual/` live under `tests/`.
  // Project-level `testMatch` (below) partitions them so the
  // two workflows (`--project=smoke` and `--project=visual`)
  // each see only the specs they should run.
  testDir: './tests',

  // Smoke + visual specs are independent; parallelism within a
  // single spec adds no value here and would complicate the
  // per-test screenshot baseline lifecycle.
  fullyParallel: false,
  workers: IS_CI ? 1 : undefined,

  // One retry in CI handles transient Vercel preview flakes
  // (cold-start, DNS propagation). Local: zero retries — fail
  // fast so the dev sees the issue immediately.
  retries: IS_CI ? 1 : 0,

  // 30s per test is plenty for the 3 smoke checks + 1 visual
  // screenshot. The convoy file's success metric is < 5min
  // total workflow runtime; per-test 30s is well inside that.
  timeout: 30_000,
  expect: { timeout: 10_000 },

  // List reporter in dev for human readability; add HTML in CI
  // so the `Upload Playwright report on failure` step
  // (preview-smoke.yml line 104) has a populated `playwright-report/`
  // to upload. `open: 'never'` keeps the HTML from auto-launching
  // a browser tab in headless CI.
  reporter: IS_CI ? [['list'], ['html', { open: 'never' }]] : 'list',

  // Visual baselines live at `tests/visual/__screenshots__/<arg>{ext}`.
  // Workflow `visual-diff.yml` line 94 uploads this exact path as the
  // artifact — keep them aligned. {arg} is the snapshot name from
  // `toHaveScreenshot('home.png')` without the extension; {ext} is
  // the extension with the leading dot.
  //
  // Cross-platform note (Risk R3): this template drops Playwright's
  // default `-<browser>-<platform>` suffix. That means a Mac dev
  // running `npm run test:visual:update` overwrites the Linux-CI
  // baseline. Per Decision 4, we don't commit baselines this convoy;
  // operator seeds via `mcr.microsoft.com/playwright:v1.60.0-noble`
  // Docker on Linux. Multi-platform support is the
  // `seed-visual-baselines-on-linux` follow-up convoy's job.
  snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}',

  use: {
    baseURL: BASE_URL,
    // Headers apply to BOTH browser `page.goto(...)` calls AND the
    // test-level `request` fixture's APIRequestContext (verified
    // against Playwright docs: `testOptions.extraHTTPHeaders` is
    // shared between browser context and APIRequestContext
    // construction). This is why `tests/smoke/app.smoke.spec.ts`'s
    // third test (`request.get('/api/health')`) reaches the
    // protected preview without re-injecting the header in the
    // spec body. If a future hotfix shows the header NOT
    // propagating to APIRequestContext, see Risk R1 in the
    // convoy file's Architecture section.
    extraHTTPHeaders: BYPASS_SECRET
      ? { 'x-vercel-protection-bypass': BYPASS_SECRET }
      : undefined,
    // Trace OFF this convoy. Enabling it would land the bypass
    // header in the HAR payload (Risk R6); a future polish convoy
    // owns the trace-on + HAR-sanitization decision.
    trace: 'off',
    screenshot: 'off',
    video: 'off',
  },

  projects: [
    {
      // `Preview smoke` workflow invokes `--project=smoke` (per
      // preview-smoke.yml line 98). The testMatch keeps the
      // visual specs out of this project.
      name: 'smoke',
      testMatch: 'smoke/**/*.spec.@(ts|js)',
    },
    {
      // `Screenshot diff` workflow invokes `--project=visual`
      // (per visual-diff.yml line 82). The testMatch keeps the
      // smoke specs out of this project (so a `--project=visual`
      // run doesn't redundantly execute the smoke tests).
      name: 'visual',
      testMatch: 'visual/**/*.spec.@(ts|js)',
    },
  ],
});

Two things the implementer can vary without breaking AC:

  1. Comment wording — the substantive choices (predicate, error message contents, path template) must stay; the explanatory prose can be tightened.
  2. Property ordering inside defineConfig({...}) — Playwright doesn't care; alphabetical or grouped-by-concern are both fine.

Things the implementer MUST NOT change:

  • The IS_CI predicate (=== 'true', not === 'true' || === '1').
  • The throw vs console.warn branch logic.
  • The snapshotPathTemplate value (the workflow's artifact upload path depends on it).
  • The two project names (smoke, visual) — workflow YAML invokes them verbatim.
  • The two testMatch patterns — partitioning is the whole reason for two projects.
  • The trace: 'off' setting (Risk R6).
  • BYPASS_SECRET ? { ... } : undefined ternary — passing extraHTTPHeaders: { 'x-vercel-protection-bypass': undefined } would still send the header with the literal string 'undefined', which 401s loudly. The conditional is load-bearing.

tests/visual/homepage.spec.ts (new file)

  • Verbatim shape:
import { test, expect } from '@playwright/test';

/**
 * Visual baseline for the public homepage.
 *
 * FIRST RUN (no committed baseline yet):
 *   The Screenshot diff workflow runs `playwright test --project=visual
 *   --update-snapshots=none` (per .github/workflows/visual-diff.yml). With
 *   no baseline file at `tests/visual/__screenshots__/home.png` AND the
 *   `none` flag, this test FAILS — and that's the documented end state of
 *   the `adopt-playwright-smoke` convoy (Decision 4 in
 *   `.convoys/adopt-playwright-smoke.md`). The workflow's
 *   `continue-on-error: true` swallows the failure and the comment-on-PR
 *   step posts "Visual Diff — view run" with empty artifacts.
 *
 * SEEDING THE BASELINE (post-merge follow-up):
 *   Run `npm run test:visual:update` in a Linux environment so the
 *   generated PNG matches what CI will produce. The cleanest path is the
 *   Playwright Docker image:
 *
 *     docker run --rm -v "$PWD":/work -w /work \
 *       mcr.microsoft.com/playwright:v1.60.0-noble \
 *       sh -c "npm ci && BASE_URL=https://<preview>.vercel.app \
 *         VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
 *         npm run test:visual:update"
 *
 *   Then commit `tests/visual/__screenshots__/home.png`. This is tracked
 *   as the `seed-visual-baselines-on-linux` follow-up convoy.
 */
const BASE = process.env.BASE_URL ?? 'http://localhost:3000';

test.describe('visual: public homepage', () => {
  test('home renders consistently against baseline', async ({ page }) => {
    await page.goto(BASE);
    await expect(page).toHaveScreenshot('home.png');
  });
});

Notes:

  • BASE constant mirrors the existing tests/smoke/app.smoke.spec.ts shape (line 11). Don't read process.env.BASE_URL inside the test body.
  • Snapshot name is the literal string 'home.png'. The snapshotPathTemplate from playwright.config.js resolves this to tests/visual/__screenshots__/home.png.
  • test.describe block name 'visual: public homepage' matches the smoke spec's naming pattern ('smoke: app boots and core pages render').
  • Default viewport (1280×720) is fine for a first-pass baseline; do NOT set viewport: overrides this convoy.
  • Do NOT add await page.waitForLoadState('networkidle') — networkidle is unreliable on a JS-heavy Next.js app and causes false flakes. page.goto already waits for load by default; that's enough for a homepage baseline.
  • Do NOT add { fullPage: true } to toHaveScreenshot. Default (viewport-only) keeps the baseline file small (~50KB) and easier to review on PRs.

.gitignore

  • Add the following block AFTER the existing .code-review-graph/ line (current line 39):

# Playwright test runner artifacts (generated on every local run;
# never committed). Baselines under `tests/visual/__screenshots__/`
# are EXPLICITLY NOT ignored — they must be committed when they exist.
/playwright-report/
/test-results/
/.playwright/

Three rules:

  1. /playwright-report/ — the HTML reporter's output (config reporter block).
  2. /test-results/ — Playwright's default --output directory for trace/screenshot/video on failure.
  3. /.playwright/ — Playwright's local browser cache and other internal state.

Do NOT add tests/visual/__screenshots__/ to .gitignore. Baselines are committed artifacts (per Decision 4 they don't exist yet, but when they do, they MUST be tracked). This is a boot-the-brief finding — see Finding 3 below.

Cross-file checks

  • No secret leaks. Grep before commit:
    rg -i 'console\.log.*VERCEL_AUTOMATION_BYPASS_SECRET' playwright.config.js tests/
    
    Expected: zero matches. The two intentional references (the if (IS_CI && !BYPASS_SECRET) throw and the if (!BYPASS_SECRET && !IS_CI) console.warn) name the env var but do NOT echo its value.
  • No baseline files committed. Grep before commit:
    ls tests/visual/__screenshots__/ 2>/dev/null && echo "FAIL: directory exists with files; do NOT commit" || echo "OK: no baselines"
    
    Expected: OK: no baselines. If you ran npm run test:visual:update locally to verify the spec resolves, delete the generated PNG before commit.
  • npm run test:run exit 0 (vitest baseline unchanged):
    npm run test:run
    
    Expected: Tests 21 passed (21) per the existing vitest suite (16 auth + 5 Layout). If any vitest test fails, investigate before merge — this brief should be a pure additive change with no vitest impact.
  • npm run lint exit code unchanged (Decision 5):
    npm run lint 2>&1 | tail -3
    
    Expected: ✖ 128 problems (81 errors, 47 warnings) — the established baseline. If it grows by 1-2 new problems from the new visual spec, investigate (Risk R8); the architect's empirical check on the existing .ts smoke spec showed exit 0 so the new spec should be silent too.
  • npm run build exit 0 — Playwright is a devDep and should not affect the Next.js build graph. Run as a smoke check:
    npm run build 2>&1 | tail -5
    
    Expected: build success (Turbopack compile ~1-2s, 23 static pages + 47 API routes per the post-bump-next-js baseline).
  • Diff hygiene. git diff main..HEAD --stat should show only the 5 files listed in the brief frontmatter. No whitespace-only changes elsewhere.

Acceptance criterion #1 — end-state behavior

After this brief lands on the convoy branch and a Vercel preview deployment is published for the PR:

  • Preview smoke workflow: wait-action succeeds in ≤90s (already proven by PR #17 at 194ms); npm ci succeeds; npx playwright install --with-deps chromium succeeds (~30-60s); Run smoke tests step REACHES npx playwright test --project=smoke; 3 smoke tests execute against the Vercel preview. Pass/fail outcome:
    • Most likely PASS: home renders (existing pages don't 5xx), /login renders (existing page), /api/health returns 2xx (verified handler). If any of these fail, the failure is a real signal — investigate before declaring the brief incomplete.
    • Total workflow runtime: < 5 minutes (convoy success metric).
  • Screenshot diff workflow: wait-action succeeds; npx playwright install succeeds; Capture screenshots (PR) step REACHES npx playwright test --project=visual --update-snapshots=none. The single visual test FAILS because no baseline exists. continue-on-error: true swallows the failure. Upload screenshots + diffs step uploads tests/visual/__screenshots__/ (empty) + test-results/ (contains the failure detail). Comment on PR with diff link step posts "## Visual Diff" comment with the run URL. This is the documented end state of this convoy per Decision 4 — operator follows up with the seed-on-Linux PR.
  • The bypass secret does NOT appear in any line of either workflow's run log. Same verification pattern as the fix-vercel-deployment-protection-in-ci brief (download logs, grep for first 8 chars of the secret value, expect zero hits). GitHub Actions auto-masks registered secrets; this brief's contribution (Decision 2's throw and console.warn branches) does not interpolate the secret value into any string.

Manual verification (in addition to CI on push)

Run these in order. Paste relevant output (with secrets redacted) into the PR description.

  • Local install + lockfile parity.

    npm install
    npm ci  # second run, verify lockfile is in sync
    

    Expected: both succeed exit 0. npm ci is the critical one — if it errors, the lockfile is out of sync and the implementer needs to re-run npm install and re-commit.

  • Config loads cleanly.

    npx playwright --version
    npx playwright test --list --project=smoke
    npx playwright test --list --project=visual
    

    Expected:

    • Version 1.60.x (or later 1.60 patch).
    • --list --project=smoke enumerates 3 tests from tests/smoke/app.smoke.spec.ts.
    • --list --project=visual enumerates 1 test from tests/visual/homepage.spec.ts. If --list shows specs in the wrong project, the testMatch patterns are wrong — fix before commit.
  • Config Decision-2 fail-loud branch. Simulate CI without the secret:

    unset VERCEL_AUTOMATION_BYPASS_SECRET
    CI=true npx playwright test --list --project=smoke
    

    Expected: exit code 1, error includes "VERCEL_AUTOMATION_BYPASS_SECRET is required in CI" and the gh secret set rotation command. If it does NOT error, the Decision 2 logic is wrong.

  • Config Decision-2 warn-and-continue branch. Simulate dev without the secret:

    unset VERCEL_AUTOMATION_BYPASS_SECRET
    unset CI
    npx playwright test --list --project=smoke
    

    Expected: stderr includes the [playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ... warning, command proceeds and lists tests with exit 0.

  • Smoke spec runs against localhost. In one terminal:

    npm run dev
    

    In another:

    BASE_URL=http://localhost:3000 npm run test:smoke
    

    Expected: 3 tests pass against the local Next.js dev server. If any fail locally, the spec body has a real issue OR a local config drift — investigate before pushing.

  • Visual spec fail-on-missing-baseline behavior. Against localhost:

    BASE_URL=http://localhost:3000 npx playwright test --project=visual --update-snapshots=none
    

    Expected: 1 test FAILS with "A snapshot doesn't exist at tests/visual/screenshots/home.png". This is the desired Decision-4 behavior. If you accidentally generate a baseline locally with --update-snapshots, delete it before commit:

    rm -rf tests/visual/__screenshots__/
    
  • .gitignore actually ignores the right things. After a local Playwright run (which creates playwright-report/ and test-results/):

    git status --short
    

    Expected: no playwright-report/ or test-results/ entries. If they appear, the .gitignore entries are wrong (missing leading / or wrong directory name).

  • No bypass-secret leak in spec output. After running npm run test:smoke with the bypass var set:

    VERCEL_AUTOMATION_BYPASS_SECRET=fake-secret-value BASE_URL=http://localhost:3000 \
      npm run test:smoke 2>&1 | grep -i 'fake-secret-value' && echo "FAIL: secret leaked" || echo "OK: secret not in output"
    

    Expected: OK: secret not in output. (fake-secret-value is just a probe string for the grep; the real secret never enters this command.)

  • Push and observe the first workflow run. From convoy/adopt-playwright-smoke:

    git push -u origin HEAD
    

    Then watch both workflows. Expect the end-state described in Acceptance criterion #1 above: smoke passes (or fails on a real assertion), visual fails on missing baseline + posts the comment.

Boot-the-brief findings (preempted by the architect; do not re-investigate)

Finding 1 — @playwright/test@^1.60.0 resolves cleanly against the current dep tree

npm view @playwright/test version1.60.0 (released 2026-05-11). No peer-dep conflicts with the existing react@^18.3.1 / next@^16.2.6 / eslint@^9.39.4 graph (Playwright has no React or Next peers). The bundled Chromium binary downloaded by npx playwright install --with-deps chromium (already in both workflow steps — see preview-smoke.yml line 95 and visual-diff.yml line 79) matches the installed package version, so no version drift between the JS API and the browser binary.

Finding 2 — ESLint already parses .ts test files cleanly

Verified 2026-05-24 on the convoy branch HEAD (c8f1541):

$ npx eslint tests/smoke/app.smoke.spec.ts
$ echo $?
0

Zero output, exit 0. The eslint-config-next/core-web-vitals chain bundled by eslint-config-next@16 carries typescript-eslint as a hard dep (one of the reasons typescript@^5.9.3 is in devDependencies per AGENTS.md Gotcha #9). The new tests/visual/homepage.spec.ts is structurally identical to the existing smoke spec (same imports, same patterns) so will also lint clean. No eslint.config.mjs change is needed (Decision 5).

Finding 3 — tests/visual/__screenshots__/ MUST NOT be in .gitignore

Default Playwright snapshot behavior: baselines are committed source-of-truth (visual regression depends on having a known-good reference). The convoy file's success metric requires the visual workflow's Upload screenshots + diffs step (visual-diff.yml line 88-96) to find files at tests/visual/__screenshots__/ once baselines exist. Adding the path to .gitignore would silently break that step in any future PR that touches pages/** after baselines are committed in the follow-up convoy.

Finding 4 — extraHTTPHeaders applies to the test-level request fixture

Per Playwright fixtures docs, the test-level request fixture is "Isolated APIRequestContext instance for each test." Per testOptions docs, extraHTTPHeaders is "An object containing additional HTTP headers to be sent with every request." The testOptions surface is shared between browser context and APIRequestContext construction — the request fixture inherits the header. This is why tests/smoke/app.smoke.spec.ts's third test (request.get('/api/health')) reaches the protected preview without re-injecting the header in the spec body. See Risk R1 in the convoy file's Architecture section for the hotfix path if this propagation breaks empirically.

Finding 5 — pages/api/health.js returns 200 anonymously

The 7-line handler does NOT call getUserFromRequest, does NOT call checkAuthRateLimit, and does NOT gate on req.method. The Vercel preview's edge-protection layer terminates above the function: a request with the bypass header reaches the function as if anonymous, and the function returns { status: 'ok', ... } with HTTP 200. The third smoke test (request.get('/api/health')) will pass cleanly against a Vercel preview, given the extraHTTPHeaders plumb (Finding 4).

Finding 6 — Repo "type": "module" requires ESM-shape playwright.config.js

package.json line 5 declares "type": "module". vitest.config.js and next.config.js both follow the ESM import {...} from '...'; export default ... shape. CommonJS-style module.exports = { ... } in playwright.config.js would throw at load time with ReferenceError: module is not defined in ES module scope. The spec'd playwright.config.js matches the ESM shape — do NOT regress to CommonJS.

Finding 7 — snapshotPathTemplate token reference

Per Playwright testProject.snapshotPathTemplate docs, supported tokens are: {arg}, {ext}, {projectName}, {snapshotDir}, {testDir}, {testFileDir}, {testFileName}, {testFilePath}. There is no {platform} or {browserName} token — the default platform-suffix-in-filename behavior is built into the default template, and any custom template loses it. This is the structural reason for Risk R3 (cross-platform mismatch) and feeds Decision 4 (defer baseline commit to a Linux-Docker follow-up convoy).

Finding 8 — --update-snapshots=none semantics

Per 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 (what visual-diff.yml line 82 uses) NEVER creates snapshots; missing snapshots fail the test. The workflow's continue-on-error: true + comment-on-PR step is the documented safety net. First CI run of this brief on a touching-pages/** PR will produce a "Visual Diff — view run" comment with empty artifacts; that's the desired Decision-4 end state.

Out of scope (do not do these)

  • Do not rename tests/smoke/app.smoke.spec.ts to .js (Decision 1).
  • Do not commit any baseline image to tests/visual/__screenshots__/ (Decision 4).
  • Do not edit eslint.config.mjs (Decision 5 + Finding 2).
  • Do not edit any .github/workflows/*.yml file (those are owned by other convoys; the brief's job is to MAKE the YAML's npx playwright test invocation work, not to modify the YAML).
  • Do not edit AGENTS.md (doc-writer pass at convoy close owns § 7 updates and the seed-on-Linux instructions).
  • Do not add a test:smoke:local or test:e2e wrapper script that boots next dev automatically (Decision 6).
  • Do not add a vitest watch/setup change to support Playwright — the two runners stay independent (Test plan section of the convoy file).
  • Do not enable trace: 'on' or trace: 'retain-on-failure' in playwright.config.js's use: block (Risk R6).
  • Do not add a webServer: block to playwright.config.js (auto-starts Next.js — explicitly rejected by Decision 6).
  • Do not pin @playwright/test to an exact version (1.60.0 without the ^) — patch upgrades are desired (Risk R9 mitigation path).
  • Do not add a tsconfig.json to the repo just because the new spec is .ts. Lint already accepts it; tsc-noEmit is not run in CI; the JS-only repo policy (AGENTS.md Gotcha #9) explicitly defers TypeScript adoption to a separate convoy.
  • Do not bump node-version: '20' in any workflow (out of scope; Vercel default is 20).
  • Do not run npm audit fix as part of this brief. If npm install surfaces audit warnings, note them in the PR description but do NOT take action — audit churn is a separate concern.

Rationale (≤3 sentences)

The post-PR-17 workflows already invoke npx playwright test --project={smoke,visual} and already export BASE_URL + VERCEL_AUTOMATION_BYPASS_SECRET to the test step's env: — this brief is the small bridge that makes those invocations actually find a config, a dep, and a runnable visual spec. Bundling the visual spec with the config in one brief is the right call because they're semantically coupled (the visual spec depends on the visual project being declared in the config, which depends on @playwright/test being installed); splitting would force interim states that don't compile or run. Decision 4 (no baselines now) and Decision 6 (no auto-boot wrapper) explicitly keep the surface area small so the convoy ships in one PR; baseline curation and DX wrappers are queued as follow-up convoys (seed-visual-baselines-on-linux, possibly adopt-test-smoke-local) that operators can take or leave based on actual usage friction.