Skip to content

fix(mcp): return resource read results for resources - #8436

Open
ousamabenyounes wants to merge 3 commits into
api-platform:mainfrom
ousamabenyounes:fix-8428-mcp-resource-read-result
Open

fix(mcp): return resource read results for resources#8436
ousamabenyounes wants to merge 3 commits into
api-platform:mainfrom
ousamabenyounes:fix-8428-mcp-resource-read-result

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Jul 31, 2026

Copy link
Copy Markdown

Fixes #8428.

What changed

  • Return ReadResourceResult for McpResource operations handled by StructuredContentProcessor.
  • Preserve the existing CallToolResult path for MCP tools.
  • Add a unit regression test that asserts resource reads expose contents via TextResourceContents.
  • Align the root and split-package mcp/sdk constraints with symfony/mcp-bundle dev-main, which now requires mcp/sdk ^0.7.

RED

docker run --rm --label com.ora.cleanup.scope=contribute -v "$PWD":/app -w /app composer:2.9.8 sh -lc 'composer update symfony/mcp-bundle --dry-run'

Your requirements could not be resolved to an installable set of packages.
Root composer.json requires mcp/sdk ^0.6 ... package is fixed to v0.7.0 (lock file version) by a partial update and that version does not match.

Original behavior RED:

docker run --rm --label com.ora.cleanup.scope=contribute -v "$PWD":/app -w /app php:8.4-cli vendor/bin/phpunit src/Mcp/Tests/State/StructuredContentProcessorTest.php --filter testMcpResourceReturnsReadResourceResult

Failed asserting that an instance of class Mcp\Schema\Result\CallToolResult is an instance of class Mcp\Schema\Result\ReadResourceResult.
Tests: 1, Assertions: 1, Failures: 1.

GREEN

./run-ci.sh

composer update symfony/mcp-bundle mcp/sdk --dry-run: OK
composer install --no-progress: OK
vendor/bin/phpunit src/Mcp/Tests/State/StructuredContentProcessorTest.php --filter testMcpResourceReturnsReadResourceResult: Tests: 1, Assertions: 5, PHPUnit Notices: 1
php-cs-fixer dry-run on touched files: Found 0 of 4 files that can be fixed.

@ousamabenyounes

Copy link
Copy Markdown
Author

Pushed 4ea680245 to fix the CI dependency-resolution failure.

The CI logs were failing before tests because symfony/mcp-bundle dev-main now requires mcp/sdk ^0.7 while this branch still required ^0.6. I aligned the root and src/Mcp package constraints with the lock file.

Validation: reproduced the Composer conflict locally (RED), then reran ./run-ci.sh (Composer dry-run, install, MCP regression test, and CS fixer dry-run) successfully.

Signed-off-by: Ousama Ben Younes <benyounes.ousama@gmail.com>
@ousamabenyounes

Copy link
Copy Markdown
Author

Pushed an additional CI fix in 6c18dc3.\n\nWhat changed:\n- Laravel provider now passes the new Symfony MCP bundle MiddlewareFactory argument required by the current McpController constructor.\n- Removed obsolete SerializerPropertyMetadataFactory is_array() guards that PHPStan reported as always true after the typed helper return.\n\nLocal validation:\n- Laravel package build + tests: 220 passed / 673 assertions.\n- PHPStan targeted check on src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php: no errors.\n- PHP CS Fixer dry-run on both touched files: 0 fixable files.

@ousamabenyounes

ousamabenyounes commented Jul 31, 2026

Copy link
Copy Markdown
Author

Follow-up on the remaining red checks after 6c18dc3:

  • The original MCP dependency/Laravel/PHPStan failures are green now.
  • PHPUnit (PHP 8.5) (Elasticsearch 8.4.0) failed before tests while resolving docker:latest from Docker Hub (i/o timeout).
  • The remaining lowest failures for doctrine-orm, doctrine-odm, and json-api match the latest failing main CI run 29720752858, so they look like upstream baseline failures rather than this PR.

I tried to rerun the failed workflow, but GitHub returned run ... cannot be rerun; its workflow file may be broken while the run was still settling.

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.

[MCP] resources/read returns a CallToolResult instead of ReadResourceResult for McpResource operations

1 participant