fix: address sandbox provider review feedback

Author: zouyonghe

astrbot/core/astr_agent_tool_exec.py

@@ -20,6 +20,9 @@
 from astrbot.core.astr_main_agent_resources import (
     BACKGROUND_TASK_RESULT_WOKE_SYSTEM_PROMPT,
 )
+from astrbot.core.computer.sandbox_tool_binding import (
+    resolve_sandbox_provider_bindings,
+)
 from astrbot.core.cron.events import CronMessageEvent
 from astrbot.core.message.components import Image
 from astrbot.core.message.message_event_result import (
@@ -237,14 +240,11 @@ def _get_runtime_computer_tools(
                 edit_tool.name: edit_tool,
                 grep_tool.name: grep_tool,
             }
-            from astrbot.core.computer.computer_client import get_sandbox_provider_info
-
-            provider_info = get_sandbox_provider_info(booter)
-            if provider_info:
-                for tool_name in provider_info.get("tool_names", []):
-                    provider_tool = tool_mgr.get_func(tool_name)
-                    if provider_tool and getattr(provider_tool, "active", True):
-                        tools[provider_tool.name] = provider_tool
+            _provider_info, provider_tools = resolve_sandbox_provider_bindings(
+                booter, tool_mgr
+            )
+            for provider_tool in provider_tools:
+                tools[provider_tool.name] = provider_tool
             return tools
         if runtime == "local":
             shell_tool = tool_mgr.get_builtin_tool(ExecuteShellTool)

astrbot/core/astr_main_agent.py

@@ -1002,16 +1002,18 @@ def _apply_sandbox_tools(
     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))
-    from astrbot.core.computer.computer_client import get_sandbox_provider_info
+    from astrbot.core.computer.sandbox_tool_binding import (
+        resolve_sandbox_provider_bindings,
+    )
 
-    provider_info = get_sandbox_provider_info(booter)
-    if provider_info:
-        for tool_name in provider_info.get("tool_names", []):
-            tool = tool_mgr.get_func(tool_name)
-            if tool and getattr(tool, "active", True):
-                req.func_tool.add_tool(tool)
+    provider_info, provider_tools = resolve_sandbox_provider_bindings(booter, tool_mgr)
+    for tool in provider_tools:
+        req.func_tool.add_tool(tool)
 
     req.system_prompt = f"{req.system_prompt or ''}\n{SANDBOX_MODE_PROMPT}\n"
+    provider_system_prompt = (provider_info or {}).get("system_prompt", "").strip()
+    if provider_system_prompt:
+        req.system_prompt = f"{req.system_prompt or ''}\n{provider_system_prompt}\n"
 
 
 def _proactive_cron_job_tools(req: ProviderRequest, plugin_context: Context) -> None:

astrbot/core/computer/computer_client.py

@@ -29,6 +29,7 @@ def _sandbox_provider_info(provider_id: str, provider: SandboxProvider) -> dict:
         "provider_id": provider_id,
         "capabilities": sorted(getattr(provider, "capabilities", set())),
         "tool_names": sorted(getattr(provider, "tool_names", set())),
+        "system_prompt": str(getattr(provider, "system_prompt", "") or ""),
     }
 
 

astrbot/core/computer/sandbox_manager.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import asyncio
+import inspect
 import time
 import uuid
 from dataclasses import dataclass
@@ -39,6 +40,12 @@ def save_registry(self) -> None:
         except Exception as exc:
             logger.warning("[Computer] Failed to save sandbox registry: %s", exc)
 
+    async def save_registry_async(self) -> None:
+        try:
+            await self.registry.save_async()
+        except Exception as exc:
+            logger.warning("[Computer] Failed to save sandbox registry: %s", exc)
+
     def _sandbox_boot_lock(self, sandbox_id: str) -> asyncio.Lock:
         lock = self.boot_locks.get(sandbox_id)
         if lock is None:
@@ -101,7 +108,10 @@ async def booter_available(self, booter: ComputerBooter) -> bool:
         available = getattr(booter, "available", None)
         if available is None:
             return True
-        return await available()
+        result = available() if callable(available) else available
+        if inspect.isawaitable(result):
+            result = await result
+        return bool(result)
 
     def acquire_lease(
         self, sandbox_id: str, session_id: str, *, ttl: float | None = None
@@ -137,7 +147,7 @@ def sandbox_controlled_by_other_session(
             return False
         return bool(lease_expires_at and lease_expires_at > time.time())
 
-    def _upsert_new_sandbox_record(
+    async def _upsert_new_sandbox_record(
         self, context: Context, session_id: str, provider_id: str, create_config: dict
     ) -> str:
         provider = self.get_provider(provider_id)
@@ -152,7 +162,7 @@ def _upsert_new_sandbox_record(
                 connect_info=provider.build_connect_info(sandbox_id, create_config),
             )
         )
-        self.save_registry()
+        await self.save_registry_async()
         return sandbox_id
 
     async def get_or_create_booter(
@@ -168,22 +178,26 @@ async def get_or_create_booter(
             current_record is None or current_record.get("provider") != provider_id
         ):
             self.registry.set_current_sandbox_id(session_id, None)
-            self.save_registry()
+            await self.save_registry_async()
+            current_sandbox_id = None
+            current_record = None
         if (
             current_sandbox_id
             and current_record
             and current_record.get("provider") == provider_id
             and current_sandbox_id in self.session_booter
         ):
             if not self.acquire_lease(current_sandbox_id, session_id):
-                raise RuntimeError(f"Sandbox {current_sandbox_id} is busy")
-            booter = self.session_booter[current_sandbox_id]
-            if await self.booter_available(booter):
-                self.registry.touch_sandbox(current_sandbox_id)
-                self.save_registry()
-                self.schedule_idle_cleanup(current_sandbox_id, idle_timeout)
-                return booter
-            self.session_booter.pop(current_sandbox_id, None)
+                self.registry.set_current_sandbox_id(session_id, None)
+                await self.save_registry_async()
+            else:
+                booter = self.session_booter[current_sandbox_id]
+                if await self.booter_available(booter):
+                    self.registry.touch_sandbox(current_sandbox_id)
+                    await self.save_registry_async()
+                    self.schedule_idle_cleanup(current_sandbox_id, idle_timeout)
+                    return booter
+                self.session_booter.pop(current_sandbox_id, None)
 
         created_target_record = False
         target_sandbox_id = self.get_default_sandbox_id(provider_id)
@@ -204,10 +218,10 @@ async def get_or_create_booter(
                 )
             )
             self.registry.set_default_sandbox_id(record["sandbox_id"])
-            self.save_registry()
+            await self.save_registry_async()
 
         if self.sandbox_controlled_by_other_session(target_sandbox_id, session_id):
-            target_sandbox_id = self._upsert_new_sandbox_record(
+            target_sandbox_id = await self._upsert_new_sandbox_record(
                 context, session_id, provider_id, create_config
             )
             created_target_record = True
@@ -217,7 +231,7 @@ async def get_or_create_booter(
                 if target_sandbox_id in self.session_booter and not self.acquire_lease(
                     target_sandbox_id, session_id
                 ):
-                    target_sandbox_id = self._upsert_new_sandbox_record(
+                    target_sandbox_id = await self._upsert_new_sandbox_record(
                         context, session_id, provider_id, create_config
                     )
                     created_target_record = True
@@ -230,10 +244,10 @@ async def get_or_create_booter(
                     self.session_booter.pop(target_sandbox_id, None)
                     self.registry.release_lease(target_sandbox_id)
                     self.registry.update_sandbox_status(target_sandbox_id, "unknown")
-                    self.save_registry()
+                    await self.save_registry_async()
 
                 if not self.acquire_lease(target_sandbox_id, session_id):
-                    target_sandbox_id = self._upsert_new_sandbox_record(
+                    target_sandbox_id = await self._upsert_new_sandbox_record(
                         context, session_id, provider_id, create_config
                     )
                     created_target_record = True
@@ -252,7 +266,7 @@ async def get_or_create_booter(
                             target_sandbox_id, "unknown"
                         )
                     self.drop_boot_lock(target_sandbox_id)
-                    self.save_registry()
+                    await self.save_registry_async()
                     raise
                 setattr(client, "sandbox_id", target_sandbox_id)
                 self.session_booter[target_sandbox_id] = client
@@ -262,7 +276,7 @@ async def get_or_create_booter(
         self.registry.touch_sandbox(target_sandbox_id)
         self.registry.update_sandbox_status(target_sandbox_id, "running")
         self.registry.set_current_sandbox_id(session_id, target_sandbox_id)
-        self.save_registry()
+        await self.save_registry_async()
         self.schedule_idle_cleanup(target_sandbox_id, idle_timeout)
         return self.session_booter[target_sandbox_id]
 
@@ -295,13 +309,13 @@ async def create_sandbox_uncontrolled(
         except Exception:
             self.registry.delete_sandbox(sandbox_id)
             self.drop_boot_lock(sandbox_id)
-            self.save_registry()
+            await self.save_registry_async()
             raise
         setattr(client, "sandbox_id", sandbox_id)
         self.session_booter[sandbox_id] = client
         self.registry.touch_sandbox(sandbox_id)
         self.registry.update_sandbox_status(sandbox_id, "running")
-        self.save_registry()
+        await self.save_registry_async()
         self.schedule_idle_cleanup(sandbox_id, idle_timeout)
         return self.registry.get_sandbox(sandbox_id) or record
 
@@ -319,7 +333,7 @@ async def create_sandbox(
         if not self.acquire_lease(sandbox_id, session_id):
             raise RuntimeError(f"Sandbox {sandbox_id} is busy")
         self.registry.set_current_sandbox_id(session_id, sandbox_id)
-        self.save_registry()
+        await self.save_registry_async()
         return self.registry.get_sandbox(sandbox_id) or sandbox
 
     def list_sandboxes(self) -> list[dict]:
@@ -395,13 +409,15 @@ async def switch_current_sandbox_checked(
         if not await self.booter_available(booter):
             self.session_booter.pop(sandbox_id, None)
             self.registry.update_sandbox_status(sandbox_id, "unknown")
-            self.save_registry()
+            await self.save_registry_async()
             raise RuntimeError(f"Sandbox {sandbox_id} is not running")
         if not self.acquire_lease(sandbox_id, session_id):
             raise RuntimeError(f"Sandbox {sandbox_id} is busy")
-        return self._set_current_sandbox_after_lease(session_id, sandbox_id, record)
+        return await self._set_current_sandbox_after_lease(
+            session_id, sandbox_id, record
+        )
 
-    def _set_current_sandbox_after_lease(
+    async def _set_current_sandbox_after_lease(
         self, session_id: str, sandbox_id: str, record: dict
     ) -> dict:
         previous_sandbox_id = self.registry.get_current_sandbox_id(session_id)
@@ -411,7 +427,7 @@ def _set_current_sandbox_after_lease(
                 self.registry.release_lease(previous_sandbox_id)
         self.registry.set_current_sandbox_id(session_id, sandbox_id)
         self.registry.touch_sandbox(sandbox_id)
-        self.save_registry()
+        await self.save_registry_async()
         return self.registry.get_sandbox(sandbox_id) or record
 
     def get_current_sandbox(self, session_id: str) -> dict:
@@ -497,7 +513,7 @@ async def destroy_sandbox(self, session_id: str, sandbox_id: str) -> dict:
         self.clear_idle_state(sandbox_id)
         self.registry.delete_sandbox(sandbox_id)
         self.drop_boot_lock(sandbox_id)
-        self.save_registry()
+        await self.save_registry_async()
         return record
 
     async def get_observer_booter_by_id(
@@ -516,10 +532,10 @@ async def get_observer_booter_by_id(
         if not await self.booter_available(booter):
             self.session_booter.pop(sandbox_id, None)
             self.registry.update_sandbox_status(sandbox_id, "unknown")
-            self.save_registry()
+            await self.save_registry_async()
             raise RuntimeError(f"Sandbox {sandbox_id} is not running")
         self.registry.touch_sandbox(sandbox_id)
-        self.save_registry()
+        await self.save_registry_async()
         idle_timeout = record.get("idle_timeout") or 0
         self.schedule_idle_cleanup(sandbox_id, float(idle_timeout))
         return booter
@@ -530,7 +546,7 @@ async def reconcile_on_startup(self) -> None:
         self.session_booter.clear()
         for sandbox_id in list(self.idle_state):
             self.clear_idle_state(sandbox_id)
-        self.registry.save()
+        await self.save_registry_async()
 
     async def cleanup_managed_sandboxes(self) -> None:
         managed_records = self.list_sandboxes()
@@ -546,7 +562,7 @@ async def cleanup_managed_sandboxes(self) -> None:
                 provider = self.get_provider(record.get("provider", ""))
             except RuntimeError as provider_error:
                 self.registry.update_sandbox_status(sandbox_id, "unknown")
-                self.save_registry()
+                await self.save_registry_async()
                 logger.warning(
                     "[Computer] Skip managed sandbox cleanup for unsupported provider: sandbox_id=%s error=%s",
                     sandbox_id,
@@ -560,7 +576,7 @@ async def cleanup_managed_sandboxes(self) -> None:
                     self.session_booter.pop(sandbox_id, None)
                 except Exception as shutdown_err:
                     self.registry.update_sandbox_status(sandbox_id, "unknown")
-                    self.save_registry()
+                    await self.save_registry_async()
                     logger.warning(
                         "[Computer] Failed to shutdown managed sandbox %s: %s",
                         sandbox_id,
@@ -570,7 +586,7 @@ async def cleanup_managed_sandboxes(self) -> None:
             self.clear_idle_state(sandbox_id)
             self.registry.delete_sandbox(sandbox_id)
             self.drop_boot_lock(sandbox_id)
-        self.registry.save()
+        await self.save_registry_async()
 
     def clear_idle_state(self, sandbox_id: str) -> None:
         state = self.idle_state.pop(sandbox_id, None)
@@ -628,7 +644,7 @@ async def _expire_when_idle(
                     except Exception as shutdown_err:
                         self.session_booter[sandbox_id] = booter
                         self.registry.update_sandbox_status(sandbox_id, "unknown")
-                        self.save_registry()
+                        await self.save_registry_async()
                         logger.warning(
                             "[Computer] Failed to shutdown idle sandbox %s: %s",
                             sandbox_id,
@@ -640,7 +656,7 @@ async def _expire_when_idle(
                 else:
                     self.registry.delete_sandbox(sandbox_id)
                     self.drop_boot_lock(sandbox_id)
-                self.save_registry()
+                await self.save_registry_async()
                 return
         except asyncio.CancelledError:
             raise

astrbot/core/computer/sandbox_provider.py

@@ -10,6 +10,7 @@ class SandboxProvider(Protocol):
     provider_id: str
     capabilities: set[str]
     tool_names: set[str]
+    system_prompt: str
 
     def build_create_config(self, context: Context, session_id: str) -> dict: ...
 

astrbot/core/computer/sandbox_registry.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import asyncio
 import json
 import time
 from copy import deepcopy
@@ -327,9 +328,16 @@ def load(self) -> None:
             self._payload.get("session_current") or {}
         )
 
-    def save(self) -> None:
+    def _write_payload(self, payload: dict[str, Any]) -> None:
         self.storage_path.parent.mkdir(parents=True, exist_ok=True)
         self.storage_path.write_text(
-            json.dumps(self._payload, ensure_ascii=False, indent=2, sort_keys=True),
+            json.dumps(payload, ensure_ascii=False, indent=2, sort_keys=True),
             encoding="utf-8",
         )
+
+    def save(self) -> None:
+        self._write_payload(deepcopy(self._payload))
+
+    async def save_async(self) -> None:
+        payload = deepcopy(self._payload)
+        await asyncio.to_thread(self._write_payload, payload)

astrbot/core/computer/sandbox_tool_binding.py

@@ -0,0 +1,23 @@
+from __future__ import annotations
+
+from typing import Any
+
+
+def resolve_sandbox_provider_bindings(
+    provider_id: str | None,
+    tool_mgr: Any,
+) -> tuple[dict | None, list[Any]]:
+    """Return provider metadata and active provider tools for sandbox mode."""
+    from astrbot.core.computer.computer_client import get_sandbox_provider_info
+
+    normalized_provider_id = "" if provider_id is None else str(provider_id).lower()
+    provider_info = get_sandbox_provider_info(normalized_provider_id)
+    if not provider_info:
+        return None, []
+
+    tools = []
+    for tool_name in provider_info.get("tool_names", []):
+        tool = tool_mgr.get_func(tool_name)
+        if tool and getattr(tool, "active", True):
+            tools.append(tool)
+    return provider_info, tools