docs: post-convoy cleanup for adopt-playwright-smoke
Reflects the merged adopt-playwright-smoke convoy (PR #18, squash commit7b6f751) 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>
This commit is contained in:
Randall Stillwell
parent
7b6f7519b2
commit
eefc390636
3 changed files with 220 additions and 12 deletions
|
|
@ -13,8 +13,9 @@ skip:
|
|||
- 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
|
||||
status: shipped
|
||||
created: 2026-05-24
|
||||
shipped: 2026-05-24
|
||||
parent: ship-readiness
|
||||
addresses: P1 #10 step 2 (launch sequence step 10)
|
||||
depends_on:
|
||||
|
|
@ -657,3 +658,166 @@ slice_dependencies:
|
|||
|
||||
Single brief; no `/multitask` fan-out. Conductor dispatches
|
||||
serially.
|
||||
|
||||
## As-shipped
|
||||
|
||||
Shipped 2026-05-24 as squash commit `7b6f751` (PR #18). The convoy
|
||||
shipped in one PR (PR #18 architect-commit `3ac527e`, implementer-commit
|
||||
`c72d006`) — Brief 1 as planned, with two small implementer deviations
|
||||
from the brief's verbatim shape (both lint-baseline-preserving and
|
||||
behavior-neutral). Capturing the deviations + the cross-validation
|
||||
finding + empirical CI metrics here so the next architect / reviewer
|
||||
has the audit trail.
|
||||
|
||||
### Decisions ratified by operator at gate 1
|
||||
|
||||
Three of six decisions were architect-self-ratifiable (Decisions 2, 3,
|
||||
5 — see § Decisions). The remaining three needed operator ratification,
|
||||
and all three were ratified verbatim at gate 1:
|
||||
|
||||
- **Decision 1 — keep `.ts` for Playwright specs.** `tests/smoke/app.smoke.spec.ts`
|
||||
stayed `.ts`; the new `tests/visual/homepage.spec.ts` also shipped
|
||||
as `.ts`. Empirically `npx eslint` exits 0 on both files against
|
||||
the current config (`eslint-config-next@16`'s bundled
|
||||
`typescript-eslint` chain parses them) — no `eslint.config.mjs`
|
||||
change needed, lint baseline held at 128 problems.
|
||||
- **Decision 4 — defer visual baselines to a Linux-Docker follow-up
|
||||
PR.** No baseline images committed. `tests/visual/__screenshots__/`
|
||||
does not exist in the tree at merge; the visual workflow's
|
||||
`--update-snapshots=none` flag + `continue-on-error: true` + the
|
||||
PR-comment step are the documented Decision-4 safety net.
|
||||
- **Decision 6 — three simple scripts, no auto-boot wrapper.**
|
||||
`package.json` got `test:smoke`, `test:visual`, `test:visual:update`
|
||||
in that order. No `test:smoke:local` / `test:e2e` / `next dev`
|
||||
auto-boot variant. Operator's local flow is "boot `next dev` in
|
||||
one terminal, run `BASE_URL=http://localhost:3000 npm run test:smoke`
|
||||
in another" — same shape as the existing `test` / `test:run`
|
||||
scripts.
|
||||
|
||||
### Implementer deviations from Brief 1's verbatim shape
|
||||
|
||||
Two small deviations, both surfaced in the implementer's PR report
|
||||
and both lint-baseline-preserving:
|
||||
|
||||
1. **Removed the brief's `// eslint-disable-next-line no-console`
|
||||
directive on the dev warn-and-continue branch of
|
||||
`playwright.config.js`.** The brief specified the directive
|
||||
verbatim to suppress an expected `no-console` complaint on the
|
||||
`console.warn(...)` call. Empirically the current ESLint config
|
||||
does NOT flag `console.warn` at all (only `console.log`), so the
|
||||
`eslint-disable-next-line` directive itself becomes an unused-
|
||||
directive lint error (`Unused eslint-disable directive`) and
|
||||
would have regressed the baseline from 128 → 129. Removing the
|
||||
directive is the behavior-neutral fix: the `console.warn` line
|
||||
still runs unchanged, just without the no-longer-needed disable
|
||||
comment. Lint baseline held at exactly 128 problems
|
||||
post-implementation.
|
||||
2. **Placed `@playwright/test` first in `devDependencies` for strict
|
||||
alphabetical order.** The brief's prose was internally
|
||||
inconsistent about placement (it called for "alphabetical
|
||||
position" but then described the wrong neighbors — `@playwright`
|
||||
sorts before `@testing-library/react` lexically). The implementer
|
||||
followed the alphabetical rule rather than the prose's example,
|
||||
so the final ordering is `@playwright/test` →
|
||||
`@testing-library/dom` → `@testing-library/react` → `autoprefixer`
|
||||
→ ... Behavior-neutral; matches the convention used elsewhere in
|
||||
the file (`dependencies` is alphabetical too).
|
||||
|
||||
Both deviations are explicitly behavior-neutral — same code paths
|
||||
execute, same env-var predicates, same lint count. Documenting them
|
||||
here so a future reviewer comparing the brief's verbatim spec to
|
||||
the merged diff sees the rationale instead of flagging drift.
|
||||
|
||||
### Cross-validation: smoke test 2 locks in PR #15's "Sign in" CTA
|
||||
|
||||
`tests/smoke/app.smoke.spec.ts`'s second test (`'sign-in page renders'`)
|
||||
navigates to `/login` and asserts `await expect(page.getByRole('button',
|
||||
{ name: /sign in/i })).toBeVisible({ timeout: 10_000 })`. That
|
||||
assertion lights up only because the page renders a sign-in-named
|
||||
control — which is exactly the `<Link href="/login">Sign in</Link>`
|
||||
CTA that the `fix-layout-default-user` convoy (PR #15, squash
|
||||
`ca302a8`) added to `components/Layout.js`'s logged-out branch when
|
||||
it replaced the leaky maintainer-email default prop. **This convoy
|
||||
effectively locks in a regression test for that earlier convoy's
|
||||
work** — if a future change reverts to a hardcoded default user (or
|
||||
breaks the logged-out CTA wording) the smoke check now fails the
|
||||
PR. Surfaced organically from CI green; not a planned acceptance
|
||||
criterion of this convoy but worth noting because P0 #7's resolved
|
||||
state is now defended by a real CI signal, not just the 5 vitest
|
||||
assertions in `test/components/Layout.test.js`.
|
||||
|
||||
### As-shipped metrics (from post-merge run 26376162598 on `main`)
|
||||
|
||||
- `Playwright smoke` workflow total runtime: **59 seconds**, exit 0
|
||||
(was: fast-fail at "playwright not installed" / "no config" before
|
||||
this convoy — never reached `npx playwright test`). Comfortably
|
||||
inside the < 5-minute success metric.
|
||||
- `Run smoke tests` step: **3/3 tests pass in 2.9s** against the
|
||||
Vercel preview URL with the `x-vercel-protection-bypass` header
|
||||
applied:
|
||||
- `home redirects or renders without 5xx` → ✓ 683ms
|
||||
- `sign-in page renders` (the cross-validation above) → ✓ 459ms
|
||||
- `public health endpoint responds` (`/api/health` 2xx) → ✓ 571ms
|
||||
- Step breakdown: `Wait for Vercel Preview deployment` → success
|
||||
(~200ms range, per PR #17's plumbing); `npm ci` + `setup-node` +
|
||||
`playwright install --with-deps chromium` → success; `Run smoke
|
||||
tests` → **success** (the failure mode shifted from "no config"
|
||||
in PR #17's end state to "all green" here, which is the convoy's
|
||||
target end state).
|
||||
- `Screenshot diff` workflow: **not triggered on PR #18 itself**.
|
||||
Its `paths:` filter excludes test-infra-only changes (the PR
|
||||
touched only `package.json`, `package-lock.json`,
|
||||
`playwright.config.js`, `tests/visual/homepage.spec.ts`,
|
||||
`.gitignore`, and the convoy/brief docs — none of those are
|
||||
under `pages/**` / `components/**` / `styles/**` /
|
||||
`tailwind.config.js` / `postcss.config.js`). First real trigger
|
||||
fires on the next PR touching any of those paths; at that point
|
||||
the documented Decision-4 end state (test fails on missing
|
||||
baseline → `continue-on-error: true` swallows it → comment-on-PR
|
||||
step posts "Visual Diff — view run" with empty artifacts) gets
|
||||
its first live exercise.
|
||||
- Bypass secret leak check: **0 matches** against the raw workflow
|
||||
log (per AC #7's grep-for-first-8-chars-of-secret pattern). GitHub
|
||||
Actions auto-masks registered secrets; this convoy's
|
||||
Decision-2 branches (`throw` in CI, `console.warn` in dev) name
|
||||
the env var but never interpolate its value into any string,
|
||||
so the mask never had to engage on output from our code.
|
||||
|
||||
### Operator action required going forward
|
||||
|
||||
`seed-visual-baselines-on-linux` is queued as the follow-up convoy
|
||||
(see `.convoys/ship-readiness.md` § Queued convoys). Until that
|
||||
PR lands, every `Screenshot diff` run on a PR touching
|
||||
`pages/**` / `components/**` / `styles/**` will fail at the test
|
||||
step and post a comment with empty artifacts. That is the
|
||||
documented end state of this convoy per Decision 4; no operator
|
||||
intervention is required to keep `Playwright smoke` green
|
||||
(smoke runs against the existing spec, which has no baseline
|
||||
dependency).
|
||||
|
||||
If/when the operator rotates `VERCEL_AUTOMATION_BYPASS_SECRET`
|
||||
via the Vercel dashboard, both workflows fail with Vercel SSO
|
||||
challenge pages on every PR until the GitHub secret is re-seeded
|
||||
(`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`).
|
||||
Same human-responsibility pattern as `JWT_SECRET` rotation;
|
||||
documented in `AGENTS.md` § 7.
|
||||
|
||||
### What did NOT change
|
||||
|
||||
- `tests/smoke/app.smoke.spec.ts` (existed pre-convoy; untouched
|
||||
per Decision 1).
|
||||
- `eslint.config.mjs` (Decision 5 + Finding 2; empirically clean
|
||||
without any change).
|
||||
- `pages/api/health.js` (already returns 200 anonymously; smoke
|
||||
test 3 passes against it without any handler change).
|
||||
- Any source under `pages/**` / `components/**` / `lib/**` /
|
||||
`scripts/**` (this convoy is test-infra-only by scope).
|
||||
- `vitest.config.js` / `test/setup.js` / anything under `test/`
|
||||
(the two runners stay independent per the convoy's Test plan §;
|
||||
`npm run test:run` still passes 21/21 at merge — no vitest
|
||||
regression from the new `@playwright/test` install).
|
||||
- Any `.github/workflows/*.yml` file (owned by PR #16 / PR #17 /
|
||||
`fix-lint-baseline`; this convoy made the YAML's existing
|
||||
invocations work, not modified them).
|
||||
- `tests/visual/__screenshots__/` (does not exist; Decision 4
|
||||
defers to `seed-visual-baselines-on-linux`).
|
||||
|
|
|
|||
|
|
@ -153,11 +153,11 @@ These MUST land before any anonymous traffic touches the production URL.
|
|||
|
||||
- **Impact:** The first agent-driven refactor of `getUserFromRequest` (P0 #1) is high-blast-radius with no safety net.
|
||||
- **Fix sequence:**
|
||||
1. Install `vitest`. Add `npm run test:run` script.
|
||||
2. Install `@playwright/test`. Wire up `tests/smoke/app.smoke.spec.ts` (already drafted; needs `playwright.config.ts`).
|
||||
3. Re-enable the `test:` job in `.github/workflows/ci.yml` (commented out at install time).
|
||||
1. Install `vitest`. Add `npm run test:run` script. **RESOLVED** by `fix-auth-bypass` Brief 5, commit `1629afb`.
|
||||
2. Install `@playwright/test`. Wire up `tests/smoke/app.smoke.spec.ts` (already drafted; needs `playwright.config.ts`). **RESOLVED 2026-05-24** by `adopt-playwright-smoke`, PR #18 squash `7b6f751` — 3/3 smoke tests pass in 2.9s, full workflow 59s, zero secret leaks. See § Queued convoys and `.convoys/adopt-playwright-smoke.md` § As-shipped.
|
||||
3. Re-enable the `test:` job in `.github/workflows/ci.yml` (commented out at install time). Next remaining step in this fix sequence.
|
||||
4. Add unit tests for `lib/permission-middleware.js`, `lib/slug-utils.js`, `pages/api/auth-utils.js`.
|
||||
5. Wire `preview-smoke.yml` to run against the Vercel preview URL.
|
||||
5. Wire `preview-smoke.yml` to run against the Vercel preview URL. **RESOLVED 2026-05-24** by `fix-vercel-deployment-protection-in-ci` (PR #17, `9a3e077`) + `adopt-playwright-smoke` (PR #18, `7b6f751`).
|
||||
- **Owns:** `role-architect` (test strategy) → `role-implementer` (initial suite).
|
||||
|
||||
### 11. No migration tool — `scripts/add-*.js` graveyard
|
||||
|
|
@ -281,7 +281,7 @@ Each phase is one Conductor-created convoy. Don't run more than two in parallel
|
|||
7. **`migration-tool`** (P1 #11). One PR. Backfill + first new migration.
|
||||
8. **`single-sql-client`** (P1 #8). 2-3 PRs, fanned out via multitask once per-file briefs are written.
|
||||
9. **`single-auth-provider`** (P1 #9). 3-5 PRs via multitask.
|
||||
10. **`adopt-playwright-smoke`** (P1 #10 step 2). One PR.
|
||||
10. **`adopt-playwright-smoke`** (P1 #10 step 2). One PR. **RESOLVED 2026-05-24** — PR #18 squash `7b6f751`; smoke 3/3 green in 2.9s, full workflow 59s, zero secret leaks. See § Queued convoys for the full as-shipped block.
|
||||
11. **`schema-cleanup`** (P2 #14). Multi-PR convoy via multitask.
|
||||
12. **`god-component-split`** (P2 #13). One convoy per file; fan out via multitask once architect's `slice_dependencies` are written.
|
||||
13. **`launch-polish`** (P3). UX/IA/a11y/docs convoy.
|
||||
|
|
@ -299,6 +299,25 @@ Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch seq
|
|||
- **`single-auth-provider`** (priority: P1 quality, also listed as launch sequence step 9). Collapse `lib/auth-context.js` + `lib/admin-auth.js` into `lib/use-auth.js`. Surfaced again as a follow-up in P0 #7 § Flagged-but-deferred (4 pages still import the legacy `useAuth`).
|
||||
- **`cleanup-mobile-nav-dead-props`** (priority: P3 polish). `components/MobileNavigation.js` accepts a dead `user` prop; remove it. Surfaced in P0 #7 § Flagged-but-deferred. May fold into `god-component-split` (P2 #13) if that lands first.
|
||||
- **`bump-eslint-10`** (priority: P2 hygiene; upstream-blocked). Bump ESLint from v9 to v10 once `typescript-eslint` ships a v10-tested release and `eslint-config-next` bundles it. Surfaced in `.convoys/bump-next-js.md` § Decisions D.
|
||||
- **`seed-visual-baselines-on-linux`** (priority: P2 CI infra; **operator action required**). Generate Linux baselines for `tests/visual/__screenshots__/` in the `mcr.microsoft.com/playwright:v1.60.0-noble` Docker image and commit them in a small follow-up PR. Mac-generated baselines would silently overwrite Linux CI baselines because `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token (Risk R3 + Boot-the-brief Finding 7 in `.convoys/adopt-playwright-smoke.md`). Until this PR lands, every `Screenshot diff` run on a PR touching `pages/**` / `components/**` / `styles/**` / Tailwind/PostCSS config fails at the test step and posts a "Visual Diff — view run" comment with empty artifacts — that's the documented Decision-4 end state of `adopt-playwright-smoke`, not a regression. One small PR with just the PNG baseline(s). Surfaced 2026-05-24 as the follow-up to `adopt-playwright-smoke` (PR #18).
|
||||
- **`adopt-playwright-smoke`** (priority: P1 quality, also listed as launch sequence step 10 / P1 #10 step 2) — **RESOLVED 2026-05-24**.
|
||||
- **Resolved by:** squash commit `7b6f751` (PR #18, architect-commit `3ac527e`, implementer-commit `c72d006`). Brief 1 shipped as planned with two small lint-baseline-preserving deviations from the brief's verbatim shape (documented in the convoy file's § As-shipped).
|
||||
- **As-shipped surface:** `@playwright/test@^1.60.0` added to `devDependencies`; new `playwright.config.js` at repo root (ESM, two projects partitioned by `testMatch` — `smoke` + `visual`, CI-fail-loud / dev-warn predicate on `VERCEL_AUTOMATION_BYPASS_SECRET` per Decision 2, `snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}'` aligned with `visual-diff.yml`'s artifact upload path); new `tests/visual/homepage.spec.ts` (1 test, no baseline committed per Decision 4); three new `package.json` scripts (`test:smoke`, `test:visual`, `test:visual:update`); three new `.gitignore` entries (`/playwright-report/`, `/test-results/`, `/.playwright/`). No `eslint.config.mjs` change (Decision 5 + Finding 2 verified clean empirically). No source touched under `pages/**` / `components/**` / `lib/**`.
|
||||
- **Implementer deviations** (both behavior-neutral, both lint-baseline-preserving):
|
||||
1. Removed Brief 1's `// eslint-disable-next-line no-console` directive on `playwright.config.js`'s `console.warn` branch — the current ESLint config does not flag `console.warn` at all, so the disable directive itself would have regressed lint from 128 → 129 as an "Unused eslint-disable directive" error.
|
||||
2. Placed `@playwright/test` first in `devDependencies` for strict alphabetical correctness — the brief's prose was internally inconsistent on neighbors (`@playwright` sorts lexically before `@testing-library/react`).
|
||||
- **As-shipped metrics** (from post-merge `Playwright smoke` 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).
|
||||
- `Run smoke tests` step: **3/3 tests pass in 2.9s** against the Vercel preview with `x-vercel-protection-bypass` header applied — `home redirects or renders without 5xx` ✓ 683ms / `sign-in page renders` ✓ 459ms / `public health endpoint responds` ✓ 571ms.
|
||||
- `Screenshot diff` workflow: **not triggered on PR #18 itself** because its `paths:` filter excludes test-infra-only changes; first real trigger fires on the next PR touching `pages/**` / `components/**` / `styles/**` / `tailwind.config.js` / `postcss.config.js`. At that point the documented Decision-4 end state runs live (test fails on missing baseline → `continue-on-error: true` swallows → comment-on-PR step posts run link with empty artifacts).
|
||||
- Bypass secret leak check: **0 matches** in the raw workflow log. GitHub Actions auto-masks registered secrets; our Decision-2 branches name the env var but never interpolate the value into any string.
|
||||
- **Cross-validation finding** (not a planned AC; surfaced organically from CI green): smoke test 2 (`'sign-in page renders'`) asserts `await expect(page.getByRole('button', { name: /sign in/i })).toBeVisible()` against `/login`, which only passes because `components/Layout.js` renders the `<Link href="/login">Sign in</Link>` CTA on the logged-out branch that PR #15 (`fix-layout-default-user`, `ca302a8`) introduced. P0 #7's resolved state is now defended by a live CI signal — if a future PR reverts to a hardcoded default user or breaks the CTA wording, smoke fails the PR (in addition to the 5 vitest assertions in `test/components/Layout.test.js`).
|
||||
- **Operator action required going forward:** `seed-visual-baselines-on-linux` (above) is the follow-up. Until it lands, `Screenshot diff` runs post a "Visual Diff — view run" comment with empty artifacts on every UI-touching PR — that is the Decision-4 end state, not a regression. No operator action is required to keep `Playwright smoke` green.
|
||||
- **Flagged-but-deferred** (deliberately out of scope per the convoy file, restated here for the audit trail):
|
||||
1. `seed-visual-baselines-on-linux` — see above.
|
||||
2. `adopt-test-smoke-local` (possible follow-up) — a `test:smoke:local` wrapper that auto-boots `next dev`. Explicitly rejected by Decision 6; queue only if dev friction proves out.
|
||||
3. Deeper E2E coverage beyond the 3 existing smoke checks — per-feature work in feature convoys, not a test-infra concern.
|
||||
- **Owns:** `role-architect` (3 of 6 decisions self-ratified — D2 CI predicate, D3 two-project shape, D5 no-eslint-change; 3 of 6 operator-ratified — D1 keep `.ts`, D4 defer baselines, D6 simple scripts) → `role-implementer` (Brief 1, plus the two deviations above).
|
||||
- **`fix-vercel-deployment-protection-in-ci`** (priority: P2 CI infra) — **RESOLVED 2026-05-24**.
|
||||
- **Resolved by:** squash commit `9a3e077` (PR #17), comprising three commits, not one. Operator prereq seeded 2026-05-24T20:03:31Z (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET`; confirmed via `gh secret list`); the implementer dispatch waited on that visibility per the convoy file's "Operator action required" gate.
|
||||
- **Three-commit reality** (Brief 1 + two scope expansions found during CI validation):
|
||||
|
|
|
|||
37
AGENTS.md
37
AGENTS.md
|
|
@ -68,16 +68,41 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560
|
|||
|
||||
## 6. Testing
|
||||
|
||||
- **Unit-test runner:** `vitest@^3.2.4` (installed via `fix-auth-bypass` Brief 5, commit `1629afb`). `npm test` for watch mode; `npm run test:run` for the CI / single-shot mode. Config in `vitest.config.js`, setup in `test/setup.js` (sets `JWT_SECRET` + `NODE_ENV=test` before any module loads). Specs live under `test/` mirroring source layout (`test/lib/*.test.js`, `test/api/*.test.js`).
|
||||
- **Coverage today:** 16 unit tests covering the post-`fix-auth-bypass` auth surface — `lib/auth-secret.js` (3), `lib/permission-middleware.js::getUserFromRequest` (8, incl. a negative regression against the old synthetic-admin shape — Gotcha #2), and `pages/api/auth-utils.js` (5). These tests lock in the contracts established by Briefs 1 and 2; do not weaken them when refactoring auth.
|
||||
- **CI:** the `test:` job in `.github/workflows/ci.yml` runs `npm run test:run` on every PR and push to `main` and is **blocking** (no `|| true`, no `continue-on-error`). A red test job blocks merge.
|
||||
- **E2E / smoke runner:** `@playwright/test` is **still pending** — queued for the `adopt-playwright-smoke` convoy (see `.convoys/ship-readiness.md` § Proposed launch sequence step 10). Until it lands, `preview-smoke.yml` and `visual-diff.yml` are no-ops on the smoke side.
|
||||
- **Manual QA:** `TESTING_GUIDE.md` still applies for surfaces not yet covered by automated tests (UI flows, scanner camera path, import jobs).
|
||||
- **Unit-test runner:** `vitest@^3.2.4` (installed via `fix-auth-bypass` Brief 5, commit `1629afb`). `npm test` for watch mode; `npm run test:run` for the CI / single-shot mode. Config in `vitest.config.js`, setup in `test/setup.js` (sets `JWT_SECRET` + `NODE_ENV=test` before any module loads). Specs live under `test/` mirroring source layout (`test/lib/*.test.js`, `test/api/*.test.js`, `test/components/*.test.js`). Last green: 21/21 tests pass.
|
||||
- **Vitest coverage today:** 21 unit tests — `lib/auth-secret.js` (3), `lib/permission-middleware.js::getUserFromRequest` (8, incl. a negative regression against the old synthetic-admin shape — Gotcha #2), `pages/api/auth-utils.js` (5), and `components/Layout.js` (5 regression-lock assertions for the post-PR-#15 logged-out branch — Gotcha #8). These tests lock in the contracts established by `fix-auth-bypass` Briefs 1 + 2 and `fix-layout-default-user`; do not weaken them when refactoring auth or Layout.
|
||||
- **E2E / smoke runner:** `@playwright/test@^1.60.0` (installed via `adopt-playwright-smoke`, PR #18 squash `7b6f751`). Config in `playwright.config.js` (root, ESM) declares two projects:
|
||||
- `smoke` — `tests/smoke/**/*.spec.@(ts|js)`; invoked by `.github/workflows/preview-smoke.yml`. `npm run test:smoke` locally.
|
||||
- `visual` — `tests/visual/**/*.spec.@(ts|js)`; invoked by `.github/workflows/visual-diff.yml`. `npm run test:visual` locally; `npm run test:visual:update` to (re-)seed baselines.
|
||||
|
||||
Local-run convention: boot `next dev` in one terminal, then in another run `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`). No `next dev` auto-boot in the test scripts (Decision 6 of `adopt-playwright-smoke`).
|
||||
- **Browsers must be installed once locally:** `npx playwright install --with-deps chromium`. CI re-runs this on every workflow run (it's cached when possible).
|
||||
- **Visual baselines:** none committed yet. `tests/visual/__screenshots__/` is intentionally absent and intentionally NOT in `.gitignore` (baselines, when they exist, must be committed). First-run baseline generation MUST happen in a Linux environment so the PNG matches what CI produces. Recommended 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=<preview-url> \
|
||||
VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
|
||||
npm run test:visual:update"
|
||||
```
|
||||
|
||||
Mac-generated baselines will NOT match Linux CI — `playwright.config.js`'s custom `snapshotPathTemplate` has no `{platform}` token, so a Mac update silently overwrites the canonical Linux baseline. Tracked as the queued `seed-visual-baselines-on-linux` convoy (see `.convoys/ship-readiness.md` § Queued convoys).
|
||||
- **CI behavior:**
|
||||
- **Vitest:** the `test:` job in `.github/workflows/ci.yml` runs `npm run test:run` on every PR and push to `main` and is **blocking** (no `|| true`, no `continue-on-error`). A red test job blocks merge.
|
||||
- **Playwright smoke:** runs on every PR via `preview-smoke.yml`. Gate skip via `pipeline: skip smoke` in the PR body (handled in the `gate:` job's Decide step via env-var routing — see § 7's shell-injection note). Last measured runtime: 59s end-to-end, 3/3 tests pass in 2.9s (PR #18 post-merge run).
|
||||
- **Screenshot diff:** runs only on PRs touching `pages/**` / `components/**` / `styles/**` / `tailwind.config.js` / `postcss.config.js` via `visual-diff.yml`. **First `Screenshot diff` run after `adopt-playwright-smoke` will fail at the test step** because no baseline exists yet; `continue-on-error: true` swallows the failure and the comment-on-PR step posts "Visual Diff — view run" with empty artifacts. That is the documented Decision-4 end state of `adopt-playwright-smoke`, not a regression — it stays that way until `seed-visual-baselines-on-linux` lands.
|
||||
- **Manual QA:** `TESTING_GUIDE.md` still applies for flows not yet covered by automated tests (scanner camera path, card-import jobs, multi-step UI wizards). The automated smoke + visual suite is steadily eclipsing it; `TESTING_GUIDE.md` will be renamed to `docs/MANUAL_QA.md` and trimmed to truly-manual-only flows in a future cleanup convoy (see `.convoys/ship-readiness.md` § Role-doc-writer findings).
|
||||
|
||||
## 7. Deployment
|
||||
|
||||
- **Vercel** auto-deploys `main` and creates Preview deployments for every PR. `vercel.json` and `.vercel/` are committed. CI in `.github/workflows/` runs lint + types (no duplicate build — Vercel handles it).
|
||||
- **Preview protection bypass for automation.** The project has a Protection Bypass for Automation token exposed locally as `VERCEL_AUTOMATION_BYPASS_SECRET` in `.env.local` (not committed) and seeded into GitHub Actions as a repo secret (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET`, 2026-05-24). The `preview-smoke` and `visual-diff` workflows pass the bypass as a **query parameter** on `wait-for-vercel-preview@v1.3.2`'s `path:` input — `path: '/?x-vercel-protection-bypass=…'`, bare form, **without** `&x-vercel-set-bypass-cookie=true` (the cookie variant returns 307 + Set-Cookie and axios in Node has no cookie jar, so it 401s on the redirect). The header form (`x-vercel-protection-bypass: <secret>`) is reserved for the future `playwright.config.js` that `adopt-playwright-smoke` will ship (Playwright has its own request shape and `extraHTTPHeaders` is cleaner there). Do not log or echo the value. If the operator rotates the token in the Vercel dashboard, re-seed the GitHub secret via `gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`. See `.convoys/fix-vercel-deployment-protection-in-ci.md`.
|
||||
- **Preview protection bypass for automation.** The project has a Protection Bypass for Automation token exposed locally as `VERCEL_AUTOMATION_BYPASS_SECRET` in `.env.local` (not committed) and seeded into GitHub Actions as a repo secret (`gh secret set VERCEL_AUTOMATION_BYPASS_SECRET`, 2026-05-24). The secret is consumed in two shapes:
|
||||
1. **Query parameter** on `wait-for-vercel-preview@v1.3.2`'s `path:` input in both `preview-smoke.yml` and `visual-diff.yml` — `path: '/?x-vercel-protection-bypass=…'`, bare form, **without** `&x-vercel-set-bypass-cookie=true` (the cookie variant returns 307 + Set-Cookie and axios in Node has no cookie jar, so it 401s on the redirect). Plumbed by PR #17 (`fix-vercel-deployment-protection-in-ci`, squash `9a3e077`).
|
||||
2. **HTTP header** in `playwright.config.js`'s `use.extraHTTPHeaders` — `'x-vercel-protection-bypass': <secret>`. Playwright's browser context has a real cookie jar so this shape works there, and the testOptions surface forwards the header to the test-level `request` fixture's `APIRequestContext` as well, so both `page.goto(...)` calls and `request.get('/api/health')` calls hit the protected preview correctly without per-spec header injection. Plumbed by PR #18 (`adopt-playwright-smoke`, squash `7b6f751`) per Decision 2 of that convoy.
|
||||
|
||||
Decision 2 also wires a **fail-loud-in-CI / warn-in-dev** predicate: `if (process.env.CI === 'true' && !process.env.VERCEL_AUTOMATION_BYPASS_SECRET) throw ...` (with an error message that names the env var, the `gh secret set` rotation command, and points at this section); otherwise `console.warn` once and continue with `extraHTTPHeaders` undefined. Same fail-closed / warn-and-no-op shape as `lib/rate-limit.js`'s Upstash predicate — see Gotcha #12.
|
||||
|
||||
Do not log or echo the value. If the operator rotates the token in the Vercel dashboard, re-seed the GitHub secret via `gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<new value>"`. See `.convoys/fix-vercel-deployment-protection-in-ci.md` and `.convoys/adopt-playwright-smoke.md`.
|
||||
- **Shell-injection hardening in workflow YAML.** Never inline `${{ github.event.* }}` directly into a `run:` block — route the value through the step's `env:` block and quote it (`"$VAR_NAME"`) in shell. PR #17's CI validation caught a real syntax error from a PR body containing `(` because the gate-job's Decide step inlined `${{ github.event.pull_request.body }}` straight into bash; commit `b6f8688` swept both `preview-smoke.yml` and `visual-diff.yml` to the `env:` + quoted-shell pattern. This is GitHub's official Security Hardening guidance ("Security hardening for GitHub Actions" → "Using a third-party action"). Apply to any new workflow that reads PR body / title / branch name / commit messages in shell.
|
||||
|
||||
## 8. Code graph
|
||||
|
|
|
|||
Loading…
Reference in a new issue