diff --git a/skillclaw/cli.py b/skillclaw/cli.py index 529c702..017fad5 100644 --- a/skillclaw/cli.py +++ b/skillclaw/cli.py @@ -892,6 +892,9 @@ def skills_pull(): cfg, hub = _require_sharing(cs) click.echo(f"Pulling skills from {_sharing_target(cfg)} ...") result = hub.pull_skills(cfg.skills_dir) + if result.get("locked_out"): + click.echo("Skipped: another pull is already running (skills left untouched).") + return msg = ( f"Done: {result['downloaded']} downloaded, " f"{result['skipped']} unchanged, " diff --git a/skillclaw/dashboard_ingest.py b/skillclaw/dashboard_ingest.py index 3db7ce3..a6f6205 100644 --- a/skillclaw/dashboard_ingest.py +++ b/skillclaw/dashboard_ingest.py @@ -18,7 +18,7 @@ from evolve_server.core.skill_registry import SkillIDRegistry from evolve_server.core.utils import build_skill_md from evolve_server.storage.oss_helpers import fetch_version_bundle, load_version_bundle_record -from skillclaw.skill_bundle import bundle_entrypoint_text, read_skill_bundle_with_meta +from skillclaw.skill_bundle import bundle_entrypoint_text, iter_skill_md_paths, read_skill_bundle_with_meta from .config import SkillClawConfig from .skill_hub import SkillHub @@ -241,7 +241,8 @@ def _load_local_skills(config: SkillClawConfig, warnings: list[str]) -> dict[str stats = {} skills: dict[str, dict[str, Any]] = {} - for skill_path in sorted(skills_dir.rglob("SKILL.md")): + for skill_md in iter_skill_md_paths(skills_dir): + skill_path = Path(skill_md) bundle_files, bundle_records, local_tree_sha = read_skill_bundle_with_meta(skill_path.parent) try: raw = bundle_entrypoint_text(bundle_files) diff --git a/skillclaw/dashboard_store.py b/skillclaw/dashboard_store.py index 59b9e4f..e0937a9 100644 --- a/skillclaw/dashboard_store.py +++ b/skillclaw/dashboard_store.py @@ -11,7 +11,8 @@ def _json_dumps(value: Any) -> str: - return json.dumps(value, ensure_ascii=False) + # date/datetime из фронтматтера скиллов не сериализуются напрямую — пишем ISO-строкой + return json.dumps(value, ensure_ascii=False, default=str) def _json_loads(raw: str | None, default: Any) -> Any: diff --git a/skillclaw/skill_bundle.py b/skillclaw/skill_bundle.py index 0f50263..35d07d1 100644 --- a/skillclaw/skill_bundle.py +++ b/skillclaw/skill_bundle.py @@ -2,6 +2,7 @@ from __future__ import annotations +import glob import hashlib import os import shutil @@ -18,6 +19,34 @@ class SkillBundleError(ValueError): """Raised when a bundle is malformed or a bundle path is unsafe.""" +def is_hermes_skill_root(skills_dir: str | os.PathLike[str]) -> bool: + """True для каталога скиллов hermes — он один допускает раскладку с категориями.""" + return os.path.realpath(str(skills_dir)) == os.path.realpath( + os.path.join(os.path.expanduser("~"), ".hermes", "skills") + ) + + +def iter_skill_md_paths(skills_dir: str | os.PathLike[str]) -> list[str]: + """Пути к SKILL.md настоящих скиллов — единая точка обхода дерева. + + Скиллом считается / или, в раскладке hermes, //. + SKILL.md глубже — пример или вложенный набор внутри чужого бандла: такие каталоги + ломали mirror-pull (удалялись как stale вместе с родителем) и засоряли каталог + скиллов, доступных агенту. + """ + root = str(skills_dir) + if not is_hermes_skill_root(root): + return sorted(glob.glob(os.path.join(root, "*", _BUNDLE_ENTRYPOINT))) + + out: list[str] = [] + for path in glob.glob(os.path.join(root, "**", _BUNDLE_ENTRYPOINT), recursive=True): + rel = os.path.relpath(os.path.dirname(path), root) + if len(rel.split(os.sep)) > 2: + continue + out.append(path) + return sorted(out) + + def _coerce_bytes(data: bytes | bytearray | str) -> bytes: if isinstance(data, (bytes, bytearray)): return bytes(data) diff --git a/skillclaw/skill_hub.py b/skillclaw/skill_hub.py index 3bd16d7..357a9db 100644 --- a/skillclaw/skill_hub.py +++ b/skillclaw/skill_hub.py @@ -15,12 +15,13 @@ from __future__ import annotations -import glob +import fcntl import hashlib import json import logging import os import shutil +from contextlib import contextmanager from datetime import datetime, timezone from typing import Any, Collection, Optional @@ -32,6 +33,7 @@ bundle_file_records, bundle_has_only_entrypoint, bundle_tree_sha256, + iter_skill_md_paths, read_skill_bundle_with_meta, write_skill_bundle, ) @@ -39,6 +41,42 @@ logger = logging.getLogger(__name__) +@contextmanager +def _pull_lock(skills_dir: str): + """Межпроцессный лок на pull: launchd-инстанс, поллер и CLI ходят в один каталог. + + Отдаёт False, если pull уже идёт где-то ещё, — цикл просто пропускается. + """ + lock_path = os.path.join(os.path.dirname(os.path.abspath(skills_dir)), ".skillclaw_pull.lock") + fd = os.open(lock_path, os.O_CREAT | os.O_RDWR, 0o644) + try: + try: + fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + except OSError: + yield False + return + try: + yield True + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + finally: + os.close(fd) + + +def _restore_skills_dir(skills_dir: str, backup_dir: str, stamp: str) -> None: + """Откат на бэкап: сначала уводим сломанный каталог целиком, потом кладём копию. + + rmtree по месту оставлял недоудалённые каталоги и валился на `Directory not empty`. + """ + broken_dir = f"{skills_dir}.broken_{stamp}" + if os.path.isdir(skills_dir): + os.rename(skills_dir, broken_dir) + try: + shutil.copytree(backup_dir, skills_dir) + finally: + shutil.rmtree(broken_dir, ignore_errors=True) + + def _is_hermes_skill_root(skills_dir: str) -> bool: return os.path.realpath(skills_dir) == os.path.realpath(os.path.join(os.path.expanduser("~"), ".hermes", "skills")) @@ -323,10 +361,7 @@ def push_skills( Returns {"uploaded": N, "skipped": M, "filtered": F, "total_local": T}. """ - if _is_hermes_skill_root(skills_dir): - paths = sorted(glob.glob(os.path.join(skills_dir, "**", "SKILL.md"), recursive=True)) - else: - paths = sorted(glob.glob(os.path.join(skills_dir, "*", "SKILL.md"))) + paths = iter_skill_md_paths(skills_dir) if not paths: logger.info("[SkillHub] no local skills to push") return {"uploaded": 0, "skipped": 0, "filtered": 0, "total_local": 0} @@ -469,18 +504,9 @@ def _list_local_skill_dirs(skills_dir: str) -> dict[str, list[str]]: out: dict[str, list[str]] = {} if not os.path.isdir(skills_dir): return out - if _is_hermes_skill_root(skills_dir): - for path in sorted(glob.glob(os.path.join(skills_dir, "**", "SKILL.md"), recursive=True)): - skill_dir = os.path.dirname(path) - name = os.path.basename(skill_dir) - out.setdefault(name, []).append(skill_dir) - return out - for entry in os.scandir(skills_dir): - if not entry.is_dir(): - continue - skill_md = os.path.join(entry.path, "SKILL.md") - if os.path.isfile(skill_md): - out.setdefault(entry.name, []).append(entry.path) + for path in iter_skill_md_paths(skills_dir): + skill_dir = os.path.dirname(path) + out.setdefault(os.path.basename(skill_dir), []).append(skill_dir) return out @classmethod @@ -531,6 +557,31 @@ def _remove_duplicate_local_skill_dirs( shutil.rmtree(skill_dir) logger.info("[SkillHub] removed duplicate local skill dir: %s", skill_dir) + def _mirror_pull_is_noop( + self, + skills_dir: str, + manifest: dict[str, dict[str, Any]], + local_skills: dict[str, str], + local_dirs_by_name: dict[str, list[str]], + skip_set: set[str], + ) -> bool: + """True, если mirror-pull ничего не изменит: нет stale, дублей и расхождений по хэшу.""" + if set(local_skills) - set(manifest): + return False + if any(len(dirs) > 1 for dirs in local_dirs_by_name.values()): + return False + + for name, rec in manifest.items(): + category = str(rec.get("category", "general") or "general") + target_dir = self._resolve_pull_target_dir(skills_dir, name, category, local_dirs_by_name) + if name in skip_set: + if not os.path.exists(os.path.join(target_dir, "SKILL.md")): + return False + continue + if not os.path.isdir(target_dir) or not self._local_bundle_matches_record(target_dir, rec): + return False + return True + @staticmethod def _prune_backups(backup_root: str, prefix: str, keep: int = 3) -> None: """Keep only newest `keep` backups for current skills dir.""" @@ -552,6 +603,33 @@ def pull_skills( mirror: bool = True, skip_names: Optional[Collection[str]] = None, include_names: Optional[Collection[str]] = None, + ) -> dict[str, Any]: + """Serialize pulls across processes, then delegate to :meth:`_pull_skills_locked`.""" + with _pull_lock(skills_dir) as acquired: + if not acquired: + logger.info("[SkillHub] pull already running in another process, cycle skipped") + return { + "downloaded": 0, + "skipped": 0, + "deleted": 0, + "total_remote": 0, + "restored_from_backup": False, + "backup_dir": "", + "locked_out": True, + } + return self._pull_skills_locked( + skills_dir, + mirror=mirror, + skip_names=skip_names, + include_names=include_names, + ) + + def _pull_skills_locked( + self, + skills_dir: str, + mirror: bool = True, + skip_names: Optional[Collection[str]] = None, + include_names: Optional[Collection[str]] = None, ) -> dict[str, Any]: """Mirror cloud skills to local directory with backup + rollback safety. @@ -694,6 +772,19 @@ def _result( backup_dir="", ) + # Сверяем хэши до копирования: mirror-pull переписывает весь каталог (бэкап + staging) + # на каждом цикле поллера, а обычно менять нечего — 102 скилла копировались раз в 30 секунд. + if self._mirror_pull_is_noop(skills_dir, manifest, local_skills, local_dirs_by_name, skip_set): + logger.info("[SkillHub] pull complete: 0 downloaded, %d skipped, 0 deleted, %d total remote", len(manifest), len(manifest)) + return _result( + downloaded=0, + skipped=len(manifest), + deleted=0, + total_remote=len(manifest), + restored_from_backup=False, + backup_dir="", + ) + parent_dir = os.path.dirname(os.path.abspath(skills_dir)) base_name = os.path.basename(os.path.abspath(skills_dir)) stamp = datetime.now(timezone.utc).strftime("%Y%m%dT%H%M%S%fZ") @@ -756,7 +847,11 @@ def _result( remote_names = set(manifest.keys()) local_names = set(local_skills.keys()) for stale in sorted(local_names - remote_names): - shutil.rmtree(local_skills[stale], ignore_errors=False) + stale_dir = local_skills[stale] + # каталог мог уйти вместе с родительским скиллом — это не повод откатывать весь pull + if not os.path.isdir(stale_dir): + continue + shutil.rmtree(stale_dir, ignore_errors=True) deleted += 1 for name in sorted(remote_names): @@ -778,9 +873,7 @@ def _result( except Exception as e: logger.warning("[SkillHub] mirror pull failed, restoring backup: %s", e) try: - if os.path.isdir(skills_dir): - shutil.rmtree(skills_dir) - shutil.copytree(backup_dir, skills_dir) + _restore_skills_dir(skills_dir, backup_dir, stamp) restored_from_backup = True logger.info("[SkillHub] local skills restored from backup: %s", backup_dir) except Exception as restore_err: diff --git a/skillclaw/skill_manager.py b/skillclaw/skill_manager.py index 2acbfd9..47c9d3d 100644 --- a/skillclaw/skill_manager.py +++ b/skillclaw/skill_manager.py @@ -47,7 +47,6 @@ communication, automation, agentic, productivity, common_mistakes """ -import glob import hashlib import json import logging @@ -59,7 +58,7 @@ import yaml -from .skill_bundle import list_skill_bundle_paths +from .skill_bundle import iter_skill_md_paths, list_skill_bundle_paths logger = logging.getLogger(__name__) @@ -318,11 +317,7 @@ def _load_skills(self) -> Dict[str, Any]: return result def _skill_md_paths(self) -> list[str]: - if self._is_hermes_skill_root(): - pattern = os.path.join(self._skills_dir, "**", "SKILL.md") - return sorted(glob.glob(pattern, recursive=True)) - pattern = os.path.join(self._skills_dir, "*", "SKILL.md") - return sorted(glob.glob(pattern)) + return iter_skill_md_paths(self._skills_dir) def _compute_skills_fingerprint(self) -> tuple[tuple[str, int, int], ...]: fingerprint: list[tuple[str, int, int]] = []