From 35656d6a9fe2ac76ddcf40335ede7361504ddf5b Mon Sep 17 00:00:00 2001 From: yetuge <182123929+yetuge@users.noreply.github.com> Date: Sun, 1 Nov 2026 08:08:08 +0800 Subject: [PATCH] fix(agents): ignore malformed entries in a serialized MCP routing index (#7751) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): ignore malformed entries in a serialized MCP routing index `McpRoutingMiddleware._normalize_index` documents that it accepts "arbitrary serialized routing data, not only the output of the builder", but two shapes escaped the coercion and raised instead: - an entry that is not a mapping (`str`, `list`, `None`, a number) reached `.get` and raised `AttributeError`; - a non-finite `priority` raised `OverflowError` from `int()` — JSON accepts `Infinity`/`-Infinity` literals, so a hand-edited or foreign index can carry one. The middleware is constructed per run, so either shape fails every model call in that process instead of the entry being ignored the way an entry with no keywords already is. Both now degrade to "not indexable": a non-mapping entry is skipped, and a priority that cannot be a finite int falls back to 0 (the existing value for `None`/non-numeric priorities). Valid entries are unchanged. Tests cover each rejected shape, the control shapes that already degraded, and the builder-shaped output that must stay identical. Dropping only the mapping guard fails the non-mapping cases; dropping only `OverflowError` fails the non-finite cases. * fix(mcp): handle infinite routing priorities in the builder --------- Co-authored-by: Willem Jiang --- .../middlewares/mcp_routing_middleware.py | 9 +- .../packages/harness/deerflow/tools/AGENTS.md | 5 + .../deerflow/tools/builtins/tool_search.py | 2 +- .../test_mcp_routing_index_normalization.py | 96 +++++++++++++++++++ 4 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 backend/tests/test_mcp_routing_index_normalization.py diff --git a/backend/packages/harness/deerflow/agents/middlewares/mcp_routing_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/mcp_routing_middleware.py index 34f67c0133..5db114830d 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/mcp_routing_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/mcp_routing_middleware.py @@ -59,9 +59,16 @@ class McpRoutingMiddleware(AgentMiddleware[AgentState]): name = str(raw_name) if not name: continue + # A serialized index is untrusted shape: anything that is not a + # mapping carries no priority or keywords, so skip it instead of + # failing on ``.get``. Middleware construction happens per run, and + # one malformed entry would take every model call down with it. + if not isinstance(raw_entry, Mapping): + continue try: priority = int(raw_entry.get("priority", 0)) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): + # OverflowError: int() refuses inf, which JSON accepts. priority = 0 raw_keywords = raw_entry.get("keywords") or [] if not isinstance(raw_keywords, Sequence) or isinstance(raw_keywords, (str, bytes)): diff --git a/backend/packages/harness/deerflow/tools/AGENTS.md b/backend/packages/harness/deerflow/tools/AGENTS.md index 66a8af3308..d76b6b35b5 100644 --- a/backend/packages/harness/deerflow/tools/AGENTS.md +++ b/backend/packages/harness/deerflow/tools/AGENTS.md @@ -138,6 +138,11 @@ opaque source links. This preserves retrieval evidence across the delegation boundary without placing provider IDs in model-visible text. Never reconstruct source records from the child's prose or replace them with fresh provider reads. +MCP routing priority coercion stays aligned between `tool_search._routing_priority` +and `McpRoutingMiddleware._normalize_index`: use `int(value)`, falling back to +`0` on `TypeError`, `ValueError`, or `OverflowError`. Cover both the real builder +and serialized-index paths when changing this contract. + `DeferredToolCatalog.search` reserves `keywords:` for opt-in literal, casefolded, whitespace-separated terms (256 query characters, 16 unique terms). Rank by term coverage across name/description, then name hits; ties preserve catalog order and diff --git a/backend/packages/harness/deerflow/tools/builtins/tool_search.py b/backend/packages/harness/deerflow/tools/builtins/tool_search.py index 206869d2a2..1daca5b95f 100644 --- a/backend/packages/harness/deerflow/tools/builtins/tool_search.py +++ b/backend/packages/harness/deerflow/tools/builtins/tool_search.py @@ -326,7 +326,7 @@ def _routing_priority(value: Any) -> int: # serialized data), so keep the two coercion rules in sync if either changes. try: return int(value) - except (TypeError, ValueError): + except (TypeError, ValueError, OverflowError): return 0 diff --git a/backend/tests/test_mcp_routing_index_normalization.py b/backend/tests/test_mcp_routing_index_normalization.py new file mode 100644 index 0000000000..2585ef5ff2 --- /dev/null +++ b/backend/tests/test_mcp_routing_index_normalization.py @@ -0,0 +1,96 @@ +"""A serialized MCP routing index must not break middleware construction. + +``McpRoutingMiddleware._normalize_index`` documents that it accepts "arbitrary +serialized routing data, not only the output of the builder". Two shapes the +loader can produce nevertheless escaped the coercion: + +- an entry that is not a mapping (``str`` / ``list`` / ``None``) raised + ``AttributeError`` on ``.get``; +- a non-finite ``priority`` raised ``OverflowError`` from ``int()`` — JSON + accepts ``Infinity``, so a hand-edited or foreign index can carry one. + +Construction happens once per run, so either shape turns every model call in +that process into an exception instead of the entry being ignored. +""" + +from __future__ import annotations + +import pytest +from langchain_core.messages import HumanMessage +from langchain_core.tools import tool + +from deerflow.agents.middlewares.mcp_routing_middleware import McpRoutingMiddleware +from deerflow.tools.builtins.tool_search import assemble_deferred_tools, build_mcp_routing_middleware +from deerflow.tools.mcp_metadata import tag_mcp_routing, tag_mcp_tool + +normalize = McpRoutingMiddleware._normalize_index + + +@pytest.mark.parametrize("bad_entry", ["just-a-string", ["keyword"], None, 7, 1.5]) +def test_non_mapping_entries_are_skipped(bad_entry): + """The malformed entry is dropped; its valid sibling still normalizes.""" + index = {"tool.bad": bad_entry, "tool.good": {"priority": 2, "keywords": ["search"]}} + + assert normalize(index) == {"tool.good": (2, ("search",))} + + +@pytest.mark.parametrize("priority", [float("inf"), float("-inf")]) +def test_non_finite_priority_falls_back_to_zero(priority): + """int() refuses infinity, and JSON accepts it as a literal.""" + assert normalize({"tool.a": {"priority": priority, "keywords": ["k"]}}) == {"tool.a": (0, ("k",))} + + +def test_nan_priority_falls_back_to_zero(): + assert normalize({"tool.a": {"priority": float("nan"), "keywords": ["k"]}}) == {"tool.a": (0, ("k",))} + + +def test_valid_entries_are_unchanged(): + """Control: the builder's own output still normalizes exactly as before.""" + index = { + "tool.a": {"priority": 3, "keywords": ["search", "find"]}, + "tool.b": {"priority": 0, "keywords": ["read"]}, + } + + assert normalize(index) == {"tool.a": (3, ("search", "find")), "tool.b": (0, ("read",))} + + +def test_entry_without_keywords_is_dropped(): + """Existing rule: an entry that cannot match text is not indexed.""" + assert normalize({"tool.a": {"priority": 1, "keywords": []}}) == {} + assert normalize({"tool.a": {"priority": 1, "keywords": "search"}}) == {} + + +def test_empty_name_is_dropped(): + assert normalize({"": {"priority": 1, "keywords": ["k"]}}) == {} + + +@pytest.mark.parametrize( + ("priority", "expected"), + [ + pytest.param(float("inf"), 0, id="positive-infinity"), + pytest.param(float("-inf"), 0, id="negative-infinity"), + pytest.param(float("nan"), 0, id="nan"), + pytest.param(None, 0, id="none"), + pytest.param("invalid", 0, id="invalid-string"), + pytest.param([], 0, id="invalid-list"), + pytest.param("7", 7, id="numeric-string"), + pytest.param(7.5, 7, id="finite-float"), + pytest.param(-2, -2, id="negative-int"), + pytest.param(True, 1, id="bool"), + ], +) +def test_builder_and_serialized_index_share_priority_coercion(priority, expected): + @tool + def routed_search(query: str) -> str: + """Search test data.""" + return query + + routing = {"mode": "prefer", "priority": priority, "keywords": ["search"]} + tag_mcp_routing(tag_mcp_tool(routed_search), routing) + tools, setup = assemble_deferred_tools([routed_search], enabled=True) + + middleware = build_mcp_routing_middleware(tools, setup, top_k=1) + + assert isinstance(middleware, McpRoutingMiddleware) + assert middleware._routing_index == normalize({"routed_search": routing}) == {"routed_search": (expected, ("search",))} + assert middleware._matched_names({"messages": [HumanMessage(content="search")]}) == ["routed_search"]