fix(background-review): freeze review fork tool snapshot generation against compaction refresh
Freezes review_agent._tool_snapshot_generation to _FROZEN_TOOL_SNAPSHOT_GENERATION (2_147_483_647) when inheriting the parent tool surface for same-model cache parity. When in-place compaction boundaries trigger refresh_agent_mcp_tools(content_aware=True), the staleness guard in _publish_tool_snapshot refuses the rebuild (snapshot_generation < published_gen), preventing agent.tools from being reconstructed from the raw registry and preserving inherited memory-provider and late tools across compaction boundaries (#103579). Adds unit regression test verifying tool preservation across content_aware refresh. Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
This commit is contained in:
@@ -842,6 +842,13 @@ def _fork_init_kwargs(agent: Any, rt: Dict[str, Any], routed: bool, max_iteratio
|
||||
return kwargs
|
||||
|
||||
|
||||
# Sentinel generation above any live registry generation. Mid-review compaction
|
||||
# boundaries re-run refresh_agent_mcp_tools(content_aware=True); freezing the
|
||||
# snapshot generation prevents compaction from rebuilding agent.tools and dropping
|
||||
# inherited provider tools (#103579).
|
||||
_FROZEN_TOOL_SNAPSHOT_GENERATION: int = 2_147_483_647
|
||||
|
||||
|
||||
def _inherit_parent_tool_surface(review_agent: Any, agent: Any) -> None:
|
||||
"""Copy the parent's advertised tools for a same-model cache-parity fork.
|
||||
|
||||
@@ -851,6 +858,10 @@ def _inherit_parent_tool_surface(review_agent: Any, agent: Any) -> None:
|
||||
restricted by the thread whitelist. Copying the whole surface covers every
|
||||
dynamically injected tool (memory-provider, late MCP, and plugin-registered),
|
||||
not just memory tools.
|
||||
|
||||
Freezes ``_tool_snapshot_generation`` so mid-review compaction boundaries
|
||||
(which invoke ``refresh_agent_mcp_tools(content_aware=True)``) refuse the
|
||||
rebuild and do not clobber the inherited tools (#103579).
|
||||
"""
|
||||
parent_tools = getattr(agent, "tools", None)
|
||||
if not isinstance(parent_tools, (list, tuple)) or not parent_tools:
|
||||
@@ -863,6 +874,7 @@ def _inherit_parent_tool_surface(review_agent: Any, agent: Any) -> None:
|
||||
and isinstance(entry.get("function"), dict)
|
||||
and isinstance(entry["function"].get("name"), str)
|
||||
}
|
||||
review_agent._tool_snapshot_generation = _FROZEN_TOOL_SNAPSHOT_GENERATION
|
||||
|
||||
|
||||
def build_cache_parity_fork(
|
||||
|
||||
@@ -103,6 +103,7 @@ def _make_recorder_class(captured=None, record_on_run=()):
|
||||
self.session_id = None
|
||||
self.tools = None
|
||||
self.valid_tool_names = set()
|
||||
self._tool_snapshot_generation = 0
|
||||
self.ephemeral_system_prompt = kwargs.get("ephemeral_system_prompt")
|
||||
|
||||
def run_conversation(self, *args, **kwargs):
|
||||
@@ -407,3 +408,48 @@ def test_routed_review_fork_does_not_inherit_tool_surface():
|
||||
assert routed
|
||||
assert fork.tools is None
|
||||
|
||||
|
||||
def test_review_fork_inherited_tools_survive_compaction_refresh():
|
||||
"""Inherited tools survive mid-review compaction refresh (#103579).
|
||||
|
||||
Acceptance criterion 1 requires the fork to advertise the same tools[] as
|
||||
the parent when targeting the same cache scope. Mid-review compaction
|
||||
boundaries invoke refresh_agent_mcp_tools(content_aware=True), which re-reads
|
||||
the live registry and drops memory provider tools unless the snapshot
|
||||
generation staleness check refuses the rebuild.
|
||||
"""
|
||||
import run_agent
|
||||
from agent.background_review import build_cache_parity_fork
|
||||
from tools.mcp_tool_agent import refresh_agent_mcp_tools
|
||||
|
||||
agent = _make_agent_stub(run_agent.AIAgent)
|
||||
parent_tools = [
|
||||
{"type": "function", "function": {"name": "terminal_command"}},
|
||||
{"type": "function", "function": {"name": "read_file"}},
|
||||
{"type": "function", "function": {"name": "memory"}},
|
||||
{"type": "function", "function": {"name": "fact_store"}},
|
||||
{"type": "function", "function": {"name": "fact_feedback"}},
|
||||
]
|
||||
agent.tools = parent_tools
|
||||
|
||||
_Recorder = _make_recorder_class()
|
||||
|
||||
with patch.object(run_agent, "AIAgent", _Recorder):
|
||||
fork, _rt, routed = build_cache_parity_fork(agent, max_iterations=5)
|
||||
assert not routed
|
||||
assert fork._tool_snapshot_generation == 2_147_483_647
|
||||
assert [t["function"]["name"] for t in fork.tools] == [
|
||||
"terminal_command", "read_file", "memory", "fact_store", "fact_feedback"
|
||||
]
|
||||
|
||||
# Simulate mid-review compaction boundary tool refresh
|
||||
added = refresh_agent_mcp_tools(fork, content_aware=True)
|
||||
assert added == set()
|
||||
assert [t["function"]["name"] for t in fork.tools] == [
|
||||
"terminal_command", "read_file", "memory", "fact_store", "fact_feedback"
|
||||
]
|
||||
assert fork.valid_tool_names == {
|
||||
"terminal_command", "read_file", "memory", "fact_store", "fact_feedback"
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user