ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 11 Packages Projects Releases Wiki Activity

refactor(tools): skills_sync reuses skill_usage name/suppression readers, compact docs

Browse Source
This commit is contained in:
Teknium
2026-09-02 22:24:40 -07:00
parent 1523a3ba9c
commit ea20e366d5
3 changed files with 83 additions and 150 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/skills_sync.py
+42 -92
View File
@@ -1,15 +1,11 @@
#!/usr/bin/env python3
"""
Skills Sync -- Manifest-based seeding and updating of bundled skills.
"""Skills Sync -- manifest-based seeding and updating of bundled skills.
Copies bundled skills from the repo's skills/ directory into ~/.hermes/skills/,
tracking each synced skill's origin hash in ~/.hermes/skills/.bundled_manifest
(v2: one "skill_name:origin_hash" line each; v1 plain-name lines auto-migrate).
Update logic: NEW skills are copied and recorded; EXISTING skills update only
when bundled changed AND the user copy still matches the origin hash (a differing
user copy is user-customized -> SKIP); skills the user DELETED are not re-added;
skills REMOVED upstream are cleaned from the manifest.
Copies repo skills/ into ~/.hermes/skills/, tracking each synced skill's origin
hash in .bundled_manifest (v2 "name:hash" lines; v1 plain names auto-migrate).
NEW skills are copied and recorded; EXISTING skills update only when bundled
changed AND the user copy still matches the origin hash (else user-customized ->
SKIP); user-DELETED skills are not re-added; upstream-REMOVED ones leave the manifest.
"""
import hashlib
@@ -22,17 +18,16 @@ from dataclasses import dataclass, field
from pathlib import Path
from typing import Dict, Iterator, List, Optional, Set, Tuple
# Force stdout/stderr to UTF-8: on GBK-style Windows locales the default encoding
# can't represent the glyphs printed here (✓ ↑ →) and raises mid-run; install.ps1
# also parses this script's stdout as UTF-8 and aborts on a GBK byte stream.
# Force UTF-8 stdout/stderr: GBK-style Windows locales can't encode the glyphs
# printed here (✓ ↑ →), and install.ps1 parses this script's stdout as UTF-8.
for _stream in (sys.stdout, sys.stderr):
if hasattr(_stream, "reconfigure"):
try:
_stream.reconfigure(encoding="utf-8", errors="replace")
except (ValueError, TypeError):
pass
try:
_stream.reconfigure(encoding="utf-8", errors="replace")
except (AttributeError, ValueError, TypeError):
pass
from hermes_constants import get_bundled_skills_dir, get_hermes_home, get_optional_skills_dir
from agent.skill_utils import ESSENTIAL_SKILLS, is_excluded_skill_path
from tools.skill_usage import _read_skill_name, read_suppressed_names # noqa: F401 (re-exported)
from utils import atomic_write_text
logger = logging.getLogger(__name__)
@@ -41,10 +36,9 @@ HERMES_HOME = get_hermes_home()
SKILLS_DIR = HERMES_HOME / "skills"
MANIFEST_FILE = SKILLS_DIR / ".bundled_manifest"
# Import-time snapshots backing the call-time accessors below. Long-lived
# multi-profile runtimes import this module once and later retarget HERMES_HOME
# via set_hermes_home_override(); frozen constants would then resolve (and for
# reset_bundled_skill() DELETE) against the wrong profile. The accessors honor
# Import-time snapshots backing the call-time accessors: long-lived multi-profile
# runtimes retarget HERMES_HOME after import, and frozen constants would resolve
# (and for reset_bundled_skill() DELETE) against the wrong profile. Accessors honor
# an explicitly patched module global and otherwise re-resolve on every call.
_HERMES_HOME_AT_IMPORT = HERMES_HOME
_SKILLS_DIR_AT_IMPORT = SKILLS_DIR
@@ -68,9 +62,8 @@ def _manifest_file() -> Path:
return _live(MANIFEST_FILE, _MANIFEST_FILE_AT_IMPORT, lambda: _skills_dir() / ".bundled_manifest")
# Written by `hermes profile create --no-skills` / the installer's `--no-skills`;
# when present in HERMES_HOME, sync_skills() seeds only the essential skills. Mirrors
# hermes_cli.profiles.NO_BUNDLED_SKILLS_MARKER (literal: no CLI import in this module).
# Written by `hermes profile create --no-skills` / installer `--no-skills`: sync seeds
# only essential skills. Mirrors hermes_cli.profiles.NO_BUNDLED_SKILLS_MARKER (no CLI import here).
NO_BUNDLED_SKILLS_MARKER = ".no-bundled-skills"
@@ -125,20 +118,8 @@ def _read_manifest() -> Dict[str, str]:
def _read_suppressed_names() -> set:
"""Built-in skills the curator pruned — must NOT be re-seeded. Delegates to
``tools.skill_usage`` (source of truth), falling back to reading ``.curator_suppressed``
directly if that import is unavailable in a packaged/update context."""
try:
from tools.skill_usage import read_suppressed_names
return read_suppressed_names()
except Exception:
path = _skills_dir() / ".curator_suppressed"
try:
lines = path.read_text(encoding="utf-8").splitlines() if path.exists() else []
except OSError:
return set()
return {line.strip() for line in lines if line.strip() and not line.strip().startswith("#")}
"""Built-in skills the curator pruned — must NOT be re-seeded (tests patch this name)."""
return read_suppressed_names()
def _write_manifest(entries: Dict[str, str]):
@@ -152,28 +133,10 @@ def _write_manifest(entries: Dict[str, str]):
logger.debug("Failed to write skills manifest %s: %s", _manifest_file(), e, exc_info=True)
def _read_skill_name(skill_md: Path, fallback: str) -> str:
"""Read the name field from SKILL.md YAML frontmatter, falling back to *fallback*."""
try:
content = skill_md.read_text(encoding="utf-8", errors="replace")[:4000]
except OSError:
return fallback
in_frontmatter = False
for stripped in map(str.strip, content.split("\n")):
if stripped == "---":
if in_frontmatter:
break
in_frontmatter = True
elif in_frontmatter and stripped.startswith("name:"):
if value := stripped.split(":", 1)[1].strip().strip("\"'"):
return value
return fallback
def _discover_bundled_skills(bundled_dir: Path) -> List[Tuple[str, Path]]:
"""``(skill_name, skill_dir)`` for every SKILL.md under the bundled dir. Exclusions
are evaluated relative to the bundled tree: the install prefix itself may contain
``venv``/``site-packages``, which once made every wheel install discover zero skills."""
"""``(skill_name, skill_dir)`` per SKILL.md under the bundled dir. Exclusions are
evaluated relative to the bundled tree: the install prefix itself may contain
``venv``/``site-packages`` (which once made wheel installs discover zero skills)."""
if not bundled_dir.exists():
return []
return [
@@ -225,12 +188,9 @@ def _copy_dir(src: Path, dest: Path) -> None:
def _recover_renamed_skill(st: "_SyncState", skill_name: str, dest: Path) -> Optional[str]:
"""Move a bundled skill's stale copy to its new canonical path after an upstream
RENAME/RECATEGORIZATION (manifest key still matches, ``dest`` doesn't exist yet;
otherwise the skill is misread as user-deleted and the old dir stranded forever).
RENAME/RECATEGORIZATION (else it is misread as user-deleted and stranded forever).
Only a copy byte-identical to the origin hash — proof *we* placed it — is moved;
user-edited or hub-installed copies are left. Returns the rel source path on move.
"""
user-edited or hub-installed copies stay. Returns the rel source path on move."""
origin_hash = st.manifest.get(skill_name, "")
if not origin_hash:
return None
@@ -248,8 +208,7 @@ def _recover_renamed_skill(st: "_SyncState", skill_name: str, dest: Path) -> Opt
if rel in st.hub_paths: # the hub owns its install paths
continue
if _dir_hash(candidate) != origin_hash:
# Moving a customized copy would edit the user's work; leaving it
# avoids a duplicate-name collision. Warn so they migrate deliberately.
# Moving a customized copy would edit the user's work; warn so they migrate deliberately.
st.say(
f" ⚠ {skill_name}: upstream moved this skill to {_rel_skills_posix(dest)}, but your "
f"modified copy at {rel} was kept — it will not receive updates. "
@@ -279,8 +238,7 @@ class _SyncState:
suppressed: List[str] = field(default_factory=list)
relocated: List[str] = field(default_factory=list)
shadowed_by_external: List[str] = field(default_factory=list)
# Rename-recovery indexes are expensive on host bind mounts: built lazily,
# only when a tracked skill is actually missing from its canonical path.
# Rename-recovery indexes are expensive on bind mounts: built lazily, only when needed.
active_index: Optional[Dict[str, List[Path]]] = None
hub_paths: Set[str] = field(default_factory=set)
@@ -304,8 +262,8 @@ def _recover_orphan_backup(dest: Path) -> None:
def _defer_to_external(st: _SyncState, skill_name: str, dest: Path, bundled_hash: str) -> None:
"""An external_dirs source provides this skill; a local copy would be a name collision
the loader refuses to resolve. Defer for ALL manifest states and self-heal a stale local
shadow from an earlier sync — only when byte-identical (a user's own skill differs)."""
the loader refuses. Defer for ALL manifest states and remove a stale local shadow from
an earlier sync — only when byte-identical (a user's own skill differs)."""
st.shadowed_by_external.append(skill_name)
st.skipped += 1
st.say(f" ⇢ {skill_name} (deferred to external_dirs, not written to local tree)")
@@ -320,8 +278,8 @@ def _install_new_skill(st: _SyncState, skill_name: str, skill_src: Path, dest: P
try:
if dest.exists():
# Never overwrite a same-named user skill. Baseline the manifest only when
# byte-identical to bundled: recording bundled_hash for a differing copy
# would read as "user-modified" forever and block every bundled update.
# byte-identical: recording bundled_hash for a differing copy would read as
# "user-modified" forever and block every bundled update.
st.skipped += 1
if _dir_hash(dest) == bundled_hash:
st.manifest[skill_name] = bundled_hash
@@ -336,15 +294,14 @@ def _install_new_skill(st: _SyncState, skill_name: str, skill_src: Path, dest: P
st.manifest[skill_name] = bundled_hash
st.say(f" + {skill_name}")
except (OSError, IOError) as e:
st.say(f" ! Failed to copy {skill_name}: {e}")
# Not added to manifest — next sync retries.
st.say(f" ! Failed to copy {skill_name}: {e}") # not in manifest — next sync retries
def _replace_skill_dir(skill_src: Path, dest: Path) -> None:
"""Replace ``dest`` with a fresh copy of ``skill_src`` via a ``.bak`` sibling,
restoring the original on failure."""
backup = dest.with_suffix(".bak")
if backup.exists(): # a stale .bak would make shutil.move() nest dest INSIDE it; dest is authoritative
if backup.exists(): # a stale .bak would make shutil.move() nest dest INSIDE it
_rmtree_writable(backup)
shutil.move(str(dest), str(backup))
try:
@@ -373,8 +330,7 @@ def _update_existing_skill(st: _SyncState, skill_name: str, skill_src: Path, des
return
user_hash = _dir_hash(dest)
if not origin_hash:
# v1 migration: baseline from the user's copy so future syncs can detect edits.
# Can't tell user-edit from upstream change — be safe and skip.
# v1 migration: baseline from the user's copy (can't tell user-edit from upstream change).
st.manifest[skill_name] = user_hash
st.skipped += 1
return
@@ -408,11 +364,9 @@ def _seed_category_descriptions(bundled_dir: Path, only_dirs: Optional[Set[Path]
def sync_skills(quiet: bool = False) -> dict:
"""Sync bundled skills into ~/.hermes/skills/ using the manifest. Returns a dict
with keys copied, updated, skipped, user_modified, cleaned, suppressed, relocated,
total_bundled, optional_provenance_backfilled, shadowed_by_external, skipped_opt_out."""
# Opted-out profiles seed ONLY ESSENTIAL_SKILLS: the system prompt always points at
# ``hermes-agent``, so even a Blank Slate profile keeps it.
"""Sync bundled skills into ~/.hermes/skills/ using the manifest; returns the
per-category result dict (see the final return). Opted-out profiles seed ONLY
ESSENTIAL_SKILLS: the system prompt always points at ``hermes-agent``."""
essential_only = (_hermes_home() / NO_BUNDLED_SKILLS_MARKER).exists()
if essential_only and not quiet:
print(" (profile opted out of bundled skills via .no-bundled-skills — seeding essential skills only)")
@@ -453,7 +407,7 @@ def sync_skills(quiet: bool = False) -> dict:
st.skipped += 1 # in manifest but not on disk — user deleted it
# Clean manifest entries for skills removed upstream. Skipped when opted out: bundled_skills
# is only the essential set there, so cleaning would drop tracking for every other skill.
# is only the essential set there, so cleaning would drop tracking for everything else.
cleaned = [] if essential_only else sorted(set(st.manifest) - {name for name, _ in bundled_skills})
for name in cleaned:
del st.manifest[name]
@@ -475,14 +429,10 @@ def sync_skills(quiet: bool = False) -> dict:
def _rmtree_writable(path: Path) -> None:
"""Remove a directory tree, making read-only entries writable first (Nix/deb/rpm
sources keep r-x dirs; unlinking a child needs a writable parent, so chmod both).
Scope guard: refuses anything not a STRICT child of the active profile's skills
root, so a bad path join / missing HERMES_HOME / malicious manifest entry raises
a loud ValueError instead of wiping ``~/.hermes``. Callers always pass a skill dir
or its ``.bak`` sibling; the skills root itself must never be removed.
"""
"""rmtree that first makes read-only entries writable (Nix/deb/rpm keep r-x dirs;
unlinking a child needs a writable parent, so chmod both). Scope guard: refuses
anything not a STRICT child of the active skills root, so a bad join / missing
HERMES_HOME / malicious manifest entry raises instead of wiping ``~/.hermes``."""
target = Path(path).resolve()
skills_root = _skills_dir().resolve()
if skills_root not in target.parents:
tools/skills_sync_bundled_ops.py
+19 -34
View File
@@ -1,24 +1,19 @@
"""Bundled-skill maintenance ops: reset, diff, list-modified, opt-out, remove-pristine.
Extracted from ``tools.skills_sync``. Profile-scoped paths and patchable helpers
(``_get_bundled_dir``, ``sync_skills``, ...) are resolved through ``_ss()`` at
call time so monkeypatching ``tools.skills_sync`` keeps working.
Profile-scoped paths and patchable helpers (``_get_bundled_dir``, ``sync_skills``, ...)
are resolved through ``_ss()`` at call time so monkeypatching ``tools.skills_sync`` works.
"""
from pathlib import Path
from typing import List, Optional, Tuple
def _ss():
from tools import skills_sync
return skills_sync
from tools.skills_sync_optional import _skill_file_list, _ss
def _is_tracked_user_modification(origin_hash: str, user_hash: str) -> bool:
"""Whether an on-disk skill is a user modification ``hermes update`` keeps. Shared by
the sync loop and ``list_user_modified_bundled_skills`` so they never drift: needs a
recorded origin hash (un-baselined v1 entries don't count) AND differing content."""
"""User modification ``hermes update`` keeps: a recorded origin hash (un-baselined v1
entries don't count) AND differing content. Shared by the sync loop and
``list_user_modified_bundled_skills`` so they never drift."""
return bool(origin_hash) and user_hash != origin_hash
@@ -29,12 +24,10 @@ def _bundled_by_name(bundled_dir: Path) -> dict:
def reset_bundled_skill(name: str, restore: bool = False) -> dict:
"""Reset a bundled skill's manifest tracking so future syncs work normally.
An edited bundled skill stays ``user_modified`` forever — even after copying the
bundled version back — because the manifest holds the OLD origin hash; clearing
the entry breaks that loop. ``restore`` also deletes the user's copy so the next
sync re-copies bundled. Returns ``{ok, action, message, synced}``; action is
manifest_cleared / restored / not_in_manifest / bundled_missing / not_reset.
"""
An edited bundled skill stays ``user_modified`` forever because the manifest holds
the OLD origin hash; clearing the entry breaks that loop. ``restore`` also deletes
the user's copy so the next sync re-copies bundled. Returns ``{ok, action, message,
synced}``; action is manifest_cleared / restored / not_in_manifest / bundled_missing / not_reset."""
ss = _ss()
manifest = ss._read_manifest()
bundled_dir = ss._get_bundled_dir()
@@ -49,8 +42,8 @@ def reset_bundled_skill(name: str, restore: bool = False) -> dict:
return _fail("not_in_manifest", f"'{name}' is not a tracked bundled skill. Nothing to reset. "
f"(Hub-installed skills use `hermes skills uninstall`.)")
# Delete the user's copy BEFORE touching the manifest so a failed rmtree
# cannot leave the skill in a manifest-less limbo state.
# Delete the user's copy BEFORE touching the manifest so a failed rmtree can't
# leave the skill in a manifest-less limbo.
deleted_user_copy = False
if restore:
if not is_bundled:
@@ -115,8 +108,6 @@ def diff_bundled_skill(name: str) -> dict:
modified / added (only in user copy) / removed (only in bundled) / binary."""
import difflib
from tools.skills_sync_optional import _skill_file_list
ss = _ss()
def _fail(found: bool, message: str) -> dict:
@@ -165,9 +156,8 @@ _OPT_OUT_MESSAGES = { # (enabled, changed) -> message
def set_bundled_skills_opt_out(enabled: bool) -> dict:
"""Toggle the .no-bundled-skills marker: the on-disk half of ``hermes skills
opt-out`` / ``opt-in`` that stops installer/update/sync seeding. Removing
already-present skills is a separate step (``remove_pristine_bundled_skills``).
"""Toggle the .no-bundled-skills marker (the on-disk half of ``hermes skills
opt-out`` / ``opt-in``); removing present skills is ``remove_pristine_bundled_skills``.
Returns ``{ok, changed, marker, message}``."""
ss = _ss()
marker = ss._hermes_home() / ss.NO_BUNDLED_SKILLS_MARKER
@@ -189,14 +179,10 @@ def set_bundled_skills_opt_out(enabled: bool) -> dict:
def remove_pristine_bundled_skills(dry_run: bool = False) -> dict:
"""Delete bundled skills that are present, manifest-tracked, AND unmodified.
Removed ONLY when in the sync manifest (genuinely bundled, not hub/hand-written),
still in the bundled source (hash-comparable), and byte-identical to the origin
hash; everything else lands in ``skipped``. Removed skills lose their manifest
entry so a later opt-in re-seed treats them as new.
Returns ``{ok, removed, skipped: [{name, reason}], dry_run, message}``.
"""
"""Delete bundled skills that are manifest-tracked (genuinely bundled), still in the
bundled source (hash-comparable) AND byte-identical to the origin hash; everything
else lands in ``skipped``. Removed skills lose their manifest entry so a later
opt-in re-seed treats them as new. Returns ``{ok, removed, skipped: [{name, reason}], dry_run, message}``."""
ss = _ss()
manifest = ss._read_manifest()
bundled_dir = ss._get_bundled_dir()
@@ -211,8 +197,7 @@ def remove_pristine_bundled_skills(dry_run: bool = False) -> dict:
continue
dest = ss._compute_relative_dest(src, bundled_dir)
if not dest.exists():
# Already gone from disk; just forget the stale manifest entry.
if not dry_run:
if not dry_run: # already gone from disk; forget the stale manifest entry
manifest.pop(name, None)
continue
if ss._dir_hash(dest) != origin_hash:
tools/skills_sync_optional.py
+22 -24
View File
@@ -1,8 +1,7 @@
"""Official optional-skill provenance: hub-lock backfill and restore.
Extracted from ``tools.skills_sync``. Profile-scoped paths and patchable
helpers are resolved through ``_ss()`` at call time so tests and multi-profile
runtimes that patch ``tools.skills_sync`` globals keep working.
Profile-scoped paths and patchable helpers are resolved through ``_ss()`` at call
time so tests and multi-profile runtimes patching ``tools.skills_sync`` keep working.
"""
import json
@@ -66,24 +65,26 @@ def _hub_lock_entries(data: Optional[dict]) -> List[dict]:
def _read_hub_install_paths() -> Set[str]:
"""Install paths recorded in the hub lock, as POSIX strings. Hub-installed skills
are owned by the hub, never by bundled sync: rename recovery must not move them even
when content matches a bundled origin hash, or the lock's ``install_path`` dangles."""
"""Hub-lock install paths as POSIX strings. Hub-installed skills are owned by the
hub: rename recovery must not move them even when content matches a bundled
origin hash, or the lock's ``install_path`` dangles."""
return {str(e["install_path"]).strip("/") for e in _hub_lock_entries(_load_hub_lock()) if e.get("install_path")}
def _write_hub_lock(lock_path: Path, data: dict) -> None:
"""Atomic write so a crash mid-write can't wipe all provenance (the
JSONDecodeError fallback in the reader resets ``installed`` to empty)."""
"""Atomic: a crash mid-write must not wipe all provenance (the reader's
JSONDecodeError fallback resets ``installed`` to empty)."""
atomic_write_text(lock_path, json.dumps(data, indent=2, ensure_ascii=False) + "\n", tmp_prefix=".lock_")
def _iter_optional_skills(optional_dir: Path, *, root_relative: bool) -> Iterator[Tuple[Path, Path, str]]:
"""Yield ``(skill_md, src, install_path)`` for every safe official optional skill."""
for skill_md in sorted(optional_dir.rglob("SKILL.md")):
if root_relative and is_excluded_skill_path(skill_md.relative_to(optional_dir), root=optional_dir):
continue
if not root_relative and is_excluded_skill_path(skill_md):
if root_relative:
excluded = is_excluded_skill_path(skill_md.relative_to(optional_dir), root=optional_dir)
else:
excluded = is_excluded_skill_path(skill_md)
if excluded:
continue
try:
yield skill_md, skill_md.parent, _safe_rel_install_path(skill_md.parent, optional_dir)
@@ -92,9 +93,8 @@ def _iter_optional_skills(optional_dir: Path, *, root_relative: bool) -> Iterato
def _optional_skill_index() -> Dict[str, Tuple[str, str, Path]]:
"""Official optional skills keyed by BOTH folder name and frontmatter name, so callers
may pass either the hub-lock slug or the user-facing name. Values are
``(folder_name, install_path, source_dir)``."""
"""Official optional skills keyed by BOTH folder name and frontmatter name (hub-lock
slug or user-facing name). Values are ``(folder_name, install_path, source_dir)``."""
ss = _ss()
optional_dir = ss._get_optional_dir()
index: Dict[str, Tuple[str, str, Path]] = {}
@@ -133,9 +133,8 @@ def _find_active_copies(folder_name: str, src_frontmatter: str, dest: Path) -> L
def restore_official_optional_skill(name: str, *, restore: bool = False) -> dict:
"""Restore one or all official optional skills from repo source. ``restore=False``
only performs exact-match provenance backfill; ``restore=True`` repairs mutated /
reorganized skills by backing up matching active copies and copying the official
source into its canonical path."""
only backfills exact-match provenance; ``restore=True`` also backs up matching
active copies and copies the official source into its canonical path."""
ss = _ss()
def _fail(message: str) -> dict:
@@ -191,10 +190,9 @@ def _index_installed_skill_dirs_by_name() -> Dict[str, List[Path]]:
def _relocated_dest(src_name: str, index: Dict[str, List[Path]]) -> Optional[Tuple[Path, str]]:
"""The active tree may hold a skill under a DIFFERENT category path than the
repo (upstream reorganizes; the installed copy keeps its old location). Fall
back to a UNIQUE same-directory-name match — an ambiguous name gives no basis
to pick one. Returns ``(dest, install_path)`` or None."""
"""The active tree may hold a skill under a DIFFERENT category path than the repo
(upstream reorganized; the installed copy kept its location). Fall back to a UNIQUE
same-directory-name match; ambiguity gives no basis to pick. ``(dest, install_path)`` or None."""
candidates = index.get(src_name, [])
if len(candidates) != 1:
return None
@@ -207,9 +205,9 @@ def _relocated_dest(src_name: str, index: Dict[str, List[Path]]) -> Optional[Tup
def _backfill_optional_provenance(quiet: bool = False) -> List[str]:
"""Mark already-present official optional skills as hub-installed: skills that used
to be bundled (or were hand-copied) and now live under optional-skills/ get official
provenance when byte-identical to the source. Modified/local skills are left alone."""
"""Mark already-present official optional skills as hub-installed: formerly bundled
(or hand-copied) skills now under optional-skills/ get official provenance when
byte-identical to the source. Modified/local skills are left alone."""
ss = _ss()
optional_dir = ss._get_optional_dir()
if not optional_dir.exists():