From 299ed762ff57f5bd10eb28bb444a052e56bf826b Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 12:29:50 +0100 Subject: [PATCH] fix(mcp): resolve the agent resource provider structurally, not nominally MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #19 moved the plans:// and sessions:// rendering behind AgentResourceProvider and I described the dead endpoints as an interim, waiting on dappcore/agent to depend on this package and implement the interface. That day does not come: agent maintains its own copy of Core\Mcp rather than depending on this package, so it can never name the interface in an implements clause. An interim with no end is a decision that has not been made, so it is made here. The controller now accepts any object bound under the interface name that provides read(). A provider able to name the interface still implements it and is accepted nominally; one that cannot binds and satisfies the method. Both work, neither package imports the other, and two endpoints that would otherwise have stayed dead permanently come back on as soon as agent binds. Half a contract is rejected rather than accepted: an object with neither read() nor the interface returns null, so the controller answers a clean not-found instead of fatalling mid-request. The contract also loses entries(). Nothing in this package ever called it — GET servers/{id}/resources lists a server's own configured resources, which is a different concept — so it was a method an implementer had to satisfy for no consumer. Providers are still free to offer listing for their own transports; agent's registry does, for its stdio server. This package asks only for what it uses. Four tests cover the resolution: a duck-typed binding with no implements clause, a nominal implementation, an object missing read(), and nothing bound at all. Suite: 21 failed, 303 passed, from 21 failed, 299 passed — the four new tests, no change to the existing failures. Co-Authored-By: Virgil --- php/src/Mcp/Controllers/McpApiController.php | 29 ++++++- .../Contracts/AgentResourceProvider.php | 23 ++--- .../AgentResourceProviderBindingTest.php | 84 +++++++++++++++++++ 3 files changed, 122 insertions(+), 14 deletions(-) create mode 100644 php/tests/Feature/AgentResourceProviderBindingTest.php diff --git a/php/src/Mcp/Controllers/McpApiController.php b/php/src/Mcp/Controllers/McpApiController.php index 38ba2e8..f05001a 100644 --- a/php/src/Mcp/Controllers/McpApiController.php +++ b/php/src/Mcp/Controllers/McpApiController.php @@ -379,16 +379,27 @@ protected function resourceServerContent(string $scheme, string $uri): ?array } /** - * Resolve the bound agent resource provider, if the agent module supplies one. + * Resolve the bound agent resource provider, if a module supplies one. * - * Absent when this package is installed without dappcore/agent, which is a + * Absent when this package is installed without an agent module, which is a * legitimate deployment — the protocol surface stands on its own and the * agent resources are an optional extension to it. * + * Structural, not nominal. AgentResourceProvider states the contract and a + * provider that can name the interface should implement it, but + * dappcore/agent maintains its own copy of Core\Mcp rather than depending + * on this package, so it cannot reference the interface to implement it. + * Requiring `instanceof` would therefore have left these endpoints dead + * permanently rather than until the binding landed — an interim that never + * ends is a decision, so it is made here: bind under the interface name and + * satisfy its two methods, and the endpoints work either way. + * + * @return object|null A read() provider, or null when unbound + * * @example * $provider = $this->agentResourceProvider(); */ - protected function agentResourceProvider(): ?AgentResourceProvider + protected function agentResourceProvider(): ?object { if (! app()->bound(AgentResourceProvider::class)) { return null; @@ -396,7 +407,17 @@ protected function agentResourceProvider(): ?AgentResourceProvider $provider = app(AgentResourceProvider::class); - return $provider instanceof AgentResourceProvider ? $provider : null; + if (! is_object($provider)) { + return null; + } + + if ($provider instanceof AgentResourceProvider) { + return $provider; + } + + // Duck-typed fallback: read() is the whole contract this package + // consumes, so it is the whole check. + return method_exists($provider, 'read') ? $provider : null; } /** diff --git a/php/src/Mcp/Resources/Contracts/AgentResourceProvider.php b/php/src/Mcp/Resources/Contracts/AgentResourceProvider.php index ac7bb18..34260eb 100644 --- a/php/src/Mcp/Resources/Contracts/AgentResourceProvider.php +++ b/php/src/Mcp/Resources/Contracts/AgentResourceProvider.php @@ -21,19 +21,22 @@ * Nothing is bound when the agent module is absent, and the controller answers * "unavailable" rather than pretending — the resources are optional to this * package, not to the protocol. + * + * read() only, deliberately. An earlier draft also required entries() for + * listing, which nothing here ever called: this package's + * GET servers/{id}/resources lists a server's own configured resources, a + * different concept. A contract method with no consumer is over-specification + * that an implementer has to satisfy for nothing, so it is not asked for. + * Providers are free to offer listing for their own transports — + * dappcore/agent's registry does, for its stdio server. + * + * Satisfied structurally as well as nominally: a provider that cannot name this + * interface (because it maintains its own copy of Core\Mcp rather than + * depending on this package) binds under the interface name and implements + * read(), and McpApiController accepts it. */ interface AgentResourceProvider { - /** - * Entries to advertise when listing resources. - * - * @return array - * - * @example - * $provider->entries(); // [['uri' => 'plans://all', ...]] - */ - public function entries(): array; - /** * Read one resource, or null when nothing serves that URI. * diff --git a/php/tests/Feature/AgentResourceProviderBindingTest.php b/php/tests/Feature/AgentResourceProviderBindingTest.php new file mode 100644 index 0000000..4ee8e9b --- /dev/null +++ b/php/tests/Feature/AgentResourceProviderBindingTest.php @@ -0,0 +1,84 @@ +agentResourceProvider(); + } + }; + + return $controller->provider(); + } + + public function test_provider_good_accepts_a_duck_typed_binding(): void + { + // No implements clause — exactly what a package that cannot name the + // interface is able to bind. + $this->app->bind(AgentResourceProvider::class, fn (): object => new class + { + public function read(string $uri): ?array + { + return ['uri' => $uri, 'mimeType' => 'text/markdown', 'text' => '# Plan']; + } + }); + + $provider = $this->resolve(); + + $this->assertNotNull($provider); + $this->assertSame('# Plan', $provider->read('plans://all')['text']); + } + + public function test_provider_good_accepts_a_nominal_implementation(): void + { + $this->app->bind(AgentResourceProvider::class, fn (): AgentResourceProvider => new class implements AgentResourceProvider + { + public function read(string $uri): ?array + { + return null; + } + }); + + $this->assertInstanceOf(AgentResourceProvider::class, $this->resolve()); + } + + public function test_provider_bad_rejects_an_object_without_read(): void + { + // Half a contract would fatal mid-request; the controller must see + // nothing rather than something unusable. + $this->app->bind(AgentResourceProvider::class, fn (): object => new class + { + public function somethingElse(): void {} + }); + + $this->assertNull($this->resolve()); + } + + public function test_provider_ugly_is_null_when_nothing_is_bound(): void + { + // The package installed without any agent module — a legitimate deploy. + $this->assertNull($this->resolve()); + } +}