[Server] bugfix: Cache discovery is unavailable

[okxaas]

Changes

Moved discovery state registration logic from the Discoverer class to the Builder class.

Specific Changes

1. Discoverer.php: Removed the setDiscoveryState() call, making the discover() method solely responsible for returning a DiscoveryState object.

2. Builder.php: Added handling and registration of the returned DiscoveryState object to the performDiscovery() method.

Why This Refactor Was Needed

Separation of Concerns

• Before: Discoverer could not return results in a cached state and did not call $registry->setDiscoveryState.
• After: Discoverer is solely responsible for discovering and returning results, while Builder coordinates the entire build process.

Impact

• Backward Compatibility: No impact on external APIs.
• Internal Improvements: Improved code cohesion and maintainability.
• Performance: No performance impact, only code reorganization.

[chr-hertel]

Hey @okxaas, thanks for reaching out - not sure I understand how you ran into an error, but I still this change makes sense 👍

See #115 and especially #116 for what I changed to provide more insights.

Anyhow, like i wrote, makes sense still to me, but you need to address this one as well:

php-sdk/src/Capability/Discovery/Discoverer.php

Lines 92 to 101 in 6100ffc

	if (empty($absolutePaths)) {
	$this->logger->warning('No valid discovery directories found to scan.', [
	'configured_paths' => $directories,
	'base_path' => $basePath,
	]);
	$emptyState = new DiscoveryState();
	$this->registry->setDiscoveryState($emptyState);
	return $emptyState;

cc @xentixar any thoughts on this?

[okxaas]

Hey @chr-hertel, with discovery caching enabled, the system was unable to utilize the cached content during actual runtime. Although the cache files were generated, the cached content couldn't be accessed. The issue was resolved after adding $registry->setDiscoveryState($discoveryState) to the performDiscovery method, which allowed the system to properly use the cached content.

[okxaas]

I read through #115 and #116 and the issue, I came up with was caused by cache generation and the discovery state of the cache not being registered.

[xentixar]

Hey @chr-hertel, good catch! You're right - the PR is incomplete.

The refactoring removes setDiscoveryState() from the main path (line ~130) but leaves it at line 99 in the early-return path when no directories are found. For consistency, that call should also be removed:

Line 92-101 should change from:

if (empty($absolutePaths)) {
    $this->logger->warning('No valid discovery directories found to scan.', [
        'configured_paths' => $directories,
        'base_path' => $basePath,
    ]);

    $emptyState = new DiscoveryState();
    $this->registry->setDiscoveryState($emptyState);

    return $emptyState;
}

To:

if (empty($absolutePaths)) {
    $this->logger->warning('No valid discovery directories found to scan.', [
        'configured_paths' => $directories,
        'base_path' => $basePath,
    ]);

    return new DiscoveryState();
}

This way, Builder::performDiscovery() consistently handles state registration for all paths.

@okxaas - Can you update line 99 to complete the refactoring?

[lvluoyue]

hi, This issue occurs in CachedDiscoverer.php at lines 66-68, where there's an inconsistency with Discoverer.php lines 129-133. The problem is that CachedDiscoverer.php is missing the setDiscoveryState call.

//  CachedDiscoverer.php
        $discoveryState = $this->discoverer->discover($basePath, $directories, $excludeDirs);

        $this->cache->set($cacheKey, $discoveryState);

        return $discoveryState;

// Discoverer.php
        $discoveryState = new DiscoveryState($tools, $resources, $prompts, $resourceTemplates);

        $this->registry->setDiscoveryState($discoveryState);

        return $discoveryState;

[okxaas]

@lvluoyue Very correct expression @xentixar

[chr-hertel]

Understood & makes sense, still needs the patch here:

php-sdk/src/Capability/Discovery/Discoverer.php

Lines 92 to 101 in 6100ffc

	if (empty($absolutePaths)) {
	$this->logger->warning('No valid discovery directories found to scan.', [
	'configured_paths' => $directories,
	'base_path' => $basePath,
	]);
	$emptyState = new DiscoveryState();
	$this->registry->setDiscoveryState($emptyState);
	return $emptyState;

Thanks already! :) 👍