diff --git a/agent/verify/runner.py b/agent/verify/runner.py index c327dde636..4016c1638b 100644 --- a/agent/verify/runner.py +++ b/agent/verify/runner.py @@ -182,15 +182,13 @@ def _run_start_phase( return ReadinessResult(url, ready, status, time.monotonic() - started, error, _tail(output)) -def _running_compose_containers(root: Path) -> list[str] | None: - """Names of currently-running containers for the compose project at *root*, - via ``docker compose ps`` (read-only; never mutates anything). +def _compose_live_state_reason(root: Path) -> str | None: + """Why ``docker compose build``/``up`` must not run at *root*, or ``None`` to proceed. - ``None`` only when docker itself is absent -- the build phase would fail the - same way, so there is nothing to protect. A hung daemon or a non-zero probe - is reported as a refusal: containers may be live and unobservable, which is - exactly the #103567 loss window, and ``docker compose build`` would not have - fared better against the same daemon. + Read-only ``docker compose ps`` probe. Only a missing docker binary proceeds -- the + build phase would fail the same way, so there is nothing to protect. A hung daemon + or a non-zero probe refuses: containers may be live and unobservable, which is + exactly the #103567 loss window. """ try: result = subprocess.run( @@ -200,11 +198,12 @@ def _running_compose_containers(root: Path) -> list[str] | None: except FileNotFoundError: return None except subprocess.TimeoutExpired: - return [""] + return "docker compose ps timed out after 15s; live containers cannot be ruled out" if result.returncode != 0: detail = (result.stderr or result.stdout or "").strip().splitlines() - return [f""] - return [line for line in result.stdout.splitlines() if line.strip()] + return f"docker compose ps failed (exit {result.returncode}): {detail[-1] if detail else 'no output'}" + names = [line for line in result.stdout.splitlines() if line.strip()] + return f"this compose project already has running container(s): {', '.join(names)}" if names else None def run_verify( @@ -229,16 +228,15 @@ def run_verify( mutating = ("build" in selected) or ("start" in selected and not skip_start) if recipe.kind == "compose" and mutating: - running = _running_compose_containers(root) - if running: + reason = _compose_live_state_reason(root) + if reason: result.phases.append(PhaseResult( phase="build", command=recipe.build[0] if recipe.build else "docker compose build", exit_code=1, duration=0.0, output_tail=( - "Refusing to run: this compose project already has running " - f"container(s) ({', '.join(running)}). `docker compose build` + " - "`up` would replace them on an image-hash change, destroying any " - "container-local state they carry. If you intend to rebuild this " - "live deployment, run `docker compose build`/`up` yourself." + f"Refusing to run: {reason}. `docker compose build` + `up` would replace " + "live containers on an image-hash change, destroying any container-local " + "state they carry. If you intend to rebuild this live deployment, run " + "`docker compose build`/`up` yourself." ), )) return result diff --git a/tests/verify/test_environment_and_runner.py b/tests/verify/test_environment_and_runner.py index 2108a2fe36..89905d14a8 100644 --- a/tests/verify/test_environment_and_runner.py +++ b/tests/verify/test_environment_and_runner.py @@ -116,11 +116,7 @@ class TestRunner: class TestComposeGuard: - """Regression for issue #103567: a compose recipe must refuse to run - against a project directory that's already a live compose deployment - with running containers -- ``docker compose build`` + ``up`` replaces - them on an image-hash change, destroying container-local state (a real - incident: a kanban worker's state DB was lost this way).""" + """#103567: a compose recipe must not build/up over a live deployment.""" def _compose_recipe(self): return Recipe( @@ -153,10 +149,7 @@ class TestComposeGuard: result = run_verify(tmp_path, self._compose_recipe(), skip_start=False) assert not result.ok - assert "Refusing to run" in result.phases[0].output_tail - if isinstance(probe, MagicMock) and probe.returncode == 0: - assert "myproject-db-1" in result.phases[0].output_tail - assert "myproject-web-1" in result.phases[0].output_tail + assert result.phases[0].exit_code == 1 # Only the read-only probe ran -- never build or up. assert len(calls) == 1 assert calls[0][:3] == ["docker", "compose", "ps"] @@ -182,7 +175,7 @@ class TestComposeGuard: def test_guard_skipped_when_no_mutating_phase_selected(self, tmp_path, monkeypatch): probe = MagicMock() - monkeypatch.setattr("agent.verify.runner._running_compose_containers", probe) + monkeypatch.setattr("agent.verify.runner._compose_live_state_reason", probe) with patch("agent.verify.runner._run_phase_command") as mock_phase: mock_phase.return_value = MagicMock(ok=True, phase="test")