From 6829136f422d95998857866d120793c83696ec3a Mon Sep 17 00:00:00 2001 From: istos Date: Sat, 1 Aug 2026 07:32:54 +0200 Subject: [PATCH] start: a pinned port stays pinned across a restart MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The free-port probe bound without SO_REUSEADDR while the board's own ThreadingHTTPServer sets it, so the socket a just-stopped board left in TIME_WAIT read as "taken by something else": a routine stop/start walked the board to the next port and wrote that over the user's BOARD_PORT pin. The probe now binds exactly as the server does, which is the whole race. Behind it, a held port gets a few seconds (BOARD_PORT_WAIT, 5s) to clear before the walk, re-asking is_our_board each beat — a restart races its own predecessor far more often than a stranger takes the port. Walking off a pinned port still persists, since the hooks and agents read BOARD_PORT and must reach the live board, but it now says so in full: the right file (manager/local/.env, not manager/.env), old → new, and how to reclaim the pin. Tested end to end against a scratch host with a stub board.py, over real sockets: a genuine TIME_WAIT remnant, a listener that lets go mid-wait, a listener that does not, and our own board answering. Co-Authored-By: Claude Opus 5 --- AGENTS.md | 18 +- start.sh | 53 +++++- tests/test_install_first_boot.py | 4 +- tests/test_pinned_port.py | 284 +++++++++++++++++++++++++++++++ 4 files changed, 346 insertions(+), 13 deletions(-) create mode 100644 tests/test_pinned_port.py diff --git a/AGENTS.md b/AGENTS.md index 72c9e1c..ca9c7a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -209,9 +209,21 @@ serving in the foreground (Ctrl-C stops it). Three cases: - this project's board already answers on the port → just reopens the browser - the port is free → starts on it -- something else occupies it → takes the next free port **and persists it to - `manager/.env`**, so the hooks and agents — which read the same file — - follow the board rather than reporting to a port it no longer serves. +- something else occupies it → waits a few seconds for it to clear, then + takes the next free port **and persists it to `manager/local/.env`**, so + the hooks and agents — which read the same file — follow the board rather + than reporting to a port it no longer serves. Overwriting a port the user + pinned is not done quietly: the hop names the file, both ports and how to + reclaim the old one. + +The probe behind those three cases binds the way the board itself binds — +`127.0.0.1` with `SO_REUSEADDR`, which `ThreadingHTTPServer` sets — so the +socket a just-stopped board leaves in `TIME_WAIT` does not read as +occupied. A probe stricter than its server would hop a restart off its own +pinned port. The wait on top covers the rest of a predecessor's shutdown — +five seconds, or whatever `BOARD_PORT_WAIT` says in start.sh's environment +— because a restart races its own board far more often than a stranger +takes the port. Extra arguments pass through to `board.py` (e.g. `./start.sh --no-open`). diff --git a/start.sh b/start.sh index dab6d9b..eb9f266 100755 --- a/start.sh +++ b/start.sh @@ -4,12 +4,19 @@ # ./.task-manager/start.sh # foreground; Ctrl-C stops the board # ./.task-manager/start.sh --no-open # extra args pass through to board.py # -# Port logic (BOARD_PORT from env, else manager/.env, else 26071): +# Port logic (BOARD_PORT from env, else manager/local/.env, else 26071): # - our own board already answering there -> just open the browser # - port free -> start on it -# - something else squatting on it -> take the next free port AND -# persist it to manager/.env, so the hooks and agents (which read the same -# file) follow the board to its new port instead of reporting into the void. +# - held by something else -> wait a few seconds for it to +# clear (a restart races its own predecessor's shutdown far more often +# than a stranger takes the port), and only then take the next free port +# AND persist it to manager/local/.env, so the hooks and agents (which +# read the same file) follow the board instead of reporting into the void. +# +# The probe binds exactly as the board does — 127.0.0.1 with SO_REUSEADDR, +# which is what ThreadingHTTPServer sets — because a probe stricter than the +# server lies: a socket the just-stopped board left in TIME_WAIT would read +# as occupied and hop the board off a port its user deliberately pinned. set -euo pipefail TM="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" @@ -17,6 +24,13 @@ MANAGER="$TM/manager" CORE="$MANAGER/core" ENV_FILE="$MANAGER/local/.env" +# Seconds to let a held port clear before walking off it. Shutdown is quick; +# this is grace for a predecessor, not patience for a squatter. Anything +# that isn't a number falls back to the default rather than failing a start +# on an arithmetic comparison. +busy_wait="${BOARD_PORT_WAIT:-5}" +case "$busy_wait" in ''|*[!0-9]*) busy_wait=5 ;; esac + port="${BOARD_PORT:-}" if [ -z "$port" ] && [ -f "$ENV_FILE" ]; then port="$(sed -n 's/^[[:space:]]*BOARD_PORT[[:space:]]*=[[:space:]]*//p' "$ENV_FILE" | tail -1 | tr -d "'\"")" @@ -31,6 +45,9 @@ is_free() { python3 - "$1" <<'PY' import socket, sys s = socket.socket() +# The board's ThreadingHTTPServer sets this; a probe without it would call a +# TIME_WAIT remnant "busy" on a port the server itself could bind. +s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) try: s.bind(("127.0.0.1", int(sys.argv[1]))) except OSError: @@ -52,12 +69,31 @@ sys.exit(0 if data.get("board", {}).get("root") == sys.argv[2] else 1) ' "$1" "$TM/tasks" } +open_board() { + echo "Board already running at http://127.0.0.1:$1/ — opening it." + python3 -m webbrowser -t "http://127.0.0.1:$1/" >/dev/null +} + if is_our_board "$port"; then - echo "Board already running at http://127.0.0.1:$port/ — opening it." - python3 -m webbrowser -t "http://127.0.0.1:$port/" >/dev/null + open_board "$port" exit 0 fi +if ! is_free "$port"; then + # Held by someone. Give it a moment: the usual holder is the board this + # start is replacing, and it lets go within a second or two. + echo "Port $port is busy — waiting up to ${busy_wait}s for it to clear." + waited=0 + while [ "$waited" -lt "$busy_wait" ] && ! is_free "$port"; do + sleep 1 + waited=$((waited + 1)) + if is_our_board "$port"; then # a board came up during the wait + open_board "$port" + exit 0 + fi + done +fi + if ! is_free "$port"; then original=$port for offset in $(seq 1 20); do @@ -68,8 +104,9 @@ if ! is_free "$port"; then echo "error: ports $original-$((original + 20)) all busy — set BOARD_PORT yourself." >&2 exit 1 fi - echo "Port $original is taken by something else — using $port instead." - echo "Persisting BOARD_PORT=$port to manager/.env so hooks and agents follow." + echo "Port $original is held by another process — using $port instead." + echo "Rewriting BOARD_PORT in manager/local/.env: $original → $port, so the hooks and agents follow the live board." + echo "To reclaim $original: free it, then set BOARD_PORT=$original in manager/local/.env." python3 - "$ENV_FILE" "$port" <<'PY' import pathlib, sys path, port = pathlib.Path(sys.argv[1]), sys.argv[2] diff --git a/tests/test_install_first_boot.py b/tests/test_install_first_boot.py index add310d..4f82f93 100644 --- a/tests/test_install_first_boot.py +++ b/tests/test_install_first_boot.py @@ -371,8 +371,8 @@ class FirstRunSettings(unittest.TestCase): one other writer, and it must leave the rest of it intact.""" run_install_tty(self.tm, ["team", "", ""]) text = (REPO / "start.sh").read_text(encoding="utf-8") - snippet = (text.split("Persisting BOARD_PORT", 1)[1] - .split("<<'PY'\n", 1)[1].split("\nPY\n", 1)[0]) + snippet = (text.split('python3 - "$ENV_FILE" "$port" <<\'PY\'\n', 1)[1] + .split("\nPY\n", 1)[0]) result = subprocess.run( [sys.executable, "-c", snippet, str(self.env_file), "26072"], capture_output=True, text=True) diff --git a/tests/test_pinned_port.py b/tests/test_pinned_port.py new file mode 100644 index 0000000..aeb7d69 --- /dev/null +++ b/tests/test_pinned_port.py @@ -0,0 +1,284 @@ +"""A pinned port stays pinned. start.sh's port handling, run end to end as +a subprocess against a scratch host whose board.py is a stub that records +how it was launched. Run with: python3 -m unittest discover -s tests + +The bug this guards: the free-port probe bound without SO_REUSEADDR while +the board's ThreadingHTTPServer sets it, so a socket the just-stopped board +left in TIME_WAIT read as "taken by something else" — and start.sh then +walked to the next port and wrote that over the user's own BOARD_PORT pin. +The sockets here are real: TIME_WAIT is produced by an actual closed +connection, and a holder is an actual listener. +""" + +import json +import os +import shutil +import socket +import subprocess +import sys +import tempfile +import threading +import unittest +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +STAGES = ["backlog", "to-do", "in-progress", "review", "done"] + +# A board.py that serves nothing and only records the port it was told to +# use — every assertion about "which port did we start on" reads this. +STUB_BOARD = """#!/usr/bin/env python3 +import json, pathlib, sys +pathlib.Path(__file__).with_name("launched.json").write_text( + json.dumps(sys.argv[1:]), encoding="utf-8") +""" + +ENV_TEMPLATE = """# The port the board serves on. +BOARD_PORT={port} + +# Seconds between disk polls of the stage directories. +BOARD_WATCH_INTERVAL=2 +BOARD_SYNC= +""" + + +def make_host(root: Path, port: int) -> Path: + """A host project whose board is pinned to `port`: the real start.sh and + install.py, a stub board.py, and a local/.env with the pin plus a couple + of other settings, so a rewrite that damages the file is visible.""" + host = root / "host" + (host / ".claude").mkdir(parents=True) + tm = host / ".task-manager" + tm.mkdir() + shutil.copy(REPO / "start.sh", tm / "start.sh") + shutil.copy(REPO / "install.py", tm / "install.py") + shutil.copytree(REPO / "manager" / "core" / "adapters" / "claude", + tm / "manager" / "core" / "adapters" / "claude") + shutil.copy(REPO / "manager" / "core" / ".env.example", + tm / "manager" / "core" / ".env.example") + (tm / "manager" / "core" / "board.py").write_text(STUB_BOARD, + encoding="utf-8") + for stage in STAGES + ["archive"]: + d = tm / "tasks" / stage + d.mkdir(parents=True) + (d / ".gitkeep").touch() + local = tm / "manager" / "local" + local.mkdir(parents=True) + # An .env on disk also disarms install.py's first-boot clean, so the + # start under test does nothing but wire, probe and launch. + (local / ".env").write_text(ENV_TEMPLATE.format(port=port), + encoding="utf-8") + return tm + + +def clean_env() -> dict: + """No BOARD_* leaking in from the developer's shell: the pin under test + is the one in local/.env. BROWSER keeps the browser-open path silent — + `python3 -m webbrowser` runs it, and a test must not raise a window.""" + env = {k: v for k, v in os.environ.items() if not k.startswith("BOARD_")} + env["BROWSER"] = "/usr/bin/true" + return env + + +def run_start(tm: Path, wait: str = "2", **overrides) -> subprocess.CompletedProcess: + env = clean_env() + env["BOARD_PORT_WAIT"] = wait + env.update(overrides) + return subprocess.run( + ["bash", str(tm / "start.sh")], capture_output=True, text=True, + cwd=tm.parent, env=env, stdin=subprocess.DEVNULL, timeout=120) + + +def launched(tm: Path): + """The arguments the stub board.py was started with, or None if start.sh + never got that far.""" + marker = tm / "manager" / "core" / "launched.json" + return json.loads(marker.read_text(encoding="utf-8")) if marker.is_file() else None + + +def free_port(count: int = 1) -> int: + """The first of `count` consecutive ports nothing holds — including + nothing in TIME_WAIT, since the probe here binds without SO_REUSEADDR.""" + for base in range(27100, 27600, count): + held, free = [], True + for candidate in range(base, base + count): + s = socket.socket() + try: + s.bind(("127.0.0.1", candidate)) + except OSError: + free = False + s.close() + break + held.append(s) + for s in held: + s.close() + if free: + return base + raise AssertionError("no run of free ports to test with") + + +def leave_time_wait(port: int) -> None: + """Leave a real socket in TIME_WAIT on `port`: connect to a listener and + close the server's end first, which is exactly what a board shutting + down does to the browser tab still attached to it.""" + listener = socket.socket() + listener.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + listener.bind(("127.0.0.1", port)) + listener.listen(1) + client = socket.create_connection(("127.0.0.1", port)) + conn, _ = listener.accept() + conn.close() + client.close() + listener.close() + + +def plain_bind_refuses(port: int) -> bool: + """Would the old probe — a bind with no SO_REUSEADDR — call this port + busy? False means the platform does not reproduce the bug at all, and + the test says so rather than passing on nothing.""" + s = socket.socket() + try: + s.bind(("127.0.0.1", port)) + return False + except OSError: + return True + finally: + s.close() + + +class Holder: + """Something on the port that is not this project's board: an HTTP + server that answers /api/state with `root` (None = answers nothing the + probe recognises, i.e. a stranger).""" + + def __init__(self, port: int, root: str | None = None): + payload = json.dumps({"board": {"root": root}}).encode() + + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + if self.path == "/api/state" and root is not None: + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + else: + self.send_error(404) + + def log_message(self, *args): + pass + + self.server = ThreadingHTTPServer(("127.0.0.1", port), Handler) + threading.Thread(target=self.server.serve_forever, daemon=True).start() + + def stop(self): + self.server.shutdown() + self.server.server_close() + + +class PinnedPort(unittest.TestCase): + def setUp(self): + self.scratch = Path(tempfile.mkdtemp()).resolve() + self.addCleanup(shutil.rmtree, self.scratch, True) + self.port = free_port(2) + self.tm = make_host(self.scratch, self.port) + self.env_file = self.tm / "manager" / "local" / ".env" + self.env_before = self.env_file.read_text(encoding="utf-8") + + def env_values(self) -> dict: + return dict( + line.split("=", 1) + for line in self.env_file.read_text(encoding="utf-8").splitlines() + if "=" in line and not line.lstrip().startswith("#")) + + def test_a_socket_in_time_wait_does_not_move_the_pin(self): + """The reported bug: stop the board, start it again, and the socket + its own shutdown left behind sends it to the next port.""" + leave_time_wait(self.port) + if not plain_bind_refuses(self.port): + self.skipTest("this platform lets a plain bind reuse TIME_WAIT") + result = run_start(self.tm) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertEqual(launched(self.tm), ["--port", str(self.port)]) + self.assertEqual(self.env_file.read_text(encoding="utf-8"), + self.env_before) + self.assertNotIn("Rewriting BOARD_PORT", result.stdout) + self.assertNotIn("is busy", result.stdout) + + def test_the_probe_agrees_with_the_server_it_probes_for(self): + """The root cause, on its own: start.sh's is_free must call a port + free exactly when the board could bind it — free over TIME_WAIT + (ThreadingHTTPServer sets SO_REUSEADDR), busy under a listener.""" + text = (REPO / "start.sh").read_text(encoding="utf-8") + probe = (text.split('python3 - "$1" <<\'PY\'\n', 1)[1] + .split("\nPY\n", 1)[0]) + leave_time_wait(self.port) + if not plain_bind_refuses(self.port): + self.skipTest("this platform lets a plain bind reuse TIME_WAIT") + + def is_free(port: int) -> bool: + return subprocess.run([sys.executable, "-c", probe, str(port)], + capture_output=True).returncode == 0 + + self.assertTrue(is_free(self.port), "TIME_WAIT read as occupied") + holder = Holder(self.port + 1) + self.addCleanup(holder.stop) + self.assertFalse(is_free(self.port + 1), "a listener read as free") + + def test_a_holder_that_lets_go_during_the_wait_keeps_the_pin(self): + """A predecessor still shutting down when the new start probes: the + brief retry is what keeps the restart on its own port.""" + holder = Holder(self.port) + timer = threading.Timer(1.5, holder.stop) + timer.start() + self.addCleanup(timer.cancel) + result = run_start(self.tm, wait="20") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn(f"Port {self.port} is busy — waiting", result.stdout) + self.assertEqual(launched(self.tm), ["--port", str(self.port)]) + self.assertEqual(self.env_file.read_text(encoding="utf-8"), + self.env_before) + + def test_a_foreign_holder_makes_it_wait_then_walk_and_say_so(self): + """Genuinely occupied: the hop is right — the hooks and agents read + BOARD_PORT and must reach the live board — but it has to be said in + full, since it overwrites something the user chose.""" + holder = Holder(self.port) + self.addCleanup(holder.stop) + result = run_start(self.tm, wait="2") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + moved = self.port + 1 + self.assertIn(f"Port {self.port} is busy — waiting up to 2s", + result.stdout) + self.assertIn(f"Port {self.port} is held by another process — " + f"using {moved} instead.", result.stdout) + self.assertIn(f"manager/local/.env: {self.port} → {moved}", + result.stdout) + self.assertIn(f"To reclaim {self.port}", result.stdout) + self.assertIn(f"set BOARD_PORT={self.port} in manager/local/.env", + result.stdout) + self.assertNotIn("to manager/.env", result.stdout) + self.assertEqual(launched(self.tm), ["--port", str(moved)]) + values = self.env_values() + self.assertEqual(values["BOARD_PORT"], str(moved)) + self.assertEqual(values["BOARD_WATCH_INTERVAL"], "2") + self.assertIn("# Seconds between disk polls of the stage directories.", + self.env_file.read_text(encoding="utf-8")) + + def test_our_own_board_short_circuits_before_any_probe(self): + """A board of ours already answering there is the one case that + neither probes nor hops: it opens the tab and stops.""" + holder = Holder(self.port, root=str(self.tm / "tasks")) + self.addCleanup(holder.stop) + result = run_start(self.tm) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + self.assertIn(f"Board already running at http://127.0.0.1:{self.port}/", + result.stdout) + self.assertIsNone(launched(self.tm)) + self.assertNotIn("is busy", result.stdout) + self.assertEqual(self.env_file.read_text(encoding="utf-8"), + self.env_before) + + +if __name__ == "__main__": + unittest.main()