diff --git a/astrbot/core/computer/computer_client.py b/astrbot/core/computer/computer_client.py index fed81ab22..bf698b941 100644 --- a/astrbot/core/computer/computer_client.py +++ b/astrbot/core/computer/computer_client.py @@ -1,10 +1,10 @@ -import os +import json import shutil import uuid from pathlib import Path from astrbot.api import logger -from astrbot.core.skills.skill_manager import SANDBOX_SKILLS_ROOT +from astrbot.core.skills.skill_manager import SANDBOX_SKILLS_ROOT, SkillManager from astrbot.core.star.context import Context from astrbot.core.utils.astrbot_path import ( get_astrbot_skills_path, @@ -16,49 +16,225 @@ from .booters.local import LocalBooter session_booter: dict[str, ComputerBooter] = {} local_booter: ComputerBooter | None = None +_MANAGED_SKILLS_FILE = ".astrbot_managed_skills.json" + + +def _list_local_skill_dirs(skills_root: Path) -> list[Path]: + skills: list[Path] = [] + for entry in sorted(skills_root.iterdir()): + if not entry.is_dir(): + continue + skill_md = entry / "SKILL.md" + if skill_md.exists(): + skills.append(entry) + return skills + + +def _build_sync_and_scan_command() -> str: + script = f""" +import json +import shutil +import zipfile +from pathlib import Path + +root = Path({SANDBOX_SKILLS_ROOT!r}) +zip_path = root / "skills.zip" +tmp_extract = Path(f"{{root}}_tmp_extract") +managed_file = root / {_MANAGED_SKILLS_FILE!r} + + +def parse_description(text: str) -> str: + if not text.startswith("---"): + return "" + lines = text.splitlines() + if not lines or lines[0].strip() != "---": + return "" + end_idx = None + for i in range(1, len(lines)): + if lines[i].strip() == "---": + end_idx = i + break + if end_idx is None: + return "" + for line in lines[1:end_idx]: + if ":" not in line: + continue + key, value = line.split(":", 1) + if key.strip().lower() == "description": + return value.strip().strip('"').strip("'") + return "" + + +def remove_tree(path: Path) -> None: + if not path.exists(): + return + if path.is_dir(): + shutil.rmtree(path, ignore_errors=True) + else: + path.unlink(missing_ok=True) + + +def load_managed_skills() -> list[str]: + if not managed_file.exists(): + return [] + try: + payload = json.loads(managed_file.read_text(encoding="utf-8")) + except Exception: + return [] + if not isinstance(payload, dict): + return [] + items = payload.get("managed_skills", []) + if not isinstance(items, list): + return [] + result: list[str] = [] + for item in items: + if isinstance(item, str) and item.strip(): + result.append(item.strip()) + return result + + +def collect_skills() -> list[dict[str, str]]: + skills: list[dict[str, str]] = [] + if not root.exists(): + return skills + for skill_dir in sorted(root.iterdir()): + if not skill_dir.is_dir(): + continue + skill_md = skill_dir / "SKILL.md" + if not skill_md.is_file(): + continue + description = "" + try: + text = skill_md.read_text(encoding="utf-8") + description = parse_description(text) + except Exception: + description = "" + skills.append( + {{ + "name": skill_dir.name, + "description": description, + "path": f"{SANDBOX_SKILLS_ROOT}/{{skill_dir.name}}/SKILL.md", + }} + ) + return skills + + +root.mkdir(parents=True, exist_ok=True) +for managed_name in load_managed_skills(): + remove_tree(root / managed_name) + +current_managed: list[str] = [] +if zip_path.exists(): + remove_tree(tmp_extract) + tmp_extract.mkdir(parents=True, exist_ok=True) + with zipfile.ZipFile(zip_path) as zf: + zf.extractall(tmp_extract) + for entry in sorted(tmp_extract.iterdir()): + if not entry.is_dir(): + continue + target = root / entry.name + remove_tree(target) + shutil.copytree(entry, target) + current_managed.append(entry.name) + +remove_tree(tmp_extract) +remove_tree(zip_path) +managed_file.write_text( + json.dumps({{"managed_skills": current_managed}}, ensure_ascii=False, indent=2), + encoding="utf-8", +) +print( + json.dumps( + {{ + "managed_skills": current_managed, + "skills": collect_skills(), + }}, + ensure_ascii=False, + ) +) +""".strip() + return ( + "if command -v python3 >/dev/null 2>&1; then PYBIN=python3; " + "elif command -v python >/dev/null 2>&1; then PYBIN=python; " + "else echo 'python not found in sandbox' >&2; exit 127; fi; " + "$PYBIN - <<'PY'\n" + f"{script}\n" + "PY" + ) + + +def _shell_exec_succeeded(result: dict) -> bool: + if "success" in result: + return bool(result.get("success")) + exit_code = result.get("exit_code") + return exit_code in (0, None) + + +def _decode_sync_payload(stdout: str) -> dict | None: + text = stdout.strip() + if not text: + return None + candidates = [text] + candidates.extend([line.strip() for line in text.splitlines() if line.strip()]) + for candidate in reversed(candidates): + try: + payload = json.loads(candidate) + except Exception: + continue + if isinstance(payload, dict): + return payload + return None + + +def _update_sandbox_skills_cache(payload: dict | None) -> None: + if not isinstance(payload, dict): + return + skills = payload.get("skills", []) + if not isinstance(skills, list): + return + SkillManager().set_sandbox_skills_cache(skills) async def _sync_skills_to_sandbox(booter: ComputerBooter) -> None: - skills_root = get_astrbot_skills_path() - if not os.path.isdir(skills_root): - return - if not any(Path(skills_root).iterdir()): + skills_root = Path(get_astrbot_skills_path()) + if not skills_root.is_dir(): return + local_skill_dirs = _list_local_skill_dirs(skills_root) - temp_dir = get_astrbot_temp_path() - os.makedirs(temp_dir, exist_ok=True) - zip_base = os.path.join(temp_dir, "skills_bundle") - zip_path = f"{zip_base}.zip" + temp_dir = Path(get_astrbot_temp_path()) + temp_dir.mkdir(parents=True, exist_ok=True) + zip_base = temp_dir / "skills_bundle" + zip_path = zip_base.with_suffix(".zip") try: - if os.path.exists(zip_path): - os.remove(zip_path) - shutil.make_archive(zip_base, "zip", skills_root) - remote_zip = Path(SANDBOX_SKILLS_ROOT) / "skills.zip" - logger.info("Uploading skills bundle to sandbox...") - await booter.shell.exec(f"mkdir -p {SANDBOX_SKILLS_ROOT}") - upload_result = await booter.upload_file(zip_path, str(remote_zip)) - if not upload_result.get("success", False): - raise RuntimeError("Failed to upload skills bundle to sandbox.") - # Extract into a temp folder first, then replace skills atomically. - tmp_extract_root = f"{SANDBOX_SKILLS_ROOT}_tmp_extract" - await booter.shell.exec( - f"mkdir -p {SANDBOX_SKILLS_ROOT} {tmp_extract_root} && " - f"rm -rf {tmp_extract_root}/* && " - f"(unzip -o {remote_zip} -d {tmp_extract_root} || " - f"python3 -c \"import zipfile; z=zipfile.ZipFile('{remote_zip}'); " - f"z.extractall('{tmp_extract_root}')\" || " - f"python -c \"import zipfile; z=zipfile.ZipFile('{remote_zip}'); " - f"z.extractall('{tmp_extract_root}')\") && " - f"find {SANDBOX_SKILLS_ROOT} -mindepth 1 -delete && " - f"cp -a {tmp_extract_root}/. {SANDBOX_SKILLS_ROOT}/ && " - f"rm -rf {tmp_extract_root} && " - f"rm -f {remote_zip}" - ) + if local_skill_dirs: + if zip_path.exists(): + zip_path.unlink() + shutil.make_archive(str(zip_base), "zip", str(skills_root)) + remote_zip = Path(SANDBOX_SKILLS_ROOT) / "skills.zip" + logger.info("Uploading skills bundle to sandbox...") + await booter.shell.exec(f"mkdir -p {SANDBOX_SKILLS_ROOT}") + upload_result = await booter.upload_file(str(zip_path), str(remote_zip)) + if not upload_result.get("success", False): + raise RuntimeError("Failed to upload skills bundle to sandbox.") + else: + logger.info( + "No local skills found. Keeping sandbox built-ins and refreshing metadata." + ) + await booter.shell.exec(f"rm -f {SANDBOX_SKILLS_ROOT}/skills.zip") + + sync_result = await booter.shell.exec(_build_sync_and_scan_command()) + if not _shell_exec_succeeded(sync_result): + raise RuntimeError( + "Failed to apply sandbox skill sync strategy: " + f"stderr={sync_result.get('stderr', '')}" + ) + payload = _decode_sync_payload(str(sync_result.get("stdout", "") or "")) + _update_sandbox_skills_cache(payload) finally: - if os.path.exists(zip_path): + if zip_path.exists(): try: - os.remove(zip_path) + zip_path.unlink() except Exception: logger.warning(f"Failed to remove temp skills zip: {zip_path}") diff --git a/astrbot/core/skills/skill_manager.py b/astrbot/core/skills/skill_manager.py index 1e6f01a6d..fab38d1d1 100644 --- a/astrbot/core/skills/skill_manager.py +++ b/astrbot/core/skills/skill_manager.py @@ -7,6 +7,7 @@ import shutil import tempfile import zipfile from dataclasses import dataclass +from datetime import datetime, timezone from pathlib import Path, PurePosixPath from astrbot.core.utils.astrbot_path import ( @@ -16,9 +17,11 @@ from astrbot.core.utils.astrbot_path import ( ) SKILLS_CONFIG_FILENAME = "skills.json" +SANDBOX_SKILLS_CACHE_FILENAME = "sandbox_skills_cache.json" DEFAULT_SKILLS_CONFIG: dict[str, dict] = {"skills": {}} # SANDBOX_SKILLS_ROOT = "/home/shared/skills" SANDBOX_SKILLS_ROOT = "skills" +_SANDBOX_SKILLS_CACHE_VERSION = 1 _SKILL_NAME_RE = re.compile(r"^[A-Za-z0-9._-]+$") @@ -91,7 +94,9 @@ def build_skills_prompt(skills: list[SkillInfo]) -> str: class SkillManager: def __init__(self, skills_root: str | None = None) -> None: self.skills_root = skills_root or get_astrbot_skills_path() - self.config_path = os.path.join(get_astrbot_data_path(), SKILLS_CONFIG_FILENAME) + data_path = Path(get_astrbot_data_path()) + self.config_path = str(data_path / SKILLS_CONFIG_FILENAME) + self.sandbox_skills_cache_path = str(data_path / SANDBOX_SKILLS_CACHE_FILENAME) os.makedirs(self.skills_root, exist_ok=True) os.makedirs(get_astrbot_temp_path(), exist_ok=True) @@ -109,6 +114,54 @@ class SkillManager: with open(self.config_path, "w", encoding="utf-8") as f: json.dump(config, f, ensure_ascii=False, indent=4) + def _load_sandbox_skills_cache(self) -> dict: + if not os.path.exists(self.sandbox_skills_cache_path): + return {"version": _SANDBOX_SKILLS_CACHE_VERSION, "skills": []} + try: + with open(self.sandbox_skills_cache_path, encoding="utf-8") as f: + data = json.load(f) + if not isinstance(data, dict): + return {"version": _SANDBOX_SKILLS_CACHE_VERSION, "skills": []} + skills = data.get("skills", []) + if not isinstance(skills, list): + skills = [] + return { + "version": int(data.get("version", _SANDBOX_SKILLS_CACHE_VERSION)), + "skills": skills, + } + except Exception: + return {"version": _SANDBOX_SKILLS_CACHE_VERSION, "skills": []} + + def _save_sandbox_skills_cache(self, cache: dict) -> None: + cache["version"] = _SANDBOX_SKILLS_CACHE_VERSION + cache["updated_at"] = datetime.now(timezone.utc).isoformat() + with open(self.sandbox_skills_cache_path, "w", encoding="utf-8") as f: + json.dump(cache, f, ensure_ascii=False, indent=2) + + def set_sandbox_skills_cache(self, skills: list[dict]) -> None: + """Persist sandbox skill metadata discovered from runtime side.""" + deduped: dict[str, dict[str, str]] = {} + for item in skills: + if not isinstance(item, dict): + continue + name = str(item.get("name", "")).strip() + if not name or not _SKILL_NAME_RE.match(name): + continue + description = str(item.get("description", "") or "") + path = str(item.get("path", "") or "") + if not path: + path = f"{SANDBOX_SKILLS_ROOT}/{name}/SKILL.md" + deduped[name] = { + "name": name, + "description": description, + "path": path.replace("\\", "/"), + } + cache = { + "version": _SANDBOX_SKILLS_CACHE_VERSION, + "skills": [deduped[name] for name in sorted(deduped)], + } + self._save_sandbox_skills_cache(cache) + def list_skills( self, *, @@ -125,7 +178,7 @@ class SkillManager: config = self._load_config() skill_configs = config.get("skills", {}) modified = False - skills: list[SkillInfo] = [] + skills_by_name: dict[str, SkillInfo] = {} for entry in sorted(Path(self.skills_root).iterdir()): if not entry.is_dir(): @@ -151,20 +204,50 @@ class SkillManager: else: path_str = str(skill_md) path_str = path_str.replace("\\", "/") - skills.append( - SkillInfo( + skills_by_name[skill_name] = SkillInfo( + name=skill_name, + description=description, + path=path_str, + active=active, + ) + + if runtime == "sandbox": + cache = self._load_sandbox_skills_cache() + for item in cache.get("skills", []): + if not isinstance(item, dict): + continue + skill_name = str(item.get("name", "")).strip() + if ( + not skill_name + or skill_name in skills_by_name + or not _SKILL_NAME_RE.match(skill_name) + ): + continue + active = skill_configs.get(skill_name, {}).get("active", True) + if skill_name not in skill_configs: + skill_configs[skill_name] = {"active": active} + modified = True + if active_only and not active: + continue + description = str(item.get("description", "") or "") + if show_sandbox_path: + path_str = f"{SANDBOX_SKILLS_ROOT}/{skill_name}/SKILL.md" + else: + path_str = str(item.get("path", "") or "") + if not path_str: + path_str = f"{SANDBOX_SKILLS_ROOT}/{skill_name}/SKILL.md" + skills_by_name[skill_name] = SkillInfo( name=skill_name, description=description, - path=path_str, + path=path_str.replace("\\", "/"), active=active, ) - ) if modified: config["skills"] = skill_configs self._save_config(config) - return skills + return [skills_by_name[name] for name in sorted(skills_by_name)] def set_skill_active(self, name: str, active: bool) -> None: config = self._load_config() diff --git a/astrbot/dashboard/routes/skills.py b/astrbot/dashboard/routes/skills.py index 9c58a4a7e..327cc6f41 100644 --- a/astrbot/dashboard/routes/skills.py +++ b/astrbot/dashboard/routes/skills.py @@ -116,6 +116,11 @@ class SkillsRoute(Route): skill_mgr = SkillManager() skill_name = skill_mgr.install_skill_from_zip(temp_path, overwrite=True) + try: + await sync_skills_to_active_sandboxes() + except Exception: + logger.warning("Failed to sync uploaded skills to active sandboxes.") + return ( Response() .ok({"name": skill_name}, "Skill uploaded successfully.") @@ -163,6 +168,10 @@ class SkillsRoute(Route): if not name: return Response().error("Missing skill name").__dict__ SkillManager().delete_skill(name) + try: + await sync_skills_to_active_sandboxes() + except Exception: + logger.warning("Failed to sync deleted skills to active sandboxes.") return Response().ok({"name": name}).__dict__ except Exception as e: logger.error(traceback.format_exc()) diff --git a/tests/test_computer_skill_sync.py b/tests/test_computer_skill_sync.py new file mode 100644 index 000000000..777cd44fc --- /dev/null +++ b/tests/test_computer_skill_sync.py @@ -0,0 +1,123 @@ +from __future__ import annotations + +import asyncio +from pathlib import Path + +from astrbot.core.computer import computer_client + + +class _FakeShell: + def __init__(self, sync_payload_json: str): + self.sync_payload_json = sync_payload_json + self.commands: list[str] = [] + + async def exec(self, command: str, **kwargs): + _ = kwargs + self.commands.append(command) + if "PYBIN" in command and "managed_skills" in command: + return { + "success": True, + "stdout": self.sync_payload_json, + "stderr": "", + "exit_code": 0, + } + return {"success": True, "stdout": "", "stderr": "", "exit_code": 0} + + +class _FakeBooter: + def __init__(self, sync_payload_json: str): + self.shell = _FakeShell(sync_payload_json) + self.uploads: list[tuple[str, str]] = [] + + async def upload_file(self, path: str, file_name: str) -> dict: + self.uploads.append((path, file_name)) + return {"success": True} + + +def test_sync_skills_keeps_builtin_skills_when_local_is_empty(monkeypatch, tmp_path: Path): + skills_root = tmp_path / "skills" + temp_root = tmp_path / "temp" + skills_root.mkdir(parents=True, exist_ok=True) + temp_root.mkdir(parents=True, exist_ok=True) + + captured = {"skills": None} + + def _fake_set_cache(self, skills): + captured["skills"] = skills + + 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", + _fake_set_cache, + ) + + booter = _FakeBooter( + '{"skills":[{"name":"python-sandbox","description":"ship","path":"skills/python-sandbox/SKILL.md"}]}' + ) + asyncio.run(computer_client._sync_skills_to_sandbox(booter)) + + assert booter.uploads == [] + assert any(cmd == "rm -f skills/skills.zip" for cmd in booter.shell.commands) + assert captured["skills"] == [ + { + "name": "python-sandbox", + "description": "ship", + "path": "skills/python-sandbox/SKILL.md", + } + ] + + +def test_sync_skills_uses_managed_strategy_instead_of_wiping_all( + 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) + + captured = {"skills": None} + + def _fake_set_cache(self, skills): + captured["skills"] = skills + + 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", + _fake_set_cache, + ) + + 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 len(booter.uploads) == 1 + assert booter.uploads[0][1] == "skills/skills.zip" + assert not any( + "find skills -mindepth 1 -delete" in cmd for cmd in booter.shell.commands + ) + assert captured["skills"] == [ + { + "name": "custom-agent-skill", + "description": "", + "path": "skills/custom-agent-skill/SKILL.md", + } + ] + diff --git a/tests/test_skill_manager_sandbox_cache.py b/tests/test_skill_manager_sandbox_cache.py new file mode 100644 index 000000000..88923ec10 --- /dev/null +++ b/tests/test_skill_manager_sandbox_cache.py @@ -0,0 +1,104 @@ +from __future__ import annotations + +from pathlib import Path + +from astrbot.core.skills.skill_manager import SkillManager + + +def _write_skill(root: Path, name: str, description: str) -> None: + skill_dir = root / name + skill_dir.mkdir(parents=True, exist_ok=True) + skill_dir.joinpath("SKILL.md").write_text( + f"---\ndescription: {description}\n---\n# {name}\n", + encoding="utf-8", + ) + + +def test_list_skills_merges_local_and_sandbox_cache(monkeypatch, tmp_path: Path): + data_dir = tmp_path / "data" + temp_dir = tmp_path / "temp" + skills_root = tmp_path / "skills" + data_dir.mkdir(parents=True, exist_ok=True) + temp_dir.mkdir(parents=True, exist_ok=True) + skills_root.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr( + "astrbot.core.skills.skill_manager.get_astrbot_data_path", + lambda: str(data_dir), + ) + monkeypatch.setattr( + "astrbot.core.skills.skill_manager.get_astrbot_temp_path", + lambda: str(temp_dir), + ) + + mgr = SkillManager(skills_root=str(skills_root)) + _write_skill(skills_root, "custom-local", "local description") + + mgr.set_sandbox_skills_cache( + [ + { + "name": "python-sandbox", + "description": "ship built-in", + "path": "/app/skills/python-sandbox/SKILL.md", + }, + { + "name": "custom-local", + "description": "should be ignored by local override", + "path": "skills/custom-local/SKILL.md", + }, + ] + ) + + skills = mgr.list_skills(runtime="sandbox") + by_name = {item.name: item for item in skills} + + assert sorted(by_name) == ["custom-local", "python-sandbox"] + assert by_name["custom-local"].description == "local description" + assert by_name["custom-local"].path == "skills/custom-local/SKILL.md" + assert by_name["python-sandbox"].description == "ship built-in" + assert by_name["python-sandbox"].path == "skills/python-sandbox/SKILL.md" + + +def test_sandbox_cached_skill_respects_active_and_display_path( + monkeypatch, + tmp_path: Path, +): + data_dir = tmp_path / "data" + temp_dir = tmp_path / "temp" + skills_root = tmp_path / "skills" + data_dir.mkdir(parents=True, exist_ok=True) + temp_dir.mkdir(parents=True, exist_ok=True) + skills_root.mkdir(parents=True, exist_ok=True) + + monkeypatch.setattr( + "astrbot.core.skills.skill_manager.get_astrbot_data_path", + lambda: str(data_dir), + ) + monkeypatch.setattr( + "astrbot.core.skills.skill_manager.get_astrbot_temp_path", + lambda: str(temp_dir), + ) + + mgr = SkillManager(skills_root=str(skills_root)) + mgr.set_sandbox_skills_cache( + [ + { + "name": "browser-automation", + "description": "gull built-in", + "path": "/app/skills/browser-automation/SKILL.md", + } + ] + ) + + all_skills = mgr.list_skills( + runtime="sandbox", + active_only=False, + show_sandbox_path=False, + ) + assert len(all_skills) == 1 + assert all_skills[0].path == "/app/skills/browser-automation/SKILL.md" + + mgr.set_skill_active("browser-automation", False) + active_skills = mgr.list_skills(runtime="sandbox", active_only=True) + assert active_skills == [] +