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

[mrparker0980]

What does this PR do?

DockerEnvironment.execute() has an out-of-band recovery path (added in #39415)
that recreates the container and re-runs the command when it detects the
container has been removed. The problem: it decided "container gone" purely by
substring-scanning the command's own combined stdout/stderr for phrases like
"No such container" / "is not running". Those phrases are extremely common in
legitimate non-zero output — systemctl status x, service x status,
docker compose ps, kubectl, or any script that prints "service X is not
running". When that false positive fired, the live container was torn down and
the user's command was executed a second time. For anything with side effects
(git push, file writes, package install, network POST, a DB migration) that
means silent double execution and lost in-container state. It is on by default
because persist_across_processes defaults to True.

The fix keeps the cheap substring scan as a first-pass filter but no longer
trusts it on its own. Before recovering, execute() now asks Docker directly via
docker inspect -f '{{.State.Running}}' and only proceeds when Docker confirms
the container is actually missing or not running. If the container is still
alive (the common false-positive case) the original result passes through
untouched and the command never re-runs. The probe fails safe: if the daemon is
unreachable or the inspect times out we do not recover, so a live container is
never destroyed on an unverified guess.

Why a positive inspect rather than tighter regex anchoring? Anchoring the
patterns to Docker's CLI error format would shrink the false-positive surface
but not close it — program output is fully attacker/agent-controllable and can
reproduce any prefix. Asking the daemon for ground truth is the only check that
cannot be spoofed by command output.

Related Issue

N/A

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• tools/environments/docker.py: add _container_confirmed_gone() which runs
  docker inspect -f '{{.State.Running}}' and reports gone only when Docker
  says the container is absent (rc != 0) or stopped; gate the recovery branch in
  execute() on it (after the existing returncode / persist / substring
  checks, so no extra docker inspect runs on ordinary failures). Clarify the
  _is_container_gone() docstring that it is only a pre-filter.
• tests/tools/test_docker_environment.py: regression test proving a non-zero
  command whose own output says "is not running" does NOT recover while the
  container is alive; a test that recovery still fires once the probe confirms
  the container is gone; and a unit test of _container_confirmed_gone() across
  running / stopped / missing / daemon-unreachable states.
• scripts/release.py: add the contributor's email to AUTHOR_MAP (release
  gate requirement for new authors).

How to Test

1. uv run python -m pytest tests/tools/test_docker_environment.py -q — all
   pass, including the new test_execute_does_not_recover_when_container_still_running.
2. Reproduce the original bug by reverting tools/environments/docker.py: the
   new regression test fails because _recreate_container is invoked and the
   command runs twice.
3. uv run ruff check tools/environments/docker.py tests/tools/test_docker_environment.py
   and uv run python scripts/check-windows-footguns.py --all — both clean.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix
• I've run the tests and all pass
• I've added tests for my changes (required for bug fixes)
• I've tested on my platform: macOS 15 (Darwin 25.5)

Documentation & Housekeeping

• I've updated relevant documentation (docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture — N/A
• I've considered cross-platform impact (Windows, macOS) — inspect probe and
  timeouts are platform-neutral; no new POSIX-only calls
• I've updated tool descriptions/schemas if I changed tool behavior — N/A

[liuhao1024]

✅ Verified — docker liveness probe prevents false-positive recovery on substring match

Reviewed tools/environments/docker.py _container_confirmed_gone() and execute(), plus 4 new tests in tests/tools/test_docker_environment.py.

• Two-phase guard: The substring scan (_is_container_gone) is kept as a cheap first-pass gate. Only when it matches does _container_confirmed_gone() run docker inspect to positively confirm the container is absent or stopped. This prevents recovery when command output itself contains phrases like "is not running" (e.g., systemctl status nginx)
• Fail-safe on probe failure: subprocess.TimeoutExpired and OSError are caught and return False — a live container is never torn down on an unverified guess. Confirmed in test_container_confirmed_gone_reports_state which explicitly tests the timeout path
• Stopped-but-existing container: Returns True (rc 0, stdout "false") — correct, since a stopped container can't execute commands and recovery is the right response
• Condition ordering: returncode != 0 and _persist_across_processes are checked before the substring scan, avoiding unnecessary string matching on successful commands
• Test coverage: The test_execute_does_not_recover_when_container_still_running test is the critical regression test — it simulates systemctl status output that trips the substring filter while the container is alive, and asserts recovery does NOT fire

Fix correctly addresses the race between substring matching and actual container state. No issues found.

[tonydwb]

Code Review Summary

Verdict: Approved

• The liveness-probe change is sound: container recovery is now gated on _container_confirmed_gone(), which requires docker inspect to confirm the container is absent before recreating and re-running user commands.
• Tests pin the critical edge case: non-zero output containing "not running" while the container is alive no longer triggers recovery.

Minor note: the AUTHOR_MAP addition in scripts/release.py is unrelated to the runtime fix. Not harmful, but it adds release metadata noise to a bugfix PR. Consider separating in future PRs.

Reviewed by Hermes Agent

[teknium1]

Thanks for isolating the command-output false positive; current main still has that problem in tools/environments/docker.py:1183-1190, where a substring match can invoke recovery and rerun the command.

Problems

• tools/environments/docker.py:1007 treats every non-zero docker inspect result as confirmation that the container is gone. Because the probe uses check=False, a daemon/CLI error can be returned normally rather than raised; this still permits recovery on an unverified result, contrary to the fail-safe stated in the PR.
• tests/tools/test_docker_environment.py covers missing, running, stopped, and timeout cases, but not a non-missing non-zero inspect result.

Suggested changes

• Return False for non-zero inspect responses unless their diagnostic specifically establishes that this container is absent, and add the corresponding no-recovery regression test.
• Salvage against current tools/environments/docker.py; the PR is currently conflicting and the Docker subprocess code has moved.

This is an automated hermes-sweeper review.

[teknium1]

This is not fail-safe for every non-zero inspect result: with check=False, daemon/CLI failures arrive here as a CompletedProcess rather than an exception. Please confirm a specific missing-container diagnostic before returning True; otherwise preserve the original command result, and add a regression test for a non-missing non-zero probe.