fix(web-e2e): expect the auth origin the app actually uses, not the one we exported - #855
Merged
Conversation
…ne we exported
The web e2e suite was UNRUNNABLE on any machine that had run `pnpm dev:secrets`,
and had been since the harness was written. Reproduced deterministically here:
183 seconds, then
next dev answered ... but /org/…/events never entered its route table
The route was serving correctly within a second. It redirected to
http://localhost:3001/login while the harness sat waiting for its own
http://127.0.0.1:3199, so the readiness predicate returned false on every poll
for the full three-minute budget and then blamed the route scan.
The harness exported AUTH_BASE_URL to `next dev` via process.env and assumed
that settled it. It does not: `getAuthBaseUrl()` resolves BINDING-FIRST
(`workerEnv() ?? process.env`), and under `next dev` the binding comes from
getPlatformProxy, which reads apps/web/.dev.vars — where the secrets manifest
has always written AUTH_BASE_URL=http://localhost:3001. The app's precedence is
correct; the harness's assumption was not.
CI has no .dev.vars, so it took the fallback and stayed green — the inverse of
the usual failure and exactly the class this repo's local-parity work exists to
remove. The origin is now read from .dev.vars when present and falls back to
the harness's isolated one otherwise, so CI behaviour is byte-for-byte
unchanged.
⚠️ This does NOT explain the one CI failure. That remains unproven, and I am
not claiming otherwise. What it does is make that failure mode self-diagnosing:
a 307 to a DIFFERENT origin cannot be a partial route table — the catch-all
rejects `org` and calls notFound(), so it emits 404 and never a redirect — so
a redirect proves the route is serving and the EXPECTATION is wrong. The loop
now says so immediately instead of polling it for 180 seconds.
Verified by restoring the old behaviour behind the fix: it now fails in 4
seconds with "configuration mismatch, not a slow route scan", and the
misleading "never entered its route table" message does not appear at all.
With the fix in place: 25 passed in 31s.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BRmGUnxeYsQoG9c8BCZcae
The review found a real bug in the fix, and it was the same bug one level down: my hand-rolled parser would have silently reinstated the 180-second misdiagnosis it was written to remove. `scripts/dev-preflight.mjs` strips surrounding quotes when it reads the very same file. Mine did not, so `AUTH_BASE_URL="http://localhost:3001"` would have been read WITH its quotes, never matched the redirect, and produced exactly the three-minute "never entered its route table" this PR exists to eliminate. It now parses the way that parser does — skipping comments, matching the key exactly rather than by prefix, and stripping quotes. Verified end to end with a quoted value in a real .dev.vars: 25 passed in 33s. Also switched both predicates from `startsWith` to an ORIGIN comparison. `startsWith` cannot tell http://localhost:3001 from http://localhost:30010, so a genuinely wrong origin sharing a prefix would have read as correct — the readiness check would have accepted the wrong server, and the fast-fail would have missed the one case it exists for. A relative or unparseable Location is deliberately neither a match nor a mismatch: it means "keep polling". A false fast-fail trades a slow correct answer for a quick wrong one, which is the worse bargain of the two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BRmGUnxeYsQoG9c8BCZcae
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
1 participant
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.
The web e2e suite was unrunnable on any machine that had run
pnpm dev:secrets, and had been since the harness was written. Reproduced deterministically: 183 seconds, thennext dev answered … but /org/…/events never entered its route table.The route was serving correctly within a second. It redirected to
http://localhost:3001/loginwhile the harness waited for its ownhttp://127.0.0.1:3199, so the readiness predicate returned false on every poll for the full budget — then blamed the route scan.The harness exported
AUTH_BASE_URLviaprocess.envand assumed that settled it. It doesn't:getAuthBaseUrl()resolves binding-first (workerEnv() ?? process.env), and undernext devthe binding comes fromgetPlatformProxyreadingapps/web/.dev.vars— where the manifest has always writtenAUTH_BASE_URL=http://localhost:3001. The app's precedence is correct; the harness's assumption wasn't.CI has no
.dev.vars, so it took the fallback and stayed green — green on CI, dead on every developer machine.This does not explain the one CI failure, and I'm not claiming it does. CI behaviour is byte-for-byte unchanged (same fallback). What it does is make that failure mode self-diagnosing: a 307 to a different origin cannot be a partial route table — the catch-all 404s and never redirects — so a redirect proves the route is serving and the expectation is wrong. The loop now says so at once.
Test plan
pnpm lint989 pass, 0 fail🤖 Generated with Claude Code