From 9d1136c35f5d9f1886f0e5b82bbda645723ed617 Mon Sep 17 00:00:00 2001
From: Christopher Hertel <mail@christopher-hertel.de>
Date: Sat, 5 Sep 2026 02:49:55 +0200
Subject: [PATCH] [Capability] Make CompletionProvider's providerClass usable

The constructor's exactly-one-source check never counted providerClass, so
the form always threw, and discovery stored the (always null) provider for it.
---
 .../Attribute/CompletionProvider.php          | 13 +++--
 src/Capability/Discovery/Discoverer.php       |  2 +-
 tests/Integration/CompletionTest.php          | 57 +++++++++++++++++++
 .../Fixture/Completion/BookingElements.php    | 38 +++++++++++++
 .../Completion/SeatCompletionProvider.php     | 42 ++++++++++++++
 tests/Integration/Fixture/completion.php      | 31 ++++++++++
 .../Attribute/CompletionProviderTest.php      | 18 +++++-
 7 files changed, 192 insertions(+), 9 deletions(-)
 create mode 100644 tests/Integration/CompletionTest.php
 create mode 100644 tests/Integration/Fixture/Completion/BookingElements.php
 create mode 100644 tests/Integration/Fixture/Completion/SeatCompletionProvider.php
 create mode 100644 tests/Integration/Fixture/completion.php

diff --git a/src/Capability/Attribute/CompletionProvider.php b/src/Capability/Attribute/CompletionProvider.php
index 9e8dc802..73dbba5f 100644
--- a/src/Capability/Attribute/CompletionProvider.php
+++ b/src/Capability/Attribute/CompletionProvider.php
@@ -21,9 +21,12 @@
 class CompletionProvider
 {
     /**
-     * @param class-string<ProviderInterface>|ProviderInterface|null $provider if a class-string, it will be resolved
-     *                                                                         from the container at the point of use
-     * @param ?array<int, int|float|string>                          $values   a list of values to use for completion
+     * @param class-string<ProviderInterface>|null                   $providerClass a provider class, resolved from the
+     *                                                                              container at the point of use
+     * @param class-string<ProviderInterface>|ProviderInterface|null $provider      if a class-string, it will be resolved
+     *                                                                              from the container at the point of use
+     * @param ?array<int, int|float|string>                          $values        a list of values to use for completion
+     * @param class-string|null                                      $enum          an enum whose cases are the completions
      */
     public function __construct(
         public ?string $providerClass = null,
@@ -31,8 +34,8 @@ public function __construct(
         public ?array $values = null,
         public ?string $enum = null,
     ) {
-        if (1 !== \count(array_filter([$provider, $values, $enum]))) {
-            throw new InvalidArgumentException('Only one of provider, values, or enum can be set.');
+        if (1 !== \count(array_filter([$providerClass, $provider, $values, $enum]))) {
+            throw new InvalidArgumentException('Only one of providerClass, provider, values, or enum can be set.');
         }
     }
 }
diff --git a/src/Capability/Discovery/Discoverer.php b/src/Capability/Discovery/Discoverer.php
index 5b7e4765..09caa07b 100644
--- a/src/Capability/Discovery/Discoverer.php
+++ b/src/Capability/Discovery/Discoverer.php
@@ -326,7 +326,7 @@ private function getCompletionProviders(\ReflectionMethod $reflectionMethod): ar
                 if ($attributeInstance->provider) {
                     $completionProviders[$param->getName()] = $attributeInstance->provider;
                 } elseif ($attributeInstance->providerClass) {
-                    $completionProviders[$param->getName()] = $attributeInstance->provider;
+                    $completionProviders[$param->getName()] = $attributeInstance->providerClass;
                 } elseif ($attributeInstance->values) {
                     $completionProviders[$param->getName()] = new ListCompletionProvider($attributeInstance->values);
                 } elseif ($attributeInstance->enum) {
diff --git a/tests/Integration/CompletionTest.php b/tests/Integration/CompletionTest.php
new file mode 100644
index 00000000..f0633de3
--- /dev/null
+++ b/tests/Integration/CompletionTest.php
@@ -0,0 +1,57 @@
+<?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\Integration;
+
+use Mcp\Schema\PromptReference;
+use PHPUnit\Framework\Attributes\TestDox;
+
+/**
+ * Argument completion driven by a `#[CompletionProvider(providerClass: …)]`.
+ *
+ * The fixture's provider only exists in the container, so completions coming
+ * back at all is what proves the attribute reached the registry and the
+ * container was asked to build it.
+ *
+ * @see Fixture/completion.php for the server under test
+ */
+final class CompletionTest extends IntegrationTestCase
+{
+    #[TestDox('a providerClass argument completes from the container-built provider')]
+    public function testProviderClassCompletesFromTheContainer(): void
+    {
+        $client = $this->connect('completion');
+
+        $result = $client->complete(new PromptReference('book_seat'), ['name' => 'seat', 'value' => '12']);
+
+        $this->assertSame(['12A', '12B'], $result->values);
+    }
+
+    #[TestDox('an empty value offers every completion the provider knows')]
+    public function testEmptyValueOffersEveryCompletion(): void
+    {
+        $client = $this->connect('completion');
+
+        $result = $client->complete(new PromptReference('book_seat'), ['name' => 'seat', 'value' => '']);
+
+        $this->assertSame(['12A', '12B', '14C'], $result->values);
+    }
+
+    #[TestDox('an argument without a provider completes to nothing')]
+    public function testUnknownArgumentCompletesToNothing(): void
+    {
+        $client = $this->connect('completion');
+
+        $result = $client->complete(new PromptReference('book_seat'), ['name' => 'unknown', 'value' => '1']);
+
+        $this->assertSame([], $result->values);
+    }
+}
diff --git a/tests/Integration/Fixture/Completion/BookingElements.php b/tests/Integration/Fixture/Completion/BookingElements.php
new file mode 100644
index 00000000..a859cc89
--- /dev/null
+++ b/tests/Integration/Fixture/Completion/BookingElements.php
@@ -0,0 +1,38 @@
+<?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\Integration\Fixture\Completion;
+
+use Mcp\Capability\Attribute\CompletionProvider;
+use Mcp\Capability\Attribute\McpPrompt;
+
+/**
+ * @author Christopher Hertel <mail@christopher-hertel.de>
+ */
+final class BookingElements
+{
+    /**
+     * Confirms a seat booking.
+     *
+     * @param string $seat the seat to book
+     *
+     * @return array the prompt messages
+     */
+    #[McpPrompt(name: 'book_seat')]
+    public function bookSeat(
+        #[CompletionProvider(providerClass: SeatCompletionProvider::class)]
+        string $seat,
+    ): array {
+        return [
+            ['role' => 'user', 'content' => \sprintf('Book seat %s for me.', $seat)],
+        ];
+    }
+}
diff --git a/tests/Integration/Fixture/Completion/SeatCompletionProvider.php b/tests/Integration/Fixture/Completion/SeatCompletionProvider.php
new file mode 100644
index 00000000..a749fcf7
--- /dev/null
+++ b/tests/Integration/Fixture/Completion/SeatCompletionProvider.php
@@ -0,0 +1,42 @@
+<?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\Integration\Fixture\Completion;
+
+use Mcp\Capability\Completion\ProviderInterface;
+
+/**
+ * A provider that cannot be built without its seat map.
+ *
+ * The constructor takes a scalar the auto-wiring container cannot supply, so a
+ * completion that comes back with seats in it proves the provider was taken
+ * from the container rather than instantiated on the spot.
+ *
+ * @author Christopher Hertel <mail@christopher-hertel.de>
+ */
+final class SeatCompletionProvider implements ProviderInterface
+{
+    /**
+     * @param list<string> $seats
+     */
+    public function __construct(
+        private readonly array $seats,
+    ) {
+    }
+
+    public function getCompletions(string $currentValue): array
+    {
+        return array_values(array_filter(
+            $this->seats,
+            static fn (string $seat): bool => str_starts_with($seat, $currentValue),
+        ));
+    }
+}
diff --git a/tests/Integration/Fixture/completion.php b/tests/Integration/Fixture/completion.php
new file mode 100644
index 00000000..f33e9511
--- /dev/null
+++ b/tests/Integration/Fixture/completion.php
@@ -0,0 +1,31 @@
+<?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.
+ */
+
+/*
+ * Server for {@see \Mcp\Tests\Integration\CompletionTest}.
+ */
+
+use Mcp\Capability\Registry\Container;
+use Mcp\Server;
+use Mcp\Server\Transport\StdioTransport;
+use Mcp\Tests\Integration\Fixture\Completion\SeatCompletionProvider;
+
+require_once dirname(__DIR__, 3).'/vendor/autoload.php';
+
+$container = new Container();
+$container->set(SeatCompletionProvider::class, new SeatCompletionProvider(['12A', '12B', '14C']));
+
+Server::builder()
+    ->setServerInfo('integration-server', '1.0.0')
+    ->setContainer($container)
+    ->setDiscovery(__DIR__, ['Completion'])
+    ->build()
+    ->run(new StdioTransport());
diff --git a/tests/Unit/Capability/Attribute/CompletionProviderTest.php b/tests/Unit/Capability/Attribute/CompletionProviderTest.php
index 19a78750..9914a0a9 100644
--- a/tests/Unit/Capability/Attribute/CompletionProviderTest.php
+++ b/tests/Unit/Capability/Attribute/CompletionProviderTest.php
@@ -22,6 +22,17 @@ public function testCanBeConstructedWithProviderClass(): void
         $attribute = new CompletionProvider(provider: CompletionProviderFixture::class);
 
         $this->assertSame(CompletionProviderFixture::class, $attribute->provider);
+        $this->assertNull($attribute->providerClass);
+        $this->assertNull($attribute->values);
+        $this->assertNull($attribute->enum);
+    }
+
+    public function testCanBeConstructedWithProviderClassArgument(): void
+    {
+        $attribute = new CompletionProvider(providerClass: CompletionProviderFixture::class);
+
+        $this->assertSame(CompletionProviderFixture::class, $attribute->providerClass);
+        $this->assertNull($attribute->provider);
         $this->assertNull($attribute->values);
         $this->assertNull($attribute->enum);
     }
@@ -58,14 +69,14 @@ public function testCanBeConstructedWithEnumClass(): void
     public function testThrowsExceptionWhenNoParametersProvided(): void
     {
         $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Only one of provider, values, or enum can be set');
+        $this->expectExceptionMessage('Only one of providerClass, provider, values, or enum can be set');
         new CompletionProvider();
     }
 
     public function testThrowsExceptionWhenMultipleParametersProvided(): void
     {
         $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Only one of provider, values, or enum can be set');
+        $this->expectExceptionMessage('Only one of providerClass, provider, values, or enum can be set');
         new CompletionProvider(
             provider: CompletionProviderFixture::class,
             values: ['test']
@@ -75,8 +86,9 @@ public function testThrowsExceptionWhenMultipleParametersProvided(): void
     public function testThrowsExceptionWhenAllParametersProvided(): void
     {
         $this->expectException(\InvalidArgumentException::class);
-        $this->expectExceptionMessage('Only one of provider, values, or enum can be set');
+        $this->expectExceptionMessage('Only one of providerClass, provider, values, or enum can be set');
         new CompletionProvider(
+            providerClass: CompletionProviderFixture::class,
             provider: CompletionProviderFixture::class,
             values: ['test'],
             enum: StatusEnum::class