feat(skills): refuse skill_manage writes on pinned skills (NousResearch#17562)

Extend curator's pin flag from 'skip auto-transitions' to 'no agent
edits at all'. All five skill_manage mutation actions (edit, patch,
delete, write_file, remove_file) now refuse pinned skills with a
message pointing the user at `hermes curator unpin <name>`.

Motivation: pin used to only stop the curator's own maintenance pass
from touching a skill. Nothing prevented the main agent from editing
or deleting a pinned skill via skill_manage in-session. This gives
users a hard fence against unwanted agent edits — same semantics as
curator pinning, extended to the write tool.

Create is unaffected (you can't pin a name that doesn't exist yet,
and name collisions already error out). Broken sidecars fail open
rather than lock the agent out.

The schema description advertises the new refusal so models know
not to route around it with rename/recreate tricks.

Author: teknium1

tests/tools/test_skill_manager_tool.py

@@ -714,3 +714,114 @@ def test_create_still_writes_to_local_root(self, tmp_path):
         assert (local / "fresh-skill" / "SKILL.md").exists()
         assert not (external / "fresh-skill").exists()
 
+
+
+# ---------------------------------------------------------------------------
+# Pinned-skill guard — skill_manage refuses all writes to pinned skills.
+# The user unpins via `hermes curator unpin <name>`.
+# ---------------------------------------------------------------------------
+
+class TestPinnedGuard:
+    """Every mutation action must refuse when the skill is pinned."""
+
+    @staticmethod
+    def _pin(name: str):
+        """Return a patch context that marks *name* as pinned in skill_usage."""
+        def _fake_get_record(skill_name, _name=name):
+            return {"pinned": True} if skill_name == _name else {"pinned": False}
+        return patch("tools.skill_usage.get_record", side_effect=_fake_get_record)
+
+    def test_edit_refuses_pinned(self, tmp_path):
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            with self._pin("my-skill"):
+                result = _edit_skill("my-skill", VALID_SKILL_CONTENT_2)
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        assert "hermes curator unpin my-skill" in result["error"]
+        # Original content preserved
+        content = (tmp_path / "my-skill" / "SKILL.md").read_text()
+        assert "A test skill" in content
+
+    def test_patch_refuses_pinned(self, tmp_path):
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            with self._pin("my-skill"):
+                result = _patch_skill("my-skill", "Do the thing.", "Do the new thing.")
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        assert "hermes curator unpin my-skill" in result["error"]
+        content = (tmp_path / "my-skill" / "SKILL.md").read_text()
+        assert "Do the thing." in content  # unchanged
+
+    def test_patch_supporting_file_refuses_pinned(self, tmp_path):
+        """Pin covers supporting files too, not just SKILL.md."""
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            _write_file("my-skill", "references/api.md", "original")
+            with self._pin("my-skill"):
+                result = _patch_skill(
+                    "my-skill", "original", "modified",
+                    file_path="references/api.md",
+                )
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        assert (tmp_path / "my-skill" / "references" / "api.md").read_text() == "original"
+
+    def test_delete_refuses_pinned(self, tmp_path):
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            with self._pin("my-skill"):
+                result = _delete_skill("my-skill")
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        # Skill still exists
+        assert (tmp_path / "my-skill" / "SKILL.md").exists()
+
+    def test_write_file_refuses_pinned(self, tmp_path):
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            with self._pin("my-skill"):
+                result = _write_file("my-skill", "references/api.md", "content")
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        assert not (tmp_path / "my-skill" / "references" / "api.md").exists()
+
+    def test_remove_file_refuses_pinned(self, tmp_path):
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            _write_file("my-skill", "references/api.md", "content")
+            with self._pin("my-skill"):
+                result = _remove_file("my-skill", "references/api.md")
+        assert result["success"] is False
+        assert "pinned" in result["error"].lower()
+        # File still there
+        assert (tmp_path / "my-skill" / "references" / "api.md").exists()
+
+    def test_unpinned_skills_still_editable(self, tmp_path):
+        """Sanity check: the guard doesn't fire for unpinned skills.
+
+        Only the specifically-pinned skill is refused; a sibling skill must
+        still be freely editable.
+        """
+        with _skill_dir(tmp_path):
+            _create_skill("pinned-one", VALID_SKILL_CONTENT)
+            _create_skill("free-one", VALID_SKILL_CONTENT)
+            with self._pin("pinned-one"):
+                blocked = _edit_skill("pinned-one", VALID_SKILL_CONTENT_2)
+                allowed = _edit_skill("free-one", VALID_SKILL_CONTENT_2)
+        assert blocked["success"] is False
+        assert allowed["success"] is True
+
+    def test_broken_sidecar_fails_open(self, tmp_path):
+        """If skill_usage.get_record raises, we allow the write through.
+
+        Rationale: a corrupted telemetry file shouldn't lock the agent out
+        of skills it would otherwise be allowed to touch.
+        """
+        with _skill_dir(tmp_path):
+            _create_skill("my-skill", VALID_SKILL_CONTENT)
+            with patch("tools.skill_usage.get_record",
+                       side_effect=RuntimeError("sidecar broken")):
+                result = _edit_skill("my-skill", VALID_SKILL_CONTENT_2)
+        assert result["success"] is True

tools/skill_manager_tool.py

@@ -130,6 +130,33 @@ def _containing_skills_root(skill_path: Path) -> Path:
     return SKILLS_DIR
 
 
+def _pinned_guard(name: str) -> Optional[str]:
+    """Return a refusal message if *name* is pinned, else None.
+
+    Pinned skills are off-limits to the agent's skill_manage tool.  The only
+    way to modify one is for the user to unpin it via
+    ``hermes curator unpin <name>`` (or edit it directly by hand).  This
+    mirrors the curator's own pinned-skip behavior but extends the guard
+    to tool-driven writes as well, giving users a hard fence against
+    accidental agent edits.
+
+    Best-effort: if the sidecar is unreadable we let the write through
+    rather than block on a broken telemetry file.
+    """
+    try:
+        from tools import skill_usage
+        rec = skill_usage.get_record(name)
+        if rec.get("pinned"):
+            return (
+                f"Skill '{name}' is pinned and cannot be modified by "
+                f"skill_manage. Ask the user to run "
+                f"`hermes curator unpin {name}` if they want the change."
+            )
+    except Exception:
+        logger.debug("pinned-guard lookup failed for %s", name, exc_info=True)
+    return None
+
+
 MAX_SKILL_CONTENT_CHARS = 100_000   # ~36k tokens at 2.75 chars/token
 MAX_SKILL_FILE_BYTES = 1_048_576    # 1 MiB per supporting file
 
@@ -408,6 +435,10 @@ def _edit_skill(name: str, content: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found. Use skills_list() to see available skills."}
 
+    pinned_err = _pinned_guard(name)
+    if pinned_err:
+        return {"success": False, "error": pinned_err}
+
     skill_md = existing["path"] / "SKILL.md"
     # Back up original content for rollback
     original_content = skill_md.read_text(encoding="utf-8") if skill_md.exists() else None
@@ -448,6 +479,10 @@ def _patch_skill(
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
+    pinned_err = _pinned_guard(name)
+    if pinned_err:
+        return {"success": False, "error": pinned_err}
+
     skill_dir = existing["path"]
 
     if file_path:
@@ -527,6 +562,10 @@ def _delete_skill(name: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
+    pinned_err = _pinned_guard(name)
+    if pinned_err:
+        return {"success": False, "error": pinned_err}
+
     skill_dir = existing["path"]
     skills_root = _containing_skills_root(skill_dir)
     shutil.rmtree(skill_dir)
@@ -570,6 +609,10 @@ def _write_file(name: str, file_path: str, file_content: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found. Create it first with action='create'."}
 
+    pinned_err = _pinned_guard(name)
+    if pinned_err:
+        return {"success": False, "error": pinned_err}
+
     target, err = _resolve_skill_target(existing["path"], file_path)
     if err:
         return {"success": False, "error": err}
@@ -604,6 +647,10 @@ def _remove_file(name: str, file_path: str) -> Dict[str, Any]:
     if not existing:
         return {"success": False, "error": f"Skill '{name}' not found."}
 
+    pinned_err = _pinned_guard(name)
+    if pinned_err:
+        return {"success": False, "error": pinned_err}
+
     skill_dir = existing["path"]
 
     target, err = _resolve_skill_target(skill_dir, file_path)
@@ -736,7 +783,10 @@ def skill_manage(
         "After difficult/iterative tasks, offer to save as a skill. "
         "Skip for simple one-offs. Confirm with user before creating/deleting.\n\n"
         "Good skills: trigger conditions, numbered steps with exact commands, "
-        "pitfalls section, verification steps. Use skill_view() to see format examples."
+        "pitfalls section, verification steps. Use skill_view() to see format examples.\n\n"
+        "Pinned skills are off-limits — all write actions refuse with a message "
+        "pointing the user to `hermes curator unpin <name>`. Don't try to route "
+        "around this by renaming or recreating."
     ),
     "parameters": {
         "type": "object",