fix(background_review): inherit parent toolset config for tools[] cache parity (salvage #29568)#29704
Merged
Conversation
…[] cache-stable ## Summary The background skill/memory-review fork constructed a child `AIAgent` without propagating `enabled_toolsets` / `disabled_toolsets` from the parent. When the parent narrowed its toolset (via `hermes tools disable` or `config.yaml`), the fork's default `enabled_toolsets=None` expanded to "all registered tools" — and the fork's outbound request body sent a wider `tools[]` array than the parent's main-turn request. Anthropic's prompt-cache key includes the `tools[]` array byte-for-byte, so this divergence forked the cache lineage on every nudge and forced a full prefix rewrite. On a captured ~4 hour Claude-via-Hermes session this cost roughly 4.3 M cache-write tokens — about half of those attributable to the per-nudge alternation between the main turn's narrowed `tools[]` and the review fork's wider `tools[]`. ## Goal Extend the byte-stability invariant established by PR #17276 (which fixed `system`) to the `tools[]` slot of the request body, so the review fork's outbound request hits the parent's warmed Anthropic prefix cache regardless of how the parent's toolset is configured. ## Implementation Two-line change in `agent/background_review.py`: pass `enabled_toolsets=getattr(agent, "enabled_toolsets", None)` and the matching `disabled_toolsets` kwarg into the `AIAgent(...)` call inside `_spawn_background_review`. Adds an explanatory block comment that calls out the cache-key dependency and the relationship to PR #17276. The post-construction runtime whitelist (`set_thread_tool_whitelist({memory, skills})`) is untouched — it still gates which tools the model is allowed to *dispatch*. This change aligns only what the request body *transmits*, not what the review is allowed to do, so the safety contract from issue #15204 remains intact. ## Testing - `tests/run_agent/test_background_review_cache_parity.py`: new `test_review_fork_inherits_parent_toolset_config` asserts the parent's `enabled_toolsets` and `disabled_toolsets` reach the review-fork constructor as kwargs. - `tests/run_agent/test_background_review_toolset_restriction.py`: the existing `test_background_review_does_not_narrow_toolset_schema` was inverted (its old "must NOT pass enabled_toolsets" rule was built on the assumption that the parent always ran with the registry default — wrong in practice when the parent is narrowed). Renamed to `test_background_review_matches_parent_toolset_config` and updated to assert the parent's value propagates verbatim. - Verified the new positive test fails without the fix and passes with it. - Full suite for `test_background_review*`: ``` $ python -m pytest tests/run_agent/test_background_review.py \ tests/run_agent/test_background_review_summary.py \ tests/run_agent/test_background_review_toolset_restriction.py \ tests/run_agent/test_background_review_cache_parity.py -q 18 passed in 1.85s ``` ## Scope - `agent/background_review.py`: 2 added kwargs + explanatory comment. - Two test files: one new positive test, one inverted existing test. - No production code paths outside the review fork; no schema changes; no public-API changes. Refs: ziliangpeng#1 (root-cause analysis with wire-level cache-write measurements). Extends PR #17276's `system`-bytes invariant to the `tools[]` slot.
- Replace 18-line comment block with 3-line invariant statement - Trim test docstrings from multi-paragraph to single-line summaries - Trim assertion messages from 4-line to 2-line mismatch reports - Replace 5-line WHAT comments in stubs with 1-line WHY comments - Add ziliangdotme@gmail.com -> ziliangpeng to AUTHOR_MAP
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-argument-type |
1 |
First entries
agent/background_review.py:408: [invalid-argument-type] invalid-argument-type: Argument to `AIAgent.__init__` is incorrect: Expected `list[str]`, found `Any | None`
✅ Fixed issues: none
Unchanged: 4743 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
This was referenced May 22, 2026
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Salvage of #29568 by @ziliangpeng. The background review fork's
AIAgent()constructor was missingenabled_toolsets/disabled_toolsetskwargs — when the parent narrowed its toolset, the fork defaulted to all registered tools, diverging thetools[]array and breaking Anthropic's prefix cache.Two-line fix: propagate both kwargs from the parent. Extends the byte-stability invariant from PR #25434 (
systemslot) to thetools[]slot.Changes
agent/background_review.py: addenabled_toolsets/disabled_toolsetskwargs + 3-line commenttests/run_agent/test_background_review_cache_parity.py: new positive test, updated stubtests/run_agent/test_background_review_toolset_restriction.py: inverted test to match new invariant, updated stubscripts/release.py: add AUTHOR_MAP entry for contributor emailFollow-up (our commit)
Trimmed ~65 lines of verbose comments/docstrings from the original PR. Added AUTHOR_MAP entry for
ziliangdotme@gmail.com.Validation
18/18 background review tests pass.
Fixes #29567. Cherry-picked from #29568 with @ziliangpeng's authorship preserved.