From d2e3e526fd34853c499934ccaf2ac1b5bd265088 Mon Sep 17 00:00:00 2001 From: Stable Genius <259448942+stablegenius49@users.noreply.github.com> Date: Wed, 11 Mar 2026 18:16:47 -0700 Subject: [PATCH] fix: resync reused sandbox skills after local updates --- astrbot/core/computer/computer_client.py | 57 ++++++++++++++- tests/test_computer_skill_sync.py | 29 ++++++++ tests/unit/test_computer.py | 88 ++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 1 deletion(-) diff --git a/astrbot/core/computer/computer_client.py b/astrbot/core/computer/computer_client.py index 6e80ac3ab7..1d2bffd051 100644 --- a/astrbot/core/computer/computer_client.py +++ b/astrbot/core/computer/computer_client.py @@ -1,3 +1,4 @@ +import hashlib import json import os import shutil @@ -31,6 +32,41 @@ def _list_local_skill_dirs(skills_root: Path) -> list[Path]: return skills +def _compute_local_skills_revision(skills_root: Path) -> str: + """Return a stable fingerprint for the current local skills tree. + + Includes all managed skill files so sandbox reuse can detect local skill + updates even when the same sandbox session stays alive. + """ + if not skills_root.is_dir(): + return "missing" + + digest = hashlib.sha256() + local_skill_dirs = _list_local_skill_dirs(skills_root) + if not local_skill_dirs: + digest.update(b"empty") + return digest.hexdigest() + + for skill_dir in local_skill_dirs: + for path in sorted(skill_dir.rglob("*")): + relative = path.relative_to(skills_root).as_posix() + digest.update(relative.encode("utf-8")) + stat = path.stat() + digest.update(str(stat.st_mtime_ns).encode("utf-8")) + if path.is_file(): + digest.update(str(stat.st_size).encode("utf-8")) + return digest.hexdigest() + + +def _get_booter_skills_revision(booter: ComputerBooter) -> str | None: + value = getattr(booter, "_astrbot_skills_revision", None) + return value if isinstance(value, str) and value else None + + +def _set_booter_skills_revision(booter: ComputerBooter, revision: str) -> None: + setattr(booter, "_astrbot_skills_revision", revision) + + def _discover_bay_credentials(endpoint: str) -> str: """Try to auto-discover Bay API key from credentials.json. @@ -374,6 +410,7 @@ async def _sync_skills_to_sandbox(booter: ComputerBooter) -> None: skills_root = Path(get_astrbot_skills_path()) if not skills_root.is_dir(): return + local_skills_revision = _compute_local_skills_revision(skills_root) local_skill_dirs = _list_local_skill_dirs(skills_root) temp_dir = Path(get_astrbot_temp_path()) @@ -403,10 +440,12 @@ async def _sync_skills_to_sandbox(booter: ComputerBooter) -> None: await _apply_skills_to_sandbox(booter) payload = await _scan_sandbox_skills(booter) _update_sandbox_skills_cache(payload) + _set_booter_skills_revision(booter, local_skills_revision) managed = payload.get("managed_skills", []) if isinstance(payload, dict) else [] logger.info( - "[Computer] Sandbox skill sync complete: managed=%d", + "[Computer] Sandbox skill sync complete: managed=%d, revision=%s", len(managed), + local_skills_revision[:12], ) finally: if zip_path.exists(): @@ -428,6 +467,9 @@ async def get_booter( elif runtime == "none": raise RuntimeError("Sandbox runtime is disabled by configuration.") + skills_root = Path(get_astrbot_skills_path()) + current_skills_revision = _compute_local_skills_revision(skills_root) + sandbox_cfg = config.get("provider_settings", {}).get("sandbox", {}) booter_type = sandbox_cfg.get("booter", "shipyard_neo") @@ -436,6 +478,19 @@ async def get_booter( if not await booter.available(): # rebuild session_booter.pop(session_id, None) + elif _get_booter_skills_revision(booter) != current_skills_revision: + logger.info( + "[Computer] Local skills changed for session=%s, refreshing sandbox before reuse", + session_id, + ) + try: + await _sync_skills_to_sandbox(booter) + except Exception as e: + logger.warning( + "Failed to refresh sandbox skills before reusing session %s: %s", + session_id, + e, + ) if session_id not in session_booter: uuid_str = uuid.uuid5(uuid.NAMESPACE_DNS, session_id).hex logger.info( diff --git a/tests/test_computer_skill_sync.py b/tests/test_computer_skill_sync.py index 777cd44fcb..1ae1dbc903 100644 --- a/tests/test_computer_skill_sync.py +++ b/tests/test_computer_skill_sync.py @@ -74,6 +74,35 @@ def _fake_set_cache(self, skills): ] +def test_sync_skills_sets_revision_after_success(monkeypatch, tmp_path: Path): + skills_root = tmp_path / "skills" + temp_root = tmp_path / "temp" + skill_dir = skills_root / "custom-agent-skill" + skill_dir.mkdir(parents=True, exist_ok=True) + skill_dir.joinpath("SKILL.md").write_text("# demo", encoding="utf-8") + temp_root.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr( + "astrbot.core.computer.computer_client.get_astrbot_skills_path", + lambda: str(skills_root), + ) + monkeypatch.setattr( + "astrbot.core.computer.computer_client.get_astrbot_temp_path", + lambda: str(temp_root), + ) + monkeypatch.setattr( + "astrbot.core.computer.computer_client.SkillManager.set_sandbox_skills_cache", + lambda self, skills: None, + ) + + booter = _FakeBooter( + '{"skills":[{"name":"custom-agent-skill","description":"","path":"skills/custom-agent-skill/SKILL.md"}]}' + ) + asyncio.run(computer_client._sync_skills_to_sandbox(booter)) + + assert getattr(booter, "_astrbot_skills_revision", None) + + def test_sync_skills_uses_managed_strategy_instead_of_wiping_all( monkeypatch, tmp_path: Path, diff --git a/tests/unit/test_computer.py b/tests/unit/test_computer.py index 07a5449c19..a818b7c43c 100644 --- a/tests/unit/test_computer.py +++ b/tests/unit/test_computer.py @@ -732,6 +732,94 @@ async def test_get_booter_reuses_existing(self): # Cleanup computer_client.session_booter.clear() + @pytest.mark.asyncio + async def test_get_booter_refreshes_existing_session_when_skills_change(self): + """Test get_booter re-syncs local skills before reusing a stale sandbox.""" + from astrbot.core.computer import computer_client + + computer_client.session_booter.clear() + + mock_context = MagicMock() + mock_config = MagicMock() + mock_config.get = lambda key, default=None: { + "provider_settings": { + "computer_use_runtime": "sandbox", + "sandbox": { + "booter": "shipyard", + "shipyard_endpoint": "http://localhost:8080", + "shipyard_access_token": "test_token", + } + } + }.get(key, default) + mock_context.get_config = MagicMock(return_value=mock_config) + + mock_booter = MagicMock() + mock_booter.available = AsyncMock(return_value=True) + mock_booter._astrbot_skills_revision = "rev-old" + + sync_mock = AsyncMock() + with ( + patch( + "astrbot.core.computer.computer_client._compute_local_skills_revision", + return_value="rev-new", + ), + patch( + "astrbot.core.computer.computer_client._sync_skills_to_sandbox", + sync_mock, + ), + ): + computer_client.session_booter["test-session"] = mock_booter + + booter = await computer_client.get_booter(mock_context, "test-session") + assert booter is mock_booter + sync_mock.assert_awaited_once_with(mock_booter) + + computer_client.session_booter.clear() + + @pytest.mark.asyncio + async def test_get_booter_skips_resync_when_skills_revision_matches(self): + """Test get_booter reuses existing booter without redundant syncs.""" + from astrbot.core.computer import computer_client + + computer_client.session_booter.clear() + + mock_context = MagicMock() + mock_config = MagicMock() + mock_config.get = lambda key, default=None: { + "provider_settings": { + "computer_use_runtime": "sandbox", + "sandbox": { + "booter": "shipyard", + "shipyard_endpoint": "http://localhost:8080", + "shipyard_access_token": "test_token", + } + } + }.get(key, default) + mock_context.get_config = MagicMock(return_value=mock_config) + + mock_booter = MagicMock() + mock_booter.available = AsyncMock(return_value=True) + mock_booter._astrbot_skills_revision = "rev-same" + + sync_mock = AsyncMock() + with ( + patch( + "astrbot.core.computer.computer_client._compute_local_skills_revision", + return_value="rev-same", + ), + patch( + "astrbot.core.computer.computer_client._sync_skills_to_sandbox", + sync_mock, + ), + ): + computer_client.session_booter["test-session"] = mock_booter + + booter = await computer_client.get_booter(mock_context, "test-session") + assert booter is mock_booter + sync_mock.assert_not_awaited() + + computer_client.session_booter.clear() + @pytest.mark.asyncio async def test_get_booter_rebuild_unavailable(self): """Test get_booter rebuilds when existing booter is unavailable."""