fix: run the native preflights before anything is written

The Docker-stack check ran after logs/ was created, .env written and the
migrations applied, so a conflict left a migrated database and partial files
behind with no install.json — exactly the directory that down and uninstall
then refuse. It runs immediately after the port is resolved now.

Alongside it, `up --native` refuses a port it cannot bind. Neither service
manager confirms that the API bound, and /api/health carries no installation
identity, so a second install on the same port would have been answered by the
first and reported success while its own API was dead. An install re-running on
its own port is the exception, since its services are what hold it.
This commit is contained in:
Alex committed 2026-09-17 00:03:06 +01:00
1 parent 4976b3104c
commit 217310e201
2 files changed
+62 -3

No files matched your search

+33 -3
View File
@@ -8,6 +8,7 @@ import os
import re
import secrets
import shutil
import socket
import subprocess
import sys
import tempfile
@@ -284,6 +285,33 @@ def _service_names(directory: Path) -> tuple[str, str]:
return native.service_names(directory, stack.stack_dir(None))
def _port_is_free(port: int) -> bool:
"""Whether the loopback port can still be bound."""
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as probe:
try:
probe.bind(("127.0.0.1", port))
except OSError:
return False
return True
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.
"""
if _port_is_free(port):
return
if _record(directory).get("mode") == "native" and any(services.is_running(name) for name in names):
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."
)
def _refuse_other_docker_stack(context: Context, directory: Path, port: int) -> None:
"""A Docker stack elsewhere on this port would answer the health check the native API failed."""
try:
@@ -340,6 +368,11 @@ def _native_up(args, context: Context, directory: Path) -> int:
port = _port_number(existing["DOCSGPT_PORT"], f"DOCSGPT_PORT in {env_path}")
else:
port = stack.DEFAULT_PORT
names = _service_names(directory)
_refuse_other_docker_stack(context, directory, port)
_refuse_busy_port(directory, port, services, names)
updates: dict[str, Optional[str]] = {
"POSTGRES_URI": postgres,
"API_URL": f"http://127.0.0.1:{port}",
@@ -369,8 +402,6 @@ def _native_up(args, context: Context, directory: Path) -> int:
if context.run([*launcher, "migrate"], stack_env) != 0:
raise DeployError("`docsgpt migrate` failed; check the database URL and that the server is reachable.")
_refuse_other_docker_stack(context, directory, port)
# The record goes in before the services: if one fails to start, this is still a native install
# that status, down and uninstall can see and clean up, rather than orphaned units.
now = datetime.now(timezone.utc).isoformat(timespec="seconds")
@@ -378,7 +409,6 @@ def _native_up(args, context: Context, directory: Path) -> int:
record.update(version=context.version, mode="native", updated_at=now)
(directory / stack.RECORD_FILE).write_text(json.dumps(record, indent=2) + "\n", encoding="utf-8")
names = _service_names(directory)
for unit in native.units_for(directory, launcher, port, directory, names):
services.install(unit)
for name in names:
+29
View File
@@ -2,6 +2,7 @@
import json
import plistlib
import socket
import sys
import types
from pathlib import Path
@@ -273,9 +274,37 @@ class TestNativeUp:
directory = tmp_path / "native"
argv = ["up", "--native", "--dir", str(directory), "--yes", "--port", "7099",
"--postgres-uri", "postgresql://localhost/d"]
migrated = []
context.run = lambda args, env=None: migrated.append(args) or 0
with pytest.raises(DeployError, match="already runs a DocsGPT stack"):
_run(argv, context)
assert not (directory / "install.json").exists(), "it refuses before recording the install"
assert not (directory / ".env").exists(), "and before writing any settings"
assert not (directory / "logs").exists()
assert migrated == [], "and before touching the database"
def test_a_port_something_else_holds_is_refused(self, tmp_path):
"""Nothing confirms the API bound its port, and a generic health check answers from the holder."""
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as held:
held.bind(("127.0.0.1", 0))
held.listen(1)
port = held.getsockname()[1]
argv = ["up", "--native", "--dir", str(tmp_path), "--yes", "--port", str(port),
"--postgres-uri", "postgresql://localhost/d"]
with pytest.raises(DeployError, match="already in use"):
_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."""
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
def test_a_database_url_is_required(self, tmp_path):
with pytest.raises(DeployError, match="--postgres-uri"):