[Capability] Harden structuredContent extraction (#417)
* Address review feedback on resource_link 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. * Apply the structuredContent object rule to object results too `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. * Document structured tool output `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. * Gate structuredContent on the negotiated protocol revision 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. * Resolve the protocol revision on RequestContext Keeps the `_meta`-then-session lookup in one place instead of per handler, and exposes the negotiated revision to tool handlers. * Document reading the negotiated revision from RequestContext * Warn when a self-built CallToolResult carries an invalid structuredContent 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 commit is contained in:
Christopher Hertel
GitHub
parent
82aac972c1
commit
becce29c41
2 changed files with 616 additions and 36 deletions
|
|
@ -477,6 +477,143 @@ class CallToolHandlerTest extends TestCase
|
|||
$this->assertArrayNotHasKey('structuredContent', $response->result->jsonSerialize());
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideStructuredContentRevisions
|
||||
*/
|
||||
public function testStructuredContentFollowsTheNegotiatedRevision(?string $negotiated, ?array $expected): void
|
||||
{
|
||||
$listResult = [['id' => 1], ['id' => 2]];
|
||||
$request = $this->createCallToolRequest('list_things', []);
|
||||
$toolReference = $this->createToolReference('list_things', static fn () => $listResult);
|
||||
|
||||
$this->session
|
||||
->method('get')
|
||||
->with('protocol_version')
|
||||
->willReturn($negotiated);
|
||||
|
||||
$this->registry
|
||||
->method('getTool')
|
||||
->willReturn($toolReference);
|
||||
|
||||
$this->referenceHandler
|
||||
->method('handle')
|
||||
->willReturn($listResult);
|
||||
|
||||
$toolReference
|
||||
->method('formatResult')
|
||||
->willReturn([new TextContent('[{"id":1},{"id":2}]')]);
|
||||
|
||||
$response = $this->handler->handle($request, $this->session);
|
||||
|
||||
$this->assertInstanceOf(Response::class, $response);
|
||||
$this->assertSame($expected, $response->result->structuredContent);
|
||||
}
|
||||
|
||||
/**
|
||||
* How a revision is resolved is {@see \Mcp\Server\RequestContext}'s business
|
||||
* and covered there; this only pins that the handler applies it.
|
||||
*
|
||||
* @return iterable<string, array{?string, ?array<mixed>}>
|
||||
*/
|
||||
public static function provideStructuredContentRevisions(): iterable
|
||||
{
|
||||
// A list is only emittable from 2026-07-28 (SEP-2106) on. Without a
|
||||
// negotiated revision the handler assumes the stricter handshake rule.
|
||||
yield 'no negotiated revision' => [null, null];
|
||||
yield '2025-11-25' => ['2025-11-25', null];
|
||||
yield '2026-07-28' => ['2026-07-28', [['id' => 1], ['id' => 2]]];
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideSelfBuiltResults
|
||||
*/
|
||||
public function testSelfBuiltResultIsSentUnchangedAndOnlyWarnedAbout(
|
||||
?string $negotiated,
|
||||
?array $structuredContent,
|
||||
int $expectedWarnings,
|
||||
): void {
|
||||
$request = $this->createCallToolRequest('build_result', []);
|
||||
$toolReference = $this->createToolReference('build_result', static fn () => null);
|
||||
$callToolResult = new CallToolResult([new TextContent('Built by hand')], false, $structuredContent);
|
||||
|
||||
$this->session
|
||||
->method('get')
|
||||
->with('protocol_version')
|
||||
->willReturn($negotiated);
|
||||
|
||||
$this->registry
|
||||
->method('getTool')
|
||||
->willReturn($toolReference);
|
||||
|
||||
$this->referenceHandler
|
||||
->method('handle')
|
||||
->willReturn($callToolResult);
|
||||
|
||||
$toolReference
|
||||
->expects($this->never())
|
||||
->method('formatResult');
|
||||
|
||||
$this->logger
|
||||
->expects($this->exactly($expectedWarnings))
|
||||
->method('warning');
|
||||
|
||||
$response = $this->handler->handle($request, $this->session);
|
||||
|
||||
// Warned about, never rewritten: building the result is an explicit opt-out.
|
||||
$this->assertInstanceOf(Response::class, $response);
|
||||
$this->assertSame($callToolResult, $response->result);
|
||||
$this->assertSame($structuredContent, $response->result->structuredContent);
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{?string, ?array<mixed>, int}>
|
||||
*/
|
||||
public static function provideSelfBuiltResults(): iterable
|
||||
{
|
||||
yield 'list before SEP-2106' => ['2025-11-25', [['id' => 1]], 1];
|
||||
yield 'list without a negotiated revision' => [null, [['id' => 1]], 1];
|
||||
yield 'list from SEP-2106 on' => ['2026-07-28', [['id' => 1]], 0];
|
||||
yield 'object before SEP-2106' => ['2025-11-25', ['items' => [['id' => 1]]], 0];
|
||||
yield 'none at all' => ['2025-11-25', null, 0];
|
||||
// Dropped by `CallToolResult::jsonSerialize()` anyway, so nothing to warn about.
|
||||
yield 'empty' => ['2025-11-25', [], 0];
|
||||
}
|
||||
|
||||
public function testDeclaredOutputSchemaWithoutStructuredContentIsLogged(): void
|
||||
{
|
||||
$listResult = [['id' => 1]];
|
||||
$request = $this->createCallToolRequest('list_things', []);
|
||||
$toolReference = $this->createToolReference('list_things', static fn () => $listResult, [
|
||||
'type' => 'object',
|
||||
'properties' => ['items' => ['type' => 'array']],
|
||||
]);
|
||||
|
||||
$this->registry
|
||||
->method('getTool')
|
||||
->willReturn($toolReference);
|
||||
|
||||
$this->referenceHandler
|
||||
->method('handle')
|
||||
->willReturn($listResult);
|
||||
|
||||
$toolReference
|
||||
->method('formatResult')
|
||||
->willReturn([new TextContent('[{"id":1}]')]);
|
||||
|
||||
$this->logger
|
||||
->expects($this->once())
|
||||
->method('warning')
|
||||
->with(
|
||||
$this->stringContains('outputSchema'),
|
||||
$this->callback(static fn (array $context): bool => 'list_things' === $context['name'] && 'array' === $context['result_type']),
|
||||
);
|
||||
|
||||
$response = $this->handler->handle($request, $this->session);
|
||||
|
||||
$this->assertInstanceOf(Response::class, $response);
|
||||
$this->assertNull($response->result->structuredContent);
|
||||
}
|
||||
|
||||
public function testValidationError(): void
|
||||
{
|
||||
$schema = [
|
||||
|
|
|
|||
89
tests/Unit/Server/RequestContextTest.php
Normal file
89
tests/Unit/Server/RequestContextTest.php
Normal file
|
|
@ -0,0 +1,89 @@
|
|||
<?php
|
||||
|
||||
/*
|
||||
* This file is part of the official PHP MCP SDK.
|
||||
*
|
||||
* A collaboration between Symfony and the PHP Foundation.
|
||||
*
|
||||
* For the full copyright and license information, please view the LICENSE
|
||||
* file that was distributed with this source code.
|
||||
*/
|
||||
|
||||
namespace Mcp\Tests\Unit\Server;
|
||||
|
||||
use Mcp\Schema\Enum\ProtocolVersion;
|
||||
use Mcp\Schema\Request\CallToolRequest;
|
||||
use Mcp\Server\RequestContext;
|
||||
use Mcp\Server\Session\SessionInterface;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
class RequestContextTest extends TestCase
|
||||
{
|
||||
public function testProtocolVersionComesFromTheSessionInTheHandshakeEra(): void
|
||||
{
|
||||
$context = new RequestContext(
|
||||
$this->createSession('2025-06-18'),
|
||||
$this->createRequest(),
|
||||
);
|
||||
|
||||
$this->assertSame(ProtocolVersion::V2025_06_18, $context->getProtocolVersion());
|
||||
}
|
||||
|
||||
public function testPerRequestMetaTakesPrecedenceOverTheSession(): void
|
||||
{
|
||||
// Modern revisions have no `initialize`, so the revision travels with every
|
||||
// single request instead of being negotiated once.
|
||||
$context = new RequestContext(
|
||||
$this->createSession('2025-11-25'),
|
||||
$this->createRequest(['io.modelcontextprotocol/protocolVersion' => '2026-07-28']),
|
||||
);
|
||||
|
||||
$this->assertSame(ProtocolVersion::V2026_07_28, $context->getProtocolVersion());
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider provideUnusableVersions
|
||||
*/
|
||||
public function testUnusableVersionFallsBackToTheNewestHandshakeRevision(mixed $stored): void
|
||||
{
|
||||
$context = new RequestContext(
|
||||
$this->createSession($stored),
|
||||
$this->createRequest(),
|
||||
);
|
||||
|
||||
$this->assertSame(ProtocolVersion::latestHandshake(), $context->getProtocolVersion());
|
||||
}
|
||||
|
||||
/**
|
||||
* @return iterable<string, array{mixed}>
|
||||
*/
|
||||
public static function provideUnusableVersions(): iterable
|
||||
{
|
||||
yield 'never negotiated' => [null];
|
||||
yield 'unknown revision' => ['1999-01-01'];
|
||||
yield 'not a string' => [20260728];
|
||||
}
|
||||
|
||||
private function createSession(mixed $protocolVersion): SessionInterface
|
||||
{
|
||||
$session = $this->createMock(SessionInterface::class);
|
||||
$session->method('get')->with('protocol_version')->willReturn($protocolVersion);
|
||||
|
||||
return $session;
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, mixed>|null $meta
|
||||
*/
|
||||
private function createRequest(?array $meta = null): CallToolRequest
|
||||
{
|
||||
$request = CallToolRequest::fromArray([
|
||||
'jsonrpc' => '2.0',
|
||||
'method' => CallToolRequest::getMethod(),
|
||||
'id' => 'test-request',
|
||||
'params' => ['name' => 'test_tool', 'arguments' => []],
|
||||
]);
|
||||
|
||||
return null === $meta ? $request : $request->withMeta($meta);
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue