Skip to content

fix(curator): protect skills.external_dirs from autonomous curation (#47688) - #52849

Merged
teknium1 merged 2 commits into
mainfrom
hermes/hermes-ee17e635
Jun 26, 2026
Merged

teknium1 merged 2 commits into
mainfrom
hermes/hermes-ee17e635

Conversation

@teknium1

Copy link
Copy Markdown
Collaborator

Summary

The skill curator no longer touches skills.external_dirs skills — the user's
shared, often version-controlled skill library is now read-only to autonomous
curation, while foreground user-directed edits still work.

Salvages #51875 (@yu-xin-c) and closes the gap that made its headline guard
inert, plus credits @x7peeps (#47752) who reported the same fix first.

Root cause

External-dir skills were visible to the curator's LLM review fork, which could
archive/skill_manage/raw-terminal them in place. The automatic sweep was
already safe (rooted at the local skills dir), but the LLM agent operated
through general-purpose tools with no external-ownership boundary.

#51875 added an is_external_skill_path boundary and a background-review write
guard in skill_manage — but that guard only fires when
is_background_review() is true, and the curator fork ran under the default
assistant_tool origin
, so the guard never triggered during the curation
pass it was built to stop. This PR fixes that keystone.

Changes

  • agent/skill_utils.py: is_external_skill_path() — central ownership boundary.
  • tools/skill_usage.py: external skills are ineligible for curation; excluded
    from list_agent_created_skill_names / agent_created_report (so the LLM is
    never shown them); archive_skill refuses them.
  • tools/skill_manager_tool.py: _background_review_write_guard refuses
    autonomous patch/edit/delete/write_file/remove_file on external (and
    bundled / hub-installed) skills; foreground user edits still pass.
  • agent/curator.py: set _memory_write_origin="background_review" on the
    review fork so turn_context binds the origin and the guard actually fires;
    prompt rule marks external dirs read-only.
  • Tests: external-path predicate, eligibility/archive refusal, candidate-list
    exclusion, background-review skill_manage refusal, and a regression test
    asserting the fork runs under the background_review origin.

Validation

Scenario Result
Curator (background_review) patch/delete of external skill refused, file untouched
Foreground (assistant_tool) patch of same external skill succeeds (design nuance preserved)
External skill in curator candidate list (even with stale created_by: agent) excluded
is_curation_eligible(external) / archive_skill(external) False / refused

scripts/run_tests.sh tests/agent/test_skill_utils.py tests/tools/test_skill_usage.py tests/tools/test_skill_manager_tool.py tests/agent/test_curator.py tests/agent/test_turn_context.py → 236 passed, 0 failed. Plus an isolated E2E run against a temp HERMES_HOME confirming the table above.

Known residual

The raw-terminal mv/rm path is deterred by the prompt rule (the LLM is
never handed an external path in the candidate list), not hard-blocked — shell
commands can't be intercepted per-path. The enforceable layers (candidate-list
exclusion, eligibility, archive_skill, skill_manage guard) are all hard.

Closes #47688. Supersedes #51875 and #47752.

Infographic

skill-curator-external-dirs-protected

yu-xin-c and others added 2 commits June 25, 2026 21:23
…ration

The salvaged #51875 added a background-review write guard in skill_manage
that refuses mutations to skills.external_dirs skills — but it only fires
when is_background_review() is true. The curator's LLM review fork ran with
the default _memory_write_origin='assistant_tool', so the guard never
triggered during the exact curation pass it exists to protect against
(GH-47688).

- Set _memory_write_origin='background_review' on the curator review fork so
  turn_context binds it onto the write-origin ContextVar and the guard fires.
- Add a regression test asserting the fork runs under the background_review
  origin (the invariant linking the fork to the guard).
- AUTHOR_MAP: map yu-xin-c for the salvaged commit.
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: hermes/hermes-ee17e635 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11374 on HEAD, 11371 on base (🆕 +3)

🆕 New issues (3):

Rule Count
unresolved-attribute 3
First entries
agent/curator.py:1856: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_memory_write_origin` on type `AIAgent`
run_agent.py:2989: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`

✅ Fixed issues (1):

Rule Count
invalid-assignment 1
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`

Unchanged: 5996 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/skills Skills system (list, view, manage) P1 High — major feature broken, no workaround labels Jun 26, 2026
@teknium1
teknium1 merged commit 4d04c65 into main Jun 26, 2026
27 checks passed
@teknium1
teknium1 deleted the hermes/hermes-ee17e635 branch June 26, 2026 05:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P1 High — major feature broken, no workaround tool/skills Skills system (list, view, manage) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Curator's LLM consolidation agent mutates/deletes skills in external_dirs; per-profile pins can't protect shared skills

3 participants