[Capability] Harden structuredContent extraction - #417
Merged
chr-hertel merged 7 commits intoAug 15, 2026
Merged
Conversation
chr-hertel
requested review from
CodeWithKyrian,
Nyholm and
soyuka
as code owners
August 14, 2026 21:17
Contributor
|
Did you want me to do a merge /rebase to resolve conflicts? |
Member
Author
|
@cancan101 no, all good - just a follow up. your commits will be dropped after merging #400 in a sec |
chr-hertel
force-pushed
the
feat/resource-link-content-review
branch
from
August 14, 2026 21:25
64b3a3d to
fa158fc
Compare
chr-hertel
changed the title
structuredContent extraction, tighten ResourceLink validation
chr-hertel
changed the title
structuredContent extraction, tighten ResourceLink validationstructuredContent extraction
Three follow-ups from review of the resource_link work: * PromptResultFormatter: add a regression test proving the optional fields of a typed resource_link block (title, description, mimeType, size, annotations, _meta) survive formatting. The delegation to ResourceLink::fromArray() already landed while rebasing onto the PromptResultFormatter refactor, but the existing test only supplied uri/name and so could not catch a reintroduced field drop. * ResourceLink::fromArray(): validate optional fields consistently, the way the equivalent ResourceDefinition::fromArray() already does. description, mimeType and size now raise InvalidArgumentException instead of surfacing a TypeError (or silently coercing, for size's (int) cast); annotations goes through Annotations::tryFromArray(), which carries the is_array() guard that icons already had; icons uses Icon::listFromArray() so a non-array entry is reported in context. * ToolReference::extractStructuredContent(): never emit a list as structuredContent. The Content guard added earlier fixed only one instance of the real invariant - structuredContent must be a JSON object, and a PHP list can never be one. Tool::fromArray() already enforces the matching rule by rejecting an outputSchema whose type is not "object", so this aligns the runtime path with it. The test asserting the opposite for an array-typed outputSchema contradicted the phpstan type and the fromArray() check introduced alongside it in the same commit, so it is replaced by tests for the two list shapes.
`extractStructuredContent()` guards raw array results against being emitted as a JSON array, but the object branch handed back whatever `json_decode()` produced. A `JsonSerializable` returning a list or a scalar slipped straight through, producing exactly the `structuredContent` the array guard exists to prevent — and a return value that contradicts the method's own `array<string, mixed>|null` signature. Check the decoded value before returning it, and cover the object branch, which had no tests at all.
`outputSchema` and `structuredContent` were undocumented: the tool return value docs covered only the `content` side, and the schema generation section is about tool parameters. Add a "Structured Output" subsection covering how to declare the schema, which return values populate `structuredContent`, and why a list has to be wrapped in a key to get structured output at all.
chr-hertel
force-pushed
the
feat/resource-link-content-review
branch
from
August 15, 2026 00:50
7513d9f to
254c1cf
Compare
SEP-2106 (revision 2026-07-28) widens structuredContent to any JSON value; earlier revisions require an object. Resolve the revision per request and apply the matching rule, and warn when a declared outputSchema yields none.
Keeps the `_meta`-then-session lookup in one place instead of per handler, and exposes the negotiated revision to tool handlers.
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens structured tool output by applying protocol-aware extraction and improves ResourceLink validation and documentation.
Changes:
- Gates structured content shapes by protocol revision.
- Adds protocol resolution through
RequestContext. - Strengthens
ResourceLinkvalidation and test coverage.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/Capability/Registry/ToolReference.php |
Adds revision-aware structured-content extraction. |
src/Server/RequestContext.php |
Resolves the request protocol revision. |
src/Server/Handler/Request/CallToolHandler.php |
Applies revision-aware extraction and warnings. |
src/Schema/Content/ResourceLink.php |
Validates optional resource-link fields. |
tests/Unit/Capability/RegistryTest.php |
Tests structured-content extraction rules. |
tests/Unit/Server/RequestContextTest.php |
Tests protocol-version resolution. |
tests/Unit/Server/Handler/Request/CallToolHandlerTest.php |
Tests handler integration and warnings. |
tests/Unit/Schema/Content/ResourceLinkTest.php |
Tests invalid optional fields. |
tests/Unit/Capability/Formatter/PromptResultFormatterTest.php |
Tests preservation of resource-link fields. |
docs/server-client-communication.md |
Documents protocol access. |
docs/mcp-elements.md |
Documents structured output behavior. |
CHANGELOG.md |
Records the compatibility change. |
Suppressed comments (2)
src/Capability/Registry/ToolReference.php:80
- The protocol revision only widens which root shapes are legal; it does not override a declared
outputSchema. With a 2026 request, a tool whose schema is['type' => 'object']can return a list and this method emits it, guaranteeing a schema mismatch that strict clients may reject. The extracted value needs output-schema validation (or at minimum root-type enforcement) before it is returned.
$objectOnly = !($protocolVersion ?? ProtocolVersion::latestHandshake())->isAtLeast(ProtocolVersion::V2026_07_28);
src/Capability/Registry/ToolReference.php:87
- The new 2026 path accepts an empty list, but it still never reaches the wire:
CallToolResult::jsonSerialize()uses a truthiness check at line 147, sostructuredContent: []is omitted. Use a null check in that serializer and cover an empty-list result.
if ($objectOnly && array_is_list($toolExecutionResult)) {
return null;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ntent Returning a CallToolResult opts out of the extraction rules, so the value is still sent unchanged — but a JSON array is not valid before SEP-2106.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #400.
structuredContentfollows the negotiated protocol revision. Up to2025-11-25it must be a JSON object, soextractStructuredContent()drops a PHP list or an object serializing to a JSON array — strict clients reject the whole tool call over one. SEP-2106, part of2026-07-28, widened it to any JSON value; from there both are emitted as-is. Objects serializing to a scalar and arrays holdingContentare never emitted, in any revision:CallToolResult::$structuredContentis typed?array, and content blocks are already carried incontent.RequestContext::getProtocolVersion()resolves the revision from the request's_meta(modern era) or the session (handshake era), falling back to the newest handshake revision. First consumer of theprotocol_versionInitializeHandlerhas been writing all along, and reachable from tool handlers via argument injection.outputSchemabut returns something unsendable, or hands back a self-builtCallToolResultwhosestructuredContentthe revision does not allow — that one is passed through unchanged, since building the result is an explicit opt-out.ResourceLink::fromArray()validated optional fields inconsistently:description/mimeTypereached the constructor as aTypeError,sizewas silently(int)-cast,annotationslacked theis_array()guardiconshad. Now mirrorsResourceDefinition::fromArray().resource_linkblock keeps its optional fields throughPromptResultFormatter.Drops
testExtractStructuredContentReturnsArrayDirectlyForArrayOutputSchema, which asserted a list is emitted unconditionally; replaced by revision-specific cases.BC: tools returning a list no longer advertise
structuredContentto clients below2026-07-28; the value stays incontent.Left for the 2026-07-28 work:
Tool::fromArray()still rejects every non-objectoutputSchema, soClient::listTools()cannot parse a 2026-07-28 tool listing;CallToolResult::$structuredContentcannot carry a scalar; and the truthiness check in itsjsonSerialize()drops a legal0/false/""once that type widens.