diff --git a/hermes_cli/skills_hub.py b/hermes_cli/skills_hub.py index dd0aae4686828..dd4c4e0d79b3c 100644 --- a/hermes_cli/skills_hub.py +++ b/hermes_cli/skills_hub.py @@ -1052,9 +1052,21 @@ def do_check(name: Optional[str] = None, console: Optional[Console] = None) -> N c.print(f"[dim]{update_count} update(s) available across {len(results)} checked skill(s)[/]\n") -def do_update(name: Optional[str] = None, console: Optional[Console] = None) -> None: - """Update hub-installed skills with upstream changes.""" - from tools.skills_hub import HubLockFile, check_for_skill_updates +def do_update(name: Optional[str] = None, console: Optional[Console] = None, + force: bool = False) -> None: + """Update hub-installed skills with upstream changes. + + Skills whose on-disk content no longer matches the hash recorded at + install time have been edited locally; updating them would silently + destroy the user's work (``do_install(force=True)`` rmtree-replaces the + directory). Those are skipped by default and only overwritten when + ``force=True``. Mirrors the user-modified protection bundled skills + already get from ``hermes update`` (ported from + paperclipai/paperclip#10978's explicit-merge-mode rule: destructive + replacement must be an explicit caller choice, never a rerun default). + """ + from tools.skills_hub import SKILLS_DIR, HubLockFile, check_for_skill_updates + from tools.skills_guard import content_hash c = console or _console lock = HubLockFile() @@ -1063,9 +1075,25 @@ def do_update(name: Optional[str] = None, console: Optional[Console] = None) -> c.print("[dim]No updates available.[/]\n") return + skipped_local: list[str] = [] for entry in updates: installed = lock.get_installed(entry["name"]) category = _derive_category_from_install_path(installed.get("install_path", "")) if installed else "" + if installed and not force: + recorded_hash = installed.get("content_hash", "") + skill_path = SKILLS_DIR / installed.get("install_path", "") + if recorded_hash and skill_path.is_dir(): + try: + disk_hash = content_hash(skill_path) + except OSError: + disk_hash = recorded_hash + if disk_hash != recorded_hash: + skipped_local.append(entry["name"]) + c.print( + f"[yellow]Skipping:[/] {entry['name']} — you have local edits " + "(update would overwrite them)." + ) + continue c.print(f"[bold]Updating:[/] {entry['name']}") # Pin the update to the source registry recorded in the lockfile. # Without this, a bare (slash-less) identifier such as "reddit" falls @@ -1082,7 +1110,15 @@ def do_update(name: Optional[str] = None, console: Optional[Console] = None) -> source_id=entry.get("source", "") or None, ) - c.print(f"[bold green]Updated {len(updates)} skill(s).[/]\n") + updated_count = len(updates) - len(skipped_local) + if updated_count: + c.print(f"[bold green]Updated {updated_count} skill(s).[/]\n") + if skipped_local: + c.print( + f"[dim]{len(skipped_local)} skill(s) kept your local edits: " + f"{', '.join(sorted(skipped_local))}.[/]" + ) + c.print("[dim]Overwrite with: hermes skills update --force[/]\n") def do_audit(name: Optional[str] = None, console: Optional[Console] = None, @@ -1745,7 +1781,8 @@ def skills_command(args) -> None: elif action == "check": do_check(name=getattr(args, "name", None)) elif action == "update": - do_update(name=getattr(args, "name", None)) + do_update(name=getattr(args, "name", None), + force=getattr(args, "force", False)) elif action == "audit": do_audit(name=getattr(args, "name", None), deep=getattr(args, "deep", False)) @@ -1929,8 +1966,10 @@ def handle_skills_slash(cmd: str, console: Optional[Console] = None) -> None: do_check(name=name, console=c) elif action == "update": - name = args[0] if args else None - do_update(name=name, console=c) + force = "--force" in args + pos = [a for a in args if not a.startswith("--")] + name = pos[0] if pos else None + do_update(name=name, console=c, force=force) elif action == "audit": name = args[0] if args and not args[0].startswith("--") else None diff --git a/hermes_cli/subcommands/skills.py b/hermes_cli/subcommands/skills.py index 4eb68a01b1c7a..1f016b2f5b571 100644 --- a/hermes_cli/subcommands/skills.py +++ b/hermes_cli/subcommands/skills.py @@ -139,6 +139,11 @@ def build_skills_parser(subparsers, *, cmd_skills: Callable) -> None: nargs="?", help="Specific skill to update (default: all outdated skills)", ) + skills_update.add_argument( + "--force", + action="store_true", + help="Overwrite skills you have edited locally (they are skipped by default)", + ) skills_audit = skills_subparsers.add_parser( "audit", help="Re-scan installed hub skills" diff --git a/tests/hermes_cli/test_skills_hub.py b/tests/hermes_cli/test_skills_hub.py index 8e087e2758fb4..9507fb9c6b563 100644 --- a/tests/hermes_cli/test_skills_hub.py +++ b/tests/hermes_cli/test_skills_hub.py @@ -313,3 +313,84 @@ def test_do_search_json_flag_emits_full_identifiers(capsys): # Table render must be suppressed — sink should be empty (no "Searching for:" header). assert "Searching for:" not in sink.getvalue() + + +# --------------------------------------------------------------------------- +# Local-edit protection in do_update (ported from paperclipai/paperclip#10978) +# --------------------------------------------------------------------------- + + +def _update_env(monkeypatch, tmp_path, *, edit_after_install: bool): + """Install a fake hub skill on disk, optionally edit it, and wire mocks. + + Returns (console_sink, installs_list). + """ + import hermes_cli.skills_hub as cli_hub + import tools.skills_hub as hub + from tools.skills_guard import content_hash + + skills_dir = tmp_path / "skills" + skill_dir = skills_dir / "category" / "hub-skill" + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("# hub-skill\noriginal\n") + + recorded = content_hash(skill_dir) + if edit_after_install: + (skill_dir / "SKILL.md").write_text("# hub-skill\nuser edited\n") + + monkeypatch.setattr(hub, "SKILLS_DIR", skills_dir) + monkeypatch.setattr(hub, "check_for_skill_updates", lambda **_kwargs: [{ + "name": "hub-skill", + "identifier": "someone/hub-skill", + "source": "github", + "status": "update_available", + }]) + monkeypatch.setattr(hub, "HubLockFile", lambda: type("L", (), { + "get_installed": lambda self, name: { + "install_path": "category/hub-skill", + "content_hash": recorded, + } + })()) + + installs = [] + monkeypatch.setattr( + cli_hub, "do_install", + lambda identifier, category="", force=False, console=None, source_id=None: + installs.append(identifier), + ) + + sink = StringIO() + console = Console(file=sink, force_terminal=False, color_system=None) + return console, sink, installs + + +def test_do_update_skips_locally_edited_skill(monkeypatch, tmp_path): + """A hub skill whose on-disk hash drifted from the lockfile is skipped.""" + console, sink, installs = _update_env(monkeypatch, tmp_path, edit_after_install=True) + + do_update(console=console) + + assert installs == [] + out = sink.getvalue() + assert "local edits" in out + assert "--force" in out + + +def test_do_update_force_overwrites_local_edits(monkeypatch, tmp_path): + """--force restores the destructive replace for edited skills.""" + console, sink, installs = _update_env(monkeypatch, tmp_path, edit_after_install=True) + + do_update(console=console, force=True) + + assert installs == ["someone/hub-skill"] + assert "local edits" not in sink.getvalue() + + +def test_do_update_unmodified_skill_updates_normally(monkeypatch, tmp_path): + """No local drift -> the update proceeds without --force.""" + console, sink, installs = _update_env(monkeypatch, tmp_path, edit_after_install=False) + + do_update(console=console) + + assert installs == ["someone/hub-skill"] + assert "Updated 1 skill(s)" in sink.getvalue() diff --git a/website/docs/user-guide/features/skills.md b/website/docs/user-guide/features/skills.md index 40bf6d33dcdc0..129277dfe0155 100644 --- a/website/docs/user-guide/features/skills.md +++ b/website/docs/user-guide/features/skills.md @@ -754,10 +754,13 @@ The hub now tracks enough provenance to re-check upstream copies of installed sk hermes skills check # Report which installed hub skills changed upstream hermes skills update # Reinstall only the skills with updates available hermes skills update react # Update one specific installed hub skill +hermes skills update react --force # Overwrite a skill you've edited locally ``` This uses the stored source identifier plus the current upstream bundle content hash to detect drift. +Skills you have edited locally (the on-disk content no longer matches the hash recorded at install time) are **skipped** by `hermes skills update` so your changes are never silently overwritten. Pass `--force` to replace them with the upstream version anyway. + :::tip GitHub rate limits Skills hub operations use the GitHub API, which has a rate limit of 60 requests/hour for unauthenticated users. If you see rate-limit errors during install or search, set `GITHUB_TOKEN` in your `.env` file to increase the limit to 5,000 requests/hour. The error message includes an actionable hint when this happens. :::