Skip to content

Add opt-in validation of required tool result content - #1149

Open
kele5555 wants to merge 1 commit into
modelcontextprotocol:mainfrom
kele5555:codex/strict-tool-result-content-kele5555
Open

kele5555 wants to merge 1 commit into
modelcontextprotocol:mainfrom
kele5555:codex/strict-tool-result-content-kele5555

Conversation

@kele5555

@kele5555 kele5555 commented Sep 30, 2026 •

Copy link
Copy Markdown

CallToolResult.fromJson replaces missing or null content with an empty list. Applications then cannot distinguish these responses from a valid content: [] result.

This adds validateCallToolResultContent(true) to the synchronous and asynchronous client builders. When enabled, callTool checks the raw result before converting it to CallToolResult, rejecting missing, null, or non-array content with IllegalArgumentException. An explicit empty array remains valid.

The option follows the existing McpClientFeatures.Sync/Async wiring. Validation accepts both List and Object[] representations of JSON arrays, including Jackson's USE_JAVA_ARRAY_FOR_JSON_ARRAY setting.

The option defaults to false, preserving the compatibility behavior introduced in #928. It is independent of tool output-schema validation and does not enable strict validation of other MCP messages. Validation failures do not retry the tool call or undo server-side effects.

Validation on Java 17, with both Jackson 2 and Jackson 3:

  • 280 related tests passed per profile: the 25 HTTP regression cases, 12 mapper-array cases for each Jackson implementation, and 6 feature-wiring cases, plus existing regressions.
  • Tests cover synchronous/asynchronous calls, missing/null/non-array content, error results, empty/text/structured output, unknown fields, default compatibility, JSON-RPC errors, output-schema validation, and a successful call after a validation failure.
  • The initial regression run demonstrated six missing/null-content rejection failures on the original code; these are the proposed opt-in behavior, not a claim that the documented default was unintended.
  • The full Docker-dependent suite was not run.

This is a proposed public API. Please confirm the scope and naming in the related enhancement issue before requesting review.

AI-assisted implementation, submitted for human review.

Fixes #1148

Copy link
Copy Markdown

One abstraction/coverage concern with the strict path: it now depends on the concrete shape returned by transport.unmarshalFrom(..., TypeRef<Object>).

decodeToolResultWithContentValidation accepts only:

  • a raw Map<?,?> result object, and
  • content represented as List<?>/Object[].

That is true for the Jackson configurations covered here, but McpClientTransport is mapper/transport-neutral and owns unmarshalFrom. A custom mapper/transport can legally materialize a different JSON-object/array representation for Object, which would make a valid CallToolResult fail strict validation before the normal typed conversion.

Could we pin the intended contract with a mapper-neutral/custom-transport regression (and ideally normalize through an SDK JSON-object abstraction rather than concrete Java container classes)? At minimum, documenting that strict content validation requires TypeRef<Object> to materialize JSON objects as Map and arrays as List/Object[] would avoid a hidden transport requirement.

This is opt-in, so it is not a default-path regression, but it is a new requirement on an otherwise transport-neutral client feature.

AI-assisted review; checked the current head and McpClientTransport/McpClientSession path before posting.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow strict validation of CallToolResult.content during deserialization

2 participants