Skip to content

fix(skills): background review fork respects pinned skills - #53226

Merged
teknium1 merged 1 commit into
mainfrom
fix/review-fork-respect-pin-25839
Jun 26, 2026
Merged

fix(skills): background review fork respects pinned skills#53226
teknium1 merged 1 commit into
mainfrom
fix/review-fork-respect-pin-25839

Conversation

@teknium1

Copy link
Copy Markdown
Contributor

Summary

The background self-improvement review fork now respects pinned skills, matching the curator. Previously a user-pinned skill could still be edited by the autonomous review fork — the guard only blocked external / bundled / hub-installed / protected-builtin skills, not pinned agent-owned ones.

Root cause: _background_review_write_guard in tools/skill_manager_tool.py checked ownership classes but never the pin flag. The curator skips pinned skills from every auto-transition; the review fork is the same kind of no-user-present actor and should too. This was the second concrete ask in #25839 (the first — stop forging role: "user" for review prompts — was already addressed: the review prompt is the fork's own user turn, gated by a memory/skills-only tool whitelist plus write-origin provenance).

Changes

  • tools/skill_manager_tool.py: add a pin check to _background_review_write_guard, so background-origin edit/patch/delete/write_file/remove_file on a pinned skill are refused. Deliberately stricter than the foreground _pinned_guard (which only blocks delete) — with no user in the loop there is no one to consent to an edit.
  • tests/tools/test_skill_manager_tool.py: background-review patch on a pinned skill is refused; an unpinned skill is NOT over-blocked.

Validation

Origin Skill Action Result
background_review pinned patch/edit refused
background_review unpinned patch allowed
foreground pinned patch/edit allowed (unchanged)
foreground pinned delete refused (unchanged)

scripts/run_tests.sh tests/tools/test_skill_manager_tool.py tests/tools/test_skill_provenance.py → 103/103 passing.

Scope note: this addresses the pin-respect gap. The broader "gate all background-review skill writes behind approval" lever already exists as opt-in (skills.write_approval, default off) — left as-is since flipping that default is a separate product decision.

Reported by @xlionjuan (via their agent Kuri) in #25839.

Fixes #25839

Infographic

review-fork-pin

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes #25839
@github-actions

Copy link
Copy Markdown
Contributor

🔎 Lint report: fix/review-fork-respect-pin-25839 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: 11491 on HEAD, 11493 on base (✅ -2)

🆕 New 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`

✅ Fixed issues (2):

Rule Count
unresolved-attribute 2
First entries
run_agent.py:3002: [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`

Unchanged: 6037 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) P3 Low — cosmetic, nice to have labels Jun 26, 2026
@teknium1
teknium1 merged commit 525e1e7 into main Jun 26, 2026
27 checks passed
@teknium1
teknium1 deleted the fix/review-fork-respect-pin-25839 branch June 26, 2026 19:49
pai-scaffolde pushed a commit to pai-scaffolde/hermes-agent that referenced this pull request Jun 28, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…rch#53226)

The autonomous self-improvement review fork could still write to a pinned
skill — only external/bundled/hub-installed/protected-builtin skills were
guarded. The curator skips pinned skills from every auto-transition; the
review fork is the same kind of no-user-present actor and must too.

Adds a pin check to _background_review_write_guard so background-origin
edit/patch/delete/write_file/remove_file on a pinned skill are refused.
Stricter than the foreground _pinned_guard (delete-only) by design: with
no user in the loop there is no one to consent to an edit.

Fixes NousResearch#25839
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 P3 Low — cosmetic, nice to have 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.

System impersonates user role, tricking parallel agent instances into modifying skills without consent

2 participants