Make E2E test server port and cleanup safe for parallel runs by Tim020 · Pull Request #1420 · dreamteamprod/DigiScript

Tim020 · 2026-09-22T22:15:21Z

Problem

Two E2E suites running at once (e.g. separate git worktrees driven by parallel CI/agent jobs) could not coexist:

SERVER_PORT was hardcoded to 8888 in three places (playwright.config.ts, e2e/global-setup.ts, e2e/helpers.ts).
global-setup.ts's stale-port cleanup SIGKILLed whatever was listening on the port (via lsof) — a second run would destroy the first run's backend mid-suite.
PID/tempdir pointers lived in fixed filenames in os.tmpdir(), so a second run overwrote the first run's pointers, causing its teardown to kill the wrong server and delete the wrong database.
Only db_path/log_path were isolated into the per-run temp dir; compiled_script_path, draft_script_path, client_log_path and db_log_path defaulted into server/conf/ of the checkout.

Fix

All changes are in client-v3/:

New e2e/env.ts is the single source for SERVER_PORT/BASE_URL/RUN_DIR/STATE_FILE, imported by playwright.config.ts, global-setup.ts, global-teardown.ts, helpers.ts and db-snapshot.ts.
- Port resolution: E2E_PORT env var if set, else a port deterministically derived from process.cwd() (range 20000–39999). Each checkout gets its own stable port with no cross-process coordination needed, and E2E_PORT remains available to pin a specific port (e.g. to match CI locally).
global-setup.ts no longer kills whatever holds the port. It only reaps a process it can positively identify as its own leftover, by matching ps output against main.py ... --port=<port> directly — not solely via the PID recorded in state.json (a real race was found empirically between this and db-snapshot.ts's retry-restart hook, see below). If the port is genuinely held by something else, setup fails fast with a message pointing at E2E_PORT instead of touching it.
State moves from fixed tmpdir files to a per-port state.json under os.tmpdir()/digiscript-e2e-<port>/, so two runs can never clobber each other's pointers.
Full path isolation: the generated settings JSON now also isolates compiled_script_path, draft_script_path, client_log_path and db_log_path into the run's temp dir.
Server stdout/stderr now goes to a log file that's surfaced on a setup failure, instead of being discarded (stdio: 'ignore').

Verification

Concurrency proof (the real test): ran the full suite from two separate directories simultaneously — 221/221 passed in each, confirmed via lsof that two distinct derived ports were live at the same time, and a third, unrelated server already running on the old fixed port 8888 (from an unrelated parallel job) was left completely undisturbed throughout.
Foreign-listener safety: started a real listener on the derived port, confirmed setup fails fast with the intended message and leaves that process alone. (Caught and fixed a real bug in the process — the port-free probe bound 127.0.0.1 specifically, while Tornado's app.listen(port) binds all interfaces, which could give a false "port free" result on some stacks.)
Stale-recovery, both failure shapes:
- kill -9 of the Playwright runner mid-suite, leaving state.json + a live server → reaped cleanly on next run.
- A live leftover server with no state.json at all (the retry/teardown race mentioned above) → also reaped cleanly on next run, via the process-identity match.
npm run ci-lint / npm run typecheck clean.
Confirmed no files are written under server/conf/ by the E2E harness.

Design + verification detail: plans/parallel-e2e-isolation.md (gitignored, not included in this PR).

🤖 Generated with Claude Code

Two E2E suites running at once (e.g. separate worktrees driven by parallel agent jobs) could not coexist: - SERVER_PORT was hardcoded to 8888 in three places. - global-setup.ts's stale-port cleanup SIGKILLed whatever was listening on the port via lsof, destroying a second run's backend. - PID/tempdir pointers lived in fixed files in os.tmpdir(), so a second run overwrote the first run's pointers. - Only db_path/log_path were isolated per run; compiled/draft script paths and the two remaining log paths defaulted into server/conf/. Fix, all in client-v3/: - New e2e/env.ts is the single source for SERVER_PORT/BASE_URL/ RUN_DIR/STATE_FILE, imported by config, setup, teardown, helpers, and db-snapshot. Port resolution: E2E_PORT env var if set, else a port deterministically derived from process.cwd() (20000-39999), so each checkout gets its own stable port with no cross-process coordination needed. - global-setup.ts no longer kills whatever holds the port. It only reaps a process it can positively identify as its own leftover, by matching `ps` output against `main.py ... --port=<port>` directly (not solely via the PID recorded in state.json, which a crashed run's teardown can delete without having killed the live process - found via a real race against db-snapshot.ts's retry-restart hook). If the port is genuinely held by something else, setup fails fast with a message pointing at E2E_PORT instead of touching it. - State (PID + tempDir) moves from fixed tmpdir files to a per-port state.json under os.tmpdir()/digiscript-e2e-<port>/, so two runs can never clobber each other's pointers. - The generated settings JSON now isolates compiled_script_path, draft_script_path, client_log_path and db_log_path into the run's tempDir too, alongside the existing db_path/log_path. - Server stdout/stderr now goes to a log file surfaced on setup failure, instead of being discarded. Verified: two full suites run concurrently from separate directories, 221/221 passed each, confirmed via lsof that distinct ports were live simultaneously, and an unrelated server already on the old fixed port 8888 was left undisturbed throughout. Also verified: a genuine foreign listener on the derived port is refused (not killed); a stale server left behind by a killed runner is reaped, both with and without a matching state.json. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

github-actions · 2026-09-22T22:16:04Z