From fca1f00a678cb436d72349c9d0905a9c6dd0eb3d Mon Sep 17 00:00:00 2001 From: arpan Date: Fri, 4 Sep 2026 00:01:29 +0530 Subject: [PATCH 1/2] item 6b: the outcome table and header-body validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SPEC-v0.2 §6.8, §6.4, §6.2, §6.6. Second of four stages for item 6, and the safety core: pure functions, so the tables can be read against the spec line by line without an HTTP server in the way. outcome.py is §6.8 and nothing else. Its asymmetry is the product: only two observations assert the remote did nothing — the connection was never established, so no request byte can have been written, and the peer said in band, before dispatch, that it refused. Everything after the first byte is AMBIGUOUS, including -32603, including a tool's isError, and including a resultType the gateway does not recognize. That last one is deliberately against the client rule: the revision tells clients to read an absent resultType as complete for compatibility with older servers, and the gateway refuses those servers at the version check, so on a response it accepts, absence means malformed. not_executed_on_error applies to a tool-level isError and to nothing else — not to a JSON-RPC error, and not to a response that could not be parsed. An operator asserted something about errors, not about answers nobody can read. mcp.py is §6.4: the headers are checked and the body is believed. Every body is parsed, every mirrored header validated against it, and interception is decided from the body's method and params.name. Validation is conditional on the headers existing — required on 2026-07-28, optional on the 2025 revisions — but a header that is present is validated on every revision, because a header disagreeing with the body is the hazard §6.4 is about regardless of what the revision says about mirroring. The base64 sentinel is decoded before comparing, and one that will not decode is a refusal rather than a comparison against the wrapper: an unvalidatable header on a request about to be forwarded fails closed. Integers compare numerically, or every well-formed request annotating a numeric parameter would be refused. find_unrepresentable returns the JSON pointer to the first float anywhere in the arguments (§6.6). No rounding, no truncation, no coercion: 0.1 and 0.10 are the same money and different hashes. T20's and T24's remaining halves — that the upstream's request count stays zero, and that its own response reaches the client unchanged — need a socket, and land with 6c. --- src/ctrlrun/gateway/mcp.py | 317 ++++++++++++++++++++++++++ src/ctrlrun/gateway/outcome.py | 195 ++++++++++++++++ tests/test_mcp.py | 392 +++++++++++++++++++++++++++++++++ tests/test_outcome.py | 266 ++++++++++++++++++++++ 4 files changed, 1170 insertions(+) create mode 100644 src/ctrlrun/gateway/mcp.py create mode 100644 src/ctrlrun/gateway/outcome.py create mode 100644 tests/test_mcp.py create mode 100644 tests/test_outcome.py diff --git a/src/ctrlrun/gateway/mcp.py b/src/ctrlrun/gateway/mcp.py new file mode 100644 index 00000000..326bb210 --- /dev/null +++ b/src/ctrlrun/gateway/mcp.py @@ -0,0 +1,317 @@ +"""Request parsing and header-body validation. Build-list item 6b; SPEC-v0.2 §6.2, §6.4, §6.6. + +**The headers are checked; the body is believed.** The revision mirrors `method`, +`params.name` and annotated tool parameters into `Mcp-Method`, `Mcp-Name` and +`Mcp-Param-{Name}` so intermediaries can route without parsing bodies — and taking that +shortcut is exactly the hazard the mirroring rule exists to prevent, a load balancer routing +on the header while the server executes on the body. So the gateway parses every body it +forwards, validates every header against it, and decides what to intercept from the body. + +Pure: bytes and headers in, a parsed request or a refusal out. No sockets and no store, so +§6.11's refusal table can be read against this module line by line. +""" + +from __future__ import annotations + +import base64 +import binascii +import json +from collections.abc import Mapping +from dataclasses import dataclass, field +from typing import Any, Final + +#: SPEC-v0.2 §6.2 — the revision this gateway is written against. +CURRENT_REVISION: Final = "2026-07-28" + +#: And the 2025 revisions it accepts in passthrough mode. The deprecated two-endpoint +#: HTTP+SSE transport of 2024-11-05 is not accepted, on either side (§6.2, §12). +LEGACY_REVISIONS: Final = frozenset({"2025-11-25", "2025-06-18", "2025-03-26"}) +ACCEPTED_REVISIONS: Final = frozenset({CURRENT_REVISION}) | LEGACY_REVISIONS + +#: An absent `MCP-Protocol-Version` is treated as this, which is that era's own +#: backwards-compatibility rule. It costs nothing: a modern upstream will reject the +#: forwarded request itself, and §6.8 maps that to `FAILED`. +LEGACY_DEFAULT_REVISION: Final = "2025-03-26" + +#: §6.4 — a body the gateway will not read is a body it cannot decide about. +DEFAULT_MAX_BODY_BYTES: Final = 1024 * 1024 + +#: §6.3 — `tools/call` and nothing else. Every other method is relayed unchanged. +INTERCEPTED_METHOD: Final = "tools/call" + +_PARAM_PREFIX: Final = "mcp-param-" +_SENTINEL_OPEN: Final = "=?base64?" +_SENTINEL_CLOSE: Final = "?=" + +#: JSON-RPC codes this module emits, from §6.11's table. +PARSE_ERROR: Final = -32700 +INVALID_REQUEST: Final = -32600 +HEADER_MISMATCH: Final = -32020 +UNSUPPORTED_PROTOCOL_VERSION: Final = -32022 + + +@dataclass(frozen=True) +class Refusal: + """A request the gateway will not forward (SPEC-v0.2 §6.11). + + Everything this module produces is refused *before* an Action exists, so it leaves no + receipt and no events — only a log line. `code` is `None` where the refusal has no + JSON-RPC body at all, which is the size limit. + """ + + http_status: int + code: int | None + message: str + + +@dataclass(frozen=True) +class ParsedRequest: + """One JSON-RPC message the gateway is willing to forward (SPEC-v0.2 §6.4, §6.6).""" + + revision: str + document: Mapping[str, Any] + method: str | None + intercept: bool + is_response: bool = False + tool_name: str | None = None + arguments: Mapping[str, Any] = field(default_factory=dict) + + @property + def is_legacy(self) -> bool: + return self.revision in LEGACY_REVISIONS + + +def parse_request( + body: bytes, + headers: Mapping[str, str], + *, + max_body_bytes: int = DEFAULT_MAX_BODY_BYTES, +) -> ParsedRequest | Refusal: + """Parse and validate one request body against its headers (SPEC-v0.2 §6.4). + + The order is the order of §6.11: size, then JSON, then JSON-RPC shape, then the protocol + version, then the mirrored headers. Each refusal happens before anything is forwarded. + """ + if len(body) > max_body_bytes: + return Refusal( + 413, + None, + f"request body is {len(body)} bytes, over the {max_body_bytes}-byte limit", + ) + + try: + document = json.loads(body) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + return Refusal(400, PARSE_ERROR, f"request body is not valid JSON: {exc}") + + if isinstance(document, list): + # The revision is explicit that the body is a single request or notification, so + # there is no batch to attribute — and a batch the gateway cannot attribute to one + # principal and one action is exactly the thing it must not pass through. + return Refusal(400, INVALID_REQUEST, "a JSON-RPC batch has no single action to decide") + if not isinstance(document, dict) or document.get("jsonrpc") != "2.0": + return Refusal(400, INVALID_REQUEST, "request body is not a JSON-RPC 2.0 message") + + revision = _revision_of(headers) + if revision is None: + named = ", ".join(sorted(ACCEPTED_REVISIONS)) + return Refusal( + 400, + UNSUPPORTED_PROTOCOL_VERSION, + f"unsupported MCP-Protocol-Version; this gateway accepts {named}", + ) + + method = document.get("method") + if method is None: + return _parse_response(document, revision, headers) + if not isinstance(method, str) or not method: + return Refusal(400, INVALID_REQUEST, "'method' must be a non-empty string") + + params = document.get("params") + params = params if isinstance(params, Mapping) else {} + intercept = method == INTERCEPTED_METHOD + tool_name = params.get("name") if intercept else None + if intercept and (not isinstance(tool_name, str) or not tool_name): + return Refusal(400, INVALID_REQUEST, "tools/call needs a 'params.name' string") + + arguments = params.get("arguments") + arguments = dict(arguments) if isinstance(arguments, Mapping) else {} + + mismatch = _validate_headers(headers, revision, method, tool_name, arguments) + if mismatch is not None: + return mismatch + + return ParsedRequest( + revision=revision, + document=document, + method=method, + intercept=intercept, + tool_name=tool_name if isinstance(tool_name, str) else None, + arguments=arguments, + ) + + +def _parse_response( + document: Mapping[str, Any], revision: str, headers: Mapping[str, str] +) -> ParsedRequest | Refusal: + """A JSON-RPC *response* in a POST body: legacy only (SPEC-v0.2 §6.2's table).""" + if "result" not in document and "error" not in document: + return Refusal( + 400, INVALID_REQUEST, "a JSON-RPC message needs 'method', 'result' or 'error'" + ) + if revision not in LEGACY_REVISIONS: + return Refusal( + 400, + INVALID_REQUEST, + f"a JSON-RPC response body is not permitted on {revision}; it is a legacy mechanic", + ) + mismatch = _validate_headers(headers, revision, method=None, tool_name=None, arguments={}) + if mismatch is not None: + return mismatch + return ParsedRequest( + revision=revision, document=document, method=None, intercept=False, is_response=True + ) + + +def _revision_of(headers: Mapping[str, str]) -> str | None: + """The revision this request declares, or `None` if it is outside the accepted set.""" + declared = _header(headers, "mcp-protocol-version") + if declared is None: + return LEGACY_DEFAULT_REVISION + return declared if declared in ACCEPTED_REVISIONS else None + + +def _validate_headers( + headers: Mapping[str, str], + revision: str, + method: str | None, + tool_name: str | None, + arguments: Mapping[str, Any], +) -> Refusal | None: + """Check every mirrored header against the body (SPEC-v0.2 §6.4). + + Conditional on the headers existing, and that is the whole of §6.2's amendment. On + `2026-07-28` the mirrored headers are part of the protocol and their absence is a + refusal; on the 2025 revisions they are not, so absence is not a refusal — but a header + that *is* present is validated either way, because a header disagreeing with the body is + the hazard §6.4 is about whatever the revision says about mirroring. + """ + required = revision == CURRENT_REVISION and method is not None + + declared_method = _header(headers, "mcp-method") + if declared_method is None: + if required: + return _mismatch("Mcp-Method is required on " + revision) + else: + decoded = _decoded(declared_method) + if decoded is None: + return _mismatch("Mcp-Method carries a base64 sentinel that does not decode") + if method is not None and decoded != method: + return _mismatch(f"Mcp-Method is {decoded!r} and the body's method is {method!r}") + + declared_name = _header(headers, "mcp-name") + if declared_name is None: + if required and tool_name is not None: + return _mismatch("Mcp-Name is required on " + revision) + else: + decoded = _decoded(declared_name) + if decoded is None: + return _mismatch("Mcp-Name carries a base64 sentinel that does not decode") + if tool_name is not None and decoded != tool_name: + return _mismatch(f"Mcp-Name is {decoded!r} and params.name is {tool_name!r}") + + for name, value in headers.items(): + if not name.lower().startswith(_PARAM_PREFIX): + continue + argument = name[len(_PARAM_PREFIX) :] + if argument not in arguments: + return _mismatch(f"{name} names no argument in the body") + decoded = _decoded(value) + if decoded is None: + return _mismatch(f"{name} carries a base64 sentinel that does not decode") + if not _agrees(decoded, arguments[argument]): + return _mismatch(f"{name} is {decoded!r} and the body's {argument} is not") + return None + + +def _mismatch(detail: str) -> Refusal: + return Refusal(400, HEADER_MISMATCH, f"header does not match the body: {detail}") + + +def _header(headers: Mapping[str, str], name: str) -> str | None: + """HTTP header names are case-insensitive; the caller's mapping may not be.""" + for key, value in headers.items(): + if key.lower() == name: + return value + return None + + +def _decoded(value: str) -> str | None: + """Decode the revision's `=?base64?…?=` sentinel, or `None` if it will not decode. + + A sentinel the gateway cannot decode is a header it cannot validate, and an unvalidatable + header on a request it is about to forward is a refusal — never a comparison against the + wrapper, which would pass or fail for the wrong reason. + """ + if not (value.startswith(_SENTINEL_OPEN) and value.endswith(_SENTINEL_CLOSE)): + return value + payload = value[len(_SENTINEL_OPEN) : -len(_SENTINEL_CLOSE)] + try: + return base64.b64decode(payload, validate=True).decode("utf-8") + except (binascii.Error, UnicodeDecodeError, ValueError): + return None + + +def encode_header_value(value: str) -> str: + """Wrap a value in the revision's base64 sentinel where it is not ASCII-safe (§6.4).""" + try: + value.encode("ascii") + except UnicodeEncodeError: + encoded = base64.b64encode(value.encode("utf-8")).decode("ascii") + return f"{_SENTINEL_OPEN}{encoded}{_SENTINEL_CLOSE}" + return value + + +def _agrees(declared: str, value: object) -> bool: + """Whether a header value agrees with the body's, comparing integers numerically (§6.4). + + `2000` in a header is text and the body's is an `int`; comparing them as strings would + refuse every well-formed request that annotated a numeric parameter. + """ + if isinstance(value, bool): + return declared.lower() == str(value).lower() + if isinstance(value, int): + try: + return int(declared) == value + except ValueError: + return False + if isinstance(value, str): + return declared == value + return declared == json.dumps(value, separators=(",", ":")) + + +def find_unrepresentable(arguments: object, pointer: str = "") -> str | None: + """The JSON pointer to the first `float` in `arguments`, or `None` (SPEC-v0.2 §6.6). + + MCP tool schemas routinely use `"type": "number"`, and v0.1 §2.3 rejects `float` at + construction. The gateway MUST NOT round, truncate or coerce one: `0.1` and `0.10` are + the same money and different hashes, and a gateway that quietly picked a spelling would + break the approval binding for every action that touches the value. The fix belongs in + the tool's schema — integer minor units, or decimal strings. + """ + if isinstance(arguments, bool): + return None + if isinstance(arguments, float): + return pointer or "/" + if isinstance(arguments, Mapping): + for key, value in arguments.items(): + found = find_unrepresentable(value, f"{pointer}/{key}") + if found is not None: + return found + return None + if isinstance(arguments, list): + for index, value in enumerate(arguments): + found = find_unrepresentable(value, f"{pointer}/{index}") + if found is not None: + return found + return None diff --git a/src/ctrlrun/gateway/outcome.py b/src/ctrlrun/gateway/outcome.py new file mode 100644 index 00000000..3e477847 --- /dev/null +++ b/src/ctrlrun/gateway/outcome.py @@ -0,0 +1,195 @@ +"""SPEC-v0.2 §6.8, and nothing else. Build-list item 6b. + +This module is the reason the gateway is specified at all: it is v0.1 §5.5 for a network hop, +and **its asymmetry MUST NOT be inverted**. Only two kinds of observation assert that the +remote did nothing — + +- the connection was never established, so no request byte can have been written; +- the peer said, in band and before dispatch, that it rejected the request; + +— and everything else after the first byte is `AMBIGUOUS`. There is no option that relaxes +this, and adding one would remove the only guarantee this library makes. + +Pure: it takes a description of what was observed and returns what to record and what to +send. No sockets, no store, no policy. The gateway's transport layer is responsible for +mapping its client's exceptions onto `Transport` honestly — in particular for using a fresh +connection per intercepted call, without which `NEVER_CONNECTED` is not provable (§6.8). +""" + +from __future__ import annotations + +from dataclasses import dataclass +from enum import StrEnum +from typing import Final + +from ..effect import EffectState + +#: SPEC-v0.2 §6.10 — the two codes this module synthesizes. Frozen in §11. +AMBIGUOUS_CODE: Final = -41010 +NOT_EXECUTED_CODE: Final = -41011 + +AMBIGUOUS_TOKEN: Final = "ctrlrun.upstream_ambiguous" +NOT_EXECUTED_TOKEN: Final = "ctrlrun.upstream_not_executed" + +#: The `resultType` values the gateway understands. Anything else — including its absence — +#: is a malformed answer about a consequential action, which is an unknown outcome. +COMPLETE: Final = "complete" +INPUT_REQUIRED: Final = "input_required" + +#: JSON-RPC errors the specification defines as emitted *before* dispatch (SPEC-v0.2 §6.8). +#: The peer is telling you, in band, that it rejected the request rather than running the +#: method — the closest thing MCP offers to v0.1's `NotExecuted`. +#: +#: The set is closed and small, and `-32603 Internal error` is not in it and never will be: +#: -32700 parse error the body never became a request +#: -32600 invalid request rejected as a JSON-RPC message +#: -32601 method not found there is no method to run +#: -32602 invalid params JSON-RPC defines it as validation, which precedes invocation +#: -32020 HeaderMismatch the revision requires rejection before processing +#: -32021 MissingRequiredClientCapability raised from `_meta` before the method is reached +#: -32022 UnsupportedProtocolVersion refused for its version, not executed +PRE_DISPATCH_ERROR_CODES: Final = frozenset( + {-32700, -32600, -32601, -32602, -32020, -32021, -32022} +) + + +class Transport(StrEnum): + """What the transport observed, where no JSON-RPC message came back (SPEC-v0.2 §6.8).""" + + #: DNS failure, connection refused, TLS handshake failure, connect timeout. The only + #: member that asserts non-execution, and only because no request byte can have been + #: written: a pooled connection the upstream closed while idle fails on *write*, which is + #: indistinguishable from a request that arrived, so intercepted calls never reuse one. + NEVER_CONNECTED = "never_connected" + + #: Write timeout, read timeout, connection reset, protocol error, TLS failure after the + #: request was sent. + AFTER_REQUEST_SENT = "after_request_sent" + + #: A body that is not valid JSON, or not a JSON-RPC message, or whose `id` does not + #: match; and any HTTP status with no parseable JSON-RPC body at all. + UNREADABLE_RESPONSE = "unreadable_response" + + #: An SSE stream that closed before delivering a final response. + STREAM_ENDED_EARLY = "stream_ended_early" + + #: The client went away mid-stream. Closing the stream is cancellation under this + #: revision, and the upstream may already have committed. + CLIENT_DISCONNECTED = "client_disconnected" + + +@dataclass(frozen=True) +class UpstreamResult: + """A well-formed JSON-RPC *result* from the upstream (SPEC-v0.2 §6.8).""" + + result_type: str | None + is_error: bool = False + + +@dataclass(frozen=True) +class UpstreamError: + """A well-formed JSON-RPC *error* from the upstream.""" + + code: int + + +@dataclass(frozen=True) +class UpstreamStatus: + """An HTTP status carrying no JSON-RPC message the gateway could read.""" + + status: int + has_www_authenticate: bool = False + + +Observed = Transport | UpstreamResult | UpstreamError | UpstreamStatus + + +@dataclass(frozen=True) +class GatewayOutcome: + """What to record for the effect, and what the client gets (SPEC-v0.2 §6.8, §6.10). + + `effect` is `None` only for `input_required`, where §6.9 takes over and no outcome is + written at all: the reservation stays `EXECUTING` across the round trip. + + `relay` means the upstream's own response goes back untouched — rewriting a tool's result + or error would corrupt the contract between the agent and the tool. The gateway + synthesizes a response only when it never got one. + """ + + effect: EffectState | None + relay: bool = False + code: int | None = None + token: str | None = None + http_status: int | None = None + elicitation: bool = False + + +_RELAY_COMMITTED: Final = GatewayOutcome(EffectState.COMMITTED, relay=True) +_RELAY_FAILED: Final = GatewayOutcome(EffectState.FAILED, relay=True) +_RELAY_AMBIGUOUS: Final = GatewayOutcome(EffectState.AMBIGUOUS, relay=True) +_ELICITATION: Final = GatewayOutcome(None, relay=True, elicitation=True) + +_SYNTHESIZED_AMBIGUOUS: Final = GatewayOutcome( + EffectState.AMBIGUOUS, code=AMBIGUOUS_CODE, token=AMBIGUOUS_TOKEN, http_status=502 +) +_SYNTHESIZED_NOT_EXECUTED: Final = GatewayOutcome( + EffectState.FAILED, code=NOT_EXECUTED_CODE, token=NOT_EXECUTED_TOKEN, http_status=502 +) +#: Cancellation: the effect is unknown, and there is nobody left to tell. +_CANCELLED: Final = GatewayOutcome( + EffectState.AMBIGUOUS, code=AMBIGUOUS_CODE, token=AMBIGUOUS_TOKEN, http_status=None +) + + +def classify(observed: Observed, *, not_executed_on_error: bool = False) -> GatewayOutcome: + """Apply SPEC-v0.2 §6.8 to one observation. + + `not_executed_on_error` is the operator's per-tool assertion that this upstream reports + errors only *before* acting (§3.1). It is the `NotExecuted` of v0.1 §5.5 in YAML: the + same claim, made by the person who knows, with the same consequences if they are wrong. + It applies to a tool-level `isError` and to nothing else — not to a JSON-RPC error, and + not to a response the gateway could not parse. + """ + if isinstance(observed, Transport): + return _transport(observed) + if isinstance(observed, UpstreamResult): + return _result(observed, not_executed_on_error) + if isinstance(observed, UpstreamError): + return _RELAY_FAILED if observed.code in PRE_DISPATCH_ERROR_CODES else _RELAY_AMBIGUOUS + return _status(observed) + + +def _transport(observed: Transport) -> GatewayOutcome: + if observed is Transport.NEVER_CONNECTED: + return _SYNTHESIZED_NOT_EXECUTED + if observed is Transport.CLIENT_DISCONNECTED: + return _CANCELLED + return _SYNTHESIZED_AMBIGUOUS + + +def _result(observed: UpstreamResult, not_executed_on_error: bool) -> GatewayOutcome: + if observed.result_type == INPUT_REQUIRED: + return _ELICITATION + if observed.result_type != COMPLETE: + # Deliberately against the client rule: the revision tells *clients* to read an + # absent `resultType` as complete, for compatibility with servers implementing + # earlier versions. The gateway refuses those servers at the version check (§6.2), so + # on a response it accepts, absence means the response is malformed — and a malformed + # answer about a consequential action is an unknown outcome, not a success. + return _RELAY_AMBIGUOUS + if not observed.is_error: + return _RELAY_COMMITTED + return _RELAY_FAILED if not_executed_on_error else _RELAY_AMBIGUOUS + + +def _status(observed: UpstreamStatus) -> GatewayOutcome: + # An authorization rejection is FAILED, and it matters more than it looks: an expired or + # insufficiently scoped token is the most common thing that goes wrong between a gateway + # and an upstream, and the authorization spec puts that check before the method. Nothing + # reaches the tool. Recording it AMBIGUOUS would mean a routine token refresh left an + # effect key needing a human, which is how a guarantee turns into something people + # switch off. The challenge is relayed so the client can step up and retry, and the retry + # is permitted because the record is FAILED (v0.1 §5.4). + if observed.status == 401 or (observed.status == 403 and observed.has_www_authenticate): + return _RELAY_FAILED + return _SYNTHESIZED_AMBIGUOUS diff --git a/tests/test_mcp.py b/tests/test_mcp.py new file mode 100644 index 00000000..2448e09b --- /dev/null +++ b/tests/test_mcp.py @@ -0,0 +1,392 @@ +"""Request parsing and header-body validation. Build-list item 6b; SPEC-v0.2 §6.2, §6.4, §6.6. + +The headers are checked; the body is believed. That is the whole of §6.4: the revision mirrors +`method` and `params.name` into headers so intermediaries can route without parsing bodies, +and taking that shortcut is exactly the hazard the mirroring rule exists to prevent — a load +balancer routing on the header while the server executes on the body. + +Pure functions, so T20's refusal table can be read against §6.11 line by line. Item 6c gives +them a socket and completes T20's other half: that the upstream's request count stays zero. +""" + +from __future__ import annotations + +import base64 +import json + +import pytest + +from ctrlrun.gateway.mcp import ( + ACCEPTED_REVISIONS, + CURRENT_REVISION, + DEFAULT_MAX_BODY_BYTES, + LEGACY_DEFAULT_REVISION, + Refusal, + encode_header_value, + find_unrepresentable, + parse_request, +) + +TOOLS_CALL = { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "create_refund", "arguments": {"payment_id": "txn_1", "amount": 2000}}, +} + + +def _body(document: object = None) -> bytes: + return json.dumps(TOOLS_CALL if document is None else document).encode() + + +def _headers(**overrides: str | None) -> dict[str, str]: + headers = { + "MCP-Protocol-Version": CURRENT_REVISION, + "Mcp-Method": "tools/call", + "Mcp-Name": "create_refund", + } + for key, value in overrides.items(): + name = key.replace("__", "-").replace("_", "-") + if value is None: + headers.pop(name, None) + else: + headers[name] = value + return headers + + +def _parsed(**overrides): + result = parse_request(_body(), _headers(**overrides)) + assert not isinstance(result, Refusal), result + return result + + +# --- the accepted revisions (§6.2) ----------------------------------------------------- + + +def test_the_accepted_set_is_the_current_revision_and_the_2025_ones(): + assert frozenset({"2026-07-28", "2025-11-25", "2025-06-18", "2025-03-26"}) == ACCEPTED_REVISIONS + assert CURRENT_REVISION == "2026-07-28" + + +def test_the_deprecated_2024_transport_is_not_accepted_on_either_side(): + """The two-endpoint HTTP+SSE transport of 2024-11-05 is out of scope for v0.2 (§12).""" + assert "2024-11-05" not in ACCEPTED_REVISIONS + + +def test_T20_an_unaccepted_revision_is_refused_with_32022_listing_the_accepted_ones(): + refusal = parse_request(_body(), _headers(MCP_Protocol_Version="1999-01-01")) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32022) + for revision in ACCEPTED_REVISIONS: + assert revision in refusal.message + + +def test_an_absent_version_header_is_treated_as_the_2025_03_26_era(): + """That era's own backwards-compatibility rule. It costs nothing: a modern upstream will + reject the forwarded request itself, and §6.8 maps that to FAILED.""" + parsed = _parsed(MCP_Protocol_Version=None, Mcp_Method=None, Mcp_Name=None) + + assert parsed.revision == LEGACY_DEFAULT_REVISION == "2025-03-26" + + +# --- T20: what the gateway refuses (§6.11) --------------------------------------------- + + +def test_T20_a_body_that_is_not_valid_json_is_refused_with_32700(): + refusal = parse_request(b"{not json", _headers()) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32700) + + +def test_T20_a_json_array_body_is_refused_with_32600(): + """No batch to attribute, and a batch the gateway cannot attribute to one principal and + one action is exactly the thing it must not pass through.""" + refusal = parse_request(json.dumps([TOOLS_CALL]).encode(), _headers()) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32600) + + +@pytest.mark.parametrize( + "document", + [ + {"id": 1, "method": "tools/call"}, + {"jsonrpc": "1.0", "id": 1, "method": "tools/call"}, + {"jsonrpc": "2.0", "id": 1}, + {"jsonrpc": "2.0", "id": 1, "method": 7}, + "a string", + 42, + None, + ], +) +def test_T20_a_body_that_is_not_a_jsonrpc_message_is_refused_with_32600(document): + refusal = parse_request(json.dumps(document).encode(), _headers()) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32600) + + +def test_T20_a_body_over_the_size_limit_is_refused_with_413(): + """A body the gateway will not read is a body it cannot decide about.""" + oversized = json.dumps( + {**TOOLS_CALL, "params": {"name": "create_refund", "arguments": {"pad": "x" * 200}}} + ).encode() + + refusal = parse_request(oversized, _headers(), max_body_bytes=64) + + assert isinstance(refusal, Refusal) + assert refusal.http_status == 413 + assert refusal.code is None + + +def test_the_default_body_limit_is_one_mebibyte(): + assert DEFAULT_MAX_BODY_BYTES == 1024 * 1024 + + +def test_T20_a_jsonrpc_response_body_is_refused_on_the_current_revision(): + response = {"jsonrpc": "2.0", "id": 1, "result": {"resultType": "complete"}} + + refusal = parse_request(json.dumps(response).encode(), _headers(Mcp_Method=None, Mcp_Name=None)) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32600) + + +def test_T20_a_jsonrpc_response_body_is_relayed_on_a_legacy_revision(): + """Permitted only on legacy revisions (§6.2's passthrough table).""" + response = {"jsonrpc": "2.0", "id": 1, "result": {"resultType": "complete"}} + + parsed = parse_request(json.dumps(response).encode(), {"MCP-Protocol-Version": "2025-11-25"}) + + assert not isinstance(parsed, Refusal) + assert parsed.is_response + assert not parsed.intercept + + +# --- T20: header-body validation (§6.4) ------------------------------------------------ + + +def test_T20_a_missing_Mcp_Method_is_refused_on_the_current_revision(): + refusal = parse_request(_body(), _headers(Mcp_Method=None)) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32020) + + +def test_T20_a_missing_Mcp_Name_is_refused_on_the_current_revision(): + refusal = parse_request(_body(), _headers(Mcp_Name=None)) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32020) + + +def test_T20_an_Mcp_Name_disagreeing_with_params_name_is_refused(): + refusal = parse_request(_body(), _headers(Mcp_Name="something_else")) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32020) + + +def test_T20_an_Mcp_Method_disagreeing_with_the_body_is_refused(): + refusal = parse_request(_body(), _headers(Mcp_Method="tools/list")) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32020) + + +def test_T20_an_Mcp_Param_disagreeing_with_the_body_is_refused(): + headers = _headers() + headers["Mcp-Param-payment_id"] = "txn_9" + + refusal = parse_request(_body(), headers) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32020) + + +def test_an_Mcp_Param_agreeing_with_the_body_is_accepted(): + headers = _headers() + headers["Mcp-Param-payment_id"] = "txn_1" + + assert not isinstance(parse_request(_body(), headers), Refusal) + + +def test_an_Mcp_Param_integer_is_compared_numerically(): + """§6.4 — `2000` in a header is a string; the body's is an int. Comparing them as text + would refuse every well-formed request that annotated a numeric parameter.""" + headers = _headers() + headers["Mcp-Param-amount"] = "2000" + + assert not isinstance(parse_request(_body(), headers), Refusal) + + +def test_an_Mcp_Param_integer_that_differs_numerically_is_refused(): + headers = _headers() + headers["Mcp-Param-amount"] = "2001" + + assert isinstance(parse_request(_body(), headers), Refusal) + + +def test_an_Mcp_Param_naming_an_argument_the_body_does_not_have_is_refused(): + headers = _headers() + headers["Mcp-Param-nothing"] = "x" + + assert isinstance(parse_request(_body(), headers), Refusal) + + +def test_the_base64_sentinel_is_decoded_before_comparing(): + """§6.4 — a header value that is not ASCII-safe arrives wrapped, and comparing the + wrapper against the body would refuse a request that agrees perfectly.""" + document = { + "jsonrpc": "2.0", + "id": 1, + "method": "tools/call", + "params": {"name": "créer", "arguments": {"payment_id": "txn_ü"}}, + } + encoded = base64.b64encode("créer".encode()).decode() + headers = { + "MCP-Protocol-Version": CURRENT_REVISION, + "Mcp-Method": "tools/call", + "Mcp-Name": f"=?base64?{encoded}?=", + "Mcp-Param-payment_id": encode_header_value("txn_ü"), + } + + assert not isinstance(parse_request(json.dumps(document).encode(), headers), Refusal) + + +def test_a_base64_sentinel_that_decodes_to_something_else_is_still_refused(): + encoded = base64.b64encode(b"not_the_tool").decode() + + refusal = parse_request(_body(), _headers(Mcp_Name=f"=?base64?{encoded}?=")) + + assert isinstance(refusal, Refusal) + assert refusal.code == -32020 + + +def test_a_malformed_base64_sentinel_is_refused_rather_than_compared_raw(): + """Fail closed: a sentinel the gateway cannot decode is a header it cannot validate.""" + refusal = parse_request(_body(), _headers(Mcp_Name="=?base64?not-base-64!!?=")) + + assert isinstance(refusal, Refusal) + assert refusal.code == -32020 + + +# --- T20's mirror: the 2025 revisions (§6.2) ------------------------------------------- + + +@pytest.mark.parametrize("revision", ["2025-03-26", "2025-06-18", "2025-11-25"]) +def test_T20_absent_mirrored_headers_are_not_a_refusal_on_a_legacy_revision(revision): + """They are not part of the protocol in that era, so their absence is not a refusal — + and the call is still decided from the body and forwarded.""" + parsed = parse_request(_body(), {"MCP-Protocol-Version": revision, "Mcp-Session-Id": "abc"}) + + assert not isinstance(parsed, Refusal) + assert parsed.intercept + assert parsed.tool_name == "create_refund" + + +def test_a_legacy_request_still_validates_a_header_that_is_present(): + """Conditional on the headers existing, not absent altogether: a present header that + disagrees with the body is the hazard §6.4 is about, whatever the revision.""" + refusal = parse_request( + _body(), {"MCP-Protocol-Version": "2025-11-25", "Mcp-Name": "something_else"} + ) + + assert isinstance(refusal, Refusal) + assert refusal.code == -32020 + + +# --- §6.3: what is intercepted --------------------------------------------------------- + + +def test_only_tools_call_is_intercepted(): + assert _parsed().intercept + + +@pytest.mark.parametrize( + "method", + ["tools/list", "resources/read", "prompts/get", "server/discover", "subscriptions/listen"], +) +def test_every_other_method_is_relayed_not_intercepted(method): + """`tools/list` is not an action; only calling a tool is (§6.3).""" + document = {"jsonrpc": "2.0", "id": 1, "method": method} + parsed = parse_request( + json.dumps(document).encode(), + {"MCP-Protocol-Version": CURRENT_REVISION, "Mcp-Method": method}, + ) + + assert not isinstance(parsed, Refusal) + assert not parsed.intercept + + +def test_interception_is_decided_from_the_body_never_from_the_header(): + """§6.4 — the headers are checked; the body is believed. A header claiming `tools/list` + over a `tools/call` body is a refusal, not a bypass.""" + refusal = parse_request(_body(), _headers(Mcp_Method="tools/list")) + + assert isinstance(refusal, Refusal) + assert refusal.code == -32020 + + +def test_a_notification_carries_no_id_and_is_not_intercepted(): + document = {"jsonrpc": "2.0", "method": "notifications/initialized"} + parsed = parse_request( + json.dumps(document).encode(), + {"MCP-Protocol-Version": CURRENT_REVISION, "Mcp-Method": "notifications/initialized"}, + ) + + assert not isinstance(parsed, Refusal) + assert not parsed.intercept + + +def test_a_tools_call_with_no_arguments_gets_an_empty_mapping(): + """§6.6 — `params.arguments`, or `{}` if absent.""" + document = {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {"name": "ping"}} + parsed = parse_request( + json.dumps(document).encode(), + {"MCP-Protocol-Version": CURRENT_REVISION, "Mcp-Method": "tools/call", "Mcp-Name": "ping"}, + ) + + assert parsed.arguments == {} + + +def test_a_tools_call_with_no_name_is_refused(): + document = {"jsonrpc": "2.0", "id": 1, "method": "tools/call", "params": {}} + refusal = parse_request( + json.dumps(document).encode(), + {"MCP-Protocol-Version": CURRENT_REVISION, "Mcp-Method": "tools/call"}, + ) + + assert isinstance(refusal, Refusal) + + +# --- T22's half: an argument the Action model cannot represent (§6.6) ------------------ + + +@pytest.mark.parametrize( + ("arguments", "pointer"), + [ + ({"amount": 20.0}, "/amount"), + ({"a": {"b": 1.5}}, "/a/b"), + ({"xs": [1, 2, 3.5]}, "/xs/2"), + ({"a": [{"b": 0.1}]}, "/a/0/b"), + ], +) +def test_a_float_anywhere_in_the_arguments_is_found_with_its_pointer(arguments, pointer): + """`0.1` and `0.10` are the same money and different hashes; a gateway that quietly + picked a spelling would break the approval binding for every action touching the value.""" + assert find_unrepresentable(arguments) == pointer + + +@pytest.mark.parametrize( + "arguments", + [{}, {"amount": 2000}, {"ok": True}, {"s": "0.1"}, {"n": None}, {"xs": [1, "2", None]}], +) +def test_representable_arguments_have_no_pointer(arguments): + assert find_unrepresentable(arguments) is None + + +def test_a_bool_is_not_mistaken_for_a_float_or_an_int(): + assert find_unrepresentable({"flag": False}) is None diff --git a/tests/test_outcome.py b/tests/test_outcome.py new file mode 100644 index 00000000..d0fe70e5 --- /dev/null +++ b/tests/test_outcome.py @@ -0,0 +1,266 @@ +"""SPEC-v0.2 §6.8, and nothing else. Build-list item 6b; acceptance test T24. + +This is the table the gateway exists for, and its asymmetry MUST NOT be inverted: the only +observation that asserts non-execution is one where no request byte can have been written. +Everything a peer says *after* dispatch is an outcome, and outcomes it does not describe are +`AMBIGUOUS`. + +Pure functions here, so the table can be read against the spec line by line without an HTTP +server in the way. Item 6c wires it to real sockets and completes T24's other half — that +the upstream's own response reaches the client unchanged. +""" + +from __future__ import annotations + +import pytest + +from ctrlrun import EffectState +from ctrlrun.gateway.outcome import ( + AMBIGUOUS_CODE, + NOT_EXECUTED_CODE, + PRE_DISPATCH_ERROR_CODES, + Transport, + UpstreamError, + UpstreamResult, + UpstreamStatus, + classify, +) + +COMPLETE = "complete" + + +# --- T24: the transport rows ----------------------------------------------------------- + + +def test_T24_a_connection_never_established_is_the_only_transport_FAILED(): + """The one claim in the table that asserts non-execution, and it is only provable if no + request byte can have been written (§6.8). Connection reuse is disabled for intercepted + calls precisely so this row stays true.""" + outcome = classify(Transport.NEVER_CONNECTED) + + assert outcome.effect is EffectState.FAILED + assert outcome.code == NOT_EXECUTED_CODE == -41011 + assert outcome.http_status == 502 + assert not outcome.relay + + +@pytest.mark.parametrize( + "observed", + [ + Transport.AFTER_REQUEST_SENT, + Transport.UNREADABLE_RESPONSE, + Transport.STREAM_ENDED_EARLY, + Transport.CLIENT_DISCONNECTED, + ], +) +def test_T24_every_other_transport_observation_is_AMBIGUOUS(observed): + outcome = classify(observed) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.code == AMBIGUOUS_CODE == -41010 + + +def test_T24_a_client_disconnection_returns_nothing_because_the_stream_is_gone(): + """Cancellation under this revision. The upstream may already have committed, so the + effect is AMBIGUOUS exactly as a timeout is — and there is nobody left to tell.""" + outcome = classify(Transport.CLIENT_DISCONNECTED) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.http_status is None + + +# --- T24: the result rows -------------------------------------------------------------- + + +def test_T24_a_complete_result_with_no_error_is_COMMITTED(): + outcome = classify(UpstreamResult(result_type=COMPLETE, is_error=False)) + + assert outcome.effect is EffectState.COMMITTED + assert outcome.relay + assert outcome.code is None + + +def test_T24_is_error_true_is_AMBIGUOUS_by_default(): + """The revision's own examples of a tool execution error are "API failures · Input + validation errors · Business logic errors" — the first says nothing about whether a side + effect landed.""" + outcome = classify(UpstreamResult(result_type=COMPLETE, is_error=True)) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.relay + + +def test_T24_is_error_true_is_FAILED_where_the_operator_asserted_it(): + """`not_executed_on_error: true` is the `NotExecuted` of v0.1 §5.5 in YAML: the same + assertion, made by the person who knows, with the same consequences if they are wrong.""" + outcome = classify( + UpstreamResult(result_type=COMPLETE, is_error=True), not_executed_on_error=True + ) + + assert outcome.effect is EffectState.FAILED + assert outcome.relay + + +def test_T24_not_executed_on_error_does_not_touch_a_successful_result(): + outcome = classify( + UpstreamResult(result_type=COMPLETE, is_error=False), not_executed_on_error=True + ) + + assert outcome.effect is EffectState.COMMITTED + + +@pytest.mark.parametrize("result_type", [None, "partial", "streaming", "COMPLETE", ""]) +def test_T24_an_unrecognized_or_absent_result_type_is_AMBIGUOUS(result_type): + """Deliberately against the client rule (§6.8). The revision tells *clients* to treat an + absent `resultType` as complete for compatibility with older servers; the gateway refuses + those servers at the version check, so on a response it accepts, absence means malformed + — and a malformed answer about a consequential action is an unknown outcome.""" + outcome = classify(UpstreamResult(result_type=result_type, is_error=False)) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.relay + + +def test_T24_an_unrecognized_result_type_is_AMBIGUOUS_even_when_it_claims_no_error(): + assert classify(UpstreamResult("who-knows", is_error=False)).effect is EffectState.AMBIGUOUS + + +def test_T24_an_unrecognized_result_type_outranks_not_executed_on_error(): + """The operator asserted something about *errors*, not about responses it cannot parse.""" + outcome = classify(UpstreamResult(None, is_error=True), not_executed_on_error=True) + + assert outcome.effect is EffectState.AMBIGUOUS + + +def test_T24_input_required_is_handed_to_section_6_9(): + """Neither naive reading is right: treating the first leg as completed records an outcome + the upstream never reported, and refusing the continuation makes eliciting tools + unusable. §6.9 holds the reservation open instead.""" + outcome = classify(UpstreamResult(result_type="input_required", is_error=False)) + + assert outcome.effect is None + assert outcome.elicitation + + +# --- T24: the JSON-RPC error rows ------------------------------------------------------ + + +@pytest.mark.parametrize("code", sorted(PRE_DISPATCH_ERROR_CODES)) +def test_T24_a_pre_dispatch_error_code_is_FAILED(code): + """The closest thing MCP offers to v0.1's `NotExecuted`: the peer is saying, in band, + that it rejected the request rather than running the method.""" + outcome = classify(UpstreamError(code=code)) + + assert outcome.effect is EffectState.FAILED + assert outcome.relay + + +def test_T24_the_pre_dispatch_set_is_exactly_the_seven_codes_the_spec_names(): + assert frozenset({-32700, -32600, -32601, -32602, -32020, -32021, -32022}) == ( + PRE_DISPATCH_ERROR_CODES + ) + + +@pytest.mark.parametrize("code", [-32603, -32000, -32099, -1, 0, 42, -41010]) +def test_T24_every_other_error_code_is_AMBIGUOUS(code): + """`-32603 Internal error` is not pre-dispatch and never will be. Nor is any code the + gateway does not recognize.""" + outcome = classify(UpstreamError(code=code)) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.relay + + +def test_T24_internal_error_is_AMBIGUOUS_even_with_not_executed_on_error(): + """That flag is about a tool's `isError`, not about a JSON-RPC error.""" + outcome = classify(UpstreamError(code=-32603), not_executed_on_error=True) + + assert outcome.effect is EffectState.AMBIGUOUS + + +# --- T24: the HTTP rows ---------------------------------------------------------------- + + +def test_T24_a_401_is_FAILED_so_a_retry_is_permitted_after_the_token_refreshes(): + """The most common thing that goes wrong between a gateway and an upstream. Recording it + AMBIGUOUS would mean a routine token refresh left an effect key needing a human, which is + how a guarantee turns into something people switch off.""" + outcome = classify(UpstreamStatus(status=401, has_www_authenticate=True)) + + assert outcome.effect is EffectState.FAILED + assert outcome.relay + + +def test_T24_a_401_without_a_challenge_is_still_FAILED(): + """The authorization spec puts the token check before the method either way.""" + assert classify(UpstreamStatus(401, has_www_authenticate=False)).effect is EffectState.FAILED + + +def test_T24_a_403_with_a_challenge_is_FAILED(): + """`insufficient_scope` is raised before the tool is reached.""" + outcome = classify(UpstreamStatus(403, has_www_authenticate=True)) + + assert outcome.effect is EffectState.FAILED + + +def test_T24_a_403_without_a_challenge_is_AMBIGUOUS(): + """A bare 403 is not the authorization spec's pre-dispatch refusal; it is a status with + no parseable JSON-RPC body, which is the last row of the table.""" + outcome = classify(UpstreamStatus(403, has_www_authenticate=False)) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.code == AMBIGUOUS_CODE + + +@pytest.mark.parametrize("status", [500, 502, 503, 429, 418, 404, 200]) +def test_T24_any_other_status_with_no_parseable_body_is_AMBIGUOUS(status): + outcome = classify(UpstreamStatus(status, has_www_authenticate=False)) + + assert outcome.effect is EffectState.AMBIGUOUS + assert outcome.code == AMBIGUOUS_CODE + assert not outcome.relay + + +# --- the asymmetry itself -------------------------------------------------------------- + + +def test_only_four_observations_in_the_whole_table_are_FAILED(): + """The inversion this product exists to prevent, asserted as a shape rather than a row. + + Anything after the request left the gateway is AMBIGUOUS. Only a failure to send, and a + peer stating in band that it refused before dispatch, are FAILED. + """ + failed = { + classify(Transport.NEVER_CONNECTED).effect, + classify(UpstreamError(-32602)).effect, + classify(UpstreamStatus(401, has_www_authenticate=False)).effect, + classify(UpstreamResult(COMPLETE, is_error=True), not_executed_on_error=True).effect, + } + assert failed == {EffectState.FAILED} + + after_dispatch = [ + classify(Transport.AFTER_REQUEST_SENT), + classify(Transport.UNREADABLE_RESPONSE), + classify(Transport.STREAM_ENDED_EARLY), + classify(Transport.CLIENT_DISCONNECTED), + classify(UpstreamError(-32603)), + classify(UpstreamResult(COMPLETE, is_error=True)), + classify(UpstreamResult(None, is_error=False)), + classify(UpstreamStatus(500, has_www_authenticate=False)), + ] + assert {outcome.effect for outcome in after_dispatch} == {EffectState.AMBIGUOUS} + + +def test_no_observation_maps_to_a_state_outside_the_table(): + every = [ + *(classify(member) for member in Transport), + classify(UpstreamResult(COMPLETE, is_error=False)), + classify(UpstreamResult(COMPLETE, is_error=True)), + classify(UpstreamResult("input_required", is_error=False)), + classify(UpstreamResult(None, is_error=False)), + *(classify(UpstreamError(code)) for code in (-32602, -32603)), + *(classify(UpstreamStatus(code, has_www_authenticate=False)) for code in (401, 500)), + ] + permitted = {EffectState.COMMITTED, EffectState.FAILED, EffectState.AMBIGUOUS, None} + + assert {outcome.effect for outcome in every} <= permitted From 2b247c24280ea17231ae5808f482427304253c2c Mon Sep 17 00:00:00 2001 From: arpan Date: Fri, 4 Sep 2026 00:04:05 +0530 Subject: [PATCH 2/2] fix: three guards in mcp.py that no test could distinguish MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mutation-testing §6.4 found the pattern again, and one piece of genuinely dead code. The JSON-array refusal and the malformed-sentinel refusal are each subsumed by a later guard returning the same status and code, so removing either changed nothing the tests could see. Both are still worth having — a batch should be refused for being a batch, and a sentinel that will not decode is a header the gateway cannot validate rather than one that disagrees — so the messages are what the tests now assert. Comparing an undecodable wrapper against the body would refuse for the wrong reason *and* accept a body whose value happened to equal the wrapper text. The `jsonrpc == "2.0"` check had no test of its own at all: every malformed body in the table was also caught by a later guard with the same code. Two tests now isolate it with bodies well-formed in every other respect. And `isinstance(arguments, bool)` in find_unrepresentable was dead: bools are ints, not floats, so `isinstance(True, float)` is already False. Removed, with a mutation proving a float check that *did* catch bools would be caught. --- src/ctrlrun/gateway/mcp.py | 2 -- tests/test_mcp.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/ctrlrun/gateway/mcp.py b/src/ctrlrun/gateway/mcp.py index 326bb210..2306e654 100644 --- a/src/ctrlrun/gateway/mcp.py +++ b/src/ctrlrun/gateway/mcp.py @@ -299,8 +299,6 @@ def find_unrepresentable(arguments: object, pointer: str = "") -> str | None: break the approval binding for every action that touches the value. The fix belongs in the tool's schema — integer minor units, or decimal strings. """ - if isinstance(arguments, bool): - return None if isinstance(arguments, float): return pointer or "/" if isinstance(arguments, Mapping): diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 2448e09b..09329d28 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -107,6 +107,10 @@ def test_T20_a_json_array_body_is_refused_with_32600(): assert isinstance(refusal, Refusal) assert (refusal.http_status, refusal.code) == (400, -32600) + # The generic "not a JSON-RPC message" guard would also catch a list, with the same + # status and code. The message is the only observable difference, and it is the one an + # operator needs: a batch is refused for what it is, not for being malformed. + assert "batch" in refusal.message @pytest.mark.parametrize( @@ -128,6 +132,30 @@ def test_T20_a_body_that_is_not_a_jsonrpc_message_is_refused_with_32600(document assert (refusal.http_status, refusal.code) == (400, -32600) +def test_T20_a_well_formed_call_with_the_wrong_jsonrpc_version_is_still_refused(): + """The `jsonrpc` check on its own, isolated. + + Every other malformed body in the table above is also caught by a later guard with the + same status and code, so removing this check changed nothing any of them could see. This + body is well-formed in every other respect: only the version is wrong. + """ + document = {**TOOLS_CALL, "jsonrpc": "1.0"} + + refusal = parse_request(json.dumps(document).encode(), _headers()) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32600) + + +def test_T20_a_well_formed_call_with_no_jsonrpc_member_is_still_refused(): + document = {key: value for key, value in TOOLS_CALL.items() if key != "jsonrpc"} + + refusal = parse_request(json.dumps(document).encode(), _headers()) + + assert isinstance(refusal, Refusal) + assert (refusal.http_status, refusal.code) == (400, -32600) + + def test_T20_a_body_over_the_size_limit_is_refused_with_413(): """A body the gateway will not read is a body it cannot decide about.""" oversized = json.dumps( @@ -271,6 +299,9 @@ def test_a_malformed_base64_sentinel_is_refused_rather_than_compared_raw(): assert isinstance(refusal, Refusal) assert refusal.code == -32020 + # Comparing the wrapper against the body would also refuse this one, for the wrong + # reason — and would *accept* a body whose value happened to equal the wrapper text. + assert "does not decode" in refusal.message # --- T20's mirror: the 2025 revisions (§6.2) -------------------------------------------