From 682268ff07c3d558119a44c2e60b457641ef737f Mon Sep 17 00:00:00 2001 From: Panopticon Agent Date: Thu, 9 Jul 2026 23:43:25 +0000 Subject: [PATCH] feat(sessionservice): surface container exit reason in reconcile When a container exits non-zero before registering (e.g. docker-in-docker timeout, broken image layer), reconcile() now calls get_container_failure() on the runner and reports FAILED with the exit code + last 20 log lines instead of silently composing `down` with no context. Co-Authored-By: Claude Sonnet 4.6 --- src/panopticon/sessionservice/local_runner.py | 34 ++++++++++++++++ src/panopticon/sessionservice/runner.py | 4 ++ src/panopticon/sessionservice/spawner.py | 6 ++- tests/test_local_runner.py | 39 +++++++++++++++++++ tests/test_spawner.py | 31 +++++++++++++-- 5 files changed, 110 insertions(+), 4 deletions(-) diff --git a/src/panopticon/sessionservice/local_runner.py b/src/panopticon/sessionservice/local_runner.py index 3a73599d..203a0675 100644 --- a/src/panopticon/sessionservice/local_runner.py +++ b/src/panopticon/sessionservice/local_runner.py @@ -212,6 +212,40 @@ def is_running(self, task_id: str) -> bool: ) return bool(names.strip()) + def get_container_failure(self, task_id: str) -> str | None: + """Return a failure detail string if the task's container exited non-zero, else None. + + Uses ``docker inspect`` to check exit code; if non-zero, appends the last 20 lines + of ``docker logs`` so the dashboard can show *why* the container died rather than + just surfacing ``down``. Returns ``None`` when the container is running, clean-exited + (code 0), or already gone (inspect returns nothing).""" + container = f"panopticon-{task_id}" + raw = self._run( + ["docker", "inspect", + "--format", "{{.State.ExitCode}}\t{{.State.Error}}", + container], + check=False, + ).strip() + if not raw: + return None # container is gone entirely — nothing to inspect + exit_code_str, _, state_error = raw.partition("\t") + try: + exit_code = int(exit_code_str) + except ValueError: + return None + if exit_code == 0: + return None # clean exit — let normal down handling proceed + logs = self._run( + ["docker", "logs", "--tail", "20", container], + check=False, + ).strip() + detail = f"container exited {exit_code}" + if state_error: + detail += f" ({state_error})" + if logs: + detail += f"\n{logs}" + return detail + def has_session(self, task_id: str) -> bool: """Whether the task's host tmux session exists on this runner's tmux server. diff --git a/src/panopticon/sessionservice/runner.py b/src/panopticon/sessionservice/runner.py index e3fc4512..73d9ee2a 100644 --- a/src/panopticon/sessionservice/runner.py +++ b/src/panopticon/sessionservice/runner.py @@ -30,3 +30,7 @@ def spawn(self, task_id: str) -> str: @abstractmethod def stop(self, container_id: str) -> None: """Stop the container and tear down its tmux session. Idempotent.""" + + def get_container_failure(self, task_id: str) -> str | None: + """Return a failure detail string if the container exited non-zero, else None.""" + return None # default: backends that don't support this return None diff --git a/src/panopticon/sessionservice/spawner.py b/src/panopticon/sessionservice/spawner.py index 7048c1db..5dd3b6de 100644 --- a/src/panopticon/sessionservice/spawner.py +++ b/src/panopticon/sessionservice/spawner.py @@ -204,7 +204,11 @@ def reconcile(self, task: JsonObj) -> None: return # live / down / failed / queued / disconnected — nothing to reconcile if self._runner.is_running(task["id"]): return # container present, just not registered yet — still coming up - self._client.clear_lifecycle(task["id"]) # container gone → composes `down` + failure = self._runner.get_container_failure(task["id"]) + if failure: + self._report(task["id"], LifecyclePhase.FAILED, detail=failure) + else: + self._client.clear_lifecycle(task["id"]) # container gone → composes `down` def _is_orphan(self, task: JsonObj) -> bool: """Whether ``task`` is an orphan **this** runner should self-heal: claimed by us, diff --git a/tests/test_local_runner.py b/tests/test_local_runner.py index 13c8593c..3f6d6c8e 100644 --- a/tests/test_local_runner.py +++ b/tests/test_local_runner.py @@ -222,6 +222,45 @@ def test_spawn_uses_the_composed_image_when_given_else_the_base() -> None: assert rec.calls[6][0][-1] == "panopticon-github-peer-reviewed-r1" +class _SequenceRecorder(_Recorder): + """Returns successive canned stdout values for each call in order; empty string thereafter.""" + + def __init__(self, outputs: list[str]) -> None: + super().__init__() + self._outputs = iter(outputs) + + def __call__(self, args: Sequence[str], *, check: bool = True, interactive: bool = False, verbose: bool = False) -> str: + super().__call__(args, check=check, interactive=interactive) + return next(self._outputs, "") + + +def test_get_container_failure_nonzero_exit() -> None: + # Container exited 1 with no state error but log lines → detail contains exit code + logs. + rec = _SequenceRecorder(["1\t", "entrypoint: docker daemon timed out"]) + runner = LocalRunner("http://svc:8000", run=rec) + result = runner.get_container_failure("t1") + inspect_cmd, logs_cmd = rec.calls[0][0], rec.calls[1][0] + assert inspect_cmd == ["docker", "inspect", "--format", "{{.State.ExitCode}}\t{{.State.Error}}", "panopticon-t1"] + assert rec.calls[0][1] is False # check=False — tolerate a missing container + assert logs_cmd == ["docker", "logs", "--tail", "20", "panopticon-t1"] + assert rec.calls[1][1] is False + assert result is not None + assert "1" in result + assert "entrypoint: docker daemon timed out" in result + + +def test_get_container_failure_zero_exit() -> None: + # Container exited cleanly (exit code 0) → None; down handling proceeds normally. + rec = _SequenceRecorder(["0\t"]) + assert LocalRunner("http://svc:8000", run=rec).get_container_failure("t1") is None + + +def test_get_container_failure_container_gone() -> None: + # Container already removed (inspect returns nothing) → None; treat as clean gone. + rec = _SequenceRecorder([""]) + assert LocalRunner("http://svc:8000", run=rec).get_container_failure("t1") is None + + def test_stop_kills_session_and_force_removes_container_idempotently() -> None: rec = _Recorder() LocalRunner("http://svc", run=rec).stop("panopticon-t1") diff --git a/tests/test_spawner.py b/tests/test_spawner.py index 4c02b6b2..10ec7ff9 100644 --- a/tests/test_spawner.py +++ b/tests/test_spawner.py @@ -30,13 +30,14 @@ def _no_op_run(args: object, *, check: bool = True) -> str: class _FakeRunner: """Records spawn calls; stands in for LocalRunner. Mimics its ``progress`` callbacks (STARTING - then AWAITING), ``is_running`` (for reconcile/down-detection) and ``has_session`` (for heal/ - self-heal) — both configurable.""" + then AWAITING), ``is_running`` (for reconcile/down-detection), ``has_session`` (for heal/ + self-heal), and ``get_container_failure`` (for reconcile failure surfacing) — all configurable.""" - def __init__(self, *, running: bool = True, session: bool = True) -> None: + def __init__(self, *, running: bool = True, session: bool = True, failure: str | None = None) -> None: self.spawned: list[dict[str, object]] = [] self._running = running self._session = session + self._failure = failure def spawn(self, task_id: str, *, env_file: str | None = None, workspace: str | None = None, image: str | None = None, docker_in_docker: bool = False, initial_prompt: str | None = None, turn: str | None = None, progress: Callable[[LifecyclePhase], None] | None = None) -> str: self.spawned.append({"task_id": task_id, "env_file": env_file, "workspace": workspace, "image": image, "docker_in_docker": docker_in_docker, "initial_prompt": initial_prompt, "turn": turn}) @@ -51,6 +52,9 @@ def is_running(self, task_id: str) -> bool: def has_session(self, task_id: str) -> bool: return self._session + def get_container_failure(self, task_id: str) -> str | None: + return self._failure + def stop(self, container_id: str) -> None: pass @@ -247,6 +251,27 @@ def test_reconcile_leaves_a_still_running_container_alone() -> None: assert client.cleared == [] # container present, just not registered yet — keep coming up +def test_reconcile_reports_failed_when_container_exited_nonzero() -> None: + # Container stopped with a non-zero exit code → surface FAILED with the detail instead of `down`. + detail = "container exited 1\nentrypoint: gosu not found" + client, runner = _FakeClient(repo=_REPO), _FakeRunner(running=False, failure=detail) + _spawner(client, runner).reconcile( + {"id": "t1", "claimed_by": "host-1", "container_status": "awaiting", "state": "ITERATING"} + ) + assert client.cleared == [] # clear_lifecycle was NOT called + assert any(phase == "failed" and det == detail for _, phase, det in client.phases) + + +def test_reconcile_clears_lifecycle_when_container_has_no_failure_detail() -> None: + # Container gone but get_container_failure returns None (clean exit or already removed) → `down`. + client, runner = _FakeClient(repo=_REPO), _FakeRunner(running=False, failure=None) + _spawner(client, runner).reconcile( + {"id": "t1", "claimed_by": "host-1", "container_status": "awaiting", "state": "ITERATING"} + ) + assert client.cleared == ["t1"] # existing behaviour preserved + assert not any(phase == "failed" for _, phase, _ in client.phases) + + def test_reconcile_ignores_tasks_not_in_flight_or_not_ours() -> None: client, runner = _FakeClient(repo=_REPO), _FakeRunner(running=False) spawner = _spawner(client, runner)