Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions agent/background_review.py
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,23 @@ def _bg_review_auto_deny(command, description, **kwargs):
# parent below so memory(action="add") writes from
# the review still land on disk; the review just
# has zero side effects on external providers.
# Match parent's toolset config so ``tools[]`` is byte-identical
# in the request body — Anthropic's cache key includes it.
# (The runtime whitelist below still restricts dispatch.)
# Build the restricted toolset for the review fork BEFORE constructing
# the agent. Using the parent's full toolset (via enabled_toolsets) would
# advertise tools that the runtime whitelist denies, causing a storm of
# "denied non-whitelisted tool" errors (see issue #61521).
# Gate the built-in memory tool on the profile's memory_enabled flag.
# Hardcoding ["memory", "skills"] granted the review LLM the MEMORY.md
# read/write tool even when a profile set memory_enabled: false,
# contaminating a memory-disabled profile (#54937 layer 2).
review_toolsets = ["skills"]
if agent._memory_enabled or agent._user_profile_enabled:
review_toolsets.insert(0, "memory")

# Use the restricted toolset for the review fork. The comment below
# about byte-identical tools[] for cache parity is preserved as context,
# but the actual fix prioritizes correct toolset semantics over cache
# parity — the per-review cost increase is acceptable compared to the
# functional breakage (storm of tool denials and broken self-improvement).
review_agent = AIAgent(
model=_rt.get("model") or agent.model,
max_iterations=16,
Expand All @@ -691,7 +705,7 @@ def _bg_review_auto_deny(command, description, **kwargs):
api_key=_rt.get("api_key") or None,
credential_pool=getattr(agent, "_credential_pool", None),
parent_session_id=agent.session_id,
enabled_toolsets=getattr(agent, "enabled_toolsets", None),
enabled_toolsets=review_toolsets,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This applies schema narrowing to remote cache-backed providers too. Commit 5fe0672 intentionally restored parent tools[] parity because Anthropic’s cache key includes tools and measured lower review cost; preserve the parent schema remotely and scope narrowing to endpoints without that cache benefit.

disabled_toolsets=getattr(agent, "disabled_toolsets", None),
skip_memory=True,
)
Expand Down Expand Up @@ -781,13 +795,6 @@ def _bg_review_auto_deny(command, description, **kwargs):
clear_thread_tool_whitelist,
)

# Gate the built-in memory tool on the profile's memory_enabled flag.
# Hardcoding ["memory", "skills"] granted the review LLM the MEMORY.md
# read/write tool even when a profile set memory_enabled: false,
# contaminating a memory-disabled profile (#54937 layer 2).
review_toolsets = ["skills"]
if review_agent._memory_enabled or review_agent._user_profile_enabled:
review_toolsets.insert(0, "memory")
review_whitelist = {
t["function"]["name"]
for t in get_tool_definitions(
Expand Down
12 changes: 10 additions & 2 deletions tests/run_agent/test_background_review_cache_parity.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,9 +229,17 @@ def close(self):
review_skills=False,
)

assert captured.get("enabled_toolsets") == agent.enabled_toolsets, (
# With the schema-level toolset narrowing (PR #61529), the review fork
# uses a restricted toolset (["memory", "skills"]) rather than inheriting
# the parent's full toolset. This prevents the fork from advertising tools
# that the runtime whitelist denies (issue #61521).
expected_toolsets = ["skills"]
if agent._memory_enabled or agent._user_profile_enabled:
expected_toolsets.insert(0, "memory")

assert captured.get("enabled_toolsets") == expected_toolsets, (
f"enabled_toolsets mismatch: {captured.get('enabled_toolsets')!r} "
f"vs expected {agent.enabled_toolsets!r}"
f"vs expected {expected_toolsets!r}"
)
assert captured.get("disabled_toolsets") == agent.disabled_toolsets, (
f"disabled_toolsets mismatch: {captured.get('disabled_toolsets')!r} "
Expand Down
13 changes: 11 additions & 2 deletions tests/run_agent/test_background_review_toolset_restriction.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,18 @@ def _capture_init(self, *args, **kwargs):
)

assert "enabled_toolsets" in captured, "AIAgent.__init__ was not called"
assert captured["enabled_toolsets"] == agent.enabled_toolsets, (

# With the schema-level toolset narrowing (PR #61529), the review fork
# uses a restricted toolset (["memory", "skills"]) rather than inheriting
# the parent's full toolset. This prevents the fork from advertising tools
# that the runtime whitelist denies (issue #61521).
expected_toolsets = ["skills"]
if agent._memory_enabled or agent._user_profile_enabled:
expected_toolsets.insert(0, "memory")

assert captured["enabled_toolsets"] == expected_toolsets, (
f"enabled_toolsets mismatch: {captured['enabled_toolsets']!r} "
f"vs expected {agent.enabled_toolsets!r}"
f"vs expected {expected_toolsets!r}"
)
assert captured["disabled_toolsets"] == agent.disabled_toolsets, (
f"disabled_toolsets mismatch: {captured['disabled_toolsets']!r} "
Expand Down
Loading