-
Notifications
You must be signed in to change notification settings - Fork 9.6k
fix(tool-search): reliably hide deferred MCP schemas by removing the ContextVar (closures + graph state) #3342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1a48fb7
6f79ee3
2c6a210
d45ae6d
3379ee5
7d3f14b
c0a94aa
4cf13dd
4052efe
ad8feb7
432f0e6
92d463e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,15 @@ | ||
| """Middleware to filter deferred tool schemas from model binding. | ||
|
|
||
| When tool_search is enabled, MCP tools are registered in the DeferredToolRegistry | ||
| and passed to ToolNode for execution, but their schemas should NOT be sent to the | ||
| LLM via bind_tools (that's the whole point of deferral — saving context tokens). | ||
|
|
||
| This middleware intercepts wrap_model_call and removes deferred tools from | ||
| request.tools so that model.bind_tools only receives active tool schemas. | ||
| The agent discovers deferred tools at runtime via the tool_search tool. | ||
| When tool_search is enabled, MCP tools are still passed to ToolNode for | ||
| execution, but their schemas must NOT be sent to the LLM via bind_tools until | ||
| the model has discovered them via tool_search. This middleware removes the | ||
| still-deferred tools from request.tools before model binding, and blocks tool | ||
| calls to tools that have not been promoted yet. | ||
|
|
||
| The deferred name set and the catalog hash are injected at construction time | ||
| (no ContextVar). Promotion state is read from graph state (``state["promoted"]``), | ||
| scoped by catalog hash so a stale persisted promotion cannot expose a renamed | ||
| or drifted tool. | ||
| """ | ||
|
|
||
| import logging | ||
|
|
@@ -24,47 +27,49 @@ | |
|
|
||
|
|
||
| class DeferredToolFilterMiddleware(AgentMiddleware[AgentState]): | ||
| """Remove deferred tools from request.tools before model binding. | ||
| """Hide deferred tool schemas from the bound model until promoted. | ||
|
|
||
| ToolNode still holds all tools (including deferred) for execution routing, | ||
| but the LLM only sees active tool schemas — deferred tools are discoverable | ||
| via tool_search at runtime. | ||
| but the LLM only sees active tool schemas plus tools that have already been | ||
| promoted (recorded in ``state["promoted"]`` under the current catalog hash). | ||
| """ | ||
|
|
||
| def _filter_tools(self, request: ModelRequest) -> ModelRequest: | ||
| from deerflow.tools.builtins.tool_search import get_deferred_registry | ||
|
|
||
| registry = get_deferred_registry() | ||
| if not registry: | ||
| return request | ||
| def __init__(self, deferred_names: frozenset[str], catalog_hash: str | None): | ||
| super().__init__() | ||
| self._deferred = deferred_names | ||
| self._catalog_hash = catalog_hash | ||
|
|
||
| deferred_names = registry.deferred_names | ||
| active_tools = [t for t in request.tools if getattr(t, "name", None) not in deferred_names] | ||
| def _promoted(self, state) -> set[str]: | ||
| promoted = (state or {}).get("promoted") | ||
| if promoted and promoted.get("catalog_hash") == self._catalog_hash: | ||
| return set(promoted.get("names") or []) | ||
| return set() | ||
|
|
||
| if len(active_tools) < len(request.tools): | ||
| logger.debug(f"Filtered {len(request.tools) - len(active_tools)} deferred tool schema(s) from model binding") | ||
| def _hidden(self, state) -> set[str]: | ||
| return set(self._deferred) - self._promoted(state) | ||
|
|
||
| return request.override(tools=active_tools) | ||
| def _filter_tools(self, request: ModelRequest) -> ModelRequest: | ||
| if not self._deferred: | ||
| return request | ||
| hide = self._hidden(request.state) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Verified against the pinned The suggested fallback would actually break this path. |
||
| if not hide: | ||
| return request | ||
| active = [t for t in request.tools if getattr(t, "name", None) not in hide] | ||
| if len(active) < len(request.tools): | ||
| logger.debug("Filtered %d deferred tool schema(s) from model binding", len(request.tools) - len(active)) | ||
| return request.override(tools=active) | ||
|
|
||
| def _blocked_tool_message(self, request: ToolCallRequest) -> ToolMessage | None: | ||
| from deerflow.tools.builtins.tool_search import get_deferred_registry | ||
|
|
||
| registry = get_deferred_registry() | ||
| if not registry: | ||
| return None | ||
|
|
||
| tool_name = str(request.tool_call.get("name") or "") | ||
| if not tool_name: | ||
| if not self._deferred: | ||
| return None | ||
|
|
||
| if not registry.contains(tool_name): | ||
| name = str(request.tool_call.get("name") or "") | ||
| if not name or name not in self._hidden(request.state): | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as the For completeness: here |
||
| return None | ||
|
|
||
| tool_call_id = str(request.tool_call.get("id") or "missing_tool_call_id") | ||
| return ToolMessage( | ||
| content=(f"Error: Tool '{tool_name}' is deferred and has not been promoted yet. Call tool_search first to expose and promote this tool's schema, then retry."), | ||
| content=(f"Error: Tool '{name}' is deferred and has not been promoted yet. Call tool_search first to expose and promote this tool's schema, then retry."), | ||
| tool_call_id=tool_call_id, | ||
| name=tool_name, | ||
| name=name, | ||
| status="error", | ||
| ) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional — and a good thing to lock down.
tool_searchis appended after policy filtering, but it's only ever produced when MCP tools survive that filter:build_deferred_tool_setupreturns aNonetool when no deferred tool remains, and its catalog is derived from the already policy-filtered list, so it can never expose a tool the allowlist denied. Couplingtool_search's presence to surviving deferred tools also removes an upstream footgun where an allowlist that permitted MCP tools but omittedtool_searchwould strip the search tool and leave those deferred tools permanently unreachable.Locked the contract with two regression tests in
tests/test_deferred_tool_crosscontext.py—test_policy_denied_mcp_yields_no_tool_search_end_to_endandtest_tool_search_appended_after_policy_but_never_exposes_denied_tool— in 92d463e.