fix(orchestrator): only recreate the orchestrator on a real source change

ensure_running() unconditionally recreated the bot-bottle-orchestrator
container on every bottle launch, added so bind-mounted code changes take
effect (the process never reloads). But the orchestrator's per-bottle
egress auth tokens live only in its process memory (Orchestrator._tokens),
by design never persisted to disk. So starting a second bottle recreated
the container and silently dropped the FIRST bottle's already-injected
token -- its next authed egress call 403s with "env var EGRESS_TOKEN_0 is
unset", and it needs a full restart to recover.

Now the orchestrator container is labeled with a content hash of its
bind-mounted bot_bottle source, mirroring the gateway's existing
image-staleness check. ensure_running only recreates on a hash mismatch
(a real code change), so a bottle launch that isn't accompanied by a code
change leaves a healthy orchestrator -- and every other active bottle's
in-memory tokens -- alone.

Fixes #381.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

bot_bottle/orchestrator/lifecycle.py

@@ -16,6 +16,7 @@ names + the published port).
 
 from __future__ import annotations
 
+import hashlib
 import time
 import urllib.error
 import urllib.request
@@ -29,6 +30,10 @@ from .gateway import GATEWAY_IMAGE, GATEWAY_NETWORK, DockerGateway
 DEFAULT_PORT = 8099
 ORCHESTRATOR_NAME = "bot-bottle-orchestrator"
 ORCHESTRATOR_LABEL = "bot-bottle-orchestrator=1"
+# Baked onto the container as a label so `ensure_running` can tell whether the
+# running process is executing the *current* bind-mounted source — see
+# `_source_hash`.
+ORCHESTRATOR_SOURCE_HASH_LABEL = "bot-bottle-orchestrator-source-hash"
 
 # The repo root is bind-mounted into the control-plane container so
 # `python -m bot_bottle.orchestrator` resolves the package (the orchestrator
@@ -46,6 +51,22 @@ class OrchestratorStartError(RuntimeError):
     """The orchestrator container did not become healthy within the timeout."""
 
 
+def _source_hash(repo_root: Path) -> str:
+    """Content hash of the orchestrator's bind-mounted Python source (the
+    `bot_bottle` package the control-plane process imports). This only
+    changes when the code that would actually run inside the container
+    changes — `ensure_running` recreates the container on a mismatch and
+    otherwise leaves a healthy one alone, so a bottle launch that isn't
+    accompanied by a code change doesn't restart the process and drop every
+    *other* active bottle's in-memory egress tokens (`Orchestrator._tokens`
+    in `service.py`, never persisted to disk by design)."""
+    h = hashlib.sha256()
+    for path in sorted((repo_root / "bot_bottle").rglob("*.py")):
+        h.update(str(path.relative_to(repo_root)).encode())
+        h.update(path.read_bytes())
+    return h.hexdigest()
+
+
 class OrchestratorService:
     """Manages the orchestrator control-plane container + the shared gateway.
     Callers only need `ensure_running()` + `url`."""
@@ -88,14 +109,17 @@ class OrchestratorService:
         proc = run_docker(["docker", "ps", "--filter", f"name=^/{name}$", "--format", "{{.Names}}"])
         return name in proc.stdout.split()
 
-    def _run_orchestrator_container(self) -> None:
+    def _run_orchestrator_container(self, source_hash: str) -> None:
         """Start the control-plane container (idempotent: clears a stale
-        fixed-name container first). Register-only broker → no docker socket."""
+        fixed-name container first). Register-only broker → no docker socket.
+        Labels the container with `source_hash` so a later `ensure_running`
+        can detect a real code change (see `_source_hash`)."""
         run_docker(["docker", "rm", "--force", ORCHESTRATOR_NAME])
         proc = run_docker([
             "docker", "run", "--detach",
             "--name", ORCHESTRATOR_NAME,
             "--label", ORCHESTRATOR_LABEL,
+            "--label", f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={source_hash}",
             "--network", self.network,
             # Host CLI reaches the control plane here; bound to loopback so it
             # is not exposed on the host's external interfaces.
@@ -119,26 +143,46 @@ class OrchestratorService:
     def _gateway(self) -> DockerGateway:
         return DockerGateway(self.image, network=self.network, orchestrator_url=self.internal_url)
 
+    def _orchestrator_source_current(self, current_hash: str) -> bool:
+        """True iff the running orchestrator container was created from the
+        *current* bind-mounted source. Mirrors `DockerGateway`'s
+        image-staleness check, but by content hash rather than image id since
+        the orchestrator runs bind-mounted source, not a built image."""
+        if not self._container_running(ORCHESTRATOR_NAME):
+            return False
+        proc = run_docker([
+            "docker", "inspect", "--format",
+            "{{ index .Config.Labels \"" + ORCHESTRATOR_SOURCE_HASH_LABEL + "\" }}",
+            ORCHESTRATOR_NAME,
+        ])
+        if proc.returncode != 0:
+            return True  # can't compare -> don't churn a working container
+        return proc.stdout.strip() == current_hash
+
     def ensure_running(
         self, *, startup_timeout: float = DEFAULT_STARTUP_TIMEOUT_SECONDS,
     ) -> str:
         """Ensure the control plane + shared gateway are up; return the host
-        control-plane URL. Idempotent — a healthy control plane and a running
-        gateway are left untouched. Raises `OrchestratorStartError` on
-        timeout."""
+        control-plane URL. Idempotent — a healthy control plane running
+        current code and a running gateway are left untouched. Raises
+        `OrchestratorStartError` on timeout."""
         gateway = self._gateway()
         gateway.ensure_built()  # rebuild the bundle image on a source change
         gateway.ensure_running()  # creates the shared network + (re)starts gateway
 
-        # Always (re)create the orchestrator container. It runs the repo's code
-        # bind-mounted, but the Python process loaded that code at startup and
-        # won't reload — so reusing a healthy-but-stale container would keep
-        # running OLD control-plane code (e.g. dropping the tokens field). Cheap
-        # (~seconds); the registry DB persists and the current launch
-        # re-registers its own in-memory state. (The dedicated orchestrator
-        # image follow-up replaces this with image-staleness detection.)
+        # Recreate the orchestrator container only when its bind-mounted
+        # source has actually changed since it started — its Python process
+        # loaded that code at startup and won't reload, so a stale container
+        # would keep running OLD control-plane code. Recreating on *every*
+        # launch (the prior behaviour) would drop every other active
+        # bottle's in-memory egress tokens each time a new bottle starts,
+        # since the orchestrator process holds them only in memory (#381).
+        current_hash = _source_hash(self._repo_root)
+        if self.is_healthy() and self._orchestrator_source_current(current_hash):
+            return self.url
+
         log.info("starting orchestrator container", context={"name": ORCHESTRATOR_NAME})
-        self._run_orchestrator_container()
+        self._run_orchestrator_container(current_hash)
 
         deadline = time.monotonic() + startup_timeout
         while time.monotonic() < deadline:

tests/unit/test_orchestrator_lifecycle.py

@@ -10,8 +10,10 @@ from unittest.mock import MagicMock, Mock, patch
 
 from bot_bottle.orchestrator.lifecycle import (
     ORCHESTRATOR_NAME,
+    ORCHESTRATOR_SOURCE_HASH_LABEL,
     OrchestratorService,
     OrchestratorStartError,
+    _source_hash,
 )
 from tests.unit import use_bottle_root
 
@@ -28,6 +30,10 @@ def _health(status: int) -> MagicMock:
     return m
 
 
+def _proc(returncode: int = 0, stdout: str = "", stderr: str = "") -> Mock:
+    return Mock(returncode=returncode, stdout=stdout, stderr=stderr)
+
+
 class TestOrchestratorService(unittest.TestCase):
     def setUp(self) -> None:
         self._tmp = tempfile.TemporaryDirectory()
@@ -46,25 +52,67 @@ class TestOrchestratorService(unittest.TestCase):
         with patch(_URLOPEN, side_effect=urllib.error.URLError("refused")):
             self.assertFalse(self.svc.is_healthy())
 
-    def test_ensure_running_always_recreates_orchestrator(self) -> None:
-        # Even when a control plane is already healthy, the orchestrator is
-        # recreated so bind-mounted code changes take effect (its process
-        # won't reload). The gateway is ensured too.
-        run = Mock(return_value=Mock(returncode=0, stderr=""))
+    def test_ensure_running_noop_when_healthy_and_source_unchanged(self) -> None:
+        # A healthy control plane already running the *current* bind-mounted
+        # source is left alone — recreating it on every launch would drop
+        # every other active bottle's in-memory egress tokens (#381).
+        current = _source_hash(self.svc._repo_root)
+        calls: list[list[str]] = []
+
+        def fake(argv: list[str]) -> Mock:
+            calls.append(argv)
+            if argv[:2] == ["docker", "ps"]:
+                return _proc(stdout=ORCHESTRATOR_NAME)
+            if argv[:2] == ["docker", "inspect"]:
+                return _proc(stdout=current)
+            return _proc()
+
         with patch(_URLOPEN, return_value=_health(200)), \
-                patch(_GATEWAY) as gw_cls, patch(_RUN, run), patch(_SLEEP):
+                patch(_GATEWAY) as gw_cls, patch(_RUN, side_effect=fake), patch(_SLEEP):
             self.assertEqual(self.svc.url, self.svc.ensure_running())
         gw_cls.return_value.ensure_running.assert_called()   # gateway kept up
-        runs = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]]
-        self.assertEqual(1, len(runs))                       # orchestrator recreated
+        runs = [c for c in calls if c[:2] == ["docker", "run"]]
+        rms = [c for c in calls if c[:3] == ["docker", "rm", "--force"] and ORCHESTRATOR_NAME in c]
+        self.assertEqual([], runs)   # not recreated
+        self.assertEqual([], rms)
+
+    def test_ensure_running_recreates_when_source_changed(self) -> None:
+        # Healthy, but the running container's label doesn't match the
+        # current source hash (a real code change) — recreate so it takes
+        # effect, same as the gateway's image-staleness check.
+        calls: list[list[str]] = []
+
+        def fake(argv: list[str]) -> Mock:
+            calls.append(argv)
+            if argv[:2] == ["docker", "ps"]:
+                return _proc(stdout=ORCHESTRATOR_NAME)
+            if argv[:2] == ["docker", "inspect"]:
+                return _proc(stdout="stale-hash")
+            return _proc()
+
+        with patch(_URLOPEN, return_value=_health(200)), \
+                patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP):
+            self.assertEqual(self.svc.url, self.svc.ensure_running())
+        runs = [c for c in calls if c[:2] == ["docker", "run"]]
+        self.assertEqual(1, len(runs))
         self.assertIn(ORCHESTRATOR_NAME, runs[0])
+        # the fresh container is labeled with the current hash, not the stale one
+        current = _source_hash(self.svc._repo_root)
+        self.assertIn(f"{ORCHESTRATOR_SOURCE_HASH_LABEL}={current}", runs[0])
 
     def test_ensure_running_starts_orchestrator_container_when_absent(self) -> None:
-        run = Mock(return_value=Mock(returncode=0, stderr=""))
+        calls: list[list[str]] = []
+
+        def fake(argv: list[str]) -> Mock:
+            calls.append(argv)
+            if argv[:2] == ["docker", "ps"]:
+                return _proc(stdout="")   # not running
+            return _proc()
+
         with patch(_URLOPEN, side_effect=[urllib.error.URLError("down"), _health(200)]), \
-                patch(_GATEWAY), patch(_RUN, run), patch(_SLEEP):
+                patch(_GATEWAY), patch(_RUN, side_effect=fake), patch(_SLEEP):
             self.assertEqual(self.svc.url, self.svc.ensure_running())
-        runs = [c.args[0] for c in run.call_args_list if c.args[0][:2] == ["docker", "run"]]
+        runs = [c for c in calls if c[:2] == ["docker", "run"]]
         self.assertEqual(1, len(runs))
         argv = runs[0]
         self.assertIn(ORCHESTRATOR_NAME, argv)