From 220ccbe59aba101a6336126a43beb4aba8223d45 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 22 Oct 2026 12:25:59 -0500 Subject: [PATCH 1/3] fix(local-api): probe readiness directly, keep live sessions, isolate smoke files The readiness probe used urlopen, which honors http_proxy. Without a loopback entry in no_proxy it asked the proxy: an answering proxy reported a server ready when nothing listened, and an unreachable one made the launcher stop a healthy server. It now uses a proxy-free opener. A second launch on a free port bound fine and replaced a running server's session.json, which #308's failed-bind guard cannot see. The launcher now refuses while the session's pid is alive and its loopback port answers. The .PHONY test planted stray files in backend/ and deleted them, destroying any real backend/local-api. It now runs make from tmp_path. The make helper drops an inherited PORT and MAKEFLAGS, so a supported PORT override no longer fails the default-port test, and the empty-port test passes --session-dir. Co-Authored-By: Claude Opus 5.5 --- backend/Makefile | 3 +- backend/docs/local-capture-api.md | 6 + backend/scripts/local_capture_api.py | 70 ++++++- backend/tests/test_local_capture_api.py | 240 ++++++++++++++++++++---- 4 files changed, 281 insertions(+), 38 deletions(-) diff --git a/backend/Makefile b/backend/Makefile index b10596ed..67b3c00f 100644 --- a/backend/Makefile +++ b/backend/Makefile @@ -89,7 +89,8 @@ e2e-dev: # Burst11: the one-command local capture API. Serves loopback only (127.0.0.1); # the iOS simulator shares the Mac's loopback, a physical phone does not. # Writes backend/runs/local-api/session.json (mode 0600, gitignored) with the -# base URL, app and reviewer tokens, pid and git sha. Stop with Ctrl-C. +# base URL, app and reviewer tokens, pid and git sha. Stop with Ctrl-C. A second +# launch refuses to start while that file's server still runs. # The documented default (docs/local-capture-api.md) is port 8000; override # with PORT=8123 make local-api. The empty-value guard catches an exported # PORT= so the launcher never sees a bare --port. diff --git a/backend/docs/local-capture-api.md b/backend/docs/local-capture-api.md index 006e69f3..c9290baa 100644 --- a/backend/docs/local-capture-api.md +++ b/backend/docs/local-capture-api.md @@ -106,6 +106,12 @@ against an unknown working directory; and `gitSha` is the full head SHA so a cap identifies the code that served it. Tokens appear nowhere else — not in stdout, not in the uvicorn access log (which keeps upload URLs as `/v1/uploads/{token}` route shapes anyway). +One session file belongs to one live server. A second `make local-api` against the same session +directory exits with an error while the first is still running (its `pid` is alive and its `baseUrl` +port accepts connections), so clients reading the file keep working credentials, even when the +second launch asks for a different `PORT`. Stop the first server, or pass the launcher a different +`--session-dir`. A file left behind by a server that has exited is replaced. + ## What is a stand-in `HSS_ORCH_STAGE_STUBS=1` runs every one of the twelve orchestrator stages as its committed stub diff --git a/backend/scripts/local_capture_api.py b/backend/scripts/local_capture_api.py index fcec49e0..a710e7ec 100644 --- a/backend/scripts/local_capture_api.py +++ b/backend/scripts/local_capture_api.py @@ -42,6 +42,7 @@ import threading import time import urllib.error +import urllib.parse import urllib.request from datetime import UTC, datetime from pathlib import Path @@ -139,11 +140,61 @@ def _write_session(path: Path, session: dict) -> None: os.chmod(path, 0o600) +def _pid_alive(pid: object) -> bool: + # bool is an int, and 0 or a negative pid addresses a process group, for + # which os.kill(pid, 0) succeeds whether or not the server still runs. + if not isinstance(pid, int) or isinstance(pid, bool) or pid <= 0: + return False + try: + os.kill(pid, 0) + except PermissionError: + return True # alive, owned by another user + except (ProcessLookupError, OverflowError): + return False + return True + + +def _live_session(session_path: Path) -> dict | None: + """The session at `session_path` if the server that wrote it still serves. + + Live means its pid is alive and its loopback port accepts a connection. + Either alone misleads: a crashed server's pid can be reused, and its port + can be taken by another process. A missing or unreadable file is not live; + no client could use its credentials either. + """ + try: + session = json.loads(session_path.read_text()) + except (OSError, ValueError): + return None + if not isinstance(session, dict) or not _pid_alive(session.get("pid")): + return None + try: + url = urllib.parse.urlsplit(str(session.get("baseUrl", ""))) + host, port = url.hostname, url.port + except ValueError: + return None + if host not in LOOPBACK_HOSTS or port is None: + return None + try: + with socket.create_connection((host, port), timeout=1): + return session + except OSError: + return None + + +# The readiness probe must reach the listener itself. urlopen honors +# http_proxy, and without a loopback entry in no_proxy it asks the proxy +# instead: a proxy that answers reports a server ready that is not serving, and +# one that cannot reach loopback makes the launcher stop a healthy server +# (Greptile P1 on #281). +_DIRECT = urllib.request.build_opener(urllib.request.ProxyHandler({})) + + def _wait_health(base: str, timeout_s: float) -> bool: deadline = time.monotonic() + timeout_s while time.monotonic() < deadline: try: - with urllib.request.urlopen(f"{base}/v1/healthz", timeout=2) as response: + with _DIRECT.open(f"{base}/v1/healthz", timeout=2) as response: if response.status == 200: return True except (urllib.error.URLError, TimeoutError, OSError): @@ -255,6 +306,20 @@ def main(argv: list[str] | None = None) -> int: ) return 2 + session_dir = args.session_dir + volume_root = session_dir / "volume" + session_path = session_dir / "session.json" + # A second launch on a free port would bind and replace a running server's + # credentials, so the failed-bind guard below never fires; refuse instead. + live = _live_session(session_path) + if live is not None: + print( + f"refusing to start: {session_path} belongs to a live local capture API " + f"(pid {live['pid']}, {live['baseUrl']}). Stop it first, or pass another --session-dir.", + file=sys.stderr, + ) + return 1 + import uvicorn try: @@ -273,9 +338,6 @@ def main(argv: list[str] | None = None) -> int: hmac_secret = secrets.token_hex(32) git_sha = _git_sha() - session_dir = args.session_dir - volume_root = session_dir / "volume" - session_path = session_dir / "session.json" session = { "baseUrl": f"{base}/v1", "appToken": app_token, diff --git a/backend/tests/test_local_capture_api.py b/backend/tests/test_local_capture_api.py index 48c12fd3..faf46fc7 100644 --- a/backend/tests/test_local_capture_api.py +++ b/backend/tests/test_local_capture_api.py @@ -22,6 +22,7 @@ import base64 import hashlib +import http.server import importlib.util import json import os @@ -29,7 +30,9 @@ import stat import subprocess import sys +import threading import time +import urllib.request from pathlib import Path import httpx @@ -76,6 +79,13 @@ def _free_port() -> int: return sock.getsockname()[1] +def _loopback_client(**kwargs) -> httpx.Client: + """An httpx client that ignores http_proxy and friends: every request in + this file targets the launcher on 127.0.0.1, and a proxy from the + developer's shell must not stand between the test and that socket.""" + return httpx.Client(trust_env=False, **kwargs) + + class LocalServer: """The launcher as a subprocess, plus the session it promised on disk.""" @@ -92,7 +102,7 @@ def base(self) -> str: def client(self, token: str | None) -> httpx.Client: headers = {"Authorization": f"Bearer {token}"} if token else {} - return httpx.Client(base_url=f"{self.base}/v1", headers=headers, timeout=30) + return _loopback_client(base_url=f"{self.base}/v1", headers=headers, timeout=30) def log(self) -> str: return self.log_path.read_text() @@ -127,13 +137,14 @@ def _start_server( text=True, ) log.close() - # The launcher writes session.json before binding, so wait for healthz. + # session.json appears once the port is bound, before uvicorn serves, so + # wait for healthz. deadline = time.monotonic() + 90 while time.monotonic() < deadline: if proc.poll() is not None: pytest.fail(f"local capture API exited early with {proc.returncode}:\n{log_path.read_text()}") try: - with httpx.Client(timeout=2) as probe: + with _loopback_client(timeout=2) as probe: if probe.get(f"http://127.0.0.1:{port}/v1/healthz").status_code == 200: return proc except httpx.HTTPError: @@ -210,7 +221,7 @@ def _drive_to_terminal(client: httpx.Client, workdir: Path, packet_id: str) -> t assert registered.status_code == 200, registered.text assert len(registered.json()["files"]) == len(files) - with httpx.Client(timeout=60) as uploads: + with _loopback_client(timeout=60) as uploads: for body, target in zip(files, registered.json()["files"], strict=True): put = uploads.put( target["upload"]["url"], @@ -339,7 +350,7 @@ def test_malformed_finalize_is_refused_4xx(server: LocalServer, tmp_path: Path) def test_missing_and_wrong_bearer_get_401(server: LocalServer) -> None: - with httpx.Client(base_url=f"{server.base}/v1", timeout=30) as anon: + with _loopback_client(base_url=f"{server.base}/v1", timeout=30) as anon: missing = anon.get("/captures/whatever/events") assert missing.status_code == 401, missing.text errors = missing.json()["errors"] @@ -417,26 +428,197 @@ def test_failed_bind_preserves_the_previous_session(tmp_path: Path) -> None: assert "could not bind" in result.stderr -def _make_n_local_api(env: dict[str, str] | None = None) -> str: - """Print the `make -n local-api` recipe from backend/ without executing it.""" +def _stop(proc: subprocess.Popen | None) -> None: + if proc is None or proc.poll() is not None: + return + proc.terminate() + try: + proc.wait(timeout=15) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + + +def test_second_launch_refuses_a_live_session(tmp_path: Path) -> None: + """A second launch on a different port binds fine, so the failed-bind guard + never fires; it must still leave the live server's session.json alone.""" + + session_dir = tmp_path / "session" + first = _start_server(session_dir, tmp_path / "first.log") + second = None + try: + before = (session_dir / "session.json").read_bytes() + live = json.loads(before) + second = subprocess.Popen( + [ + sys.executable, + str(SCRIPT), + "--host", + "127.0.0.1", + "--port", + str(_free_port()), + "--session-dir", + str(session_dir), + ], + cwd=ROOT, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + _, stderr = second.communicate(timeout=30) + except subprocess.TimeoutExpired: + second.kill() + second.communicate() + pytest.fail("the second launch kept running beside a live session") + assert second.returncode != 0 + assert "live local capture API" in stderr, stderr + assert (session_dir / "session.json").read_bytes() == before + # The credentials on disk still open the running server. + headers = {"Authorization": f"Bearer {live['appToken']}"} + with _loopback_client(base_url=live["baseUrl"], headers=headers, timeout=10) as client: + assert client.get("/captures/whatever/events").status_code != 401 + finally: + _stop(second) + _stop(first) + + +def test_live_session_needs_a_live_pid_and_an_answering_port(tmp_path: Path) -> None: + launcher = _launcher_module() + path = tmp_path / "session.json" + assert launcher._live_session(path) is None # no file + path.write_text("{not json") + assert launcher._live_session(path) is None # unreadable: no client can use it either + + listener = socket.socket() + listener.bind(("127.0.0.1", 0)) + listener.listen(1) + open_port = listener.getsockname()[1] + exited = subprocess.Popen([sys.executable, "-c", "pass"]) + exited.wait() + try: + cases = [ + (os.getpid(), f"127.0.0.1:{open_port}", True), + # A crashed server whose pid was reused: nothing listens. + (os.getpid(), f"127.0.0.1:{_free_port()}", False), + # The pid is gone; whatever owns the port now is not this session. + (exited.pid, f"127.0.0.1:{open_port}", False), + # os.kill(0, 0) would signal this process group and succeed. + (0, f"127.0.0.1:{open_port}", False), + (True, f"127.0.0.1:{open_port}", False), + (str(os.getpid()), f"127.0.0.1:{open_port}", False), + # The launcher only ever writes loopback; never probe anything else. + (os.getpid(), f"192.0.2.1:{open_port}", False), + ] + for pid, hostport, live in cases: + path.write_text(json.dumps({"baseUrl": f"http://{hostport}/v1", "pid": pid})) + assert (launcher._live_session(path) is not None) is live, (pid, hostport) + finally: + listener.close() + + +class _AnswerEverything(http.server.BaseHTTPRequestHandler): + """A stand-in proxy that answers 200 to any request and records it.""" + + def do_GET(self) -> None: + self.server.hits.append(self.path) + body = b"ok" + self.send_response(200) + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *args) -> None: + pass + + +def test_readiness_needs_the_listener_not_a_shell_proxy(monkeypatch) -> None: + """Greptile P1 on #281: urlopen honors http_proxy, so the readiness probe + asked the proxy instead of the listener. A proxy that answers reported a + server ready when nothing was listening; one that cannot reach loopback + made the launcher kill a healthy server.""" + + launcher = _launcher_module() + proxy = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _AnswerEverything) + proxy.hits = [] + threading.Thread(target=proxy.serve_forever, daemon=True).start() + try: + for key in ("http_proxy", "HTTP_PROXY"): + monkeypatch.setenv(key, f"http://127.0.0.1:{proxy.server_address[1]}") + for key in ("no_proxy", "NO_PROXY"): + monkeypatch.delenv(key, raising=False) + # urlopen builds its opener, proxy settings included, on first use and + # caches it; clear the cache so the probe sees the env set above. + monkeypatch.setattr(urllib.request, "_opener", None) + nothing_listening = f"http://127.0.0.1:{_free_port()}" + assert launcher._wait_health(nothing_listening, 1.0) is False + assert proxy.hits == [] + finally: + proxy.shutdown() + proxy.server_close() + + +def test_launcher_reports_ready_under_an_unreachable_proxy(tmp_path: Path) -> None: + dead_proxy = f"http://127.0.0.1:{_free_port()}" + log_path = tmp_path / "proxied.log" + proc = _start_server( + tmp_path / "session", + log_path, + extra_args=["--health-timeout", "20"], + extra_env={"http_proxy": dead_proxy, "HTTP_PROXY": dead_proxy, "no_proxy": "", "NO_PROXY": ""}, + ) + try: + deadline = time.monotonic() + 30 + while "local capture API ready" not in log_path.read_text(): + assert proc.poll() is None, log_path.read_text() + assert time.monotonic() < deadline, log_path.read_text() + time.sleep(0.25) + finally: + _stop(proc) + + +def _make_n(target: str, env: dict[str, str] | None = None, cwd: Path = ROOT) -> str: + """Print backend/Makefile's recipe for `target` without executing it. + + Make resolves target files against `cwd`, so a test that plants stray + files passes its own temp directory; `-I ROOT` keeps the Makefile's + relative `include infra/deploy.mk` working from there. The shell's PORT is + a supported override (`PORT=8123 make local-api`), and a command-line + `PORT=` reaches a child make through MAKEFLAGS, so both are dropped and a + test that wants a port passes it in `env`. + """ + inherited = {key: value for key, value in os.environ.items() if key not in {"PORT", "MAKEFLAGS"}} proc = subprocess.run( - ["make", "-n", "local-api"], - cwd=ROOT, + ["make", "-f", str(ROOT / "Makefile"), "-I", str(ROOT), "-n", target], + cwd=cwd, capture_output=True, text=True, timeout=30, - env={**os.environ, **(env or {})}, + env={**inherited, **(env or {})}, ) assert proc.returncode == 0, proc.stderr return proc.stdout +def _make_n_local_api(env: dict[str, str] | None = None) -> str: + return _make_n("local-api", env) + + def test_make_local_api_without_port_passes_the_documented_default() -> None: # The documented flow (docs/local-capture-api.md) is bare `make local-api` # on port 8000; the recipe must not hand the launcher a bare --port. assert "--port 8000" in _make_n_local_api() +@pytest.mark.parametrize(("name", "value"), [("PORT", "8123"), ("MAKEFLAGS", "-- PORT=8123")]) +def test_default_port_check_ignores_the_runners_port(monkeypatch, name: str, value: str) -> None: + # Greptile P2 on #281: `PORT=8123 make local-api-smoke` (or + # `make local-api-smoke PORT=8123`, which arrives as MAKEFLAGS) is a + # supported setting, and the default-port assertion must not fail under it. + monkeypatch.setenv(name, value) + assert "--port 8000" in _make_n_local_api() + + def test_make_local_api_honors_an_explicit_port_override() -> None: stdout = _make_n_local_api({"PORT": "8123"}) assert "--port 8123" in stdout @@ -452,7 +634,9 @@ def test_launcher_refuses_an_empty_port_loudly(tmp_path: Path) -> None: log_path = tmp_path / "empty-port.log" with log_path.open("w") as log: proc = subprocess.run( - [sys.executable, str(SCRIPT), "--host", "127.0.0.1", "--port="], + # --session-dir keeps a regression here from writing the real + # backend/runs/local-api/session.json a running server owns. + [sys.executable, str(SCRIPT), "--host", "127.0.0.1", "--port=", "--session-dir", str(tmp_path)], cwd=ROOT, stdout=log, stderr=subprocess.STDOUT, @@ -469,27 +653,17 @@ def test_launcher_refuses_an_empty_port_loudly(tmp_path: Path) -> None: def test_make_local_api_targets_survive_a_stray_file(tmp_path: Path) -> None: # The Codex P2 on this PR: local-api / local-api-smoke must be .PHONY, or a # stray file (or directory) of the same name makes `make local-api` report - # "up to date" and never start the server. - strays = [ROOT / "local-api", ROOT / "local-api-smoke"] - try: - for stray in strays: - stray.write_text("") - recipe = _make_n_local_api() - assert "up to date" not in recipe, recipe - assert "local_capture_api.py" in recipe, recipe - smoke = subprocess.run( - ["make", "-n", "local-api-smoke"], - cwd=ROOT, - capture_output=True, - text=True, - timeout=30, - ) - assert smoke.returncode == 0, smoke.stderr - assert "up to date" not in smoke.stdout, smoke.stdout - assert "test_local_capture_api.py" in smoke.stdout, smoke.stdout - finally: - for stray in strays: - stray.unlink(missing_ok=True) + # "up to date" and never start the server. The strays go in tmp_path, never + # backend/: planting them there truncated and then deleted any real + # backend/local-api or backend/local-api-smoke (Greptile P1 on #281). + for name in ("local-api", "local-api-smoke"): + (tmp_path / name).write_text("") + recipe = _make_n("local-api", cwd=tmp_path) + assert "up to date" not in recipe, recipe + assert "local_capture_api.py" in recipe, recipe + smoke = _make_n("local-api-smoke", cwd=tmp_path) + assert "up to date" not in smoke, smoke + assert "test_local_capture_api.py" in smoke, smoke def _launcher_module(): @@ -576,7 +750,7 @@ def test_launcher_overrides_a_poisoned_shell_and_stays_healthy(tmp_path: Path) - }, ) try: - with httpx.Client(timeout=10) as probe: + with _loopback_client(timeout=10) as probe: body = probe.get(f"http://127.0.0.1:{_port_from_session(session_dir)}/v1/healthz").json() assert body["status"] == "ok", body assert body["state"] == "memory", body From 832314ca0cc7f37123f94b6c72db1ec79c7e4295 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Thu, 22 Oct 2026 12:40:10 -0500 Subject: [PATCH 2/3] test(local-api): hold refusing ports open so no server can take them A port from _free_port() is released before the launcher picks its own, so the dead proxy could in rare cases land on the server and let the old probe pass. A bound, never-listening socket refuses connections and keeps the port. Co-Authored-By: Claude Opus 5.5 --- backend/tests/test_local_capture_api.py | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/backend/tests/test_local_capture_api.py b/backend/tests/test_local_capture_api.py index faf46fc7..bd8e80d2 100644 --- a/backend/tests/test_local_capture_api.py +++ b/backend/tests/test_local_capture_api.py @@ -79,6 +79,15 @@ def _free_port() -> int: return sock.getsockname()[1] +def _refusing_port() -> socket.socket: + """A loopback port that refuses connections: bound, never listening. It + stays reserved while the socket is open, so no server this test starts + can land on it.""" + sock = socket.socket() + sock.bind(("127.0.0.1", 0)) + return sock + + def _loopback_client(**kwargs) -> httpx.Client: """An httpx client that ignores http_proxy and friends: every request in this file targets the launcher on 127.0.0.1, and a proxy from the @@ -494,13 +503,14 @@ def test_live_session_needs_a_live_pid_and_an_answering_port(tmp_path: Path) -> listener.bind(("127.0.0.1", 0)) listener.listen(1) open_port = listener.getsockname()[1] + refusing = _refusing_port() exited = subprocess.Popen([sys.executable, "-c", "pass"]) exited.wait() try: cases = [ (os.getpid(), f"127.0.0.1:{open_port}", True), # A crashed server whose pid was reused: nothing listens. - (os.getpid(), f"127.0.0.1:{_free_port()}", False), + (os.getpid(), f"127.0.0.1:{refusing.getsockname()[1]}", False), # The pid is gone; whatever owns the port now is not this session. (exited.pid, f"127.0.0.1:{open_port}", False), # os.kill(0, 0) would signal this process group and succeed. @@ -515,6 +525,7 @@ def test_live_session_needs_a_live_pid_and_an_answering_port(tmp_path: Path) -> assert (launcher._live_session(path) is not None) is live, (pid, hostport) finally: listener.close() + refusing.close() class _AnswerEverything(http.server.BaseHTTPRequestHandler): @@ -542,6 +553,7 @@ def test_readiness_needs_the_listener_not_a_shell_proxy(monkeypatch) -> None: proxy = http.server.ThreadingHTTPServer(("127.0.0.1", 0), _AnswerEverything) proxy.hits = [] threading.Thread(target=proxy.serve_forever, daemon=True).start() + nothing_listening = _refusing_port() try: for key in ("http_proxy", "HTTP_PROXY"): monkeypatch.setenv(key, f"http://127.0.0.1:{proxy.server_address[1]}") @@ -550,16 +562,18 @@ def test_readiness_needs_the_listener_not_a_shell_proxy(monkeypatch) -> None: # urlopen builds its opener, proxy settings included, on first use and # caches it; clear the cache so the probe sees the env set above. monkeypatch.setattr(urllib.request, "_opener", None) - nothing_listening = f"http://127.0.0.1:{_free_port()}" - assert launcher._wait_health(nothing_listening, 1.0) is False + base = f"http://127.0.0.1:{nothing_listening.getsockname()[1]}" + assert launcher._wait_health(base, 1.0) is False assert proxy.hits == [] finally: proxy.shutdown() proxy.server_close() + nothing_listening.close() def test_launcher_reports_ready_under_an_unreachable_proxy(tmp_path: Path) -> None: - dead_proxy = f"http://127.0.0.1:{_free_port()}" + dead = _refusing_port() + dead_proxy = f"http://127.0.0.1:{dead.getsockname()[1]}" log_path = tmp_path / "proxied.log" proc = _start_server( tmp_path / "session", @@ -575,6 +589,7 @@ def test_launcher_reports_ready_under_an_unreachable_proxy(tmp_path: Path) -> No time.sleep(0.25) finally: _stop(proc) + dead.close() def _make_n(target: str, env: dict[str, str] | None = None, cwd: Path = ROOT) -> str: From ca7e89c98918f8e5cb51757b737d3c1ba40d1579 Mon Sep 17 00:00:00 2001 From: SamGu-NRX Date: Fri, 23 Oct 2026 10:53:07 -0500 Subject: [PATCH 3/3] fix(local-api): lock session directory across launches Hold an exclusive flock from the live-session check through server shutdown so concurrent launches cannot replace credentials. Add deterministic race and dead-owner regression tests. Co-Authored-By: Claude Code --- backend/scripts/local_capture_api.py | 196 ++++++++++++++++-------- backend/tests/test_local_capture_api.py | 97 ++++++++++++ 2 files changed, 226 insertions(+), 67 deletions(-) diff --git a/backend/scripts/local_capture_api.py b/backend/scripts/local_capture_api.py index a710e7ec..901534b7 100644 --- a/backend/scripts/local_capture_api.py +++ b/backend/scripts/local_capture_api.py @@ -33,6 +33,7 @@ from __future__ import annotations import argparse +import fcntl import json import os import secrets @@ -140,6 +141,48 @@ def _write_session(path: Path, session: dict) -> None: os.chmod(path, 0o600) +class _SessionDirectoryLocked(Exception): + """Another launcher owns the session directory lock.""" + + def __init__(self, session: dict | None): + self.session = session + + +def _open_session_lock(session_dir: Path, session_path: Path) -> int: + """Own the session directory until the server exits. + + A blocked launch waits briefly for the owner to publish its session. That + turns the startup race into the same clear live-session refusal as a later + launch, while a dead owner is handled by the kernel releasing its flock. + """ + session_dir.mkdir(parents=True, exist_ok=True) + lock_path = session_dir / ".session.lock" + flags = os.O_RDWR | os.O_CREAT | os.O_NOFOLLOW + fd = os.open(lock_path, flags, 0o600) + try: + deadline = time.monotonic() + 5.0 + while True: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + os.chmod(lock_path, 0o600) + return fd + except BlockingIOError: + live = _live_session(session_path) + if live is not None: + raise _SessionDirectoryLocked(live) from None + if time.monotonic() >= deadline: + raise _SessionDirectoryLocked(None) from None + time.sleep(0.05) + except BaseException: + os.close(fd) + raise + + +def _close_session_lock(fd: int) -> None: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + + def _pid_alive(pid: object) -> bool: # bool is an int, and 0 or a negative pid addresses a process group, for # which os.kill(pid, 0) succeeds whether or not the server still runs. @@ -309,77 +352,96 @@ def main(argv: list[str] | None = None) -> int: session_dir = args.session_dir volume_root = session_dir / "volume" session_path = session_dir / "session.json" - # A second launch on a free port would bind and replace a running server's - # credentials, so the failed-bind guard below never fires; refuse instead. - live = _live_session(session_path) - if live is not None: - print( - f"refusing to start: {session_path} belongs to a live local capture API " - f"(pid {live['pid']}, {live['baseUrl']}). Stop it first, or pass another --session-dir.", - file=sys.stderr, - ) + try: + lock_fd = _open_session_lock(session_dir, session_path) + except _SessionDirectoryLocked as exc: + if exc.session is None: + print( + f"refusing to start: {session_dir} is locked by another local capture API launch", + file=sys.stderr, + ) + else: + print( + f"refusing to start: {session_path} belongs to a live local capture API " + f"(pid {exc.session['pid']}, {exc.session['baseUrl']}). Stop it first, or pass another --session-dir.", + file=sys.stderr, + ) return 1 - import uvicorn - try: - listener = _listen(args.host, args.port) - except OSError as exc: - print(f"could not bind local capture API on {args.host}:{args.port}: {exc}", file=sys.stderr) - return 1 - port = int(listener.getsockname()[1]) - base = _public_url(args.host, port) - - # Fresh per-run credentials: a device bearer for the app and harness, an - # explicit reviewer bearer for the review page (server/api/auth.py reads - # `reviewer: