refactor(sandbox): reuse E2B kill helper during eviction (#4298)

* refactor(sandbox): reuse E2B kill helper during eviction

* test(sandbox): preserve close on kill lookup failure

backend/packages/harness/deerflow/community/e2b_sandbox/e2b_sandbox_provider.py

@@ -924,19 +924,14 @@ class E2BSandboxProvider(SandboxProvider):
             )
             return evict_id
 
-        try:
-            kill = getattr(client, "kill", None)
-            if callable(kill):
-                kill()
-        except Exception as e:
-            logger.warning("Failed to kill evicted e2b sandbox %s: %s", evict_id, e)
-        finally:
-            close = getattr(client, "close", None)
-            if callable(close):
-                try:
-                    close()
-                except Exception:
-                    pass
+        if error := self._kill_client(client):
+            logger.warning("Failed to kill evicted e2b sandbox %s: %s", evict_id, error)
+        close = getattr(client, "close", None)
+        if callable(close):
+            try:
+                close()
+            except Exception:
+                pass
         logger.info("Evicted warm-pool e2b sandbox %s", evict_id)
         return evict_id
 
@@ -1030,12 +1025,11 @@ class E2BSandboxProvider(SandboxProvider):
     ) -> Exception | None:
         """Kill a remote VM and return an exception for the caller to log."""
         if client is None:
-            return
-        kill = getattr(client, "kill", None)
-        if not callable(kill):
-            return
+            return None
         try:
-            kill()
+            kill = getattr(client, "kill", None)
+            if callable(kill):
+                kill()
         except Exception as e:
             return e
         return None

backend/tests/test_e2b_sandbox_provider.py

@@ -575,6 +575,51 @@ def test_kill_client_returns_exception_without_raising():
     assert p._kill_client(client) is error
 
 
+def test_kill_client_ignores_missing_or_uncallable_clients():
+    p = _make_provider()
+
+    assert p._kill_client(None) is None
+    assert p._kill_client(SimpleNamespace()) is None
+
+
+def test_evict_oldest_warm_closes_client_when_kill_lookup_raises(monkeypatch):
+    p = _make_provider()
+    fake_cls = _install_fake_sdk(monkeypatch, p)
+    error = RuntimeError("kill unavailable")
+
+    class ClientWithBrokenKill:
+        def __init__(self) -> None:
+            self.closed = False
+
+        @property
+        def kill(self):
+            raise error
+
+        def close(self) -> None:
+            self.closed = True
+
+    client = ClientWithBrokenKill()
+    fake_cls.connect_factory = lambda _sid, **_kw: client
+    p._warm_pool["sb-warm"] = ("seed", 12345.0)
+
+    assert p._evict_oldest_warm() == "sb-warm"
+    assert client.closed is True
+
+
+def test_evict_oldest_warm_uses_kill_helper_and_closes_client(monkeypatch):
+    p = _make_provider()
+    fake_cls = _install_fake_sdk(monkeypatch, p)
+    client = FakeClient(sandbox_id="sb-warm")
+    fake_cls.connect_factory = lambda _sid, **_kw: client
+    p._warm_pool["sb-warm"] = ("seed", 12345.0)
+    kill_client = MagicMock(return_value=None)
+    p._kill_client = kill_client
+
+    assert p._evict_oldest_warm() == "sb-warm"
+    kill_client.assert_called_once_with(client)
+    assert client.closed is True
+
+
 def test_discover_remote_sandbox_returns_none_when_list_raises(monkeypatch):
     p = _make_provider()
     fake_cls = _install_fake_sdk(monkeypatch, p)