fix(tools): restore executor allowlist on message_agent inject success
ensure_message_agent_tool() returned True on the schema-present branch without re-adding message_agent to agent.valid_tool_names. A long-lived Bot Chat whose tool surface was rebuilt (compaction / MCP refresh republishes the allowlist from the registry snapshot, which never contains the injected tool) then advertised message_agent while the executor rejected every call, and the model fell back to hermes -p shellouts that time out. Success now means both halves hold. Re-applied onto the current any()-shaped gate; semantic change and test are the contributor's (#96109). Closes #96105
This commit is contained in:
@@ -86,6 +86,30 @@ def test_injects_only_into_bot_chat_on_managed_install(tmp_path):
|
||||
assert len(agent.tools) == 1
|
||||
|
||||
|
||||
def test_restores_allowlist_when_schema_survives_surface_refresh(tmp_path):
|
||||
"""#96105: success must restore the executor allowlist on the
|
||||
schema-present branch.
|
||||
|
||||
A long-lived Bot Chat whose tool surface is reconstructed can keep the
|
||||
schema while the executor's allowlist is rebuilt empty. The injector then
|
||||
returned True while dispatch would reject every ``message_agent`` call —
|
||||
advertised but non-dispatchable. Restore-on-success contract: whenever
|
||||
``ensure_message_agent_tool()`` returns True, ``MESSAGE_AGENT_TOOL_NAME``
|
||||
is in ``valid_tool_names`` whenever that attribute is a set.
|
||||
"""
|
||||
home = _managed_home(tmp_path)
|
||||
agent = _FakeAgent(home, title="Bot Chat")
|
||||
assert bot_mode_dm.ensure_message_agent_tool(agent) is True
|
||||
assert len(agent.tools) == 1
|
||||
|
||||
# capability refresh: schema survives, executor allowlist rebuilt empty
|
||||
agent.valid_tool_names = set()
|
||||
assert bot_mode_dm.ensure_message_agent_tool(agent) is True
|
||||
assert bot_mode_dm.MESSAGE_AGENT_TOOL_NAME in agent.valid_tool_names
|
||||
# byte-stable: no duplicate schema was appended
|
||||
assert len(agent.tools) == 1
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"title",
|
||||
["", "My research chat", "Group: room-abc123", "handoff-12ab34cd"],
|
||||
|
||||
@@ -136,16 +136,19 @@ def ensure_message_agent_tool(agent: Any) -> bool:
|
||||
if not getattr(agent, "_bot_mode_protocol", True):
|
||||
return False
|
||||
tools = getattr(agent, "tools", None)
|
||||
if tools and any(
|
||||
present = bool(tools) and any(
|
||||
isinstance(t, dict) and t.get("function", {}).get("name") == MESSAGE_AGENT_TOOL_NAME
|
||||
for t in tools
|
||||
):
|
||||
return True
|
||||
if not message_agent_authorized(agent):
|
||||
return False
|
||||
if agent.tools is None:
|
||||
agent.tools = []
|
||||
agent.tools.append(message_agent_tool_schema())
|
||||
)
|
||||
if not present:
|
||||
if not message_agent_authorized(agent):
|
||||
return False
|
||||
if agent.tools is None:
|
||||
agent.tools = []
|
||||
agent.tools.append(message_agent_tool_schema())
|
||||
# Success means BOTH halves hold: a tool-surface rebuild (compaction, MCP refresh)
|
||||
# can keep the schema while valid_tool_names is republished without it, and an
|
||||
# advertised-but-nondispatchable tool sends the model hunting for shellouts (#96105).
|
||||
valid = getattr(agent, "valid_tool_names", None)
|
||||
if isinstance(valid, set):
|
||||
valid.add(MESSAGE_AGENT_TOOL_NAME)
|
||||
|
||||
Reference in New Issue
Block a user