fix: clearer docker backend preflight errors (#1276)
* feat: improve context compaction handoff summaries Adapt PR #916 onto current main by replacing the old context summary marker with a clearer handoff wrapper, updating the summarization prompt for resume-oriented summaries, and preserving the current call_llm-based compression path. * fix: clearer error when docker backend is unavailable * fix: preserve docker discovery in backend preflight Follow up on salvaged PR #940 by reusing find_docker() during the new availability check so non-PATH Docker Desktop installs still work. Add a regression test covering the resolved executable path. --------- Co-authored-by: aydnOktay <xaydinoktay@gmail.com>
This commit is contained in:
parent
486cb772b8
commit
6036793f60
2 changed files with 158 additions and 0 deletions
88
tests/tools/test_docker_environment.py
Normal file
88
tests/tools/test_docker_environment.py
Normal file
|
|
@ -0,0 +1,88 @@
|
||||||
|
import logging
|
||||||
|
import subprocess
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from tools.environments import docker as docker_env
|
||||||
|
|
||||||
|
|
||||||
|
def _make_dummy_env(**kwargs):
|
||||||
|
"""Helper to construct DockerEnvironment with minimal required args."""
|
||||||
|
return docker_env.DockerEnvironment(
|
||||||
|
image=kwargs.get("image", "python:3.11"),
|
||||||
|
cwd=kwargs.get("cwd", "/root"),
|
||||||
|
timeout=kwargs.get("timeout", 60),
|
||||||
|
cpu=kwargs.get("cpu", 0),
|
||||||
|
memory=kwargs.get("memory", 0),
|
||||||
|
disk=kwargs.get("disk", 0),
|
||||||
|
persistent_filesystem=kwargs.get("persistent_filesystem", False),
|
||||||
|
task_id=kwargs.get("task_id", "test-task"),
|
||||||
|
volumes=kwargs.get("volumes", []),
|
||||||
|
network=kwargs.get("network", True),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_ensure_docker_available_logs_and_raises_when_not_found(monkeypatch, caplog):
|
||||||
|
"""When docker cannot be found, raise a clear error before mini-swe setup."""
|
||||||
|
|
||||||
|
monkeypatch.setattr(docker_env, "find_docker", lambda: None)
|
||||||
|
monkeypatch.setattr(
|
||||||
|
docker_env.subprocess,
|
||||||
|
"run",
|
||||||
|
lambda *args, **kwargs: pytest.fail("subprocess.run should not be called when docker is missing"),
|
||||||
|
)
|
||||||
|
|
||||||
|
with caplog.at_level(logging.ERROR):
|
||||||
|
with pytest.raises(RuntimeError) as excinfo:
|
||||||
|
_make_dummy_env()
|
||||||
|
|
||||||
|
assert "Docker executable not found in PATH or known install locations" in str(excinfo.value)
|
||||||
|
assert any(
|
||||||
|
"no docker executable was found in PATH or known install locations"
|
||||||
|
in record.getMessage()
|
||||||
|
for record in caplog.records
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_ensure_docker_available_logs_and_raises_on_timeout(monkeypatch, caplog):
|
||||||
|
"""When docker version times out, surface a helpful error instead of hanging."""
|
||||||
|
|
||||||
|
def _raise_timeout(*args, **kwargs):
|
||||||
|
raise subprocess.TimeoutExpired(cmd=["/custom/docker", "version"], timeout=5)
|
||||||
|
|
||||||
|
monkeypatch.setattr(docker_env, "find_docker", lambda: "/custom/docker")
|
||||||
|
monkeypatch.setattr(docker_env.subprocess, "run", _raise_timeout)
|
||||||
|
|
||||||
|
with caplog.at_level(logging.ERROR):
|
||||||
|
with pytest.raises(RuntimeError) as excinfo:
|
||||||
|
_make_dummy_env()
|
||||||
|
|
||||||
|
assert "Docker daemon is not responding" in str(excinfo.value)
|
||||||
|
assert any(
|
||||||
|
"/custom/docker version' timed out" in record.getMessage()
|
||||||
|
for record in caplog.records
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_ensure_docker_available_uses_resolved_executable(monkeypatch):
|
||||||
|
"""When docker is found outside PATH, preflight should use that resolved path."""
|
||||||
|
|
||||||
|
calls = []
|
||||||
|
|
||||||
|
def _run(cmd, **kwargs):
|
||||||
|
calls.append((cmd, kwargs))
|
||||||
|
return subprocess.CompletedProcess(cmd, 0, stdout="Docker version", stderr="")
|
||||||
|
|
||||||
|
monkeypatch.setattr(docker_env, "find_docker", lambda: "/opt/homebrew/bin/docker")
|
||||||
|
monkeypatch.setattr(docker_env.subprocess, "run", _run)
|
||||||
|
|
||||||
|
docker_env._ensure_docker_available()
|
||||||
|
|
||||||
|
assert calls == [
|
||||||
|
(["/opt/homebrew/bin/docker", "version"], {
|
||||||
|
"capture_output": True,
|
||||||
|
"text": True,
|
||||||
|
"timeout": 5,
|
||||||
|
})
|
||||||
|
]
|
||||||
|
|
||||||
|
|
@ -82,6 +82,72 @@ _SECURITY_ARGS = [
|
||||||
_storage_opt_ok: Optional[bool] = None # cached result across instances
|
_storage_opt_ok: Optional[bool] = None # cached result across instances
|
||||||
|
|
||||||
|
|
||||||
|
def _ensure_docker_available() -> None:
|
||||||
|
"""Best-effort check that the docker CLI is available before use.
|
||||||
|
|
||||||
|
Reuses ``find_docker()`` so this preflight stays consistent with the rest of
|
||||||
|
the Docker backend, including known non-PATH Docker Desktop locations.
|
||||||
|
"""
|
||||||
|
docker_exe = find_docker()
|
||||||
|
if not docker_exe:
|
||||||
|
logger.error(
|
||||||
|
"Docker backend selected but no docker executable was found in PATH "
|
||||||
|
"or known install locations. Install Docker Desktop and ensure the "
|
||||||
|
"CLI is available."
|
||||||
|
)
|
||||||
|
raise RuntimeError(
|
||||||
|
"Docker executable not found in PATH or known install locations. "
|
||||||
|
"Install Docker and ensure the 'docker' command is available."
|
||||||
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
result = subprocess.run(
|
||||||
|
[docker_exe, "version"],
|
||||||
|
capture_output=True,
|
||||||
|
text=True,
|
||||||
|
timeout=5,
|
||||||
|
)
|
||||||
|
except FileNotFoundError:
|
||||||
|
logger.error(
|
||||||
|
"Docker backend selected but the resolved docker executable '%s' could "
|
||||||
|
"not be executed.",
|
||||||
|
docker_exe,
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
raise RuntimeError(
|
||||||
|
"Docker executable could not be executed. Check your Docker installation."
|
||||||
|
)
|
||||||
|
except subprocess.TimeoutExpired:
|
||||||
|
logger.error(
|
||||||
|
"Docker backend selected but '%s version' timed out. "
|
||||||
|
"The Docker daemon may not be running.",
|
||||||
|
docker_exe,
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
raise RuntimeError(
|
||||||
|
"Docker daemon is not responding. Ensure Docker is running and try again."
|
||||||
|
)
|
||||||
|
except Exception:
|
||||||
|
logger.error(
|
||||||
|
"Unexpected error while checking Docker availability.",
|
||||||
|
exc_info=True,
|
||||||
|
)
|
||||||
|
raise
|
||||||
|
else:
|
||||||
|
if result.returncode != 0:
|
||||||
|
logger.error(
|
||||||
|
"Docker backend selected but '%s version' failed "
|
||||||
|
"(exit code %d, stderr=%s)",
|
||||||
|
docker_exe,
|
||||||
|
result.returncode,
|
||||||
|
result.stderr.strip(),
|
||||||
|
)
|
||||||
|
raise RuntimeError(
|
||||||
|
"Docker command is available but 'docker version' failed. "
|
||||||
|
"Check your Docker installation."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
class DockerEnvironment(BaseEnvironment):
|
class DockerEnvironment(BaseEnvironment):
|
||||||
"""Hardened Docker container execution with resource limits and persistence.
|
"""Hardened Docker container execution with resource limits and persistence.
|
||||||
|
|
||||||
|
|
@ -120,6 +186,10 @@ class DockerEnvironment(BaseEnvironment):
|
||||||
logger.warning(f"docker_volumes config is not a list: {volumes!r}")
|
logger.warning(f"docker_volumes config is not a list: {volumes!r}")
|
||||||
volumes = []
|
volumes = []
|
||||||
|
|
||||||
|
# Fail fast if Docker is not available rather than surfacing a cryptic
|
||||||
|
# FileNotFoundError deep inside the mini-swe-agent stack.
|
||||||
|
_ensure_docker_available()
|
||||||
|
|
||||||
from minisweagent.environments.docker import DockerEnvironment as _Docker
|
from minisweagent.environments.docker import DockerEnvironment as _Docker
|
||||||
|
|
||||||
# Build resource limit args
|
# Build resource limit args
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue