Fix stale HTTP connection events and test port ownership

build.zig.zon

@@ -4,28 +4,19 @@
     .fingerprint = 0x306d84a90c011454,
     .minimum_zig_version = "0.16.0",
     .dependencies = .{
-        // FORK, not upstream — `valthon/zap` at upstream v0.10.6 (f6099ece) plus ONE
-        // commit: a two-line fix to the bundled facil.io (issue #380).
-        //
-        // `fio_poll`'s review loop force-closes on any error event, deriving the uuid
-        // from the raw descriptor in the event via `fd2uuid(fd)`. A poll/epoll result
-        // set is a SNAPSHOT: by the time the loop reaches an entry, another thread may
-        // have closed that descriptor and the OS may have reissued the number to an
-        // unrelated socket opened elsewhere in the process. `fd2uuid` re-reads the
-        // descriptor's CURRENT counter, so `uuid_is_valid()` can never reject a stale
-        // event — facil.io then closes a descriptor it does not own. Surfaced here as
-        // an intermittent EBADF panic in a queue worker whose SMTP socket was closed
-        // between `connect()` and `getsockname()`; it can take out ANY descriptor,
-        // including SQLite's. The fix gates the close on `.open`, the flag facil.io's
-        // own `fio_is_closed()` already consults.
-        //
-        // Upstream (including v0.11.0) still carries the unguarded call, so bumping
-        // the pin does NOT fix this. When re-pinning zap, rebase this commit onto the
-        // new upstream tag rather than dropping back to a plain zigzap/zap url — and
-        // keep `zap_version` / `facil_version` in build.zig in sync with the base tag.
+        // Fork of upstream v0.10.6 (f6099ece), retaining issue #380's protection
+        // for descriptors outside facil.io and the connection-generation fix:
+        // https://github.com/valthon/zap/pull/1
+        // Poll snapshots and deferred callbacks retain the registered connection
+        // identity. Validation under the connection lock prevents an old event
+        // from closing, flushing or invoking callbacks on a recycled descriptor.
+        // Flush and close revalidate identity inside their respective locks. This fixes
+        // connection resets and truncated HTTP responses during browser startup.
+        // Preserve both fixes when upgrading; keep build.zig's upstream versions
+        // in sync with the fork's base tag.
         .zap = .{
-            .url = "git+https://github.com/valthon/zap#1d7cb3b8cf288f9f78d767d19159b491d208a051",
-            .hash = "zap-0.10.6-GoeB80SLJACZeWlVU-LnGsweCgPJdMlYVkRsWNoOlzvi",
+            .url = "git+https://github.com/valthon/zap#2bfc918ff1e07ced69fb5535c19b56251c9fcea0",
+            .hash = "zap-0.10.6-GoeB8_TjJABuc_dEhxCk4cDo7Bwu3-3-3BeY6b5jaaVg",
         },
     },
     .paths = .{

changelog.d/http-connection-generation.md

@@ -0,0 +1,3 @@
+### Fixes
+
+- Prevent stale HTTP connection events from closing a replacement connection after the operating system reuses its descriptor, avoiding connection resets and truncated pages during browser navigation.

tests/admin/conftest.py

@@ -1,5 +1,6 @@
 import os, re, socket, subprocess, tempfile, time, shutil, pathlib, pytest
 import json, urllib.request, urllib.error
+from contextlib import ExitStack, contextmanager
 from _bin import resolve_binary
 
 REPO = pathlib.Path(__file__).resolve().parents[2]
@@ -29,8 +30,41 @@
 # so an inherited ZIGBASE_SERVE_BACKGROUND=1 would be a mistake to honor.
 os.environ["ZIGBASE_SERVE_BACKGROUND"] = "0"
 
-def _free_port():
-    s = socket.socket(); s.bind(("127.0.0.1", 0)); p = s.getsockname()[1]; s.close(); return p
+@contextmanager
+def _reserve_port():
+    """Hold a non-listening loopback reservation while a REUSEADDR HTTP child binds.
+
+    An explicit context owns the socket through successful startup; fixtures
+    may retain it until server cleanup. Other workers' port-zero allocations
+    cannot select it during the spawn/bind gap.
+    facil.io sets SO_REUSEADDR too, so its listener can bind while this socket
+    remains open. This helper is for ZigBase HTTP servers, not SMTP/mock servers.
+    """
+    with socket.socket() as reservation:
+        reservation.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+        reservation.bind(("127.0.0.1", 0))
+        yield reservation.getsockname()[1]
+
+def _server_diagnostics(node, proc, log_path):
+    try:
+        output = pathlib.Path(log_path).read_text(errors="replace")
+    except OSError as error:
+        output = f"<server log unavailable: {error}>"
+    return f"\nServer diagnostics for {node.nodeid}: pid={proc.pid}, exit_before_cleanup={proc.poll()!r}\n{log_path}:\n{output}"
+
+
+@pytest.hookimpl(hookwrapper=True)
+def pytest_runtest_makereport(item, call):
+    outcome = yield
+    report = outcome.get_result()
+    setattr(item, "rep_" + report.when, report)
+    if report.when == "setup" and report.failed:
+        context = getattr(item, "_server_diagnostic_context", None)
+        if context is not None:
+            # A passing teardown's captured output is hidden after setup errors,
+            # including under xdist. Attach live evidence before report delivery.
+            report.sections.append(("Server diagnostics", _server_diagnostics(item, *context)))
+
 
 @pytest.fixture(scope="session")
 def binary():
@@ -66,33 +100,45 @@ def _cleanup_su_templates():
 
 @pytest.fixture()
 def server(binary, request):
-    data = tempfile.mkdtemp(prefix="zb_admin_")
-    shutil.copytree(_su_template_for(binary), data, dirs_exist_ok=True)
-    port = _free_port()
-    extra_env = getattr(request, "param", None) or {}
-    env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
-           # The harness owns this process as a FOREGROUND child (terminate() in the
-           # finally). Without the pin, running the suite from inside an AI-agent
-           # session (CLAUDECODE etc.) would trigger serve's auto-backgrounding: the
-           # parent exits after readiness, terminate() hits the dead parent, and the
-           # detached child leaks past the test.
-           "ZIGBASE_SERVE_BACKGROUND": "0", **extra_env}
-    # Plain-HTTP local test server: opt out of Secure cookies (default-on) so the
-    # browser stores the auth/CSRF cookies over http://; the default loopback bind
-    # and auto-generated JWT secret are exactly what we want.
-    log_path = os.path.join(data, "server.log")
-    with open(log_path, "wb") as log:
-        proc = subprocess.Popen([binary, "serve", "--insecure-cookies"], env=env,
-                                stdout=log, stderr=subprocess.STDOUT)
-    base = f"http://127.0.0.1:{port}"
-    try:
-        _wait_reachable_or_fail(proc, port, log_path)
-        yield base
-    finally:
+    with tempfile.TemporaryDirectory(prefix="zb_admin_") as data, _reserve_port() as port:
+        shutil.copytree(_su_template_for(binary), data, dirs_exist_ok=True)
+        extra_env = getattr(request, "param", None) or {}
+        env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
+               # The harness owns this process as a FOREGROUND child (terminate() in the
+               # finally). Without the pin, running the suite from inside an AI-agent
+               # session (CLAUDECODE etc.) would trigger serve's auto-backgrounding: the
+               # parent exits after readiness, terminate() hits the dead parent, and the
+               # detached child leaks past the test.
+               "ZIGBASE_SERVE_BACKGROUND": "0", **extra_env}
+        # Plain-HTTP local test server: opt out of Secure cookies (default-on) so the
+        # browser stores the auth/CSRF cookies over http://; the default loopback bind
+        # and auto-generated JWT secret are exactly what we want.
+        log_path = os.path.join(data, "server.log")
+        with open(log_path, "wb") as log:
+            proc = subprocess.Popen([binary, "serve", "--insecure-cookies"], env=env,
+                                    stdout=log, stderr=subprocess.STDOUT)
+        base = f"http://127.0.0.1:{port}"
+        ready = False
         try:
-            _stop_server(proc)
+            _wait_reachable_or_fail(proc, port, log_path)
+            ready = True
+            request.node._server_diagnostic_context = (proc, log_path)
+            yield base
         finally:
-            shutil.rmtree(data, ignore_errors=True)
+            try:
+                exited = proc.poll()
+                failed = any(
+                    getattr(getattr(request.node, "rep_" + phase, None), "failed", False)
+                    for phase in ("setup", "call")
+                )
+                if exited is not None or failed:
+                    diagnostics = _server_diagnostics(request.node, proc, log_path)
+                    if ready and exited is not None and not failed:
+                        pytest.fail("Server exited unexpectedly before fixture cleanup." + diagnostics, pytrace=False)
+                    print(diagnostics)
+            finally:
+                request.node.__dict__.pop("_server_diagnostic_context", None)
+                _stop_server(proc)
 
 # Launching Chromium (~0.5s) once per test dominated the wall time of the (now
 # parallel) suite. Reuse ONE browser process per test session — under pytest-xdist
@@ -148,53 +194,64 @@ def _stop_server(proc):
         proc.wait(timeout=5)
 
 
-def _wait_reachable_or_fail(proc, port, log_path, timeout_s=5.0):
+def _wait_reachable_or_fail(proc, port, log_path, timeout_s=5.0, *, log_start=0):
     """Require HTTP health readiness, not merely an open socket; raise instead of returning
     silently if the server dies early or never comes up. A fixture that raises here
     fails cleanly at setup, instead of yielding a base URL nothing is listening on and
     leaving every test in the fixture to fail later with a generic connection error."""
     deadline = time.monotonic() + timeout_s
     opener = urllib.request.build_opener(urllib.request.ProxyHandler({}))
-    while time.monotonic() < deadline:
-        if proc.poll() is not None:
-            break  # process already exited; no point polling further
-        try:
-            with opener.open(f"http://127.0.0.1:{port}/api/health", timeout=0.2) as response:
-                body = json.load(response)
-                if response.status == 200 and isinstance(body, dict) and body.get("status") == "ok":
-                    return
-        except (OSError, ValueError, urllib.error.URLError):
-            pass
-        time.sleep(0.1)
+    output = ""
+    reader = None
+    with ExitStack() as logs:
+        while time.monotonic() < deadline:
+            if proc.poll() is not None:
+                break  # process already exited; no point polling further
+            if reader is None:
+                try:
+                    reader = logs.enter_context(pathlib.Path(log_path).open("rb"))
+                    reader.seek(log_start)
+                except FileNotFoundError:
+                    pass
+            if reader is not None:
+                output += reader.read().decode(errors="replace")
+            if re.search(r"cannot bind [^\n]*:" + str(port) + r": ListenError", output):
+                break  # This child lost the bind; a foreign healthy listener is irrelevant.
+            # Bind success belongs to the spawned PID, not merely this port. The PID
+            # also distinguishes restarts whose logs append to an earlier launch.
+            listening = (re.search(r'zigbase listening on http://[^"\\\s]+:' + str(port) + r'\b', output) or
+                         re.search(r"Listening on port " + str(port) + r"\b", output))
+            own_process = re.search(r"\* Root pid: " + str(proc.pid) + r"(?:\s|$)", output)
+            if listening and own_process:
+                try:
+                    with opener.open(f"http://127.0.0.1:{port}/api/health", timeout=0.2) as response:
+                        body = json.load(response)
+                        if response.status == 200 and isinstance(body, dict) and body.get("status") == "ok" and proc.poll() is None:
+                            return
+                except (OSError, ValueError, urllib.error.URLError):
+                    pass
+            time.sleep(0.1)
     _stop_server(proc)
     output = pathlib.Path(log_path).read_text(errors="replace") if pathlib.Path(log_path).exists() else "<no output captured>"
     raise AssertionError(f"server on port {port} never became reachable (exit={proc.returncode}):\n{output}")
 
 @pytest.fixture()
 def auth2_server(auth2_binary):
     """A live table-mode server with the beforeAuthSuccess fixture hook registered."""
-    data = tempfile.mkdtemp(prefix="zb_auth2_")
-    shutil.copytree(_su_template_for(auth2_binary), data, dirs_exist_ok=True)
-    port = _free_port()
-    # Foreground pin: see the server fixture's comment on ZIGBASE_SERVE_BACKGROUND.
-    env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
-           "ZIGBASE_SERVE_BACKGROUND": "0"}
-    log_path = os.path.join(data, "server.log")
-    with open(log_path, "wb") as log:
-        proc = subprocess.Popen([auth2_binary, "serve", "--insecure-cookies"], env=env,
-                                stdout=log, stderr=subprocess.STDOUT)
-    try:
-        _wait_reachable_or_fail(proc, port, log_path)
-    except AssertionError:
-        shutil.rmtree(data, ignore_errors=True)
-        raise
-    try:
-        yield f"http://127.0.0.1:{port}"
-    finally:
+    with tempfile.TemporaryDirectory(prefix="zb_auth2_") as data, _reserve_port() as port:
+        shutil.copytree(_su_template_for(auth2_binary), data, dirs_exist_ok=True)
+        # Foreground pin: see the server fixture's comment on ZIGBASE_SERVE_BACKGROUND.
+        env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
+               "ZIGBASE_SERVE_BACKGROUND": "0"}
+        log_path = os.path.join(data, "server.log")
+        with open(log_path, "wb") as log:
+            proc = subprocess.Popen([auth2_binary, "serve", "--insecure-cookies"], env=env,
+                                    stdout=log, stderr=subprocess.STDOUT)
         try:
-            _stop_server(proc)
+            _wait_reachable_or_fail(proc, port, log_path)
+            yield f"http://127.0.0.1:{port}"
         finally:
-            shutil.rmtree(data, ignore_errors=True)
+            _stop_server(proc)
 
 @pytest.fixture()
 def auth2_page(_browser, auth2_server):

tests/admin/test_agent_contracts.py

@@ -7,14 +7,13 @@
 fixture sends stdout/stderr to `DEVNULL` (conftest.py:52) and this test needs to read
 the log stream.
 """
+from conftest import _reserve_port
 import json, os, pathlib, shutil, socket, sqlite3, subprocess, tempfile, time, urllib.request, urllib.error
 import pytest
 
 REPO = pathlib.Path(__file__).resolve().parents[2]
 
 
-def _free_port():
-    s = socket.socket(); s.bind(("127.0.0.1", 0)); p = s.getsockname()[1]; s.close(); return p
 
 
 class LoggedServer:
@@ -40,33 +39,33 @@ def get(self, path):
 def logged_server(binary, request):
     flags = list(getattr(request, "param", []) or [])
     data = tempfile.mkdtemp(prefix="zb_logged_")
-    port = _free_port()
-    log_path = pathlib.Path(data) / "server.log"
-    # Foreground pin: this fixture asserts on the log file its own redirect
-    # captures, so serve must NOT auto-background itself when the suite runs
-    # inside an AI-agent session (CLAUDECODE etc.) — backgrounding would route
-    # the request log to <data-dir>/serve.log and leak the detached child past
-    # terminate(). See tests/admin/conftest.py's server fixture for the same pin.
-    env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
-           "ZIGBASE_SERVE_BACKGROUND": "0"}
-    with open(log_path, "w") as log:
-        proc = subprocess.Popen(
-            [binary, "serve", "--insecure-cookies", *flags],
-            env=env, stdout=log, stderr=subprocess.STDOUT,
-        )
-        for _ in range(50):
+    with _reserve_port() as port:
+        log_path = pathlib.Path(data) / "server.log"
+        # Foreground pin: this fixture asserts on the log file its own redirect
+        # captures, so serve must NOT auto-background itself when the suite runs
+        # inside an AI-agent session (CLAUDECODE etc.) — backgrounding would route
+        # the request log to <data-dir>/serve.log and leak the detached child past
+        # terminate(). See tests/admin/conftest.py's server fixture for the same pin.
+        env = {**os.environ, "ZIGBASE_DATA_DIR": data, "ZIGBASE_HTTP_PORT": str(port),
+               "ZIGBASE_SERVE_BACKGROUND": "0"}
+        with open(log_path, "w") as log:
+            proc = subprocess.Popen(
+                [binary, "serve", "--insecure-cookies", *flags],
+                env=env, stdout=log, stderr=subprocess.STDOUT,
+            )
+            for _ in range(50):
+                try:
+                    with socket.create_connection(("127.0.0.1", port), timeout=0.2):
+                        break
+                except OSError:
+                    time.sleep(0.1)
+            else:
+                proc.terminate()
+                pytest.fail(f"server never became reachable; log:\n{log_path.read_text()}")
             try:
-                with socket.create_connection(("127.0.0.1", port), timeout=0.2):
-                    break
-            except OSError:
-                time.sleep(0.1)
-        else:
-            proc.terminate()
-            pytest.fail(f"server never became reachable; log:\n{log_path.read_text()}")
-        try:
-            yield LoggedServer(f"http://127.0.0.1:{port}", proc, log_path, data)
-        finally:
-            proc.terminate(); proc.wait(timeout=5); shutil.rmtree(data, ignore_errors=True)
+                yield LoggedServer(f"http://127.0.0.1:{port}", proc, log_path, data)
+            finally:
+                proc.terminate(); proc.wait(timeout=5); shutil.rmtree(data, ignore_errors=True)
 
 
 @pytest.mark.parametrize("logged_server", [["--log-format", "json"]], indirect=True)