fix: harden sandbox cleanup and tool scoping

Author: zouyonghe

astrbot/core/astr_agent_tool_exec.py

@@ -342,7 +342,7 @@ def _build_handoff_toolset(
         # "all tools", including runtime computer-use tools.
         if tools is None:
             toolset = ToolSet()
-            for registered_tool in llm_tools.func_list:
+            for registered_tool in getattr(tool_mgr, "func_list", llm_tools.func_list):
                 if isinstance(registered_tool, HandoffTool):
                     continue
                 if registered_tool.active and cls._tool_available_for_runtime_config(
@@ -359,7 +359,7 @@ def _build_handoff_toolset(
         toolset = ToolSet()
         for tool_name_or_obj in tools:
             if isinstance(tool_name_or_obj, str):
-                registered_tool = llm_tools.get_func(tool_name_or_obj)
+                registered_tool = tool_mgr.get_func(tool_name_or_obj)
                 if (
                     registered_tool
                     and registered_tool.active

astrbot/core/astr_main_agent.py

@@ -520,6 +520,13 @@ async def _ensure_persona_and_skills(
                     "If you need to use these capabilities, ask the user to enable Computer Use in the AstrBot WebUI -> Config."
                 )
     tmgr = plugin_context.get_llm_tool_manager()
+    persona_tools_configured = bool(persona and persona.get("tools") is not None)
+    req._persona_tools_configured = persona_tools_configured
+    req._persona_allowed_tool_names = (
+        {str(tool_name) for tool_name in persona.get("tools", [])}
+        if persona_tools_configured
+        else None
+    )
 
     # inject toolset in the persona
     if (persona and persona.get("tools") is None) or not persona:
@@ -1068,28 +1075,42 @@ def _apply_sandbox_tools(
         req.func_tool = ToolSet()
     if req.system_prompt is None:
         req.system_prompt = ""
+    allowed_tool_names = getattr(req, "_persona_allowed_tool_names", None)
+    persona_tools_configured = bool(getattr(req, "_persona_tools_configured", False))
+
     tool_mgr = llm_tools
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(ExecuteShellTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(ListSandboxesTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(ListSandboxProvidersTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(GetCurrentSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(CreateSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(SwitchSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(KeepAliveSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(ReleaseSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(SetSandboxRetentionPolicyTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(TakeoverSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(DestroySandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(ScreenshotSandboxTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(CopyFileBetweenSandboxesTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(PythonTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(FileUploadTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(FileDownloadTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(FileReadTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(FileWriteTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(FileEditTool))
-    req.func_tool.add_tool(tool_mgr.get_builtin_tool(GrepTool))
-    req.system_prompt = f"{req.system_prompt or ''}\n{SANDBOX_MODE_PROMPT}\n"
+    added_tool = False
+
+    def add_sandbox_tool(tool_cls) -> None:
+        nonlocal added_tool
+        tool = tool_mgr.get_builtin_tool(tool_cls)
+        if persona_tools_configured and tool.name not in allowed_tool_names:
+            return
+        req.func_tool.add_tool(tool)
+        added_tool = True
+
+    add_sandbox_tool(ExecuteShellTool)
+    add_sandbox_tool(ListSandboxesTool)
+    add_sandbox_tool(ListSandboxProvidersTool)
+    add_sandbox_tool(GetCurrentSandboxTool)
+    add_sandbox_tool(CreateSandboxTool)
+    add_sandbox_tool(SwitchSandboxTool)
+    add_sandbox_tool(KeepAliveSandboxTool)
+    add_sandbox_tool(ReleaseSandboxTool)
+    add_sandbox_tool(SetSandboxRetentionPolicyTool)
+    add_sandbox_tool(TakeoverSandboxTool)
+    add_sandbox_tool(DestroySandboxTool)
+    add_sandbox_tool(ScreenshotSandboxTool)
+    add_sandbox_tool(CopyFileBetweenSandboxesTool)
+    add_sandbox_tool(PythonTool)
+    add_sandbox_tool(FileUploadTool)
+    add_sandbox_tool(FileDownloadTool)
+    add_sandbox_tool(FileReadTool)
+    add_sandbox_tool(FileWriteTool)
+    add_sandbox_tool(FileEditTool)
+    add_sandbox_tool(GrepTool)
+    if added_tool:
+        req.system_prompt = f"{req.system_prompt or ''}\n{SANDBOX_MODE_PROMPT}\n"
 
 
 def _proactive_cron_job_tools(req: ProviderRequest, plugin_context: Context) -> None:

astrbot/core/computer/computer_client.py

@@ -205,7 +205,13 @@ def _pop_live_booter(sandbox_id: str):
             preserved += 1
             continue
         if booter is not None and provider is not None:
-            await _safe_destroy_booter(provider, booter, record)
+            if not await _safe_destroy_booter(provider, booter, record):
+                sandbox_manager.session_booter[sandbox_id] = booter
+                sandbox_manager.registry.update_sandbox_status(
+                    sandbox_id,
+                    "error",
+                )
+                continue
         sandbox_manager.registry.delete_sandbox(sandbox_id)
         removed += 1
 
@@ -225,7 +231,14 @@ def _pop_live_booter(sandbox_id: str):
         sandbox_manager.clear_idle_state(sandbox_id)
         sandbox_manager.drop_boot_lock(sandbox_id)
         if provider is not None:
-            await _safe_destroy_booter(provider, booter, record)
+            if not await _safe_destroy_booter(provider, booter, record):
+                sandbox_manager.session_booter[sandbox_id] = booter
+                if sandbox_manager.registry.get_sandbox(sandbox_id) is not None:
+                    sandbox_manager.registry.update_sandbox_status(
+                        sandbox_id,
+                        "error",
+                    )
+                continue
         if sandbox_manager.registry.get_sandbox(sandbox_id) is not None:
             sandbox_manager.registry.delete_sandbox(sandbox_id)
         removed += 1
@@ -252,15 +265,17 @@ def detach_sandbox_provider(provider_id: str) -> None:
 
 async def _safe_destroy_booter(
     provider: SandboxProvider, booter: ComputerBooter, record: dict
-) -> None:
+) -> bool:
     try:
         await provider.destroy_booter(booter, record)
+        return True
     except Exception as exc:
         logger.warning(
             "Background destroy_booter failed for sandbox %s: %s",
             record.get("sandbox_id"),
             exc,
         )
+        return False
 
 
 async def _safe_shutdown_booter(booter: ComputerBooter, record: dict) -> None:

astrbot/core/computer/sandbox_manager.py

@@ -1313,25 +1313,33 @@ async def _destroy_sandbox_cleanup(
         sandbox_id: str,
         record: dict,
     ) -> None:
+        destroy_err: Exception | None = None
         async with self._sandbox_boot_lock(sandbox_id):
             current = self.registry.get_sandbox(sandbox_id) or record
             booter = self.session_booter.get(sandbox_id)
             if booter is not None:
                 try:
                     await provider.destroy_booter(booter, current)
-                except Exception as destroy_err:
+                except Exception as exc:
+                    destroy_err = exc
                     logger.warning(
                         "[Computer] destroy_booter failed for %s: %s",
                         sandbox_id,
-                        destroy_err,
+                        exc,
                     )
-                finally:
+                    self.registry.update_sandbox_status(sandbox_id, SandboxStatus.ERROR)
+                    await self.save_registry_async()
+                else:
                     self.clear_runtime_state(sandbox_id)
-            self.registry.delete_sandbox(sandbox_id)
-            await self.save_registry_async()
+            if destroy_err is None:
+                self.registry.delete_sandbox(sandbox_id)
+                await self.save_registry_async()
 
         self.drop_boot_lock(sandbox_id)
 
+        if destroy_err is not None:
+            raise destroy_err
+
         if hasattr(provider, "on_sandbox_destroyed"):
             try:
                 await provider.on_sandbox_destroyed(record)

astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py

@@ -194,9 +194,7 @@ async def send_message(
                     await cls._upload_file_segment(bot, seg, is_group, session_id)
                 except Exception:
                     messages = await cls._parse_onebot_json(MessageChain([seg]))
-                    await cls._dispatch_send(
-                        bot, event, is_group, session_id, messages
-                    )
+                    await cls._dispatch_send(bot, event, is_group, session_id, messages)
             else:
                 messages = await cls._parse_onebot_json(MessageChain([seg]))
                 if not messages:

astrbot/dashboard/routes/config.py

@@ -307,16 +307,9 @@ async def _validate_neo_connectivity(
         return "⚠️ Shipyard Neo endpoint 未设置"
 
     access_token = sandbox.get("shipyard_neo_access_token", "")
-    if not access_token:
-        # Try auto-discovery
-        from astrbot.core.computer.computer_client import _discover_bay_credentials
-
-        access_token = _discover_bay_credentials(endpoint)
-
     if not access_token:
         return (
-            "⚠️ 未找到 Bay API Key。请填写访问令牌，"
-            "或确保 Bay 的 credentials.json 可被自动发现。"
+            "⚠️ 未找到 Bay API Key。请填写访问令牌，或切换到新的 Sandbox Provider 配置。"
         )
 
     # Connectivity check

tests/test_dashboard.py

@@ -1081,7 +1081,11 @@ async def test_auth_rate_limit_uses_same_bucket_across_paths(
     cfg = core_lifecycle_td.astrbot_config["dashboard"]
     rl_original = cfg.get("auth_rate_limit", {})
     tp_original = cfg.get("trust_proxy_headers", False)
-    cfg["auth_rate_limit"] = {"enable": True, "average_interval": 3600.0, "max_burst": 1}
+    cfg["auth_rate_limit"] = {
+        "enable": True,
+        "average_interval": 3600.0,
+        "max_burst": 1,
+    }
     cfg["trust_proxy_headers"] = True
 
     try:
@@ -1113,7 +1117,11 @@ async def test_auth_rate_limit_separates_different_client_ips(
     cfg = core_lifecycle_td.astrbot_config["dashboard"]
     rl_original = cfg.get("auth_rate_limit", {})
     tp_original = cfg.get("trust_proxy_headers", False)
-    cfg["auth_rate_limit"] = {"enable": True, "average_interval": 3600.0, "max_burst": 1}
+    cfg["auth_rate_limit"] = {
+        "enable": True,
+        "average_interval": 3600.0,
+        "max_burst": 1,
+    }
     cfg["trust_proxy_headers"] = True
 
     try:
@@ -1157,7 +1165,11 @@ async def test_auth_rate_limit_ignores_proxy_headers_by_default(
     cfg = core_lifecycle_td.astrbot_config["dashboard"]
     rl_original = cfg.get("auth_rate_limit", {})
     tp_original = cfg.get("trust_proxy_headers", False)
-    cfg["auth_rate_limit"] = {"enable": True, "average_interval": 3600.0, "max_burst": 1}
+    cfg["auth_rate_limit"] = {
+        "enable": True,
+        "average_interval": 3600.0,
+        "max_burst": 1,
+    }
     cfg["trust_proxy_headers"] = False
 
     try:
@@ -1570,6 +1582,27 @@ async def test_config_save_rejects_recovery_code_for_protected_totp_changes(
         )
 
 
+@pytest.mark.asyncio
+async def test_validate_neo_connectivity_without_token_returns_warning_not_import_error():
+    from astrbot.dashboard.routes.config import _validate_neo_connectivity
+
+    warning = await _validate_neo_connectivity(
+        {
+            "provider_settings": {
+                "computer_use_runtime": "sandbox",
+                "sandbox": {
+                    "booter": "shipyard_neo",
+                    "shipyard_neo_endpoint": "http://127.0.0.1:65535",
+                    "shipyard_neo_access_token": "",
+                },
+            }
+        }
+    )
+
+    assert warning is not None
+    assert "API Key" in warning
+
+
 @pytest.mark.asyncio
 async def test_auth_totp_setup_with_valid_code_returns_recovery_code(
     app: Quart,

tests/unit/test_astr_agent_tool_exec.py

@@ -296,6 +296,44 @@ async def test_build_handoff_toolset_uses_registered_provider_tools_only(
         FunctionToolExecutor._runtime_computer_tools_cache.clear()
 
 
+@pytest.mark.asyncio
+async def test_build_handoff_toolset_uses_scoped_tool_manager_for_all_tools():
+    from astrbot.core.agent.tool import FunctionTool
+
+    allowed_tool = FunctionTool(
+        name="allowed_tool",
+        parameters={"type": "object", "properties": {}},
+        description="allowed",
+    )
+    disallowed_tool = FunctionTool(
+        name="disallowed_tool",
+        parameters={"type": "object", "properties": {}},
+        description="disallowed",
+    )
+    previous_tools = list(llm_tools.func_list)
+    FunctionToolExecutor._runtime_computer_tools_cache.clear()
+    llm_tools.func_list = [allowed_tool, disallowed_tool]
+    tool_mgr = SimpleNamespace(func_list=[allowed_tool], get_func=lambda name: None)
+    context = SimpleNamespace(
+        get_config=lambda **_kwargs: {
+            "provider_settings": {"computer_use_runtime": "none"}
+        },
+        get_llm_tool_manager=lambda: tool_mgr,
+    )
+    run_context = ContextWrapper(
+        context=SimpleNamespace(event=_DummyEvent([]), context=context)
+    )
+
+    try:
+        toolset = FunctionToolExecutor._build_handoff_toolset(run_context, None)
+        assert toolset is not None
+        assert "allowed_tool" in toolset.names()
+        assert "disallowed_tool" not in toolset.names()
+    finally:
+        llm_tools.func_list = previous_tools
+        FunctionToolExecutor._runtime_computer_tools_cache.clear()
+
+
 @pytest.mark.asyncio
 async def test_background_wake_preserves_computer_runtime_config(
     monkeypatch: pytest.MonkeyPatch,

tests/unit/test_astr_main_agent.py

@@ -742,6 +742,22 @@ async def test_ensure_tools_from_persona(self, mock_event, mock_context):
 
         assert req.func_tool is not None
 
+    def test_apply_sandbox_tools_respects_explicit_empty_persona_tool_list(self):
+        module = ama
+        req = ProviderRequest(func_tool=ToolSet())
+        req._persona_tools_configured = True
+        req._persona_allowed_tool_names = set()
+        config = module.MainAgentBuildConfig(
+            tool_call_timeout=60,
+            computer_use_runtime="sandbox",
+            provider_settings={"computer_use_runtime": "sandbox"},
+        )
+
+        module._apply_sandbox_tools(config, req)
+
+        assert req.func_tool is not None
+        assert req.func_tool.empty()
+
     @pytest.mark.asyncio
     async def test_ensure_persona_keeps_all_sandbox_provider_tools_in_sandbox_runtime(
         self, mock_event, mock_context

tests/unit/test_sandbox_computer_client.py

@@ -614,6 +614,49 @@ async def fake_destroy_booter(booter, record):
     assert persistent_booter.shutdown_calls == 1
 
 
+@pytest.mark.asyncio
+async def test_cleanup_sandbox_provider_preserves_temporary_record_when_destroy_fails(
+    monkeypatch, tmp_path
+):
+    from astrbot.core.computer import computer_client
+    from astrbot.core.computer.sandbox_manager import SandboxManager
+    from astrbot.core.computer.sandbox_registry import SandboxRegistry
+
+    provider = FakeProvider()
+
+    async def fake_destroy_booter(booter, record):
+        raise RuntimeError("destroy failed")
+
+    provider.destroy_booter = fake_destroy_booter
+    manager = SandboxManager(
+        registry=SandboxRegistry(tmp_path / "sandbox_registry.json"),
+        providers={provider.provider_id: provider},
+    )
+    monkeypatch.setattr(computer_client, "sandbox_manager", manager)
+    manager.registry.upsert_sandbox(
+        sandbox_id="generic-temp",
+        sandbox_name="Temp",
+        provider="generic",
+        managed=True,
+        created_by_astrbot=True,
+        owner_user_id="session-a",
+        owner_session_id="session-a",
+        connect_info={},
+        retention_policy="temporary",
+        status="running",
+    )
+    booter = FakeBooter()
+    booter.provider_id = provider.provider_id
+    manager.session_booter["generic-temp"] = booter
+
+    await computer_client.cleanup_sandbox_provider("generic")
+
+    record = manager.registry.get_sandbox("generic-temp")
+    assert record is not None
+    assert record["status"] == "error"
+    assert manager.session_booter["generic-temp"] is booter
+
+
 @pytest.mark.asyncio
 async def test_cleanup_sandbox_provider_cleans_live_booter_without_registry_record(
     monkeypatch, tmp_path