From 27e58f469f97ea7905ba56505a22642d798a441d Mon Sep 17 00:00:00 2001 From: Renn F Date: Thu, 30 Jul 2026 17:51:12 +0200 Subject: [PATCH] feat(runtime): stamp spawned containers with the stack's own compose-project labels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Agent spawns (all five provider paths), intake/secretary chats, and sandbox sidecars now carry com.docker.compose.project/service/oneoff/ config-hash labels copied from the orchestrator's own compose project, so a Docker UI (UGOS) groups them under the stack's project and they die with the stack: bare compose stop/restart affects them, compose down removes them (the config-hash label must be PRESENT for down to even see the container — compose filters its API listing on that key before the orphan predicate runs, verified live), and up -d deliberately does not resurrect them since the orchestrator respawns its own agents. Self-discovery reads the orchestrator's own container id from /proc/self/mountinfo keyed on the root-independent /containers// segment — the UGREEN NAS data-root is /volume1/@docker on btrfs, so its mountinfo reads /@docker/containers/..., never the textbook /var/lib/docker path (verified against the live NAS) — with a HOSTNAME short-id fallback, then one docker inspect cached per process. Only definitive outcomes cache; a transient inspect failure logs and retries on the next spawn. Outside compose the helper yields nothing and every spawn command is byte-for-byte unchanged. --- roboco/llm/providers/codex.py | 2 + roboco/llm/providers/gemini.py | 2 + roboco/llm/providers/grok.py | 2 + roboco/llm/providers/kimi.py | 2 + roboco/runtime/compose_labels.py | 216 +++++++++++++ roboco/runtime/orchestrator.py | 8 + roboco/runtime/sandbox.py | 2 + .../llm/providers/test_gemini_provider.py | 28 ++ tests/unit/llm/test_providers.py | 74 +++++ tests/unit/runtime/test_compose_labels.py | 302 ++++++++++++++++++ tests/unit/runtime/test_intake_spawn.py | 48 +++ tests/unit/runtime/test_sandbox_env.py | 50 +++ .../unit/runtime/test_sandbox_provisioner.py | 57 ++++ .../runtime/test_secretary_spawn_shutdown.py | 63 ++++ 14 files changed, 856 insertions(+) create mode 100644 roboco/runtime/compose_labels.py create mode 100644 tests/unit/runtime/test_compose_labels.py diff --git a/roboco/llm/providers/codex.py b/roboco/llm/providers/codex.py index 82dc0b84..d4e2afca 100644 --- a/roboco/llm/providers/codex.py +++ b/roboco/llm/providers/codex.py @@ -42,6 +42,7 @@ from typing import TYPE_CHECKING, Protocol from roboco.config import settings from roboco.llm.providers._docker import container_running, stop_container from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult +from roboco.runtime.compose_labels import compose_label_args if TYPE_CHECKING: from roboco.models.runtime import OrchestratorAgentConfig as AgentConfig @@ -149,6 +150,7 @@ class CodexCliProvider(AgentProvider): self._append_codex_auth_mount(cmd) self._append_usage_mount(cmd, hosts) self._append_codex_env(cmd, config, initial_prompt) + cmd.extend(await compose_label_args(config.agent_id)) cmd.append(self._image) proc = await asyncio.create_subprocess_exec( diff --git a/roboco/llm/providers/gemini.py b/roboco/llm/providers/gemini.py index 7b8b763a..5677cbe0 100644 --- a/roboco/llm/providers/gemini.py +++ b/roboco/llm/providers/gemini.py @@ -58,6 +58,7 @@ from typing import TYPE_CHECKING, Protocol from roboco.config import settings from roboco.llm.providers._docker import container_running, stop_container from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult +from roboco.runtime.compose_labels import compose_label_args if TYPE_CHECKING: from roboco.models.runtime import OrchestratorAgentConfig as AgentConfig @@ -174,6 +175,7 @@ class GeminiCliProvider(AgentProvider): self._append_gemini_auth_mount(cmd) self._append_usage_mount(cmd, hosts) self._append_gemini_env(cmd, config, initial_prompt) + cmd.extend(await compose_label_args(config.agent_id)) cmd.append(self._image) proc = await asyncio.create_subprocess_exec( diff --git a/roboco/llm/providers/grok.py b/roboco/llm/providers/grok.py index ef18e10b..d915e57e 100644 --- a/roboco/llm/providers/grok.py +++ b/roboco/llm/providers/grok.py @@ -34,6 +34,7 @@ from typing import TYPE_CHECKING, Protocol from roboco.llm.providers._docker import container_running, stop_container from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult +from roboco.runtime.compose_labels import compose_label_args if TYPE_CHECKING: from roboco.models.runtime import OrchestratorAgentConfig as AgentConfig @@ -144,6 +145,7 @@ class GrokCliProvider(AgentProvider): self._append_grok_auth_mount(cmd) self._append_usage_mount(cmd, hosts) self._append_grok_env(cmd, config, initial_prompt) + cmd.extend(await compose_label_args(config.agent_id)) cmd.append(self._image) proc = await asyncio.create_subprocess_exec( diff --git a/roboco/llm/providers/kimi.py b/roboco/llm/providers/kimi.py index 50d15286..2f0d6c5c 100644 --- a/roboco/llm/providers/kimi.py +++ b/roboco/llm/providers/kimi.py @@ -64,6 +64,7 @@ from typing import TYPE_CHECKING, Protocol from roboco.config import settings from roboco.llm.providers._docker import container_running, stop_container from roboco.llm.providers.base import AgentProvider, ProviderError, SpawnResult +from roboco.runtime.compose_labels import compose_label_args if TYPE_CHECKING: from roboco.models.runtime import OrchestratorAgentConfig as AgentConfig @@ -175,6 +176,7 @@ class KimiCliProvider(AgentProvider): self._append_kimi_auth_mount(cmd) self._append_usage_mount(cmd, hosts) self._append_kimi_env(cmd, config, initial_prompt) + cmd.extend(await compose_label_args(config.agent_id)) cmd.append(self._image) proc = await asyncio.create_subprocess_exec( diff --git a/roboco/runtime/compose_labels.py b/roboco/runtime/compose_labels.py new file mode 100644 index 00000000..a22a0673 --- /dev/null +++ b/roboco/runtime/compose_labels.py @@ -0,0 +1,216 @@ +"""Discover the orchestrator's own Docker Compose project, once per process. + +Every container the orchestrator creates as a sibling of itself — agent +spawns (including the persistent intake/secretary chats) and sandbox +DB/Redis/Mongo sidecars — should carry the same +``com.docker.compose.project``/``com.docker.compose.service`` labels the +compose stack stamps on its own service containers. That buys two things: a +Docker UI (e.g. UGOS) groups every spawned container under the same project +as the stack, and `docker compose down --remove-orphans` sweeps them away +WITH the stack — the CEO wants agents to die with their orchestrator, since +an agent whose orchestrator is gone can't do anything. + +**Full compose-lifecycle semantics (intended, CEO-decided).** These labeled +sibling containers are NOT compose-file services — they exist only via +`docker run`, invisible to the compose file itself. That gives them exactly +the lifecycle a plain `--label` buys and no more: a bare `docker compose +stop` stops them (label-matched, like any other project container) and a +bare `docker compose restart` restarts them; `docker compose down` removes +them — verified empirically both with and without `--remove-orphans` (see +below for why `--remove-orphans` specifically needs a 4th label to even see +them). Critically, `docker compose up -d` does NOT resurrect them — `up` +only reconciles containers for services declared in the compose file, and a +labeled sidecar declares no service. This is deliberate, not a gap: an agent +without its orchestrator can't do anything, so it should never survive a +`down`, and after a redeploy (`up -d` bringing the orchestrator back) the +orchestrator's own startup respawns its agents itself — nothing about +resurrecting them is compose's job. + +Verified empirically (live `docker compose down --remove-orphans`, compose +v5.3.1): a sidecar needs FOUR labels, not the three one would guess from +project/service/oneoff alone. Compose's own container listing +(``getDefaultFilters`` in ``docker/compose`` ``pkg/compose/containers.go``) +unconditionally adds a ``label=com.docker.compose.config-hash`` filter to +the Docker API query it runs BEFORE the orphan predicate ever sees a +container — a sidecar missing that label key is invisible to `down` at the +API level, orphan or not, no matter how correctly project/service/oneoff are +set. Its value is never read (only the label's presence gates the API-level +list), so a fixed placeholder is correct. + +Self-identification reads this container's own ``/proc/self/mountinfo`` +first, rather than its hostname. Docker's default (no ``hostname:`` in +compose — true today for the orchestrator service in both docker-compose.yml +and docker-compose.registry.yml) sets a container's hostname to its own +short id, which would work too — but silently breaks the moment a future +compose edit adds an explicit ``hostname:`` for the orchestrator service, +with nothing erroring anywhere to catch it. Docker bind-mounts /etc/hostname, +/etc/hosts, and resolv.conf from a per-container config dir into every +container regardless of any hostname override, so the real id is recoverable +from our own mountinfo independent of hostname entirely — and independent of +the docker data-root path too: a real UGREEN NAS container's mountinfo reads +``/@docker/containers//hostname`` (data-root ``/volume1/@docker``, a +btrfs mount), not the textbook ``/var/lib/docker/containers//hostname``, +so the pattern below keys on the root-independent ``/containers//`` +suffix rather than assuming a specific data-root prefix. The HOSTNAME env var +is a second-tier fallback (Docker's default-hostname-is-the-short-id +behaviour, verified true for the orchestrator service today) for a runtime +that doesn't bind-mount those per-container files the way Docker does. +""" + +from __future__ import annotations + +import asyncio +import os +import re +from dataclasses import dataclass +from pathlib import Path + +import structlog + +logger = structlog.get_logger(__name__) + +_MOUNTINFO_PATH = "/proc/self/mountinfo" +# Root-independent: matches both /var/lib/docker/containers//hostname and +# a NAS's /@docker/containers//hostname (or any other docker data-root) — +# see module docstring. +_CONTAINER_ID_RE = re.compile( + r"/containers/([0-9a-f]{64})/(?:hostname|hosts|resolv\.conf)" +) +_HOSTNAME_ID_RE = re.compile(r"^[0-9a-f]{12,64}$") +_DOCKER_INSPECT_TIMEOUT_SECONDS = 5.0 + +# Any value works (compose never reads it — see module docstring); it only +# has to be a present label key. +_CONFIG_HASH_PLACEHOLDER = "roboco-sidecar" + + +@dataclass +class _DiscoveryCache: + """Mutated in place (never rebound) so `_discover` needs no `global`. + + ``discovered`` only flips True on a DEFINITIVE outcome (a resolved + project, or a confirmed "not under compose") — a transient failure + (docker missing, a timeout, a nonzero inspect) leaves it False so the + next spawn retries instead of caching a false negative forever. + """ + + discovered: bool = False + project: str | None = None + + +_lock = asyncio.Lock() +_cache = _DiscoveryCache() + + +def _id_from_mountinfo() -> str | None: + """This process's own container id, parsed from /proc/self/mountinfo. + + None outside a container (file absent/unreadable) or under a container + runtime that doesn't bind-mount per-container config the way Docker does. + """ + try: + with Path(_MOUNTINFO_PATH).open(encoding="utf-8") as fh: + content = fh.read() + except OSError: + return None + match = _CONTAINER_ID_RE.search(content) + return match.group(1) if match else None + + +def _id_from_hostname_env() -> str | None: + """Fallback: Docker's default (no `hostname:` override) sets HOSTNAME to + the container's own short id; `docker inspect` accepts a short-id prefix. + """ + hostname = os.environ.get("HOSTNAME", "") + return hostname if _HOSTNAME_ID_RE.match(hostname) else None + + +def _own_container_id() -> str | None: + """Mountinfo first (robust to a hostname override), HOSTNAME second.""" + return _id_from_mountinfo() or _id_from_hostname_env() + + +async def _inspect_compose_project(container_id: str) -> tuple[bool, str | None]: + """`docker inspect` our own compose-project label. + + Returns ``(definitive, project)``. ``definitive`` is False for a + transient failure (docker CLI missing, a timeout, a nonzero inspect exit + — daemon not ready yet, etc.) that deserves a retry on the next call, and + True for a completed inspect regardless of whether the label was present + (a container genuinely not started by compose has no label — that is a + real, stable answer, not a failure). + """ + try: + proc = await asyncio.create_subprocess_exec( + "docker", + "inspect", + "--format", + '{{ index .Config.Labels "com.docker.compose.project" }}', + container_id, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.DEVNULL, + ) + stdout, _ = await asyncio.wait_for( + proc.communicate(), timeout=_DOCKER_INSPECT_TIMEOUT_SECONDS + ) + except (OSError, TimeoutError): + return False, None + if proc.returncode != 0: + return False, None + return True, (stdout.decode().strip() or None) + + +async def _discover() -> str | None: + """Resolve + cache the orchestrator's own compose project, once. + + Lock-guarded so two concurrent first-callers (e.g. two agent spawns + racing at startup) can't both run `docker inspect`; every call after a + DEFINITIVE resolution is a plain attribute read once the lock is free. + """ + async with _lock: + if _cache.discovered: + return _cache.project + container_id = _own_container_id() + if not container_id: + _cache.project = None + _cache.discovered = True + logger.info("compose project discovery: no container id found") + return None + definitive, project = await _inspect_compose_project(container_id) + if not definitive: + logger.warning( + "compose project discovery failed transiently; will retry", + container_id=container_id, + ) + return None + _cache.project = project + _cache.discovered = True + if project: + logger.info("compose project resolved", project=project) + else: + logger.info("container is not part of a compose project") + return project + + +async def compose_label_args(service: str) -> list[str]: + """Ready-to-splice ``--label`` argv for a sibling container of ``service``. + + Empty when the orchestrator isn't running under compose, or discovery + hasn't yet succeeded — every call site's docker run cmd is then + byte-for-byte unchanged, matching current behaviour on dev machines / + tests / the eval harness (and self-heals on the next spawn after a + transient discovery failure). + """ + project = await _discover() + if not project: + return [] + return [ + "--label", + f"com.docker.compose.project={project}", + "--label", + f"com.docker.compose.service={service}", + "--label", + "com.docker.compose.oneoff=False", + "--label", + f"com.docker.compose.config-hash={_CONFIG_HASH_PLACEHOLDER}", + ] diff --git a/roboco/runtime/orchestrator.py b/roboco/runtime/orchestrator.py index 54cbd2e5..c635950c 100644 --- a/roboco/runtime/orchestrator.py +++ b/roboco/runtime/orchestrator.py @@ -72,6 +72,7 @@ from roboco.models.runtime import ( WaitingRecord, ) from roboco.models.sandbox import SandboxInfo +from roboco.runtime.compose_labels import compose_label_args from roboco.runtime.sandbox import SandboxProvisioner from roboco.seeds.initial_data import AGENT_UUIDS from roboco.services.task import ( @@ -3704,6 +3705,7 @@ class AgentOrchestrator: self._append_sandbox_marker_env(cmd, config.sandbox_available_services) else: self._append_gate_env(cmd) + cmd.extend(await compose_label_args(config.agent_id)) self._append_image_and_claude_args(cmd, config, initial_prompt) proc = await asyncio.create_subprocess_exec( @@ -5373,6 +5375,9 @@ class AgentOrchestrator: model=route.model_name, ) ) + # Insert before the trailing image element (docker run flags must + # precede the image, not follow it). + cmd[-1:-1] = await compose_label_args(INTAKE_AGENT_ID) container_id = await self._run_container_cmd(cmd) # Shutdown may have begun while this (non-blocking) spawn was in flight @@ -5582,6 +5587,9 @@ class AgentOrchestrator: model=route.model_name, ) ) + # Insert before the trailing image element (docker run flags must + # precede the image, not follow it). + cmd[-1:-1] = await compose_label_args(SECRETARY_AGENT_ID) container_id = await self._run_container_cmd(cmd) # Shutdown may have begun while this (non-blocking) spawn was in flight diff --git a/roboco/runtime/sandbox.py b/roboco/runtime/sandbox.py index 8c30e7c3..fbc45e91 100644 --- a/roboco/runtime/sandbox.py +++ b/roboco/runtime/sandbox.py @@ -29,6 +29,7 @@ from roboco.models.sandbox import ( SandboxEngine, SandboxInfo, ) +from roboco.runtime.compose_labels import compose_label_args if TYPE_CHECKING: from collections.abc import Awaitable, Callable @@ -197,6 +198,7 @@ class SandboxProvisioner: SANDBOX_LABEL, "--label", f"{_OWNER_LABEL_KEY}={_owner_label(agent_id)}", + *await compose_label_args(name), ] for mount in engine.tmpfs: args += ["--tmpfs", mount] diff --git a/tests/unit/llm/providers/test_gemini_provider.py b/tests/unit/llm/providers/test_gemini_provider.py index 3c061e94..ef78e7f1 100644 --- a/tests/unit/llm/providers/test_gemini_provider.py +++ b/tests/unit/llm/providers/test_gemini_provider.py @@ -189,6 +189,34 @@ async def test_gemini_spawn_wires_gateway_env_and_image_last() -> None: ) +async def test_gemini_spawn_adds_compose_labels_before_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the orchestrator resolves its own compose project, the sibling + agent container carries it too (UGOS grouping + `down --remove-orphans`) + — and the labels land BEFORE the image, never after (docker parses + anything past the image as the container command, not a flag).""" + + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.llm.providers.gemini.compose_label_args", _fake_label_args + ) + host = _FakeHost() + provider = GeminiCliProvider(host, image="roboco-agent-gemini:test") + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_config()) + cmd = list(exec_mock.call_args.args) + assert "com.docker.compose.service=be-dev-1" in cmd + assert cmd.index("com.docker.compose.service=be-dev-1") < cmd.index( + "roboco-agent-gemini:test" + ) + assert cmd[-1] == "roboco-agent-gemini:test" + + async def test_gemini_spawn_mounts_auth_when_present( _isolate_gemini_auth: Path, ) -> None: diff --git a/tests/unit/llm/test_providers.py b/tests/unit/llm/test_providers.py index 1b417841..c86c51ae 100644 --- a/tests/unit/llm/test_providers.py +++ b/tests/unit/llm/test_providers.py @@ -266,6 +266,34 @@ async def test_grok_spawn_wires_gateway_env_and_image_last() -> None: ) +async def test_grok_spawn_adds_compose_labels_before_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the orchestrator resolves its own compose project, the sibling + agent container carries it too (UGOS grouping + `down --remove-orphans`) + — and the labels land BEFORE the image, never after (docker parses + anything past the image as the container command, not a flag).""" + + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.llm.providers.grok.compose_label_args", _fake_label_args + ) + host = _FakeHost() + provider = GrokCliProvider(host, image="roboco-agent-grok:test") + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_config()) + cmd = list(exec_mock.call_args.args) + assert "com.docker.compose.service=be-dev-1" in cmd + assert cmd.index("com.docker.compose.service=be-dev-1") < cmd.index( + "roboco-agent-grok:test" + ) + assert cmd[-1] == "roboco-agent-grok:test" + + async def test_grok_spawn_mounts_auth_when_present(_isolate_grok_auth: Path) -> None: (_isolate_grok_auth / "auth.json").write_text("{}", encoding="utf-8") host = _FakeHost() @@ -421,6 +449,29 @@ async def test_codex_spawn_wires_gateway_env_and_image_last() -> None: ) +async def test_codex_spawn_adds_compose_labels_before_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.llm.providers.codex.compose_label_args", _fake_label_args + ) + host = _FakeHost() + provider = CodexCliProvider(host, image="roboco-agent-codex:test") + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_codex_config()) + cmd = list(exec_mock.call_args.args) + assert "com.docker.compose.service=be-dev-1" in cmd + assert cmd.index("com.docker.compose.service=be-dev-1") < cmd.index( + "roboco-agent-codex:test" + ) + assert cmd[-1] == "roboco-agent-codex:test" + + async def test_codex_spawn_mounts_auth_when_present( _isolate_codex_auth: Path, ) -> None: @@ -571,6 +622,29 @@ async def test_kimi_spawn_wires_gateway_env_and_image_last() -> None: ) +async def test_kimi_spawn_adds_compose_labels_before_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.llm.providers.kimi.compose_label_args", _fake_label_args + ) + host = _FakeHost() + provider = KimiCliProvider(host, image="roboco-agent-kimi:test") + with patch( + "asyncio.create_subprocess_exec", AsyncMock(return_value=_proc()) + ) as exec_mock: + await provider.spawn(_kimi_config()) + cmd = list(exec_mock.call_args.args) + assert "com.docker.compose.service=be-dev-1" in cmd + assert cmd.index("com.docker.compose.service=be-dev-1") < cmd.index( + "roboco-agent-kimi:test" + ) + assert cmd[-1] == "roboco-agent-kimi:test" + + async def test_kimi_spawn_mounts_auth_when_present(_isolate_kimi_auth: Path) -> None: creds_dir = _isolate_kimi_auth / "credentials" creds_dir.mkdir(parents=True, exist_ok=True) diff --git a/tests/unit/runtime/test_compose_labels.py b/tests/unit/runtime/test_compose_labels.py new file mode 100644 index 00000000..8ba9d100 --- /dev/null +++ b/tests/unit/runtime/test_compose_labels.py @@ -0,0 +1,302 @@ +"""roboco.runtime.compose_labels — self-id + cached compose-project discovery. + +Every docker-run site splices ``compose_label_args(service)`` in; on a dev +machine / CI runner / the eval harness (never a compose-managed container) +discovery finds nothing and every call site is byte-for-byte unchanged — +that fallback is exercised implicitly by every OTHER spawn-cmd test in the +suite (none of them run inside a real compose stack), so this file focuses +on the helper's own mechanics: mountinfo parsing (including the real UGREEN +NAS btrfs shape), the HOSTNAME fallback, the docker-inspect path, the +definitive-vs-transient cache semantics, and the once-per-process cache. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING +from unittest.mock import AsyncMock, MagicMock, patch + +import pytest +from roboco.runtime import compose_labels + +if TYPE_CHECKING: + from pathlib import Path + +_FAKE_CONTAINER_ID = "a" * 64 +# From a live UGREEN NAS container (DockerRootDir /volume1/@docker, a btrfs +# mount) — captured 2026-07-30. Root field (4th) carries the bind-mount +# source path; NOT /var/lib/docker. +_NAS_HOSTNAME_LINE = ( + "614 613 0:59 /@docker/containers/" + "87ea7acf042857b338432c5a06563cdc4d5cef97d959efcb57da154b34e924ba/hostname " + "/etc/hostname rw,relatime - btrfs /dev/bcache0 " + "rw,ssd,space_cache=v2,subvolid=257\n" +) + + +@pytest.fixture(autouse=True) +def _reset_cache(monkeypatch: pytest.MonkeyPatch) -> None: + """Every test starts as if discovery never ran (module-level cache).""" + monkeypatch.setattr(compose_labels, "_cache", compose_labels._DiscoveryCache()) + + +@pytest.fixture(autouse=True) +def _clear_hostname_env(monkeypatch: pytest.MonkeyPatch) -> None: + """Deterministic: the mountinfo-only tests must not accidentally pick up + a real HOSTNAME from the runner's own environment via the fallback tier. + Tests exercising the fallback set HOSTNAME explicitly.""" + monkeypatch.delenv("HOSTNAME", raising=False) + + +def _proc(returncode: int = 0, stdout: bytes = b"", stderr: bytes = b"") -> MagicMock: + proc = MagicMock() + proc.returncode = returncode + proc.communicate = AsyncMock(return_value=(stdout, stderr)) + return proc + + +# --------------------------------------------------------------------------- +# _id_from_mountinfo / _id_from_hostname_env / _own_container_id +# --------------------------------------------------------------------------- + + +def test_own_container_id_none_when_mountinfo_missing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr( + compose_labels, "_MOUNTINFO_PATH", str(tmp_path / "does-not-exist") + ) + assert compose_labels._own_container_id() is None + + +def test_own_container_id_none_when_no_docker_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A real host's mountinfo (not a Docker container) has no matching line.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("1 2 0:1 / / rw,relatime - ext4 /dev/root rw\n") + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + assert compose_labels._own_container_id() is None + + +def test_own_container_id_parses_docker_bind_mount( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The textbook /var/lib/docker data-root shape.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text( + "614 613 253:1 /var/lib/docker/containers/" + f"{_FAKE_CONTAINER_ID}/hostname /etc/hostname rw,relatime " + "- ext4 /dev/root rw\n" + ) + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + assert compose_labels._own_container_id() == _FAKE_CONTAINER_ID + + +def test_own_container_id_parses_nas_btrfs_data_root( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The real deploy target: UGREEN NAS, DockerRootDir /volume1/@docker, a + btrfs subvolume mount — NOT /var/lib/docker. The id must still parse + since the regex keys on the root-independent `/containers//` + suffix, not a specific data-root prefix.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text(_NAS_HOSTNAME_LINE) + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + assert ( + compose_labels._own_container_id() + == "87ea7acf042857b338432c5a06563cdc4d5cef97d959efcb57da154b34e924ba" + ) + + +def test_own_container_id_falls_back_to_hostname_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """No usable mountinfo (e.g. a runtime that doesn't bind-mount per-container + config) => fall back to Docker's default HOSTNAME-is-the-short-id.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("1 2 0:1 / / rw,relatime - ext4 /dev/root rw\n") + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + monkeypatch.setenv("HOSTNAME", "545315347e2f") + assert compose_labels._own_container_id() == "545315347e2f" + + +def test_hostname_env_rejects_a_real_hostname(monkeypatch: pytest.MonkeyPatch) -> None: + """A non-container HOSTNAME (a real machine name) must not be mistaken + for a container id.""" + monkeypatch.setenv("HOSTNAME", "MacBook-Pro.local") + assert compose_labels._id_from_hostname_env() is None + + +def test_mountinfo_takes_priority_over_hostname_env( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Mountinfo wins when both are available — robust to a hostname override.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text( + "614 613 253:1 /var/lib/docker/containers/" + f"{_FAKE_CONTAINER_ID}/hostname /etc/hostname rw,relatime " + "- ext4 /dev/root rw\n" + ) + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + monkeypatch.setenv("HOSTNAME", "545315347e2f") + assert compose_labels._own_container_id() == _FAKE_CONTAINER_ID + + +# --------------------------------------------------------------------------- +# compose_label_args — end to end (discovery + cache) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_compose_label_args_empty_outside_a_container( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """No container id at all (dev machine / test env) => no labels, no docker call.""" + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: None) + with patch("asyncio.create_subprocess_exec") as create_exec: + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [] + create_exec.assert_not_called() + + +@pytest.mark.asyncio +async def test_compose_label_args_present_when_project_resolved( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(return_value=_proc(returncode=0, stdout=b"roboco-nas\n")), + ): + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [ + "--label", + "com.docker.compose.project=roboco-nas", + "--label", + "com.docker.compose.service=be-dev-1", + "--label", + "com.docker.compose.oneoff=False", + "--label", + f"com.docker.compose.config-hash={compose_labels._CONFIG_HASH_PLACEHOLDER}", + ] + + +@pytest.mark.asyncio +async def test_hostname_fallback_id_reaches_docker_inspect( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """End to end: mountinfo empty, HOSTNAME=12-hex => docker inspect is + called with that hostname value as the target container id.""" + mountinfo = tmp_path / "mountinfo" + mountinfo.write_text("1 2 0:1 / / rw,relatime - ext4 /dev/root rw\n") + monkeypatch.setattr(compose_labels, "_MOUNTINFO_PATH", str(mountinfo)) + monkeypatch.setenv("HOSTNAME", "545315347e2f") + create_exec = AsyncMock(return_value=_proc(returncode=0, stdout=b"roboco-nas\n")) + with patch("asyncio.create_subprocess_exec", create_exec): + result = await compose_labels.compose_label_args("be-dev-1") + assert "com.docker.compose.project=roboco-nas" in result + assert create_exec.call_args.args[-1] == "545315347e2f" + + +@pytest.mark.asyncio +async def test_compose_label_args_empty_when_inspect_fails_transiently( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(return_value=_proc(returncode=1, stderr=b"no such container\n")), + ): + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [] + # Transient (nonzero inspect) — must NOT poison the cache forever. + assert compose_labels._cache.discovered is False + + +@pytest.mark.asyncio +async def test_transient_inspect_failure_retries_on_next_call( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A first-call transient failure (e.g. daemon not ready yet) must not + cache a permanent None — the very next spawn gets a fresh attempt.""" + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(return_value=_proc(returncode=1, stderr=b"daemon not ready\n")), + ): + first = await compose_labels.compose_label_args("be-dev-1") + assert first == [] + + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(return_value=_proc(returncode=0, stdout=b"roboco-nas\n")), + ): + second = await compose_labels.compose_label_args("be-dev-1") + assert "com.docker.compose.project=roboco-nas" in second + + +@pytest.mark.asyncio +async def test_compose_label_args_empty_when_label_absent( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A real container not launched by compose has no project label — the + `--format` template resolves to an empty string, not an error. This IS + a definitive outcome (a completed inspect), so it gets cached.""" + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(return_value=_proc(returncode=0, stdout=b"\n")), + ): + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [] + assert compose_labels._cache.discovered is True + assert compose_labels._cache.project is None + + +@pytest.mark.asyncio +async def test_compose_label_args_empty_when_docker_missing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """docker CLI absent (FileNotFoundError from create_subprocess_exec) — + transient, not cached.""" + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + with patch( + "asyncio.create_subprocess_exec", + AsyncMock(side_effect=FileNotFoundError("docker not found")), + ): + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [] + assert compose_labels._cache.discovered is False + + +@pytest.mark.asyncio +async def test_compose_label_args_empty_on_timeout( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + proc = MagicMock() + proc.communicate = AsyncMock(side_effect=TimeoutError) + with patch("asyncio.create_subprocess_exec", AsyncMock(return_value=proc)): + result = await compose_labels.compose_label_args("be-dev-1") + assert result == [] + assert compose_labels._cache.discovered is False + + +@pytest.mark.asyncio +async def test_discovery_runs_docker_inspect_only_once( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Second/third call for a different service reuses the cached project — + the whole point of caching it process-wide.""" + monkeypatch.setattr(compose_labels, "_own_container_id", lambda: _FAKE_CONTAINER_ID) + create_exec = AsyncMock(return_value=_proc(returncode=0, stdout=b"roboco-nas\n")) + with patch("asyncio.create_subprocess_exec", create_exec): + first = await compose_labels.compose_label_args("be-dev-1") + second = await compose_labels.compose_label_args("fe-qa-1") + + assert create_exec.call_count == 1 + assert "com.docker.compose.service=be-dev-1" in first + assert "com.docker.compose.service=fe-qa-1" in second + # Both share the same resolved project. + assert "com.docker.compose.project=roboco-nas" in first + assert "com.docker.compose.project=roboco-nas" in second diff --git a/tests/unit/runtime/test_intake_spawn.py b/tests/unit/runtime/test_intake_spawn.py index a0ded54d..9af12154 100644 --- a/tests/unit/runtime/test_intake_spawn.py +++ b/tests/unit/runtime/test_intake_spawn.py @@ -789,6 +789,54 @@ class TestSpawnIntakeSession: # The cloned cwd reached the docker cmd. assert "ROBOCO_WORKSPACE=/data/workspaces/roboco/board/intake-1" in run_calls[0] + @pytest.mark.asyncio + async def test_spawn_adds_compose_labels_before_image( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """When the orchestrator resolves its own compose project, the + persistent intake container carries it too — spliced in right before + the trailing image element (docker run flags must precede the + image).""" + orch = _make_minimal_orchestrator() + run_calls: list[list[str]] = [] + _wire_spawn_mocks(monkeypatch, orch, run_calls) + + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.runtime.orchestrator.compose_label_args", _fake_label_args + ) + + await orch.spawn_intake_session("sess-labels", project_slug="roboco") + + assert len(run_calls) == 1 + cmd = run_calls[0] + assert cmd[-3] == "--label" + assert cmd[-2] == f"com.docker.compose.service={INTAKE_AGENT_ID}" + + @pytest.mark.asyncio + async def test_spawn_omits_compose_labels_outside_compose( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """The real helper returns [] outside a compose stack — the cmd + shape (image last) is byte-for-byte unchanged from today.""" + orch = _make_minimal_orchestrator() + run_calls: list[list[str]] = [] + _wire_spawn_mocks(monkeypatch, orch, run_calls) + + async def _no_labels(_service: str) -> list[str]: + return [] + + monkeypatch.setattr( + "roboco.runtime.orchestrator.compose_label_args", _no_labels + ) + + await orch.spawn_intake_session("sess-no-labels", project_slug="roboco") + + assert len(run_calls) == 1 + assert not any(a.startswith("com.docker.compose.") for a in run_calls[0]) + @pytest.mark.asyncio async def test_scope_must_be_exactly_one(self) -> None: orch = _make_minimal_orchestrator() diff --git a/tests/unit/runtime/test_sandbox_env.py b/tests/unit/runtime/test_sandbox_env.py index 6313abae..b9b96c26 100644 --- a/tests/unit/runtime/test_sandbox_env.py +++ b/tests/unit/runtime/test_sandbox_env.py @@ -120,3 +120,53 @@ async def test_spawn_container_stale_clear_runs_with_teardown_sandbox_false( teardown_sandbox=False, stop_reason="pre_spawn_stale_clear", ) + + +@pytest.mark.asyncio +async def test_spawn_container_adds_compose_labels_before_image_args( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the orchestrator resolves its own compose project, the spawned + agent container carries it too — inserted before + `_append_image_and_claude_args` runs, so the label flags precede the + image argument in the final docker run cmd.""" + orch = AgentOrchestrator.__new__(AgentOrchestrator) + calls: list[str] = [] + _stub_spawn_container_collaborators(monkeypatch, orch, calls) + + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.runtime.orchestrator.compose_label_args", _fake_label_args + ) + exec_mock = AsyncMock(return_value=_fake_proc()) + monkeypatch.setattr(asyncio, "create_subprocess_exec", exec_mock) + + await orch._spawn_container(_config(["postgres"])) + + cmd = list(exec_mock.call_args.args) + assert cmd == ["--label", "com.docker.compose.service=dev-1"] + + +@pytest.mark.asyncio +async def test_spawn_container_omits_compose_labels_outside_compose( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real helper returns [] outside a compose stack (dev machines, CI, + the eval harness) — the cmd is byte-for-byte unchanged from today.""" + orch = AgentOrchestrator.__new__(AgentOrchestrator) + calls: list[str] = [] + _stub_spawn_container_collaborators(monkeypatch, orch, calls) + + async def _no_labels(_service: str) -> list[str]: + return [] + + monkeypatch.setattr("roboco.runtime.orchestrator.compose_label_args", _no_labels) + exec_mock = AsyncMock(return_value=_fake_proc()) + monkeypatch.setattr(asyncio, "create_subprocess_exec", exec_mock) + + await orch._spawn_container(_config(["postgres"])) + + cmd = list(exec_mock.call_args.args) + assert cmd == [] diff --git a/tests/unit/runtime/test_sandbox_provisioner.py b/tests/unit/runtime/test_sandbox_provisioner.py index 15b649ae..faf12ec2 100644 --- a/tests/unit/runtime/test_sandbox_provisioner.py +++ b/tests/unit/runtime/test_sandbox_provisioner.py @@ -129,6 +129,63 @@ async def test_provision_labels_are_correct() -> None: assert "roboco.sandbox.owner=roboco-agent-dev-2" in labels +@pytest.mark.asyncio +async def test_provision_adds_compose_labels_when_orchestrator_is_compose_managed( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the orchestrator resolves its own compose project, every sandbox + sidecar carries it too — so `docker compose down --remove-orphans` sweeps + the sidecar away with the stack, and UGOS groups it under the same + project.""" + + async def _fake_label_args(service: str) -> list[str]: + return [ + "--label", + "com.docker.compose.project=roboco-nas", + "--label", + f"com.docker.compose.service={service}", + "--label", + "com.docker.compose.oneoff=False", + "--label", + "com.docker.compose.config-hash=roboco-sidecar", + ] + + monkeypatch.setattr(sandbox_module, "compose_label_args", _fake_label_args) + runner = _FakeRunner(run_rc=0, exec_rc=0) + provisioner = SandboxProvisioner(network=_NETWORK, runner=runner) + + await provisioner.provision("dev-3", ["postgres"]) + + run_call = next(c for c in runner.calls if c[0] == "run") + label_indices = [i for i, a in enumerate(run_call) if a == "--label"] + labels = [run_call[i + 1] for i in label_indices] + assert "com.docker.compose.project=roboco-nas" in labels + expected_service = sandbox_module.SANDBOX_ENGINES["postgres"].container_name( + "dev-3" + ) + assert f"com.docker.compose.service={expected_service}" in labels + + +@pytest.mark.asyncio +async def test_provision_omits_compose_labels_outside_compose( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real discovery helper returns [] outside a compose stack (dev + machines, CI, the eval harness) — no com.docker.compose.* label leaks in.""" + + async def _no_labels(_service: str) -> list[str]: + return [] + + monkeypatch.setattr(sandbox_module, "compose_label_args", _no_labels) + runner = _FakeRunner(run_rc=0, exec_rc=0) + provisioner = SandboxProvisioner(network=_NETWORK, runner=runner) + + await provisioner.provision("dev-4", ["postgres"]) + + run_call = next(c for c in runner.calls if c[0] == "run") + assert not any(a.startswith("com.docker.compose.") for a in run_call) + + @pytest.mark.asyncio async def test_provision_mongo_engine() -> None: runner = _FakeRunner(run_rc=0, exec_rc=0) diff --git a/tests/unit/runtime/test_secretary_spawn_shutdown.py b/tests/unit/runtime/test_secretary_spawn_shutdown.py index e0c89b71..124d9280 100644 --- a/tests/unit/runtime/test_secretary_spawn_shutdown.py +++ b/tests/unit/runtime/test_secretary_spawn_shutdown.py @@ -145,6 +145,69 @@ async def test_shutdown_mid_spawn_removes_container_and_skips_registration( assert closed == ["sess-sec-orphan"] +@pytest.mark.asyncio +async def test_secretary_spawn_adds_compose_labels_before_image( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """When the orchestrator resolves its own compose project, the persistent + Secretary container carries it too — spliced in right before the + trailing image element (docker run flags must precede the image).""" + orch = _make_orchestrator() + removed: list[str] = [] + _wire_secretary_spawn_mocks(monkeypatch, orch, removed, flip_running_on_run=False) + + captured: list[list[str]] = [] + + async def _run_capture(cmd: list[str]) -> str: + captured.append(cmd) + return "containerid0123456789" + + monkeypatch.setattr(orch, "_run_container_cmd", _run_capture) + + async def _fake_label_args(service: str) -> list[str]: + return ["--label", f"com.docker.compose.service={service}"] + + monkeypatch.setattr( + "roboco.runtime.orchestrator.compose_label_args", _fake_label_args + ) + + await orch.spawn_secretary_session("sess-labels", initial_message=None) + + assert len(captured) == 1 + cmd = captured[0] + assert cmd[-3] == "--label" + assert cmd[-2] == f"com.docker.compose.service={SECRETARY_AGENT_ID}" + + +@pytest.mark.asyncio +async def test_secretary_spawn_omits_compose_labels_outside_compose( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The real helper returns [] outside a compose stack — the cmd shape + (image last) is byte-for-byte unchanged from today.""" + orch = _make_orchestrator() + removed: list[str] = [] + _wire_secretary_spawn_mocks(monkeypatch, orch, removed, flip_running_on_run=False) + + captured: list[list[str]] = [] + + async def _run_capture(cmd: list[str]) -> str: + captured.append(list(cmd)) + return "containerid0123456789" + + monkeypatch.setattr(orch, "_run_container_cmd", _run_capture) + + async def _no_labels(_service: str) -> list[str]: + return [] + + monkeypatch.setattr("roboco.runtime.orchestrator.compose_label_args", _no_labels) + + await orch.spawn_secretary_session("sess-no-labels", initial_message=None) + + assert len(captured) == 1 + assert not any(a.startswith("com.docker.compose.") for a in captured[0]) + + @pytest.mark.asyncio async def test_running_spawn_registers_normally( monkeypatch: pytest.MonkeyPatch,