Archilyzer · Source

archilyzer

Archilyzer
git clone https://archilyzer.pages.dev/source/archilyzer.git
Log | Files | Refs | README | LICENSE

commit 564df959ba21fc38369e28de1d82c2dff707bcd4
parent b57494480c474b532ec1ad852222112abc101caa
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date:   Tue, 11 Aug 2026 16:48:50 -0400

Serialize e2e runs into one machine-wide queue

Running two suites at once ate the box (the serial suite is ~24 min and
e2e:sharded fans out to 4 containers), so every e2e entry point now takes a
global flock on <git-common-dir>/e2e-queue.lock and waits its turn, printing
who holds the queue. WORKTREES.md previously encouraged the opposite.

It also closes a silent, destructive bug. The playwright configs set
reuseExistingServer: !CI, and worktree.mjs hands .claude/worktrees/* checkouts
main's own port block, so a second run attached to another session's editor
test server and the suite's 303 resetData() calls wiped that session's fixture
data with no error. After acquiring the lock a run now verifies its ports are
free and aborts, naming the port, its env var and the owning pid, instead of
driving someone else's server.

The lock is deliberately NOT held by wrapping the command in flock. flock(1)
does not set FD_CLOEXEC on its lock fd, so a shell-spawned descendant -- which
is exactly how playwright's webServer starts `pnpm dev:test` -- inherits it and
holds the lock until that stray dies. Verified: after such a run ends, a
`flock -n` probe still reports the lock held. Given this suite's history of
leaked fixture processes, one orphan would have deadlocked every worktree
permanently. So the lock is held by a dedicated child with no children of its
own, and the real command runs as its sibling. Release is by closing the
holder's stdin, and the kernel drops the lock when its fd closes -- so a
SIGKILLed run cannot strand the queue and no stale-lock recovery exists.

Wrapping is at the package level, so `pnpm --filter editor run e2e` is covered
too; QUEUE_LOCK_HELD makes a nested invocation pass through rather than
deadlock against its parent's lock. e2e:sharded's docker build moves into
run-sharded-e2e.mjs so it runs under the lock as well -- the image tag is
fixed, so two worktrees building at once left it pointing at whichever
finished last.

Also fixes two latent port bugs the preflight would otherwise trip over:
OLLAMA_STUB_PORT was missing from PORT_BASES, so every worktree shared one
stub port, and export/playwright.config.ts read PORT, which under `wt run` is
the editor's test port.

Escape hatches: E2E_QUEUE=0, E2E_PORT_CHECK=0, E2E_QUEUE_TIMEOUT.
Not queued on purpose: e2e:ui, and a raw `playwright test` for one spec.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Diffstat:
MAGENTS.md | 16++++++++++++----
MWORKTREES.md | 71+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------
Meditor/package.json | 2+-
Mexport/package.json | 6+++---
Mexport/playwright.config.ts | 6+++++-
Mpackage.json | 3++-
Ascripts/queue-lock.mjs | 568+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Ascripts/queue-lock.test.mjs | 244+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mscripts/run-sharded-e2e.mjs | 42+++++++++++++++++++++++++++++++++++++++---
Mscripts/worktree.mjs | 1+
10 files changed, 940 insertions(+), 19 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md @@ -6,10 +6,18 @@ This version has breaking changes — APIs, conventions, and file structure may # Parallel work with git worktrees -To run more than one checkout at once (parallel dev servers / e2e), use git worktrees with -per-worktree non-colliding ports. `pnpm wt add <branch>` creates one; `pnpm wt list` shows -each worktree's port block. `pnpm dev:editor` / `pnpm e2e` auto-assign ports per worktree. -See [WORKTREES.md](WORKTREES.md) for the port scheme and the shared-data caveat. +To run more than one checkout at once, use git worktrees with per-worktree non-colliding +ports. `pnpm wt add <branch>` creates one; `pnpm wt list` shows each worktree's port block. +`pnpm dev:editor` auto-assigns ports per worktree, so dev servers run in parallel. + +**e2e does not run in parallel.** Every e2e entry point takes a machine-global lock, so one +suite runs at a time and the rest wait. If `pnpm e2e` prints `waiting for the e2e queue — +held by …` and sits there, **that is working as intended, not a hung command** — the serial +suite is ~24 minutes. A run that wins the lock but finds its ports already bound aborts and +names the offending pid, instead of silently driving another session's server. Bypasses: +`E2E_QUEUE=0`, `E2E_PORT_CHECK=0`, `E2E_QUEUE_TIMEOUT=<seconds>`. + +See [WORKTREES.md](WORKTREES.md) for the port scheme, the queue, and the shared-data caveat. # Roadmap diff --git a/WORKTREES.md b/WORKTREES.md @@ -1,9 +1,11 @@ # Parallel development with git worktrees Git worktrees let you check out several branches at once, each in its own directory, -sharing one `.git`. This repo supports running multiple worktrees **simultaneously** — -parallel dev servers and parallel e2e suites — by giving each worktree its own -non-colliding block of ports. +sharing one `.git`. This repo supports running multiple worktrees **simultaneously** by +giving each worktree its own non-colliding block of ports. + +That applies to **dev servers**. **e2e suites are deliberately serialized**: one run at a +time across the whole machine, everyone else waits in line. See [The e2e queue](#the-e2e-queue). The helper is `scripts/worktree.mjs`, exposed as `pnpm wt`. @@ -20,6 +22,7 @@ defaults — nothing changes for the primary checkout. | `EXPORT_PORT` | 3010 | export server launched by the editor e2e suite | | `EXPORT_DEV_PORT` | 3000 | export real dev (`pnpm dev:export`) | | `EXPORT_E2E_PORT` | 3020 | export's own Playwright suite | +| `OLLAMA_STUB_PORT` | 11435 | digest-lane stub server in the editor e2e suite | | `PLAYWRIGHT_BASE_URL` | `http://localhost:3011` | node-side fetches in specs | So worktree #1 runs editor on **3101**, test server on **3111**, export on **3110**, etc. @@ -60,10 +63,66 @@ pnpm wt add feature-x # creates ../feature-x, prints its ports, seeds s # terminal B (../feature-x): pnpm dev:editor -> http://localhost:3101 # both run at once, no EADDRINUSE -# parallel e2e — run in each worktree at the same time: -pnpm e2e # main: editor 3011 + export 3010 -cd ../feature-x && pnpm e2e # feature-x: editor 3111 + export 3110 +# e2e is queued, not parallel — start it anywhere, it waits its turn: +pnpm e2e # main: runs now +cd ../feature-x && pnpm e2e # feature-x: prints "waiting …", starts when main finishes +``` + +## The e2e queue + +Every e2e entry point takes a machine-global lock before it runs, so **exactly one suite +runs at a time**. Starting a second one is not an error — it prints who holds the queue and +blocks until its turn: + ``` +queue-lock: waiting for the e2e queue — held by yt-dlp-transcript-browser (main, pid 2401494) for 4m12s + (one e2e run at a time, machine-wide; E2E_QUEUE=0 to bypass) +queue-lock: still waiting (5m00s) +``` + +**A long wait here is normal, not a hang** — the serial suite is ~24 minutes. + +Queued: `pnpm e2e`, `pnpm e2e:sharded` (including its `docker build`), and export's `e2e`, +`e2e:hub`, `e2e:2origin`. Wrapping is at the *package* level, so `pnpm --filter editor run +e2e` is covered too, and a nested invocation passes through instead of deadlocking. Not +queued on purpose: a raw `pnpm --filter editor exec playwright test`, the escape hatch for +debugging a single spec, and the `e2e:ui` interactive sessions. + +The lock lives at `<git-common-dir>/e2e-queue.lock`, which resolves to the same file from +every worktree. The kernel releases it when the holding process's fd closes, so a `kill -9` +or a crashed run can never strand the queue — there is no stale-lock recovery to run. + +### Aborting on a busy port + +After taking the lock, a run checks that the ports it is about to use are actually free. If +one is not, it **aborts** rather than starting: + +``` +queue-lock: ABORTING — this suite's ports are already in use. + PORT=3011 pid 2244791 cwd /home/user/Projects/yt-dlp-transcript-browser/editor + next-server (v16.2.3) + kill 2244791 +``` + +This closes a silent, destructive bug. The Playwright configs set +`reuseExistingServer: !CI`, so a run that found port 3011 already bound used to attach to +**whoever else's** editor test server was there — and the suite's 303 `resetData()` calls +would then wipe that session's fixture data with no error at all. Because we hold the queue +lock at that moment, no *queued* run can own those ports: what you are looking at is a +leftover from a killed run, or a server someone started by hand. + +The trade-off: a hand-started `pnpm dev:test` on 3011 is no longer silently reused, so the +~30s server boot is no longer skippable. `E2E_PORT_CHECK=0` restores the old behavior. + +### Escape hatches + +| Variable | Effect | +|---|---| +| `E2E_QUEUE=0` | Skip the queue entirely (the port preflight still runs) | +| `E2E_PORT_CHECK=0` | Skip the port preflight, reusing whatever servers are up | +| `E2E_QUEUE_TIMEOUT=<seconds>` | Give up waiting after N seconds (default: wait forever) | + +Verify the queue with `pnpm test:scripts`. ## Data directories diff --git a/editor/package.json b/editor/package.json @@ -10,7 +10,7 @@ "build": "next build", "start": "next start --port ${EDITOR_PORT:-3001}", "lint": "eslint", - "e2e": "playwright test", + "e2e": "node ../scripts/queue-lock.mjs --ports PORT:3011,EXPORT_PORT:3010,OLLAMA_STUB_PORT:11435 -- playwright test", "e2e:ui": "playwright test --ui" }, "dependencies": { diff --git a/export/package.json b/export/package.json @@ -19,9 +19,9 @@ "build:hub": "pnpm run compose:hub && INSTANCE_MODE=hub next build", "start": "serve out", "lint": "eslint", - "e2e": "playwright test", - "e2e:hub": "playwright test --config playwright.hub.config.ts", - "e2e:2origin": "playwright test --config playwright.2origin.config.ts", + "e2e": "node ../scripts/queue-lock.mjs --ports EXPORT_E2E_PORT:3020 -- playwright test", + "e2e:hub": "node ../scripts/queue-lock.mjs --ports HUB_PORT:3041 -- playwright test --config playwright.hub.config.ts", + "e2e:2origin": "node ../scripts/queue-lock.mjs --ports ORIGIN_B_PORT:4610,HUB_A_PORT:4611 -- playwright test --config playwright.2origin.config.ts", "e2e:ui": "playwright test --ui", "deploy": "pnpm dlx wrangler pages deploy out" }, diff --git a/export/playwright.config.ts b/export/playwright.config.ts @@ -3,7 +3,11 @@ import path from "node:path"; import { defineConfig, devices } from "@playwright/test"; import { buildFixtureSettings } from "./e2e/fixtures/data"; -const PORT = Number(process.env.PORT ?? 3020); +// EXPORT_E2E_PORT, not PORT: under `wt run` (scripts/worktree.mjs) PORT is the +// *editor's* test-server port (3011), so reading it here aimed this suite at the +// editor's server. PORT is kept as a fallback for a bare `playwright test` with +// a hand-set port. +const PORT = Number(process.env.EXPORT_E2E_PORT ?? process.env.PORT ?? 3020); const baseURL = `http://localhost:${PORT}`; const TEST_SETTINGS_FILE = path.resolve(process.cwd(), "test-settings.json"); diff --git a/package.json b/package.json @@ -17,7 +17,8 @@ "deploy:homepage": "pnpm --filter homepage run deploy", "e2e": "node scripts/worktree.mjs run -- pnpm --filter editor run e2e", "wt": "node scripts/worktree.mjs", - "e2e:sharded": "docker build -f Dockerfile.test -t yt-dlp-transcript-browser-e2e . && node scripts/run-sharded-e2e.mjs", + "e2e:sharded": "node scripts/run-sharded-e2e.mjs", + "test:scripts": "node --test scripts/*.test.mjs", "lint": "pnpm --filter export run lint" }, "devDependencies": { diff --git a/scripts/queue-lock.mjs b/scripts/queue-lock.mjs @@ -0,0 +1,568 @@ +#!/usr/bin/env node +// Global e2e queue: exactly one e2e run at a time, machine-wide. +// +// Every checkout of this repo shares one lock file, so a suite started in a +// second worktree waits for the first to finish instead of racing it. That is +// both a resource decision (the serial suite is ~24 min and e2e:sharded fans +// out to 4 containers) and a correctness one: playwright.config's +// `reuseExistingServer: !CI` means a second run that finds port 3011 already +// bound silently drives the *other* worktree's test server, and resetData() +// then wipes that session's fixtures with no error at all. +// +// node scripts/queue-lock.mjs [--name e2e] [--ports PORT:3011,...] -- <cmd...> +// +// WHY THE LOCK IS HELD BY A SEPARATE CHILD. +// flock(1) deliberately keeps its lock fd open across exec — that is what the +// -o/--close flag exists to undo — so the fd is inherited by every descendant, +// transitively. Running the suite *under* flock would therefore hold the lock +// until the last grandchild dies, not until playwright exits. This suite leaks +// grandchildren for real (see editor/e2e/fixtureProcs.ts, which exists because +// 27 strays once accumulated, and e2e/fixtures/bin/fake-parakeet-stitch.mjs, +// which ignores SIGTERM on purpose), and one leaked stray would deadlock every +// worktree on the machine forever — strictly worse than the bug being fixed. +// +// So the lock is held by a dedicated child that has no children of its own, +// and the real command runs as its SIBLING: +// +// node queue-lock.mjs (this process) +// |-- flock -F <lock> node queue-lock.mjs --hold (owns the fd; no kids) +// `-- playwright test -> next dev -> fixtures (no lock fd anywhere) +// +// Acquisition is signalled by the holder writing ACQUIRED on a pipe, so the +// waiting banner is cancelled deterministically rather than by polling. +// Release is by closing the holder's stdin: it exits, and the kernel drops the +// lock when the fd closes. That makes the queue crash-proof in both +// directions — a SIGKILLed run can never strand it, so there is no stale-lock +// recovery code here on purpose. +import { execFileSync, spawn } from "node:child_process"; +import fs from "node:fs"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const SELF = fileURLToPath(import.meta.url); + +// Set in the environment of the command we run, so a nested invocation (root +// `pnpm e2e` -> `pnpm --filter editor run e2e`, both of which go through this +// wrapper) passes straight through instead of deadlocking against the lock its +// own parent is holding. Verified to survive nested `pnpm --filter` calls. +export const HELD_ENV = "QUEUE_LOCK_HELD"; + +const PROBE_HELD_EXIT = 91; // `flock -n -E 91`: distinguishes held from failed +const WAIT_TIMEOUT_EXIT = 92; // `flock -w N -E 92` +const EXIT_PORTS = 1; // preflight found a bound port +const EXIT_TIMEOUT = 3; // gave up waiting for the queue +const TICK_MS = 30_000; +const DEFAULT_PORT_GRACE_MS = 3_000; + +// ---------------------------------------------------------------- arguments + +function parseArgv(argv) { + const sep = argv.indexOf("--"); + const flags = sep === -1 ? argv : argv.slice(0, sep); + const cmd = sep === -1 ? [] : argv.slice(sep + 1); + const opts = { + name: "e2e", + portSpec: null, + hold: false, + timeoutMs: defaultTimeoutMs(), + portGraceMs: Number(process.env.E2E_PORT_GRACE_MS ?? DEFAULT_PORT_GRACE_MS), + }; + for (let i = 0; i < flags.length; i++) { + const f = flags[i]; + if (f === "--hold") opts.hold = true; + else if (f === "--name") opts.name = flags[++i]; + else if (f === "--ports") opts.portSpec = flags[++i]; + else if (f === "--timeout") opts.timeoutMs = Number(flags[++i]) * 1000; + else if (f === "--port-grace") opts.portGraceMs = Number(flags[++i]); + else { + process.stderr.write(`queue-lock: unknown flag '${f}'\n`); + process.exit(2); + } + } + return { opts, cmd }; +} + +// Waiting forever is the point of the feature, so that is the default. A +// bounded wait is available for anything that would rather fail than block. +function defaultTimeoutMs() { + const raw = process.env.E2E_QUEUE_TIMEOUT; + if (raw == null || raw === "") return 0; + const n = Number(raw); + return Number.isFinite(n) && n > 0 ? n * 1000 : 0; +} + +// "PORT:3011,EXPORT_PORT:3010" -> [{envVar, port}]. The literal is the same +// default the matching playwright config uses, so the check and the run always +// agree; an env var (e.g. from `wt run`) wins, exactly as it does in the config. +function parsePorts(spec) { + if (!spec || process.env.E2E_PORT_CHECK === "0") return []; + return spec + .split(",") + .map((s) => s.trim()) + .filter(Boolean) + .map((entry) => { + const [envVar, fallback] = entry.split(":"); + const port = Number(process.env[envVar] ?? fallback); + if (!Number.isInteger(port) || port <= 0) { + process.stderr.write(`queue-lock: bad --ports entry '${entry}'\n`); + process.exit(2); + } + return { envVar, port }; + }); +} + +// ------------------------------------------------------------- lock location + +function gitOut(args) { + try { + return execFileSync("git", args, { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + } catch { + return null; + } +} + +// The same .git for every checkout: `--git-common-dir` resolves to the main +// repository's .git from the main worktree, from ../feat-* siblings, and from +// .claude/worktrees/* alike — so one file is genuinely machine-global. Living +// inside .git/ it is also untracked by construction (no .gitignore entry). +function lockFileFor(name) { + if (process.env.E2E_QUEUE_LOCK_FILE) return process.env.E2E_QUEUE_LOCK_FILE; + const dir = + gitOut(["rev-parse", "--path-format=absolute", "--git-common-dir"]) ?? + os.tmpdir(); + return path.join(dir, `${name}-queue.lock`); +} + +// ------------------------------------------------------- holder bookkeeping + +function holderFileFor(lock) { + return `${lock}.holder.json`; +} + +function writeHolderJson(lock, name, cmd) { + const info = { + name, + pid: process.pid, + worktree: gitOut(["rev-parse", "--show-toplevel"]) ?? process.cwd(), + branch: gitOut(["rev-parse", "--abbrev-ref", "HEAD"]) ?? "?", + cmd: cmd.join(" "), + startedAt: new Date().toISOString(), + }; + const file = holderFileFor(lock); + try { + // tmp+rename so a reader never sees a half-written file. + const tmp = `${file}.${process.pid}.tmp`; + fs.writeFileSync(tmp, `${JSON.stringify(info, null, 2)}\n`); + fs.renameSync(tmp, file); + } catch { + /* informational only — never fail a run over it */ + } + return file; +} + +// Purely informational ("who am I waiting behind"): the authoritative +// acquire/release signal is the lock itself, so a missing, stale, or corrupt +// file must never break the queue. +function readHolderJson(lock) { + try { + const info = JSON.parse(fs.readFileSync(holderFileFor(lock), "utf8")); + let alive = true; + try { + process.kill(info.pid, 0); + } catch { + alive = false; + } + return { ...info, alive }; + } catch { + return null; + } +} + +function describeHolder(lock) { + const h = readHolderJson(lock); + if (!h) return "another run (details unavailable)"; + const where = h.worktree ? path.basename(h.worktree) : "?"; + const age = h.startedAt ? ` for ${humanAge(Date.parse(h.startedAt))}` : ""; + const dead = h.alive ? "" : " — pid gone, releasing"; + return `${where} (${h.branch}, pid ${h.pid})${age}${dead}`; +} + +function humanAge(startedMs) { + if (!Number.isFinite(startedMs)) return "?"; + return humanDuration(Date.now() - startedMs); +} + +function humanDuration(ms) { + const s = Math.max(0, Math.round(ms / 1000)); + return s < 60 ? `${s}s` : `${Math.floor(s / 60)}m${String(s % 60).padStart(2, "0")}s`; +} + +// -------------------------------------------------------------- the holder + +// Runs with the flock fd inherited from `flock -F`, and deliberately has no +// children of its own. +function runHolder() { + // Ctrl-C reaches the whole foreground process group at once. If this process + // died here the lock would drop while the run it is guarding is still + // shutting down and still bound to :3011/:3010 — the next waiter would wake + // up and abort on ports that are a second away from being free. The wrapper + // decides when to let go, by closing our stdin. + for (const sig of ["SIGINT", "SIGTERM", "SIGHUP"]) process.on(sig, () => {}); + process.stdout.write("ACQUIRED\n"); + process.stdin.resume(); + const bye = () => process.exit(0); + process.stdin.on("end", bye); + process.stdin.on("close", bye); + process.stdin.on("error", bye); +} + +// ------------------------------------------------------------ acquire/release + +function flockArgs(lock, timeoutMs) { + // -F/--no-fork execs directly, so the spawned process IS the holder and we + // can talk to it over its own stdio. + const args = ["-F"]; + if (timeoutMs > 0) { + args.push("-w", String(Math.ceil(timeoutMs / 1000)), "-E", String(WAIT_TIMEOUT_EXIT)); + } + args.push(lock, process.execPath, SELF, "--hold"); + return args; +} + +// Non-blocking probe, purely so the banner can name who is ahead of us. A +// false "free" here is harmless: the blocking acquire below is the real gate. +function isHeld(lock) { + try { + execFileSync("flock", ["-n", "-E", String(PROBE_HELD_EXIT), lock, "true"], { + stdio: "ignore", + }); + return false; + } catch (err) { + if (err.status === PROBE_HELD_EXIT) return true; + if (err.code === "ENOENT") throw missingFlock(); + return false; + } +} + +function missingFlock() { + return new Error( + "queue-lock: `flock` not found — install util-linux, or set E2E_QUEUE=0 to run unqueued", + ); +} + +function acquire(lock, timeoutMs) { + const holder = spawn("flock", flockArgs(lock, timeoutMs), { + stdio: ["pipe", "pipe", "inherit"], + }); + return new Promise((resolve, reject) => { + let buf = ""; + let settled = false; + holder.stdout.setEncoding("utf8"); + holder.stdout.on("data", (chunk) => { + buf += chunk; + if (!settled && buf.includes("ACQUIRED")) { + settled = true; + resolve(holder); + } + }); + holder.on("error", (err) => + reject(err.code === "ENOENT" ? missingFlock() : err), + ); + holder.on("exit", (code) => { + if (settled) { + // Expected: release() closed its stdin because our run finished. + if (holder.releasing) return; + // Unexpected: the lock is gone but our run is still going — say so + // loudly, because another run can now start on top of this one. + process.stderr.write( + `\nqueue-lock: WARNING the lock holder died (exit ${code}) mid-run\n`, + ); + return; + } + if (code === WAIT_TIMEOUT_EXIT) { + reject(Object.assign(new Error("queue-lock: timed out waiting"), { timeout: true })); + } else { + reject(new Error(`queue-lock: flock exited ${code} before acquiring`)); + } + }); + }); +} + +function release(holder) { + return new Promise((resolve) => { + holder.releasing = true; // distinguishes this from the holder crashing + if (holder.exitCode != null || holder.signalCode != null) return resolve(); + const hard = setTimeout(() => { + try { + holder.kill("SIGKILL"); + } catch { + /* already gone */ + } + }, 2000); + holder.on("exit", () => { + clearTimeout(hard); + resolve(); + }); + try { + holder.stdin.end(); + } catch { + try { + holder.kill("SIGKILL"); + } catch { + /* already gone */ + } + } + }); +} + +// ------------------------------------------------------------ port preflight + +function tryBind(port, host) { + return new Promise((resolve) => { + const server = net.createServer(); + server.once("error", (err) => resolve(err.code ?? "EUNKNOWN")); + server.listen({ port, host, exclusive: true }, () => + server.close(() => resolve(null)), + ); + }); +} + +// Only EADDRINUSE counts as occupied. A 0.0.0.0 bind fails even when the +// listener is bound to 127.0.0.1 only (SO_REUSEADDR does not permit +// overlapping listens), verified against the live ollama stub on +// 127.0.0.1:11435 and against next-server on *:3011. The `::` probe closes the +// IPv6-loopback-only hole; EAFNOSUPPORT there on a v6-less host must not abort. +async function isOccupied(port) { + for (const host of ["0.0.0.0", "::"]) { + if ((await tryBind(port, host)) === "EADDRINUSE") return true; + } + return false; +} + +// Best-effort owner lookup for the abort message. `ss` may be absent. +function culpritOf(port) { + let out; + try { + out = execFileSync("ss", ["-ltnpH", `sport = :${port}`], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }); + } catch { + return null; + } + const pid = /pid=(\d+)/.exec(out)?.[1]; + if (!pid) return null; + let cwd = "?"; + let cmd = "?"; + try { + cwd = fs.readlinkSync(`/proc/${pid}/cwd`); + } catch { + /* gone or not ours */ + } + try { + cmd = fs + .readFileSync(`/proc/${pid}/cmdline`, "utf8") + .split("\0") + .filter(Boolean) + .join(" ") + .slice(0, 140); + } catch { + /* gone or not ours */ + } + return { pid, cwd, cmd }; +} + +// Runs only AFTER the lock is held. Before that, a busy port is most likely a +// legitimately running queued suite, and aborting on it would be nonsense. +async function preflight(ports, graceMs) { + if (!ports.length) return; + const deadline = Date.now() + Math.max(0, graceMs); + for (;;) { + const busy = []; + for (const p of ports) if (await isOccupied(p.port)) busy.push(p); + if (!busy.length) return; + // The previous run's `next dev` may still be releasing the port in the + // moment we take the lock. Poll briefly so a clean handoff is a one-second + // wait rather than a spurious failure. + if (Date.now() >= deadline) { + process.stderr.write( + "\nqueue-lock: ABORTING — this suite's ports are already in use.\n" + + "We hold the queue lock, so no queued run is using them: these are\n" + + "leftovers from a killed run, or servers started by hand.\n\n", + ); + for (const p of busy) { + const c = culpritOf(p.port); + process.stderr.write( + c + ? ` ${p.envVar}=${p.port} pid ${c.pid} cwd ${c.cwd}\n ${c.cmd}\n kill ${c.pid}\n` + : ` ${p.envVar}=${p.port} (owner unknown — try: ss -ltnp | grep ${p.port})\n`, + ); + } + process.stderr.write( + "\nSet E2E_PORT_CHECK=0 to run anyway (the suite will attach to those servers).\n", + ); + process.exit(EXIT_PORTS); + } + await new Promise((r) => setTimeout(r, 250)); + } +} + +// ---------------------------------------------------------------- run + wait + +function runCommand(cmd, name) { + return new Promise((resolve) => { + const child = spawn(cmd[0], cmd.slice(1), { + stdio: "inherit", + env: { ...process.env, [HELD_ENV]: name }, + }); + installSignalHandlers(() => child); + child.on("error", (err) => { + process.stderr.write(`queue-lock: ${err.message}\n`); + resolve(1); + }); + child.on("exit", (code, signal) => { + // Never re-raise onto ourselves the way worktree.mjs's cmdRun does: we + // have handlers installed, so re-raising would loop. + resolve(signal ? 128 + (os.constants.signals[signal] ?? 0) : (code ?? 1)); + }); + }); +} + +let signalHits = 0; +function installSignalHandlers(getChild) { + for (const sig of ["SIGINT", "SIGTERM", "SIGHUP"]) { + process.on(sig, () => { + signalHits++; + const child = getChild(); + if (signalHits === 1) { + // The terminal already delivered this to the whole foreground process + // group, child included. We deliberately do not forward it: a second + // SIGINT is precisely how playwright skips globalTeardown, which is + // how this repo gets orphaned fixtures. We also stay alive, so the + // lock is not released while that tree is still shutting down. + process.stderr.write( + `\nqueue-lock: ${sig} — waiting for the run to stop (again to force)\n`, + ); + return; + } + process.stderr.write("queue-lock: forcing shutdown\n"); + try { + child?.kill("SIGKILL"); + } catch { + /* already gone */ + } + }); + } +} + +function startWaitBanner(lock) { + const t0 = Date.now(); + process.stderr.write( + `queue-lock: waiting for the e2e queue — held by ${describeHolder(lock)}\n` + + " (one e2e run at a time, machine-wide; E2E_QUEUE=0 to bypass)\n", + ); + const timer = setInterval(() => { + process.stderr.write( + `queue-lock: still waiting (${humanDuration(Date.now() - t0)})\n`, + ); + }, TICK_MS); + timer.unref?.(); + return () => { + clearInterval(timer); + process.stderr.write( + `queue-lock: acquired after ${humanDuration(Date.now() - t0)}\n`, + ); + }; +} + +// ------------------------------------------------------------- the entry point + +/** + * Run `fn` with the global queue lock held, after checking `ports` are free. + * Used both by the CLI below and directly by scripts/run-sharded-e2e.mjs. + */ +export async function withQueue(opts, fn) { + const name = opts.name ?? "e2e"; + const ports = parsePorts(opts.portSpec ?? null); + const graceMs = opts.portGraceMs ?? Number(process.env.E2E_PORT_GRACE_MS ?? DEFAULT_PORT_GRACE_MS); + const timeoutMs = opts.timeoutMs ?? defaultTimeoutMs(); + + // "Don't queue" never means "don't check the ports": the preflight is what + // turns a silent cross-worktree data wipe into a loud abort. + if (process.env.E2E_QUEUE === "0" || process.env[HELD_ENV] === name) { + await preflight(ports, graceMs); + return fn(); + } + + const lock = lockFileFor(name); + let stopBanner = null; + if (isHeld(lock)) stopBanner = startWaitBanner(lock); + + let holder; + try { + holder = await acquire(lock, timeoutMs); + } catch (err) { + if (err.timeout) { + process.stderr.write( + `\nqueue-lock: gave up after ${humanDuration(timeoutMs)} waiting for ${lock}\n` + + " Raise or unset E2E_QUEUE_TIMEOUT, or set E2E_QUEUE=0 to bypass the queue.\n", + ); + process.exit(EXIT_TIMEOUT); + } + throw err; + } + stopBanner?.(); + + const holderFile = writeHolderJson(lock, name, opts.cmd ?? [name]); + const cleanup = () => { + try { + fs.rmSync(holderFile, { force: true }); + } catch { + /* best effort */ + } + try { + holder.kill("SIGKILL"); + } catch { + /* already gone */ + } + }; + process.on("exit", cleanup); + + try { + await preflight(ports, graceMs); + return await fn(); + } finally { + try { + fs.rmSync(holderFile, { force: true }); + } catch { + /* best effort */ + } + await release(holder); + } +} + +async function main() { + const { opts, cmd } = parseArgv(process.argv.slice(2)); + if (opts.hold) return runHolder(); + if (cmd.length === 0) { + process.stderr.write( + "usage: queue-lock.mjs [--name e2e] [--ports PORT:3011,...] -- <cmd...>\n", + ); + process.exit(2); + } + const code = await withQueue({ ...opts, cmd }, () => runCommand(cmd, opts.name)); + process.exit(code); +} + +// Guarded so `import { withQueue }` has no side effects. +if (process.argv[1] && fs.realpathSync(process.argv[1]) === fs.realpathSync(SELF)) { + main().catch((err) => { + process.stderr.write(`${err?.message ?? err}\n`); + process.exit(1); + }); +} diff --git a/scripts/queue-lock.test.mjs b/scripts/queue-lock.test.mjs @@ -0,0 +1,244 @@ +// Tests for the global e2e queue (scripts/queue-lock.mjs). +// +// Every test drives the real CLI against a throwaway lock file via +// E2E_QUEUE_LOCK_FILE, so none of them can touch the actual .git/e2e-queue.lock +// or any real port. Run with: pnpm test:scripts +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import fs from "node:fs"; +import net from "node:net"; +import os from "node:os"; +import path from "node:path"; +import test from "node:test"; +import { fileURLToPath } from "node:url"; + +const SCRIPT = fileURLToPath(new URL("./queue-lock.mjs", import.meta.url)); + +function tmpDir() { + return fs.mkdtempSync(path.join(os.tmpdir(), "queue-lock-test-")); +} + +// Run the wrapper to completion, capturing output. `env` is merged over the +// current environment; E2E_PORT_CHECK defaults off so tests that are not about +// the preflight never probe a port. +function runLock(args, env = {}) { + return new Promise((resolve) => { + const child = spawn(process.execPath, [SCRIPT, ...args], { + env: { E2E_PORT_CHECK: "0", ...process.env, ...env }, + stdio: ["ignore", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (d) => (stdout += d)); + child.stderr.on("data", (d) => (stderr += d)); + child.on("exit", (code) => resolve({ code, stdout, stderr })); + }); +} + +// A command that records "S<id>" when it starts and "E<id>" when it ends, so +// overlapping runs are visible as interleaving in the log. +function markerCmd(logFile, id, holdMs) { + return [ + process.execPath, + "-e", + `const fs=require("fs");` + + `fs.appendFileSync(${JSON.stringify(logFile)},"S${id}");` + + `setTimeout(()=>fs.appendFileSync(${JSON.stringify(logFile)},"E${id}"),${holdMs});`, + ]; +} + +const delay = (ms) => new Promise((r) => setTimeout(r, ms)); + +test("serializes concurrent invocations", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const log = path.join(dir, "order.log"); + const env = { E2E_QUEUE_LOCK_FILE: lock }; + + const runs = ["1", "2", "3"].map((id) => + runLock(["--", ...markerCmd(log, id, 250)], env), + ); + for (const r of await Promise.all(runs)) assert.equal(r.code, 0); + + const order = fs.readFileSync(log, "utf8"); + // Each run must fully close before the next opens: S1E1S2E2... in some + // permutation of ids, with no S following an unmatched S. + assert.match(order, /^(S(\d)E\2){3}$/, `interleaved: ${order}`); +}); + +test("serves waiters in arrival order (FIFO)", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const log = path.join(dir, "fifo.log"); + const env = { E2E_QUEUE_LOCK_FILE: lock }; + + const runs = []; + for (const id of ["1", "2", "3"]) { + runs.push(runLock(["--", ...markerCmd(log, id, 300)], env)); + await delay(150); // stagger arrivals so the intended order is unambiguous + } + await Promise.all(runs); + + assert.equal(fs.readFileSync(log, "utf8"), "S1E1S2E2S3E3"); +}); + +test("prints a banner naming the holder while waiting", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const env = { E2E_QUEUE_LOCK_FILE: lock }; + + const first = runLock(["--", process.execPath, "-e", "setTimeout(()=>{},600)"], env); + await delay(200); + const second = await runLock(["--", process.execPath, "-e", "0"], env); + await first; + + assert.equal(second.code, 0); + assert.match(second.stderr, /waiting for the e2e queue/); + assert.match(second.stderr, /pid \d+/); // holder.json was read back + assert.match(second.stderr, /acquired after/); +}); + +test("QUEUE_LOCK_HELD passes straight through without deadlocking", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const env = { E2E_QUEUE_LOCK_FILE: lock }; + + // Hold the lock for a good while, then prove a nested invocation (the shape + // of root `pnpm e2e` -> `pnpm --filter editor run e2e`) does not wait for it. + const held = runLock(["--", process.execPath, "-e", "setTimeout(()=>{},1500)"], env); + await delay(300); + + const t0 = Date.now(); + const nested = await runLock(["--", process.execPath, "-e", "0"], { + ...env, + QUEUE_LOCK_HELD: "e2e", + }); + const elapsed = Date.now() - t0; + + assert.equal(nested.code, 0); + assert.ok(elapsed < 1000, `nested run waited ${elapsed}ms — it deadlocked on its parent's lock`); + assert.doesNotMatch(nested.stderr, /waiting for the e2e queue/); + await held; +}); + +test("E2E_QUEUE=0 bypasses the queue entirely", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const env = { E2E_QUEUE_LOCK_FILE: lock }; + + const held = runLock(["--", process.execPath, "-e", "setTimeout(()=>{},1500)"], env); + await delay(300); + + const t0 = Date.now(); + const bypass = await runLock(["--", process.execPath, "-e", "0"], { ...env, E2E_QUEUE: "0" }); + assert.equal(bypass.code, 0); + assert.ok(Date.now() - t0 < 1000, "E2E_QUEUE=0 still waited for the lock"); + await held; +}); + +test("a SIGKILLed run releases the lock immediately", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const env = { E2E_PORT_CHECK: "0", ...process.env, E2E_QUEUE_LOCK_FILE: lock }; + + const victim = spawn( + process.execPath, + [SCRIPT, "--", process.execPath, "-e", "setTimeout(()=>{},30000)"], + { env, stdio: "ignore" }, + ); + await delay(500); // let it acquire + victim.kill("SIGKILL"); + await new Promise((r) => victim.on("exit", r)); + + // No stale-lock recovery code exists, by design: the kernel drops the lock + // when the holder's fd closes. If that ever stops being true, this hangs. + const t0 = Date.now(); + const next = await runLock(["--", process.execPath, "-e", "0"], { E2E_QUEUE_LOCK_FILE: lock }); + assert.equal(next.code, 0); + assert.ok(Date.now() - t0 < 5000, "lock survived a SIGKILLed holder"); +}); + +test("a leaked shell-spawned stray does not keep the lock", async (t) => { + // The regression test for why the lock is held by a dedicated child rather + // than by wrapping the command in `flock` directly: flock(1) does not set + // FD_CLOEXEC on its lock fd, so a stray started through a shell — which is + // exactly how playwright's webServer launches `pnpm dev:test` — inherits it + // and would hold the queue until that stray dies. This suite leaks such + // strays for real (see editor/e2e/fixtureProcs.ts). + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const pidFile = path.join(dir, "stray.pid"); + + await runLock( + ["--", "sh", "-c", `setsid sleep 30 >/dev/null 2>&1 & echo $! > ${pidFile}; sleep 0.2`], + { E2E_QUEUE_LOCK_FILE: lock }, + ); + + const strayPid = Number(fs.readFileSync(pidFile, "utf8").trim()); + t.after(() => { + try { + process.kill(strayPid, "SIGKILL"); + } catch { + /* already gone */ + } + }); + assert.doesNotThrow(() => process.kill(strayPid, 0), "stray should still be running"); + + const t0 = Date.now(); + const next = await runLock(["--", process.execPath, "-e", "0"], { E2E_QUEUE_LOCK_FILE: lock }); + assert.equal(next.code, 0); + assert.ok(Date.now() - t0 < 5000, "a leaked stray held the queue lock"); +}); + +test("the port preflight aborts non-zero, naming the port and its env var", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + + const server = net.createServer(); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + const port = server.address().port; + + const result = await runLock( + ["--ports", `TEST_PORT:${port}`, "--port-grace", "0", "--", process.execPath, "-e", "0"], + { E2E_QUEUE_LOCK_FILE: lock, E2E_PORT_CHECK: "1" }, + ); + await new Promise((r) => server.close(r)); + + assert.notEqual(result.code, 0, "a bound port must abort the run"); + assert.match(result.stderr, new RegExp(`TEST_PORT=${port}`)); + assert.match(result.stderr, /E2E_PORT_CHECK=0/); // the documented override +}); + +test("the port preflight passes when the port is free", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + + // Bind and release, so we know the port was free at the moment we checked. + const server = net.createServer(); + await new Promise((r) => server.listen(0, "127.0.0.1", r)); + const port = server.address().port; + await new Promise((r) => server.close(r)); + + const result = await runLock( + ["--ports", `TEST_PORT:${port}`, "--", process.execPath, "-e", 'console.log("ran")'], + { E2E_QUEUE_LOCK_FILE: lock, E2E_PORT_CHECK: "1" }, + ); + assert.equal(result.code, 0); + assert.match(result.stdout, /ran/); +}); + +test("propagates the command's exit code", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + const result = await runLock(["--", process.execPath, "-e", "process.exit(42)"], { + E2E_QUEUE_LOCK_FILE: lock, + }); + assert.equal(result.code, 42); +}); + +test("removes its holder.json when the run finishes", async () => { + const dir = tmpDir(); + const lock = path.join(dir, "q.lock"); + await runLock(["--", process.execPath, "-e", "0"], { E2E_QUEUE_LOCK_FILE: lock }); + assert.equal(fs.existsSync(`${lock}.holder.json`), false); +}); diff --git a/scripts/run-sharded-e2e.mjs b/scripts/run-sharded-e2e.mjs @@ -4,6 +4,7 @@ import os from "node:os"; import path from "node:path"; import { mkdirSync } from "node:fs"; import { fileURLToPath } from "node:url"; +import { withQueue } from "./queue-lock.mjs"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const REPO_ROOT = path.resolve(__dirname, ".."); @@ -13,7 +14,10 @@ const IMAGE = process.env.IMAGE ?? "yt-dlp-transcript-browser-e2e"; // Flags this script consumes itself; everything else on the command line is // forwarded verbatim to each shard's `playwright test` (see parseArgs). +// OWN_FLAGS take a value (and so skip the next argv entry); OWN_BOOL_FLAGS do +// not — putting a boolean in the first set would silently eat the flag after it. const OWN_FLAGS = new Set(["--shards", "--retries"]); +const OWN_BOOL_FLAGS = new Set(["--no-build"]); function parseIntArg(flag, envVar, fallback, min) { const argIdx = process.argv.indexOf(flag); @@ -61,6 +65,7 @@ function parsePassthrough() { i++; // skip the flag's value too continue; } + if (OWN_BOOL_FLAGS.has(argv[i])) continue; // no value to skip rest.push(argv[i]); } return rest; @@ -69,6 +74,8 @@ function parsePassthrough() { const SHARDS = parseShards(); const RETRIES = parseRetries(); const PASSTHROUGH = parsePassthrough(); +const SKIP_BUILD = + process.argv.includes("--no-build") || process.env.SKIP_BUILD === "1"; function run(cmd, args, opts = {}) { return new Promise((resolve, reject) => { @@ -148,7 +155,25 @@ async function cleanReportDir() { mkdirSync(REPORT_DIR, { recursive: true }); } -async function main() { +// The image tag is fixed, so two worktrees building at once would leave the tag +// pointing at whichever finished last and the shards would run *another +// checkout's* code. `COPY . .` also reads test-transcripts/ and +// test-settings.json, which a concurrent host run rewrites on every resetData(). +// Both are why the build lives inside the queue lock now instead of in a +// `docker build && …` shell chain in package.json. A no-change rebuild is +// layer-cached and costs seconds; --no-build skips it while iterating. +async function buildImage() { + console.log(`Building ${IMAGE} from Dockerfile.test ...`); + const code = await run("docker", ["build", "-f", "Dockerfile.test", "-t", IMAGE, "."], { + cwd: REPO_ROOT, + }); + if (code !== 0) { + console.error(`docker build exited ${code}`); + process.exit(code); + } +} + +async function runShards() { console.log(`Running ${SHARDS} shard(s) using image ${IMAGE}`); console.log(`Retries per shard: ${RETRIES}`); if (PASSTHROUGH.length) { @@ -156,6 +181,7 @@ async function main() { } const t0 = Date.now(); + if (!SKIP_BUILD) await buildImage(); await cleanReportDir(); const shardPromises = []; @@ -202,8 +228,18 @@ async function main() { console.log("(shard output did not contain a list-reporter summary; see the HTML report)"); } - const failed = results.some(({ code }) => code !== 0); - process.exit(failed ? 1 : 0); + return results.some(({ code }) => code !== 0) ? 1 : 0; +} + +// One e2e run at a time, machine-wide — N containers each running `next start` +// would starve a concurrent host suite into flakes. No port preflight: the +// shards have their own network namespaces and bind no host ports, so checking +// 3011/3010 here would abort on an unrelated dev server for no reason. +// Returning (rather than exiting) from runShards lets the lock be released +// normally. See scripts/queue-lock.mjs. +async function main() { + const code = await withQueue({ name: "e2e", cmd: ["e2e:sharded"] }, runShards); + process.exit(code); } main().catch((err) => { diff --git a/scripts/worktree.mjs b/scripts/worktree.mjs @@ -18,6 +18,7 @@ const PORT_BASES = { EXPORT_PORT: 3010, // export server launched by editor e2e EXPORT_DEV_PORT: 3000, // export real dev EXPORT_E2E_PORT: 3020, // export's own Playwright suite + OLLAMA_STUB_PORT: 11435, // digest-lane stub server in the editor e2e suite HOMEPAGE_DEV_PORT: 3030, // homepage (hub) real dev HOMEPAGE_PORT: 3031, // homepage static `serve out` (start:homepage) HOMEPAGE_E2E_PORT: 3040, // homepage's own Playwright suite