fix(sandbox): preserve registry runtime state

Author: zouyonghe

astrbot/core/computer/sandbox_manager.py

@@ -49,10 +49,6 @@ def _sandbox_boot_lock(self, sandbox_id: str) -> asyncio.Lock:
     def drop_boot_lock(self, sandbox_id: str) -> None:
         self.boot_locks.pop(sandbox_id, None)
 
-    def get_idle_timeout(self, config: dict, provider_id: str) -> float:
-        _ = config, provider_id
-        return 0.0
-
     def build_record_payload(
         self,
         *,

astrbot/core/computer/sandbox_models.py

@@ -40,13 +40,23 @@ class SandboxRecord:
     labels: dict[str, Any] = field(default_factory=dict)
     notes: str | None = None
 
+    @staticmethod
+    def _required_string(data: dict[str, Any], field_name: str) -> str:
+        value = data[field_name]
+        if not isinstance(value, str):
+            raise ValueError(f"{field_name} must be a non-empty string")
+        value = value.strip()
+        if not value:
+            raise ValueError(f"{field_name} must be a non-empty string")
+        return value
+
     @classmethod
     def from_dict(cls, data: dict[str, Any]) -> SandboxRecord:
         return cls(
-            sandbox_id=str(data["sandbox_id"]),
-            sandbox_name=str(data["sandbox_name"]),
-            booter_type=str(data["booter_type"]),
-            provider=str(data["provider"]),
+            sandbox_id=cls._required_string(data, "sandbox_id"),
+            sandbox_name=cls._required_string(data, "sandbox_name"),
+            booter_type=cls._required_string(data, "booter_type"),
+            provider=cls._required_string(data, "provider"),
             managed=bool(data["managed"]),
             created_by_astrbot=bool(data["created_by_astrbot"]),
             is_default=bool(data.get("is_default", False)),

astrbot/core/computer/sandbox_registry.py

@@ -69,16 +69,16 @@ def upsert_sandbox(
         owner_session_id: str | None,
         connect_info: dict[str, Any],
         is_default: bool = False,
-        status: str = "running",
-        idle_timeout: int | float | None = None,
-        expires_at: float | None = None,
-        retention_policy: str = "temporary",
-        last_used_at: float | None = None,
-        controller_user_id: str | None = None,
-        controller_session_id: str | None = None,
-        lease_expires_at: float | None = None,
-        labels: dict[str, Any] | None = None,
-        notes: str | None = None,
+        status: str | object = _UNSET,
+        idle_timeout: int | float | None | object = _UNSET,
+        expires_at: float | None | object = _UNSET,
+        retention_policy: str | object = _UNSET,
+        last_used_at: float | None | object = _UNSET,
+        controller_user_id: str | None | object = _UNSET,
+        controller_session_id: str | None | object = _UNSET,
+        lease_expires_at: float | None | object = _UNSET,
+        labels: dict[str, Any] | None | object = _UNSET,
+        notes: str | None | object = _UNSET,
     ) -> dict[str, Any]:
         record = self._payload["sandboxes"].get(sandbox_id, {})
         record.update(
@@ -92,19 +92,39 @@ def upsert_sandbox(
                 "is_default": is_default,
                 "owner_user_id": owner_user_id,
                 "owner_session_id": owner_session_id,
-                "controller_user_id": controller_user_id,
-                "controller_session_id": controller_session_id,
-                "lease_expires_at": lease_expires_at,
-                "last_used_at": last_used_at,
-                "idle_timeout": idle_timeout,
-                "expires_at": expires_at,
-                "retention_policy": retention_policy,
-                "status": status,
                 "connect_info": deepcopy(connect_info),
-                "labels": deepcopy(labels) if labels is not None else {},
-                "notes": notes,
             }
         )
+        defaults = {
+            "controller_user_id": None,
+            "controller_session_id": None,
+            "lease_expires_at": None,
+            "last_used_at": None,
+            "idle_timeout": None,
+            "expires_at": None,
+            "retention_policy": "temporary",
+            "status": "running",
+            "labels": {},
+            "notes": None,
+        }
+        updates = {
+            "controller_user_id": controller_user_id,
+            "controller_session_id": controller_session_id,
+            "lease_expires_at": lease_expires_at,
+            "last_used_at": last_used_at,
+            "idle_timeout": idle_timeout,
+            "expires_at": expires_at,
+            "retention_policy": retention_policy,
+            "status": status,
+            "labels": deepcopy(labels) if labels is not _UNSET else _UNSET,
+            "notes": notes,
+        }
+        for field_name, default_value in defaults.items():
+            value = updates[field_name]
+            if value is _UNSET:
+                record.setdefault(field_name, deepcopy(default_value))
+            else:
+                record[field_name] = value
         record = SandboxRecord.from_dict(record).to_dict()
         self._payload["sandboxes"][sandbox_id] = record
         if is_default or (managed and self._payload["default_sandbox_id"] is None):

tests/unit/test_sandbox_models.py

@@ -1,3 +1,5 @@
+import pytest
+
 from astrbot.core.computer.sandbox_models import (
     SandboxRecord,
     SandboxRetentionPolicy,
@@ -81,3 +83,28 @@ def test_sandbox_record_detects_active_control_lease():
     assert record.is_controlled_by("session-a", now=100.0) is True
     assert record.is_controlled_by("session-b", now=100.0) is False
     assert record.has_active_lease(now=201.0) is False
+
+
+@pytest.mark.parametrize(
+    "field,value",
+    [
+        ("sandbox_id", None),
+        ("sandbox_id", ""),
+        ("sandbox_name", "   "),
+        ("booter_type", None),
+        ("provider", ""),
+    ],
+)
+def test_sandbox_record_rejects_empty_required_string_fields(field, value):
+    payload = {
+        "sandbox_id": "sb-1",
+        "sandbox_name": "worker",
+        "booter_type": "cua",
+        "provider": "cua",
+        "managed": True,
+        "created_by_astrbot": True,
+    }
+    payload[field] = value
+
+    with pytest.raises(ValueError, match=field):
+        SandboxRecord.from_dict(payload)

tests/unit/test_sandbox_registry.py

@@ -146,3 +146,29 @@ def test_sandbox_registry_updates_config_and_status(tmp_path):
     assert record["expires_at"] == 200
     assert record["retention_policy"] == "persistent"
     assert record["status"] == "stopped"
+
+
+def test_sandbox_registry_upsert_preserves_runtime_fields_when_omitted(tmp_path):
+    registry = SandboxRegistry(storage_path=tmp_path / "sandbox_registry.json")
+    _upsert(
+        registry,
+        "sb-1",
+        controller_user_id="user-a",
+        controller_session_id="session-a",
+        lease_expires_at=200,
+        last_used_at=100,
+        idle_timeout=30,
+        status="unknown",
+    )
+
+    _upsert(registry, "sb-1", sandbox_name="renamed", connect_info={"name": "renamed"})
+
+    record = registry.get_sandbox("sb-1")
+    assert record["sandbox_name"] == "renamed"
+    assert record["connect_info"] == {"name": "renamed"}
+    assert record["controller_user_id"] == "user-a"
+    assert record["controller_session_id"] == "session-a"
+    assert record["lease_expires_at"] == 200
+    assert record["last_used_at"] == 100
+    assert record["idle_timeout"] == 30
+    assert record["status"] == "unknown"