fix(docker): confirm container is gone before recovery re-runs the command

scripts/release.py

@@ -45,6 +45,7 @@
 
 # Auto-extracted from noreply emails + manual overrides
 AUTHOR_MAP = {
+    "290881485+mrparker0980@users.noreply.github.com": "mrparker0980",
     "yusufalweshdemir@gmail.com": "Dusk1e",
     "804436395@qq.com": "LaPhilosophie",
     "266365592+bmoore210@users.noreply.github.com": "bmoore210",

tests/tools/test_docker_environment.py

@@ -1833,3 +1833,134 @@ def _fail_recreate(self):
     result = env.execute("badcmd")
     assert result.get("returncode") == 127
     assert "command not found" in result.get("output", "")
+
+
+def _patch_inspect(monkeypatch, running):
+    """Make ``docker inspect -f '{{.State.Running}}'`` report *running*.
+
+    *running* may be True/False (container exists) to emit the corresponding
+    ``true``/``false`` line with rc 0, or the string ``"missing"`` to emulate
+    a removed container (rc 1, "No such object"). Other docker subcommands fall
+    back to the standard mock so container creation during recovery still works.
+    """
+    base_calls = _mock_subprocess_run(monkeypatch)
+    real_run = docker_env.subprocess.run
+
+    def _run(cmd, **kwargs):
+        if isinstance(cmd, list) and len(cmd) >= 2 and cmd[1] == "inspect":
+            if running == "missing":
+                return subprocess.CompletedProcess(
+                    cmd, 1, stdout="", stderr="Error: No such object: x"
+                )
+            return subprocess.CompletedProcess(
+                cmd, 0, stdout="true\n" if running else "false\n", stderr=""
+            )
+        return real_run(cmd, **kwargs)
+
+    monkeypatch.setattr(docker_env.subprocess, "run", _run)
+    return base_calls
+
+
+def test_execute_does_not_recover_when_container_still_running(monkeypatch):
+    """Regression: a non-zero command whose *own output* contains a
+    container-gone phrase (e.g. ``systemctl status`` printing "is not running")
+    must NOT tear down and re-run the command while the container is alive.
+
+    Before the liveness probe was added, the substring scan alone fired
+    recovery here, recreating the container and executing the user's command a
+    second time — duplicating any side effects.
+    """
+    monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
+    _patch_inspect(monkeypatch, running=True)
+    env = _make_dummy_env(
+        persistent_filesystem=True,
+        persist_across_processes=True,
+    )
+
+    super_calls = []
+
+    def _fake_super_execute(self, command, cwd="", **kwargs):
+        super_calls.append(command)
+        # Mimics `systemctl status nginx` on a stopped unit: real, non-zero,
+        # and the text trips the substring filter.
+        return {"output": "Unit nginx.service is not running", "returncode": 3}
+
+    def _fail_recreate(self):
+        pytest.fail("recreation must not run while the container is alive")
+
+    monkeypatch.setattr(docker_env.BaseEnvironment, "execute", _fake_super_execute)
+    monkeypatch.setattr(
+        docker_env.DockerEnvironment, "_recreate_container", _fail_recreate
+    )
+
+    result = env.execute("systemctl status nginx")
+
+    assert super_calls == ["systemctl status nginx"], (
+        "the command must run exactly once when the container is still alive"
+    )
+    assert result.get("returncode") == 3
+    assert "is not running" in result.get("output", "")
+
+
+def test_execute_recovers_only_after_liveness_probe_confirms_gone(monkeypatch):
+    """The full recovery path still fires when the substring filter matches
+    AND ``docker inspect`` confirms the container is genuinely gone.
+    """
+    monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
+    _patch_inspect(monkeypatch, running="missing")
+    env = _make_dummy_env(
+        persistent_filesystem=True,
+        persist_across_processes=True,
+    )
+
+    outputs = iter([
+        {"output": "Error response from daemon: No such container: hermes-x", "returncode": 1},
+        {"output": "ok", "returncode": 0},
+    ])
+
+    def _fake_super_execute(self, command, cwd="", **kwargs):
+        return next(outputs)
+
+    recreate_calls = []
+
+    def _fake_recreate(self):
+        recreate_calls.append(True)
+        self._container_id = "recovered-container-id"
+        return True
+
+    monkeypatch.setattr(docker_env.BaseEnvironment, "execute", _fake_super_execute)
+    monkeypatch.setattr(
+        docker_env.DockerEnvironment, "_recreate_container", _fake_recreate
+    )
+
+    result = env.execute("echo hi")
+
+    assert recreate_calls == [True], "recovery should fire once the probe confirms the container is gone"
+    assert result.get("returncode") == 0
+    assert result.get("output") == "ok"
+
+
+def test_container_confirmed_gone_reports_state(monkeypatch):
+    """``_container_confirmed_gone`` reflects the real docker inspect result:
+    running → False, stopped → True, missing → True, and fails safe (False)
+    when the probe can't reach the daemon.
+    """
+    monkeypatch.setattr(docker_env, "find_docker", lambda: "/usr/bin/docker")
+
+    _patch_inspect(monkeypatch, running=True)
+    env = _make_dummy_env(persistent_filesystem=True, persist_across_processes=True)
+    env._container_id = "live"
+    assert env._container_confirmed_gone() is False
+
+    _patch_inspect(monkeypatch, running=False)
+    assert env._container_confirmed_gone() is True
+
+    _patch_inspect(monkeypatch, running="missing")
+    assert env._container_confirmed_gone() is True
+
+    # Daemon unreachable / probe times out → fail safe, never recover blindly.
+    def _boom(cmd, **kwargs):
+        raise subprocess.TimeoutExpired(cmd, 10)
+
+    monkeypatch.setattr(docker_env.subprocess, "run", _boom)
+    assert env._container_confirmed_gone() is False

tools/environments/docker.py

@@ -966,9 +966,51 @@ def _run_bash(self, cmd_string: str, *, login: bool = False,
     )
 
     def _is_container_gone(self, output: str) -> bool:
-        """Return True if the output indicates the container no longer exists."""
+        """Return True if the output *might* indicate the container is gone.
+
+        This is only a cheap first-pass filter: the phrases live in
+        ``_NO_CONTAINER_PATTERNS`` precisely because Docker emits them when an
+        ``exec`` target has vanished, but they also occur verbatim in perfectly
+        ordinary command output (``systemctl status``, ``docker compose ps``, a
+        script echoing "service X is not running"). A match here is necessary
+        but NOT sufficient to trigger recovery — :meth:`execute` confirms the
+        container's real state via :meth:`_container_confirmed_gone` before
+        tearing anything down.
+        """
         return any(p in output for p in self._NO_CONTAINER_PATTERNS)
 
+    def _container_confirmed_gone(self) -> bool:
+        """Ask Docker directly whether the current container is dead.
+
+        Recovery re-creates the container *and re-runs the user's command*, so
+        a false positive duplicates every side effect (a second ``git push``,
+        file write, package install, network POST). We must therefore never
+        rely on the substring scan in :meth:`_is_container_gone` alone — that
+        scans command-controlled output. Instead we run ``docker inspect`` and
+        only report the container gone when Docker itself confirms it is absent
+        or not running.
+
+        Fail safe: if the probe can't reach the daemon or times out we return
+        ``False`` so a live container is never torn down on an unverified guess
+        (the original error is surfaced to the caller unchanged).
+        """
+        if not self._container_id:
+            return True
+        try:
+            probe = subprocess.run(
+                [self._docker_exe, "inspect", "-f", "{{.State.Running}}", self._container_id],
+                capture_output=True, text=True, timeout=10, check=False,
+            )
+        except (subprocess.TimeoutExpired, OSError) as e:
+            logger.debug("container liveness probe failed: %s — skipping recovery", e)
+            return False
+        if probe.returncode != 0:
+            # Non-zero almost always means "Error: No such object" — the
+            # container is genuinely gone, so recovery is warranted.
+            return True
+        # rc 0: stdout is "true" (running) or "false" (exists but stopped).
+        return probe.stdout.strip().lower() != "true"
+
     def _recreate_container(self) -> bool:
         """Recreate the container after it was removed out-of-band.
 
@@ -1057,10 +1099,16 @@ def execute(self, command: str, cwd: str = "", **kwargs) -> dict:
         transparently before retrying once.
         """
         result = super().execute(command, cwd, **kwargs)
+        # The substring scan and persist flag are cheap gates kept first so we
+        # don't spawn a ``docker inspect`` after every non-zero command. Only
+        # once they pass do we positively confirm the container is actually
+        # gone — sniffing command output alone would re-run side-effecting
+        # commands whenever their *own* output mentioned "is not running".
         if (
             result.get("returncode", 0) != 0
-            and self._is_container_gone(result.get("output", ""))
             and self._persist_across_processes
+            and self._is_container_gone(result.get("output", ""))
+            and self._container_confirmed_gone()
         ):
             if self._recreate_container():
                 result = super().execute(command, cwd, **kwargs)