test(comments): cover reader comment auth in e2e via a Clerk dev instance

[axross]

Summary

Reader-facing blog-post comments authenticate through Clerk (GitHub OAuth), but the E2E suite never exercised that path — its "authenticated" state is a Payload admin session used only to seed comments. Two reader journeys were already catalogued in e2e/scenarios.md but left untested and marked "needs Clerk test tokens": post.comments.sign-in and post.comments.submit.

This wires the suite to a Clerk development instance via the official @clerk/testing package:

• A Playwright globalSetup fetches a Testing Token when the dev keys are present (and is a no-op otherwise), run in the main process so every worker inherits it.
• A signInAsReader helper establishes a reader session through a server-side sign-in ticket. This is deliberate: Clerk's helper cannot drive a real GitHub OAuth redirect, and the ticket strategy is independent of enabled first factors, so it works on the GitHub-OAuth-only instance without automating GitHub's login.
• The two tests drive the real composer → CSRF-token → pending-write path.

Everything is gated on NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY, so unconfigured runs (local without setup, PR CI, forked previews) skip the new tests and stay green. The e2e CI job passes the dev keys and a +clerk_test reader from secrets that resolve to empty until provided — keeping Clerk optional, as requested.

Why not test real GitHub OAuth: you don't test a third party's login in your own suite; establishing a Clerk session via the testing helpers and asserting the app's behavior is the standard, supported approach and covers all of our code (middleware, provider, CSRF, pending write).

Related issues

Closes #155

Verification

This environment has no Clerk keys, so the new tests skip by design; everything else was run:

• npm run format — clean. npm run lint — clean (one pre-existing unrelated warning in scripts/prune-preview-blobs.mjs).
• npx tsc --noEmit — 0 errors project-wide (covers the e2e sources, which next build does not).
• npm run test:unit — 186 passed / 23 suites.
• npm run build — succeeds (after npm run migrate:up to create the local schema; an earlier failure was an unmigrated local DB, not the diff).
• npx playwright test --list — without keys, post.comments.sign-in / post.comments.submit are not defined and post.comments.unavailable-hidden still is; the suite loads with no errors.
• globalSetup no-op path executed directly without keys — returns cleanly.
• A real npx playwright test run confirmed globalSetup integrates into the harness (the run proceeded past it); it could not complete here because the pinned Playwright browser binary (chrome-headless-shell) isn't installed in this environment — CI installs it via npx playwright install --with-deps chromium.

Acceptance criteria status:

• ✅ Without Clerk keys, the two new tests are skipped and the suite loads unchanged; unavailable-hidden / draft-no-composer env-gating behaves as before. (verified via --list)
• ✅ @clerk/testing added as a devDependency; lint, unit, and build pass. (verified)
• ✅ e2e/scenarios.md facet tags for both scenarios match the catalog (posts / may); no catalog/facet errors on load, and the must gate is unaffected. (verified via --list)
• ⏳ Needs Clerk dev keys to run: the submit test signs a reader in, submits, sees the pending acknowledgment, and persists a pending comment with a non-empty authorProviderId that stays hidden from the public page; the sign-in test shows the affordance. (implemented + type/lint-checked; runtime validation pending keys — see trade-off below)

Trade-off: the actual Clerk-authenticated path only runs where the dev keys are configured, so it is not exercised in PR CI. It runs in the post-merge main e2e job once the secrets are added, or locally with keys in .env.local.

Independent review: clean — 0 code defects. The single "Important" item was a REVIEW.md-mandated disclosure that the three ⏳ criteria above can't execute in PR CI without Clerk keys (the deferral by design); it was dispositioned and the thread resolved. The reviewer independently confirmed the @clerk/testing API match, the composer data-testids, and --list parity on the unconfigured baseline.

Risks and breaking changes

• No application runtime code changes. The diff is E2E tests/helpers, a Playwright globalSetup, .env.example, the e2e CI job env, e2e/scenarios.md, and a devDependency. App behavior is unchanged.
• The CI env additions are inert when the secrets are unset (they resolve to empty → isClerkAvailable false → tests skip). clerkSetup() rejects a production secret key, guarding against pointing the suite at production.

Notes for reviewers

• Start at e2e/global-setup.ts (why globalSetup, not the setup project — token propagation across workers) and e2e/helpers/api/clerk.ts (why the server-side ticket over OAuth), then the two tests in e2e/tests/routes/posts/comments.test.ts.
• The sign-in mechanism assumes the emailAddress ticket works on the OAuth-only dev instance (expected; validated once keys are provided). If a dev instance restricts it, the fallback is enabling email code on that instance and using email_code + OTP 424242 — no app change needed.
• To exercise the Clerk path: set dev-instance NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY / CLERK_SECRET_KEY + a +clerk_test reader locally, or add them as Actions secrets (and optionally the CLERK_TEST_READER_EMAIL var) for the post-merge main run.

[axross]

@claude review

---

Generated by Claude Code

[github-actions]

🚀 Preview deployment

https://btnopen-pr-156.vercel.app

Deployed 8f65e00. Served by an isolated Turso database (preview-pr-156) seeded from repository fixtures — no production data — with media in a dedicated preview store; both are destroyed automatically when this pull request closes. This URL is stable for the pull request and always serves the latest preview.

[claude]

Code review

1 important, 0 nits

I reviewed this against REVIEW.md's mandatory checks (skill conformance + acceptance criteria) and the code itself.

Skill conformance: No violations found against the e2e-testing-guidelines, application-security-requirements, or development-guidelines skills. signInAsReader matches @clerk/testing's documented signIn({ page, emailAddress }) API exactly; the new process.env reads in e2e/global-setup.ts and e2e/helpers/api/clerk.ts follow the project's existing noProcessEnv suppression pattern (matches e2e/helpers/api/auth.ts); the two new tests reuse the file's existing locator, step, and cleanup conventions; e2e/scenarios.md tags/priority match the catalog; the PR body follows the template and links issue #155 correctly.

Verification I ran independently: npx tsc --noEmit (0 errors), npx biome check/format on the changed files (clean), npm run build (succeeds, after npm run migrate:up locally — matches the author's own note), and npx playwright test --list on both main and this branch without Clerk keys (identical 15 tests either time, confirming the new Clerk-gated tests add zero effect to the unconfigured baseline).

1 Important finding: Left inline — three of #155's acceptance criteria (the post.comments.sign-in/post.comments.submit runtime behavior and npm run coverage:scenarios) can't be confirmed from this diff or this PR's own CI, since the e2e job only runs post-merge on main and no Clerk dev credentials are available in either this PR's checks or this review environment. This isn't a code defect — the implementation matches the Clerk testing library's documented API and the composer's actual data-testids — it's flagged because the review policy requires naming any acceptance criterion that can't be confirmed from the diff.

This review is advisory and does not gate merging.