Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 131 additions & 2 deletions apps/web/playwright/deepest-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ import { describe, expect, it } from "vitest";
import {
ROUTE_TYPES_FILE,
appRoutes,
authOriginMismatch,
byDepthDesc,
deepestAppRoute,
routeTypesInclude,
routeTypesListRoutes,
effectiveAuthOrigin,
isRouteTableReady,
probeUrlFor,
routeTypesInclude,
routeTypesListRoutes,
urlSegments,
} from "./deepest-route";

Expand Down Expand Up @@ -218,3 +220,130 @@ describe("isRouteTableReady", () => {
expect(isRouteTableReady(302, `${AUTH}/login`, AUTH)).toBe(false);
});
});

// ── The auth origin the probe must expect ────────────────────────────────────────────────────────────
// The harness passed AUTH_BASE_URL to `next dev` via process.env and then demanded a 307 to that origin.
// But apps/web resolves it BINDING-FIRST (`getAuthBaseUrl`: workerEnv() ?? process.env), and under
// `next dev` the binding comes from getPlatformProxy — which reads `.dev.vars`. So on any machine that has
// run `pnpm dev:secrets`, the app redirected to http://localhost:3001 while the probe waited for
// http://127.0.0.1:3199, and the suite burned its whole 180s budget on a route that was serving correctly
// within a second. Green on CI (no `.dev.vars` there), dead on every developer machine — the inverse of
// the usual failure, and exactly the class this repo's local-parity work exists to remove.
describe("effectiveAuthOrigin", () => {
it("uses the origin the app will ACTUALLY resolve when .dev.vars sets one", () => {
expect(
effectiveAuthOrigin("AUTH_BASE_URL=http://localhost:3001\n", "http://127.0.0.1:3199"),
).toBe("http://localhost:3001");
});

it("falls back to the harness's isolated origin when there is no .dev.vars", () => {
// CI's case. Keeping the synthetic origin there preserves the property the comment relies on: an
// origin nothing else in the app points at, so nothing can wander off to a real auth server.
expect(effectiveAuthOrigin(null, "http://127.0.0.1:3199")).toBe("http://127.0.0.1:3199");
});

it("ignores a blank or absent binding rather than expecting an empty origin", () => {
// `pnpm dev:secrets` writes an unconfigured key as `NAME=`; treating "" as configured would make the
// probe demand a redirect to the empty string and never match.
expect(effectiveAuthOrigin("AUTH_BASE_URL=\n", "http://127.0.0.1:3199")).toBe(
"http://127.0.0.1:3199",
);
expect(effectiveAuthOrigin("OTHER=x\n", "http://127.0.0.1:3199")).toBe("http://127.0.0.1:3199");
});

it("strips a trailing slash so the startsWith marker cannot miss", () => {
expect(effectiveAuthOrigin("AUTH_BASE_URL=http://localhost:3001/\n", "http://x")).toBe(
"http://localhost:3001",
);
});
});

// A 307 to the WRONG origin cannot be a partial route table: the catch-all rejects `org` and calls
// notFound(), so it emits 404 and never a redirect. Only the real route redirects — meaning the route is
// serving and the harness simply expects the wrong origin. Polling that for 180s and then blaming the
// scan is what cost this session an hour; it is a configuration mismatch and should say so at once.
describe("authOriginMismatch", () => {
it("recognises a 307 to a different origin as a config mismatch", () => {
expect(authOriginMismatch(307, "http://localhost:3001/login", "http://127.0.0.1:3199")).toBe(
true,
);
});

it("is not triggered by the catch-all, which cannot redirect at all", () => {
expect(authOriginMismatch(404, null, "http://127.0.0.1:3199")).toBe(false);
expect(authOriginMismatch(200, null, "http://127.0.0.1:3199")).toBe(false);
});

it("is not triggered by the correct redirect", () => {
expect(authOriginMismatch(307, "http://127.0.0.1:3199/login", "http://127.0.0.1:3199")).toBe(
false,
);
});

it("does not fire on a 307 with no Location, which is malformed rather than mismatched", () => {
expect(authOriginMismatch(307, null, "http://127.0.0.1:3199")).toBe(false);
});
});

// The parser must accept everything a real `.dev.vars` can hold, because anything it mishandles silently
// reinstates the 180s misdiagnosis this file exists to remove. `scripts/dev-preflight.mjs` strips
// surrounding quotes; a hand-rolled parser that did not would read `"http://localhost:3001"` — quotes
// included — and never match the redirect.
describe("effectiveAuthOrigin parses what .dev.vars actually contains", () => {
it("strips surrounding quotes, as the repo's own .dev.vars parser does", () => {
expect(effectiveAuthOrigin('AUTH_BASE_URL="http://localhost:3001"\n', "http://x")).toBe(
"http://localhost:3001",
);
expect(effectiveAuthOrigin("AUTH_BASE_URL='http://localhost:3001'\n", "http://x")).toBe(
"http://localhost:3001",
);
});

it("ignores a COMMENTED-OUT key rather than reading it as configuration", () => {
expect(effectiveAuthOrigin("# AUTH_BASE_URL=http://commented\n", "http://x")).toBe("http://x");
});

it("is not fooled by a key that merely ends with the name", () => {
expect(effectiveAuthOrigin("NEXT_AUTH_BASE_URL=http://other\n", "http://x")).toBe("http://x");
});

it("finds the key wherever it sits in the file", () => {
expect(effectiveAuthOrigin("A=1\nAUTH_BASE_URL=http://found\nB=2\n", "http://x")).toBe(
"http://found",
);
});

it("strips a trailing slash from the FALLBACK too, not just the value", () => {
expect(effectiveAuthOrigin(null, "http://127.0.0.1:3199/")).toBe("http://127.0.0.1:3199");
});
});

// `startsWith` cannot distinguish http://localhost:3001 from http://localhost:30010, so a genuinely
// mismatched origin sharing a prefix would go unflagged — and fall back to the 180s timeout this PR is
// removing. Compare ORIGINS.
describe("origin comparison is exact, not prefix-based", () => {
it("does not treat a longer port as the expected origin", () => {
expect(isRouteTableReady(307, "http://localhost:30010/login", "http://localhost:3001")).toBe(
false,
);
expect(authOriginMismatch(307, "http://localhost:30010/login", "http://localhost:3001")).toBe(
true,
);
});

it("still accepts the genuine redirect", () => {
expect(isRouteTableReady(307, "http://localhost:3001/login", "http://localhost:3001")).toBe(
true,
);
expect(authOriginMismatch(307, "http://localhost:3001/login", "http://localhost:3001")).toBe(
false,
);
});

it("treats an unparseable or relative Location as 'keep polling', never as a mismatch", () => {
// Conservative on purpose: a false fast-fail would replace a slow correct answer with a fast wrong
// one, which is a worse trade than the timeout it avoids.
expect(authOriginMismatch(307, "/login", "http://localhost:3001")).toBe(false);
expect(isRouteTableReady(307, "/login", "http://localhost:3001")).toBe(false);
});
});
75 changes: 74 additions & 1 deletion apps/web/playwright/deepest-route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export function isRouteTableReady(
location: string | null,
authOrigin: string,
): boolean {
return status === 307 && (location ?? "").startsWith(authOrigin);
return status === 307 && locationHasOrigin(location, authOrigin) === true;
}

// ── Waiting for the SCAN, not for a request ──────────────────────────────────────────────────────────
Expand Down Expand Up @@ -159,3 +159,76 @@ export function routeTypesInclude(text: string, route: readonly string[]): boole
const want = `/${urlSegments(route).join("/")}`;
return routeTypesListRoutes(text).includes(want);
}

/**
* The auth origin the app will ACTUALLY redirect to, given the `.dev.vars` this machine has.
*
* The probe's whole discriminator is "a 307 to the auth origin, which the catch-all cannot produce". That
* only works if the harness expects the origin the app really uses — and the harness does not get to
* choose. `getAuthBaseUrl()` resolves BINDING-FIRST (`workerEnv() ?? process.env`), and under `next dev`
* the binding comes from `getPlatformProxy`, which reads `apps/web/.dev.vars`. So a machine that has run
* `pnpm dev:secrets` redirects to http://localhost:3001 no matter what the harness puts in process.env.
*
* The symptom was maximally misleading: a route serving correctly within a second, reported after 180
* seconds as "never entered its route table". Green on CI, where there is no `.dev.vars` at all, and dead
* on every developer machine.
*
* Falling back to the harness's own origin keeps CI isolated exactly as before — an origin nothing else in
* the app points at — so this widens nothing where the old assumption already held.
*/
export function effectiveAuthOrigin(devVars: string | null, fallback: string): string {
// Parsed the way `scripts/dev-preflight.mjs` parses the same file, because anything this mishandles
// silently reinstates the 180-second misdiagnosis above. In particular a QUOTED value — which that
// parser strips and a naive one does not — would be read with its quotes and never match the redirect.
let value = "";
for (const raw of (devVars ?? "").split("\n")) {
const line = raw.trim();
if (line === "" || line.startsWith("#")) continue; // a commented key is not configuration
const at = line.indexOf("=");
if (at < 0 || line.slice(0, at).trim() !== "AUTH_BASE_URL") continue; // exact key, not a suffix match
value = line.slice(at + 1).trim();
if (value.length >= 2 && /^(".*"|'.*')$/s.test(value)) value = value.slice(1, -1);
break;
}
// A blank is UNSET — `pnpm dev:secrets` writes an unconfigured key as `NAME=`, and demanding a redirect
// to the empty string would never match.
return stripTrailingSlash(value === "" ? fallback : value);
}

const stripTrailingSlash = (s: string): string => s.replace(/\/+$/, "");

/**
* Does `location` point at `origin`? Compared as ORIGINS, never as a string prefix.
*
* `startsWith` cannot tell http://localhost:3001 from http://localhost:30010, so a genuinely mismatched
* origin that happens to share a prefix would read as correct — the readiness predicate would accept the
* wrong server, and the fast-fail would miss the case it exists for.
*
* A relative or unparseable Location is NOT a match and NOT a mismatch: it means "keep polling". A false
* fast-fail would trade a slow correct answer for a quick wrong one, which is the worse bargain.
*/
export function locationHasOrigin(location: string | null, origin: string): boolean | null {
if (location === null) return null;
try {
return new URL(location).origin === new URL(origin).origin;
} catch {
return null; // relative or malformed — undecidable, so decide nothing
}
}

/**
* Is this response a CONFIGURATION mismatch rather than a route table still filling in?
*
* The catch-all `(app)/[...legacy]` rejects a first segment of `org` and calls `notFound()`, so a partial
* table yields 404 — never a redirect. A 307 therefore proves the real route is serving; if its Location
* points somewhere other than the origin the harness expects, the route is fine and the EXPECTATION is
* wrong. Continuing to poll that for the full boot budget and then reporting "never entered its route
* table" sends the reader through the wrong subsystem entirely, which is precisely what happened.
*/
export function authOriginMismatch(
status: number,
location: string | null,
authOrigin: string,
): boolean {
return status === 307 && locationHasOrigin(location, authOrigin) === false;
}
36 changes: 35 additions & 1 deletion apps/web/playwright/global-setup.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { spawn, type ChildProcess } from "node:child_process";
import { readFileSync } from "node:fs";
import { readFile } from "node:fs/promises";
import { resolve } from "node:path";
import { setTimeout as sleep } from "node:timers/promises";
Expand All @@ -11,7 +12,9 @@ import { setupSchema } from "../../../packages/db/test/migrate";
import { startEphemeralPostgres, type EphemeralPostgres } from "../../../packages/db/test/pg";
import {
ROUTE_TYPES_FILE,
authOriginMismatch,
deepestAppRoute,
effectiveAuthOrigin,
isRouteTableReady,
probeUrlFor,
routeTypesInclude,
Expand Down Expand Up @@ -124,7 +127,27 @@ async function seed(appUrl: string, ownerUrl: string): Promise<Fixture> {
}

/** Where the gate sends an unauthenticated request — see `AUTH_BASE_URL` below. */
const AUTH_ORIGIN = "http://127.0.0.1:3199";
const HARNESS_AUTH_ORIGIN = "http://127.0.0.1:3199";

/**
* The origin the probe must expect — read from this machine's `.dev.vars`, not assumed.
*
* apps/web resolves AUTH_BASE_URL binding-first, and under `next dev` the binding comes from
* getPlatformProxy reading `.dev.vars`. So the harness cannot impose its own origin via process.env, and
* assuming it could made the suite unrunnable on any machine that had run `pnpm dev:secrets` — see
* `effectiveAuthOrigin`. Resolved ONCE at module load: the file cannot change mid-run, and re-reading it
* per poll would be a second way for the loop to fail.
*/
const AUTH_ORIGIN = effectiveAuthOrigin(
(() => {
try {
return readFileSync(resolve(process.cwd(), ".dev.vars"), "utf8");
} catch {
return null; // CI has none, which is the isolated case the fallback preserves
}
})(),
HARNESS_AUTH_ORIGIN,
);

/** What the probe saw, so a timeout can report it instead of guessing. */
type ProbeResult = { ready: boolean; status: number; location: string | null };
Expand Down Expand Up @@ -273,6 +296,17 @@ async function startDevServer(appConnectionString: string): Promise<ChildProcess
}
lastProbe = await routeTableIsComplete(probeUrlFor(deepestRoute));
if (lastProbe.ready) return child;
// A 307 elsewhere is not a partial table — the catch-all 404s and cannot redirect at all — so the
// route is serving and the EXPECTED origin is wrong. Say so now rather than after 180s of polling
// under a message blaming the scan.
if (authOriginMismatch(lastProbe.status, lastProbe.location, AUTH_ORIGIN)) {
throw new Error(
`e2e: the deepest route IS serving — it redirected to ${lastProbe.location} — but this harness ` +
`expected ${AUTH_ORIGIN}. That is a configuration mismatch, not a slow route scan. apps/web ` +
`resolves AUTH_BASE_URL binding-first (getPlatformProxy reads apps/web/.dev.vars), so a value ` +
`there wins over the one this harness exports.`,
);
}
/* serving, but the route table is still partial — keep polling */
} catch (err) {
if (err instanceof Error && err.message.startsWith("e2e:")) {
Expand Down
Loading