fix(skills): create reusing a dead skill's name gets archived by the curator (#65992)

[stantheman0128]

What does this PR do?

Reproduces and fixes #65992: skill_manage(action="create", name="test-skill", category="devops") returns success with skill_md pointing at skills/devops/test-skill/SKILL.md, but the file ends up at skills/.archive/test-skill/SKILL.md, and subsequent skill_view/patch calls fail with "not found".

Root cause: .usage.json records are keyed by skill name and survive any removal that did not go through skill_manage(delete) (a manual rm -rf, a crashed delete). When a create reuses such a name, the new skill inherits the dead record's inactivity clock. The curator's next apply_automatic_transitions() pass (kicked off on CLI startup in a daemon thread, and from gateway housekeeping) reads the expired anchor, decides the seconds-old skill is long inactive, and archive_skill() relocates the directory to skills/.archive/<name>/, flattening the category. That is exactly the reported filesystem shape. It also explains why checking archive_after_days=90 against the skill's age did not implicate the curator: the clock that matters is the record's, not the directory's. The reporter's code analysis was correct that the create path never references .archive; the relocation happens in the pass that follows.

Fix: a successful create now starts a new life for the name by discarding any leftover usage record (skill_usage.forget), the same reset a hard delete already performs. The collision check in _create_skill guarantees no live skill owns the record at that point. Placing the reset inside _create_skill covers both callers, the skill_manage tool and the dashboard's create endpoint.

One limitation, stated for review: if a curator pass is already mid-walk when the create lands, the pass still holds a snapshot row with the stale clock, so an archive in that narrow window remains possible. Closing it would require the prune to revalidate the live record under the usage lock right before the destructive move. I kept this PR to the deterministic root cause and can follow up if that hardening is wanted.

Related: #24068 adds a manual hermes curator repair-usage command that reconciles orphan records. Complementary, but it does not prevent this bug, since nothing runs it automatically between the create and the curator pass.

Related Issue

Fixes #65992

Type of Change

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

Changes Made

• tools/skill_manager_tool.py: _create_skill() forgets any leftover usage record after a successful write and security scan (best-effort, same error posture as the existing telemetry block).
• tools/skill_usage.py: forget() docstring now documents the second caller.
• tests/tools/test_skill_manager_tool.py: unit tests. A create discards a stale record from a previous life; a failed duplicate create leaves the live skill's record untouched.
• tests/agent/test_curator_activity.py: end-to-end regression against a temp HERMES_HOME with real imports. Seeds a stale agent-created record, creates a fresh skill under a category via skill_manage, runs apply_automatic_transitions, and asserts the skill stays at its reported path instead of moving to skills/.archive/<name>/.

How to Test

1. Run the regression tests: scripts/run_tests.sh tests/agent/test_curator_activity.py tests/tools/test_skill_manager_tool.py
2. To see the bug, run tests/agent/test_curator_activity.py::test_fresh_create_reusing_dead_skill_name_is_not_archived on a checkout without the fix: the pass archives the seconds-old skill (counts["archived"] == 1) and the file moves to skills/.archive/fresh-skill/ while the create response still points at skills/devops/fresh-skill/SKILL.md.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate (closest is fix(curator): repair orphan usage records #24068, discussed above)
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass (I ran the four affected suites per file instead: test_skill_manager_tool.py, test_curator_activity.py, test_skill_usage.py, test_curator.py; 224 passed. One pre-existing failure on my machine, test_symlinked_skill_dir_refused, fails identically on main because unelevated Windows cannot create symlinks, WinError 1314.)
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: Windows 11, Python 3.13

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — updated the forget() docstring
• I've updated cli-config.yaml.example if I added/changed config keys — N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — scripts/check-windows-footguns.py is clean on all touched files
• I've updated tool descriptions/schemas if I changed tool behavior — N/A (no schema change)

Screenshots / Logs

Standalone repro on current main (temp HERMES_HOME, stale agent-created record for test-skill with last_used_at 200 days old, then create + one automatic-transition pass):

CREATE: True ...\skills\devops\test-skill\SKILL.md
PRUNE COUNTS: {'marked_stale': 0, 'archived': 1, 'reactivated': 0, 'checked': 1, 'seeded': 0}
skills\.archive\test-skill\SKILL.md
at reported path: False
at .archive path: True

Same script with this fix applied:

CREATE: True ...\skills\devops\test-skill\SKILL.md
PRUNE COUNTS: {'marked_stale': 0, 'archived': 0, 'reactivated': 0, 'checked': 0, 'seeded': 0}
skills\devops\test-skill\SKILL.md
at reported path: True
at .archive path: False

New regression test, red before the fix, green after:

>       assert counts["archived"] == 0
E       assert 1 == 0
tests\agent\test_curator_activity.py:105: AssertionError

tests/agent/test_curator_activity.py .. [100%]  2 passed
tests/tools/test_skill_manager_tool.py  105 passed, 3 skipped
tests/tools/test_skill_usage.py  47 passed
tests/agent/test_curator.py  70 passed

Transparency: I diagnosed and developed this fix with an AI coding assistant (Claude) under my direction and reviewed the diagnosis, diff, and test evidence myself.

[tonydwb]

Code Review Summary

Verdict: Comment

Looks Good

• Fixes real bug: skill_manage(create) was incorrectly archiving newly created skills that reused the name of a previously removed skill
• Root cause well-explained: stale usage record left by removed skill had an expired inactivity clock, causing the automatic-transition pass to archive the new skill immediately
• Fix properly discards the stale usage record on successful create
• New test covers this exact scenario

Note

• 123 additions, 1 file changed — well-scoped fix
• No security concerns

---

Reviewed by Hermes Agent

[teknium1]

Thanks for isolating the stale-record cause and covering the ordinary sequential create → transition path.

Problems

• tools/skill_manager_tool.py:846 removes the stale record, but an already-running pass can have obtained its stale row at agent/curator.py:328. That pass later calls archive_skill(name) at agent/curator.py:371-374; archive_skill() resolves and renames the current directory without checking the usage record again (tools/skill_usage.py:704-740). The fresh skill can therefore still be moved to .archive in the interleaving described in the PR body.

Suggested changes

• Revalidate the live usage record under coordinated lifecycle locking immediately before archival, and add a barrier-based regression for the snapshot/create/archive ordering.

This is an automated hermes-sweeper review.

[teknium1]

forget() fixes the sequential path, but it cannot invalidate a stale row already returned by agent_created_report(). That pass can still reach archive_skill(name), which resolves and renames the newly created directory without checking the record again. Please coordinate the create reset with archive-time revalidation/locking and cover that interleaving.

[stantheman0128]

Thanks for catching the in-flight race. Tightened that path in f2f76a328.

What changed:

• archive_skill() now takes optional inactivity cutoffs. When the automatic transition pass calls it, it re-reads the live usage record under _usage_file_lock right before the rename and bails if the clock no longer warrants archival (forgotten record, fresh create, etc.).
• skill_manage(create) clears any leftover usage record before mkdir, so the new directory is never visible while the stale clock is still on disk.
• Added a barrier test that forces snapshot -> create/forget -> archive ordering.

Evidence (Windows 11, Python 3.13 venv):

tests/agent/test_curator_activity.py  3 passed
tests/agent/test_curator.py  70 passed
tests/tools/test_skill_usage.py  47 passed
tests/tools/test_skill_manager_tool.py  105 passed, 3 skipped, 1 failed

The one failure is pre-existing: test_symlinked_skill_dir_refused hits WinError 1314 (symlink privilege) on unelevated Windows, same as on main.

scripts/check-windows-footguns.py clean on the touched files.

[stantheman0128]

Closing as author to bring our open PRs on hermes-agent back within a healthy throttle (we had 10 open with only sweeper keep_open and no concrete maintainer change requests for days).

Keeping three Windows-focused PRs open for now:

• fix(desktop): avoid Explorer DnD image preview freeze on Windows #66546 (Explorer DnD image preview freeze on Windows)
• fix(tools): translate MSYS paths to Windows-native before shell lint (#66494) #66524 (MSYS path translation before shell lint)
• fix(browser): keep local Chromium headless on Windows Desktop (#64867) #66523 (local Chromium headless on Windows Desktop)

Happy to reopen this one if a maintainer wants it prioritized. Thanks for the patience.