fix(profiles): polled profile lists never walk skill trees; vanished skill dirs no longer abort enumeration

GET /api/profiles, the profiles.list RPC and the /api/profiles/projects/tree fan-out are polled by the Desktop every few seconds (roster tick, focus, gateway-open). Each call ran list_profiles() -> _count_skills() -> Path.rglob("SKILL.md") over EVERY profile once the 30 s TTL expired: ~4 fs calls per skill, 2.4-4.7 s per walk on 7-84 profile installs, ~half a core at idle, and on Linux enough to starve the renderer's 60 s API timeout. A skill dir removed mid-walk raised FileNotFoundError out of rglob and aborted the whole profile list. - list_profiles(lazy_skill_count=True): skill_count is the last known value; a missing/aged entry schedules ONE background _count_skills per profile per 60 s recheck window, so the request thread does zero skill-tree I/O and the refresh cadence is decoupled from the poll rate. The two polled callers and the REST fallback entry use it; the sync default (CLI, detail views) is unchanged. - _walk_skill_count: the repo walker (agent.skill_utils.iter_skill_index_files, os.walk with excluded/support dirs pruned) instead of rglob — half the fs calls and best-effort on subtrees that vanish mid-walk. profiles.describe uses the same walker. - _profile_targets always uses profiles_to_serve (pure directory read): projects/tree and sessions/pull-requests only ever consumed name/path. - Skill-count TTL 30 s -> 600 s (signature invalidation still catches skill add/remove immediately on the next refresh). Live repro (5 profiles x 200 SKILL.md, temp HERMES_HOME, py3.11): before: GET /api/profiles 2175 stat + 2015 scandir per cold call, profiles.list 2189 + 2015, projects/tree 2177 + 2015; a skill dir removed mid-walk -> FileNotFoundError from list_profiles() after: GET /api/profiles 0 skill-tree stats/scandir on the request thread (counts land from the background refresh by the next poll), profiles.list 0, projects/tree 0; _count_skills (detail/control) still reports 200 with 203 scandir; the vanished-dir case returns 199 and list_profiles() enumerates all 5 profiles. Fixes #114041 Co-authored-by: KoNit-K <konit.block@protonmail.com>
2026-09-18 00:55:55 -07:00
parent 6689bddeb7
commit 4590ef8b58
7 changed files with 166 additions and 31 deletions
--- a/hermes_cli/profiles.py
+++ b/hermes_cli/profiles.py
@@ -10,12 +10,12 @@ import shutil
 import stat
 import subprocess
 import sys
+import threading
 import time
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Dict, List, Optional, Tuple

-from agent.skill_utils import is_excluded_skill_path
 from hermes_cli.archive_safe import archive_root_dirs, make_targz, normalize_archive_parts, safe_extract_targz
 from hermes_constants import (
    LOCAL_RUNTIME_ROOT_DIRS, clear_named_profile_deleted, mark_named_profile_deleted, named_profile_has_identity,
@@ -647,13 +647,18 @@ def _served_by_running_multiplexer(profile_name: str) -> bool:
        return False


-# In-process skill-count cache. ``rglob("SKILL.md")`` walks every skill's sub-trees; the
-# default profile alone has ~270 skills and ``list_profiles`` counts EVERY profile (16+), so
-# an uncached scan costs ~6s — enough for the desktop's per-request calls to time out and
-# the sidebar to render "全部智能体 0". Keyed by skills dir, invalidated when the tree
-# signature changes (skill add/remove) or after a short TTL (deep edits).
+# In-process skill-count cache. Counting walks every skill's sub-tree (~4 fs calls per
+# skill); ``list_profiles`` counts EVERY profile, and its two remaining Desktop callers
+# (``GET /api/profiles``, ``profiles.list``) are POLLED every few seconds. Keyed by skills
+# dir; a walk is repeated only when the tree signature changes (skill add/remove) or after
+# the TTL (deep edits). Polled callers never walk: ``lazy_skill_count`` serves the last known
+# value and refreshes stale entries on a background thread, at most once per recheck window
+# per profile — the TTL is decoupled from the poll rate (#114041).
 _SKILL_COUNT_CACHE: dict[str, tuple[float, float, int]] = {}
-_SKILL_COUNT_TTL_SECONDS = 30.0
+_SKILL_COUNT_TTL_SECONDS = 600.0
+_SKILL_COUNT_RECHECK_SECONDS = 60.0
+_SKILL_COUNT_NEXT_CHECK: dict[str, float] = {}
+_SKILL_COUNT_LOCK = threading.Lock()


 def _skills_dir_signature(skills_dir: Path) -> float:
@@ -676,8 +681,22 @@ def _skills_dir_signature(skills_dir: Path) -> float:
    return sig


+def _walk_skill_count(skills_dir: Path) -> int:
+    """One ``os.walk`` over the skills tree (prunes ``.git``/``node_modules``/support dirs
+    instead of statting them). Best-effort: a subtree that vanishes mid-walk (a concurrent
+    skill install/update) is skipped, never raised — one profile's churn must not abort the
+    whole enumeration."""
+    from agent.skill_utils import iter_skill_index_files
+    try:
+        return sum(1 for _ in iter_skill_index_files(skills_dir, "SKILL.md"))
+    except OSError:
+        return 0
+
+
 def _count_skills(profile_dir: Path) -> int:
-    """Count installed skills in a profile (cached by skills-dir signature)."""
+    """Count installed skills in a profile (cached by skills-dir signature + TTL). Walks
+    synchronously when stale — detail surfaces (``hermes profile info``, ``profiles.describe``)
+    want the fresh number; polled lists go through :func:`_cached_skill_count`."""
    skills_dir = profile_dir / "skills"
    if not skills_dir.is_dir():
        return 0
@@ -687,11 +706,29 @@ def _count_skills(profile_dir: Path) -> int:
    cached = _SKILL_COUNT_CACHE.get(key)
    if cached is not None and cached[0] == signature and (now - cached[1]) < _SKILL_COUNT_TTL_SECONDS:
        return cached[2]
-    count = sum(1 for md in skills_dir.rglob("SKILL.md") if not is_excluded_skill_path(md))
+    count = _walk_skill_count(skills_dir)
    _SKILL_COUNT_CACHE[key] = (signature, now, count)
    return count


+def _cached_skill_count(profile_dir: Path) -> int:
+    """Last known skill count with ZERO skill-tree I/O on the calling thread. A never-counted
+    or aged entry schedules one background :func:`_count_skills` per profile per recheck
+    window (which itself re-walks only on signature change / TTL); the next poll picks the
+    result up. ``0`` until the first refresh lands."""
+    key = str(profile_dir / "skills")
+    now = time.time()
+    with _SKILL_COUNT_LOCK:
+        due = _SKILL_COUNT_NEXT_CHECK.get(key, 0.0) <= now
+        if due:
+            _SKILL_COUNT_NEXT_CHECK[key] = now + _SKILL_COUNT_RECHECK_SECONDS
+    if due:
+        threading.Thread(target=_count_skills, args=(profile_dir,),
+                         name="hermes-skill-count", daemon=True).start()
+    cached = _SKILL_COUNT_CACHE.get(key)
+    return cached[2] if cached is not None else 0
+
+
 # profile.yaml — per-profile metadata (description, role, etc.)
 # Deliberately tiny and separate from ``config.yaml`` (user-facing Hermes config, ~5000
 # lines of defaults): this is metadata ABOUT the profile. Missing file -> empty defaults,
@@ -764,7 +801,8 @@ def set_profile_display_name(profile_name: str, display_name: str) -> str:

 # CRUD operations

-def _profile_info(name: str, path: Path, *, is_default: bool, alias_name: Optional[str] = None) -> ProfileInfo:
+def _profile_info(name: str, path: Path, *, is_default: bool, alias_name: Optional[str] = None,
+                  lazy_skill_count: bool = False) -> ProfileInfo:
    """Build one :class:`ProfileInfo` from a profile directory."""
    model, provider = _read_config_model(path)
    dist_name, dist_version, dist_source = _read_distribution_meta(path)
@@ -775,27 +813,34 @@ def _profile_info(name: str, path: Path, *, is_default: bool, alias_name: Option
    gateway_running = _check_gateway_running(path)
    if not is_default:
        gateway_running = gateway_running or _served_by_running_multiplexer(name)
+    skill_count = _cached_skill_count(path) if lazy_skill_count else _count_skills(path)
    return ProfileInfo(
        name=name, path=path, is_default=is_default, gateway_running=gateway_running, model=model,
-        provider=provider, has_env=(path / ".env").exists(), skill_count=_count_skills(path),
+        provider=provider, has_env=(path / ".env").exists(), skill_count=skill_count,
        alias_path=alias_path, alias_name=alias_name, distribution_name=dist_name,
        distribution_version=dist_version, distribution_source=dist_source,
        **meta,
    )


-def list_profiles() -> List[ProfileInfo]:
-    """Return info for all profiles, including the default."""
+def list_profiles(*, lazy_skill_count: bool = False) -> List[ProfileInfo]:
+    """Return info for all profiles, including the default.
+
+    ``lazy_skill_count=True`` is for POLLED callers (``GET /api/profiles``, ``profiles.list``):
+    ``skill_count`` is the last known value, refreshed off-request, so the request never walks
+    a skill tree (#114041). Default ``False`` counts synchronously (CLI, detail views)."""
    profiles = []
    default_home = _get_default_hermes_home()
    if default_home.is_dir():
-        profiles.append(_profile_info("default", default_home, is_default=True))
+        profiles.append(_profile_info("default", default_home, is_default=True,
+                                      lazy_skill_count=lazy_skill_count))
    named = _iter_named_profile_dirs()
    if named:
        alias_map = build_alias_map()  # ONCE, not per profile (was the dominant cost)
        for entry in named:
            alias_name = alias_map.get(normalize_profile_name(entry.name))
-            profiles.append(_profile_info(entry.name, entry, is_default=False, alias_name=alias_name))
+            profiles.append(_profile_info(entry.name, entry, is_default=False, alias_name=alias_name,
+                                          lazy_skill_count=lazy_skill_count))
    return profiles


--- a/hermes_cli/web_routers/profiles.py
+++ b/hermes_cli/web_routers/profiles.py
@@ -187,16 +187,16 @@ def _best_effort(log_msg: str, *args, fn, default=None):
        return default


-def _profile_targets(log_label: str, *, lightweight: bool) -> List[Tuple[str, Path]]:
-    """(name, home) for every profile, falling back to ``default`` alone. ``lightweight``
-    uses ``profiles_to_serve`` (name/path only) instead of ``list_profiles``, which parses
-    config/meta and probes gateways/skills per profile — too heavy per sidebar refresh."""
+def _profile_targets(log_label: str) -> List[Tuple[str, Path]]:
+    """(name, home) for every profile, falling back to ``default`` alone. Uses
+    ``profiles_to_serve`` (pure directory read) instead of ``list_profiles``, which parses
+    config/meta and probes gateways per profile — every caller here is a polled sidebar
+    fan-out that only needs name/path (#114041)."""
    from hermes_cli import profiles as profiles_mod
    try:
-        targets = (list(profiles_mod.profiles_to_serve(multiplex=True)) if lightweight
-                   else [(info.name, info.path) for info in profiles_mod.list_profiles()])
+        targets = list(profiles_mod.profiles_to_serve(multiplex=True))
    except Exception:
-        _log.exception("%s: list_profiles failed", log_label)
+        _log.exception("%s: profile enumeration failed", log_label)
        targets = []
    if not targets:
        targets.append(("default", profiles_mod.get_profile_dir("default")))
@@ -395,7 +395,7 @@ def get_profiles_sessions(
        raise HTTPException(status_code=400, detail="order must be one of: created, recent")

    targets = ([_cron_profile_home(profile)] if profile and profile != "all"
-               else _profile_targets("GET /api/profiles/sessions", lightweight=True))
+               else _profile_targets("GET /api/profiles/sessions"))

    # Source scoping (see /api/sessions): recents pass exclude_sources=cron, the cron-jobs
    # section source=cron — two independent lists so cron sessions can't starve recents.
@@ -446,7 +446,7 @@ def get_profiles_sessions_sidebar(

    See #42651, #65710, #70629.
    """
-    targets = _profile_targets("GET /api/profiles/sessions/sidebar", lightweight=True)
+    targets = _profile_targets("GET /api/profiles/sessions/sidebar")

    recents_scope = (recents_profile or "all").strip() or "all"
    recents_exclude_list = [s for s in (recents_exclude or "").split(",") if s.strip()]
@@ -591,7 +591,7 @@ def get_profiles_projects_tree(preview_limit: int = 3, session_limit: int = 2000
    scoped_session_ids: List[str] = []
    errors: List[Dict[str, str]] = []

-    for name, home in _profile_targets("GET /api/profiles/projects/tree", lightweight=False):
+    for name, home in _profile_targets("GET /api/profiles/projects/tree"):
        def _read(db, name=name, home=home):
            with _hermes_home_scope(home):
                tree, _active_id = gateway_server._build_project_tree(
@@ -643,7 +643,7 @@ def post_profiles_sessions_pull_requests(body: SessionPrScanBody):
                # Oldest-first, so a later `gh pr create` (the replacement PR) wins.
                found[pr["session_id"]] = {"number": parsed[0], "url": parsed[1]}

-    for name, home in _profile_targets("POST /api/profiles/sessions/pull-requests", lightweight=False):
+    for name, home in _profile_targets("POST /api/profiles/sessions/pull-requests"):
        _read_profile_db(name, home, None, _read)

    # ``scanned``: every id looked at, so the caller can remember "nothing there".
@@ -654,7 +654,8 @@ def post_profiles_sessions_pull_requests(body: SessionPrScanBody):
 def _read_profiles():
    from hermes_cli import profiles as profiles_mod
    try:
-        profiles = profiles_mod.list_profiles()
+        # Polled by the Desktop on every focus/roster tick: never walk skill trees in-request.
+        profiles = profiles_mod.list_profiles(lazy_skill_count=True)
        return {"profiles": [_profile_to_dict(p) for p in profiles]}
    except Exception:
        _log.exception("GET /api/profiles failed; falling back to profile directory scan")
--- a/hermes_cli/web_server_profiles.py
+++ b/hermes_cli/web_server_profiles.py
@@ -106,7 +106,7 @@ def _fallback_profile_entry(profiles_mod, name: str, home: Path, *, is_default:
    return {
        "name": name, "path": str(home), "is_default": is_default, "model": model,
        "provider": provider, "has_env": has_env,
-        "skill_count": _safe(lambda: profiles_mod._count_skills(home), 0),
+        "skill_count": _safe(lambda: profiles_mod._cached_skill_count(home), 0),
        "gateway_running": _safe(gateway_running, False),
        "description": meta("description", ""), "description_auto": meta("description_auto", False),
        "bot_title": meta("bot_title", ""),
--- a/tests/hermes_cli/test_profiles.py
+++ b/tests/hermes_cli/test_profiles.py
@@ -757,6 +757,91 @@ class TestListProfiles:
        assert "alpha" in names
        assert "beta" in names

+    def test_lazy_skill_count_never_walks_in_the_polled_request(self, profile_env, monkeypatch):
+        """Polled surfaces (``profiles.list`` RPC, ``GET /api/profiles``) must render
+        ``skill_count`` without any skill-tree walk on the request thread; the count arrives
+        from one background refresh per profile per recheck window (#114041). Control: the
+        synchronous ``list_profiles()`` still walks and reports the fresh number."""
+        import threading
+        import tui_gateway.server as srv
+
+        skills = profile_env / ".hermes" / "skills" / "cat"
+        for i in range(3):
+            (skills / f"s{i}").mkdir(parents=True)
+            (skills / f"s{i}" / "SKILL.md").write_text("# s\n", encoding="utf-8")
+        profiles._SKILL_COUNT_CACHE.clear()
+        profiles._SKILL_COUNT_NEXT_CHECK.clear()
+
+        walks: list[str] = []
+        real_walk = profiles._walk_skill_count
+
+        def spy(skills_dir):
+            walks.append(threading.current_thread().name)
+            return real_walk(skills_dir)
+
+        monkeypatch.setattr(profiles, "_walk_skill_count", spy)
+
+        def _rpc():
+            return srv._methods["profiles.list"](1, {"include_sessions": False})["result"]["profiles"]
+
+        first = _rpc()
+        assert walks == [] or set(walks) == {"hermes-skill-count"}
+        assert first[0]["skill_count"] in (0, 3)  # 0 until the refresh lands, never a stall
+        for t in threading.enumerate():
+            if t.name == "hermes-skill-count":
+                t.join(timeout=10)
+        assert walks == ["hermes-skill-count"]
+        assert _rpc()[0]["skill_count"] == 3
+        assert list_profiles(lazy_skill_count=True)[0].skill_count == 3
+        assert walks == ["hermes-skill-count"]  # a second poll inside the window schedules nothing
+
+        # Control: the detail/CLI path counts synchronously on the caller's thread.
+        profiles._SKILL_COUNT_CACHE.clear()
+        assert list_profiles()[0].skill_count == 3
+        assert walks[-1] == threading.current_thread().name
+
+    def test_skill_count_survives_subtree_vanishing_mid_walk(self, profile_env, monkeypatch):
+        """A skill removed while the tree is being counted (concurrent install/update) must
+        degrade the count, not abort profile enumeration with ``FileNotFoundError``."""
+        skills = profile_env / ".hermes" / "skills" / "cat"
+        for i in range(4):
+            (skills / f"s{i}" / "references").mkdir(parents=True)
+            (skills / f"s{i}" / "SKILL.md").write_text("# s\n", encoding="utf-8")
+        profiles._SKILL_COUNT_CACHE.clear()
+        real_scandir = os.scandir
+
+        class _Listing:
+            """A pre-read scandir result (context manager + iterator, like the real one)."""
+            def __init__(self, entries):
+                self._it = iter(entries)
+
+            def __enter__(self):
+                return self
+
+            def __exit__(self, *exc):
+                return False
+
+            def __iter__(self):
+                return self
+
+            def __next__(self):
+                return next(self._it)
+
+            def close(self):
+                pass
+
+        def vanishing_scandir(path=".", *args, **kwargs):
+            with real_scandir(path, *args, **kwargs) as listing:
+                entries = list(listing)
+            if not isinstance(path, int) and os.fspath(path) == str(skills):
+                # Listed, then gone before the walk descends into it.
+                shutil.rmtree(skills / "s3", ignore_errors=True)
+            return _Listing(entries)
+
+        monkeypatch.setattr(os, "scandir", vanishing_scandir)
+        assert profiles._count_skills(profile_env / ".hermes") == 3
+        assert [p.name for p in list_profiles()] == ["default"]
+

 # ===================================================================
 # TestActiveProfile
--- a/tests/hermes_cli/test_web_read_coalescing.py
+++ b/tests/hermes_cli/test_web_read_coalescing.py
@@ -30,7 +30,7 @@ async def test_profiles_burst_leaves_threadpool_status_responsive(monkeypatch):
    release = threading.Event()
    calls = []

-    def slow_profiles():
+    def slow_profiles(**_kwargs):  # the router passes ``lazy_skill_count=True``
        calls.append(1)
        assert release.wait(3)
        return ["example"]
--- a/tui_gateway/methods_profiles.py
+++ b/tui_gateway/methods_profiles.py
@@ -252,7 +252,8 @@ def _(rid, params: dict) -> dict:
    from hermes_cli.profiles import list_profiles
    include_sessions = is_truthy_value(params.get("include_sessions", True))
    out = []
-    for p in list_profiles():
+    # Roster polls this every 5s: ``skill_count`` is the last known value, refreshed off-request.
+    for p in list_profiles(lazy_skill_count=True):
        row = {"name": p.name, "path": str(p.path), "is_default": bool(p.is_default), "model": p.model,
               "provider": p.provider, "description": p.description or "",
               "display_name": p.display_name or "", "skill_count": p.skill_count or 0}
@@ -434,6 +435,7 @@ def _(rid, params: dict) -> dict:
    if err is not None:
        return err
    with _hermes_home_scope(profile_dir):
+        from agent.skill_utils import iter_skill_index_files
        from hermes_cli.config import load_config
        from hermes_cli.skills_config import get_disabled_skills
        cfg = load_config() or {}
@@ -441,7 +443,7 @@ def _(rid, params: dict) -> dict:
        skills_root = profile_dir / "skills"
        installed = [
            {"name": md.parent.name, "enabled": md.parent.name.lower() not in disabled}
-            for md in (sorted(skills_root.rglob("SKILL.md")) if skills_root.is_dir() else ())]
+            for md in (iter_skill_index_files(skills_root, "SKILL.md") if skills_root.is_dir() else ())]
        toolsets_out, pinned_set = _describe_toolsets(cfg)
        soul_path = profile_dir / "SOUL.md"
        soul = _try(lambda: soul_path.read_text(encoding="utf-8", errors="replace") if soul_path.is_file() else "", "")
--- a/website/docs/reference/profile-commands.md
+++ b/website/docs/reference/profile-commands.md
@@ -174,6 +174,8 @@ hermes profile show <name>

 Displays details about a profile including its home directory, configured model, gateway status, skills count, and configuration file status.

+The skills count here (and in `hermes profile list`) is counted on the spot. The Desktop and dashboard profile lists are polled every few seconds, so they show the last known count instead and refresh it in the background — a freshly started backend may briefly show `0` skills for a profile until the first background count lands, and a skill you just installed appears in those lists within about a minute.
+
 This shows the profile's Hermes home directory, not the terminal working directory. Terminal commands start from `terminal.cwd` (or the launch directory on the local backend when `cwd: "."`).

 | Argument | Description |