From 4590ef8b58a129d30cf20a5b4a35c23240eebb96 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Fri, 18 Sep 2026 00:55:55 -0700 Subject: [PATCH] fix(profiles): polled profile lists never walk skill trees; vanished skill dirs no longer abort enumeration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- hermes_cli/profiles.py | 75 +++++++++++++---- hermes_cli/web_routers/profiles.py | 25 +++--- hermes_cli/web_server_profiles.py | 2 +- tests/hermes_cli/test_profiles.py | 85 ++++++++++++++++++++ tests/hermes_cli/test_web_read_coalescing.py | 2 +- tui_gateway/methods_profiles.py | 6 +- website/docs/reference/profile-commands.md | 2 + 7 files changed, 166 insertions(+), 31 deletions(-) diff --git a/hermes_cli/profiles.py b/hermes_cli/profiles.py index 9cc7e62db6..6bedee98ec 100644 --- 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 diff --git a/hermes_cli/web_routers/profiles.py b/hermes_cli/web_routers/profiles.py index bb2141bdcb..1adc8ce25a 100644 --- 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") diff --git a/hermes_cli/web_server_profiles.py b/hermes_cli/web_server_profiles.py index 9842627a16..03c7672205 100644 --- 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", ""), diff --git a/tests/hermes_cli/test_profiles.py b/tests/hermes_cli/test_profiles.py index 298c316de7..6df1b99110 100644 --- 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 diff --git a/tests/hermes_cli/test_web_read_coalescing.py b/tests/hermes_cli/test_web_read_coalescing.py index d1e1a0db8e..946fb82503 100644 --- 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"] diff --git a/tui_gateway/methods_profiles.py b/tui_gateway/methods_profiles.py index 78ba30a637..83938e5d71 100644 --- 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 "", "") diff --git a/website/docs/reference/profile-commands.md b/website/docs/reference/profile-commands.md index 0f7a83ca30..8e34e33136 100644 --- a/website/docs/reference/profile-commands.md +++ b/website/docs/reference/profile-commands.md @@ -174,6 +174,8 @@ hermes profile show 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 |