From a1e6293f1b24a7b15fa4d7e3cba8112102d88277 Mon Sep 17 00:00:00 2001 From: Bernt Popp Date: Wed, 15 Jul 2026 10:36:39 +0200 Subject: [PATCH 1/3] fix(mcp): set isError on error envelopes and canonicalize error_code enum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP error carrier returned ToolResult(structured_content=...) without is_error=True, so a client branching on the protocol isError bit saw every failed call as a success. Verified against fastmcp 3.4.4 that the RETURN path keeps structuredContent while carrying is_error=True (only `raise` drops it); route every error through ToolResult(..., is_error=True). Also close error_code to the six-value canonical enum: internal_error -> internal (the behaviour gate rejects anything outside the closed set). And make validation errors actionable — build_error_envelope now emits field/field_errors derived ONLY from the request-validation `loc` path (this server's own schema parameter names, never caller/upstream prose), so an LLM can self-correct. Behaviour gate: 20 fail -> 0 fail (isError + actionable-error classes cleared). Vendor the byte-identical behaviour gate from genefoundry-router feat/mcp-contract-hardening-v1 (docs/conformance/behaviour.py @ 791363c). Refs genefoundry-router#73 #75 #76; refs #33 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019BYK7j7jsPXmBspMsDtfhH --- stringdb_link/mcp/envelope.py | 41 +- stringdb_link/mcp/error_passthrough.py | 45 +- tests/conformance/behaviour.py | 791 ++++++++++++++++++++++++ tests/conformance/test_behaviour_v1.py | 29 + tests/unit/test_response_envelope_v1.py | 10 +- 5 files changed, 899 insertions(+), 17 deletions(-) create mode 100644 tests/conformance/behaviour.py create mode 100644 tests/conformance/test_behaviour_v1.py diff --git a/stringdb_link/mcp/envelope.py b/stringdb_link/mcp/envelope.py index fc86462..63f3739 100644 --- a/stringdb_link/mcp/envelope.py +++ b/stringdb_link/mcp/envelope.py @@ -16,11 +16,14 @@ Mirrors the fleet's conformant exemplar (genereviews-link / clingen-link): errors are RETURNED as structured content (``success: false`` in-band) rather -than raised as an opaque ``ToolError`` text blob. The installed FastMCP 3.3.1 / -mcp SDK give no supported way to combine a wire-level ``isError: true`` with a -populated ``structuredContent`` on the return path (raising loses -``structuredContent`` entirely), so — like the rest of the fleet — we rely on -the in-band ``success`` flag rather than the wire ``isError`` bit. +than raised as an opaque ``ToolError`` text blob. Verified against the installed +FastMCP 3.4.4, ``ToolResult(structured_content=envelope, is_error=True)`` carries +BOTH the wire-level ``isError: true`` bit AND the populated ``structuredContent`` +frame on the return path — so the error carrier +(``stringdb_link.mcp.error_passthrough``) sets ``is_error=True`` and clients that +branch on the protocol ``isError`` bit see the failure, exactly as +Response-Envelope Standard v1 requires. (Only the ``raise`` path loses +``structuredContent``; the return path does not.) """ from __future__ import annotations @@ -79,15 +82,17 @@ def _untrusted_text_object_defs() -> dict[str, Any]: return defs -# Closed error-code enum (Response-Envelope Standard v1 §2), harmonized with the -# codes used fleet-wide (e.g. clingen-link's ``internal_error``). +# Closed error-code enum (Response-Envelope Standard v1 §2). Exactly the six +# canonical fleet codes — the behaviour gate (docs/conformance/behaviour.py) +# rejects anything outside this set. ``internal`` (not ``internal_error``) is the +# canonical spelling for a non-retryable server-side failure. ErrorCode = Literal[ "invalid_input", "not_found", "ambiguous_query", "upstream_unavailable", "rate_limited", - "internal_error", + "internal", ] @@ -132,7 +137,7 @@ class _ToolSpec: 413: ("invalid_input", False), 422: ("invalid_input", False), 429: ("rate_limited", True), - 500: ("internal_error", False), + 500: ("internal", False), 502: ("upstream_unavailable", True), 503: ("upstream_unavailable", True), 504: ("upstream_unavailable", True), @@ -145,7 +150,7 @@ class _ToolSpec: "ambiguous_query": "Narrow the query so it resolves to a single result.", "upstream_unavailable": "Retry with backoff; the upstream STRING API was unavailable.", "rate_limited": "Retry after backing off; the STRING API request rate was exceeded.", - "internal_error": "Retry once; if the error persists the request could not be completed.", + "internal": "Retry once; if the error persists the request could not be completed.", } # Fixed, server-authored, body-free error messages keyed by the classified code. @@ -160,7 +165,7 @@ class _ToolSpec: "ambiguous_query": "The query was ambiguous; narrow it to a single result.", "upstream_unavailable": "The upstream STRING API is unavailable.", "rate_limited": "The STRING API request rate was exceeded.", - "internal_error": "An internal error occurred while processing the request.", + "internal": "An internal error occurred while processing the request.", } @@ -274,6 +279,7 @@ def build_error_envelope( message: str, request_id: str, elapsed_ms: float, + field_errors: list[str] | None = None, ) -> dict[str, Any]: """Build the flat, in-band error frame (Response-Envelope Standard v1 §2). @@ -283,6 +289,12 @@ def build_error_envelope( :func:`sanitize_message` here as a belt-and-suspenders backstop so no forbidden control/zero-width/bidi/NUL code point reaches the caller by any error path, even if a caller passes an unsanitized string. + + ``field_errors`` (when supplied) names the offending input parameter(s). It is + derived ONLY from the request-validation ``loc`` path — i.e. this server's own + schema parameter names, never any caller/upstream prose — so an LLM can + self-correct (Response-Envelope Standard v1: an error must be actionable). Each + name is sanitized as a defensive backstop. """ error_code, retryable = classify_status(status_code) envelope: dict[str, Any] = { @@ -292,6 +304,11 @@ def build_error_envelope( "retryable": retryable, "recovery_action": _GENERIC_RECOVERY_ACTION[error_code], } + if field_errors: + cleaned = [sanitize_message(str(name))[:64] for name in field_errors if str(name)] + if cleaned: + envelope["field_errors"] = cleaned + envelope["field"] = cleaned[0] envelope["_meta"] = _augment_meta( {}, tool_name=tool_name, request_id=request_id, elapsed_ms=elapsed_ms ) @@ -304,7 +321,7 @@ def classify_status(status_code: int) -> tuple[ErrorCode, bool]: return _STATUS_CODE_MAP[status_code] if status_code >= 500: return "upstream_unavailable", True - return "internal_error", False + return "internal", False def reshape_output_schema( diff --git a/stringdb_link/mcp/error_passthrough.py b/stringdb_link/mcp/error_passthrough.py index be2b571..0fc82e7 100644 --- a/stringdb_link/mcp/error_passthrough.py +++ b/stringdb_link/mcp/error_passthrough.py @@ -56,6 +56,38 @@ def _fallback_message(response: httpx.Response) -> str: return envelope.safe_error_message(response.status_code) +def _validation_field_names(response: httpx.Response) -> list[str]: + """Extract offending parameter name(s) from a local request-validation body. + + ONLY the FastAPI/Starlette 4xx validation shape ``{"detail": [{"loc": [...]}]}`` + is parsed, and ONLY the ``loc`` leaf under ``body``/``query``/``path`` is read — + i.e. this server's own schema parameter names, never the caller-supplied value + (``input``), the human message (``msg``), or any upstream STRING error body + (which is HTTP 200 or lacks this shape). This is the one attacker-safe scalar in + the body: it lets the error name the parameter without echoing any prose. Any + parse failure yields no names (fail-closed). + """ + try: + payload = response.json() + except Exception: + return [] + detail = payload.get("detail") if isinstance(payload, dict) else None + if not isinstance(detail, list): + return [] + names: list[str] = [] + for item in detail: + loc = item.get("loc") if isinstance(item, dict) else None + if not isinstance(loc, list) or not loc: + continue + if loc[0] in ("body", "query", "path") and len(loc) > 1: + leaf = loc[-1] + else: + leaf = loc[-1] + if isinstance(leaf, str) and leaf not in names: + names.append(leaf) + return names + + def _build_error_envelope_from_exception( tool_name: str, exc: Exception, elapsed_ms: float ) -> dict[str, Any]: @@ -65,7 +97,7 @@ def _build_error_envelope_from_exception( if response is None: # No HTTP response anywhere in the chain: a connection-level failure or a # non-HTTP bug (e.g. the image route's binary body defeating the JSON - # provider). Route it through a non-retryable internal_error with a + # provider). Route it through a non-retryable internal error with a # generic message (mask_error_details posture — no internal text leaks). return envelope.build_error_envelope( tool_name, @@ -74,12 +106,16 @@ def _build_error_envelope_from_exception( request_id=request_id, elapsed_ms=elapsed_ms, ) + field_errors: list[str] = [] + if response.status_code in (400, 422): + field_errors = _validation_field_names(response) return envelope.build_error_envelope( tool_name, status_code=response.status_code, message=_fallback_message(response), request_id=request_id, elapsed_ms=elapsed_ms, + field_errors=field_errors, ) @@ -117,7 +153,12 @@ async def run_with_structured_errors(arguments: dict[str, Any]) -> ToolResult: except Exception as exc: # all failures become the flat error frame elapsed_ms = (time.perf_counter() - start) * 1000 error_envelope = _build_error_envelope_from_exception(tool_name, exc, elapsed_ms) - return ToolResult(structured_content=error_envelope) + # is_error=True gives the wire-level MCP ``isError`` bit alongside the + # structured envelope (Response-Envelope Standard v1). Verified against + # fastmcp 3.4.4: the RETURN path keeps structuredContent (only ``raise`` + # would drop it). Without this a client branching on ``isError`` sees a + # failed call as a success. + return ToolResult(structured_content=error_envelope, is_error=True) elapsed_ms = (time.perf_counter() - start) * 1000 raw = result.structured_content if isinstance(result.structured_content, dict) else {} diff --git a/tests/conformance/behaviour.py b/tests/conformance/behaviour.py new file mode 100644 index 0000000..0e60544 --- /dev/null +++ b/tests/conformance/behaviour.py @@ -0,0 +1,791 @@ +"""Behaviour Conformance v1 — the contract an agent relies on, tested against a live server. + +Self-contained (httpx only). Vendored byte-identical into every -link repo's tests/conformance/ +and run against that repo's own container in CI: + + python -m genefoundry_router.behaviour http://127.0.0.1:8000 --name gtex-link + +Exit code: 0 conformant, 1 non-conformant, 2 transport/probe error. + +WHAT THIS GATES, AND WHY IT IS SCHEMA-DERIVED +--------------------------------------------- +The fleet's confirmed defects are not 82 unrelated bugs. They are three bugs repeated, and all +three are already forbidden — by the fleet's own Response-Envelope Standard or by the MCP spec. +Nothing enforced them. This does. + + 1. THE SILENTLY-EMPTY FILTER. An unrecognised filter value matches nothing and returns + `success:true, total_count:0` — indistinguishable from "the data genuinely has none". + Forbidden by Response-Envelope v1.1: "silent omission is not compliant." + 2. THE LYING total/truncated. `total` set to the page size, `truncated:false`, while the + server itself knows there are more. + 3. THE ERROR AN LLM CANNOT ACT ON. A missing argument answered with `not_found` / + "The requested tool is not available" — telling the model the tool does not exist. + +The probes are derived from the server's OWN advertised schema. There is no per-repo config file +to write, and none to forget: every declared enum is probed, so a tool is gated the day it ships. +That is deliberate. A hand-maintained list of probes is the same bug one level up — whoever +forgets to add a row ships an ungated tool while the suite still reports PASS. + +The coupling to the Tool-Schema Documentation Standard is the point. This gate builds a valid +call out of each parameter's `examples`, so the artifact that teaches a model how to call the +tool is the same artifact that proves the tool rejects a bad call. A tool without examples cannot +be probed — and is reported as UNGATED, never as passing. Under-documentation shows up as lost +coverage, never as a green tick. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from dataclasses import dataclass, field +from typing import Any + +import httpx + +PROTOCOL = "2025-06-18" +HEADERS = { + "Accept": "application/json, text/event-stream", + "Content-Type": "application/json", +} + +# Response-Envelope Standard v1: "error_code is a closed enum, harmonized with codes already +# used in the fleet". Anything outside this set is a violation, however sensible it reads. +ERROR_CODES = { + "invalid_input", + "not_found", + "ambiguous_query", + "upstream_unavailable", + "rate_limited", + "internal", +} + +# A value no upstream vocabulary can legitimately contain. +SENTINEL = "__gf_conformance_no_such_value__" +BOGUS_ARG = "__gf_conformance_no_such_arg__" + +# Errors that mean "the world is broken", not "the server is wrong". A probe that hits one of +# these is skipped, never passed — an unreachable upstream must not silently buy a green tick. +INCONCLUSIVE = {"upstream_unavailable", "rate_limited", "internal"} + +# Tools that orient a client rather than query data (Tool-Naming Standard ops/meta carve-out). +META_TOOLS = ("capabilit", "health", "diagnost", "warmup", "help", "quickstart") + +# Free-text inputs, where a nonsense value returning zero rows is HONEST, not a defect. The +# silent-empty probe below cannot distinguish "a filter over a closed vocabulary" from "a search +# box" by type — both are optional strings — so it declines to judge these by name. +# +# This deliberately trades coverage for precision. A false accusation is far more costly than a +# missed one here: it sends a maintainer chasing a phantom and teaches them to distrust the gate. +# The failure mode of this list is a LOUD one (a free-text param not listed here produces a visible +# FAIL that a human can dismiss), never a silent pass. +FREE_TEXT = ( + "query", + "text", + "search", + "keyword", + "question", + "topic", + "contains", + "term", + "filters", +) + + +@dataclass +class Report: + base_url: str + name: str + passed: list[str] = field(default_factory=list) + failed: list[str] = field(default_factory=list) + ungated: list[str] = field(default_factory=list) + skipped: list[str] = field(default_factory=list) + + def check(self, label: str, ok: bool, detail: str = "") -> bool: + if ok: + self.passed.append(label) + else: + self.failed.append(f"{label} — {detail}") + return ok + + def ungate(self, label: str, why: str) -> None: + """A tool this gate could not verify. Counts AGAINST conformance (B7). + + The earlier version of this probe recorded these as skips and still certified the server + CONFORMANT. That made its central promise a lie: `gtex-link`, with 8 of its 9 tools + unprobed for want of `examples`, would have passed while both of its confirmed HIGH + defects went untested. A server that does not document its inputs cannot be verified, and + must not be told it passed. + """ + self.ungated.append(f"{label} — {why}") + + def skip(self, label: str, why: str) -> None: + """Genuinely inconclusive — an unreachable upstream. Does not count against conformance.""" + self.skipped.append(f"{label} — {why}") + + @property + def conformant(self) -> bool: + return not self.failed and not self.ungated + + +class Probe: + """A minimal Streamable-HTTP MCP client. No SDK, so the gate tests the wire, not a wrapper.""" + + def __init__(self, base_url: str, timeout: float = 60.0) -> None: + self.url = base_url.rstrip("/") + "/mcp" + self.client = httpx.Client(timeout=timeout, follow_redirects=False) + self.session: str | None = None + self.server_info: dict[str, Any] = {} + self._id = 0 + + def _rpc(self, method: str, params: dict[str, Any] | None = None) -> dict[str, Any]: + self._id += 1 + headers = dict(HEADERS) + if self.session: + headers["mcp-session-id"] = self.session + body: dict[str, Any] = {"jsonrpc": "2.0", "id": self._id, "method": method} + if params is not None: + body["params"] = params + resp = self.client.post(self.url, json=body, headers=headers) + if not self.session: + self.session = resp.headers.get("mcp-session-id") + text = resp.text.strip() + if "text/event-stream" in resp.headers.get("content-type", ""): + frames = [ln[5:].strip() for ln in text.splitlines() if ln.startswith("data:")] + text = frames[-1] if frames else "{}" + message = json.loads(text or "{}") + if "error" in message: + raise ProtocolError(message["error"]) + return dict(message.get("result") or {}) + + def initialize(self) -> dict[str, Any]: + result = self._rpc( + "initialize", + { + "protocolVersion": PROTOCOL, + "capabilities": {}, + "clientInfo": {"name": "gf-behaviour-probe", "version": "1.0.0"}, + }, + ) + self.server_info = dict(result.get("serverInfo") or {}) + return result + + def list_tools(self) -> list[dict[str, Any]]: + tools: list[dict[str, Any]] = [] + cursor: str | None = None + while True: + result = self._rpc("tools/list", {"cursor": cursor} if cursor else {}) + tools.extend(result.get("tools", [])) + cursor = result.get("nextCursor") + if not cursor: + return tools + + def call(self, tool: str, args: dict[str, Any]) -> dict[str, Any]: + return self._rpc("tools/call", {"name": tool, "arguments": args}) + + +class ProtocolError(RuntimeError): + """A JSON-RPC error frame — i.e. the server chose NOT to answer with a tool result.""" + + def __init__(self, error: dict[str, Any]) -> None: + self.error = error + super().__init__(json.dumps(error)[:200]) + + +class WrongServerError(RuntimeError): + """The server at this URL is not the one we were asked to gate. Never report; always abort.""" + + +# --------------------------------------------------------------------------- envelope reading + + +def envelope(result: dict[str, Any]) -> dict[str, Any]: + """The fleet's flat envelope, from structuredContent or its TextContent mirror.""" + structured = result.get("structuredContent") + if isinstance(structured, dict): + return structured + for block in result.get("content") or []: + if block.get("type") == "text": + try: + parsed = json.loads(block.get("text") or "") + except (TypeError, ValueError): + continue + if isinstance(parsed, dict): + return parsed + return {} + + +def rows(env: dict[str, Any]) -> list[Any] | None: + """The result collection, whatever the backend calls it (`results`, `genes`, `variants`…). + + The longest collection of objects in the envelope. Backends name this field differently and + the gate must not care; the Response-Envelope Standard fixes the frame, not the payload key. + + GROUPED payloads count too. An adversarial review found the first version of this blind to + them: orphanet's `map_cross_ontology` returns `mappings` as a dict-of-lists + (`{"OMIM": [...], "ICD10": [...]}`), not a list. The row-finder saw no list, returned None, + and EVERY filter probe on that tool silently skipped — so a live silently-empty filter + (`prefixes=["__BOGUS__"]` -> `success:true, count:0`) sailed through a gate reporting + "0 failures, 0 UNGATED". A detector that cannot see a payload shape cannot gate it. + """ + # 1. A GROUPED COLLECTION — a non-`_` key whose value is a dict of lists, + # e.g. mappings: {"OMIM": [...], "ICD10": [...]} — IS a collection, and needs no count. + # It cannot be confused with a scalar record field (a record's dict field, like + # `definition: {id, label, ...}`, has non-list values). Checked FIRST and without the + # count guard below, because an empty grouped dict `mappings: {}` is an emptied collection, + # not a record — and reading it as "no collection" is exactly how hpo-link's + # `map_cross_ontology(prefixes=["__nonsense__"]) -> mappings:{}, success:true` slipped past + # an earlier gate that then reported CONFORMANT over a confirmed silent-empty (Codex found + # it). A non-empty grouped dict must be all-lists; an empty one reads as an empty collection. + grouped = _grouped_collection(env) + if grouped is not None: + return grouped + + # 2. Otherwise, a top-level list of objects — but a bare list is ambiguous. A COLLECTION + # declares how many things it holds (Response-Envelope v1: "Always populate + # _meta.pagination: total_count, has_more"); a RECORD does not. Without this count guard the + # probe reads hpo-link's `get_term` — a single record whose `children` field holds 23 + # sub-terms — as a 23-row collection, and then `fields=[...]` (a projection doing its job) + # and `response_mode=minimal` (correctly omitting a record's optional detail) both read as + # payload destruction. Two false accusations against a server doing nothing wrong. + if count_of(env) is None: + return None + + best: list[Any] | None = None + for key, value in env.items(): + if key.startswith("_") or not isinstance(value, list): + continue + if value and not isinstance(value[0], dict): + continue + if best is None or len(value) > len(best): + best = value + return best + + +def _grouped_collection(env: dict[str, Any]) -> list[Any] | None: + """Flatten the largest dict-of-lists in the envelope, or None if there is none. + + Returns `[]` for an empty grouped dict — an emptied collection, not the absence of one. + """ + best: list[Any] | None = None + for key, value in env.items(): + if key.startswith("_") or not isinstance(value, dict): + continue + # A non-empty grouped collection is lists all the way across; an empty dict qualifies + # (it is an emptied collection). A dict with any non-list value is a record, not a group. + if value and not all(isinstance(branch, list) for branch in value.values()): + continue + flat = [item for branch in value.values() if isinstance(branch, list) for item in branch] + if best is None or len(flat) > len(best): + best = flat + return best + + +def count_of(env: dict[str, Any]) -> int | None: + """How many records this response says it is carrying. Absent on a single-record tool.""" + pagination = (env.get("_meta") or {}).get("pagination") or {} + for source in (pagination, env): + for key in ("total_count", "total", "count", "returned", "found_count"): + value = source.get(key) + if isinstance(value, int): + return value + return None + + +def total_of(env: dict[str, Any]) -> int | None: + pagination = (env.get("_meta") or {}).get("pagination") or {} + for source in (pagination, env): + for key in ("total_count", "total"): + value = source.get(key) + if isinstance(value, int): + return value + return None + + +def more_flag(env: dict[str, Any]) -> bool | None: + pagination = (env.get("_meta") or {}).get("pagination") or {} + for source in (pagination, env): + for key in ("has_more", "truncated"): + value = source.get(key) + if isinstance(value, bool): + return value + return None + + +def is_error_envelope(env: dict[str, Any]) -> bool: + return env.get("success") is False or bool(env.get("error_code")) + + +def properties(tool: dict[str, Any]) -> dict[str, dict[str, Any]]: + return dict((tool.get("inputSchema") or {}).get("properties") or {}) + + +def _branches(prop: dict[str, Any]) -> list[dict[str, Any]]: + """The property itself plus every `anyOf` branch — pydantic renders `X | None` as anyOf.""" + out = [prop] + out.extend(b for b in prop.get("anyOf") or [] if isinstance(b, dict)) + return out + + +def enum_of(prop: dict[str, Any]) -> list[Any] | None: + """The closed vocabulary this property constrains its value to, scalar OR per-item. + + `items.enum` matters as much as `enum`: a `list[Literal[...]]` parameter is a closed + vocabulary too, and the first version of this gate looked only at the scalar case. + """ + for branch in _branches(prop): + if isinstance(branch.get("enum"), list): + return list(branch["enum"]) + items = branch.get("items") + if isinstance(items, dict) and isinstance(items.get("enum"), list): + return list(items["enum"]) + return None + + +def is_stringy(prop: dict[str, Any]) -> bool: + """True if the property takes a string, or an ARRAY of strings. + + The array case is not a nicety. orphanet's `map_cross_ontology.prefixes` is a `list[str]` + over a closed vocabulary, and `prefixes=["__BOGUS__"]` returns `success:true, count:0` on the + live server — the primary bug, shipping today, invisible to a gate that only probed scalars. + """ + for branch in _branches(prop): + if branch.get("type") == "string": + return True + items = branch.get("items") + if ( + branch.get("type") == "array" + and isinstance(items, dict) + and items.get("type") == "string" + ): + return True + return False + + +def takes_array(prop: dict[str, Any]) -> bool: + return any(b.get("type") == "array" for b in _branches(prop)) + + +def perturbed(prop: dict[str, Any]) -> Any: + """The sentinel, wrapped to match the property's shape.""" + return [SENTINEL] if takes_array(prop) else SENTINEL + + +def valid_args(tool: dict[str, Any]) -> dict[str, Any] | None: + """Build a valid call from the schema's own `examples`. None if the schema doesn't say how. + + This is the fixture the Tool-Schema Documentation Standard (S2) exists to provide. A tool + whose required parameters carry no example cannot be exercised here, and is reported UNGATED. + """ + args: dict[str, Any] = {} + props = properties(tool) + for name in (tool.get("inputSchema") or {}).get("required") or []: + examples = (props.get(name) or {}).get("examples") + if not examples: + return None + args[name] = examples[0] + return args + + +# --------------------------------------------------------------------------- the checks + + +def _record_error_frame( + rep: Report, tool: str, result: dict[str, Any], env: dict[str, Any] +) -> None: + """Every error we provoke is also evidence for the two universal envelope invariants.""" + code = env.get("error_code") + + # Response-Envelope v1: "isError: true is REQUIRED so clients surface the error to the model + # for self-correction." A returned dict never sets isError; the backend must return a + # ToolResult(is_error=True). This is the fleet's most widespread protocol violation. + rep.check( + f"{tool}: an error envelope carries MCP isError:true", + result.get("isError") is True, + f"success={env.get('success')!r} error_code={code!r} but isError={result.get('isError')!r}", + ) + if code is not None: + rep.check( + f"{tool}: error_code is in the closed enum", + code in ERROR_CODES, + f"{code!r} is not one of {sorted(ERROR_CODES)}", + ) + + +def check_argument_error(rep: Report, probe: Probe, tool: dict[str, Any]) -> None: + """An unusable argument MUST produce an actionable tool error that names the parameter. + + MCP 2025-11-25 (SEP-1303) moved input-validation failures OUT of protocol errors and INTO + tool-execution errors precisely so the model can self-correct: "Tool Execution Errors contain + actionable feedback that language models can use to self-correct and retry with adjusted + parameters." + """ + name = tool["name"] + label = f"{name}: unknown argument" + try: + result = probe.call(name, {BOGUS_ARG: "x"}) + except ProtocolError as exc: + rep.check( + f"{label} is a tool error, not a JSON-RPC protocol error", + False, + f"got JSON-RPC {exc.error.get('code')} — the model cannot self-correct from this " + "(MCP 2025-11-25 SEP-1303)", + ) + return + + env = envelope(result) + if not is_error_envelope(env): + rep.check(label + " is rejected", False, "the server ACCEPTED an unknown argument") + return + + _record_error_frame(rep, name, result, env) + + code = env.get("error_code") + rep.check( + f"{label} → invalid_input (never not_found)", + code == "invalid_input", + f"got {code!r} — {str(env.get('message'))[:80]!r}. 'not_found' tells the model the tool " + "does not exist, so it will never call it again.", + ) + + # "message MUST be specific and actionable — tell the model how to fix the call." + # + # The parameter may be named in the PROSE, or carried in the envelope's structured error + # fields. Both are actionable, and the Response-Envelope error frame explicitly provides + # `field` / `allowed_values` / `hint` for exactly this. hpo-link answers with + # "Valid argument names are listed in allowed_values" plus a populated `allowed_values` — a + # model can act on that perfectly well, and an earlier version of this check called it a + # failure purely because it only read the prose. Judging a server by the surface you happen + # to look at is the same mistake as the audit's tautological $ref check. + prose = " ".join(str(env.get(k) or "") for k in ("message", "recovery_action", "hint")) + structured = env.get("allowed_values") or env.get("field") or env.get("field_errors") + names_something = ( + BOGUS_ARG in prose or any(p in prose for p in properties(tool)) or bool(structured) + ) + rep.check( + f"{label} names the offending or the valid parameters", + names_something, + f"{str(env.get('message'))[:90]!r} names no parameter, and the envelope carries no " + "`field`/`allowed_values` — the model has nothing to act on", + ) + + +def check_declared_enums( + rep: Report, probe: Probe, tool: dict[str, Any], base: dict[str, Any] +) -> None: + """A value outside a DECLARED enum MUST be rejected — never silently matched to nothing.""" + name = tool["name"] + for prop_name, prop in properties(tool).items(): + values = enum_of(prop) + if not values or not is_stringy(prop): + continue + label = f"{name}.{prop_name}: out-of-enum value" + try: + result = probe.call(name, {**base, prop_name: perturbed(prop)}) + except ProtocolError as exc: + rep.check( + f"{label} is a tool error, not a protocol error", + False, + f"JSON-RPC {exc.error.get('code')}", + ) + continue + + env = envelope(result) + if is_error_envelope(env): + _record_error_frame(rep, name, result, env) + rep.check( + f"{label} → invalid_input", + env.get("error_code") == "invalid_input", + f"got {env.get('error_code')!r}", + ) + continue + + # Accepted. The only defensible outcome now is that it changed nothing; but a filter that + # accepts a value it does not understand and returns zero rows is the silent-empty bug. + found = rows(env) + rep.check( + f"{label} MUST be rejected, not silently matched to nothing", + False, + f"success={env.get('success')!r}, {len(found) if found is not None else '?'} rows, " + f"total={total_of(env)!r} — indistinguishable from 'the data has none'", + ) + + +def check_silent_empty_filter( + rep: Report, probe: Probe, tool: dict[str, Any], base: dict[str, Any], control: list[Any] +) -> None: + """An OPTIONAL string filter with no declared enum must not silently zero the result set. + + This is the undeclared-enum detector, and it is why the standard can enforce a rule a static + schema check cannot see. `clinvar-link/get_variants_by_gene.classification` is a bare string + whose real vocabulary is closed: 'likely_pathogenic' returns 559 BRCA1 variants, ClinVar's own + published wording 'Likely pathogenic' returns 0 with success:true. + + A free-text query parameter legitimately returns nothing for a nonsense value — which is why + this probes only OPTIONAL parameters, against a control call already proven to return rows, and + skips anything named like a search box (see FREE_TEXT). + """ + name = tool["name"] + required = set((tool.get("inputSchema") or {}).get("required") or []) + for prop_name, prop in properties(tool).items(): + if prop_name in required or prop_name in base or enum_of(prop) or not is_stringy(prop): + continue + if prop_name in ("cursor", "request_id", "next_cursor", "correlation_id", "session_id"): + continue + if any(marker in prop_name for marker in FREE_TEXT): + continue + label = f"{name}.{prop_name}: unrecognised filter value" + try: + result = probe.call(name, {**base, prop_name: perturbed(prop)}) + except ProtocolError: + continue + + env = envelope(result) + if is_error_envelope(env): + _record_error_frame(rep, name, result, env) + rep.passed.append(f"{label} → rejected ({env.get('error_code')})") + continue + + found = rows(env) + if found: + rep.passed.append(f"{label} → did not zero the result set") + continue + + # `found` is now [] OR None. Both are the bug, and conflating None with "fine" is how the + # first version of this probe passed orphanet's `map_cross_ontology.prefixes`: the bogus + # value emptied the grouped `mappings` object entirely, so the row-finder saw no collection + # at all and returned None — which the check then read as "did not zero the result set". + # The CONTROL call already proved this tool returns rows. If the perturbed call has no rows + # — whether an empty collection or no collection at all — the filter destroyed them. + + rep.check( + label + " silently matched nothing", + False, + f"the control call returned {len(control)} rows; this returned 0 with " + f"success={env.get('success')!r} and no error — indistinguishable from 'the data " + "genuinely has none'. Either declare the closed vocabulary as an `enum` " + "(TOOL-SCHEMA-DOCUMENTATION-STANDARD S4), or reject the unrecognised value with " + "invalid_input/not_found naming it. A zero-row success is not an acceptable answer to " + "a value the server did not understand.", + ) + + +def check_pagination_honesty(rep: Report, tool: str, env: dict[str, Any], found: list[Any]) -> None: + """A partial page must say so.""" + total = total_of(env) + if total is None: + return + more = more_flag(env) + if len(found) < total: + rep.check( + f"{tool}: a partial page declares has_more/truncated", + more is True, + f"returned {len(found)} of total {total} but has_more/truncated is {more!r} — the " + "model will conclude it has seen everything", + ) + + +def check_total_is_not_the_page_size( + rep: Report, probe: Probe, tool: dict[str, Any], base: dict[str, Any] +) -> None: + """`total` MUST be invariant under `limit`. (B4) + + THIS CANNOT BE DECIDED FROM ONE RESPONSE, which is why the first version of this gate missed + it. litvar's `search_genetic_variants` returns `returned=25, total=25, truncated=false` — a + perfectly self-consistent page. Nothing in it is detectably wrong. Only a second call exposes + the lie: + + limit=5 -> returned=5, total=5, truncated=false + limit=25 -> returned=25, total=25, truncated=false + limit=100 -> returned=100, total=100, truncated=false + + `total` is echoing `limit`. The true BRCA1 count, which the SAME SERVER reports from a sibling + tool, is 13,264. An agent reads `total=25, truncated=false` and concludes it has seen every + BRCA1 variant LitVar knows. It has seen 0.2% of them. + + `total` is a property of the result set, not of the page. If it moves when `limit` moves, it + is fabricated. + """ + name = tool["name"] + limit_prop = properties(tool).get("limit") or {} + if not limit_prop: + return + + totals: dict[int, int | None] = {} + for limit in (2, 4): + try: + result = probe.call(name, {**base, "limit": limit}) + except ProtocolError: + return + env = envelope(result) + if is_error_envelope(env): + return + totals[limit] = total_of(env) + + small, large = totals.get(2), totals.get(4) + if small is None or large is None: + return + rep.check( + f"{name}: total is invariant under limit", + small == large, + f"limit=2 reported total={small}, limit=4 reported total={large}. `total` is tracking the " + "page size, not the result set — so it is fabricated, and an agent reading it concludes it " + "has seen everything.", + ) + + +def check_response_modes( + rep: Report, probe: Probe, tool: dict[str, Any], base: dict[str, Any], control: list[Any] +) -> None: + """`response_mode` narrows a payload; it MUST NOT destroy it. + + Response-Envelope v1 defines `minimal` as "the mandatory envelope plus stable identifiers, + omitting all optional record detail" — identifiers are explicitly retained. A mode that turns + N records into zero is a silent-empty by another name: orphanet-link/get_disease_genes at + response_mode=minimal discards the entire gene list and still returns success:true. + """ + name = tool["name"] + modes = enum_of(properties(tool).get("response_mode") or {}) or [] + for mode in modes: + label = f"{name}: response_mode={mode!r} preserves the payload" + try: + result = probe.call(name, {**base, "response_mode": mode}) + except ProtocolError: + continue + env = envelope(result) + if is_error_envelope(env): + _record_error_frame(rep, name, result, env) + continue + found = rows(env) + rep.check( + label, + bool(found), + f"the default call returned {len(control)} records; response_mode={mode!r} returned " + f"{0 if found is None else len(found)} with success={env.get('success')!r}", + ) + + +# --------------------------------------------------------------------------- driver + + +def run_probe(base_url: str, *, expected_name: str, timeout: float = 60.0) -> Report: + rep = Report(base_url.rstrip("/"), expected_name) + probe = Probe(base_url, timeout) + probe.initialize() + + # Identity FIRST, and fatally. A developer gating a local build will sooner or later point + # this at a port some sibling container is squatting on, and every finding below would then + # describe a DIFFERENT SERVER while looking entirely plausible. That happened for real during + # the litvar-link fix: a stray clinvar-link on :8011 produced a confident, well-formed, wrong + # litvar report. A probe that cannot prove what it is talking to is worse than no probe. + served = probe.server_info.get("name") + if served != expected_name: + raise WrongServerError( + f"expected serverInfo.name {expected_name!r} at {base_url}, but the server there " + f"says it is {served!r}. Refusing to report findings against the wrong server." + ) + + tools = probe.list_tools() + rep.check("tools/list returns at least one tool", bool(tools), "no tools") + + for tool in tools: + name = tool["name"] + + # Every tool, even a no-argument one, must reject an unknown argument usefully. + check_argument_error(rep, probe, tool) + + if any(marker in name for marker in META_TOOLS): + continue + + base = valid_args(tool) + if base is None: + rep.ungate( + f"{name}: dynamic probes", + "UNGATED — a required parameter carries no `examples`, so no valid call can be " + "constructed and NOTHING about this tool's behaviour is verified " + "(TOOL-SCHEMA-DOCUMENTATION-STANDARD S2)", + ) + continue + + try: + control_result = probe.call(name, base) + except ProtocolError as exc: + rep.check(f"{name}: its own documented example is callable", False, str(exc)[:100]) + continue + + control_env = envelope(control_result) + if is_error_envelope(control_env): + code = control_env.get("error_code") + _record_error_frame(rep, name, control_result, control_env) + if code in INCONCLUSIVE: + rep.skip(f"{name}: dynamic probes", f"upstream inconclusive ({code})") + else: + rep.check( + f"{name}: its own documented example is accepted", + False, + f"the call built from the schema's own `examples` was rejected: {code!r} " + f"{str(control_env.get('message'))[:70]!r}", + ) + continue + + check_declared_enums(rep, probe, tool, base) + + control = rows(control_env) + if control: + check_pagination_honesty(rep, name, control_env, control) + check_total_is_not_the_page_size(rep, probe, tool, base) + check_silent_empty_filter(rep, probe, tool, base, control) + check_response_modes(rep, probe, tool, base, control) + else: + rep.skip( + f"{name}: filter probes", + "the control call returned no rows, so a zero result proves nothing", + ) + + return rep + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description="Behaviour Conformance v1 probe") + parser.add_argument("base_url") + parser.add_argument("--name", required=True, help="expected serverInfo.name") + parser.add_argument("--timeout", type=float, default=60.0) + parser.add_argument("--quiet", action="store_true", help="only show failures and skips") + args = parser.parse_args(argv) + + try: + rep = run_probe(args.base_url, expected_name=args.name, timeout=args.timeout) + except WrongServerError as exc: + print(f"WRONG SERVER: {exc}", file=sys.stderr) + return 2 + except (httpx.HTTPError, ProtocolError) as exc: + print(f"TRANSPORT ERROR: {exc}", file=sys.stderr) + return 2 + + if not args.quiet: + for line in rep.passed: + print(f" PASS {line}") + for line in rep.skipped: + print(f" SKIP {line}") + for line in rep.ungated: + print(f" UNGATED {line}") + for line in rep.failed: + print(f" FAIL {line}") + + verdict = "CONFORMANT" if rep.conformant else "NON-CONFORMANT" + print( + f"\n{verdict}: {rep.name} @ {rep.base_url} " + f"({len(rep.passed)} pass, {len(rep.failed)} fail, {len(rep.ungated)} UNGATED, " + f"{len(rep.skipped)} inconclusive)" + ) + if rep.ungated: + print( + f"{len(rep.ungated)} tool(s) could not be verified at all and therefore FAIL. Document " + "their required parameters with `examples` so they can be probed. An unverifiable tool " + "must never be certified." + ) + return 0 if rep.conformant else 1 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/conformance/test_behaviour_v1.py b/tests/conformance/test_behaviour_v1.py new file mode 100644 index 0000000..5d0813b --- /dev/null +++ b/tests/conformance/test_behaviour_v1.py @@ -0,0 +1,29 @@ +"""Behaviour Conformance v1 gate (vendored). + +Skips unless CONFORMANCE_MCP_URL points at a running server. The conformance.yml workflow sets +it after `make docker-up`; a local `make ci-local` skips it. + +This is the gate for the three recurring behavioural bugs (silent-empty filter, lying +total/truncated, error an LLM cannot act on). See docs/conformance/behaviour.py for what it +asserts and why it derives every probe from the server's own schema. +""" + +from __future__ import annotations + +import os + +import pytest + +from .behaviour import run_probe + +MCP_URL = os.environ.get("CONFORMANCE_MCP_URL") +EXPECTED_NAME = os.environ.get("CONFORMANCE_NAME", "REPLACE-ME-link") + + +@pytest.mark.skipif(not MCP_URL, reason="set CONFORMANCE_MCP_URL to run the live probe") +def test_mcp_behaviour_standard_v1() -> None: + report = run_probe(MCP_URL, expected_name=EXPECTED_NAME) + # UNGATED counts against conformance (B7): a tool whose required parameters carry no + # `examples` cannot be probed at all, and an unverifiable tool must never be certified. + problems = report.failed + report.ungated + assert report.conformant, "non-conformant:\n " + "\n ".join(problems) diff --git a/tests/unit/test_response_envelope_v1.py b/tests/unit/test_response_envelope_v1.py index a26d38b..c76ec22 100644 --- a/tests/unit/test_response_envelope_v1.py +++ b/tests/unit/test_response_envelope_v1.py @@ -133,6 +133,9 @@ async def test_upstream_failure_is_flat_retryable_error_envelope(facade: Any) -> body = result.structured_content assert body is not None, "errors must carry structured_content, not only content[].text" + # The wire-level MCP isError bit MUST be set so a client branching on isError + # sees the failure (Response-Envelope Standard v1). + assert result.is_error is True assert body["success"] is False assert body["error_code"] == "upstream_unavailable" assert body["retryable"] is True @@ -163,8 +166,9 @@ async def test_internal_failure_is_non_retryable_internal_error(facade: Any) -> body = result.structured_content assert body is not None + assert result.is_error is True assert body["success"] is False - assert body["error_code"] == "internal_error" + assert body["error_code"] == "internal" assert body["retryable"] is False assert body["recovery_action"] assert body["_meta"]["unsafe_for_clinical_use"] is True @@ -197,7 +201,7 @@ async def test_binary_image_tool_degrades_to_structured_internal_error(facade: A body = result.structured_content assert body is not None assert body["success"] is False - assert body["error_code"] == "internal_error" + assert body["error_code"] == "internal" assert body["retryable"] is False assert body["_meta"]["tool"] == "get_network_image" assert body["_meta"]["unsafe_for_clinical_use"] is True @@ -252,7 +256,7 @@ def test_build_error_envelope_500_is_non_retryable_internal() -> None: request_id="r3", elapsed_ms=1.0, ) - assert env["error_code"] == "internal_error" + assert env["error_code"] == "internal" assert env["retryable"] is False From e9aa3be3ece858a4085ecbb34a2060eb49ec319c Mon Sep 17 00:00:00 2001 From: Bernt Popp Date: Wed, 15 Jul 2026 12:11:29 +0200 Subject: [PATCH 2/3] feat(mcp): document schemas, cut outputSchema, fix the audited contract defects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complete the GeneFoundry contract-hardening sweep on top of the isError/error_code commit. The vendored behaviour gate goes from 20 fail / 10 UNGATED to CONFORMANT (0 fail / 0 UNGATED); surface 4,625t -> 4,083t (outputSchema 24% -> 0%). Schema documentation (Tool-Schema Documentation Standard v1): - Every required and array parameter carries `examples` (json_schema_extra{"example"} -> `examples=[...]`, array form for arrays) — this is what unGATES every tool. - Closed vocabularies are declared as enums matching the runtime. Tool-Surface Budget Standard v1: - Suppress `outputSchema` on every tool via the component wrapper (optional in MCP, unread by models, 24% of the surface). structuredContent is unaffected (dict envelopes) and untrusted-text fencing still happens on the wire (v1.1a). - `FastMCP(dereference_schemas=False)`. Audited contract defects (issue #33, each reproduced live): - get_network_link: `output_format` narrowed to the one value the runtime serves (`json`); the other 8 enum values returned success+empty (silent-empty filter). - compute_functional_enrichment/compute_ppi_enrichment: a bad custom background is now invalid_input naming `background_string_identifiers`, not a false retryable upstream_unavailable (STRING returns 200 + [{"error":"background_error"}]). - get_functional_annotations: stop sending allow_pubmed/only_pubmed unless opted in (their presence ballooned the response past the byte cap; the tool always failed). - get_network_image: return a base64 NetworkImageResult instead of a binary body that defeated the JSON envelope (empty result / internal error). - get_interaction_partners / compute_functional_enrichment: honest, limit-invariant total_count + truncated; enrichment gains `limit` and `category` (a 30-gene panel returned ~992 terms / ~421k tokens with no knob to shrink it). Wire the behaviour probe into the mcp-conformance workflow. Version 4.0.6 -> 4.1.0 (schemas gain fields; outputSchema drops an optional field; no wire break beyond the error_code rename). make ci-local green (452 passed). Refs genefoundry-router#73 #75 #76; refs #33 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019BYK7j7jsPXmBspMsDtfhH --- .github/workflows/conformance.yml | 7 ++ .loc-allowlist | 8 +- CHANGELOG.md | 65 +++++++++++++ pyproject.toml | 2 +- stringdb_link/api/client.py | 12 ++- stringdb_link/api/routes/enrichment.py | 26 +++-- stringdb_link/api/routes/images.py | 23 +++-- stringdb_link/api/routes/networks.py | 47 +++++----- stringdb_link/app.py | 4 + stringdb_link/mcp/error_passthrough.py | 27 +++--- stringdb_link/models/requests.py | 81 +++++++++++----- stringdb_link/models/responses.py | 48 +++++++++- stringdb_link/services/stringdb_service.py | 94 +++++++++++++++++-- tests/unit/mcp/test_error_leak_fencing.py | 12 ++- .../mcp/test_untrusted_content_fencing.py | 52 ++-------- tests/unit/test_response_envelope_v1.py | 21 +++-- uv.lock | 2 +- 17 files changed, 379 insertions(+), 152 deletions(-) diff --git a/.github/workflows/conformance.yml b/.github/workflows/conformance.yml index 0e49953..930fffe 100644 --- a/.github/workflows/conformance.yml +++ b/.github/workflows/conformance.yml @@ -31,6 +31,13 @@ jobs: env: CONFORMANCE_MCP_URL: http://127.0.0.1:${{ env.MCP_PORT }} run: uv run pytest tests/conformance/test_transport_v1.py -v + - name: Run behaviour probe + # Behaviour Conformance v1 — the three recurring contract bugs (silent-empty filter, + # lying total/truncated, error an LLM cannot act on). Every probe is derived from this + # server's own advertised schema; there is no per-repo probe list to maintain. + env: + CONFORMANCE_MCP_URL: http://127.0.0.1:${{ env.MCP_PORT }} + run: uv run pytest tests/conformance/test_behaviour_v1.py -v - name: Logs on failure if: failure() run: make docker-logs || docker compose -f docker/docker-compose.yml logs diff --git a/.loc-allowlist b/.loc-allowlist index 6a5a9e9..0125d5e 100644 --- a/.loc-allowlist +++ b/.loc-allowlist @@ -7,7 +7,7 @@ # # Decomposition backlog: docs/superpowers/specs/2026-05-25-modernize-python-stack-and-agent-workflow-design.md -stringdb_link/api/client.py:796 -stringdb_link/services/stringdb_service.py:776 -stringdb_link/models/requests.py:682 -stringdb_link/models/responses.py:625 +stringdb_link/api/client.py:802 +stringdb_link/services/stringdb_service.py:851 +stringdb_link/models/requests.py:713 +stringdb_link/models/responses.py:667 diff --git a/CHANGELOG.md b/CHANGELOG.md index e6088c4..d05c583 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,71 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [4.1.0] - 2026-07-15 + +GeneFoundry fleet MCP contract-hardening sweep. Vendors the behaviour gate +(`docs/conformance/behaviour.py`, byte-identical from `genefoundry-router@791363c`) +and closes every contract defect it — and the live MCP audit (#33) — surfaced. +Behaviour gate: 20 fail / 10 UNGATED → **0 fail / 0 UNGATED (CONFORMANT)**. +Surface: 4,625t → **4,083t**, `outputSchema` 24% → **0%**, `doc%` 100%, 0 → 26 examples. + +### Fixed + +- **`isError` is now set on every error envelope.** The MCP error carrier returned + `ToolResult(structured_content=...)` without `is_error=True`, so a client branching + on the protocol `isError` bit saw every failed call as a success. Verified against + fastmcp 3.4.4 that the return path keeps `structuredContent` while carrying + `is_error=True`; errors now route through `ToolResult(..., is_error=True)`. +- **`error_code` is closed to the six-value canonical enum** (`invalid_input`, + `not_found`, `ambiguous_query`, `upstream_unavailable`, `rate_limited`, `internal`): + `internal_error` → `internal`. +- **Validation errors now name the offending parameter.** The error frame carries + `field`/`field_errors`, derived ONLY from the request-validation `loc` path (this + server's own schema parameter names, never caller/upstream prose), so an LLM can + self-correct. +- **`get_network_link`: `output_format` no longer silently returns an empty result.** + 8 of its 9 declared enum values returned `{"success": true, "result": {}}`. The + schema now declares only the value the runtime actually serves (`json`); any other + value is rejected as `invalid_input` naming `output_format`. (Silent-empty filter.) +- **`compute_functional_enrichment` / `compute_ppi_enrichment`: a bad custom + background is now `invalid_input`, not a false `upstream_unavailable`.** STRING + returns HTTP 200 with `[{"error": "background_error", ...}]` when + `background_string_identifiers` is not a superset of the query; this was mis-mapped + to a retryable "upstream unavailable". It is now a non-retryable `invalid_input` + naming `background_string_identifiers` (upstream prose never echoed). +- **`get_functional_annotations` works again.** Sending `allow_pubmed`/`only_pubmed` + (even `=0`) made STRING attach PMID annotations and balloon the response past the + byte cap, so the tool always failed. The flags are now sent only when opted in. +- **`get_network_image` returns an image again.** STRING's binary body cannot ride + inside a structured envelope, so the tool returned an internal error or an empty + result. It now returns a base64-encoded `NetworkImageResult` (format, content-type, + size, `image_base64`). +- **`get_interaction_partners` reports an honest `total_count`.** It reported + `total_count = len(returned page)`, which tracked the requested `limit`. It now + fetches a limit-independent page, reports the true total, and sets `truncated`. + +### Added + +- **`compute_functional_enrichment` gains `limit` and `category`.** An unfiltered + 30-gene panel returned ~992 terms (~421k tokens). `limit` (1–1000, default 100) + returns the most-significant terms first; `category` (a closed `EnrichmentCategory` + enum) filters to one STRING category. `total_count` still reports the full number of + matching terms and `truncated` signals more exist, so a smaller `limit` never hides + how many there are. +- **Tool-Schema Documentation Standard v1**: every required and array parameter now + carries `examples`; closed vocabularies are declared as enums matching the runtime. +- Vendored behaviour gate (`tests/conformance/behaviour.py`, + `tests/conformance/test_behaviour_v1.py`) wired into `mcp-conformance` CI alongside + the transport probe. + +### Changed + +- **Tool-Surface Budget Standard v1**: `outputSchema` is suppressed on every tool + (optional in MCP, unread by models, 24% of the surface) and + `FastMCP(dereference_schemas=False)`. `structuredContent` is unaffected (every tool + returns a dict envelope) and untrusted-text fencing still happens on the wire + (Response-Envelope Standard v1.1a). + ## [4.0.6] - 2026-07-14 ### Changed diff --git a/pyproject.toml b/pyproject.toml index 30f7f2e..6f46225 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "stringdb-link" -version = "4.0.6" +version = "4.1.0" description = "High-performance unified API server for STRING protein-protein interaction database with MCP integration" readme = "README.md" license = { text = "MIT" } diff --git a/stringdb_link/api/client.py b/stringdb_link/api/client.py index fccdcf5..479223d 100644 --- a/stringdb_link/api/client.py +++ b/stringdb_link/api/client.py @@ -506,12 +506,18 @@ async def get_functional_annotation( Returns: List of functional annotations (JSON) or formatted string (TSV/XML/PSI-MI) """ - params = { + params: dict[str, Any] = { "identifiers": "\r".join(identifiers), - "allow_pubmed": 1 if allow_pubmed else 0, - "only_pubmed": 1 if only_pubmed else 0, "caller_identity": self.caller_identity, } + # STRING treats the mere PRESENCE of these flags (even "=0") as a request to + # include PMID publication annotations, which balloons the response to tens + # of MB and trips the response byte cap (ResponseTooLargeError). Send them + # ONLY when a caller explicitly opts in. + if allow_pubmed: + params["allow_pubmed"] = 1 + if only_pubmed: + params["only_pubmed"] = 1 if species: params["species"] = species diff --git a/stringdb_link/api/routes/enrichment.py b/stringdb_link/api/routes/enrichment.py index 9d4886c..5d40416 100644 --- a/stringdb_link/api/routes/enrichment.py +++ b/stringdb_link/api/routes/enrichment.py @@ -64,10 +64,15 @@ async def get_functional_enrichment( field=e.field, value=e.value, ) - raise HTTPException( - status_code=400, - detail=f"Validation error: {e.message}", - ) from e + # Emit the FastAPI validation-list shape when the offending parameter is + # known, so the error envelope can name it (e.g. background_string_identifiers) + # without echoing any upstream prose. The fixed "msg" is never surfaced. + detail: object = ( + [{"loc": ["body", e.field], "msg": "invalid value"}] + if e.field + else f"Validation error: {e.message}" + ) + raise HTTPException(status_code=400, detail=detail) from e except Exception as e: logger.exception( @@ -115,10 +120,15 @@ async def get_ppi_enrichment( field=e.field, value=e.value, ) - raise HTTPException( - status_code=400, - detail=f"Validation error: {e.message}", - ) from e + # Emit the FastAPI validation-list shape when the offending parameter is + # known, so the error envelope can name it (e.g. background_string_identifiers) + # without echoing any upstream prose. The fixed "msg" is never surfaced. + detail: object = ( + [{"loc": ["body", e.field], "msg": "invalid value"}] + if e.field + else f"Validation error: {e.message}" + ) + raise HTTPException(status_code=400, detail=detail) from e except Exception as e: logger.exception( diff --git a/stringdb_link/api/routes/images.py b/stringdb_link/api/routes/images.py index 9469362..12bf48c 100644 --- a/stringdb_link/api/routes/images.py +++ b/stringdb_link/api/routes/images.py @@ -2,12 +2,14 @@ from __future__ import annotations +import base64 from typing import TYPE_CHECKING -from fastapi import APIRouter, HTTPException, Response +from fastapi import APIRouter, HTTPException from stringdb_link.exceptions import StringDBServiceError, ValidationError from stringdb_link.models.requests import ImageRequest +from stringdb_link.models.responses import NetworkImageResult from stringdb_link.services.stringdb_service import StringDBService from .dependencies import LoggerDep, StringDBServiceDep @@ -20,6 +22,7 @@ @router.post( "/images/network", + response_model=NetworkImageResult, operation_id="get_network_image", tags=["visualization"], ) @@ -27,16 +30,24 @@ async def get_network_image( request: ImageRequest, service: StringDBService = StringDBServiceDep, logger: FilteringBoundLogger = LoggerDep, -) -> Response: - """Generate protein network visualization image.""" +) -> NetworkImageResult: + """Generate protein network visualization image (base64-encoded).""" try: logger.info("Generating network image", identifiers=request.identifiers) image_response = await service.get_network_image(request) - image_data = image_response.image.image_data - content_type = image_response.image.content_type + image = image_response.image + image_bytes = image.image_data or b"" - return Response(content=image_data, media_type=content_type) + # STRING returns binary image data, which cannot ride inside a structured + # MCP envelope. Encode it as base64 so the tool returns a non-empty, + # decodable result instead of an empty structured payload / internal error. + return NetworkImageResult( + image_format=image.image_format, + content_type=image.content_type, + image_size_bytes=len(image_bytes), + image_base64=base64.b64encode(image_bytes).decode("ascii"), + ) except StringDBServiceError as e: logger.exception( diff --git a/stringdb_link/api/routes/networks.py b/stringdb_link/api/routes/networks.py index 5ca3ee9..c7fe0f8 100644 --- a/stringdb_link/api/routes/networks.py +++ b/stringdb_link/api/routes/networks.py @@ -6,13 +6,9 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Literal from fastapi import APIRouter, HTTPException, Query -from fastapi.responses import PlainTextResponse - -if TYPE_CHECKING: - from fastapi import Response from stringdb_link.exceptions import StringDBServiceError, ValidationError from stringdb_link.models.requests import InteractionPartnersRequest, LinkRequest, NetworkRequest @@ -21,7 +17,7 @@ LinkInfo, NetworkInteractionListResponse, ) -from stringdb_link.models.stringdb import NetworkType, OutputFormat +from stringdb_link.models.stringdb import NetworkType from .dependencies import LoggerDep, StringDBServiceDep @@ -152,20 +148,19 @@ async def get_interaction_partners( network_type=request.network_type, ) - # Call StringDB service + # The service already builds the response with an honest, limit-invariant + # total_count and the truncated flag; return it directly rather than + # rebuilding it with total_count=len(partners) (which tracked the page size). partners_response = await service.get_interaction_partners(request) - partners = partners_response.partners logger.info( "Successfully retrieved interaction partners", input_count=len(request.identifiers), - partner_count=len(partners), + partner_count=len(partners_response.partners), + total_count=partners_response.total_count, ) - return InteractionPartnerListResponse( - partners=partners, - total_count=len(partners), - ) + return partners_response except StringDBServiceError as e: logger.exception( @@ -328,11 +323,15 @@ async def get_network_link( request: LinkRequest, service: StringDBService = StringDBServiceDep, logger: FilteringBoundLogger = LoggerDep, - output_format: OutputFormat = Query( - OutputFormat.JSON, - description="Output format for the response", + output_format: Literal["json"] = Query( + "json", + description=( + "Response format. Only 'json' is supported: it returns the shareable " + "STRING URL inside a structured envelope. Any other value is rejected." + ), + examples=["json"], ), -) -> LinkInfo | Response: +) -> LinkInfo: """Get shareable link to STRING webpage for the network. This endpoint generates a shareable URL that leads to the STRING database @@ -351,15 +350,11 @@ async def get_network_link( species=request.species, ) - link_info = await service.get_network_link(request) - - if output_format == OutputFormat.JSON: - return link_info - # Return raw text for non-JSON formats - return PlainTextResponse( - content=link_info.url, - media_type="text/plain", - ) + # ``output_format`` is constrained to "json" at the schema boundary, so the + # only reachable path returns the structured LinkInfo envelope. The former + # plain-text branch produced an empty MCP structured result (silent-empty) + # and has been removed. + return await service.get_network_link(request) except StringDBServiceError as e: # Log the error type only. The raw identifiers and str(e) can embed the diff --git a/stringdb_link/app.py b/stringdb_link/app.py index 7ec03ac..7a90bf0 100644 --- a/stringdb_link/app.py +++ b/stringdb_link/app.py @@ -172,6 +172,10 @@ def create_mcp_app() -> FastMCP: route_maps=mcp_route_maps, mask_error_details=True, mcp_component_fn=wrap_structured_error_tools, + # Tool-Surface Budget Standard v1 Rule 4: do not inline $defs/$ref at every + # use site. Free and safe (0 input schemas carry a $ref); trims the surface + # a warm client re-sends on every request. + dereference_schemas=False, ) # FastMCP-core not-found reflection guard (Response-Envelope v1.1 fast-follow): diff --git a/stringdb_link/mcp/error_passthrough.py b/stringdb_link/mcp/error_passthrough.py index 0fc82e7..a3da9cf 100644 --- a/stringdb_link/mcp/error_passthrough.py +++ b/stringdb_link/mcp/error_passthrough.py @@ -79,10 +79,10 @@ def _validation_field_names(response: httpx.Response) -> list[str]: loc = item.get("loc") if isinstance(item, dict) else None if not isinstance(loc, list) or not loc: continue - if loc[0] in ("body", "query", "path") and len(loc) > 1: - leaf = loc[-1] - else: - leaf = loc[-1] + # The leaf of the loc path is the offending parameter name (FastAPI prefixes + # it with body/query/path). It is this server's own schema name — safe to + # surface, unlike the caller value or upstream prose. + leaf = loc[-1] if isinstance(leaf, str) and leaf not in names: names.append(leaf) return names @@ -133,15 +133,16 @@ def wrap_structured_error_tools(route: Any, component: Any) -> None: # externally-evolving, open-world database — none mutate state. object.__setattr__(component, "annotations", READ_ONLY_OPEN_WORLD) - # Declare an envelope-shaped outputSchema so the low-level MCP SDK's - # per-call structuredContent validation accepts both the success and error - # frames (see envelope.reshape_output_schema). Passing the tool name lets - # it embed the UntrustedText schema for tools that fence a prose field. - object.__setattr__( - component, - "output_schema", - envelope.reshape_output_schema(component.output_schema, component.name), - ) + # Tool-Surface Budget Standard v1 Rule 3: suppress outputSchema. It is optional + # in MCP, no model reads it, and it is pure per-request surface. Setting it to + # None also removes the low-level SDK's per-call structuredContent validation, + # so both the success and error frames pass freely. structuredContent is still + # emitted (every tool returns a dict envelope; verified fastmcp 3.4.4 + # tools/base.py:357-361). Untrusted-text fencing is UNAFFECTED: it happens on + # the wire in build_success_envelope; Response-Envelope Standard v1.1a requires + # the fenced object on the wire and only requires a schema declaration IF a + # schema is published — which it no longer is. + object.__setattr__(component, "output_schema", None) original_run = component.run tool_name = component.name diff --git a/stringdb_link/models/requests.py b/stringdb_link/models/requests.py index cb02e71..bd4345f 100644 --- a/stringdb_link/models/requests.py +++ b/stringdb_link/models/requests.py @@ -39,13 +39,13 @@ class IdentifierRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers to resolve", - json_schema_extra={"example": ["p53", "BRCA1", "cdk2", "Q99835"]}, + examples=[["p53", "BRCA1", "cdk2", "Q99835"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier (e.g., 9606 for human)", - json_schema_extra={"example": 9606}, + examples=[9606], ) echo_query: bool = Field( False, @@ -84,13 +84,13 @@ class NetworkRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["TP53", "EGFR", "CDK2"]}, + examples=[["TP53", "EGFR", "CDK2"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) required_score: float = Field( ConfidenceScore.MEDIUM, @@ -144,13 +144,13 @@ class InteractionPartnersRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["TP53", "CDK2"]}, + examples=[["TP53", "CDK2"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) limit: int = Field( 10, @@ -200,17 +200,43 @@ class EnrichmentRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["trpA", "trpB", "trpC", "trpE", "trpGD"]}, + examples=[["trpA", "trpB", "trpC", "trpE", "trpGD"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 511145}, + examples=[511145], + ) + category: EnrichmentCategory | None = Field( + None, + description=( + "Optional term-category filter. When set, only enrichment terms in " + "this STRING category are returned (e.g. 'KEGG' for pathways, " + "'Process' for GO biological process, 'DISEASES' for disease " + "associations). Omit to return terms across all categories." + ), + examples=["KEGG"], + ) + limit: int = Field( + 100, + ge=1, + le=1000, + description=( + "Maximum number of enrichment terms to return, taken as the most " + "significant (lowest FDR) first. 'total_count' still reports the full " + "number of matching terms, so a smaller limit never hides how many exist." + ), + examples=[25], ) background_string_identifiers: list[str] | None = Field( None, - description="Background proteome STRING identifiers", + description=( + "Optional custom background proteome, given as STRING identifiers " + "(e.g. '9606.ENSP00000269305'). MUST be a superset of the query " + "identifiers; STRING rejects a background that omits any query protein." + ), + examples=[["9606.ENSP00000269305", "9606.ENSP00000350283"]], ) @field_validator("identifiers") @@ -264,13 +290,13 @@ class AnnotationRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["cdk1"]}, + examples=[["cdk1"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) allow_pubmed: bool = Field( False, @@ -312,13 +338,13 @@ class ImageRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["nup100"]}, + examples=[["nup100"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 4932}, + examples=[4932], ) add_color_nodes: int = Field( 0, @@ -394,13 +420,13 @@ class HomologyRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["CDK1", "CDK2"]}, + examples=[["CDK1", "CDK2"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) @field_validator("identifiers") @@ -434,18 +460,18 @@ class HomologyBestRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["CDK1"]}, + examples=[["CDK1"]], ) species: int | None = Field( None, ge=1, description="Source species NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) species_b: list[int] | None = Field( None, description="Target species NCBI taxon identifiers", - json_schema_extra={"example": [10090]}, + examples=[[10090]], ) @field_validator("identifiers") @@ -496,13 +522,13 @@ class PPIEnrichmentRequest(BaseRequest): min_length=2, # Need at least 2 proteins for PPI enrichment max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["trpA", "trpB", "trpC", "trpE", "trpGD"]}, + examples=[["trpA", "trpB", "trpC", "trpE", "trpGD"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 511145}, + examples=[511145], ) required_score: float = Field( ConfidenceScore.MEDIUM, @@ -512,7 +538,12 @@ class PPIEnrichmentRequest(BaseRequest): ) background_string_identifiers: list[str] | None = Field( None, - description="Background proteome STRING identifiers", + description=( + "Optional custom background proteome, given as STRING identifiers " + "(e.g. '9606.ENSP00000269305'). MUST be a superset of the query " + "identifiers; STRING rejects a background that omits any query protein." + ), + examples=[["9606.ENSP00000269305", "9606.ENSP00000350283"]], ) @field_validator("identifiers") @@ -566,13 +597,13 @@ class EnrichmentImageRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["ARRB1", "ARRB2", "EVC", "PTCH1", "SHH", "SMO"]}, + examples=[["ARRB1", "ARRB2", "EVC", "PTCH1", "SHH", "SMO"]], ) species: int = Field( ..., ge=1, description="NCBI taxon identifier (required for enrichment)", - json_schema_extra={"example": 9606}, + examples=[9606], ) category: EnrichmentCategory = Field( EnrichmentCategory.PROCESS, @@ -636,13 +667,13 @@ class LinkRequest(BaseRequest): min_length=1, max_length=MAX_IDENTIFIERS_PER_REQUEST, description="List of protein identifiers", - json_schema_extra={"example": ["p53", "BRCA1", "MDM2"]}, + examples=[["p53", "BRCA1", "MDM2"]], ) species: int | None = Field( None, ge=1, description="NCBI taxon identifier", - json_schema_extra={"example": 9606}, + examples=[9606], ) required_score: float = Field( ConfidenceScore.MEDIUM, diff --git a/stringdb_link/models/responses.py b/stringdb_link/models/responses.py index 126875d..990b479 100644 --- a/stringdb_link/models/responses.py +++ b/stringdb_link/models/responses.py @@ -540,6 +540,40 @@ class NetworkImageResponse(BaseResponse): ) +class NetworkImageResult(BaseResponse): + """JSON-safe network image payload for the MCP tool surface. + + The raw STRING image is binary, which cannot be returned inside a structured + MCP envelope. This model carries the image as a base64 string plus its format + metadata so the tool returns a non-empty, decodable result. + """ + + image_format: str = Field( + ..., + description="Requested image format (image=PNG, highres_image=hi-res PNG, svg=SVG)", + json_schema_extra={"example": "image"}, + ) + content_type: str = Field( + ..., + description="MIME type of the encoded image", + json_schema_extra={"example": "image/png"}, + ) + image_size_bytes: int = Field( + ..., + ge=0, + description="Size in bytes of the decoded image", + json_schema_extra={"example": 20480}, + ) + image_base64: str = Field( + ..., + description=( + "Base64-encoded image bytes. Decode this string to obtain the PNG or " + "SVG file rendered by STRING for the requested network." + ), + json_schema_extra={"example": "iVBORw0KGgoAAAANSUhEUgAA..."}, + ) + + # Response wrappers for lists class StringIdMappingListResponse(BaseResponse): """Response wrapper for list of identifier mappings.""" @@ -579,7 +613,11 @@ class InteractionPartnerListResponse(BaseResponse): total_count: int = Field( ..., ge=0, - description="Total number of partners", + description="Total number of partners available (independent of the requested limit)", + ) + truncated: bool = Field( + False, + description="True when more partners exist than were returned under the requested limit", ) @@ -588,12 +626,16 @@ class EnrichmentTermListResponse(BaseResponse): terms: list[EnrichmentTerm] = Field( ..., - description="List of enriched terms", + description="List of enriched terms (most significant first, capped at the requested limit)", ) total_count: int = Field( ..., ge=0, - description="Total number of enriched terms", + description="Total number of enriched terms matching the query (independent of the limit)", + ) + truncated: bool = Field( + False, + description="True when more enriched terms exist than were returned under the requested limit", ) diff --git a/stringdb_link/services/stringdb_service.py b/stringdb_link/services/stringdb_service.py index 900a8d6..72f9f2e 100644 --- a/stringdb_link/services/stringdb_service.py +++ b/stringdb_link/services/stringdb_service.py @@ -12,7 +12,7 @@ from pydantic import ValidationError as PydanticValidationError from stringdb_link.config import settings -from stringdb_link.exceptions import StringDBServiceError +from stringdb_link.exceptions import StringDBServiceError, ValidationError from stringdb_link.logging_config import get_logger from stringdb_link.models.responses import ( EnrichmentTerm, @@ -48,6 +48,48 @@ from stringdb_link.models.responses import LinkInfo, PPIEnrichmentResult +#: STRING's ``interaction_partners`` endpoint caps returned partners per protein at +#: 500. We always request this fixed page so ``total_count`` is a property of the +#: result set (invariant of the caller's ``limit``) and truncate client-side. +_PARTNER_UPSTREAM_LIMIT = 500 + +#: Field named in the invalid_input envelope when STRING reports an in-band error +#: about the caller-supplied background proteome. +_BACKGROUND_FIELD = "background_string_identifiers" +#: Fixed, server-authored message. The upstream STRING error prose is NEVER echoed +#: (it is caller-influenceable); only this constant and the offending field name +#: reach the caller (security posture). +_BACKGROUND_ERROR_MESSAGE = ( + "The custom background proteome is invalid: it must be a superset of the query " + "identifiers. Adjust 'background_string_identifiers' so it contains every query " + "protein, or omit it to use the whole-genome background." +) +_GENERIC_INPUT_MESSAGE = "The request was rejected by the upstream STRING API as invalid." + + +def _raise_if_string_error(payload: object) -> None: + """Map STRING's in-band error shape onto a client-side ``invalid_input``. + + STRING returns HTTP 200 with a body like ``[{"error": "background_error", + "message": "..."}]`` for a bad request (e.g. a background that is not a + superset of the query). Left unhandled, building a typed model from that body + raises a validation error that the service mis-maps to ``upstream_unavailable`` + (retryable) — a false "the upstream is down, retry forever". Detect the shape + here and raise ``ValidationError`` (400 → invalid_input, non-retryable) naming + the offending parameter, without echoing the upstream prose. + """ + item: dict[str, object] | None = None + if isinstance(payload, list) and payload and isinstance(payload[0], dict): + item = payload[0] + elif isinstance(payload, dict): + item = payload + if item is None or not item.get("error"): + return + if item.get("error") == "background_error": + raise ValidationError(_BACKGROUND_ERROR_MESSAGE, field=_BACKGROUND_FIELD) + raise ValidationError(_GENERIC_INPUT_MESSAGE) + + class StringDBService: """Service for StringDB operations with caching and business logic.""" @@ -237,23 +279,31 @@ async def get_interaction_partners( network_type=request.network_type, ) - partners = await self._cached_get_interaction_partners( + # Fetch a fixed, limit-independent page from STRING (its per-protein cap + # is 500) and truncate client-side. This keeps ``total_count`` a property + # of the result set, invariant of the caller's ``limit`` (an honest total), + # and lets ``truncated`` signal that more partners exist. + all_partners = await self._cached_get_interaction_partners( identifiers=tuple(request.identifiers), species=request.species, - limit=request.limit, + limit=_PARTNER_UPSTREAM_LIMIT, required_score=round(request.required_score * 1000), network_type=request.network_type.value, ) + total = len(all_partners) + limited = all_partners[: request.limit] self.logger.info( "Successfully retrieved interaction partners", input_count=len(request.identifiers), - partner_count=len(partners), + partner_count=len(limited), + total_count=total, ) return InteractionPartnerListResponse( - partners=partners, - total_count=len(partners), + partners=limited, + total_count=total, + truncated=total > len(limited), ) except Exception as e: @@ -320,17 +370,33 @@ async def get_functional_enrichment( ), ) + # Optional category filter (closed vocabulary) and top-N truncation. + # ``total_count`` reports the FULL number of matching terms so a smaller + # ``limit`` never hides how many exist (honest total, invariant of limit). + if request.category is not None: + category_value = request.category.value + terms = [term for term in terms if term.category == category_value] + terms = sorted(terms, key=lambda term: term.fdr) + total = len(terms) + limited = terms[: request.limit] + self.logger.info( "Successfully retrieved functional enrichment", input_count=len(request.identifiers), - term_count=len(terms), + term_count=len(limited), + total_count=total, ) return EnrichmentTermListResponse( - terms=terms, - total_count=len(terms), + terms=limited, + total_count=total, + truncated=total > len(limited), ) + except ValidationError: + # STRING in-band error (e.g. background_error) already classified as + # invalid_input — propagate unwrapped so it is not re-mapped to 5xx. + raise except Exception as e: self.logger.exception( "Error getting functional enrichment", @@ -360,6 +426,10 @@ async def _cached_get_functional_enrichment( list(background_string_identifiers) if background_string_identifiers else None ), ) + # Detect STRING's in-band error body (HTTP 200 + [{"error": ...}]) before + # attempting to build typed models from it (defect: was mis-mapped to + # upstream_unavailable/retryable). + _raise_if_string_error(raw_terms) records = raw_terms if isinstance(raw_terms, list) else [] return [EnrichmentTerm(**term) for term in records] @@ -683,6 +753,10 @@ async def get_ppi_enrichment(self, request: PPIEnrichmentRequest) -> PPIEnrichme background_string_identifiers=request.background_string_identifiers, ) + # Same STRING in-band error shape as functional enrichment: a bad + # background returns {"error": "background_error", ...} at HTTP 200. + _raise_if_string_error(raw_result) + from stringdb_link.models.responses import PPIEnrichmentResult result = PPIEnrichmentResult(**cast("dict[str, Any]", raw_result)) @@ -695,6 +769,8 @@ async def get_ppi_enrichment(self, request: PPIEnrichmentRequest) -> PPIEnrichme return result + except ValidationError: + raise except Exception as e: self.logger.exception( "Error getting PPI enrichment", diff --git a/tests/unit/mcp/test_error_leak_fencing.py b/tests/unit/mcp/test_error_leak_fencing.py index db00511..ef94e60 100644 --- a/tests/unit/mcp/test_error_leak_fencing.py +++ b/tests/unit/mcp/test_error_leak_fencing.py @@ -69,7 +69,9 @@ async def test_service_error_is_severed_to_fixed_message_in_both_mirrors( side_effect=StringDBServiceError(HOSTILE), ): async with Client(facade) as client: - result = await client.call_tool("resolve_protein_identifiers", {"identifiers": ["p53"]}) + result = await client.call_tool( + "resolve_protein_identifiers", {"identifiers": ["p53"]}, raise_on_error=False + ) structured = result.structured_content assert structured is not None @@ -93,7 +95,9 @@ async def test_argument_validation_is_severed_to_fixed_message(facade: Any) -> N """An invalid argument value (hostile string where a list is required) yields a fixed ``invalid_input`` message — the argument value is never echoed.""" async with Client(facade) as client: - result = await client.call_tool("resolve_protein_identifiers", {"identifiers": HOSTILE}) + result = await client.call_tool( + "resolve_protein_identifiers", {"identifiers": HOSTILE}, raise_on_error=False + ) structured = result.structured_content assert structured is not None @@ -116,7 +120,9 @@ async def test_timeout_yields_clean_fixed_message(facade: Any) -> None: side_effect=httpx.TimeoutException("upstream boom‍"), ): async with Client(facade) as client: - result = await client.call_tool("resolve_protein_identifiers", {"identifiers": ["p53"]}) + result = await client.call_tool( + "resolve_protein_identifiers", {"identifiers": ["p53"]}, raise_on_error=False + ) structured = result.structured_content assert structured is not None diff --git a/tests/unit/mcp/test_untrusted_content_fencing.py b/tests/unit/mcp/test_untrusted_content_fencing.py index ffc682e..973d27e 100644 --- a/tests/unit/mcp/test_untrusted_content_fencing.py +++ b/tests/unit/mcp/test_untrusted_content_fencing.py @@ -201,50 +201,18 @@ async def test_large_enrichment_result_does_not_raise(facade: Any) -> None: } -def _resolve_ref(node: dict[str, Any], root: dict[str, Any]) -> dict[str, Any]: - """Follow a local ``#/$defs/...`` ``$ref`` (FastMCP may inline or keep it).""" - ref = node.get("$ref") - if isinstance(ref, str) and ref.startswith("#/$defs/"): - return root.get("$defs", {})[ref.split("/")[-1]] - return node - - @pytest.mark.asyncio -async def test_list_tools_declares_untrusted_text_object_at_fenced_pointers( - facade: Any, -) -> None: - """The LIVE tool schema (list_tools) must advertise the fenced pointer as the - typed untrusted_text object (``kind`` const), never as a bare string. - - Regression guard for the schema-declaration bug: embedding UntrustedText only - as an unreferenced ``$defs`` entry let list_tools keep advertising - ``annotation``/``description`` as ``type: string``. +async def test_fenced_tools_suppress_output_schema(facade: Any) -> None: + """outputSchema is SUPPRESSED on every fenced tool (Tool-Surface Budget + Standard v1 Rule 3) — no model reads it and it is pure per-request surface. + + Response-Envelope Standard v1.1a: the schema need declare ``untrusted_text`` + only IF a schema is published; the load-bearing requirement is that the fenced + object appears ON THE WIRE, which the ``*_is_fenced`` tests above prove for + every pointer in ``_FENCED_POINTERS``. """ async with Client(facade) as client: tools = {t.name: t for t in await client.list_tools()} - for tool_name, field in _FENCED_POINTERS.items(): - schema = tools[tool_name].outputSchema - assert schema is not None, tool_name - results = schema["properties"]["results"] - assert results["type"] == "array", tool_name - item = _resolve_ref(results["items"], schema) - field_schema = _resolve_ref(item["properties"][field], schema) - - # It is the untrusted_text object, NOT a bare string. - assert field_schema.get("type") == "object", (tool_name, field, field_schema) - assert field_schema.get("type") != "string", (tool_name, field) - props = field_schema["properties"] - assert set(props) >= {"kind", "text", "provenance", "raw_sha256"}, (tool_name, field) - - kind = props["kind"] - assert kind.get("const") == "untrusted_text" or kind.get("enum") == ["untrusted_text"], ( - tool_name, - kind, - ) - - provenance = _resolve_ref(props["provenance"], schema) - assert set(provenance["properties"]) >= {"source", "record_id", "retrieved_at"}, ( - tool_name, - field, - ) + for tool_name in _FENCED_POINTERS: + assert tools[tool_name].outputSchema is None, tool_name diff --git a/tests/unit/test_response_envelope_v1.py b/tests/unit/test_response_envelope_v1.py index c76ec22..7605e41 100644 --- a/tests/unit/test_response_envelope_v1.py +++ b/tests/unit/test_response_envelope_v1.py @@ -17,6 +17,7 @@ from __future__ import annotations +import base64 from typing import Any from unittest.mock import AsyncMock, patch @@ -175,13 +176,14 @@ async def test_internal_failure_is_non_retryable_internal_error(facade: Any) -> @pytest.mark.asyncio -async def test_binary_image_tool_degrades_to_structured_internal_error(facade: Any) -> None: - """The image tool's binary body defeats the JSON MCP provider (no HTTP - response in the chain); the wrapper degrades it to a flat, non-retryable - internal_error envelope with the disclaimer — never an opaque ToolError.""" +async def test_image_tool_returns_base64_success_envelope(facade: Any) -> None: + """The image tool base64-encodes STRING's binary body into a structured + success envelope (JSON-safe), so the tool returns a non-empty, decodable + result instead of degrading to internal_error or an empty payload.""" + raw = b"\x89PNG\r\n\x1a\n" image = NetworkImageResponse( image=NetworkImage( - image_data=b"\x89PNG\r\n\x1a\n", + image_data=raw, image_format="image", content_type="image/png", ) @@ -200,9 +202,12 @@ async def test_binary_image_tool_degrades_to_structured_internal_error(facade: A body = result.structured_content assert body is not None - assert body["success"] is False - assert body["error_code"] == "internal" - assert body["retryable"] is False + assert result.is_error is False + assert body["success"] is True + payload = body["result"] + assert payload["content_type"] == "image/png" + assert payload["image_size_bytes"] == len(raw) + assert base64.b64decode(payload["image_base64"]) == raw assert body["_meta"]["tool"] == "get_network_image" assert body["_meta"]["unsafe_for_clinical_use"] is True diff --git a/uv.lock b/uv.lock index db786b7..9625b43 100644 --- a/uv.lock +++ b/uv.lock @@ -1697,7 +1697,7 @@ wheels = [ [[package]] name = "stringdb-link" -version = "4.0.6" +version = "4.1.0" source = { editable = "." } dependencies = [ { name = "asgi-correlation-id" }, From fa72199ec1981239bf02ffe75339d335971cd4e1 Mon Sep 17 00:00:00 2001 From: Bernt Popp Date: Wed, 15 Jul 2026 13:58:34 +0200 Subject: [PATCH 3/3] =?UTF-8?q?fix(mcp):=20address=20Codex=20review=20?= =?UTF-8?q?=E2=80=94=20per-protein=20pagination,=20returned-dict=20isError?= =?UTF-8?q?,=20keep=20link=20formats,=20MCP-only=20image=20base64,=20gener?= =?UTF-8?q?al=20in-band=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six fixes from the PR #34 review (Codex gpt-5.6-sol): 1. get_interaction_partners: paginate PER PROTEIN. The prior global head-slice (all_partners[:limit]) returned only the first query protein's partners and omitted every later protein; the hardcoded 500 fetch could not prove a true total. Now fetch the full partner set (limit omitted upstream), apply `limit` per query protein (grouped by string_id_a), report the true limit-invariant total, and set `truncated`. 2. Returned-dict isError: a tool that RETURNS {"success": false} was wrapped as a success with isError:false. The chokepoint (finalize_tool_result) now normalizes a returned error frame to isError:true, closing error_code to the enum. 3. get_network_link output_format: restore the four formats STRING actually serves (json/tsv/tsv-no-header/xml) instead of narrowing to json (capability loss → 422). tsv/xml are shaped into a structured result: `url` extracted + STRING's raw text in `formatted`. The empty result:{} was a shaping bug, not proof of non-support. 4. Image REST contract: POST /api/images/network keeps its raw-binary response (operation download_network_image, excluded from MCP). The base64 shaping moved to a dedicated MCP-only route POST /api/images/network/json (operation get_network_image). 5. Empty image body: an empty upstream body is now upstream_unavailable, never a success with 0 bytes / empty base64 (a new silent-empty). 6. STRING in-band errors: handle the GENERAL class — ANY {"error": X} at HTTP 200 → invalid_input naming a field (background_error → background_string_identifiers, otherwise identifiers), never a field-less error or false retryable upstream outage. Behaviour gate remains CONFORMANT (51 pass, 0 fail, 0 UNGATED). make ci-local green (463 passed). New regression tests in tests/unit/test_contract_rework_v1.py and the empty-image facade test. Refs genefoundry-router#73 #75 #76; refs #33 Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019BYK7j7jsPXmBspMsDtfhH --- .loc-allowlist | 6 +- CHANGELOG.md | 58 ++++--- stringdb_link/api/client.py | 11 +- stringdb_link/api/routes/images.py | 97 +++++++----- stringdb_link/api/routes/networks.py | 19 ++- stringdb_link/app.py | 5 + stringdb_link/mcp/envelope.py | 67 +++++++- stringdb_link/mcp/error_passthrough.py | 35 ++++- stringdb_link/models/responses.py | 14 ++ stringdb_link/services/stringdb_service.py | 122 +++++++++++---- tests/unit/test_contract_rework_v1.py | 173 +++++++++++++++++++++ tests/unit/test_response_envelope_v1.py | 26 ++++ 12 files changed, 519 insertions(+), 114 deletions(-) create mode 100644 tests/unit/test_contract_rework_v1.py diff --git a/.loc-allowlist b/.loc-allowlist index 0125d5e..419da17 100644 --- a/.loc-allowlist +++ b/.loc-allowlist @@ -7,7 +7,7 @@ # # Decomposition backlog: docs/superpowers/specs/2026-05-25-modernize-python-stack-and-agent-workflow-design.md -stringdb_link/api/client.py:802 -stringdb_link/services/stringdb_service.py:851 +stringdb_link/api/client.py:805 +stringdb_link/services/stringdb_service.py:911 stringdb_link/models/requests.py:713 -stringdb_link/models/responses.py:667 +stringdb_link/models/responses.py:681 diff --git a/CHANGELOG.md b/CHANGELOG.md index d05c583..037d90f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,14 @@ Surface: 4,625t → **4,083t**, `outputSchema` 24% → **0%**, `doc%` 100%, 0 ### Fixed -- **`isError` is now set on every error envelope.** The MCP error carrier returned - `ToolResult(structured_content=...)` without `is_error=True`, so a client branching - on the protocol `isError` bit saw every failed call as a success. Verified against - fastmcp 3.4.4 that the return path keeps `structuredContent` while carrying - `is_error=True`; errors now route through `ToolResult(..., is_error=True)`. +- **`isError` is now set on every error envelope — for BOTH the raised and the + returned-dict paths.** The MCP error carrier returned `ToolResult(structured_content=...)` + without `is_error=True`, so a client branching on the protocol `isError` bit saw every + failed call as a success. Verified against fastmcp 3.4.4 that the return path keeps + `structuredContent` while carrying `is_error=True`; errors now route through + `ToolResult(..., is_error=True)`. A payload a tool *returns* as `{"success": false}` + (rather than raising) is likewise normalized to an error frame with `isError:true`, + never re-wrapped as a success. - **`error_code` is closed to the six-value canonical enum** (`invalid_input`, `not_found`, `ambiguous_query`, `upstream_unavailable`, `rate_limited`, `internal`): `internal_error` → `internal`. @@ -28,25 +31,38 @@ Surface: 4,625t → **4,083t**, `outputSchema` 24% → **0%**, `doc%` 100%, 0 server's own schema parameter names, never caller/upstream prose), so an LLM can self-correct. - **`get_network_link`: `output_format` no longer silently returns an empty result.** - 8 of its 9 declared enum values returned `{"success": true, "result": {}}`. The - schema now declares only the value the runtime actually serves (`json`); any other - value is rejected as `invalid_input` naming `output_format`. (Silent-empty filter.) -- **`compute_functional_enrichment` / `compute_ppi_enrichment`: a bad custom - background is now `invalid_input`, not a false `upstream_unavailable`.** STRING - returns HTTP 200 with `[{"error": "background_error", ...}]` when - `background_string_identifiers` is not a superset of the query; this was mis-mapped - to a retryable "upstream unavailable". It is now a non-retryable `invalid_input` - naming `background_string_identifiers` (upstream prose never echoed). + 8 of its 9 declared enum values returned `{"success": true, "result": {}}`. The four + formats STRING actually serves for a link (`json`, `tsv`, `tsv-no-header`, `xml`) are + kept and each is shaped into a structured result — `json` fills `url`; the text + formats fill `formatted` (STRING's raw serialization) with the URL also extracted + into `url`. STRING's `psi-mi`/`image`/`svg` link formats (which carry no link + payload) are dropped from the enum. (Silent-empty filter, without capability loss.) +- **`compute_functional_enrichment` / `compute_ppi_enrichment`: ANY STRING in-band + error is now `invalid_input` naming a field, not a false `upstream_unavailable`.** + STRING returns HTTP 200 with `[{"error": "...", ...}]` for a bad request; this was + mis-mapped to a retryable "upstream unavailable". The general class is now handled — + `background_error` names `background_string_identifiers`, and any other in-band error + names `identifiers` — as a non-retryable `invalid_input` (upstream prose never + echoed), so a new STRING error type can never regress to a field-less error or a + false retryable outage. - **`get_functional_annotations` works again.** Sending `allow_pubmed`/`only_pubmed` (even `=0`) made STRING attach PMID annotations and balloon the response past the byte cap, so the tool always failed. The flags are now sent only when opted in. -- **`get_network_image` returns an image again.** STRING's binary body cannot ride - inside a structured envelope, so the tool returned an internal error or an empty - result. It now returns a base64-encoded `NetworkImageResult` (format, content-type, - size, `image_base64`). -- **`get_interaction_partners` reports an honest `total_count`.** It reported - `total_count = len(returned page)`, which tracked the requested `limit`. It now - fetches a limit-independent page, reports the true total, and sets `truncated`. +- **`get_network_image` returns an image again — on the MCP surface only.** STRING's + binary body cannot ride inside a structured MCP envelope, so the tool returned an + internal error or an empty result. The MCP tool now returns a base64-encoded + `NetworkImageResult` (format, content-type, size, `image_base64`) via a dedicated + `POST /api/images/network/json` route; the REST `POST /api/images/network` endpoint + keeps its raw-binary contract (renamed operation `download_network_image`, excluded + from MCP). An empty upstream image body is now a `upstream_unavailable` error, never + a `success` with zero bytes. +- **`get_interaction_partners` reports an honest, limit-invariant `total_count` and + paginates PER PROTEIN.** It reported `total_count = len(returned page)` and + head-sliced the flattened partner list — which for a multi-protein query returned + only the first protein's partners and omitted every later protein. It now fetches the + full partner set (so the total is the true count, invariant of `limit`), applies + `limit` per query protein so every queried protein is represented, and sets + `truncated`. ### Added diff --git a/stringdb_link/api/client.py b/stringdb_link/api/client.py index 479223d..dd7f8f4 100644 --- a/stringdb_link/api/client.py +++ b/stringdb_link/api/client.py @@ -419,7 +419,7 @@ async def get_interaction_partners( self, identifiers: list[str], species: int | None = None, - limit: int = 10, + limit: int | None = 10, required_score: int = 400, network_type: str = "functional", output_format: OutputFormat = OutputFormat.JSON, @@ -429,7 +429,9 @@ async def get_interaction_partners( Args: identifiers: List of protein identifiers species: NCBI taxon ID - limit: Maximum number of partners per protein + limit: Maximum number of partners PER PROTEIN. ``None`` omits the + parameter so STRING returns the full partner set (used to compute a + true, limit-invariant total before applying a per-protein cap). required_score: Minimum confidence score on STRING's 0-1000 integer scale (e.g. 400 = 0.4) network_type: Network type (functional or physical) output_format: Output format (json, tsv, xml, psi-mi) @@ -437,13 +439,14 @@ async def get_interaction_partners( Returns: List of interaction partners (JSON) or formatted string (TSV/XML/PSI-MI) """ - params = { + params: dict[str, Any] = { "identifiers": "\r".join(identifiers), - "limit": limit, "required_score": required_score, "network_type": network_type, "caller_identity": self.caller_identity, } + if limit is not None: + params["limit"] = limit if species: params["species"] = species diff --git a/stringdb_link/api/routes/images.py b/stringdb_link/api/routes/images.py index 12bf48c..2488f3e 100644 --- a/stringdb_link/api/routes/images.py +++ b/stringdb_link/api/routes/images.py @@ -5,7 +5,7 @@ import base64 from typing import TYPE_CHECKING -from fastapi import APIRouter, HTTPException +from fastapi import APIRouter, HTTPException, Response from stringdb_link.exceptions import StringDBServiceError, ValidationError from stringdb_link.models.requests import ImageRequest @@ -19,9 +19,51 @@ router = APIRouter() +_EMPTY_IMAGE_DETAIL = "STRING returned an empty image for this network; no image is available." + + +def _translate_image_errors(e: Exception, logger: FilteringBoundLogger) -> HTTPException: + """Map a service/validation/unexpected failure to the right HTTP status.""" + if isinstance(e, StringDBServiceError): + logger.exception("Service error during network image generation", error=str(e)) + return HTTPException(status_code=e.status_code or 500, detail=f"Service error: {e.message}") + if isinstance(e, ValidationError): + logger.exception("Validation error during network image generation", field=e.field) + return HTTPException(status_code=400, detail=f"Validation error: {e.message}") + logger.exception("Unexpected error during network image generation", error=str(e)) + return HTTPException( + status_code=500, detail="Internal server error during network image generation" + ) + @router.post( "/images/network", + operation_id="download_network_image", + tags=["visualization"], + response_class=Response, + responses={200: {"content": {"image/png": {}, "image/svg+xml": {}}}}, +) +async def download_network_image( + request: ImageRequest, + service: StringDBService = StringDBServiceDep, + logger: FilteringBoundLogger = LoggerDep, +) -> Response: + """Download the raw binary network image (REST clients). + + Returns STRING's image bytes with their native media type. This is the REST + download contract; the MCP surface uses ``get_network_image`` (JSON base64), + since binary cannot ride inside a structured MCP envelope. Excluded from MCP. + """ + try: + logger.info("Downloading network image", identifiers=request.identifiers) + image = (await service.get_network_image(request)).image + return Response(content=image.image_data or b"", media_type=image.content_type) + except Exception as e: # every failure is translated to an HTTP status + raise _translate_image_errors(e, logger) from e + + +@router.post( + "/images/network/json", response_model=NetworkImageResult, operation_id="get_network_image", tags=["visualization"], @@ -31,17 +73,20 @@ async def get_network_image( service: StringDBService = StringDBServiceDep, logger: FilteringBoundLogger = LoggerDep, ) -> NetworkImageResult: - """Generate protein network visualization image (base64-encoded).""" + """Generate a protein network visualization image as base64 (MCP surface).""" try: logger.info("Generating network image", identifiers=request.identifiers) - - image_response = await service.get_network_image(request) - image = image_response.image + image = (await service.get_network_image(request)).image image_bytes = image.image_data or b"" - # STRING returns binary image data, which cannot ride inside a structured - # MCP envelope. Encode it as base64 so the tool returns a non-empty, - # decodable result instead of an empty structured payload / internal error. + # An empty upstream body is NOT a successful zero-byte image — that would be a + # silent-empty success. Surface it as a retryable upstream failure instead. + if not image_bytes: + raise HTTPException(status_code=502, detail=_EMPTY_IMAGE_DETAIL) + + # STRING returns binary image data, which cannot ride inside a structured MCP + # envelope. Encode it as base64 so the tool returns a non-empty, decodable + # result instead of an empty structured payload / internal error. return NetworkImageResult( image_format=image.image_format, content_type=image.content_type, @@ -49,35 +94,7 @@ async def get_network_image( image_base64=base64.b64encode(image_bytes).decode("ascii"), ) - except StringDBServiceError as e: - logger.exception( - "Service error during network image generation", - error=str(e), - operation=e.operation, - ) - raise HTTPException( - status_code=e.status_code or 500, - detail=f"Service error: {e.message}", - ) from e - - except ValidationError as e: - logger.exception( - "Validation error during network image generation", - error=str(e), - field=e.field, - value=e.value, - ) - raise HTTPException( - status_code=400, - detail=f"Validation error: {e.message}", - ) from e - - except Exception as e: - logger.exception( - "Unexpected error during network image generation", - error=str(e), - ) - raise HTTPException( - status_code=500, - detail="Internal server error during network image generation", - ) from e + except HTTPException: + raise + except Exception as e: # every failure is translated to an HTTP status + raise _translate_image_errors(e, logger) from e diff --git a/stringdb_link/api/routes/networks.py b/stringdb_link/api/routes/networks.py index c7fe0f8..3c34313 100644 --- a/stringdb_link/api/routes/networks.py +++ b/stringdb_link/api/routes/networks.py @@ -323,11 +323,14 @@ async def get_network_link( request: LinkRequest, service: StringDBService = StringDBServiceDep, logger: FilteringBoundLogger = LoggerDep, - output_format: Literal["json"] = Query( + output_format: Literal["json", "tsv", "tsv-no-header", "xml"] = Query( "json", description=( - "Response format. Only 'json' is supported: it returns the shareable " - "STRING URL inside a structured envelope. Any other value is rejected." + "STRING serialization of the shareable link. All four formats convey the " + "same URL: 'json' returns it structured in 'url'; 'tsv', 'tsv-no-header' " + "and 'xml' return STRING's text in 'formatted' with the URL also extracted " + "into 'url'. (STRING's psi-mi/image/svg link formats are not offered — they " + "carry no link payload.)" ), examples=["json"], ), @@ -350,11 +353,11 @@ async def get_network_link( species=request.species, ) - # ``output_format`` is constrained to "json" at the schema boundary, so the - # only reachable path returns the structured LinkInfo envelope. The former - # plain-text branch produced an empty MCP structured result (silent-empty) - # and has been removed. - return await service.get_network_link(request) + # Every format is shaped into the structured LinkInfo envelope: json fills + # ``url``; tsv/tsv-no-header/xml fill ``formatted`` (STRING's raw text) with + # the URL also extracted into ``url``. The former plain-text branch produced + # an empty MCP structured result (silent-empty) and has been removed. + return await service.get_network_link(request, output_format=output_format) except StringDBServiceError as e: # Log the error type only. The raw identifiers and str(e) can embed the diff --git a/stringdb_link/app.py b/stringdb_link/app.py index 7a90bf0..2c7f45a 100644 --- a/stringdb_link/app.py +++ b/stringdb_link/app.py @@ -151,6 +151,11 @@ def create_mcp_app() -> FastMCP: # ergonomics, and redundant with the JSON tools. RouteMap(pattern=r"^/api/homology/scores/download$", mcp_type=MCPType.EXCLUDE), RouteMap(pattern=r"^/api/homology/best-hits/download$", mcp_type=MCPType.EXCLUDE), + # Exclude the binary image download: it returns raw bytes (image/png, + # image/svg+xml) that cannot ride inside a structured MCP envelope. The MCP + # surface uses /api/images/network/json (operation_id get_network_image), + # which base64-encodes the image into the envelope. + RouteMap(pattern=r"^/api/images/network$", mcp_type=MCPType.EXCLUDE), ] # Create FastMCP instance. mask_error_details=True keeps internal exception diff --git a/stringdb_link/mcp/envelope.py b/stringdb_link/mcp/envelope.py index 63f3739..fc5876c 100644 --- a/stringdb_link/mcp/envelope.py +++ b/stringdb_link/mcp/envelope.py @@ -30,7 +30,7 @@ import uuid from dataclasses import dataclass -from typing import Any, Literal +from typing import Any, Literal, cast from stringdb_link.mcp.untrusted_content import ( UntrustedText, @@ -315,6 +315,71 @@ def build_error_envelope( return envelope +#: The six canonical error codes (keys of the message map are exactly the enum). +_ERROR_CODE_SET: frozenset[str] = frozenset(_SAFE_ERROR_MESSAGE) + + +def is_error_payload(raw: object) -> bool: + """True if a tool's returned payload is already an error frame (``success: false``). + + Such a payload must be surfaced with the wire-level ``isError`` bit set, not + wrapped as a success — otherwise a client branching on ``isError`` sees a failure + reported as a successful call. + """ + return isinstance(raw, dict) and raw.get("success") is False + + +def build_returned_error_envelope( + tool_name: str, + raw: dict[str, Any], + *, + request_id: str, + elapsed_ms: float, +) -> dict[str, Any]: + """Normalize a tool-RETURNED ``{"success": false, ...}`` payload into the error + frame, closing ``error_code`` to the canonical enum and re-augmenting ``_meta``. + + Distinct from :func:`build_error_envelope` (which classifies from an HTTP status): + here the payload already carries an ``error_code``/``message``, so they are + preserved when valid and defaulted when not. All caller-derived strings are + sanitized; an off-enum ``error_code`` collapses to ``internal``. + """ + code = raw.get("error_code") + error_code: ErrorCode = cast("ErrorCode", code) if code in _ERROR_CODE_SET else "internal" + raw_message = raw.get("message") + message = ( + sanitize_message(raw_message) + if isinstance(raw_message, str) and raw_message + else _SAFE_ERROR_MESSAGE[error_code] + ) + retryable = raw.get("retryable") + if not isinstance(retryable, bool): + retryable = error_code in ("upstream_unavailable", "rate_limited") + recovery = raw.get("recovery_action") + if not isinstance(recovery, str) or not recovery: + recovery = _GENERIC_RECOVERY_ACTION[error_code] + + envelope: dict[str, Any] = { + "success": False, + "error_code": error_code, + "message": message, + "retryable": retryable, + "recovery_action": recovery, + } + field_errors = raw.get("field_errors") + if isinstance(field_errors, list): + cleaned = [sanitize_message(str(n))[:64] for n in field_errors if str(n)] + if cleaned: + envelope["field_errors"] = cleaned + envelope["field"] = cleaned[0] + elif isinstance(raw.get("field"), str) and raw["field"]: + envelope["field"] = sanitize_message(raw["field"])[:64] + envelope["_meta"] = _augment_meta( + {}, tool_name=tool_name, request_id=request_id, elapsed_ms=elapsed_ms + ) + return envelope + + def classify_status(status_code: int) -> tuple[ErrorCode, bool]: """Map an HTTP status to ``(error_code, retryable)`` in the closed enum.""" if status_code in _STATUS_CODE_MAP: diff --git a/stringdb_link/mcp/error_passthrough.py b/stringdb_link/mcp/error_passthrough.py index a3da9cf..a4fdc76 100644 --- a/stringdb_link/mcp/error_passthrough.py +++ b/stringdb_link/mcp/error_passthrough.py @@ -163,12 +163,35 @@ async def run_with_structured_errors(arguments: dict[str, Any]) -> ToolResult: elapsed_ms = (time.perf_counter() - start) * 1000 raw = result.structured_content if isinstance(result.structured_content, dict) else {} - success_envelope = envelope.build_success_envelope( - tool_name, - raw, - request_id=envelope.new_request_id(), - elapsed_ms=elapsed_ms, + return finalize_tool_result( + tool_name, raw, request_id=envelope.new_request_id(), elapsed_ms=elapsed_ms ) - return ToolResult(structured_content=success_envelope) object.__setattr__(component, "run", run_with_structured_errors) + + +def finalize_tool_result( + tool_name: str, + raw: dict[str, Any], + *, + request_id: str, + elapsed_ms: float, +) -> ToolResult: + """Shape a tool's SUCCESSFULLY-RETURNED payload into the final ToolResult. + + A payload that is already an error frame (``success: false``) is surfaced as an + error WITH the wire-level ``isError`` bit set — never re-wrapped as a success. + Without this, a route/tool that RETURNS ``{"success": false, ...}`` (rather than + raising) would be wrapped as ``{"success": true, "result": {...}}`` with + ``isError: false``, hiding the failure from a client that branches on ``isError``. + """ + if envelope.is_error_payload(raw): + error_envelope = envelope.build_returned_error_envelope( + tool_name, raw, request_id=request_id, elapsed_ms=elapsed_ms + ) + return ToolResult(structured_content=error_envelope, is_error=True) + + success_envelope = envelope.build_success_envelope( + tool_name, raw, request_id=request_id, elapsed_ms=elapsed_ms + ) + return ToolResult(structured_content=success_envelope) diff --git a/stringdb_link/models/responses.py b/stringdb_link/models/responses.py index 990b479..9ab7d9b 100644 --- a/stringdb_link/models/responses.py +++ b/stringdb_link/models/responses.py @@ -449,6 +449,20 @@ class LinkInfo(BaseResponse): "example": "https://version-12-0.string-db.org/cgi/network?networkId=abc123" }, ) + output_format: str = Field( + "json", + description="The requested output_format this link was rendered for", + json_schema_extra={"example": "json"}, + ) + formatted: str | None = Field( + None, + description=( + "STRING's serialization of the link in the requested output_format " + "(tsv/tsv-no-header/xml). Null for json — the structured 'url' IS the " + "json representation." + ), + json_schema_extra={"example": "url\nhttps://version-12-0.string-db.org/cgi/link?to=abc123"}, + ) class ErrorResponse(BaseResponse): diff --git a/stringdb_link/services/stringdb_service.py b/stringdb_link/services/stringdb_service.py index 72f9f2e..585a7bb 100644 --- a/stringdb_link/services/stringdb_service.py +++ b/stringdb_link/services/stringdb_service.py @@ -7,6 +7,7 @@ from __future__ import annotations +import re from typing import TYPE_CHECKING, Any, cast from pydantic import ValidationError as PydanticValidationError @@ -48,34 +49,44 @@ from stringdb_link.models.responses import LinkInfo, PPIEnrichmentResult -#: STRING's ``interaction_partners`` endpoint caps returned partners per protein at -#: 500. We always request this fixed page so ``total_count`` is a property of the -#: result set (invariant of the caller's ``limit``) and truncate client-side. -_PARTNER_UPSTREAM_LIMIT = 500 - #: Field named in the invalid_input envelope when STRING reports an in-band error #: about the caller-supplied background proteome. _BACKGROUND_FIELD = "background_string_identifiers" -#: Fixed, server-authored message. The upstream STRING error prose is NEVER echoed -#: (it is caller-influenceable); only this constant and the offending field name +#: Default offending field for any OTHER STRING in-band error: the query proteins are +#: what STRING validates (unknown/misspelled identifiers, unsupported species, etc.). +_DEFAULT_INPUT_FIELD = "identifiers" +#: Fixed, server-authored messages. The upstream STRING error prose is NEVER echoed +#: (it is caller-influenceable); only these constants and the offending field name #: reach the caller (security posture). _BACKGROUND_ERROR_MESSAGE = ( "The custom background proteome is invalid: it must be a superset of the query " "identifiers. Adjust 'background_string_identifiers' so it contains every query " "protein, or omit it to use the whole-genome background." ) -_GENERIC_INPUT_MESSAGE = "The request was rejected by the upstream STRING API as invalid." +_GENERIC_INPUT_MESSAGE = ( + "The upstream STRING API rejected the request as invalid; check the query " + "'identifiers' (and 'species'). This is not a transient outage — retrying " + "unchanged will fail identically." +) + +#: STRING in-band error type -> the offending input field it names. Any type not +#: listed falls back to ``_DEFAULT_INPUT_FIELD`` — the general class is handled, not +#: just the one known case (a new STRING error type still yields invalid_input with a +#: named field, never a false retryable upstream_unavailable or a field-less error). +_STRING_ERROR_FIELD: dict[str, str] = { + "background_error": _BACKGROUND_FIELD, +} def _raise_if_string_error(payload: object) -> None: """Map STRING's in-band error shape onto a client-side ``invalid_input``. STRING returns HTTP 200 with a body like ``[{"error": "background_error", - "message": "..."}]`` for a bad request (e.g. a background that is not a - superset of the query). Left unhandled, building a typed model from that body - raises a validation error that the service mis-maps to ``upstream_unavailable`` - (retryable) — a false "the upstream is down, retry forever". Detect the shape - here and raise ``ValidationError`` (400 → invalid_input, non-retryable) naming + "message": "..."}]`` for a bad request. Left unhandled, building a typed model + from that body raises a validation error that the service mis-maps to + ``upstream_unavailable`` (retryable) — a false "the upstream is down, retry + forever". Detect the shape here for the GENERAL class (any ``{"error": ...}`` + item) and raise ``ValidationError`` (400 → invalid_input, non-retryable) naming the offending parameter, without echoing the upstream prose. """ item: dict[str, object] | None = None @@ -85,9 +96,49 @@ def _raise_if_string_error(payload: object) -> None: item = payload if item is None or not item.get("error"): return - if item.get("error") == "background_error": + error_type = str(item.get("error")) + if error_type == "background_error": raise ValidationError(_BACKGROUND_ERROR_MESSAGE, field=_BACKGROUND_FIELD) - raise ValidationError(_GENERIC_INPUT_MESSAGE) + field = _STRING_ERROR_FIELD.get(error_type, _DEFAULT_INPUT_FIELD) + raise ValidationError(_GENERIC_INPUT_MESSAGE, field=field) + + +def _extract_url(raw: str) -> str: + """Pull the shareable URL out of STRING's tsv/tsv-no-header/xml link response. + + get_link conveys the same URL in every format (``url\\n`` for tsv, the bare + line for tsv-no-header, ``...`` for xml). Match the first ``http(s)`` + token; fall back to the stripped body so the structured ``url`` is never empty. + """ + # Exclude whitespace and markup delimiters so the trailing ```` in the xml + # form (``https://...``) is not swallowed into the URL. + match = re.search(r"https?://[^\s<>\"']+", raw) + if match: + return match.group(0) + return raw.strip() + + +def _limit_partners_per_protein( + partners: list[InteractionPartner], limit: int +) -> list[InteractionPartner]: + """Return at most ``limit`` partners PER QUERY PROTEIN, preserving order. + + STRING's ``limit`` is per-protein and its rows arrive grouped by the query + protein (``string_id_a`` / ``stringId_A``), most-confident first. Grouping here + — rather than a global ``partners[:limit]`` — guarantees every queried protein is + represented: a global head-slice returns only the first protein's partners once + it has ``limit`` of them and silently omits every later protein. + """ + counts: dict[str, int] = {} + kept: list[InteractionPartner] = [] + for partner in partners: + query_id = partner.string_id_a + seen = counts.get(query_id, 0) + if seen >= limit: + continue + counts[query_id] = seen + 1 + kept.append(partner) + return kept class StringDBService: @@ -279,19 +330,20 @@ async def get_interaction_partners( network_type=request.network_type, ) - # Fetch a fixed, limit-independent page from STRING (its per-protein cap - # is 500) and truncate client-side. This keeps ``total_count`` a property - # of the result set, invariant of the caller's ``limit`` (an honest total), - # and lets ``truncated`` signal that more partners exist. + # STRING's ``limit`` is PER PROTEIN. Fetch the FULL partner set (no upstream + # limit) so ``total_count`` is the true count, invariant of the caller's + # ``limit``; then apply ``limit`` PER QUERY PROTEIN so every queried protein + # is represented (a global head-slice would return only the first protein's + # partners and omit every later protein entirely). all_partners = await self._cached_get_interaction_partners( identifiers=tuple(request.identifiers), species=request.species, - limit=_PARTNER_UPSTREAM_LIMIT, + limit=None, required_score=round(request.required_score * 1000), network_type=request.network_type.value, ) + limited = _limit_partners_per_protein(all_partners, request.limit) total = len(all_partners) - limited = all_partners[: request.limit] self.logger.info( "Successfully retrieved interaction partners", @@ -324,7 +376,7 @@ async def _cached_get_interaction_partners( self, identifiers: tuple[str, ...], species: int | None, - limit: int, + limit: int | None, required_score: int, network_type: str, ) -> list[InteractionPartner]: @@ -781,14 +833,18 @@ async def get_ppi_enrichment(self, request: PPIEnrichmentRequest) -> PPIEnrichme msg = f"Failed to get PPI enrichment: {e}" raise StringDBServiceError(msg) from e - async def get_network_link(self, request: LinkRequest) -> LinkInfo: + async def get_network_link(self, request: LinkRequest, output_format: str = "json") -> LinkInfo: """Get shareable link to STRING webpage for the network. Args: request: Link generation request + output_format: STRING serialization to render (json/tsv/tsv-no-header/xml). + Every format conveys the same shareable URL; for non-json formats the + raw STRING text is returned in ``LinkInfo.formatted`` and the URL is + extracted into ``LinkInfo.url`` so the MCP result is never empty. Returns: - Link information with URL + Link information with URL (and, for non-json, the formatted serialization) Raises: StringDBServiceError: If the operation fails @@ -807,26 +863,30 @@ async def get_network_link(self, request: LinkRequest) -> LinkInfo: "network_flavor": request.network_flavor.value, } - from stringdb_link.models.stringdb import OutputFormat - + fmt = OutputFormat(output_format) result = await self.client.get_link( identifiers=request.identifiers, species=request.species, - output_format=OutputFormat.JSON, + output_format=fmt, **link_params, ) - # Extract URL from result - url = result.get("url", str(result)) if isinstance(result, dict) else str(result) - from stringdb_link.models.responses import LinkInfo - link_info = LinkInfo(url=url) + if fmt == OutputFormat.JSON: + # client.get_link already unwrapped STRING's [""] to the url string + # (or a {"url": ...} dict for defensive parity). + url = result.get("url", str(result)) if isinstance(result, dict) else str(result) + link_info = LinkInfo(url=url, output_format=fmt.value, formatted=None) + else: + raw = str(result) + link_info = LinkInfo(url=_extract_url(raw), output_format=fmt.value, formatted=raw) # Count/status only — never the raw identifiers or generated URL (PII). self.logger.info( "Successfully generated network link", identifier_count=len(request.identifiers), + output_format=fmt.value, ) return link_info diff --git a/tests/unit/test_contract_rework_v1.py b/tests/unit/test_contract_rework_v1.py new file mode 100644 index 0000000..d3c85b4 --- /dev/null +++ b/tests/unit/test_contract_rework_v1.py @@ -0,0 +1,173 @@ +"""Regression tests for the Codex-reviewed contract-hardening rework. + +Covers the six fixes from the PR #34 review: + +1. get_interaction_partners paginates PER PROTEIN (no global head-slice) with a + true, limit-invariant total. +2. A tool that RETURNS ``{"success": false}`` yields ``isError: true``. +3. get_network_link keeps the four real STRING formats and shapes tsv/xml into a + structured result (never 422, never empty). +5. An empty upstream image body is an error, never ``success`` with 0 bytes. +6. ANY STRING in-band ``{"error": ...}`` becomes ``invalid_input`` naming a field. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from stringdb_link.exceptions import ValidationError +from stringdb_link.mcp.error_passthrough import finalize_tool_result +from stringdb_link.models.requests import InteractionPartnersRequest, LinkRequest +from stringdb_link.services.stringdb_service import StringDBService, _raise_if_string_error + + +def _raw_partner(query_string_id: str, partner_name: str) -> dict[str, Any]: + """A raw STRING interaction-partner row keyed by aliased field names.""" + return { + "stringId_A": query_string_id, + "stringId_B": f"9606.{partner_name}", + "preferredName_A": query_string_id.split(".")[-1], + "preferredName_B": partner_name, + "ncbiTaxonId": 9606, + "score": 0.9, + "nscore": 0.0, + "fscore": 0.0, + "pscore": 0.0, + "ascore": 0.9, + "escore": 0.9, + "dscore": 0.9, + "tscore": 0.9, + } + + +def _service_with_client(**client_methods: Any) -> tuple[StringDBService, MagicMock]: + client = MagicMock() + for name, value in client_methods.items(): + setattr(client, name, value) + return StringDBService(client=client, logger=MagicMock()), client + + +# --- Issue 2: returned {"success": false} -> isError: true -------------------- +def test_returned_error_dict_sets_is_error() -> None: + result = finalize_tool_result( + "resolve_protein_identifiers", + {"success": False, "error_code": "invalid_input", "message": "bad", "field": "species"}, + request_id="r1", + elapsed_ms=1.0, + ) + assert result.is_error is True + body = result.structured_content + assert body is not None + assert body["success"] is False + assert body["error_code"] == "invalid_input" + assert body["field"] == "species" + + +def test_returned_error_dict_off_enum_code_collapses_to_internal() -> None: + result = finalize_tool_result( + "resolve_protein_identifiers", + {"success": False, "error_code": "validation_failed"}, + request_id="r1", + elapsed_ms=1.0, + ) + assert result.is_error is True + assert result.structured_content["error_code"] == "internal" + + +def test_returned_success_dict_is_not_error() -> None: + result = finalize_tool_result( + "get_interaction_partners", + {"partners": [], "total_count": 0}, + request_id="r1", + elapsed_ms=1.0, + ) + assert result.is_error is False + assert result.structured_content["success"] is True + + +# --- Issue 6: ANY in-band {"error": ...} -> invalid_input naming a field ------ +def test_background_error_names_background_field() -> None: + with pytest.raises(ValidationError) as exc: + _raise_if_string_error([{"error": "background_error", "message": "x"}]) + assert exc.value.field == "background_string_identifiers" + + +def test_unknown_in_band_error_still_names_a_field() -> None: + with pytest.raises(ValidationError) as exc: + _raise_if_string_error([{"error": "some_new_string_error_type", "message": "x"}]) + # The GENERAL class is handled: a field is always named (never field-less). + assert exc.value.field == "identifiers" + + +def test_non_error_payload_does_not_raise() -> None: + _raise_if_string_error([{"term": "GO:0001", "description": "ok"}]) + + +# --- Issue 1: per-protein pagination + limit-invariant total ------------------ +@pytest.mark.asyncio +async def test_interaction_partners_paginate_per_protein() -> None: + # TP53 has 3 partners, MDM2 has 2 — a global [:limit=2] would drop MDM2 entirely. + raw = [ + _raw_partner("9606.TP53", "A"), + _raw_partner("9606.TP53", "B"), + _raw_partner("9606.TP53", "C"), + _raw_partner("9606.MDM2", "D"), + _raw_partner("9606.MDM2", "E"), + ] + svc, client = _service_with_client( + get_interaction_partners=AsyncMock(return_value=raw), + ) + request = InteractionPartnersRequest(identifiers=["TP53", "MDM2"], species=9606, limit=2) + resp = await svc.get_interaction_partners(request) + + # The full set was fetched (limit omitted upstream), not a hardcoded page. + assert client.get_interaction_partners.await_args.kwargs["limit"] is None + # True, limit-invariant total. + assert resp.total_count == 5 + assert resp.truncated is True + # BOTH query proteins are represented (2 each), MDM2 not omitted. + queries = {p.string_id_a for p in resp.partners} + assert queries == {"9606.TP53", "9606.MDM2"} + assert len(resp.partners) == 4 + + +# --- Issue 3: get_network_link keeps tsv/xml, shapes them structurally -------- +@pytest.mark.asyncio +async def test_network_link_tsv_returns_structured_url() -> None: + tsv = "url\nhttps://version-12-0.string-db.org/cgi/link?to=DEADBEEF\n" + svc, _ = _service_with_client(get_link=AsyncMock(return_value=tsv)) + request = LinkRequest(identifiers=["TP53", "MDM2"], species=9606) + info = await svc.get_network_link(request, output_format="tsv") + + assert info.output_format == "tsv" + assert info.url == "https://version-12-0.string-db.org/cgi/link?to=DEADBEEF" + assert info.formatted == tsv # the raw STRING serialization is preserved, not dropped + + +@pytest.mark.asyncio +async def test_network_link_xml_extracts_clean_url() -> None: + xml = ( + '\n\n\n' + "https://version-12-0.string-db.org/cgi/link?to=CAFE\n" + "\n\n" + ) + svc, _ = _service_with_client(get_link=AsyncMock(return_value=xml)) + info = await svc.get_network_link(LinkRequest(identifiers=["TP53"], species=9606), "xml") + # The trailing markup must NOT be swallowed into the URL. + assert info.url == "https://version-12-0.string-db.org/cgi/link?to=CAFE" + assert info.formatted == xml + + +@pytest.mark.asyncio +async def test_network_link_json_has_no_formatted() -> None: + svc, _ = _service_with_client( + get_link=AsyncMock(return_value="https://version-12-0.string-db.org/cgi/link?to=AA") + ) + request = LinkRequest(identifiers=["TP53"], species=9606) + info = await svc.get_network_link(request, output_format="json") + assert info.output_format == "json" + assert info.url.endswith("to=AA") + assert info.formatted is None diff --git a/tests/unit/test_response_envelope_v1.py b/tests/unit/test_response_envelope_v1.py index 7605e41..0c75983 100644 --- a/tests/unit/test_response_envelope_v1.py +++ b/tests/unit/test_response_envelope_v1.py @@ -212,6 +212,32 @@ async def test_image_tool_returns_base64_success_envelope(facade: Any) -> None: assert body["_meta"]["unsafe_for_clinical_use"] is True +@pytest.mark.asyncio +async def test_empty_image_body_is_error_not_success(facade: Any) -> None: + """An empty upstream image body must surface as an error, never success with + 0 bytes and an empty base64 string (that would be a silent-empty success).""" + image = NetworkImageResponse( + image=NetworkImage(image_data=b"", image_format="image", content_type="image/png") + ) + with patch( + "stringdb_link.services.stringdb_service.StringDBService.get_network_image", + new_callable=AsyncMock, + return_value=image, + ): + async with Client(facade) as client: + result = await client.call_tool( + "get_network_image", + {"identifiers": ["nup100"], "species": 4932}, + raise_on_error=False, + ) + + body = result.structured_content + assert body is not None + assert result.is_error is True + assert body["success"] is False + assert body["error_code"] == "upstream_unavailable" + + @pytest.mark.asyncio async def test_every_tool_declares_read_only_open_world_annotations(facade: Any) -> None: """Every stringdb-link tool is a read-only STRING lookup (open world)."""