perf(skills): share raw config cache in skill utils (#46149)

agent/skill_utils.py

@@ -272,6 +272,49 @@ def skill_matches_environment(frontmatter: Dict[str, Any]) -> bool:
 # ── Disabled skills ───────────────────────────────────────────────────────
 
 
+_RAW_CONFIG_CACHE: Dict[Tuple[str, int, int], Dict[str, Any]] = {}
+
+
+def _raw_config_cache_clear() -> None:
+    """Test hook — drop the shared raw config cache."""
+    _RAW_CONFIG_CACHE.clear()
+
+
+def _load_raw_config() -> Dict[str, Any]:
+    """Read config.yaml with a shared mtime+size keyed cache.
+
+    This module intentionally avoids importing ``hermes_cli.config`` on the
+    skill prompt/build path. A tiny local cache gives the same repeated-read
+    win without pulling the heavier CLI config stack into startup.
+    """
+    config_path = get_config_path()
+    if not config_path.exists():
+        return {}
+    try:
+        stat = config_path.stat()
+        cache_key = (str(config_path), stat.st_mtime_ns, stat.st_size)
+    except OSError:
+        cache_key = None
+
+    if cache_key is not None:
+        cached = _RAW_CONFIG_CACHE.get(cache_key)
+        if cached is not None:
+            return cached
+
+    try:
+        parsed = yaml_load(config_path.read_text(encoding="utf-8"))
+    except Exception as e:
+        logger.debug("Could not read skill config %s: %s", config_path, e)
+        return {}
+    if not isinstance(parsed, dict):
+        return {}
+
+    if cache_key is not None:
+        _RAW_CONFIG_CACHE.clear()
+        _RAW_CONFIG_CACHE[cache_key] = parsed
+    return parsed
+
+
 def get_disabled_skill_names(platform: str | None = None) -> Set[str]:
     """Read disabled skill names from config.yaml.
 
@@ -286,15 +329,8 @@ def get_disabled_skill_names(platform: str | None = None) -> Set[str]:
     Reads the config file directly (no CLI config imports) to stay
     lightweight.
     """
-    config_path = get_config_path()
-    if not config_path.exists():
-        return set()
-    try:
-        parsed = yaml_load(config_path.read_text(encoding="utf-8"))
-    except Exception as e:
-        logger.debug("Could not read skill config %s: %s", config_path, e)
-        return set()
-    if not isinstance(parsed, dict):
+    parsed = _load_raw_config()
+    if not parsed:
         return set()
 
     skills_cfg = parsed.get("skills")
@@ -339,6 +375,7 @@ _EXTERNAL_DIRS_CACHE: Dict[Tuple[str, int], List[Path]] = {}
 def _external_dirs_cache_clear() -> None:
     """Test hook — drop the in-process cache."""
     _EXTERNAL_DIRS_CACHE.clear()
+    _raw_config_cache_clear()
 
 
 def get_external_skills_dirs() -> List[Path]:
@@ -371,11 +408,8 @@ def get_external_skills_dirs() -> List[Path]:
             # Return a copy so callers can't mutate the cached list.
             return list(cached)
 
-    try:
-        parsed = yaml_load(config_path.read_text(encoding="utf-8"))
-    except Exception:
-        return []
-    if not isinstance(parsed, dict):
+    parsed = _load_raw_config()
+    if not parsed:
         return []
 
     skills_cfg = parsed.get("skills")
@@ -587,15 +621,7 @@ def resolve_skill_config_values(
     current values (or the declared default if the key isn't set).
     Path values are expanded via ``os.path.expanduser``.
     """
-    config_path = get_config_path()
-    config: Dict[str, Any] = {}
-    if config_path.exists():
-        try:
-            parsed = yaml_load(config_path.read_text(encoding="utf-8"))
-            if isinstance(parsed, dict):
-                config = parsed
-        except Exception:
-            pass
+    config = _load_raw_config()
 
     resolved: Dict[str, Any] = {}
     for var in config_vars:

tests/agent/test_skill_utils.py

@@ -4,7 +4,10 @@ from unittest.mock import patch
 
 from agent.skill_utils import (
     extract_skill_conditions,
+    get_disabled_skill_names,
+    get_external_skills_dirs,
     iter_skill_index_files,
+    resolve_skill_config_values,
     skill_matches_platform,
 )
 
@@ -102,6 +105,69 @@ def test_iter_skill_index_files_prunes_dependency_dirs(tmp_path):
     assert found == [real / "SKILL.md"]
 
 
+def test_skill_config_helpers_share_raw_config_parse_cache(tmp_path, monkeypatch):
+    """Repeated skill config helpers should parse config.yaml only once."""
+    from agent import skill_utils
+
+    hermes_home = tmp_path / ".hermes"
+    hermes_home.mkdir()
+    external = tmp_path / "external-skills"
+    external.mkdir()
+    config_path = hermes_home / "config.yaml"
+    config_path.write_text(
+        f"""
+skills:
+  disabled:
+    - hidden-skill
+  external_dirs:
+    - {external}
+  config:
+    wiki:
+      path: ~/wiki
+""".strip(),
+        encoding="utf-8",
+    )
+    parse_count = 0
+    real_yaml_load = skill_utils.yaml_load
+
+    def counting_yaml_load(text):
+        nonlocal parse_count
+        parse_count += 1
+        return real_yaml_load(text)
+
+    monkeypatch.setenv("HERMES_HOME", str(hermes_home))
+    skill_utils._external_dirs_cache_clear()
+    getattr(skill_utils, "_raw_config_cache_clear", lambda: None)()
+    monkeypatch.setattr(skill_utils, "yaml_load", counting_yaml_load)
+
+    assert get_disabled_skill_names() == {"hidden-skill"}
+    assert get_external_skills_dirs() == [external.resolve()]
+    assert resolve_skill_config_values([
+        {"key": "wiki.path", "description": "Wiki path"}
+    ])["wiki.path"].endswith("/wiki")
+    assert parse_count == 1
+
+
+def test_skill_config_raw_cache_invalidates_on_config_edit(tmp_path, monkeypatch):
+    """Editing config.yaml should invalidate the shared raw config cache."""
+    from agent import skill_utils
+
+    hermes_home = tmp_path / ".hermes"
+    hermes_home.mkdir()
+    config_path = hermes_home / "config.yaml"
+    config_path.write_text("skills:\n  disabled: [old-skill]\n", encoding="utf-8")
+
+    monkeypatch.setenv("HERMES_HOME", str(hermes_home))
+    skill_utils._external_dirs_cache_clear()
+    assert get_disabled_skill_names() == {"old-skill"}
+
+    config_path.write_text("skills:\n  disabled: [new-skill]\n", encoding="utf-8")
+    import os
+    os.utime(config_path, None)
+
+    assert get_disabled_skill_names() == {"new-skill"}
+
+
 # ── skill_matches_platform on Termux ──────────────────────────────────────