mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-21 02:05:45 +00:00
test(mcp): cover passive skill tool visibility (#4247)
* test(mcp): cover passive skill tool visibility * test(mcp): tighten deferred discovery coverage
This commit is contained in:
+3
-1
@@ -79,7 +79,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
- **skills:** Apply `allowed-tools` only to slash-activated or actually loaded
|
||||
lead-agent skills, preventing passive enabled skills and evaluation fixtures
|
||||
from removing `task`, web, and file tools from every run. ([#4095], [#4098])
|
||||
from removing MCP, web, file, and delegation tools from every run. ([#4095],
|
||||
[#4098], [#4192])
|
||||
- **models:** Honor `api_base` on every `BaseChatOpenAI` subclass (`VllmChatModel`,
|
||||
`MindIEChatModel`, `PatchedChatMiMo`, `PatchedChatStepFun`, `PatchedChatMiniMax`),
|
||||
not just `ChatOpenAI` / `PatchedChatOpenAI`. Those five previously dropped the
|
||||
@@ -604,3 +605,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#4098]: https://github.com/bytedance/deer-flow/pull/4098
|
||||
[#4146]: https://github.com/bytedance/deer-flow/pull/4146
|
||||
[#4190]: https://github.com/bytedance/deer-flow/pull/4190
|
||||
[#4192]: https://github.com/bytedance/deer-flow/issues/4192
|
||||
|
||||
+1
-1
@@ -747,7 +747,7 @@ Config is env-driven like the others — `MonocleTracingConfig`, built in `get_t
|
||||
|
||||
**`extensions_config.json`**:
|
||||
- `mcpServers` - Map of server name → config (enabled, type, command, args, env, url, headers, oauth, description, `routing`, `tools`, `tool_call_timeout`). `routing.mode="prefer"` emits `<mcp_routing_hints>` prompt guidance; if `tool_search` defers the hinted tool, `McpRoutingMiddleware` can also auto-promote matching deferred schemas before the model call. It does not hard-disable other tools.
|
||||
- `tool_search.auto_promote_top_k` - Global MCP routing auto-promote breadth. Default `3`, clamped to `1..5`; applies only when `tool_search.enabled=true` and only to policy-filtered deferred MCP tools with `routing.mode="prefer"` and non-empty keywords.
|
||||
- `tool_search.auto_promote_top_k` - Global MCP routing auto-promote breadth. Default `3`, clamped to `1..5`; applies only when `tool_search.enabled=true` and only to deferred MCP tools with `routing.mode="prefer"` and non-empty keywords. For lead agents the deferred catalog is built from the full configured MCP set; auto-promotion never grants authority because an active skill's runtime policy still filters model-visible schemas, `tool_search` results, and execution.
|
||||
- `skills` - Map of skill name → state (enabled)
|
||||
|
||||
Both can be modified at runtime via Gateway API endpoints or `DeerFlowClient` methods.
|
||||
|
||||
@@ -358,8 +358,9 @@ def build_middlewares(
|
||||
middlewares.append(mcp_routing_middleware)
|
||||
|
||||
# Hide deferred tool schemas from model binding until tool_search promotes them.
|
||||
# The deferred set + catalog hash come from the build-time setup (assembled
|
||||
# after tool-policy filtering); promotion is read from graph state.
|
||||
# The lead deferred set + catalog hash come from the full build-time MCP
|
||||
# catalog; SkillToolPolicyMiddleware separately filters model visibility,
|
||||
# tool_search results, and execution for the active skill at runtime.
|
||||
if deferred_setup is not None and deferred_setup.deferred_names:
|
||||
from deerflow.agents.middlewares.deferred_tool_filter_middleware import DeferredToolFilterMiddleware
|
||||
|
||||
|
||||
@@ -284,7 +284,9 @@ def get_deferred_tools_prompt_section(*, deferred_names: frozenset[str] = frozen
|
||||
|
||||
Lists only names so the agent knows what exists and can use tool_search to
|
||||
load them. Returns empty string when there are no deferred tools. The set is
|
||||
computed at agent build time (after tool-policy filtering) and passed in.
|
||||
computed at agent build time and passed in. Lead-agent sets contain the full
|
||||
configured MCP catalog because active skill policy is applied at runtime;
|
||||
subagent sets may already have been filtered by their startup skill policy.
|
||||
|
||||
Lives here, next to the assembly that produces ``deferred_names``, so every
|
||||
agent-build path (lead, embedded client, subagent) renders the section the
|
||||
|
||||
@@ -128,11 +128,11 @@ def get_available_tools(
|
||||
if mcp_tools:
|
||||
logger.info(f"Using {len(mcp_tools)} cached MCP tool(s)")
|
||||
|
||||
# Tag MCP-sourced tools so deferred-tool assembly (done at
|
||||
# the agent construction site, AFTER tool-policy filtering)
|
||||
# can identify them. No ContextVar / registry is built here;
|
||||
# the deferred catalog + tool_search tool are assembled per
|
||||
# agent from the policy-filtered tool list.
|
||||
# Tag MCP-sourced tools so deferred-tool assembly at each
|
||||
# agent construction site can identify them. Lead agents
|
||||
# assemble their full configured MCP catalog and apply active
|
||||
# skill policy at runtime; subagents may pass an already
|
||||
# policy-filtered list because their skills load at startup.
|
||||
for t in mcp_tools:
|
||||
tag_mcp_tool(t)
|
||||
except ImportError:
|
||||
|
||||
@@ -367,23 +367,53 @@ def test_make_lead_agent_all_legacy_skills_preserve_all_tools(monkeypatch):
|
||||
assert tool_names == ["bash", "read_file", "update_agent", "describe_skill"]
|
||||
|
||||
|
||||
def test_make_lead_agent_does_not_apply_passive_skill_policy_when_cache_is_cold(monkeypatch):
|
||||
def test_make_lead_agent_passive_empty_skill_policy_preserves_mcp_and_other_tools_when_cache_is_cold(monkeypatch):
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from langchain_core.tools import tool
|
||||
|
||||
from deerflow.agents.lead_agent import agent as lead_agent_module
|
||||
from deerflow.agents.lead_agent import prompt as prompt_module
|
||||
from deerflow.tools.mcp_metadata import tag_mcp_tool
|
||||
|
||||
@tool
|
||||
def lightrag_query(query: str) -> str:
|
||||
"""Query a LightRAG MCP server."""
|
||||
return query
|
||||
|
||||
tag_mcp_tool(lightrag_query)
|
||||
|
||||
captured_deferred_setups = []
|
||||
|
||||
def capture_build_middlewares(*args, **kwargs):
|
||||
captured_deferred_setups.append(kwargs["deferred_setup"])
|
||||
return []
|
||||
|
||||
monkeypatch.setattr(lead_agent_module, "_resolve_model_name", lambda x=None, **kwargs: "default-model")
|
||||
monkeypatch.setattr(lead_agent_module, "create_chat_model", lambda **kwargs: "model")
|
||||
monkeypatch.setattr(lead_agent_module, "build_middlewares", lambda *args, **kwargs: [])
|
||||
monkeypatch.setattr(lead_agent_module, "build_middlewares", capture_build_middlewares)
|
||||
monkeypatch.setattr(lead_agent_module, "apply_prompt_template", lambda **kwargs: "mock_prompt")
|
||||
monkeypatch.setattr(lead_agent_module, "create_agent", lambda **kwargs: kwargs)
|
||||
monkeypatch.setattr(lead_agent_module, "load_agent_config", lambda x: AgentConfig(name="test", skills=["restricted"]))
|
||||
monkeypatch.setattr("deerflow.tools.get_available_tools", lambda **kwargs: [NamedTool("bash"), NamedTool("read_file"), NamedTool("web_search")])
|
||||
monkeypatch.setattr(
|
||||
lead_agent_module,
|
||||
"load_agent_config",
|
||||
lambda x: AgentConfig(name="test", skills=["example-safe-skill"]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"deerflow.tools.get_available_tools",
|
||||
lambda **kwargs: [
|
||||
NamedTool("bash"),
|
||||
NamedTool("read_file"),
|
||||
NamedTool("web_search"),
|
||||
lightrag_query,
|
||||
],
|
||||
)
|
||||
|
||||
mock_app_config = MagicMock()
|
||||
mock_app_config.get_model_config.return_value = SimpleNamespace(supports_thinking=False, supports_vision=False)
|
||||
mock_storage = SimpleNamespace(load_skills=lambda *, enabled_only: [_make_skill("restricted", ["read_file"])])
|
||||
mock_app_config.tool_search.enabled = True
|
||||
mock_app_config.tool_search.auto_promote_top_k = 3
|
||||
mock_storage = SimpleNamespace(load_skills=lambda *, enabled_only: [_make_skill("example-safe-skill", [])])
|
||||
|
||||
with prompt_module._enabled_skills_lock:
|
||||
prompt_module._enabled_skills_cache = None
|
||||
@@ -394,7 +424,9 @@ def test_make_lead_agent_does_not_apply_passive_skill_policy_when_cache_is_cold(
|
||||
agent_kwargs = lead_agent_module.make_lead_agent({"configurable": {"agent_name": "test"}})
|
||||
|
||||
tool_names = [tool.name for tool in agent_kwargs["tools"]]
|
||||
assert {"bash", "read_file", "web_search", "describe_skill"} <= set(tool_names)
|
||||
assert {"bash", "read_file", "web_search", "lightrag_query", "tool_search", "describe_skill"} <= set(tool_names)
|
||||
assert len(captured_deferred_setups) == 1
|
||||
assert captured_deferred_setups[0].deferred_names == frozenset({"lightrag_query"})
|
||||
|
||||
|
||||
def test_make_lead_agent_fails_closed_when_skill_policy_load_fails(monkeypatch):
|
||||
|
||||
@@ -41,8 +41,10 @@ def denied_lookup(query: str) -> str:
|
||||
class _StorageStub:
|
||||
def __init__(self, skills: list[Skill]):
|
||||
self._skills = skills
|
||||
self.load_calls = 0
|
||||
|
||||
def load_skills(self, *, enabled_only: bool = False) -> list[Skill]:
|
||||
self.load_calls += 1
|
||||
return [skill for skill in self._skills if skill.enabled or not enabled_only]
|
||||
|
||||
def get_container_root(self) -> str:
|
||||
@@ -94,6 +96,65 @@ def _deferred_setup():
|
||||
)
|
||||
|
||||
|
||||
def test_passive_empty_skill_policy_preserves_deferred_mcp_discovery_and_calling():
|
||||
passive_fixture = _skill("example-safe-skill", [])
|
||||
storage = _StorageStub([passive_fixture])
|
||||
policy = SkillToolPolicyMiddleware(slash_source_owner_token=_SLASH_SOURCE_OWNER_TOKEN)
|
||||
policy._storage = lambda: storage
|
||||
setup = _deferred_setup()
|
||||
model = _RecordingModel(
|
||||
[
|
||||
AIMessage(
|
||||
content="",
|
||||
tool_calls=[
|
||||
{
|
||||
"name": "tool_search",
|
||||
"args": {"query": "select:calc"},
|
||||
"id": "passive-search-call",
|
||||
"type": "tool_call",
|
||||
}
|
||||
],
|
||||
),
|
||||
AIMessage(
|
||||
content="",
|
||||
tool_calls=[
|
||||
{
|
||||
"name": "calc",
|
||||
"args": {"expression": "2 + 2"},
|
||||
"id": "passive-calc-call",
|
||||
"type": "tool_call",
|
||||
}
|
||||
],
|
||||
),
|
||||
AIMessage(content="done"),
|
||||
]
|
||||
)
|
||||
_CALC_CALLS.clear()
|
||||
graph = create_agent(
|
||||
model=model,
|
||||
tools=[calc, denied_lookup, setup.tool_search_tool],
|
||||
middleware=[
|
||||
policy,
|
||||
DeferredToolFilterMiddleware(setup.deferred_names, setup.catalog_hash),
|
||||
],
|
||||
state_schema=ThreadState,
|
||||
)
|
||||
|
||||
result = graph.invoke(
|
||||
{"messages": [HumanMessage(content="use the configured MCP calculator")]},
|
||||
context={},
|
||||
)
|
||||
|
||||
assert model.bound_tool_names[0] == ["tool_search"]
|
||||
assert "calc" in model.bound_tool_names[1]
|
||||
# This tool remains hidden because it was not promoted, not because the passive
|
||||
# skill applied a policy restriction.
|
||||
assert "denied_lookup" not in model.bound_tool_names[1]
|
||||
assert _CALC_CALLS == ["2 + 2"]
|
||||
assert result["promoted"] == {"catalog_hash": setup.catalog_hash, "names": ["calc"]}
|
||||
assert storage.load_calls == 0
|
||||
|
||||
|
||||
def test_active_skill_can_search_promote_and_call_allowed_deferred_tool():
|
||||
restricted = _skill("restricted", ["calc"])
|
||||
policy, context = _active_policy(restricted)
|
||||
|
||||
Reference in New Issue
Block a user