Skip to content

fix(agent): whitelist read_file in background review fork - #46684

Closed
flooryyyy wants to merge 1 commit into
NousResearch:mainfrom
flooryyyy:fix/bg-review-readfile-whitelist
Closed

fix(agent): whitelist read_file in background review fork#46684
flooryyyy wants to merge 1 commit into
NousResearch:mainfrom
flooryyyy:fix/bg-review-readfile-whitelist

Conversation

@flooryyyy

@flooryyyy flooryyyy commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Background

Background review whitelist only allows memory+skills toolsets. read_file is blocked despite being read-only and needed for the review fork to inspect source files when authoring skills.

Without read_file the model fabricates content or hits skill_manage denials when referencing external files. 15 sessions failed silently between May 14-18 before local patch.

Supersedes stale #27422 which targeted the old run_agent.py path (since refactored into agent/background_review.py).

Changes

  • Add read_file to review_whitelist after toolset-derived build
  • Update deny message and review prompt to mention read_file
  • Update module docstring to reflect expanded whitelist
  • Add privacy guard: do not copy private local file contents into durable skills

Tests

  • New test_background_review_readfile_whitelist.py (11 tests) covering whitelist composition, deny message, and prompt content
  • Update existing test_background_review_toolset_restriction.py to assert read_file is allowed and file write tools are denied

Why not the full file toolset?

Only read_file is added. write_file, patch, and search_files remain denied - the review fork should read, not write, external files.

Verification

uv run --extra dev python -m pytest tests/run_agent/test_background_review_toolset_restriction.py tests/run_agent/test_background_review_readfile_whitelist.py -v
uv run --extra dev python -m ruff check agent/background_review.py tests/run_agent/test_background_review_readfile_whitelist.py tests/run_agent/test_background_review_toolset_restriction.py

Both #40060 and #40007 address the same issue - closing in favor of this one which has full test coverage.

Background review whitelist only allows memory+skills toolsets.
read_file is blocked despite being read-only and needed for the
review fork to inspect source files when authoring skills.

Without read_file the model fabricates content or hits skill_manage
denials when referencing external files. 15 sessions failed silently
between May 14-18 before local patch.

Changes:
- Add read_file to review_whitelist after toolset-derived build
- Update deny message and review prompt to mention read_file
- Update module docstring to reflect expanded whitelist
- Add privacy guard: do not copy private local file contents
  into durable skills

Tests:
- New test_background_review_readfile_whitelist.py with 11 tests
  covering whitelist composition, deny message, and prompt content
- Update existing test_background_review_toolset_restriction.py to
  assert read_file is allowed and file write tools are denied

Supersedes original PR NousResearch#27422 which targeted run_agent.py (since
refactored into agent/background_review.py).
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/file File tools (read, write, patch, search) duplicate This issue or pull request already exists labels Jun 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #40007 — same fix (add read_file to the background review fork whitelist in agent/background_review.py) via the same mechanism. #40007 is the earliest still-open PR for this; #40060 is a competing twin with the identical approach.

Related: #27422 (your own earlier attempt, closed because it targeted the pre-refactor run_agent.py path).

@flooryyyy

Copy link
Copy Markdown
Contributor Author

left my agent to fix conflicts and bring it up to main in #27422 but it decided to re-open it instead. not sure if this should stay or be closed in favour of #40007 or

@flooryyyy

flooryyyy commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

closing - teknium closed the original #27422 (i didn't notice until after). if the fix is still wanted, #40060 and #40007 already cover it.

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 duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have tool/file File tools (read, write, patch, search) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants