Flaky pkg/server tests: port reused between test servers, requests can hit the wrong instance

Start with pkg/server/http_server_test.go, especially freeTCPPort, waitForServer, and startServeHTTPWithSession, then trace the ServeHTTP entry point and run the named flaky tests. The work is done when each test keeps its listener associated with its server instance, requests cannot reach a leftover server, and the affected pkg/server tests pass repeatedly.

pkg/server tests fail intermittently on unrelated changes. The clearest case is TestRunQuery_DTQL_RunsForAdmin, which failed and then passed on the same commit with no change.

Evidence

Run	Ref	Failed test
34350984261	main @ 337eaf1a	TestRunQuery_DTQL_RunsForAdmin
34356039231	PR #208 @ 33f0b58b	TestRunQuery_DTQL_RunsForAdmin
34354476734	main @ 4327903c	TestServeHTTP_CORS_DatatugApp, TestServeHTTP_CreateProject_AuthGate

PR #208 changed only .gitignore. Re-running the identical commit turned it green with no edit, which is what makes this a flake rather than a real failure. main was green two minutes before that run, on the same base.

Failure output:

validation error: invalid request: bad value for field [project]: missing required field
--- FAIL: TestRunQuery_DTQL_RunsForAdmin (5.04s)

That error is the interesting part. The test does set ProjectID, so the server that answered did not recognise it.

Probable mechanism

This is a hypothesis, but it explains the exact error rather than just "flaky".

freeTCPPort in pkg/server/http_server_test.go:84 picks a port by binding 127.0.0.1:0, reading the assigned port, then closing the listener and returning the bare number:

l, err := net.Listen("tcp", "127.0.0.1:0")
port := l.Addr().(*net.TCPAddr).Port
if err := l.Close(); err != nil { ... }
return port

startServeHTTPWithSession (pkg/server/http_server_test.go:143) then starts the real server on that number. Between the close and the bind there is a window where the port is unowned.

The window matters more than usual here because every such test shuts its server down gracefully in t.Cleanup, and graceful shutdown only completes once tracked connections drain. So a previous test's server can still be listening on a port the OS has already handed back out.

waitForServer (line 117) cannot tell the difference. It polls /datatug/ping and accepts any 200 pong. A leftover server from an earlier test answers that ping happily. The test then posts /datatug/exec/run_query to a server wired with a different project set, which rejects the unknown project exactly as observed.

That also fits the other two failures: they are different tests in the same package using the same helper, failing the same way for the same reason.

Suggested fix

Never let go of the port. Bind once and hand the listener to the server, rather than passing a port number and rebinding:

l, _ := net.Listen("tcp", "127.0.0.1:0")
go srv.Serve(l)          // or ServeHTTP taking a net.Listener

That closes the window entirely and needs no retry logic.

If ServeHTTP's signature should stay port-based for production, a test-only entry point taking a net.Listener would do. A cheaper partial mitigation is to make waitForServer assert an identity unique to this server instance instead of accepting any pong, which would turn a silent wrong-server hit into a clear failure. That detects the collision rather than preventing it, so it is worth doing in addition, not instead.

Why it is worth fixing now

main is protected and requires strongo_workflow / Build & test, so a flake here blocks merges until someone notices and re-runs. Two of the three failures above were on main itself.

---

Filed by Claude Code at the maintainer's request, from evidence gathered while landing unrelated CI changes. I have not attempted the fix.