Repository navigation
bug(mcp): an unparseable tool is dropped to a log line, and every discovery failure is labelled "unreachable server" #17467
Description
Problem
When an MCP server advertises a tool we cannot parse, the tool silently does not exist. When a
server fails discovery for any reason, the operator is told it was unreachable — including
when it answered fine and its schema was the problem.
Nothing distinguishes we found no tools from we rejected the tools we found.
Evidence
skills/sync/mcp_client.py:133-140 — discover_tools():
for raw in raw_tools:
try:
tools.append(MCPToolDefinition.model_validate(raw))
except Exception as exc: # noqa: BLE001
logger.warning("MCPClient: could not parse tool %s: %s", raw.get("name"), exc)The rejected tool is absent from the returned list; the reason exists only in a backend log.
services/mcp_aggregation.py:66-67 — discover_tools_multi_server():
except Exception as exc: # noqa: BLE001
logger.warning("mcp_aggregation: skipping unreachable server %s: %s", uri, exc)One except Exception covers connect failures and schema failures alike, and hardcodes the
word "unreachable" into both.
Impact
An operator adds a server, sees four of its six tools, and has no way to find out why the other
two are missing — or is actively misinformed that a reachable server was unreachable. This is
the MEASUREMENT_DISCIPLINE.md rule (nothing found must be distinguishable from did not
look) broken at the tool-discovery seam.
Discovered During
A comparative research audit — see docs/research/confidence-gated-decision-loop-harness.md.
The reference work enumerates every tool it could not compile, with the reason, and exposes
them through a dedicated command; we are behind it here.
Acceptance Criteria
-
discover_tools()returns rejected tools with their reason alongside the accepted ones,
rather than dropping them -
discover_tools_multi_server()distinguishes a transport failure from a schema failure
in both the log message and what it returns - The reason reaches an operator-facing surface, sanitized — no internal filesystem paths
or raw tracebacks in the response field - A test asserts that a server advertising one good and one malformed tool yields one
accepted tool and one recorded rejection with its reason
Activity
docs(research): land two comparative audits whose 12 issues are alrea…
test(mcp): doubles that stub only discover_tools() answer the detaile…
fix(mcp): a rejected tool and an unreachable server are different ans…
mrveiss commentedon Sep 27, 2026
Acceptance evidence, verified against merged code — 4 of 4 met, with one deviation named
Closed by #17549. The PR body ticked all four; none of it was posted here. Re-verified against
origin/mainblobs.Rejected tools are returned with their reason alongside the accepted ones. — met, by a new method rather than by changing the old one.
skills/sync/mcp_client.py:197addsdiscover_tools_detailed() -> ToolDiscovery, whereToolDiscovery.rejected: List[RejectedTool](:59) andRejectedTool.reason: str(:51). The criterion nameddiscover_tools(); the delivery left that method's contract intact and pinned it —mcp_discovery_reports_rejections_17467_test.py:61, "the plain form still returns only accepted tools". Stating the deviation rather than ticking over it: every existing caller keeps its shape, and the rejection data is available to callers that ask for it.:201records what it replaces — the reason "existed only in a backend log line".discover_tools_multi_server()distinguishes a transport failure from a schema failure.:104test_a_transport_failure_is_labelled_transportand:114test_a_schema_failure_is_not_labelled_unreachable— the second is the one that matters, since collapsing a malformed tool into "server unreachable" is the defect.:132also holds that one failing server does not cost the others their tools.The reason reaches an operator-facing surface, sanitized — no internal filesystem paths.
:166test_a_rejection_reason_is_sanitized. The construction is what makes it hold rather than the filter:_rejection_reason(mcp_client.py:62) builds the message from "the validator's STRUCTURE -- which field, which" (:77) instead of interpolating the server's text, andredact_textmasksAuthorization: Bearer <token>(:67). So the sanitisation is by construction with a redactor behind it, not a blocklist.A test asserts that a server advertising one good and one malformed tool yields one accepted and one rejected.
:45test_one_good_and_one_malformed_tool_yields_both_outcomes, with:67a non-dict entry not aborting discovery and:82the name extractor never raising.Two controls worth naming because they stop the suite passing for the wrong reason:
:151a programming error is raised, not recorded as a server failure — so a bug in our own code cannot hide as "that server is broken"; and:208a client without the detailed form raises instead of yielding mocks, so the tests cannot pass against a stub that silently answers everything.What this closure does not cover
discover_tools()still discards rejections by design. A caller that wants them must move to the detailed form; nothing forces that migration.