History

Make e2e port claiming atomic: lock port per block, walk past collisions

The hash-derived block was only collision-unlikely (28 checkouts over 400 blocks is birthday-paradox territory), and probe-then-bind raced. Each block now reserves its last port as a lock held for the suite's lifetime: claimPorts binds the lock (atomic — two racing suites can't both win), probes the remaining ports for squatters, and walks forward block-by-block until it owns a fully free block, publishing the claimed ports via E2E_*_PORT env so test workers agree. Explicit env pins skip claiming entirely. Verified by squatting the preferred block's lock+selfhost ports and watching the suite relocate one block over and pass 17/17.

Rhys Sullivan committed 5 months ago

Commit 3f3ed8f1 parent acb96656 files · +183 −86Browse files

Unified · Split

6 files
  1. e2e/AGENTS.md +7 −4
  2. e2e/scripts/ports.ts +6 −3
  3. e2e/setup/boot.ts +1 −39
  4. e2e/setup/cloud.globalsetup.ts +26 −24
  5. e2e/setup/selfhost.globalsetup.ts +20 −7
  6. e2e/src/ports.ts +123 −9
e2e/AGENTS.md+7−4
@@ -89,10 +89,13 @@ E2E_CLOUD_URL=http://127.0.0.1:<port> ../node_modules/.bin/vitest run --project · 88 unmodified lines
8989 E2E_SELFHOST_URL=http://localhost:<port> ../node_modules/.bin/vitest run --project selfhost <file>
9090 ```
9191
92−Ports are derived per checkout (hash of the repo root — see `src/ports.ts`),
93−so suites in different worktrees never fight. If a port is somehow taken, the
94−boot fails fast naming the squatting process; kill it or override with
95−`E2E_CLOUD_PORT`-style env vars.
92+Ports are claimed at boot (see `src/ports.ts`): each checkout hashes its repo
93+root to a preferred block, atomically locks it (a held lock port makes races
94+impossible), and walks to the next free block if it's locked or squatted — so
95+concurrent suites in different worktrees can never collide or attach to each
96+other's servers. `bun run ports` shows the preferred block; the boot log says
97+if a suite moved. `E2E_*_PORT` env vars pin ports explicitly (no probing) and
98+`E2E_<TARGET>_URL` attaches to a running instance.
9699
97100 Each run writes `runs/<target>/<slug>/result.json` plus any browser artifacts
98101 (trace.zip / session.mp4 / screenshots). `bun run serve` hosts the scenario ×
e2e/scripts/ports.ts+6−3
@@ -1,5 +1,8 @@
1−// Print this checkout's derived e2e ports (see src/ports.ts) so an agent or
2−// human can curl the booted servers or attach with E2E_<TARGET>_URL.
1+// Print this checkout's PREFERRED e2e ports (see src/ports.ts). These are
2+// where a suite normally boots; if the block is locked or squatted at boot
3+// time, claimPorts walks to the next free block and the suite logs the move.
4+// When attaching mid-run, the booted vite's actual port is authoritative —
5+// check the suite's log line or `ps | grep 'vite dev'`.
36 import {
47 AUTUMN_EMULATOR_PORT,
58 CLOUD_DB_PORT,
@@ -9,7 +12,7 @@ import { · 3 unmodified lines
912 import { SELFHOST_PORT } from "../targets/selfhost";
1013 import { repoRoot } from "../src/ports";
1114
12−console.log(`e2e ports for ${repoRoot}`);
15+console.log(`preferred e2e ports for ${repoRoot}`);
1316 console.log(` cloud http://127.0.0.1:${CLOUD_PORT}`);
1417 console.log(` cloud dev-db ${CLOUD_DB_PORT}`);
1518 console.log(` workos emulator ${WORKOS_EMULATOR_PORT}`);
e2e/setup/boot.ts+1−39
@@ -2,8 +2,7 @@ · 1 unmodified line
22 // server, wait until it answers HTTP, and hand vitest a teardown. The apps own
33 // what runs (their dev stack, their stub flags); this file only owns process
44 // lifecycle, so it stays target-agnostic.
5−import { execFileSync, spawn, type ChildProcess } from "node:child_process";
6−import { connect } from "node:net";
5+import { spawn, type ChildProcess } from "node:child_process";
76
87 export interface BootedProcesses {
98 readonly teardown: () => Promise<void>;
@@ -73,43 +72,6 @@ export const bootProcesses = ( · 63 unmodified lines
7372 };
7473 };
7574
76−// A port already in LISTEN before we boot means waitForHttp would silently
77−// attach to a FOREIGN server (a leaked dev server from another checkout or a
78−// crashed prior run) — every scenario then fails with baffling auth errors
79−// instead of one clear message. Fail fast and name the squatter.
80−export const ensurePortsFree = async (
81− ports: ReadonlyArray<{ readonly port: number; readonly label: string }>,
82−): Promise<void> => {
83− for (const { port, label } of ports) {
84− const inUse = await new Promise<boolean>((resolve) => {
85− const socket = connect({ port, host: "127.0.0.1" });
86− socket.once("connect", () => {
87− socket.destroy();
88− resolve(true);
89− });
90− socket.once("error", () => resolve(false));
91− socket.setTimeout(1_000, () => {
92− socket.destroy();
93− resolve(false);
94− });
95− });
96− if (!inUse) continue;
97− let owner = "(lsof unavailable)";
98− try {
99− owner = execFileSync("lsof", ["-nP", `-iTCP:${port}`, "-sTCP:LISTEN"], {
100− encoding: "utf8",
101− }).trim();
102− } catch {
103− // lsof failing is fine — the error below carries the essential fact.
104− }
105− throw new Error(
106− `e2e: port ${port} (${label}) is already in use — likely a leaked dev server ` +
107− `from another checkout or a previous run. Kill it or set the E2E_*_PORT/URL ` +
108− `env vars to relocate.\n${owner}`,
109− );
110− }
111−};
112−
11375 export const waitForHttp = async (
11476 url: string,
11577 options: { readonly timeoutMs?: number; readonly expectRedirect?: boolean } = {},
e2e/setup/cloud.globalsetup.ts+26−24
@@ -10,16 +10,9 @@ import { fileURLToPath } from "node:url"; · 9 unmodified lines
1010 // Vendored fork import (same pattern as mcporter).
1111 import { createEmulator } from "@executor-js/emulate";
1212
13−import { bootProcesses, ensurePortsFree, waitForHttp } from "./boot";
14−import {
15− CLOUD_BASE_URL,
16− CLOUD_DB_PORT,
17− CLOUD_PORT,
18− WORKOS_EMULATOR_PORT,
19− AUTUMN_EMULATOR_PORT,
20− E2E_WORKOS_CLIENT_ID,
21− E2E_COOKIE_PASSWORD,
22−} from "../targets/cloud";
13+import { claimPorts } from "../src/ports";
14+import { E2E_COOKIE_PASSWORD, E2E_WORKOS_CLIENT_ID } from "../targets/cloud";
15+import { bootProcesses, waitForHttp } from "./boot";
2316
2417 const cloudDir = fileURLToPath(new URL("../../apps/cloud/", import.meta.url));
2518
@@ -29,12 +22,19 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 3 unmodified lines
2922 return;
3023 }
3124
32− await ensurePortsFree([
33− { port: CLOUD_PORT, label: "cloud vite dev" },
34− { port: CLOUD_DB_PORT, label: "cloud dev-db (PGlite)" },
35− { port: WORKOS_EMULATOR_PORT, label: "WorkOS emulator" },
36− { port: AUTUMN_EMULATOR_PORT, label: "Autumn emulator" },
25+ // Claim a free port block (preferred block first, walk forward past
26+ // squatters/colliding checkouts) and publish via env so the test workers —
27+ // spawned after this — derive the same URLs. The imported targets/cloud
28+ // constants were computed BEFORE the claim, so use the claimed values here.
29+ const { ports, release } = await claimPorts([
30+ { envVar: "E2E_CLOUD_PORT", offset: 0, label: "cloud vite dev" },
31+ { envVar: "E2E_CLOUD_DB_PORT", offset: 1, label: "cloud dev-db (PGlite)" },
32+ { envVar: "E2E_WORKOS_EMULATOR_PORT", offset: 2, label: "WorkOS emulator" },
33+ { envVar: "E2E_AUTUMN_EMULATOR_PORT", offset: 3, label: "Autumn emulator" },
3734 ]);
35+ const cloudPort = ports.E2E_CLOUD_PORT!;
36+ const dbPort = ports.E2E_CLOUD_DB_PORT!;
37+ const baseUrl = `http://127.0.0.1:${cloudPort}`;
3838
3939 // Fresh dev DB per suite run — hermetic, like the selfhost data dir. The
4040 // WorkOS emulator mints org ids from a per-process counter, so a persisted
@@ -46,8 +46,8 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 5 unmodified lines
4646 // MCP access tokens minted by the emulator's OAuth server must carry the
4747 // app's client id as audience (what the resource server verifies).
4848 process.env.EMULATE_WORKOS_AUDIENCE = E2E_WORKOS_CLIENT_ID;
49− const workos = await createEmulator({ service: "workos", port: WORKOS_EMULATOR_PORT });
50− const autumn = await createEmulator({ service: "autumn", port: AUTUMN_EMULATOR_PORT });
49+ const workos = await createEmulator({ service: "workos", port: ports.E2E_WORKOS_EMULATOR_PORT! });
50+ const autumn = await createEmulator({ service: "autumn", port: ports.E2E_AUTUMN_EMULATOR_PORT! });
5151
5252 const env = {
5353 // Real client, emulated service.
@@ -58,16 +58,16 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 4 unmodified lines
5858 WORKOS_COOKIE_PASSWORD: E2E_COOKIE_PASSWORD,
5959 AUTUMN_SECRET_KEY: "am_test_emulate",
6060 ENCRYPTION_KEY: "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
61− DATABASE_URL: `postgresql://postgres:postgres@127.0.0.1:${CLOUD_DB_PORT}/postgres`,
61+ DATABASE_URL: `postgresql://postgres:postgres@127.0.0.1:${dbPort}/postgres`,
6262 EXECUTOR_DIRECT_DATABASE_URL: "true",
6363 CLOUDFLARE_INCLUDE_PROCESS_ENV: "true",
64− VITE_PUBLIC_SITE_URL: CLOUD_BASE_URL,
64+ VITE_PUBLIC_SITE_URL: baseUrl,
6565 // The AuthKit domain (MCP OAuth metadata + JWKS) is the emulator too.
6666 MCP_AUTHKIT_DOMAIN: workos.url,
67− MCP_RESOURCE_ORIGIN: CLOUD_BASE_URL,
67+ MCP_RESOURCE_ORIGIN: baseUrl,
6868 ALLOW_LOCAL_NETWORK: "true",
6969 // Throwaway PGlite on its own port + dir so it never fights `bun dev`.
70− DEV_DB_PORT: String(CLOUD_DB_PORT),
70+ DEV_DB_PORT: String(dbPort),
7171 DEV_DB_PATH: dbPath,
7272 };
7373
@@ -76,7 +76,7 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 2 unmodified lines
7676 { cmd: "bun", args: ["run", "scripts/dev-db.ts"], cwd: cloudDir, env },
7777 {
7878 cmd: "bunx",
79− args: ["vite", "dev", "--port", String(CLOUD_PORT), "--strictPort", "--host", "127.0.0.1"],
79+ args: ["vite", "dev", "--port", String(cloudPort), "--strictPort", "--host", "127.0.0.1"],
8080 cwd: cloudDir,
8181 env,
8282 },
@@ -85,18 +85,20 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 2 unmodified lines
8585 );
8686
8787 try {
88− await waitForHttp(CLOUD_BASE_URL);
88+ await waitForHttp(baseUrl);
8989 // The API plane is ready when login actually redirects to AuthKit.
90− await waitForHttp(`${CLOUD_BASE_URL}/api/auth/login`, { expectRedirect: true });
90+ await waitForHttp(`${baseUrl}/api/auth/login`, { expectRedirect: true });
9191 } catch (error) {
9292 await procs.teardown();
9393 await workos.close();
9494 await autumn.close();
95+ await release();
9596 throw error;
9697 }
9798 return async () => {
9899 await procs.teardown();
99100 await workos.close();
100101 await autumn.close();
102+ await release();
101103 };
102104 }
e2e/setup/selfhost.globalsetup.ts+20−7
@@ -6,8 +6,9 @@ import { rmSync } from "node:fs"; · 5 unmodified lines
66 import { resolve } from "node:path";
77 import { fileURLToPath } from "node:url";
88
9−import { bootProcesses, ensurePortsFree, waitForHttp } from "./boot";
10−import { SELFHOST_ADMIN, SELFHOST_BASE_URL, SELFHOST_PORT } from "../targets/selfhost";
9+import { claimPorts } from "../src/ports";
10+import { SELFHOST_ADMIN } from "../targets/selfhost";
11+import { bootProcesses, waitForHttp } from "./boot";
1112
1213 const selfhostDir = fileURLToPath(new URL("../../apps/host-selfhost/", import.meta.url));
1314
@@ -17,7 +18,15 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 3 unmodified lines
1718 return;
1819 }
1920
20− await ensurePortsFree([{ port: SELFHOST_PORT, label: "selfhost vite dev" }]);
21+ // Claim a free port (preferred block first, walk forward past squatters)
22+ // and publish via env so the test workers derive the same URL. The imported
23+ // targets/selfhost constants were computed BEFORE the claim — don't use them
24+ // for ports/URLs here.
25+ const { ports, release } = await claimPorts([
26+ { envVar: "E2E_SELFHOST_PORT", offset: 4, label: "selfhost vite dev" },
27+ ]);
28+ const port = ports.E2E_SELFHOST_PORT!;
29+ const baseUrl = `http://localhost:${port}`;
2130
2231 // Fresh data dir per suite run — hermetic; in-suite isolation comes from
2332 // fresh identities, not resets.
@@ -28,14 +37,14 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 4 unmodified lines
2837 [
2938 {
3039 cmd: "bunx",
31− args: ["--bun", "vite", "dev", "--port", String(SELFHOST_PORT), "--strictPort"],
40+ args: ["--bun", "vite", "dev", "--port", String(port), "--strictPort"],
3241 cwd: selfhostDir,
3342 env: {
3443 EXECUTOR_DATA_DIR: dataDir,
3544 BETTER_AUTH_SECRET: "executor-selfhost-e2e-secret-0123456789",
3645 EXECUTOR_BOOTSTRAP_ADMIN_EMAIL: SELFHOST_ADMIN.email,
3746 EXECUTOR_BOOTSTRAP_ADMIN_PASSWORD: SELFHOST_ADMIN.password,
38− EXECUTOR_WEB_BASE_URL: SELFHOST_BASE_URL,
47+ EXECUTOR_WEB_BASE_URL: baseUrl,
3948 // The harness boots loopback MCP/OAuth test servers and points the
4049 // instance at them; the hosted SSRF guard would otherwise block
4150 // outbound probes/dials to localhost. Hermetic test instance only.
@@ -47,10 +56,14 @@ export default async function setup(): Promise<(() => Promise<void>) | void> { · 5 unmodified lines
4756 );
4857
4958 try {
50− await waitForHttp(SELFHOST_BASE_URL);
59+ await waitForHttp(baseUrl);
5160 } catch (error) {
5261 await procs.teardown();
62+ await release();
5363 throw error;
5464 }
55− return procs.teardown;
65+ return async () => {
66+ await procs.teardown();
67+ await release();
68+ };
5669 }
e2e/src/ports.ts+123−9
@@ -1,11 +1,18 @@
11 // Per-checkout port derivation: every checkout (main repo, agent worktree,
2−// /tmp rig) gets its own deterministic block of e2e ports, so concurrent
3−// suites never fight over a shared default. The collision failure mode is
4−// brutal: vite's --strictPort exit is swallowed by the boot glue and
5−// waitForHttp happily attaches to the OTHER checkout's server, failing dozens
6−// of scenarios with baffling auth errors instead of one clear bind error.
7−// Individual E2E_*_PORT env vars still override, and E2E_<TARGET>_URL still
8−// attaches to a running instance.
2+// /tmp rig) hashes its repo root into a PREFERRED block of e2e ports, so
3+// concurrent suites normally never fight over a shared default. The hash is
4+// only a preference, not a guarantee (28 checkouts over 400 blocks is
5+// birthday-paradox territory) — the globalsetups call `claimPorts`, which
6+// probes the preferred block and walks forward to the next fully-free one,
7+// then publishes the claimed ports via the E2E_*_PORT env vars so vitest's
8+// test workers (spawned after globalsetup) compute the same URLs. The
9+// collision failure mode this kills is brutal: vite's --strictPort exit is
10+// swallowed by the boot glue and waitForHttp happily attaches to the OTHER
11+// checkout's server, failing dozens of scenarios with baffling auth errors
12+// instead of one clear bind error. Individual E2E_*_PORT env vars still
13+// override everything, and E2E_<TARGET>_URL still attaches to a running
14+// instance.
15+import { connect, createServer, type Server } from "node:net";
916 import { resolve } from "node:path";
1017 import { fileURLToPath } from "node:url";
1118
@@ -24,10 +31,117 @@ const hash = (text: string): number => { · 12 unmodified lines
2431 };
2532
2633 // 400 blocks of 10 ports in 42000..45999: unprivileged, clear of common dev
27−// servers, and below macOS's ephemeral range (49152+).
28−export const portBlock = 42000 + (hash(repoRoot) % 400) * 10;
34+// servers, and below macOS's ephemeral range (49152+). Offsets 0-8 are
35+// claimable; offset 9 is the block's lock port (held for the suite's
36+// lifetime to make claims atomic across concurrent suites).
37+const BLOCK_BASE = 42000;
38+const BLOCK_SIZE = 10;
39+const BLOCK_COUNT = 400;
40+const LOCK_OFFSET = BLOCK_SIZE - 1;
41+export const portBlock = BLOCK_BASE + (hash(repoRoot) % BLOCK_COUNT) * BLOCK_SIZE;
2942
3043 export const e2ePort = (envVar: string, offset: number): number => {
3144 const fromEnv = process.env[envVar];
3245 return fromEnv ? Number(fromEnv) : portBlock + offset;
3346 };
47+
48+const isListening = (port: number): Promise<boolean> =>
49+ new Promise((done) => {
50+ const socket = connect({ port, host: "127.0.0.1" });
51+ socket.once("connect", () => {
52+ socket.destroy();
53+ done(true);
54+ });
55+ socket.once("error", () => done(false));
56+ socket.setTimeout(1_000, () => {
57+ socket.destroy();
58+ done(false);
59+ });
60+ });
61+
62+export interface PortClaim {
63+ readonly envVar: string;
64+ readonly offset: number;
65+ readonly label: string;
66+}
67+
68+export interface ClaimedPorts {
69+ readonly ports: Record<string, number>;
70+ /** Releases the block's lock port — call from the suite teardown. */
71+ readonly release: () => Promise<void>;
72+}
73+
74+// Binding is atomic where probing is not: holding the block's lock port for
75+// the suite's lifetime means two suites racing for the same block can never
76+// both win (the second bind EADDRINUSEs and walks on).
77+const tryLockBlock = (block: number): Promise<Server | undefined> =>
78+ new Promise((done) => {
79+ const server = createServer();
80+ server.once("error", () => done(undefined));
81+ server.listen(block + LOCK_OFFSET, "127.0.0.1", () => done(server));
82+ });
83+
84+/**
85+ * Claim a free set of ports for a target and publish them via env. Starts at
86+ * this checkout's preferred block and walks forward block-by-block until it
87+ * can atomically lock a block whose requested ports are all free — so two
88+ * checkouts whose hashes collide (or a leaked server squatting the preferred
89+ * block) degrade to "boot one block over" instead of attaching to a foreign
90+ * server. Explicit env overrides win and are never probed or locked: if you
91+ * pin a port and it's busy, vite's --strictPort fails visibly. A target
92+ * re-claiming inside an already-locked process (cloud + selfhost projects in
93+ * one vitest run) shares the block via disjoint offsets.
94+ */
95+export const claimPorts = async (claims: ReadonlyArray<PortClaim>): Promise<ClaimedPorts> => {
96+ const ports: Record<string, number> = {};
97+ const unpinned = claims.filter((claim) => {
98+ const pinned = process.env[claim.envVar];
99+ if (pinned) ports[claim.envVar] = Number(pinned);
100+ return !pinned;
101+ });
102+ if (unpinned.length === 0) return { ports, release: async () => {} };
103+
104+ for (let attempt = 0; attempt < BLOCK_COUNT; attempt++) {
105+ const block =
106+ BLOCK_BASE + ((portBlock - BLOCK_BASE + attempt * BLOCK_SIZE) % (BLOCK_COUNT * BLOCK_SIZE));
107+ // This process may already hold the block's lock (the other target's
108+ // globalsetup in the same vitest run); reuse it instead of re-locking.
109+ let lock = heldLocks.get(block);
110+ if (!lock) {
111+ lock = await tryLockBlock(block);
112+ if (!lock) {
113+ console.warn(`[e2e] port block ${block} is locked by another suite; trying next block`);
114+ continue;
115+ }
116+ heldLocks.set(block, lock);
117+ }
118+ const busy = await Promise.all(unpinned.map((claim) => isListening(block + claim.offset)));
119+ if (busy.some(Boolean)) {
120+ const taken = unpinned
121+ .filter((_, index) => busy[index])
122+ .map((claim) => `${block + claim.offset} (${claim.label})`);
123+ console.warn(
124+ `[e2e] port block ${block} has squatters — ${taken.join(", ")}; trying next block`,
125+ );
126+ continue; // Keep the lock: a half-busy block is still ours, just unusable now.
127+ }
128+ for (const claim of unpinned) {
129+ const port = block + claim.offset;
130+ ports[claim.envVar] = port;
131+ // Workers spawn after globalsetup, so they inherit these and agree.
132+ process.env[claim.envVar] = String(port);
133+ }
134+ return {
135+ ports,
136+ release: async () => {
137+ const held = heldLocks.get(block);
138+ if (!held) return;
139+ heldLocks.delete(block);
140+ await new Promise<void>((done) => held.close(() => done()));
141+ },
142+ };
143+ }
144+ throw new Error("e2e: no free port block found — the 42000-45999 range is exhausted?");
145+};
146+
147+const heldLocks = new Map<number, Server>();

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t