fix(shipyard-neo): add readiness gate and graceful sandbox cleanup (#7881)

* fix(shipyard-neo): add readiness gate and graceful sandbox cleanup

* fix:  Add **kwargs to ComputerBooter.shutdown()

* test(shipyard-neo): add tests for readiness gate and shutdown behavior

astrbot/core/computer/booters/base.py

@@ -36,7 +36,15 @@ class ComputerBooter:
 
     async def boot(self, session_id: str) -> None: ...
 
-    async def shutdown(self) -> None: ...
+    async def shutdown(self, **kwargs) -> None:
+        """Shut down the computer sandbox.
+
+        Subclasses may accept extra keyword arguments for
+        type-specific cleanup (e.g. ``delete_sandbox`` for
+        ShipyardNeoBooter).  The default implementation ignores
+        them.
+        """
+        ...
 
     async def upload_file(self, path: str, file_name: str) -> dict:
         """Upload file to the computer.

astrbot/core/computer/booters/shipyard_neo.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import asyncio
 import os
 import shlex
 from typing import Any, cast
@@ -438,6 +439,9 @@ class ShipyardNeoBooter(ComputerBooter):
             ttl=self._ttl,
         )
 
+        # --- Readiness gate: wait until sandbox session is READY ---
+        await self._wait_until_ready(self._sandbox)
+
         self._shell = NeoShellComponent(self._sandbox)
         self._fs = NeoFileSystemComponent(self._sandbox, self._shell)
         self._python = NeoPythonComponent(self._sandbox)
@@ -455,6 +459,78 @@ class ShipyardNeoBooter(ComputerBooter):
             bool(self._bay_manager),
         )
 
+    async def _wait_until_ready(self, sandbox: Sandbox) -> None:
+        """Poll sandbox status until READY, or raise on FAILED / timeout.
+
+        Covers both warm-pool hits (near-instant) and cold starts (up to 180s).
+        On FAILED, EXPIRED, or timeout the sandbox is deleted before raising
+        so no orphan resources leak on Bay.
+        """
+        READINESS_TIMEOUT = 180  # seconds
+        POLL_INTERVAL = 2  # seconds
+
+        sandbox_id = sandbox.id
+        deadline = asyncio.get_running_loop().time() + READINESS_TIMEOUT
+
+        while True:
+            await sandbox.refresh()
+            status = getattr(sandbox.status, "value", str(sandbox.status))
+
+            if status == "ready":
+                logger.info(
+                    "[Computer] Sandbox %s is ready (profile=%s)",
+                    sandbox_id,
+                    sandbox.profile,
+                )
+                return
+
+            if status in {"failed", "expired"}:
+                logger.error(
+                    "[Computer] Sandbox %s reached terminal state: %s",
+                    sandbox_id,
+                    status,
+                )
+                try:
+                    await sandbox.delete()
+                except Exception as del_err:
+                    logger.warning(
+                        "[Computer] Failed to delete failed sandbox %s: %s",
+                        sandbox_id,
+                        del_err,
+                    )
+                raise RuntimeError(
+                    f"Sandbox {sandbox_id} is in terminal state: {status}"
+                )
+
+            remaining = deadline - asyncio.get_running_loop().time()
+            if remaining <= 0:
+                logger.error(
+                    "[Computer] Sandbox %s did not become ready within %ds "
+                    "(last status: %s)",
+                    sandbox_id,
+                    READINESS_TIMEOUT,
+                    status,
+                )
+                try:
+                    await sandbox.delete()
+                except Exception as del_err:
+                    logger.warning(
+                        "[Computer] Failed to delete timed-out sandbox %s: %s",
+                        sandbox_id,
+                        del_err,
+                    )
+                raise TimeoutError(
+                    f"Sandbox {sandbox_id} did not become ready within "
+                    f"{READINESS_TIMEOUT}s (last status: {status})"
+                )
+
+            logger.debug(
+                "[Computer] Sandbox %s status=%s, waiting...",
+                sandbox_id,
+                status,
+            )
+            await asyncio.sleep(POLL_INTERVAL)
+
     async def _resolve_profile(self, client: Any) -> str:
         """Pick the best profile for this session.
 
@@ -510,16 +586,41 @@ class ShipyardNeoBooter(ComputerBooter):
 
         return chosen
 
-    async def shutdown(self) -> None:
+    async def shutdown(self, *, delete_sandbox: bool = False) -> None:
         if self._client is not None:
             sandbox_id = getattr(self._sandbox, "id", "unknown")
+
+            # Delete sandbox on Bay BEFORE closing the HTTP client.
+            # This is critical for cleanup — calling delete after
+            # __aexit__ would fail because the httpx session is already
+            # torn down.
+            if delete_sandbox and self._sandbox is not None:
+                try:
+                    logger.info(
+                        "[Computer] Deleting Shipyard Neo sandbox: id=%s", sandbox_id
+                    )
+                    await self._sandbox.delete()
+                    logger.info(
+                        "[Computer] Shipyard Neo sandbox deleted: id=%s", sandbox_id
+                    )
+                except Exception as e:
+                    logger.warning(
+                        "[Computer] Failed to delete sandbox %s (may already be "
+                        "cleaned up by Bay GC): %s",
+                        sandbox_id,
+                        e,
+                    )
+
             logger.info(
-                "[Computer] Shutting down Shipyard Neo sandbox: id=%s", sandbox_id
+                "[Computer] Shutting down Shipyard Neo sandbox client: id=%s",
+                sandbox_id,
             )
             await self._client.__aexit__(None, None, None)
             self._client = None
             self._sandbox = None
-            logger.info("[Computer] Shipyard Neo sandbox shut down: id=%s", sandbox_id)
+            logger.info(
+                "[Computer] Shipyard Neo sandbox client shut down: id=%s", sandbox_id
+            )
 
         # NOTE: We intentionally do NOT stop the Bay container here.
         # It stays running for reuse by future sessions.  The user can

astrbot/core/computer/computer_client.py

@@ -445,7 +445,22 @@ async def get_booter(
     if session_id in session_booter:
         booter = session_booter[session_id]
         if not await booter.available():
-            # rebuild
+            # Clean up old booter before rebuilding so sandbox resources
+            # on Bay (containers, volumes, networks) are not leaked.
+            # Only ShipyardNeoBooter supports delete_sandbox; other booters
+            # (local, boxlite, cua, etc.) are not backed by a remote sandbox
+            # manager and don't need it.
+            try:
+                if booter_type == "shipyard_neo":
+                    await booter.shutdown(delete_sandbox=True)
+                else:
+                    await booter.shutdown()
+            except Exception as shutdown_err:
+                logger.warning(
+                    "[Computer] Error shutting down stale booter for session %s: %s",
+                    session_id,
+                    shutdown_err,
+                )
             session_booter.pop(session_id, None)
     if session_id not in session_booter:
         uuid_str = uuid.uuid5(uuid.NAMESPACE_DNS, session_id).hex
@@ -509,7 +524,10 @@ async def get_booter(
         except Exception as e:
             logger.error(f"Error booting sandbox for session {session_id}: {e}")
             try:
-                await client.shutdown()
+                if booter_type == "shipyard_neo":
+                    await client.shutdown(delete_sandbox=True)
+                else:
+                    await client.shutdown()
             except Exception as shutdown_error:
                 logger.warning(
                     "Failed to shutdown sandbox after boot error for session %s: %s",

tests/test_shipyard_neo_booter.py

@@ -0,0 +1,344 @@
+"""Tests for ShipyardNeoBooter — readiness gate, shutdown cleanup, and rebuild recovery."""
+
+from __future__ import annotations
+
+import asyncio
+from types import SimpleNamespace
+from unittest.mock import AsyncMock, patch
+
+import pytest
+
+
+# ═══════════════════════════════════════════════════════════════
+# _wait_until_ready
+# ═══════════════════════════════════════════════════════════════
+
+
+def _make_sandbox_mock(statuses: list[str], *, delete_side_effect=None):
+    """Build a sandbox mock that returns *statuses* in order on refresh().
+
+    After the list is exhausted subsequent refresh() calls return the last status.
+    """
+    call_count = 0
+
+    async def _refresh():
+        nonlocal call_count
+        idx = min(call_count, len(statuses) - 1)
+        call_count += 1
+        s = statuses[idx]
+        sandbox.status = SimpleNamespace(value=s)
+
+    sandbox = SimpleNamespace(
+        id="sandbox-test-1",
+        profile="python-default",
+        status=SimpleNamespace(value=statuses[0]),
+        refresh=_refresh,
+        delete=AsyncMock(side_effect=delete_side_effect),
+    )
+    return sandbox
+
+
+class TestWaitUntilReady:
+    def _make_booter(self):
+        from astrbot.core.computer.booters.shipyard_neo import ShipyardNeoBooter
+
+        return ShipyardNeoBooter(
+            endpoint_url="http://localhost:8114",
+            access_token="sk-bay-test",
+        )
+
+    @pytest.mark.asyncio
+    async def test_already_ready_returns_immediately(self):
+        """Sandbox is READY on first poll → instant return (warm hit)."""
+        booter = self._make_booter()
+        sandbox = _make_sandbox_mock(["ready"])
+
+        await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_starting_then_ready(self):
+        """Sandbox transitions STARTING → READY within timeout."""
+        booter = self._make_booter()
+        sandbox = _make_sandbox_mock(["starting", "starting", "ready"])
+
+        await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_not_called()
+
+    @pytest.mark.asyncio
+    async def test_failed_deletes_and_raises(self):
+        """Sandbox reaches FAILED → delete called → RuntimeError raised."""
+        booter = self._make_booter()
+        sandbox = _make_sandbox_mock(["starting", "failed"])
+
+        with pytest.raises(RuntimeError, match="terminal state"):
+            await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_awaited_once()
+
+    @pytest.mark.asyncio
+    async def test_expired_deletes_and_raises(self):
+        """Sandbox reaches EXPIRED → delete called → RuntimeError raised."""
+        booter = self._make_booter()
+        sandbox = _make_sandbox_mock(["starting", "expired"])
+
+        with pytest.raises(RuntimeError, match="terminal state"):
+            await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_awaited_once()
+
+    @pytest.mark.asyncio
+    async def test_timeout_deletes_and_raises(self):
+        """Sandbox never reaches READY → delete called → TimeoutError raised."""
+        booter = self._make_booter()
+        # Return 'idle' every time to simulate a stuck sandbox
+        sandbox = _make_sandbox_mock(["idle"])
+
+        # Override the deadline so we don't actually sleep 180s
+        original_time = asyncio.get_running_loop().time
+
+        call_idx = 0
+
+        def _fake_time():
+            nonlocal call_idx
+            # After one tick, jump past the deadline
+            if call_idx == 0:
+                call_idx += 1
+                return original_time()
+            # Return a value beyond the 180s timeout
+            return original_time() + 200
+
+        with patch(
+            "astrbot.core.computer.booters.shipyard_neo.asyncio.get_running_loop"
+        ) as mock_loop:
+            mock_loop.return_value.time = _fake_time
+
+            with pytest.raises(TimeoutError, match="did not become ready"):
+                await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_awaited_once()
+
+    @pytest.mark.asyncio
+    async def test_delete_failure_during_cleanup_is_safe(self):
+        """If sandbox.delete() itself throws, the original error is still raised."""
+        booter = self._make_booter()
+        sandbox = _make_sandbox_mock(
+            ["failed"],
+            delete_side_effect=RuntimeError("Bay unreachable"),
+        )
+
+        with pytest.raises(RuntimeError, match="terminal state"):
+            await booter._wait_until_ready(sandbox)
+
+        sandbox.delete.assert_awaited_once()
+
+
+# ═══════════════════════════════════════════════════════════════
+# shutdown
+# ═══════════════════════════════════════════════════════════════
+
+
+class TestShutdown:
+    def _make_booter(self):
+        from astrbot.core.computer.booters.shipyard_neo import ShipyardNeoBooter
+
+        return ShipyardNeoBooter(
+            endpoint_url="http://localhost:8114",
+            access_token="sk-bay-test",
+        )
+
+    @pytest.mark.asyncio
+    async def test_delete_sandbox_true_calls_delete(self):
+        """delete_sandbox=True → sandbox.delete() called, then client closed."""
+        booter = self._make_booter()
+        sandbox = SimpleNamespace(
+            id="sandbox-test-1",
+            delete=AsyncMock(),
+        )
+        client = SimpleNamespace(
+            __aexit__=AsyncMock(),
+        )
+        booter._sandbox = sandbox  # type: ignore[assignment]
+        booter._client = client  # type: ignore[assignment]
+
+        await booter.shutdown(delete_sandbox=True)
+
+        sandbox.delete.assert_awaited_once()
+        client.__aexit__.assert_awaited_once()
+        assert booter._client is None
+        assert booter._sandbox is None
+
+    @pytest.mark.asyncio
+    async def test_delete_sandbox_false_does_not_call_delete(self):
+        """delete_sandbox=False (default) → sandbox.delete() NOT called."""
+        booter = self._make_booter()
+        sandbox = SimpleNamespace(
+            id="sandbox-test-1",
+            delete=AsyncMock(),
+        )
+        client = SimpleNamespace(
+            __aexit__=AsyncMock(),
+        )
+        booter._sandbox = sandbox  # type: ignore[assignment]
+        booter._client = client  # type: ignore[assignment]
+
+        await booter.shutdown()  # default delete_sandbox=False
+
+        sandbox.delete.assert_not_called()
+        client.__aexit__.assert_awaited_once()
+        assert booter._client is None
+        assert booter._sandbox is None
+
+    @pytest.mark.asyncio
+    async def test_delete_failure_still_closes_client(self):
+        """If sandbox.delete() throws, HTTP client is still torn down."""
+        booter = self._make_booter()
+        sandbox = SimpleNamespace(
+            id="sandbox-test-1",
+            delete=AsyncMock(side_effect=RuntimeError("Bay gone")),
+        )
+        client = SimpleNamespace(
+            __aexit__=AsyncMock(),
+        )
+        booter._sandbox = sandbox  # type: ignore[assignment]
+        booter._client = client  # type: ignore[assignment]
+
+        # Should not raise — delete failure is logged but swallowed
+        await booter.shutdown(delete_sandbox=True)
+
+        sandbox.delete.assert_awaited_once()
+        client.__aexit__.assert_awaited_once()
+        assert booter._client is None
+        assert booter._sandbox is None
+
+    @pytest.mark.asyncio
+    async def test_no_client_is_noop(self):
+        """shutdown() on an uninitialised booter is a no-op."""
+        booter = self._make_booter()
+        # _client is None by default
+        await booter.shutdown(delete_sandbox=True)
+        # No exception → ok
+
+
+# ═══════════════════════════════════════════════════════════════
+# get_booter rebuild path
+# ═══════════════════════════════════════════════════════════════
+
+
+class TestGetBooterRebuild:
+    """Verify that stale ShipyardNeoBooter instances are cleaned up on rebuild."""
+
+    def _make_fake_context(self, booter_type: str = "shipyard_neo"):
+        """Build a context-like object for get_booter()."""
+        _cfg = {
+            "provider_settings": {
+                "computer_use_runtime": "sandbox",
+                "sandbox": {
+                    "booter": booter_type,
+                    "shipyard_neo_endpoint": "http://bay:8114",
+                    "shipyard_neo_access_token": "sk-test",
+                    "shipyard_neo_ttl": 3600,
+                    "shipyard_neo_profile": "python-default",
+                },
+            }
+        }
+        return SimpleNamespace(
+            get_config=lambda umo=None: _cfg,
+        )
+
+    @pytest.mark.asyncio
+    async def test_stale_neo_booter_calls_shutdown_with_delete(self, monkeypatch):
+        """A stale ShipyardNeoBooter gets shutdown(delete_sandbox=True) on eviction."""
+        from astrbot.core.computer import computer_client
+        from astrbot.core.computer.booters.shipyard_neo import ShipyardNeoBooter
+
+        ctx = self._make_fake_context()
+
+        stale = ShipyardNeoBooter(
+            endpoint_url="http://bay:8114", access_token="sk-test"
+        )
+        stale._sandbox = SimpleNamespace(id="stale-sandbox")  # type: ignore[assignment]
+        stale._client = SimpleNamespace(__aexit__=AsyncMock())  # type: ignore[assignment]
+        stale._sandbox.refresh = AsyncMock(side_effect=RuntimeError("sandbox gone"))  # type: ignore[union-attr]
+        # available() will return False because refresh() throws
+        stale.shutdown = AsyncMock()
+
+        monkeypatch.setitem(computer_client.session_booter, "session-1", stale)
+
+        from astrbot.core.computer.computer_client import get_booter
+
+        # get_booter should evict stale and rebuild.
+        # We need to mock the entire rebuild path so it doesn't actually
+        # try to connect to Bay.
+        async def _fake_boot(_self, _sid):
+            _self._sandbox = SimpleNamespace(  # type: ignore[assignment]
+                id="new-sandbox",
+                refresh=AsyncMock(),
+                status=SimpleNamespace(value="ready"),
+                capabilities=["python", "shell", "filesystem"],
+            )
+            _self._client = SimpleNamespace()  # type: ignore[assignment]
+            _self._shell = SimpleNamespace()  # type: ignore[assignment]
+            _self._fs = SimpleNamespace()  # type: ignore[assignment]
+            _self._python = SimpleNamespace()  # type: ignore[assignment]
+
+        with patch.object(
+            ShipyardNeoBooter, "boot", _fake_boot
+        ), patch(
+            "astrbot.core.computer.computer_client._sync_skills_to_sandbox",
+            AsyncMock(),
+        ):
+            await get_booter(ctx, "session-1")
+
+        stale.shutdown.assert_awaited_once_with(delete_sandbox=True)
+        # Old entry should be replaced
+        new_booter = computer_client.session_booter.get("session-1")
+        assert new_booter is not None
+        assert new_booter is not stale
+
+    @pytest.mark.asyncio
+    async def test_stale_non_neo_booter_calls_plain_shutdown(self, monkeypatch):
+        """Non-neo booter (e.g. shipyard) → plain shutdown() without delete_sandbox."""
+        from astrbot.core.computer import computer_client
+
+        ctx = self._make_fake_context(booter_type="shipyard")
+
+        stale = SimpleNamespace(shutdown=AsyncMock())
+        stale.available = AsyncMock(return_value=False)
+
+        monkeypatch.setitem(computer_client.session_booter, "session-1", stale)
+
+        # Patch ShipyardBooter entirely to skip its __init__ validation
+        class _FakeShipyardBooter:
+            def __init__(self, **kwargs):
+                pass
+
+            async def boot(self, _sid):
+                self._sandbox = SimpleNamespace(  # type: ignore[assignment]
+                    refresh=AsyncMock(),
+                    status=SimpleNamespace(value="ready"),
+                )
+                self._shell = SimpleNamespace()  # type: ignore[assignment]
+                self._fs = SimpleNamespace()  # type: ignore[assignment]
+                self._python = SimpleNamespace()  # type: ignore[assignment]
+
+            async def shutdown(self, **kwargs):
+                pass
+
+        with patch(
+            "astrbot.core.computer.booters.shipyard.ShipyardBooter",
+            _FakeShipyardBooter,
+        ), patch(
+            "astrbot.core.computer.computer_client._sync_skills_to_sandbox",
+            AsyncMock(),
+        ):
+            from astrbot.core.computer.computer_client import get_booter
+
+            await get_booter(ctx, "session-1")
+
+        stale.shutdown.assert_awaited_once()
+        # No delete_sandbox kwarg for non-neo booters
+        call_kwargs = stale.shutdown.call_args.kwargs
+        assert call_kwargs == {}