From f950cfbf6c518991c7ca36c447343bc783cfbfc1 Mon Sep 17 00:00:00 2001
From: stroblme <stroblme@posteo.de>
Date: Wed, 16 Sep 2026 16:47:11 +0200
Subject: [PATCH] Bind serve's socket before writing client.json

A serve that lost the port race had already signed in, so client.json
named a port another engine held and every client talked to that one.
The socket is now bound first and handed to uvicorn.
---
 backend/fluksio/cli.py    | 48 +++++++++++++++++++++++++++++----------
 backend/tests/test_cli.py | 32 ++++++++++++++++++++++++++
 docs/code/cli.md          |  3 ++-
 3 files changed, 70 insertions(+), 13 deletions(-)

diff --git a/backend/fluksio/cli.py b/backend/fluksio/cli.py
index 910e9fd..c0466de 100644
--- a/backend/fluksio/cli.py
+++ b/backend/fluksio/cli.py
@@ -495,8 +495,7 @@ def cmd_serve(args: argparse.Namespace) -> int:
 
     data_dir = _data_dir(args.data_dir, args.shared)
     # Before `_prepare`, so a refusal never migrates the database another
-    # engine is serving out of, and before `_sign_in`, so the credential keeps
-    # pointing at the engine that is actually up.
+    # engine is serving out of.
     where = already_serving(data_dir, args.host)
     if where:
         _say(f"An engine for {data_dir} is already serving at {where}.")
@@ -565,6 +564,26 @@ def cmd_serve(args: argparse.Namespace) -> int:
             else:
                 _say(f"Port {DEFAULT_PORT} is in use; serving on {port} instead.")
 
+    # Bound before the credential is written, so an engine that cannot listen
+    # leaves `client.json` naming the engine that is actually up rather than a
+    # port somebody else holds.
+    server = uvicorn.Server(
+        uvicorn.Config(
+            app,
+            host=args.host,
+            port=port,
+            log_level=args.log_level,
+            log_config=_log_config(args.log_level),
+        )
+    )
+    try:
+        sock = server.config.bind_socket()
+    except SystemExit:
+        # uvicorn has logged why, and exits rather than raising.
+        if args.port is not None:
+            _say(f"  Port {port} was asked for; leave --port off to take a free one.")
+        return 1
+
     url = f"http://{reachable}:{port}"
     token_path = _sign_in(admin_id, url, data_dir)
 
@@ -600,19 +619,24 @@ def cmd_serve(args: argparse.Namespace) -> int:
     if _appended_log():
         # Started by the dashboard, with `serve.log` as this process's stdout.
         _watch_log()
-    _hold_pidfile(
-        pidfile,
-        lambda: uvicorn.run(
-            app,
-            host=args.host,
-            port=port,
-            log_level=args.log_level,
-            log_config=_log_config(args.log_level),
-        ),
-    )
+    _hold_pidfile(pidfile, lambda: _serve_on(server, sock))
     return 0
 
 
+def _serve_on(server: Any, sock: socket.socket) -> None:
+    """What `uvicorn.run` does for a single process, on a socket already bound."""
+    from uvicorn.main import STARTUP_FAILURE
+
+    try:
+        server.run(sockets=[sock])
+    except KeyboardInterrupt:
+        pass
+    if not server.started:
+        # The app never came up — a lifespan that raised — and the exit code
+        # says so, as it did when uvicorn ran the whole thing.
+        sys.exit(STARTUP_FAILURE)
+
+
 def _log_config(level: str) -> dict[str, Any]:
     """Uvicorn's logging, with the engine's own loggers drawn the same way.
 
diff --git a/backend/tests/test_cli.py b/backend/tests/test_cli.py
index 1ede9f5..cb74c49 100644
--- a/backend/tests/test_cli.py
+++ b/backend/tests/test_cli.py
@@ -935,6 +935,38 @@ def test_serve_moves_off_a_port_that_is_taken() -> None:
     assert DEFAULT_PORT == 8000
 
 
+def test_a_port_that_cannot_be_bound_writes_no_credential(tmp_path, monkeypatch):
+    """The token is written once the socket is ours, not on the way to it.
+
+    Written first, a failed start left `client.json` naming a port another
+    project's engine holds — and every client here then talked to that one.
+    """
+    import socket
+    from types import SimpleNamespace
+
+    from fluksio import cli
+    from fluksio.core import bootstrap
+
+    admin = SimpleNamespace(id="admin", email="admin@example.com")
+    monkeypatch.setattr(cli, "_prepare", lambda data_dir: None)
+    monkeypatch.setattr(bootstrap, "ensure_superuser", lambda *a, **k: (admin, None))
+    # uvicorn's logging config would otherwise outlive this test.
+    monkeypatch.setattr(cli, "_log_config", lambda level: None)
+
+    data_dir = tmp_path / ".fluksio"
+    with socket.socket() as held:
+        held.bind(("127.0.0.1", 0))
+        held.listen(1)
+        taken = held.getsockname()[1]
+        args = cli._parser().parse_args(
+            ["serve", "--plain", "--port", str(taken), "--data-dir", str(data_dir)]
+        )
+        assert cli.cmd_serve(args) == 1
+
+    assert not (data_dir / "client.json").exists()
+    assert not (data_dir / cli.PIDFILE).exists()
+
+
 def test_a_duration_reads_the_way_an_age_does() -> None:
     """One notation for both, rather than one per place that printed one."""
     from fluksio.sdk.cli import _dur
diff --git a/docs/code/cli.md b/docs/code/cli.md
index 6f5bb2b..488096a 100644
--- a/docs/code/cli.md
+++ b/docs/code/cli.md
@@ -53,7 +53,8 @@ The default port moves out of the way when something already has it (8001,
 8002, and so on) and says which one it took; the URL written to
 `client.json` is the one it is actually on. A port you *asked* for is never
 moved off: `--port 9000` on a taken 9000 fails, because something else is
-there and you named it.
+there and you named it. It fails before `client.json` is written, so every
+client keeps talking to the engine that is actually up.
 
 What it will *not* do is start a second engine for the same instance. Before
 anything else, `serve` looks for an engine already serving this directory —