refactor: validate reconciliation inputs and neutralize CLI helpers

bot_bottle/backend/docker/util.py

@@ -6,12 +6,12 @@ from __future__ import annotations
 
 import os
 from datetime import datetime, timezone
-import re
 import shutil
 import subprocess
 from typing import Iterator
 
 from ...log import die, info
+from ...util import slugify
 
 
 def run_docker(
@@ -114,19 +114,6 @@ def docker_cp(src: str, dest: str) -> None:
             f"{(result.stderr or '').strip() or '<no stderr>'}")
 
 
-_SLUG_RE = re.compile(r"[^a-z0-9]+")
-
-
-def slugify(name: str) -> str:
-    """Lowercase, non-alnum runs → '-', trimmed. Dies on empty result."""
-    if not name:
-        die("slugify: missing name")
-    slug = _SLUG_RE.sub("-", name.lower()).strip("-")
-    if not slug:
-        die(f"name '{name}' produced an empty slug; use alphanumeric characters")
-    return slug
-
-
 def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
     """Invokes `docker build` every call. Layer cache makes no-change
     rebuilds cheap; running every time means Dockerfile edits land

bot_bottle/backend/resolve_common.py

@@ -30,6 +30,7 @@ from ..log import die
 from ..manifest import Manifest, ManifestBottle
 from ..supervisor.plan import SupervisePlan
 from ..orchestrator.supervisor import Supervisor
+from ..util import slugify
 from . import BottleSpec
 
 
@@ -44,8 +45,7 @@ def mint_slug(spec: BottleSpec) -> str:
     if spec.identity:
         return spec.identity
     if spec.label:
-        from .docker import util as docker_mod
+        return slugify(spec.label)
-        return docker_mod.slugify(spec.label)
     return bottle_identity(spec.agent_name)
 
 

bot_bottle/cli/commands/start.py

@@ -25,12 +25,11 @@ from typing import Callable
 from ...agent_provider import get_provider, runtime_for
 from ...backend import (
     Bottle,
+    BottlePlan,
     BottleSpec,
     enumerate_active_agents,
     get_bottle_backend,
 )
-from ...backend.docker import util as docker_mod
-from ...backend.docker.bottle_plan import DockerBottlePlan
 from ...bottle_state import (
     cleanup_state,
     is_preserved,
@@ -40,7 +39,7 @@ from ...image_cache import StaleImageError
 from ...log import info, die
 from ...manifest import Manifest, ManifestIndex
 from ..constants import PROG
-from ...util import read_tty_line
+from ...util import read_tty_line, slugify
 from .. import tui
 
 
@@ -257,10 +256,10 @@ def _uniquify_label_headless(label: str) -> str:
     logging the chosen label. Orchestrators fire-and-forget many bottles,
     so silently picking a free name beats erroring on every collision."""
     active_slugs = {a.slug for a in enumerate_active_agents()}
-    if docker_mod.slugify(label) not in active_slugs:
+    if slugify(label) not in active_slugs:
         return label
     n = 2
-    while docker_mod.slugify(f"{label}-{n}") in active_slugs:
+    while slugify(f"{label}-{n}") in active_slugs:
         n += 1
     chosen = f"{label}-{n}"
     info(f"label '{label}' already in use; using '{chosen}'")
@@ -274,11 +273,11 @@ def prepare_with_preflight(
     spec: BottleSpec,
     *,
     stage_dir: Path,
-    render_preflight: Callable[[DockerBottlePlan, str], None],
+    render_preflight: Callable[[BottlePlan, str], None],
     prompt_yes: Callable[[], bool],
     dry_run: bool = False,
     backend_name: str | None = None,
-) -> tuple[DockerBottlePlan | None, str]:
+) -> tuple[BottlePlan | None, str]:
     """Run `backend.prepare`, render the preflight summary via the
     injected callable, prompt y/N via the injected callable.
 
@@ -405,7 +404,7 @@ def _resolve_unique_label(label: str, color: str) -> tuple[str, str]:
     in use among running bottles.  Passes through unchanged when no
     collision is found on the first check."""
     while True:
-        slug_candidate = docker_mod.slugify(label)
+        slug_candidate = slugify(label)
         active_slugs = {a.slug for a in enumerate_active_agents()}
         if slug_candidate not in active_slugs:
             return label, color
@@ -432,7 +431,7 @@ def _select_image_policy() -> str | None:
 
 
 def _text_render_preflight():
-    def _render(plan: DockerBottlePlan, backend_name: str) -> None:
+    def _render(plan: BottlePlan, backend_name: str) -> None:
         print(file=sys.stderr)
         print(f"backend: {backend_name}", file=sys.stderr)
         print(_manifest_to_yaml(plan.manifest), file=sys.stderr)

bot_bottle/orchestrator/server.py

@@ -57,6 +57,7 @@ from __future__ import annotations
 
 import http.server
 import json
+import math
 import os
 import socketserver
 import sys
@@ -217,13 +218,18 @@ def dispatch(  # pylint: disable=too-many-return-statements,too-many-branches
         raw_ips = data.get("live_source_ips")
         if not isinstance(raw_ips, list):
             return 400, {"error": "live_source_ips (list of strings) is required"}
-        live = [ip for ip in raw_ips if isinstance(ip, str) and ip]
+        if any(not isinstance(ip, str) or not ip for ip in raw_ips):
+            return 400, {"error": "live_source_ips must contain non-empty strings"}
+        live = raw_ips
         grace = data.get("grace_seconds")
-        kwargs = (
+        kwargs: dict[str, float] = {}
-            {"grace_seconds": float(grace)}
+        if grace is not None:
-            if isinstance(grace, (int, float)) and not isinstance(grace, bool)
+            if isinstance(grace, bool) or not isinstance(grace, (int, float)):
-            else {}
+                return 400, {"error": "grace_seconds must be a non-negative finite number"}
-        )
+            parsed_grace = float(grace)
+            if not math.isfinite(parsed_grace) or parsed_grace < 0:
+                return 400, {"error": "grace_seconds must be a non-negative finite number"}
+            kwargs["grace_seconds"] = parsed_grace
         return 200, {"reaped": orch.reconcile(live, **kwargs)}
 
     if method == "POST" and route == "/attribute":

bot_bottle/util.py

@@ -9,8 +9,11 @@ import difflib
 import hashlib
 import ipaddress
 import os
+import re
 import sys
 
+from .log import die
+
 
 def sha256_hex(content: str) -> str:
     """Hex SHA-256 of a UTF-8 string."""
@@ -67,3 +70,20 @@ def expand_tilde(path: str) -> str:
         home = os.environ.get("HOME", "")
         return home + path[1:]
     return path
+
+
+_SLUG_RE = re.compile(r"[^a-z0-9]+")
+
+
+def slugify(name: str) -> str:
+    """Return a portable bottle identifier from a human-readable name.
+
+    This is deliberately a root utility: names are part of the generic CLI
+    and state model, not a Docker container concern.
+    """
+    if not name:
+        die("slugify: missing name")
+    slug = _SLUG_RE.sub("-", name.lower()).strip("-")
+    if not slug:
+        die(f"name '{name}' produced an empty slug; use alphanumeric characters")
+    return slug

tests/unit/test_orchestrator_server.py

@@ -647,10 +647,24 @@ class TestReconcileRoute(unittest.TestCase):
         self.assertEqual(200, status)
         self.assertEqual([], payload["reaped"])
 
-    def test_non_string_entries_are_ignored(self) -> None:
+    def test_non_string_entries_are_rejected(self) -> None:
-        dead = self._old("10.0.0.4")
         status, payload = dispatch(
             self.orch, "POST", "/reconcile",
             _body({"live_source_ips": [None, 7, "10.0.0.9"]}))
-        self.assertEqual(200, status)
+        self.assertEqual(400, status)
-        self.assertEqual([dead], payload["reaped"])
+        self.assertIn("live_source_ips", payload["error"])
+
+    def test_empty_live_source_ip_is_rejected(self) -> None:
+        status, payload = dispatch(
+            self.orch, "POST", "/reconcile", _body({"live_source_ips": [""]}))
+        self.assertEqual(400, status)
+        self.assertIn("live_source_ips", payload["error"])
+
+    def test_invalid_grace_seconds_is_rejected(self) -> None:
+        for value in (True, "30", -1, float("inf"), float("nan")):
+            with self.subTest(value=value):
+                status, payload = dispatch(
+                    self.orch, "POST", "/reconcile",
+                    _body({"live_source_ips": [], "grace_seconds": value}))
+                self.assertEqual(400, status)
+                self.assertIn("grace_seconds", payload["error"])