Repository navigation
test: 50 test files bind hardcoded ports (3000-3251) — one taken port reddens the suite, and 4 ports collide between files #440
Description
Symptom
The suite cannot be green on a machine that happens to use one of the test ports. On this box it
is the only failing suite in an otherwise clean run (7301 passed, 15 skipped):
FAIL test/quick-start.test.ts > Session Management
Error: listen EADDRINUSE: address already in use 127.0.0.1:3100
The port is not this repo's: an unrelated container of mine publishes 3100. Nothing about the
code under test is broken — the test file just cannot bind.
Scope (measured on master)
50 hardcoded port bindings across the test tree, range 3000–3251:
test/quick-start.test.tsusesconst TEST_PORT = 3099and then a hardcoded ladder
(3099,+1,+2,+4) — one occupied port and the whole file is red.- 4 ports are used by two files each, which is a genuine race under vitest's parallel workers
rather than a machine-specific nuisance:3162—test/qr-auth.test.ts,test/auth-security.test.ts3170—test/multiuser-auth.test.ts,test/routes/ws-routes.test.ts3230and3231—test/cod54-hook-event-auth.test.ts,test/routes/voice-routes.test.ts
test/docker-self-update.test.tsuses3000, which is Codeman's own default web port — so
running the suite on a machine where Codeman runs on its default port fails too.
Why it matters
- An environment-dependent red suite masks real regressions. A contributor with any service in
that range has to learn to ignore the failure, which is exactly the habit that lets a real one
through. - The duplicate ports are flaky, not deterministic — they pass or fail depending on worker
scheduling, which is worse than failing always. - It is invisible on a clean CI box, so it is never fixed there.
Precedent in this repo
The correct pattern already exists twice: test/webview-egress.test.ts and
test/webview-proxy-timeout.test.ts both bind an ephemeral port (server.listen(0)).
Proposed fix
Two options; the second removes the race instead of narrowing it:
- Test-side helper (no
src/change):findFreePort()— bind0, read
address().port, close, hand the number to the test. Small TOCTOU window between close and
rebind, but it is what most suites do. - Expose the actually bound port (preferred):
WebServerstoresprivate portand calls
this.app.listen({ port: this.port, host: this.host }), soport: 0currently works at the
socket level but leavesthis.port === 0and the test with no way to know where to connect. A
small accessor readingthis.app.server.address()?.port(plus using it in the startup log line)
makesnew WebServer(0, …)usable directly — no port arithmetic, no collisions, no race.
Either way the 50 sites are a mechanical change; a shared helper in test/ would keep the next
test file from reintroducing the pattern.
Offer
Happy to send a PR for this (either option, plus the mechanical sweep) — say which you prefer. It
is independent of #433 and can go in first.
Activity
Ark0N commentedon Sep 21, 2026
Option 2, please, and yes to the sweep. This is the same 3100 I just told you to ignore on #443, so I would rather it stopped existing.
Option 1 keeps the TOCTOU window and, more importantly, leaves your four duplicate pairs as a scheduling accident that happens to pass most days. Option 2 removes the class.
It is a bit more than an accessor, though. Three things, in this order:
1. Write the bound port back into
this.portafter listen.WebServerusesthis.portfor more than the socket: the startup log line (server.ts:2720),process.env.CODEMAN_API_URL(2789), the docker bridge listener (3700) and the unauthenticated-bind warnings all read it. Assign the real number fromthis.app.server.address()right afterawait this.app.listen(...)at 2713 and every one of those becomes correct for an ephemeral bind instead of printing:0. Guard the string-address and null cases rather than casting.2.
createRouteContext()snapshots the port by value, and it runs before listen.port: this.portatserver.ts:731, built fromsetupRoutes()whichstart()calls at 2653, whilelisten()is at 2713. So withport: 0the one consumer,ctx.tunnelManager.start(ctx.port, ctx.https)insystem-routes.ts:1067, would be handed a 0. Make it a getter on the context object. Nothing in the test sweep would ever catch that, which is exactly why it needs fixing in the same commit.3. Expose it properly for tests. A readonly
get boundPort(), not tests reaching into a private field.How I want it split
Two PRs.
PR 1: the three server changes above, plus the conversions that actually buy something today: the four colliding pairs (
3162,3170,3230/3231) anddocker-self-update.test.tsoff3000. Small enough to review as a pattern, and it kills every real flake immediately.PR 2: the mechanical sweep of the rest. If the accessor shape turns out wrong in review, I would rather redo five files than fifty.
One thing to add
A static guard test, so this cannot come back. The repo pins this class of rule that way already (
test/frontend-cli-no-id-branching.test.ts,test/mobile-header-buttons-policy.test.ts): a test that reads the test tree and fails on anew WebServer(<numeric literal>)outside an allowlist. Without it, the next test file copies the nearest neighbour and we are back here in six months.Out of scope, deliberately
Some of those 41 files spin up a real server for what
app.inject()already does without a port at all (test/routes/_route-test-utils.ts). That is a worthwhile cleanup and it is not this PR. Don't fold it in.3000being in the list at all is the one that bothers me most, so thanks for catching it. Anybody running Codeman on its default port while running the suite has been getting a red file for reasons that have nothing to do with their change.Ark0N commentedon Oct 9, 2026
Part 1 shipped in 1.40.0 with #556 (thanks @Randalix):
new WebServer(0, ...)now reports the port it really bound throughboundPort, the files that collided bind ephemeral ports, andtest/test-ports-guard.test.tsfails any new test that builds a WebServer on a fixed port. It already caught three during the release. Leaving this open for part 2: 43 files are still on the guard's shrink-only legacy list, plus the mobile suite, whose helper caches servers by port.test: build every in-process WebServer on port 0 (#440)
test(mobile): createTestServer() binds an ephemeral port (#440)
test: raw test servers listen on port 0 (#440)
test(guard): no legacy list left; flag raw listeners on a fixed port (#…
Merge pull request #570: test: bind every test server to an ephemeral…
Ark0N commentedon Oct 10, 2026
Closing: finished in 1.41.0. Part 1 (#556) let the server report the port it really bound, and part 2 (#570, thanks @Randalix) moved every in-process test server onto an ephemeral port, the mobile suite included, with the guard now also refusing raw listeners on a fixed port. The one fixed port left is codex-predictive-echo's separate lab server process on 3222, which the guard can't see.