fix: tie the busy-port exemption to the port the install is recorded on

The exemption asked whether any service of the install was running, so moving an
install onto a different port that something else held would pass the check and
then fail to bind, with the health poll answered by whatever owned that port —
the false success the check exists to prevent.

install.json carries the API port now, and a busy port is allowed only when it
is that port and the API service is running.
This commit is contained in:
Alex committed 2026-09-17 00:12:40 +01:00
1 parent 217310e201
commit 0d153d3c0d
2 files changed
+37 -11

No files matched your search

+16 -7
View File
@@ -298,17 +298,26 @@ def _port_is_free(port: int) -> bool:
def _refuse_busy_port(directory: Path, port: int, services, names: tuple[str, str]) -> None:
"""Whatever holds the port would answer the health check while these services failed to bind.
An install re-running on its own port is the exception: its services are holding it, and
starting them again replaces them.
The one exception is this install's own API holding the port it is recorded on. Ownership is not
inferred from the install having some service running: asking for a different port that something
else holds is how a move to a new port would look, and the API would fail to bind it.
"""
if _port_is_free(port):
return
if _record(directory).get("mode") == "native" and any(services.is_running(name) for name in names):
record = _record(directory)
api_service = names[0]
owns_the_port = (
record.get("mode") == "native"
and str(record.get("port") or "") == str(port)
and services.is_running(api_service)
)
if owns_the_port:
return
raise DeployError(
f"port {port} is already in use by something other than this install. The API would fail to "
f"bind it while the health check answered from whatever holds it, so the install would look "
f"healthy and be dead. Free the port, or give this install another one with --port."
f"port {port} is already in use by something other than this install's API. It would fail to "
f"bind while the health check answered from whatever holds the port, so the install would "
f"look healthy and be dead. Free the port, stop this install first with `docsgpt down` if it "
f"is the one holding it on another port, or choose another port with --port."
)
@@ -406,7 +415,7 @@ def _native_up(args, context: Context, directory: Path) -> int:
# that status, down and uninstall can see and clean up, rather than orphaned units.
now = datetime.now(timezone.utc).isoformat(timespec="seconds")
record = record or {"installed_at": now}
record.update(version=context.version, mode="native", updated_at=now)
record.update(version=context.version, mode="native", port=port, updated_at=now)
(directory / stack.RECORD_FILE).write_text(json.dumps(record, indent=2) + "\n", encoding="utf-8")
for unit in native.units_for(directory, launcher, port, directory, names):
+21 -4
View File
@@ -295,16 +295,33 @@ class TestNativeUp:
_run(argv, _native_context())
assert not (tmp_path / ".env").exists(), "it refuses before writing anything"
def test_an_install_may_keep_the_port_its_own_services_hold(self, tmp_path):
"""Re-running an install must not trip over the services it is about to replace."""
def test_an_install_may_keep_the_port_its_own_api_is_recorded_on(self, tmp_path):
"""Re-running an install must not trip over the API it is about to replace."""
services = FakeServices()
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe:
probe.bind(("127.0.0.1", 0))
port = probe.getsockname()[1]
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--port", str(port),
"--postgres-uri", "postgresql://localhost/d"]
assert _run(argv, _native_context(services)) == 0
assert json.loads((tmp_path / "install.json").read_text())["port"] == port
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as held:
held.bind(("127.0.0.1", port))
held.listen(1)
assert _run(["up", "--dir", str(tmp_path), "--yes"], _native_context(services)) == 0
def test_moving_an_install_onto_a_busy_port_is_refused(self, tmp_path):
"""Ownership is of one port, not of any port while some service of the install runs."""
services = FakeServices()
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--postgres-uri", "postgresql://localhost/d"]
assert _run(argv, _native_context(services)) == 0
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as held:
held.bind(("127.0.0.1", 0))
held.listen(1)
envfile.update(tmp_path / ".env", {"DOCSGPT_PORT": str(held.getsockname()[1])})
assert _run(["up", "--dir", str(tmp_path), "--yes"], _native_context(services)) == 0
elsewhere = held.getsockname()[1]
with pytest.raises(DeployError, match="already in use"):
_run(["up", "--dir", str(tmp_path), "--yes", "--port", str(elsewhere)], _native_context(services))
def test_a_database_url_is_required(self, tmp_path):
with pytest.raises(DeployError, match="--postgres-uri"):