fix(runtime): swallow torn-down docker socket in sandbox delete() (#721)

StrixDockerSandboxClient.delete() best-effort-kills the sandbox container via
containers.get(id).kill() before delegating to the SDK's delete(), suppressing
docker NotFound/APIError. But when the docker daemon socket is already going
away — the normal case on a host/CI teardown — containers.get() ->
inspect_container raises requests' ConnectionError, which is a *sibling* of
docker.errors.APIError under requests.RequestException, not a subclass. So it
escapes the APIError-only suppress and surfaces a full traceback on teardown
even though the kill is meant to be best-effort.

Add RequestException to the suppress so the best-effort kill is genuinely
best-effort regardless of daemon reachability.

Test: tests/test_docker_client_delete.py — the kill raising ConnectionError
(and NotFound/APIError) is swallowed and delete() still delegates; unrelated
errors still propagate; no-container_id is a no-op. The ConnectionError case
fails against the pre-fix APIError-only suppress.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

strix/runtime/docker_client.py

@@ -40,6 +40,7 @@ from docker import errors as docker_errors  # type: ignore[import-untyped, unuse
 from docker.models.containers import Container  # type: ignore[import-untyped, unused-ignore]
 from docker.types import Mount as DockerSDKMount  # type: ignore[import-untyped, unused-ignore]
 from docker.utils import parse_repository_tag  # type: ignore[import-untyped, unused-ignore]
+from requests.exceptions import RequestException
 
 
 logger = logging.getLogger(__name__)
@@ -148,6 +149,15 @@ class StrixDockerSandboxClient(DockerSandboxClient):
     async def delete(self, session: SandboxSession) -> SandboxSession:
         container_id = getattr(getattr(session._inner, "state", None), "container_id", None)
         if container_id:
-            with contextlib.suppress(docker_errors.NotFound, docker_errors.APIError):
+            # Best-effort kill: NotFound/APIError cover a gone or unhappy
+            # container. RequestException covers a torn-down daemon socket —
+            # containers.get() -> inspect_container raises requests'
+            # ConnectionError, which is a sibling of docker.errors.APIError
+            # under requests.RequestException (not a subclass), so it escapes
+            # an APIError-only suppress and surfaces a full traceback even
+            # though this teardown is meant to be best-effort.
+            with contextlib.suppress(
+                docker_errors.NotFound, docker_errors.APIError, RequestException
+            ):
                 self.docker_client.containers.get(container_id).kill()
         return await super().delete(session)

tests/test_docker_client_delete.py

@@ -0,0 +1,86 @@
+"""StrixDockerSandboxClient.delete() best-effort teardown.
+
+delete() kills the sandbox container before delegating to the SDK's delete().
+The kill is meant to be best-effort, but the ``contextlib.suppress`` around it
+must cover the case where the docker daemon socket is already gone: then
+``containers.get()`` -> ``inspect_container`` raises requests'
+``ConnectionError``, which is a *sibling* of ``docker.errors.APIError`` under
+``requests.RequestException`` (not a subclass), so an APIError-only suppress
+would let it escape and surface a traceback on every teardown.
+"""
+
+from __future__ import annotations
+
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from agents.sandbox.sandboxes.docker import DockerSandboxClient
+from docker import errors as docker_errors
+from requests.exceptions import ConnectionError as RequestsConnectionError
+
+from strix.runtime.docker_client import StrixDockerSandboxClient
+
+
+def _client_with_kill_error(exc: Exception) -> StrixDockerSandboxClient:
+    """A StrixDockerSandboxClient whose containers.get(...).kill() raises ``exc``."""
+    client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient)
+    docker_client = MagicMock()
+    docker_client.containers.get.side_effect = exc
+    client.docker_client = docker_client
+    return client
+
+
+def _session() -> object:
+    # delete() reads session._inner.state.container_id
+    return SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id="abc123")))
+
+
+@pytest.mark.parametrize(
+    "exc",
+    [
+        RequestsConnectionError("Connection aborted", FileNotFoundError(2, "No such file")),
+        docker_errors.NotFound("gone"),
+        docker_errors.APIError("unhappy"),
+    ],
+)
+@pytest.mark.asyncio
+async def test_delete_swallows_best_effort_kill_errors(exc):
+    """A torn-down socket (ConnectionError) or a gone/unhappy container
+    (NotFound/APIError) during the kill must not propagate; delete() still
+    delegates to the SDK's delete()."""
+    client = _client_with_kill_error(exc)
+    session = _session()
+
+    with patch.object(
+        DockerSandboxClient, "delete", new=AsyncMock(return_value=session)
+    ) as super_delete:
+        result = await client.delete(session)
+
+    assert result is session
+    super_delete.assert_awaited_once()  # teardown proceeded despite the kill error
+
+
+@pytest.mark.asyncio
+async def test_delete_does_not_swallow_unrelated_errors():
+    """A programming error (e.g. ValueError) is not part of best-effort kill and
+    must still propagate."""
+    client = _client_with_kill_error(ValueError("boom"))
+    with pytest.raises(ValueError):
+        await client.delete(_session())
+
+
+@pytest.mark.asyncio
+async def test_delete_noop_without_container_id():
+    """No container_id -> no kill attempt, just delegate."""
+    client = StrixDockerSandboxClient.__new__(StrixDockerSandboxClient)
+    client.docker_client = MagicMock()
+    session = SimpleNamespace(_inner=SimpleNamespace(state=SimpleNamespace(container_id=None)))
+
+    with patch.object(
+        DockerSandboxClient, "delete", new=AsyncMock(return_value=session)
+    ) as super_delete:
+        await client.delete(session)
+
+    client.docker_client.containers.get.assert_not_called()
+    super_delete.assert_awaited_once()