Skip to content

fix(agent): allow read-only file tools in background review whitelist - #45892

Open
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/cron-review-read-only-tools
Open

fix(agent): allow read-only file tools in background review whitelist#45892
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/cron-review-read-only-tools

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds read_file and search_files to the background review tool whitelist, allowing the review agent to inspect configuration files, prior outputs, and skill references during its post-turn memory/skill review pass.

Related Issue

Fixes #45877

Type of Change

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

Changes Made

  • agent/background_review.py: Added read_file and search_files to the review_whitelist set after the toolset-based whitelist is constructed. These tools are read-only and side-effect-free.
  • tests/run_agent/test_background_review_cache_parity.py: Added test_background_review_whitelist_includes_read_only_file_tools to verify the whitelist includes both read-only file tools.

How to Test

  1. Run the updated test: pytest tests/run_agent/test_background_review_cache_parity.py -xvs
  2. Verify the new test test_background_review_whitelist_includes_read_only_file_tools passes alongside the 3 existing tests
  3. Confirm that only read_file and search_files were added (no write tools like write_file or patch)

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
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: agent/background_review.pyreview_whitelist construction (line 470), set_thread_tool_whitelist call (line 477)
  • Blast radius: LOW — adds 2 tool names to a per-review-call whitelist set; no control flow changes
  • Related patterns: read-only tool whitelisting, cron background review

read_file and search_files are side-effect-free tools that cron agents
need to inspect config, prior outputs, and skill references. Blocking
them forces workarounds via terminal(cat/grep) or blind operation.

Fixes NousResearch#45877
@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 P3 Low — cosmetic, nice to have labels Jun 14, 2026
AIalliAI added a commit to AIalliAI/Hermes that referenced this pull request Jun 14, 2026
AIalliAI added a commit to AIalliAI/Hermes that referenced this pull request Jun 14, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused whitelist change. The current HEAD premise is valid: agent/background_review.py:806-815 derives the runtime whitelist only from memory and skills, and hermes_cli/plugins.py:2135-2141 blocks non-whitelisted tools before dispatch.

Problems

  • agent/background_review.py:819-820 and :841-843 still tell the review agent that only memory and skill tools are allowed and that it must not attempt other tools. Adding read_file/search_files to the whitelist alone therefore leaves the model instructed not to use the newly allowed tools.
  • The new test checks whitelist membership only. tests/run_agent/test_background_review_toolset_restriction.py:88-158 should also cover the intended boundary: read-only file tools allowed, write_file and patch denied.

Suggested changes

  • Update the prompt and denial text to name the two permitted read-only file tools.
  • Extend the runtime-whitelist test with positive read-only and negative write-tool assertions.

Automated hermes-sweeper review.

)
}
# Allow read-only file tools — they have no side effects and
# are needed for the review agent to inspect config, prior

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the unchanged review prompt and denial text below as well: they still say only memory/skill tools are allowed and explicitly tell the model not to attempt other tools. Without that, the model is instructed not to use this newly permitted tool.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 14, 2026
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 sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron background review blocks read-only tools (read_file, search_files)

3 participants