refactor: eliminate reflection in discovery caching system

- Create DiscoveryState class to represent discovered capabilities
- Add exportDiscoveryState() and importDiscoveryState() methods to Registry
- Refactor Discoverer to return DiscoveryState instead of modifying registry directly
- Rewrite CachedDiscoverer to use state-based caching instead of reflection
- Update ServerBuilder to work with new discovery flow
- Update all tests to use the new approach
- Add comprehensive documentation explaining the refactoring

This addresses reviewer feedback about avoiding reflection-based implementation
while maintaining the same performance benefits and backward compatibility.

Resolves the OOP design concerns by:
- Eliminating reflection usage completely
- Creating proper state objects for discovered capabilities
- Adding clean interfaces for state import/export
- Separating discovery logic from registry state management
- Maintaining encapsulation and following proper OOP principles

Author: xentixar

docs/discovery-caching.md

@@ -116,8 +116,9 @@ See the `examples/10-cached-discovery-stdio/` directory for a complete working e
 
 ## Implementation Details
 
-The caching system uses the decorator pattern:
+The caching system uses the decorator pattern with a clean state-based approach:
 - `CachedDiscoverer` wraps the existing `Discoverer` class
-- Uses reflection to extract and restore registry state
+- Uses `DiscoveryState` objects to represent discovered capabilities
 - Only caches discovered elements (not manually registered ones)
-- Maintains backward compatibility - works with or without cache
+- Maintains backward compatibility - works with or without cache
+- Clean separation of concerns between discovery and registry state management

src/Capability/Discovery/CachedDiscoverer.php

@@ -11,7 +11,6 @@
 
 namespace Mcp\Capability\Discovery;
 
-use Mcp\Capability\Registry;
 use Psr\Log\LoggerInterface;
 use Psr\SimpleCache\CacheInterface;
 
@@ -43,7 +42,7 @@ public function __construct(
      * @param array<string> $directories list of directories (relative to base path) to scan
      * @param array<string> $excludeDirs list of directories (relative to base path) to exclude from the scan
      */
-    public function discover(string $basePath, array $directories, array $excludeDirs = []): void
+    public function discover(string $basePath, array $directories, array $excludeDirs = []): DiscoveryState
     {
         $cacheKey = $this->generateCacheKey($basePath, $directories, $excludeDirs);
 
@@ -56,9 +55,8 @@ public function discover(string $basePath, array $directories, array $excludeDir
                 'directories' => $directories,
             ]);
 
-            // Restore the registry state from cache
-            $this->restoreRegistryFromCache($cachedResult);
-            return;
+            // Restore the discovery state from cache
+            return $this->restoreDiscoveryStateFromCache($cachedResult);
         }
 
         $this->logger->debug('Cache miss, performing fresh discovery', [
@@ -68,10 +66,12 @@ public function discover(string $basePath, array $directories, array $excludeDir
         ]);
 
         // Perform fresh discovery
-        $this->discoverer->discover($basePath, $directories, $excludeDirs);
+        $discoveryState = $this->discoverer->discover($basePath, $directories, $excludeDirs);
 
         // Cache the results
-        $this->cacheDiscoveryResults($cacheKey);
+        $this->cacheDiscoveryResults($cacheKey, $discoveryState);
+        
+        return $discoveryState;
     }
 
     /**
@@ -92,23 +92,21 @@ private function generateCacheKey(string $basePath, array $directories, array $e
     }
 
     /**
-     * Cache the current registry state.
+     * Cache the discovery state.
      */
-    private function cacheDiscoveryResults(string $cacheKey): void
+    private function cacheDiscoveryResults(string $cacheKey, DiscoveryState $state): void
     {
         try {
-            // Get the registry from the discoverer
-            $registry = $this->getRegistryFromDiscoverer();
-            
-            // Extract registry state for caching
-            $registryState = $this->extractRegistryState($registry);
+            // Convert state to array for caching
+            $stateData = $state->toArray();
 
             // Store in cache
-            $this->cache->set($cacheKey, $registryState, $this->cacheTtl);
+            $this->cache->set($cacheKey, $stateData, $this->cacheTtl);
 
             $this->logger->debug('Cached discovery results', [
                 'cache_key' => $cacheKey,
                 'ttl' => $this->cacheTtl,
+                'element_count' => $state->getElementCount(),
             ]);
         } catch (\Throwable $e) {
             $this->logger->warning('Failed to cache discovery results', [
@@ -119,126 +117,22 @@ private function cacheDiscoveryResults(string $cacheKey): void
     }
 
     /**
-     * Restore registry state from cached data.
+     * Restore discovery state from cached data.
      *
      * @param array<string, mixed> $cachedResult
      */
-    private function restoreRegistryFromCache(array $cachedResult): void
+    private function restoreDiscoveryStateFromCache(array $cachedResult): DiscoveryState
     {
         try {
-            $registry = $this->getRegistryFromDiscoverer();
-            $this->restoreRegistryState($registry, $cachedResult);
+            return DiscoveryState::fromArray($cachedResult);
         } catch (\Throwable $e) {
-            $this->logger->error('Failed to restore registry from cache', [
+            $this->logger->error('Failed to restore discovery state from cache', [
                 'exception' => $e->getMessage(),
             ]);
             throw $e;
         }
     }
 
-    /**
-     * Get the registry instance from the discoverer.
-     * This uses reflection to access the private registry property.
-     */
-    private function getRegistryFromDiscoverer(): Registry
-    {
-        $reflection = new \ReflectionClass($this->discoverer);
-        
-        if (!$reflection->hasProperty('registry')) {
-            throw new \RuntimeException('Discoverer does not have a registry property');
-        }
-        
-        $registryProperty = $reflection->getProperty('registry');
-        $registryProperty->setAccessible(true);
-        
-        return $registryProperty->getValue($this->discoverer);
-    }
-
-    /**
-     * Extract the current state of the registry for caching.
-     *
-     * @return array<string, mixed>
-     */
-    private function extractRegistryState(Registry $registry): array
-    {
-        // Use reflection to access registry's internal state
-        $reflection = new \ReflectionClass($registry);
-        
-        $state = [];
-        
-        // Extract tools (only discovered ones, not manual)
-        if ($reflection->hasProperty('tools')) {
-            $toolsProperty = $reflection->getProperty('tools');
-            $toolsProperty->setAccessible(true);
-            $tools = $toolsProperty->getValue($registry);
-            $state['tools'] = array_filter($tools, fn($tool) => !$tool->isManual);
-        }
-        
-        // Extract resources (only discovered ones, not manual)
-        if ($reflection->hasProperty('resources')) {
-            $resourcesProperty = $reflection->getProperty('resources');
-            $resourcesProperty->setAccessible(true);
-            $resources = $resourcesProperty->getValue($registry);
-            $state['resources'] = array_filter($resources, fn($resource) => !$resource->isManual);
-        }
-        
-        // Extract prompts (only discovered ones, not manual)
-        if ($reflection->hasProperty('prompts')) {
-            $promptsProperty = $reflection->getProperty('prompts');
-            $promptsProperty->setAccessible(true);
-            $prompts = $promptsProperty->getValue($registry);
-            $state['prompts'] = array_filter($prompts, fn($prompt) => !$prompt->isManual);
-        }
-        
-        // Extract resource templates (only discovered ones, not manual)
-        if ($reflection->hasProperty('resourceTemplates')) {
-            $resourceTemplatesProperty = $reflection->getProperty('resourceTemplates');
-            $resourceTemplatesProperty->setAccessible(true);
-            $resourceTemplates = $resourceTemplatesProperty->getValue($registry);
-            $state['resourceTemplates'] = array_filter($resourceTemplates, fn($template) => !$template->isManual);
-        }
-        
-        return $state;
-    }
-
-    /**
-     * Restore registry state from cached data.
-     *
-     * @param array<string, mixed> $cachedState
-     */
-    private function restoreRegistryState(Registry $registry, array $cachedState): void
-    {
-        $reflection = new \ReflectionClass($registry);
-        
-        // Restore tools
-        if (isset($cachedState['tools']) && $reflection->hasProperty('tools')) {
-            $toolsProperty = $reflection->getProperty('tools');
-            $toolsProperty->setAccessible(true);
-            $toolsProperty->setValue($registry, $cachedState['tools']);
-        }
-        
-        // Restore resources
-        if (isset($cachedState['resources']) && $reflection->hasProperty('resources')) {
-            $resourcesProperty = $reflection->getProperty('resources');
-            $resourcesProperty->setAccessible(true);
-            $resourcesProperty->setValue($registry, $cachedState['resources']);
-        }
-        
-        // Restore prompts
-        if (isset($cachedState['prompts']) && $reflection->hasProperty('prompts')) {
-            $promptsProperty = $reflection->getProperty('prompts');
-            $promptsProperty->setAccessible(true);
-            $promptsProperty->setValue($registry, $cachedState['prompts']);
-        }
-        
-        // Restore resource templates
-        if (isset($cachedState['resourceTemplates']) && $reflection->hasProperty('resourceTemplates')) {
-            $resourceTemplatesProperty = $reflection->getProperty('resourceTemplates');
-            $resourceTemplatesProperty->setAccessible(true);
-            $resourceTemplatesProperty->setValue($registry, $cachedState['resourceTemplates']);
-        }
-    }
-
     /**
      * Clear the discovery cache.
      * Useful for development or when files change.

src/Capability/Discovery/Discoverer.php

@@ -20,6 +20,10 @@
 use Mcp\Capability\Prompt\Completion\ListCompletionProvider;
 use Mcp\Capability\Prompt\Completion\ProviderInterface;
 use Mcp\Capability\Registry;
+use Mcp\Capability\Registry\PromptReference;
+use Mcp\Capability\Registry\ResourceReference;
+use Mcp\Capability\Registry\ResourceTemplateReference;
+use Mcp\Capability\Registry\ToolReference;
 use Mcp\Exception\ExceptionInterface;
 use Mcp\Schema\Prompt;
 use Mcp\Schema\PromptArgument;
@@ -54,13 +58,13 @@ public function __construct(
     }
 
     /**
-     * Discover MCP elements in the specified directories.
+     * Discover MCP elements in the specified directories and return the discovery state.
      *
      * @param string        $basePath    the base path for resolving directories
      * @param array<string> $directories list of directories (relative to base path) to scan
      * @param array<string> $excludeDirs list of directories (relative to base path) to exclude from the scan
      */
-    public function discover(string $basePath, array $directories, array $excludeDirs = []): void
+    public function discover(string $basePath, array $directories, array $excludeDirs = []): DiscoveryState
     {
         $startTime = microtime(true);
         $discoveredCount = [
@@ -70,6 +74,12 @@ public function discover(string $basePath, array $directories, array $excludeDir
             'resourceTemplates' => 0,
         ];
 
+        // Collections to store discovered elements
+        $tools = [];
+        $resources = [];
+        $prompts = [];
+        $resourceTemplates = [];
+
         try {
             $finder = new Finder();
             $absolutePaths = [];
@@ -86,7 +96,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
                     'base_path' => $basePath,
                 ]);
 
-                return;
+                return new DiscoveryState();
             }
 
             $finder->files()
@@ -95,7 +105,7 @@ public function discover(string $basePath, array $directories, array $excludeDir
                 ->name('*.php');
 
             foreach ($finder as $file) {
-                $this->processFile($file, $discoveredCount);
+                $this->processFile($file, $discoveredCount, $tools, $resources, $prompts, $resourceTemplates);
             }
         } catch (\Throwable $e) {
             $this->logger->error('Error during file finding process for MCP discovery', [
@@ -112,14 +122,29 @@ public function discover(string $basePath, array $directories, array $excludeDir
             'prompts' => $discoveredCount['prompts'],
             'resourceTemplates' => $discoveredCount['resourceTemplates'],
         ]);
+
+        return new DiscoveryState($tools, $resources, $prompts, $resourceTemplates);
+    }
+
+    /**
+     * Apply a discovery state to the registry.
+     * This method imports the discovered elements into the registry.
+     */
+    public function applyDiscoveryState(DiscoveryState $state): void
+    {
+        $this->registry->importDiscoveryState($state);
     }
 
     /**
      * Process a single PHP file for MCP elements on classes or methods.
      *
      * @param DiscoveredCount $discoveredCount
+     * @param array<string, ToolReference> $tools
+     * @param array<string, ResourceReference> $resources
+     * @param array<string, PromptReference> $prompts
+     * @param array<string, ResourceTemplateReference> $resourceTemplates
      */
-    private function processFile(SplFileInfo $file, array &$discoveredCount): void
+    private function processFile(SplFileInfo $file, array &$discoveredCount, array &$tools, array &$resources, array &$prompts, array &$resourceTemplates): void
     {
         $filePath = $file->getRealPath();
         if (false === $filePath) {
@@ -150,7 +175,7 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
                     foreach ($attributeTypes as $attributeType) {
                         $classAttribute = $reflectionClass->getAttributes($attributeType, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null;
                         if ($classAttribute) {
-                            $this->processMethod($invokeMethod, $discoveredCount, $classAttribute);
+                            $this->processMethod($invokeMethod, $discoveredCount, $classAttribute, $tools, $resources, $prompts, $resourceTemplates);
                             $processedViaClassAttribute = true;
                             break;
                         }
@@ -170,7 +195,7 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
                     foreach ($attributeTypes as $attributeType) {
                         $methodAttribute = $method->getAttributes($attributeType, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null;
                         if ($methodAttribute) {
-                            $this->processMethod($method, $discoveredCount, $methodAttribute);
+                            $this->processMethod($method, $discoveredCount, $methodAttribute, $tools, $resources, $prompts, $resourceTemplates);
                             break;
                         }
                     }
@@ -195,8 +220,12 @@ private function processFile(SplFileInfo $file, array &$discoveredCount): void
      * @param \ReflectionMethod                                                       $method          The target method (e.g., regular method or __invoke).
      * @param DiscoveredCount                                                         $discoveredCount pass by reference to update counts
      * @param \ReflectionAttribute<McpTool|McpResource|McpPrompt|McpResourceTemplate> $attribute       the ReflectionAttribute instance found (on method or class)
+     * @param array<string, ToolReference> $tools
+     * @param array<string, ResourceReference> $resources
+     * @param array<string, PromptReference> $prompts
+     * @param array<string, ResourceTemplateReference> $resourceTemplates
      */
-    private function processMethod(\ReflectionMethod $method, array &$discoveredCount, \ReflectionAttribute $attribute): void
+    private function processMethod(\ReflectionMethod $method, array &$discoveredCount, \ReflectionAttribute $attribute, array &$tools, array &$resources, array &$prompts, array &$resourceTemplates): void
     {
         $className = $method->getDeclaringClass()->getName();
         $classShortName = $method->getDeclaringClass()->getShortName();
@@ -213,7 +242,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun
                     $description = $instance->description ?? $this->docBlockParser->getSummary($docBlock) ?? null;
                     $inputSchema = $this->schemaGenerator->generate($method);
                     $tool = new Tool($name, $inputSchema, $description, $instance->annotations);
-                    $this->registry->registerTool($tool, [$className, $methodName]);
+                    $tools[$name] = new ToolReference($tool, [$className, $methodName], false);
                     ++$discoveredCount['tools'];
                     break;
 
@@ -225,7 +254,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun
                     $size = $instance->size;
                     $annotations = $instance->annotations;
                     $resource = new Resource($instance->uri, $name, $description, $mimeType, $annotations, $size);
-                    $this->registry->registerResource($resource, [$className, $methodName]);
+                    $resources[$instance->uri] = new ResourceReference($resource, [$className, $methodName], false);
                     ++$discoveredCount['resources'];
                     break;
 
@@ -245,7 +274,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun
                     }
                     $prompt = new Prompt($name, $description, $arguments);
                     $completionProviders = $this->getCompletionProviders($method);
-                    $this->registry->registerPrompt($prompt, [$className, $methodName], $completionProviders);
+                    $prompts[$name] = new PromptReference($prompt, [$className, $methodName], false, $completionProviders);
                     ++$discoveredCount['prompts'];
                     break;
 
@@ -257,7 +286,7 @@ private function processMethod(\ReflectionMethod $method, array &$discoveredCoun
                     $annotations = $instance->annotations;
                     $resourceTemplate = new ResourceTemplate($instance->uriTemplate, $name, $description, $mimeType, $annotations);
                     $completionProviders = $this->getCompletionProviders($method);
-                    $this->registry->registerResourceTemplate($resourceTemplate, [$className, $methodName], $completionProviders);
+                    $resourceTemplates[$instance->uriTemplate] = new ResourceTemplateReference($resourceTemplate, [$className, $methodName], false, $completionProviders);
                     ++$discoveredCount['resourceTemplates'];
                     break;
             }