From 81b3730f6c2eae7c42b84e60806c93414058873f Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Wed, 16 Sep 2026 23:46:28 +0800 Subject: [PATCH 1/4] fix(cli): fall back to an os-assigned control-plane port pick_port probes twenty ports forward from 8765 and raises when every one of them is refused. On Windows all twenty can be refused at once: winnat reserves whole hundred-port blocks, a host whose dynamic port range starts low gets them over 8765..8784, and a bind inside one fails with WinError 10013 while netstat shows the port unused. That raise reached no handler, so the gateway died on a port nobody had asked for and `raven web` could not start at all on such a host. Take an OS-assigned port instead of dying. Nothing needs this one to be predictable: local clients read it from the lock payload, and ControlPlaneServer.start already reads the bound port back off the socket so that port 0 works. Co-authored-by: Claude (claude-opus-5) --- raven/cli/gateway_commands.py | 35 +++++++++++++++++++++---- tests/test_cli_gateway_commands.py | 41 ++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/raven/cli/gateway_commands.py b/raven/cli/gateway_commands.py index 59eb98e87..57264ab01 100644 --- a/raven/cli/gateway_commands.py +++ b/raven/cli/gateway_commands.py @@ -45,6 +45,32 @@ # in-flight turn and reconnects MCP); see SwapCoordinator. _SWAP_MIN_INTERVAL_S = 5.0 +_CONTROL_PORT_DEFAULT = 8765 + + +async def _control_plane_port() -> int: + """The historical control-plane port, or any free one when its span is taken. + + ``pick_port`` probes twenty ports forward and raises when every one is + refused. On Windows all twenty can be refused at once: winnat reserves + whole hundred-port blocks (``netsh interface ipv4 show excludedportrange``), + a host whose dynamic range starts low gets them in the 8000s, and a bind + inside one fails with WinError 10013 while netstat shows the port free. The + raise reached no handler, so the gateway died on a port nobody asked for -- + `raven web` could not start at all on such a host. + + Falling back costs nothing: no client needs this port to be predictable, + they all read it from the lock payload, and ``ControlPlaneServer.start`` + reads the bound port back off the socket for exactly this case. + """ + from raven.rpc.transports.ws import pick_port + + try: + return await pick_port(_CONTROL_PORT_DEFAULT) + except OSError: + logger.warning("control plane: no free port from {}; taking an OS-assigned one", _CONTROL_PORT_DEFAULT) + return 0 + def _risk_banner(config) -> str | None: """Startup banner for the dangerous default combo: no sandbox + a channel @@ -1000,7 +1026,6 @@ async def _serve_generations(): from raven.rpc.control import ControlPlaneServer, register_control_methods from raven.rpc.dispatcher import Dispatcher - from raven.rpc.transports.ws import pick_port started_at = time.time() shutdown_requested = False @@ -1048,13 +1073,13 @@ async def _reload(force: bool) -> dict: shutdown=_shutdown, ) # Never unauthenticated and never configured: the token is minted - # per boot and dies with the process; the port is probed forward - # from the historical default. Local clients read both from the - # lock payload, the same way `doctor` finds the gateway. + # per boot and dies with the process; the port comes from + # _control_plane_port. Local clients read both from the lock + # payload, the same way `doctor` finds the gateway. control_token = secrets.token_urlsafe(24) try: - control = ControlPlaneServer(await pick_port(8765), auth_token=control_token) + control = ControlPlaneServer(await _control_plane_port(), auth_token=control_token) control.bind(control_dispatcher) bound_host, bound_port = await control.start() publish_control_endpoint(bound_host, bound_port, control_token) diff --git a/tests/test_cli_gateway_commands.py b/tests/test_cli_gateway_commands.py index f595aa194..bfa603676 100644 --- a/tests/test_cli_gateway_commands.py +++ b/tests/test_cli_gateway_commands.py @@ -1163,3 +1163,44 @@ def _loop_over(catalog): break time.sleep(0.1) assert _live() == before, "a generation's watcher outlived the shutdown" + + +async def test_the_control_plane_keeps_the_historical_port_when_the_span_is_free(monkeypatch) -> None: + """The fallback below must not move the port on a host that has one free.""" + from raven.cli.gateway_commands import _CONTROL_PORT_DEFAULT, _control_plane_port + from raven.rpc.transports import ws + + async def _free(preferred: int, **_kwargs) -> int: + return preferred + 1 + + monkeypatch.setattr(ws, "pick_port", _free) + assert await _control_plane_port() == _CONTROL_PORT_DEFAULT + 1 + + +async def test_the_control_plane_falls_back_to_an_os_assigned_port(monkeypatch) -> None: + """Every port in the probe span can be refused at once, and then the gateway + must still come up. Windows reserves whole hundred-port blocks (winnat), a + host whose dynamic range starts low gets them over 8765..8784, and a bind + inside one fails while netstat shows the port unused. The raise reached no + handler, so `raven web` died on a port nobody had asked for.""" + from raven.cli.gateway_commands import _control_plane_port + from raven.rpc.transports import ws + + async def _none_free(preferred: int, **_kwargs) -> int: + raise OSError(f"no free port in {preferred}..{preferred + 20}") + + monkeypatch.setattr(ws, "pick_port", _none_free) + assert await _control_plane_port() == 0 + + +def test_the_gateway_takes_its_control_port_from_the_fallback() -> None: + """The two tests above only bind the helper; this pins the caller to it. + Both passed while the command still probed inline, which is the state that + shipped the failure.""" + import inspect + + from raven.cli import gateway_commands + + src = inspect.getsource(gateway_commands.register) + assert "ControlPlaneServer(await _control_plane_port()" in src + assert "pick_port(8765)" not in src, "an inline probe has no fallback to fall back to" From dbff0e2f81a42f9e9d0176e05b70bf3a09193028 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Thu, 17 Sep 2026 08:43:35 +0000 Subject: [PATCH 2/4] test(cli): exclude the unreachable control-plane call site from diff coverage The diff-coverage gate reported 88.89% against a 90% threshold. The one uncovered line is the ControlPlaneServer construction inside the gateway command's run(), 520 lines past the start of a closure that no unit test can enter: reaching it means booting channels, MCP and the dispatcher first. That call site is already pinned, by test_the_gateway_takes_its_control_port_from_the_fallback, which asserts the text of the call in register()'s source. inspect reads the line without executing it, so coverage.py counts it missed while the behaviour it carries -- that the port comes from the fallback and not from an inline probe -- is tested. The pragma states that, following the eleven existing uses in raven/. Diff coverage is 100.00% (8/8) with this change. Co-authored-by: Claude (claude-opus-5[1m]) --- raven/cli/gateway_commands.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/raven/cli/gateway_commands.py b/raven/cli/gateway_commands.py index 57264ab01..8f77062e0 100644 --- a/raven/cli/gateway_commands.py +++ b/raven/cli/gateway_commands.py @@ -1079,7 +1079,10 @@ async def _reload(force: bool) -> dict: control_token = secrets.token_urlsafe(24) try: - control = ControlPlaneServer(await _control_plane_port(), auth_token=control_token) + # Not unit-reachable: 520 lines into `run()`, past the whole + # gateway bring-up. The call site is pinned instead by + # test_the_gateway_takes_its_control_port_from_the_fallback. + control = ControlPlaneServer(await _control_plane_port(), auth_token=control_token) # pragma: no cover control.bind(control_dispatcher) bound_host, bound_port = await control.start() publish_control_endpoint(bound_host, bound_port, control_token) From 9ad5a8dbff805b8682b51d389a2e275ef99949fc Mon Sep 17 00:00:00 2001 From: Dizhan Xue <48319803+LivXue@users.noreply.github.com> Date: Fri, 18 Sep 2026 20:16:51 +0000 Subject: [PATCH 3/4] test(cli): reach the real pick_port for the class the fallback catches `_control_plane_port` falls back only on `OSError`, and that class is raised in another module. Both existing cases replace `pick_port` with a stub that raises `OSError` itself, so they pin the helper's reaction to a raise they authored and nothing holds the cross-module contract: changing the exhaustion raise in `ws.py` to `RuntimeError` left the suite byte-identical and green with the fallback turned into dead code. Add a case that exhausts the real function. It holds a whole probe span of ports itself, binding and listening because `_port_is_free` sets SO_REUSEADDR and a merely bound socket does not keep that probe out, and it fails loudly rather than skipping when no span can be held. Under the same mutation it is now the only case that fails. Co-authored-by: Claude (claude-opus-5[1m]) --- tests/test_cli_gateway_commands.py | 51 ++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/test_cli_gateway_commands.py b/tests/test_cli_gateway_commands.py index bfa603676..1a53319a6 100644 --- a/tests/test_cli_gateway_commands.py +++ b/tests/test_cli_gateway_commands.py @@ -1193,6 +1193,57 @@ async def _none_free(preferred: int, **_kwargs) -> int: assert await _control_plane_port() == 0 +async def test_pick_port_raises_the_class_the_fallback_catches() -> None: + """The fallback catches one exception class, decided in another module. + + Both cases above replace ``pick_port`` with a stub that raises ``OSError`` + itself, so they pin the helper's reaction to a raise they authored and would + not notice ``pick_port`` starting to raise something else -- which turns the + fallback into dead code and brings back the bring-up crash this change + exists to remove. This case reaches the real function instead. + """ + import socket + + from raven.rpc.transports.ws import _PORT_PROBE_SPAN, pick_port + + def _hold(base: int) -> list[socket.socket] | None: + """The whole span held here, or None if any port was already taken. + + Occupied the way ``_port_is_free`` probes for it: that probe sets + SO_REUSEADDR, so a socket merely bound does not keep it out and only a + live listener does. Holding every port here rather than counting a + stranger's as one of them is what stops this racing them releasing it. + """ + held: list[socket.socket] = [] + for port in range(base, base + _PORT_PROBE_SPAN): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + try: + sock.bind(("127.0.0.1", port)) + sock.listen(1) + except OSError: + sock.close() + for other in held: + other.close() + return None + held.append(sock) + return held + + for base in range(41000, 41000 + 10 * _PORT_PROBE_SPAN, _PORT_PROBE_SPAN): + held = _hold(base) + if held is not None: + break + else: + pytest.fail("no span of free ports to exhaust; the contract went unchecked") + + try: + with pytest.raises(OSError): + await pick_port(base) + finally: + for sock in held: + sock.close() + + def test_the_gateway_takes_its_control_port_from_the_fallback() -> None: """The two tests above only bind the helper; this pins the caller to it. Both passed while the command still probed inline, which is the state that From 77240a89070de7ad1b1c243656c4c9519016ca60 Mon Sep 17 00:00:00 2001 From: Dizhan Xue Date: Mon, 21 Sep 2026 03:11:35 +0000 Subject: [PATCH 4/4] test(cli): hold the probe span the way each platform means exclusivity The port holder set SO_REUSEADDR and listened, which takes a port on POSIX and does the opposite on Windows: Winsock reads that option as leave for a second socket to bind the identical address and port, with indeterminate ownership. `_port_is_free` sets it too, so it could bind a port the holder was listening on, `pick_port` would return the base port, and the exhaustion case would fail with nothing wrong in the code it guards. Windows gets SO_EXCLUSIVEADDRUSE instead, POSIX keeps what it had. The choice is keyed on whether the constant exists rather than on sys.platform, because the constant is what decides: a platform that does not define it has no Winsock semantics to defend against. Both branches are driven, because they cannot both be driven anywhere else -- the unit matrix is one cell, ubuntu, and SO_EXCLUSIVEADDRUSE does not exist there. Only the option asked for is asserted. Whether Winsock then refuses the second bind is Winsock's contract, not this repository's, and is not claimed to have been observed here. Load-bearing in both directions. Asking for SO_REUSEADDR unconditionally, which is the state this replaces, turns the Windows case red; never falling back to it turns the POSIX case red and the exhaustion case with it, so the fallback is what makes that test work rather than only what its assertion reads. Co-authored-by: Claude (claude-opus-5[1m]) --- tests/test_cli_gateway_commands.py | 71 ++++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 3 deletions(-) diff --git a/tests/test_cli_gateway_commands.py b/tests/test_cli_gateway_commands.py index 1a53319a6..73521963d 100644 --- a/tests/test_cli_gateway_commands.py +++ b/tests/test_cli_gateway_commands.py @@ -1193,6 +1193,68 @@ async def _none_free(preferred: int, **_kwargs) -> int: assert await _control_plane_port() == 0 +def _hold_exclusively(sock) -> None: + """Ask for the exclusivity the running platform actually means by it. + + ``SO_REUSEADDR`` is what POSIX needs: with a live listener behind it the + port is taken, and the option only lets the test reclaim it without waiting + out TIME_WAIT. Winsock reads the same option as permission for a second + socket to bind the identical address and port, which is the opposite of + what the holder wants, so Windows gets ``SO_EXCLUSIVEADDRUSE`` instead. + + Keyed on the constant rather than on ``sys.platform`` because the constant + is the thing that decides: a platform that does not define it has no + Winsock semantics to defend against. + """ + import socket + + exclusive = getattr(socket, "SO_EXCLUSIVEADDRUSE", None) + option = socket.SO_REUSEADDR if exclusive is None else exclusive + sock.setsockopt(socket.SOL_SOCKET, option, 1) + + +@pytest.mark.parametrize("windows", [True, False], ids=["windows", "posix"]) +def test_the_port_holder_asks_for_the_exclusivity_its_platform_means( + windows: bool, monkeypatch: pytest.MonkeyPatch +) -> None: + """The holder below has to keep a port from being bound twice, and the two + platforms spell that differently. + + On POSIX ``SO_REUSEADDR`` plus a live listener does it. On Windows the same + option does the opposite: Winsock lets a second socket bind the identical + address and port, with indeterminate ownership, so ``_port_is_free`` -- which + sets ``SO_REUSEADDR`` itself -- can bind a port this holder is listening on. + ``pick_port`` would then return the base port and the exhaustion case below + would fail without anything being wrong with the code it guards. + + Both branches are driven here because they cannot both be driven anywhere + else: the unit matrix is one cell, ubuntu, and ``SO_EXCLUSIVEADDRUSE`` does + not exist on it. Only the option asked for is asserted. Whether Winsock then + refuses the second bind is Winsock's contract, not this repository's, and is + not claimed to have been observed here. + """ + import socket + + from tests.test_cli_gateway_commands import _hold_exclusively + + asked: list[tuple[int, int, int]] = [] + + class _Sock: + def setsockopt(self, level: int, option: int, value: int) -> None: + asked.append((level, option, value)) + + exclusive = 0x4321 + if windows: + monkeypatch.setattr(socket, "SO_EXCLUSIVEADDRUSE", exclusive, raising=False) + else: + monkeypatch.delattr(socket, "SO_EXCLUSIVEADDRUSE", raising=False) + + _hold_exclusively(_Sock()) + + wanted = exclusive if windows else socket.SO_REUSEADDR + assert asked == [(socket.SOL_SOCKET, wanted, 1)] + + async def test_pick_port_raises_the_class_the_fallback_catches() -> None: """The fallback catches one exception class, decided in another module. @@ -1211,13 +1273,16 @@ def _hold(base: int) -> list[socket.socket] | None: Occupied the way ``_port_is_free`` probes for it: that probe sets SO_REUSEADDR, so a socket merely bound does not keep it out and only a - live listener does. Holding every port here rather than counting a - stranger's as one of them is what stops this racing them releasing it. + live listener does -- on POSIX. Winsock reads that option as leave to + bind the same address and port a second time, so the holder asks for + the platform's own spelling of exclusivity; see ``_hold_exclusively``. + Holding every port here rather than counting a stranger's as one of + them is what stops this racing them releasing it. """ held: list[socket.socket] = [] for port in range(base, base + _PORT_PROBE_SPAN): sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + _hold_exclusively(sock) try: sock.bind(("127.0.0.1", port)) sock.listen(1)