test(e2e): pre-authenticated session harness to stay under the nonce rate limit - #125
Merged
Conversation
…rate limit POST /api/auth/nonce is rate-limited to 10 per minute per IP, and every Playwright test shares one IP. Signing in per test does not scale: a six-test suite already failed on back-to-back runs (measured 7x 201 then sustained 429) while each test passed in isolation, so the ~30-test target was unreachable. Add a 'setup' project that signs each dev wallet in once through the real UI and saves its session cookie; specs replay that state via signedInPage(). Nonce spend is now bounded by the number of wallets (5) rather than the number of tests, so adding tests costs nothing. Also allocates a dev wallet per spec. The SIWE tables live in a single shared 'app' schema that is NOT namespaced by PONDER_SCHEMA, so two specs sharing a key would collide on the same owned_profile/draft rows. Key #9 is called out specifically: it has delegated all its ARB away, so it is the only key that can exercise the avatar gate's 403 not_delegate branch.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced Sep 2, 2026
Context: The session harness signed in by navigating to /profile, but that route does not exist. It was deleted in 16499c1 (feat: delegate profile registration page), which is an ancestor of this branch's own merge base, so the page was already gone when the harness was written. The only page that mounts the SIWE gate, and so renders the siwe-sign-in control, is /delegates/register. Nothing caught it because no spec calls the harness yet: profile.spec signs itself in and already navigates to /delegates/register. Why: Every setup test would otherwise land on the not-found page and hang on the sign-in click until the 60s test timeout. The chromium project declares dependencies: ["setup"], so that failure takes the whole suite down with it. The constant is renamed rather than just repointed so it stops naming a page that no longer exists, which is what let the stale value survive the route rename in the first place. What: - Rename PROFILE_PATH to REGISTER_PATH and point it at /delegates/register. Both use sites (signIn, and signedInPage's default path) follow. - Replace the "controls live on the profile page" comment with a pointer to the component that actually mounts the gate. Verified: /profile has no page.tsx, no entry in the generated route manifest, no rewrite or redirect in next.config.mjs, and no middleware. app/not-found.tsx renders no sign-in control either. References: - PR #125 (test(e2e): pre-authenticated session harness) - 16499c1 feat: delegate profile registration page (#101) - code-review-e2e-session-harness.md, finding F1 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Context:
The setup project skips signing in when a saved session is still valid,
which is what makes repeat runs cost no nonces. It probed that by
opening a context from the saved storageState file and calling
/api/me, wrapped in a try/catch documented as making the harness
self-healing.
The catch never saw the most likely failure. browser.newContext() was
called above the try, and it is newContext(), not the request, that
throws when the file exists but does not parse. Confirmed against
Playwright 1.58.2: it throws on a truncated file, on a structurally
wrong one ("storageState.cookies: expected array, got string"), and on
non-JSON. existsSync() proves the file exists, not that it parses, and
a run interrupted during storageState({ path }) leaves exactly such a
partial file.
Why:
The result was a setup failure rather than a re-authentication, which
blocks the whole suite (chromium depends on setup) and can only be
cleared by deleting e2e/.auth by hand, guided by a Playwright internal
error message. Re-signing in costs one nonce; failing the run costs the
run. The comment explaining the placement is deliberate: the position
of that call is load-bearing and looks arbitrary, so it is the kind of
thing a later edit would quietly undo.
What:
- Declare context as BrowserContext | undefined, move
browser.newContext() inside the try, and close conditionally with
context?.close() in the finally.
- Record why the call has to sit inside the try.
Verified: a probe replicating the patched shape returns false, without
throwing, for truncated, wrong-typed, and non-JSON saved state, plus a
missing file. Every unusable state now means "re-authenticate".
References:
- PR #125 (test(e2e): pre-authenticated session harness)
- code-review-e2e-session-harness.md, finding F3
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Context:
The setup project was declared as name plus testMatch only, with no
`use` block, while the chromium project that consumes its output
carries devices["Desktop Chrome"]. So sign-in ran in a default
Chromium context and the saved state was replayed into Desktop Chrome
contexts.
Why:
Nothing is broken today, because what is saved is a cookie and cookies
do not vary by user agent or viewport. It is worth closing anyway: the
moment sign-in depends on either, for instance a control that only
renders at desktop widths, setup and the specs would diverge and
present as a confusing setup/spec mismatch rather than as a config
difference. Applied per project rather than hoisted into the top-level
`use` because that is the smaller change and matches how chromium
already declares it.
What:
- Give the setup project use: { ...devices["Desktop Chrome"] }.
References:
- PR #125 (test(e2e): pre-authenticated session harness)
- code-review-e2e-session-harness.md, finding F5
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Context:
The harness landed with no callers. signedInPage(), AUTH_WALLETS and
DEV_WALLETS were referenced only by the harness's own files, and
profile.spec still injected its key inline and signed itself in. So the
advertised win ("adding tests is now free") was not in effect, and the
cold-run nonce cost had gone up rather than down: 1 readiness probe
plus 5 setup plus 1 from the spec. An unexercised fixture is also an
unverified one, which is how the /profile route bug fixed earlier in
this branch survived in the first place.
Why:
Migrating the spec wholesale does not work. It ends by clicking
siwe-sign-out, and a session that is signed out is spent, so replaying
the shared saved session and then destroying it would send the profile
wallet back through the handshake on every run and defeat session reuse
for it permanently. Splitting the sign-out assertion onto a wallet that
setup never pre-authenticates keeps both properties: reuse holds for
all five shared wallets, and the full nonce to verify to cookie to
logout lifecycle stays covered. Dropping the sign-out assertion instead
was considered and rejected: it is the only coverage of that path, and
the specs that would replace it are four PRs away.
signedInPage becomes a fixture rather than staying a plain helper
because it created a BrowserContext and returned only the Page, leaking
one context per call for the worker's lifetime. A fixture closes them
in teardown. test.use({ storageState }) would be the more usual
Playwright shape but fixes one wallet per file, and specs in this stack
need several wallets in a single test.
What:
- Add e2e/fixtures/test.ts: the spec-facing `test`, carrying a
signedInPage(name, path?) fixture that tracks every context it opens
and closes them all in teardown.
- Remove the standalone signedInPage from fixtures/session.ts, export
REGISTER_PATH for the fixture's default, and point signIn's docstring
at the fixture while naming the one legitimate exception: a spec
testing the handshake or sign-out itself.
- Split profile.spec.ts. Profile management now takes
signedInPage("profile") and costs no nonce; "signing out clears the
session" keeps the lifecycle on its own wallet.
- Drop the duplicated wallet key literal from profile.spec.ts in favour
of DEV_WALLETS.profile, which also removes a comment that
contradicted wallets.ts about whether the key was shared.
- Add DEV_WALLETS.signOut (anvil account #5), deliberately absent from
AUTH_WALLETS, and record why on AUTH_WALLETS so it does not get added
later and silently undo reuse.
- Refresh the nonce arithmetic in auth.setup.ts, which still described
a six-wallet suite. Cold run is now 7, warm run 2.
The key was derived rather than recalled: all five existing
key/address pairs were confirmed with viem, which also identified #5 as
the next unused account after #1 to #4 and #9. It needs no ARB and no
delegation, since SIWE is an off-chain signature and this wallet never
reaches a voting-power gate.
Not verified end to end: the local indexer was unreachable, so the
suite was not run and these two specs have not executed against the
real app. tsc is clean and test discovery lists the expected 7 tests,
with signOut correctly getting no setup test. The one open risk is
whether the indexer's SIWE verify accepts an address with no on-chain
governance history; SiweGate gates on connection plus session only, and
noVotingPower is already expected to sign in, but the local indexer
checkout carries no SIWE code to confirm it against.
References:
- PR #125 (test(e2e): pre-authenticated session harness)
- code-review-e2e-session-harness.md, findings F2 and F4
- fix-plan-e2e-session-harness.md, Option A
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Context: global-setup waits for the backend stack by polling both /api/health and POST /api/auth/nonce once a second for up to 30s, treating a 201 from the nonce endpoint as proof that the real SIWE mount (ponder start plus a writable Postgres pool) is serving, not just that the process is up. That guarantee is worth keeping. The cost is not. The nonce endpoint is the rate-limited one, 10 per minute per IP, and it is the budget this whole branch exists to manage. Polling it meant a healthy stack spent a nonce before the suite began and a slow boot could spend up to 30, exhausting the limit and failing the run it was supposed to be clearing the way for, plus the run after it. Why: Dropping the nonce check would give up the readiness guarantee, and polling health alone can pass while the SIWE mount is still coming up. Splitting the two keeps both: health is free, so poll it, and the nonce probe only has to answer once health is up. A small bounded number of attempts covers a mount that lags health slightly, without the unbounded spend. What: - Poll /api/health at 1s intervals until it returns 200. - Then attempt the nonce probe at most 3 times, 2s apart. Normal cost is 1 nonce, worst case 3, down from up to 30. - Split the error message so "health never came up" and "health is up but SIWE is not serving" are distinguishable, and mention the rate limit as a possible cause of the latter. Measured against a stub backend: exactly 1 nonce request on a healthy stack, capped at 3 when the nonce endpoint returns 429, and the 201 requirement to pass is unchanged. This file was not part of the harness change and is strictly speaking out of its scope. It is included because the branch exists to reason about the nonce budget and this was the largest uncounted draw on it. References: - PR #125 (test(e2e): pre-authenticated session harness) - code-review-e2e-session-harness.md, Notes Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test(e2e): review follow-ups for the session harness
fionnachan
approved these changes
Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First of five in the SIWE drafts stack, reopened clean off
main. Supersedes #104.Not tied to any one feature: every later spec in this stack signs in through this harness, and the act-as spec will too when that stack is reopened.
The problem this fixes is measured, not theoretical
POST /api/auth/nonceis rate-limited to 10/min per IP, and every Playwright test shares one IP. Signing in per test does not scale — a six-test suite already failed on back-to-back runs while every test passed in isolation:So the ~30-test target was unreachable before this.
Fix, in two parts
The first part alone was not enough, which is worth knowing:
setupproject signs each dev wallet in once through the real UI (still exercising nonce → verify → cookie) and saves its session cookie. Nonce spend becomes a function of wallet count, not test count — adding tests is now free.profile.spec= 6 nonces, so two runs inside a minute still tripped the limit. Sessions last 7 days, so setup now probes/api/meand re-authenticates only on 401. Repeat runs cost zero nonces.Result — three consecutive runs, no pause:
A useful side effect: the 401 probe makes the harness self-healing. When
reset:siwetruncated sessions out from under it, setup noticed and re-authenticated on its own.Also here
A dev-wallet allocation per spec. The SIWE tables live in one shared
appschema that is not namespaced byPONDER_SCHEMA, so two specs sharing a key collide on the sameowned_profile/draftrows. Key #9 is called out specifically: it has delegated all its ARB away, making it the only key that can exercise the avatar gate's 403not_delegatebranch.