diff --git a/CHANGELOG.md b/CHANGELOG.md index 61955c0e1..5b6e7b4d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/backend/AGENTS.md b/backend/AGENTS.md index ea85b8e00..cf65f88b7 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -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 `` 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. diff --git a/backend/packages/harness/deerflow/agents/lead_agent/agent.py b/backend/packages/harness/deerflow/agents/lead_agent/agent.py index b8bc74a5c..754e7bed1 100644 --- a/backend/packages/harness/deerflow/agents/lead_agent/agent.py +++ b/backend/packages/harness/deerflow/agents/lead_agent/agent.py @@ -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 diff --git a/backend/packages/harness/deerflow/tools/builtins/tool_search.py b/backend/packages/harness/deerflow/tools/builtins/tool_search.py index 75421ef06..656ade5f0 100644 --- a/backend/packages/harness/deerflow/tools/builtins/tool_search.py +++ b/backend/packages/harness/deerflow/tools/builtins/tool_search.py @@ -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 diff --git a/backend/packages/harness/deerflow/tools/tools.py b/backend/packages/harness/deerflow/tools/tools.py index 608ce329f..ca20aeb07 100644 --- a/backend/packages/harness/deerflow/tools/tools.py +++ b/backend/packages/harness/deerflow/tools/tools.py @@ -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: diff --git a/backend/tests/test_lead_agent_skills.py b/backend/tests/test_lead_agent_skills.py index fde33d1a5..b78d6b789 100644 --- a/backend/tests/test_lead_agent_skills.py +++ b/backend/tests/test_lead_agent_skills.py @@ -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): diff --git a/backend/tests/test_skill_policy_deferred_discovery.py b/backend/tests/test_skill_policy_deferred_discovery.py index f5249d818..25271c532 100644 --- a/backend/tests/test_skill_policy_deferred_discovery.py +++ b/backend/tests/test_skill_policy_deferred_discovery.py @@ -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)