Fix mirror pull failing on nested skill bundles, concurrent pulls, and dashboard sync crash

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, "

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)

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:

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 настоящих скиллов — единая точка обхода дерева.
+
+    Скиллом считается <root>/<name> или, в раскладке hermes, <root>/<category>/<name>.
+    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)

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,13 +33,50 @@
     bundle_file_records,
     bundle_has_only_entrypoint,
     bundle_tree_sha256,
+    iter_skill_md_paths,
     read_skill_bundle_with_meta,
     write_skill_bundle,
 )
 
 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:

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]] = []