diff --git a/CHANGELOG.md b/CHANGELOG.md index 468c1e9d..69f504f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ All notable changes to `mcp/sdk` will be documented in this file. +0.10.0 +------ + +* [BC Break] `Builder::build()` throws a `RuntimeException` when `setDiscovery()` is used without `symfony/finder` and without a custom discoverer, instead of logging a warning and discovering nothing + 0.9.0 ----- diff --git a/docs/index.md b/docs/index.md index da009ef9..7b947287 100644 --- a/docs/index.md +++ b/docs/index.md @@ -74,7 +74,7 @@ That's a complete MCP server. It exposes one **tool**, `add`, and one **resource `config://calculator/settings`. Attribute discovery needs `symfony/finder` (`composer require symfony/finder`). Without -it the server still starts, but discovers nothing and only logs a warning. +it, `build()` throws a `RuntimeException` instead of starting a server with no tools. Look at what you did *not* write: no JSON Schema — `int $a, int $b` *is* the schema — no request parsing, no serialization, no protocol handling. You wrote a PHP class with diff --git a/docs/run/server-builder.md b/docs/run/server-builder.md index b1fb6a16..ab079ef9 100644 --- a/docs/run/server-builder.md +++ b/docs/run/server-builder.md @@ -160,7 +160,8 @@ $server = Server::builder() ); ``` -Discovery needs the `symfony/finder` package. Without it, the server skips discovery and logs a warning. +Discovery needs the `symfony/finder` package. Without it, `build()` throws a `RuntimeException`, unless you set your own +discoverer with `setDiscoverer()`. See [First server](../get-started/first-server.md) for the install command. **Parameters:** diff --git a/src/Server/Builder.php b/src/Server/Builder.php index 13f1eb59..1b8cbb07 100644 --- a/src/Server/Builder.php +++ b/src/Server/Builder.php @@ -71,7 +71,6 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; use Psr\SimpleCache\CacheInterface; -use Symfony\Component\Finder\Finder; /** * @phpstan-import-type Handler from ElementReference @@ -1070,12 +1069,8 @@ private function resolve(): array ]; if (null !== $this->discoveryBasePath) { - if (null !== $this->discoverer || class_exists(Finder::class)) { - $discoverer = $this->discoverer ?? $this->createDiscoverer($logger); - $loaders[] = new DiscoveryLoader($this->discoveryBasePath, $this->discoveryScanDirs, $this->discoveryExcludeDirs, $discoverer, $this->discoveryNamePatterns, $logger); - } else { - $logger->warning('File-based discovery requires symfony/finder. Skipping automatic discovery. Run: composer require symfony/finder'); - } + $discoverer = $this->discoverer ?? $this->createDiscoverer($logger); + $loaders[] = new DiscoveryLoader($this->discoveryBasePath, $this->discoveryScanDirs, $this->discoveryExcludeDirs, $discoverer, $this->discoveryNamePatterns, $logger); } $chainLoader = new ChainLoader($loaders); diff --git a/tests/Unit/Server/BuilderTest.php b/tests/Unit/Server/BuilderTest.php index 53c793f0..40299117 100644 --- a/tests/Unit/Server/BuilderTest.php +++ b/tests/Unit/Server/BuilderTest.php @@ -11,6 +11,9 @@ namespace Mcp\Tests\Unit\Server; +use Composer\Autoload\ClassLoader; +use Mcp\Capability\Discovery\Discoverer; +use Mcp\Capability\Discovery\DiscovererInterface; use Mcp\Capability\Registry; use Mcp\Capability\Registry\ElementReference; use Mcp\Capability\Registry\Loader\LoaderInterface; @@ -18,6 +21,7 @@ use Mcp\Capability\RegistryInterface; use Mcp\Exception\InvalidArgumentException; use Mcp\Exception\LogicException; +use Mcp\Exception\RuntimeException; use Mcp\Schema\Content\TextContent; use Mcp\Schema\Enum\ProtocolVersion; use Mcp\Schema\Extension\Apps\McpApps; @@ -41,12 +45,20 @@ use Mcp\Tests\Unit\Server\Extension\ThingListRequest; use Mcp\Tests\Unit\Server\Session\Fixture\RecordingSessionLock; use Mcp\Tests\Unit\Server\Session\Fixture\RecordingSessionStore; +use PHPUnit\Framework\Attributes\RunInSeparateProcess; use PHPUnit\Framework\Attributes\TestDox; use PHPUnit\Framework\TestCase; use Psr\EventDispatcher\EventDispatcherInterface; +use Psr\Log\LogLevel; +use Symfony\Component\Finder\Finder; final class BuilderTest extends TestCase { + private const DISCOVERY_DEPENDENCY_ERROR = 'File-based discovery requires symfony/finder. Run: composer require symfony/finder'; + private const FINDER_LOADED_ERROR = 'Symfony Finder was loaded after Composer autoloaders were disabled.'; + private const TEST_SERVER_NAME = 'test'; + private const TEST_SERVER_VERSION = '1.0.0'; + #[TestDox('setReferenceHandler() returns the builder for fluent chaining')] public function testSetReferenceHandlerReturnsSelf(): void { @@ -81,6 +93,49 @@ public function testBuildWithoutCustomReferenceHandler(): void $this->assertInstanceOf(Server::class, $server); } + #[RunInSeparateProcess] + #[TestDox('build() fails when file-based discovery is configured without Symfony Finder')] + public function testBuildFailsWhenDiscoveryDependencyIsMissing(): void + { + // Load every class the failing path needs before the Composer autoloaders are unregistered. + Server::builder()->setServerInfo(self::TEST_SERVER_NAME, self::TEST_SERVER_VERSION)->build(); + Server::builder() + ->setServerInfo(self::TEST_SERVER_NAME, self::TEST_SERVER_VERSION) + ->setDiscovery(__DIR__) + ->setDiscoverer($this->createStub(DiscovererInterface::class)) + ->build(); + class_exists(Discoverer::class); + class_exists(RuntimeException::class); + class_exists(LogLevel::class); + + $autoloaders = array_values(array_filter( + spl_autoload_functions(), + static fn (mixed $autoloader): bool => \is_array($autoloader) && $autoloader[0] instanceof ClassLoader, + )); + $this->assertNotEmpty($autoloaders); + $this->expectException(RuntimeException::class); + $this->expectExceptionMessage(self::DISCOVERY_DEPENDENCY_ERROR); + + foreach ($autoloaders as $autoloader) { + $autoloader[0]->unregister(); + } + + try { + if (class_exists(Finder::class)) { + throw new LogicException(self::FINDER_LOADED_ERROR); + } + + Server::builder() + ->setServerInfo(self::TEST_SERVER_NAME, self::TEST_SERVER_VERSION) + ->setDiscovery(__DIR__) + ->build(); + } finally { + foreach ($autoloaders as $autoloader) { + $autoloader[0]->register(true); + } + } + } + #[TestDox('Custom ReferenceHandler is used when calling a tool')] public function testCustomReferenceHandlerIsUsedForToolCalls(): void {