fix(agent): steer background review to permitted tools

[henrynguyeninfo1]

Summary

• preserve the background review fork's parent tool schemas for prompt-cache byte parity
• make denied file-tool results name the permitted skills_list, skill_view, and skill_manage path
• cover the model-visible read_file denial through the real thread-local dispatch gate

Root cause

The review fork intentionally advertises the parent agent's full tool schema, but runtime dispatch only permits memory and skill tools. The denial result previously said only that memory/skill tools were allowed, so it did not redirect models from read_file or patch to the APIs that satisfy the skill manager's read-before-write guard.

Testing

• uv run --extra dev pytest tests/run_agent/test_background_review.py tests/run_agent/test_background_review_cost_controls.py tests/run_agent/test_background_review_cache_parity.py tests/run_agent/test_background_review_summary.py tests/run_agent/test_background_review_toolset_restriction.py tests/test_background_review_session_isolation.py tests/test_background_review_list_shapes.py tests/hermes_cli/test_plugins.py::TestThreadToolWhitelist -q
• uv run --extra dev ruff check agent/background_review.py tests/run_agent/test_background_review_toolset_restriction.py
• uv run --extra dev python -m py_compile agent/background_review.py tests/run_agent/test_background_review_toolset_restriction.py

Tested on macOS.

Alternative considered

Narrowing the fork's advertised schemas would eliminate invalid calls earlier, but it would break the intentional byte parity with the parent request and lose prompt-cache reuse. This change keeps parity and improves self-correction at the dispatch boundary.

Fixes #61521

[alt-glitch]

This was generated by AI during triage.

Duplicate of #61618: both update the background-review denial guidance to direct denied file-tool calls toward the permitted skill and memory tools. #61618 also updates the review prompt, making it the broader earlier version.

[tonydwb]

Code Review Summary

Verdict: Approved (read-only token — formal approval requires write access)

Improves the denial message for background review when a non-whitelisted tool is called. The new message guides users toward the correct tools (skill_view, skill_manage) instead of just saying "not allowed."

• Small, helpful UX improvement
• Good test coverage verifying the new message includes skill_view, skill_manage, and memory keywords

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused mitigation. The premise is confirmed on current main: agent/background_review.py:819-835 installs a memory/skills-only runtime whitelist while retaining parent schemas for cache parity, and its current denial text at agent/background_review.py:832-833 does not identify replacement tools. The thread-local dispatcher formats that message for blocked calls at hermes_cli/plugins.py:2137-2143; tools/skill_manager_tool.py:382-408 confirms that skill_view is the required read path before a review can mutate an existing skill.

Problems

• tests/run_agent/test_background_review_toolset_restriction.py adds assertions for skill_view, skill_manage, and memory, but not skills_list. The message explicitly advertises skills_list; that portion can regress without coverage.

Suggested changes

• Assert that skills_list is present in deny_msg.

A member has also identified open PR #61618 as the broader duplicate, including equivalent proactive-prompt guidance. This is an automated hermes-sweeper review.

[teknium1]

Please also assert "skills_list" in deny_msg. The changed production message advertises it as the discovery path, but the new regression test does not currently protect that wording.

[GottZ]

This was generated by AI during triage.

Summary

Twenty-four PRs address or reference this background-review complex: they cover cache-key parity (system, tools[], and reasoning fields), reasoning inheritance, read-only file authority, configurable toolsets, multi-user review suppression, and the advertised-schema/runtime-whitelist mismatch. The merged reference implementations are #25434, #29704, and #64379; for issue #61521, #65803 is the recorded best available mitigation while preserving remote cache parity.

Related pull requests

• fix(memory): restore prefix cache hits in background review fork (~26% token saving per run) #17276 [closed] fixes — (+142/-4) — n/a: Reuses the parent system prompt and replaces schema narrowing with a thread-local dispatch whitelist, directly addressing prompt/cache divergence; closed because its implementation was salvaged into merged fix(memory): hit prefix cache in background review fork (salvage #17276 + #25427) #25434.
• fix(agent): forward reasoning_config to background review AIAgent fork #18973 [closed] partial — (+50/-0) — n/a: Forwards reasoning_config at the former run_agent.py constructor, addressing the medium-effort fallback but not the post-refactor live path; closed as superseded by fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379.
• fix(run_agent): pass reasoning_config to background review fork (#18871) #20674 [closed] partial — (+112/-0) — n/a: Implements and tests the same reasoning-config propagation at the obsolete run_agent.py site; despite its earlier keep-open salvage review, it was closed after the current-path fix landed through fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379.
• fix: reuse parent system prompt for background review to preserve cache bytes #25330 [closed] partial — (+221/-20) — n/a: Inherits the parent prompt and session metadata but retains a narrowed enabled_toolsets, so tools[] can still diverge; closed as superseded by broader follow-up work and ultimately fix(memory): hit prefix cache in background review fork (salvage #17276 + #25427) #25434.
• fix(agent): preserve prompt cache + tree-dedup across background review (reference impl for #25322 Option B) #25427 [closed] partial — (+157/-27) — n/a: Reference Option B inherits parent toolsets, cached prompt, timestamp, and session ID, resolving the reported byte differences but relying on prompt-layer tool restraint; closed after its defensive pinning was salvaged into fix(memory): hit prefix cache in background review fork (salvage #17276 + #25427) #25434.
• fix(memory): hit prefix cache in background review fork (salvage #17276 + #25427) #25434 [merged] best fix — (+438/-14) — n/a: Merged reference implementation that combines byte-identical parent system/tools[] with a mechanical thread-local whitelist, resolving all three [Bug]: Background self-improvement review's fresh AIAgent generates a system prompt that bytes-differs from the parent, busting prompt cache + preprocessing tree-dedup #25322 divergence sources while retaining dispatch safety.
• fix(gateway): skip background memory/skill review in group chats #26850 related — (+114/-1) — n/a: Adds a shared direct-chat guard so automatic review does not run in group-like contexts; this addresses multi-user memory pollution rather than the tool-denial cause, and its keep-open review requires complete taxonomy and current-path coverage.
• fix(agent): whitelist read_file in background review agent #27422 [closed] partial — (+5/-2) — n/a: Adds read_file to the old run_agent.py whitelist and updates guidance, but targets a refactored-away location; closed after the maintainer chose to retain the memory/skills-only boundary pending a bounded file-access design.
• fix(agent): forward reasoning_config to background review AIAgent fork #27510 [closed] fixes — (+55/-0) — n/a: Propagates reasoning_config at the correct post-refactor constructor and tests configured values, but lacks the accepted routed-model gate; closed as superseded by fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379 despite earlier keep-open reviews.
• fix(background_review): inherit parent's toolset config to keep tools[] cache-stable (~50% fewer cache-write tokens on long sessions) #29568 [closed] fixes — (+73/-12) — n/a: Propagates parent enabled_toolsets and disabled_toolsets, directly restoring tools[] cache parity for narrowed parents; closed because the same commit was salvaged into merged fix(background_review): inherit parent toolset config for tools[] cache parity (salvage #29568) #29704.
• fix(background_review): inherit parent toolset config for tools[] cache parity (salvage #29568) #29704 [merged] best fix — (+73/-12) — n/a: Merged reference implementation that inherits both parent toolset fields and tests non-empty configurations, resolving the tools[] cache-key divergence reported in background_review fork sends wider tools[] than parent, fragments Anthropic prefix cache (~50% wasted cache-write on long sessions) #29567.
• fix(background_review): inherit parent's reasoning_config to keep thinking/output_config cache-stable (~38% fewer cache-write tokens on long sessions) #30532 [closed] fixes — (+53/-0) — n/a: Adds parent reasoning_config inheritance and a cache-parity regression test, restoring thinking/output_config; closed after its core commit was salvaged into fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379 with a routed-path gate.
• feat(background_review): make review-fork toolsets config-overridable #36967 partial — (+23/-2) — n/a: Adds optional fork-schema overrides for provider tool-count limits while preserving parent inheritance by default, but reads the wrong configuration namespace and has no regression coverage; its keep-open review identifies a concrete salvage path under auxiliary.background_review.
• fix(agent): inherit reasoning_config in background review fork (#18871) #36995 [closed] partial — (+236/-1) — n/a: Propagates reasoning, service-tier, and request-override fields, but would overwrite already resolved route-specific overrides and lacks routed gating; closed after the reasoning portion landed through fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379, leaving service-tier inheritance as separate follow-up scope.
• fix(agent): allow background review to read files #40007 [closed] best fix — (+51/-9) — n/a: Recorded best implementation for [Bug]: background-review fork can't read an external file — model calls skill_manage(action="read_file") and gets "Unknown action" #40003 adds read_file, aligned guidance, and positive/negative whitelist tests, but it grants arbitrary readable-path authority; closed after the maintainer review rejected that one-off expansion in favor of a bounded design.
• fix(background_review): add read_file to review fork tool whitelist #40060 fixes — (+11/-1) — n/a: Adds the same unrestricted read_file exception and prompt guidance at the live path, but without boundary regression tests; it remains a duplicate of closed fix(agent): allow background review to read files #40007 and does not address the maintainer’s bounded-read requirement.
• Make background review toolsets configurable #45014 partial — (+158/-10) — n/a: Makes the runtime whitelist configurable and includes raw Tool Search schemas, but does not align the fork’s inherited schema automatically and omits the memory-disabled and real bridge-dispatch guarantees identified by its keep-open review.
• fix(agent): allow read-only file tools in background review whitelist #45892 partial — (+71/-0) — n/a: Adds read_file and search_files but leaves the prompt and deny text inconsistent and tests membership only; despite its keep-open review, fix(cron): allow read_file and search_files in background review tool whitelist #45902 contains the same additions plus the requested guidance and negative write-tool assertions.
• fix(cron): allow read_file and search_files in background review tool whitelist #45902 best fix — (+11/-3) — n/a: Recorded best implementation for Cron background review blocks read-only tools (read_file, search_files) #45877 adds both read-only tools, updates prompt and denial guidance, and tests that write_file and patch remain excluded, while still requiring reconciliation with the repository’s broader bounded-file-authority decision.
• fix(agent): allow read-only file tools in cron background review #46049 partial — (+27/-10) — n/a: Introduces a file_read toolset containing both requested readers and tests read/write separation, but the toolset is registry-extensible and misses the memory-disabled gate; despite a contributor LGTM, the maintainer review rejects this unbounded exception and fix(cron): allow read_file and search_files in background review tool whitelist #45902 is the narrower duplicate.
• fix(agent): whitelist read_file in background review fork #46684 [closed] partial — (+191/-8) — n/a: Adds only read_file with extensive prompt, deny-message, privacy, and exclusion tests, but retains arbitrary readable-path authority; closed as a duplicate of fix(agent): allow background review to read files #40007 after the underlying design direction was rejected.
• fix(agent): use restricted toolset for background review fork #61529 duplicate — (+39/-15) — n/a: Eliminates the schema/whitelist mismatch by advertising only memory/skills tools, but does so for every provider and therefore regresses the intentional remote cache-parity contract; its keep-open review requires local or non-cache-backed scoping.
• fix(background_review): inherit parent's reasoning_config for Anthropic cache parity (salvage #30532) #64379 [merged] best fix — (+155/-83) — n/a: Merged reference implementation that inherits parent reasoning configuration only on the same-model path and omits it for routed auxiliary models, resolving both the medium fallback and thinking cache divergence without exporting incompatible effort vocabularies.
• fix(agent): steer background review to permitted tools #65803 best fix — (+18/-1) — n/a: Recorded best available fix for background_review fork advertises the parent's full toolset but only permits skills+memory → per-turn tool-denial storm starves the self-improvement loop #61521 preserves parent-schema cache parity and redirects denied file-tool calls to skills_list, skill_view, and skill_manage; its keep-open review requires adding the missing skills_list assertion, and fix(agent): name substitute tools in background review deny message to prevent tool-denial storm (#61521) #61618 is identified as a broader competing duplicate with proactive prompt guidance.

Duplicates

#17276 and #25427 were salvaged into #25434; #29568 was salvaged into #29704; #18973, #20674, #27510, #30532, and the reasoning portion of #36995 converge on #64379. #27422, #40007, #40060, and #46684 share the one-off read_file approach; #45892, #45902, and #46049 share the read_file+search_files approach; #61529 and #65803 address #61521 through different trade-offs, while #65803 overlaps the broader #61618.

Suggested consolidation

Keep #65803 open with a salvage path: preserve its cache-parity-safe denial steering, add the reviewer-requested skills_list assertion, and reconcile or supersede it with #61618’s proactive prompt guidance rather than duplicating the same mitigation. Author action on #61529: rebase onto main and restrict schema narrowing to local or non-prefix-cache endpoints; close #40060 as duplicate of #40007, and close #45892 and #46049 as duplicates of #45902, while keeping arbitrary file-read authority contingent on an explicitly bounded design rather than reopening the rejected #40007 approach.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I61521(["issue #61521 (open)"])
    subgraph Dup61529 ["PRs duplicating each other"]
        P61529["PR #61529 (open)"]
        P65803["PR #65803 (open)"]
    end
    P65803 -->|best fix| I61521
    class I61521 open
    class P61529 open
    class P65803 open
    class P65803 best
    class P65803 target
    click I61521 "https://github.com/NousResearch/hermes-agent/issues/61521"
    click P61529 "https://github.com/NousResearch/hermes-agent/pull/61529"
    click P65803 "https://github.com/NousResearch/hermes-agent/pull/65803"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 24 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 179 kB of PR diffs, 96 kB of issue/PR text, 43 kB of discussion (53 comments), 51 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.