fix(skills): copy-on-write for agent edits of external skills

[cyb0rgk1tty]

What does this PR do?

The docs at External Skill Directories state:

External dirs are only scanned for skill discovery. When the agent creates or edits a skill, it always writes to ~/.hermes/skills/.

But the code didn't match. _edit_skill, _patch_skill, _write_file, and _remove_file in tools/skill_manager_tool.py all use _find_skill to locate a skill (which walks both the profile-local skills dir and every skills.external_dirs entry), then write directly to the discovered path. So an agent calling skill_edit on a skill that lives in an external directory silently mutates the shared source file, affecting every profile that references it.

This is particularly dangerous once multiple profiles point their external_dirs at a single shared skill directory (a common pattern for teams or multi-profile hosts): a single edit in profile A leaks to profiles B, C, D... with no indication to the agent or the user.

This PR makes the mutation helpers copy-on-write for external skills, matching the documented behavior.

Related Issue

Fixes #

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

tools/skill_manager_tool.py

• New helper _is_external_skill(skill_path) — returns True if the skill lives outside the profile-local SKILLS_DIR.
• New helper _copy_on_write_to_local(skill_path) — copies an external skill directory (with all supporting files) into SKILLS_DIR, preserving the skill's directory name. Raises RuntimeError if a local copy already exists (a guard that shouldn't fire in practice because _find_skill returns the local copy first).
• _edit_skill: detects external skills, copies them to SKILLS_DIR first, then writes the new content to the copy. Scan-block rollback removes the whole copy (nothing was mutated in place). The response gains a note field explaining that a profile-local override was created.
• _patch_skill: same pattern. Reads the original content from the external source before the copy, then validates the match, size, and frontmatter on the result. Any validation failure cleans up the copy so no half-applied state lingers.
• _write_file: copies the skill before writing the new supporting file; scan-block rollback removes the whole copy.
• _remove_file: copies the skill before removing the file; the original external skill retains the file.
• _delete_skill: refuses to delete external skills — returns a structured error pointing the agent at skills.disabled (to hide from the current profile) or direct filesystem removal (to unshare entirely). Deleting a local skill is unchanged.
• A shared _COPY_ON_WRITE_NOTE_TEMPLATE produces a consistent agent-facing message so the user understands why the change didn't propagate to other profiles using the shared copy.

tests/tools/test_skill_manager_tool.py

• 12 new tests covering:
  • _is_external_skill for local and sibling dirs.
  • _copy_on_write_to_local happy path and the "local already exists" guard.
  • Edit, patch, write_file, remove_file: external skill triggers copy-on-write; local copy has the change; external source is untouched; response contains the note.
  • Edit: local skills still write in place, no note.
  • Patch: copy is rolled back when the match fails (no lingering local copy).
  • Delete: external rejected with skills.disabled hint; local delete still works.
• All new tests set HERMES_HOME in addition to patching SKILLS_DIR, because _find_skill resolves external dirs via get_hermes_home(). The pre-existing tests in this file only patch SKILLS_DIR and therefore already fail on main (they fall through to the real user's ~/.hermes/). I did not touch those — happy to fix them in a separate PR if the maintainers want.

website/docs/user-guide/features/skills.md

• Updated the "Read-only" bullet in the External Skill Directories section to describe the copy-on-write behavior and the _delete_skill rejection.

How to Test

# 1. Run the new tests
pytest tests/tools/test_skill_manager_tool.py::TestIsExternalSkill \
       tests/tools/test_skill_manager_tool.py::TestCopyOnWriteHelper \
       tests/tools/test_skill_manager_tool.py::TestEditExternalSkillCopyOnWrite \
       tests/tools/test_skill_manager_tool.py::TestPatchExternalSkillCopyOnWrite \
       tests/tools/test_skill_manager_tool.py::TestWriteFileExternalSkillCopyOnWrite \
       tests/tools/test_skill_manager_tool.py::TestRemoveFileExternalSkillCopyOnWrite \
       tests/tools/test_skill_manager_tool.py::TestDeleteExternalSkillRejected -v
# Expect: 12 passed

# 2. Manual end-to-end verification
#   a. Put a skill at ~/.agents/shared/my-skill/SKILL.md
#   b. Add "~/.agents/shared" to skills.external_dirs in ~/.hermes/config.yaml
#   c. Ask the agent to edit that skill's SKILL.md
#   d. Verify:
#      - ~/.hermes/skills/my-skill/SKILL.md now exists with the new content
#      - ~/.agents/shared/my-skill/SKILL.md is unchanged
#      - The tool response includes a "note" explaining the copy-on-write

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(skills): ...)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix
• I've run the new tests and they all pass
• I've added tests for my changes
• I've tested on my platform: Ubuntu 24.04 / Python 3.12

Documentation & Housekeeping

• I've updated relevant documentation (website/docs/user-guide/features/skills.md)
• N/A — no config key changes
• N/A — no architecture changes
• N/A — Python-only change, no platform-specific code paths
• N/A — tool behavior change is documented in the updated docstrings and user docs

[teknium1]

Thanks for the detailed write-up and the copy-on-write approach, @cyb0rgk1tty — this was a real bug and your analysis was correct at the time you opened the PR.

This is an automated hermes-sweeper review.

The underlying bug (external-skill mutations silently writing to shared source paths) was fixed on main four days after this PR was opened, in commit e683c9d (fix(security): enforce path boundary checks in skill manager operations, Apr 10 2026). That commit added _is_local_skill() and wired hard-error guards into all five mutation helpers — _edit_skill, _patch_skill, _delete_skill, _write_file, and _remove_file.

Evidence:

• tools/skill_manager_tool.py lines 109–116: _is_local_skill() helper
• Lines 397–398, 440–441, 522–523, 567–568, 604–605: guard checks in each mutation helper
• website/docs/user-guide/features/skills.md line 210: docs now match the hard-refuse behavior

The fix on main takes a different UX approach (hard-refuse with an explanatory error rather than transparent copy-on-write), but the documented contract is satisfied either way. Closing as implemented on main.