From eca0fb1e9ba22522118e58a88343e63526a12bbf Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 12:05:04 +0100 Subject: [PATCH 1/2] wip: consolidate tool registries (blocked on Core\Mcp contracts) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absorbs listTools/resolve/buildDependencyGraph/call into AgentToolRegistry, rewires McpAgentServerCommand and ToolDependencyService onto it, and deletes Mcp\Services\ToolRegistry. Deliberately NOT pushed. Moving the fill off $listens — the other half of the fix — makes register() construct the tool classes, which fatals on the missing Core\Mcp\Tools\Concerns\ValidatesDependencies trait and takes the suite from 156 failed to 1321. The registries can only usefully merge once agent consumes dappcore/mcp and the tools become constructible. 19 tests still fail here: their fixture is a duck-typed anonymous class and the surviving registry requires a real AgentToolInterface. Migrating them belongs with the change that turns the server on. --- php/Mcp/Console/McpAgentServerCommand.php | 10 +-- php/Mcp/Services/ToolDependencyService.php | 9 ++- php/Mcp/Services/ToolRegistry.php | 88 ---------------------- php/Services/AgentToolRegistry.php | 78 +++++++++++++++++++ 4 files changed, 88 insertions(+), 97 deletions(-) delete mode 100644 php/Mcp/Services/ToolRegistry.php diff --git a/php/Mcp/Console/McpAgentServerCommand.php b/php/Mcp/Console/McpAgentServerCommand.php index 1bc72a29..56609b96 100644 --- a/php/Mcp/Console/McpAgentServerCommand.php +++ b/php/Mcp/Console/McpAgentServerCommand.php @@ -8,8 +8,8 @@ use Core\Mod\Agentic\Mcp\Services\McpQuotaService; use Core\Mod\Agentic\Mcp\Services\QueryAuditService; -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; use Core\Mod\Agentic\Services\AgentResourceRegistry; +use Core\Mod\Agentic\Services\AgentToolRegistry; use Illuminate\Console\Command; use InvalidArgumentException; use JsonException; @@ -35,7 +35,7 @@ class McpAgentServerCommand extends Command * $exitCode = $this->handle($toolRegistry, $quotaService, $queryAuditService); */ public function handle( - ToolRegistry $toolRegistry, + AgentToolRegistry $toolRegistry, McpQuotaService $quotaService, QueryAuditService $queryAuditService, AgentResourceRegistry $resourceRegistry, @@ -118,7 +118,7 @@ private function streamPath(string $variable, string $default): string */ private function processPayload( string $payload, - ToolRegistry $toolRegistry, + AgentToolRegistry $toolRegistry, McpQuotaService $quotaService, QueryAuditService $queryAuditService, AgentResourceRegistry $resourceRegistry, @@ -174,7 +174,7 @@ private function processPayload( */ private function processRequest( array $request, - ToolRegistry $toolRegistry, + AgentToolRegistry $toolRegistry, McpQuotaService $quotaService, QueryAuditService $queryAuditService, AgentResourceRegistry $resourceRegistry, @@ -262,7 +262,7 @@ private function processRequest( private function handleToolCall( array $params, mixed $id, - ToolRegistry $toolRegistry, + AgentToolRegistry $toolRegistry, McpQuotaService $quotaService, QueryAuditService $queryAuditService, ): array { diff --git a/php/Mcp/Services/ToolDependencyService.php b/php/Mcp/Services/ToolDependencyService.php index bc2ea2bf..9b2e44ee 100644 --- a/php/Mcp/Services/ToolDependencyService.php +++ b/php/Mcp/Services/ToolDependencyService.php @@ -7,6 +7,7 @@ namespace Core\Mod\Agentic\Mcp\Services; use Carbon\CarbonImmutable; +use Core\Mod\Agentic\Services\AgentToolRegistry; use Illuminate\Container\Container; use InvalidArgumentException; use RuntimeException; @@ -24,7 +25,7 @@ final class ToolDependencyService private array $toolCalls = []; public function __construct( - private ?ToolRegistry $registry = null, + private ?AgentToolRegistry $registry = null, private readonly ?Container $container = null, ) { $this->registry ??= $this->resolveRegistry(); @@ -104,15 +105,15 @@ public function calledTools(string $sessionId): array return array_keys($this->toolCalls[$sessionId] ?? []); } - private function resolveRegistry(): ?ToolRegistry + private function resolveRegistry(): ?AgentToolRegistry { $container = $this->container ?? Container::getInstance(); - if (! $container instanceof Container || ! $container->bound(ToolRegistry::class)) { + if (! $container instanceof Container || ! $container->bound(AgentToolRegistry::class)) { return null; } - return $container->make(ToolRegistry::class); + return $container->make(AgentToolRegistry::class); } /** diff --git a/php/Mcp/Services/ToolRegistry.php b/php/Mcp/Services/ToolRegistry.php deleted file mode 100644 index 36690baa..00000000 --- a/php/Mcp/Services/ToolRegistry.php +++ /dev/null @@ -1,88 +0,0 @@ - - */ - private array $tools = []; - - public function __construct( - private readonly ?Container $container = null, - ) {} - - public static function registerSingleton(Container $container): self - { - if (! $container->bound(self::class)) { - $container->singleton(self::class, fn (Container $app): self => new self($app)); - } - - return $container->make(self::class); - } - - public function register(mixed $tool): ToolMetadata - { - $metadata = ToolMetadata::from($tool); - - if (isset($this->tools[$metadata->name])) { - throw new InvalidArgumentException(sprintf( - 'Tool [%s] is already registered.', - $metadata->name, - )); - } - - $this->tools[$metadata->name] = $metadata; - - return $metadata; - } - - public function resolve(string $name): ?ToolMetadata - { - return $this->tools[$name] ?? null; - } - - /** - * @return array - */ - public function listTools(): array - { - return array_values($this->tools); - } - - /** - * @return array> - */ - public function buildDependencyGraph(): array - { - $graph = []; - - foreach ($this->tools as $name => $tool) { - $graph[$name] = $tool->dependencyIdentifiers(); - } - - return $graph; - } - - public function call(string $name, array $arguments = [], array $context = []): mixed - { - $tool = $this->resolve($name); - if ($tool === null) { - throw new InvalidArgumentException(sprintf( - 'Unknown tool [%s].', - $name, - )); - } - - return $tool->call($arguments, $context); - } -} diff --git a/php/Services/AgentToolRegistry.php b/php/Services/AgentToolRegistry.php index 1db01d03..45e39174 100644 --- a/php/Services/AgentToolRegistry.php +++ b/php/Services/AgentToolRegistry.php @@ -7,6 +7,7 @@ use Core\Api\Models\ApiKey; use Core\Mcp\Dependencies\HasDependencies; use Core\Mcp\Services\ToolDependencyService; +use Core\Mod\Agentic\Mcp\Data\ToolMetadata; use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; @@ -362,4 +363,81 @@ private function enforceAndRecordRateLimit(ApiKey $apiKey, string $toolName): vo ); } } + + /** + * Every registered tool as MCP metadata. + * + * Absorbed from Mcp\Services\ToolRegistry, which the stdio agent server + * read while Boot filled this registry instead — so the server listed + * nothing. One registry now, so there is one answer to "what tools exist". + * + * @return array + * + * @example + * $registry->listTools(); + */ + public function listTools(): array + { + return array_values(array_map( + static fn (AgentToolInterface $tool): ToolMetadata => ToolMetadata::from($tool), + $this->tools, + )); + } + + /** + * Resolve one tool as MCP metadata, or null when it is not registered. + * + * @example + * $registry->resolve('plan_create'); + */ + public function resolve(string $name): ?ToolMetadata + { + $tool = $this->tools[$name] ?? null; + + return $tool === null ? null : ToolMetadata::from($tool); + } + + /** + * Map each tool name to the identifiers it declares as dependencies. + * + * @return array> + * + * @example + * $registry->buildDependencyGraph(); + */ + public function buildDependencyGraph(): array + { + $graph = []; + + foreach ($this->tools as $name => $tool) { + $graph[$name] = ToolMetadata::from($tool)->dependencyIdentifiers(); + } + + return $graph; + } + + /** + * Invoke a tool directly, without the permission and dependency checks + * execute() applies. + * + * Kept distinct from execute() rather than merged into it: the stdio + * transport has no API key to check scopes against and runs its own quota + * and audit passes around this call, whereas execute() is the governed + * path used where an ApiKey is present. + * + * @throws \InvalidArgumentException If the tool is not registered + * + * @example + * $registry->call('plan_list', [], ['workspace_id' => 'ws-1']); + */ + public function call(string $name, array $arguments = [], array $context = []): mixed + { + $tool = $this->get($name); + + if (! $tool) { + throw new \InvalidArgumentException(sprintf('Unknown tool [%s].', $name)); + } + + return $tool->handle($arguments, $context); + } } From 0c388bab3b17055146591d0cb42b6839a9bb3c1e Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 13:18:18 +0100 Subject: [PATCH 2/2] fix(mcp): one tool registry, filled at boot, read by the server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent MCP server advertised no tools. Three faults, each of which alone was enough, and a green suite that noticed none of them. Boot filled Core\Mod\Agentic\Services\AgentToolRegistry. McpAgentServerCommand read Core\Mod\Agentic\Mcp\Services\ToolRegistry — a different class, never bound, so Laravel handed the command a fresh empty instance on every resolution. tools/list returned []; tools/call found nothing. Two registries meant two answers to "what tools exist", and the server asked the one nobody filled. Boot filled its registry from the McpToolsRegistering event via $listens, which ModuleScanner populates by scanning app/Core|Mod|Website. Under vendor/ that is dead, so the event never fired and the registry it did fill was empty anyway. And every tool class was fatal on load until #21, so even a correct registration would have thrown on the first `new`. That one masked the other two: nothing ever tried to construct a tool, so nothing ever failed loudly. ToolRegistry is deleted and its capability absorbed: listTools(), resolve() and buildDependencyGraph() return ToolMetadata built from the registered tools, call() invokes one without the permission and dependency checks execute() applies — kept separate because the stdio transport has no API key to check scopes against and runs its own quota and audit passes around it. The duplicate-name guard comes across too: two tools claiming one name is a wiring mistake, and silently keeping the last one means the surface serves whichever file loaded second. The fill moves from the event into register(), the same lifecycle-independent path used for resources, and is idempotent so a host that still delivers the event cannot double-register. Nineteen tests registered duck-typed anonymous classes into the loose registry. They now implement AgentToolInterface — which they arguably always should have, since it is the contract the tools they stand in for satisfy. One test goes rather than being migrated: it asserted that a payload without a callable handler is rejected, and register() is now typed, so no array can reach that validation. There is no code path left that produces the behaviour it asserted. Guarded against recurrence by the test that was missing all along: on a plain booted application, registering nothing of its own, a tool constructs, the registry is non-empty, listTools() contains plan_create, session_start and brain_remember, and the binding is one shared instance. McpAgentServerCommandTest passed throughout the outage because its beforeEach supplied a tool — it tested the plumbing with a registry the test had filled, which is precisely the blind spot. Receipts: registry holds 40 tools after a real boot, listTools() returns the same 40, plan_create among them. Suite 131 failed / 1193 passed, from 131 / 1190 — four guards added, one obsolete test removed, zero regressions confirmed by diffing failing test names. Co-Authored-By: Virgil --- php/Boot.php | 28 +++++-- php/Services/AgentToolRegistry.php | 10 +++ .../Mcp/Console/McpAgentServerCommandTest.php | 15 +++- .../Mcp/Middleware/McpAuthenticateTest.php | 11 +-- .../ValidateToolDependenciesTest.php | 21 ++++-- .../Services/AgentToolRegistryBootTest.php | 72 ++++++++++++++++++ .../Services/ToolDependencyServiceTest.php | 75 +++++++++++++++---- .../Feature/Mcp/Services/ToolRegistryTest.php | 39 +++++----- 8 files changed, 220 insertions(+), 51 deletions(-) create mode 100644 php/tests/Feature/Mcp/Services/AgentToolRegistryBootTest.php diff --git a/php/Boot.php b/php/Boot.php index 14565c4a..f611297e 100644 --- a/php/Boot.php +++ b/php/Boot.php @@ -9,7 +9,6 @@ use Core\Events\AdminPanelBooting; use Core\Events\ApiRoutesRegistering; use Core\Events\ConsoleBooting; -use Core\Events\McpToolsRegistering; use Core\Mod\Agentic\Services\AgenticManager; use Core\Mod\Agentic\Services\AgentResourceRegistry; use Core\Mod\Agentic\Services\AgentToolRegistry; @@ -34,7 +33,6 @@ class Boot extends ServiceProvider AdminPanelBooting::class => 'onAdminPanel', ApiRoutesRegistering::class => 'onApiRoutes', ConsoleBooting::class => 'onConsole', - McpToolsRegistering::class => 'onMcpTools', ]; public function boot(): void @@ -98,6 +96,8 @@ public function register(): void $this->app->singleton(AgenticManager::class); $this->app->singleton(AgentToolRegistry::class); + $this->registerAgentTools(); + // Resources are bound here rather than hung off an event the way tools // are. There is no McpResourcesRegistering to listen for, and $listens // is populated by ModuleScanner from app/Core|Mod|Website only — dead @@ -215,16 +215,30 @@ public function onConsole(ConsoleBooting $event): void } /** - * Handle MCP tools registration event. + * Fill the tool registry. + * + * Called from register(), not from the McpToolsRegistering event this used + * to listen for. $listens is populated by ModuleScanner scanning + * app/Core|Mod|Website only, so it is dead once this package is installed + * under vendor/: the event never fired and the registry stayed empty, which + * is half of why the stdio server advertised no tools. The other half was + * that it read a different registry entirely — see listTools() on the class + * this fills. * - * Note: Agent tools (plan_create, session_start, etc.) are implemented in - * the Mcp module at Mod\Mcp\Tools\Agent\* and registered via AgentToolRegistry. - * Brain tools are registered here as they belong to the Agentic module. + * @example + * $this->registerAgentTools(); */ - public function onMcpTools(McpToolsRegistering $event): void + private function registerAgentTools(): void { $registry = $this->app->make(AgentToolRegistry::class); + // Idempotent: register() runs once per application, but a host that + // still delivers the event must not double-register and trip the + // duplicate-name guard. + if ($registry->all()->isNotEmpty()) { + return; + } + $toolClasses = [ Mcp\Tools\Agent\Brain\BrainRemember::class, Mcp\Tools\Agent\Brain\BrainRecall::class, diff --git a/php/Services/AgentToolRegistry.php b/php/Services/AgentToolRegistry.php index 86eb8d4c..7a0d2db1 100644 --- a/php/Services/AgentToolRegistry.php +++ b/php/Services/AgentToolRegistry.php @@ -38,6 +38,16 @@ class AgentToolRegistry */ public function register(AgentToolInterface $tool): self { + // Absorbed from the registry this replaced: two tools claiming one name + // is a wiring mistake, and silently keeping the last one registered + // means the MCP surface serves whichever file happened to load second. + if (isset($this->tools[$tool->name()])) { + throw new \InvalidArgumentException(sprintf( + 'Tool [%s] is already registered.', + $tool->name(), + )); + } + $this->tools[$tool->name()] = $tool; // Auto-register dependencies if tool declares them diff --git a/php/tests/Feature/Mcp/Console/McpAgentServerCommandTest.php b/php/tests/Feature/Mcp/Console/McpAgentServerCommandTest.php index cf0a9684..1968a473 100644 --- a/php/tests/Feature/Mcp/Console/McpAgentServerCommandTest.php +++ b/php/tests/Feature/Mcp/Console/McpAgentServerCommandTest.php @@ -6,8 +6,9 @@ use Core\Mod\Agentic\Mcp\Console\McpAgentServerCommand; use Core\Mod\Agentic\Mcp\Services\McpQuotaService; -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; +use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; use Core\Mod\Agentic\Models\AgentPlan; +use Core\Mod\Agentic\Services\AgentToolRegistry; use Illuminate\Contracts\Console\Kernel; use Illuminate\Database\Schema\Blueprint; use Illuminate\Support\Facades\Artisan; @@ -19,7 +20,7 @@ $this->app->make(McpAgentServerCommand::class), ); - ToolRegistry::registerSingleton($this->app)->register(new class + $this->app->make(AgentToolRegistry::class)->register(new class implements AgentToolInterface { public function name(): string { @@ -44,6 +45,16 @@ public function handle(array $arguments, array $context = []): array 'value' => $arguments['value'] ?? null, ]; } + + public function requiredScopes(): array + { + return ['read']; + } + + public function category(): string + { + return 'testing'; + } }); Schema::dropIfExists('mcp_audit_entries'); diff --git a/php/tests/Feature/Mcp/Middleware/McpAuthenticateTest.php b/php/tests/Feature/Mcp/Middleware/McpAuthenticateTest.php index fa4a2e91..b9d63d73 100644 --- a/php/tests/Feature/Mcp/Middleware/McpAuthenticateTest.php +++ b/php/tests/Feature/Mcp/Middleware/McpAuthenticateTest.php @@ -6,8 +6,8 @@ use Core\Mod\Agentic\Mcp\Services\McpQuotaService; use Core\Mod\Agentic\Mcp\Services\ToolDependencyService; -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; use Core\Mod\Agentic\Services\AgentApiKeyService; +use Core\Mod\Agentic\Services\AgentToolRegistry; use Core\Mod\Agentic\Website\Mcp\Middleware\CheckMcpQuota; use Core\Mod\Agentic\Website\Mcp\Middleware\McpApiKeyAuth; use Core\Mod\Agentic\Website\Mcp\Middleware\McpAuthenticate; @@ -15,6 +15,7 @@ use Core\Mod\Agentic\Website\Mcp\Middleware\ValidateWorkspaceContext; use Illuminate\Http\Request; use Illuminate\Support\Facades\Cache; +use Symfony\Component\HttpFoundation\Response; beforeEach(function (): void { Cache::flush(); @@ -26,7 +27,7 @@ $workspace = createWorkspace(); $apiKey = createApiKey($workspace, 'Combined Auth Key'); - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpMiddlewareToolFixture('plan_list', [ ['type' => 'context_exists', 'key' => 'workspace_id', 'message' => 'Workspace context required.'], ])); @@ -72,7 +73,7 @@ new McpApiKeyAuth(app(AgentApiKeyService::class)), new CheckMcpQuota($quotaService), new ValidateWorkspaceContext, - new ValidateToolDependencies(new ToolDependencyService(new ToolRegistry, $this->app)), + new ValidateToolDependencies(new ToolDependencyService(new AgentToolRegistry, $this->app)), ); $request = Request::create('/api/v1/mcp/tools/call', 'POST', [ @@ -93,7 +94,7 @@ test('McpAuthenticate_handle_Ugly_bubbles_missing_workspace_context_failures_from_the_validation_stage', function (): void { $brokenAuth = new class(app(AgentApiKeyService::class)) extends McpApiKeyAuth { - public function handle(Request $request, Closure $next): \Symfony\Component\HttpFoundation\Response + public function handle(Request $request, Closure $next): Response { return $next($request); } @@ -103,7 +104,7 @@ public function handle(Request $request, Closure $next): \Symfony\Component\Http $brokenAuth, new CheckMcpQuota(new McpQuotaService), new ValidateWorkspaceContext, - new ValidateToolDependencies(new ToolDependencyService(new ToolRegistry, $this->app)), + new ValidateToolDependencies(new ToolDependencyService(new AgentToolRegistry, $this->app)), ); $request = Request::create('/api/v1/mcp/tools/call', 'POST', [ diff --git a/php/tests/Feature/Mcp/Middleware/ValidateToolDependenciesTest.php b/php/tests/Feature/Mcp/Middleware/ValidateToolDependenciesTest.php index e3bcdc22..c0451e00 100644 --- a/php/tests/Feature/Mcp/Middleware/ValidateToolDependenciesTest.php +++ b/php/tests/Feature/Mcp/Middleware/ValidateToolDependenciesTest.php @@ -5,13 +5,14 @@ declare(strict_types=1); use Core\Mod\Agentic\Mcp\Services\ToolDependencyService; -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; +use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; +use Core\Mod\Agentic\Services\AgentToolRegistry; use Core\Mod\Agentic\Website\Mcp\Middleware\ValidateToolDependencies; use Illuminate\Http\Request; function mcpMiddlewareToolFixture(string $name, array $dependencies = []): object { - return new class($name, $dependencies) + return new class($name, $dependencies) implements AgentToolInterface { public function __construct( private readonly string $toolName, @@ -46,11 +47,21 @@ public function handle(array $arguments, array $context = []): array 'tool' => $this->toolName, ]; } + + public function requiredScopes(): array + { + return ['read']; + } + + public function category(): string + { + return 'testing'; + } }; } test('ValidateToolDependencies_handle_Good_validates_json_rpc_tool_calls_and_records_successful_execution', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpMiddlewareToolFixture('session_start')); $registry->register(mcpMiddlewareToolFixture('report_generate', [ ['type' => 'tool', 'tool' => 'session_start', 'message' => 'Start session first.'], @@ -80,7 +91,7 @@ public function handle(array $arguments, array $context = []): array }); test('ValidateToolDependencies_handle_Bad_returns_conflict_when_required_dependencies_are_missing', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpMiddlewareToolFixture('plan_list', [ ['type' => 'context_exists', 'key' => 'workspace_id', 'message' => 'Workspace context required.'], ])); @@ -101,7 +112,7 @@ public function handle(array $arguments, array $context = []): array }); test('ValidateToolDependencies_handle_Ugly_converts_circular_dependency_failures_into_conflict_responses', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpMiddlewareToolFixture('tool_alpha', [ ['type' => 'tool', 'tool' => 'tool_bravo', 'message' => 'tool_bravo is required.'], ])); diff --git a/php/tests/Feature/Mcp/Services/AgentToolRegistryBootTest.php b/php/tests/Feature/Mcp/Services/AgentToolRegistryBootTest.php new file mode 100644 index 00000000..60cb26ba --- /dev/null +++ b/php/tests/Feature/Mcp/Services/AgentToolRegistryBootTest.php @@ -0,0 +1,72 @@ +assertSame('brain_remember', (new BrainRemember)->name()); + } + + public function test_registry_good_is_filled_by_boot_with_no_test_supplied_tools(): void + { + $registry = $this->app->make(AgentToolRegistry::class); + + $this->assertNotEmpty( + $registry->all(), + 'Boot registered no tools — the agent MCP server would advertise nothing.', + ); + } + + public function test_registry_good_exposes_those_tools_on_the_path_the_server_reads(): void + { + // listTools() is what McpAgentServerCommand answers tools/list from. + // Filled-but-unreadable was the shape of fault 3. + $names = array_map( + static fn ($tool): string => $tool->name, + $this->app->make(AgentToolRegistry::class)->listTools(), + ); + + $this->assertNotEmpty($names); + $this->assertContains('plan_create', $names); + $this->assertContains('session_start', $names); + $this->assertContains('brain_remember', $names); + } + + public function test_registry_good_is_one_shared_instance(): void + { + // A non-singleton hands the command a fresh empty registry, which is + // how the registry it used to read failed even once something filled it. + $this->assertSame( + $this->app->make(AgentToolRegistry::class), + $this->app->make(AgentToolRegistry::class), + ); + } +} diff --git a/php/tests/Feature/Mcp/Services/ToolDependencyServiceTest.php b/php/tests/Feature/Mcp/Services/ToolDependencyServiceTest.php index 57b4c4d6..62ae54ee 100644 --- a/php/tests/Feature/Mcp/Services/ToolDependencyServiceTest.php +++ b/php/tests/Feature/Mcp/Services/ToolDependencyServiceTest.php @@ -5,24 +5,69 @@ declare(strict_types=1); use Core\Mod\Agentic\Mcp\Services\ToolDependencyService; -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; +use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; +use Core\Mod\Agentic\Services\AgentToolRegistry; -function mcpDependencyToolFixture(string $name, array $dependencies = []): array +/** + * A registrable tool declaring the given dependencies. + * + * An AgentToolInterface, not the loose array payload this used to build: the + * registry these tests exercise is now the typed one, and a metadata array is + * no longer a thing that can be registered. The dependency rows stay arrays, + * which is what ToolDependencyService normalises. + */ +function mcpDependencyToolFixture(string $name, array $dependencies = []): AgentToolInterface { - return [ - 'name' => $name, - 'description' => 'Fixture tool', - 'dependencies' => $dependencies, - 'handler' => static fn (array $arguments = [], array $context = []): array => [ - 'arguments' => $arguments, - 'context' => $context, - 'tool' => $name, - ], - ]; + return new class($name, $dependencies) implements AgentToolInterface + { + public function __construct( + private readonly string $toolName, + private readonly array $toolDependencies, + ) {} + + public function name(): string + { + return $this->toolName; + } + + public function description(): string + { + return 'Fixture tool'; + } + + public function inputSchema(): array + { + return ['type' => 'object']; + } + + public function dependencies(): array + { + return $this->toolDependencies; + } + + public function handle(array $args, array $context = []): array + { + return [ + 'arguments' => $args, + 'context' => $context, + 'tool' => $this->toolName, + ]; + } + + public function requiredScopes(): array + { + return ['read']; + } + + public function category(): string + { + return 'testing'; + } + }; } test('ToolDependencyService_validateDependencies_Good_walks_transitive_tool_graphs_before_execution', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpDependencyToolFixture('session_start')); $registry->register(mcpDependencyToolFixture('session_log', [ @@ -48,7 +93,7 @@ function mcpDependencyToolFixture(string $name, array $dependencies = []): array }); test('ToolDependencyService_validateDependencies_Bad_reports_missing_context_requirements', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpDependencyToolFixture('plan_list', [ ['type' => 'context_exists', 'key' => 'workspace_id', 'message' => 'Workspace context required.'], ])); @@ -58,7 +103,7 @@ function mcpDependencyToolFixture(string $name, array $dependencies = []): array })->throws(RuntimeException::class, 'Workspace context required.'); test('ToolDependencyService_validateDependencies_Ugly_detects_circular_tool_dependencies', function (): void { - $registry = new ToolRegistry; + $registry = new AgentToolRegistry; $registry->register(mcpDependencyToolFixture('tool_alpha', [ ['type' => 'tool', 'tool' => 'tool_bravo', 'message' => 'tool_bravo is required.'], diff --git a/php/tests/Feature/Mcp/Services/ToolRegistryTest.php b/php/tests/Feature/Mcp/Services/ToolRegistryTest.php index b978942b..3d154020 100644 --- a/php/tests/Feature/Mcp/Services/ToolRegistryTest.php +++ b/php/tests/Feature/Mcp/Services/ToolRegistryTest.php @@ -4,11 +4,12 @@ declare(strict_types=1); -use Core\Mod\Agentic\Mcp\Services\ToolRegistry; +use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; +use Core\Mod\Agentic\Services\AgentToolRegistry; function mcpToolRegistryFixture(string $name, array $dependencies = []): object { - return new class($name, $dependencies) + return new class($name, $dependencies) implements AgentToolInterface { public function __construct( private readonly string $toolName, @@ -43,11 +44,24 @@ public function handle(array $arguments, array $context = []): array 'tool' => $this->toolName, ]; } + + public function requiredScopes(): array + { + return ['read']; + } + + public function category(): string + { + return 'testing'; + } }; } -test('ToolRegistry_register_resolve_listTools_buildDependencyGraph_Good_binds_the_registry_as_a_singleton', function (): void { - $registry = ToolRegistry::registerSingleton($this->app); +test('AgentToolRegistry_register_resolve_listTools_buildDependencyGraph_Good_absorbs_the_mcp_surface', function (): void { + // A fresh registry, not the container's: Boot now fills the singleton with + // the forty real tools, so registering session_start into it would collide + // with the real one and listTools() would return forty-two. + $registry = new AgentToolRegistry; $registry->register(mcpToolRegistryFixture('session_start')); $registry->register(mcpToolRegistryFixture('report_generate', [ @@ -55,10 +69,10 @@ public function handle(array $arguments, array $context = []): array ['type' => 'context_exists', 'key' => 'workspace_id'], ])); - $resolved = $this->app->make(ToolRegistry::class); $graph = $registry->buildDependencyGraph(); - expect($resolved)->toBe($registry) + expect($this->app->make(AgentToolRegistry::class)) + ->toBe($this->app->make(AgentToolRegistry::class)) ->and($registry->resolve('report_generate')?->name)->toBe('report_generate') ->and(array_map( static fn ($tool): string => $tool->name, @@ -76,18 +90,9 @@ public function handle(array $arguments, array $context = []): array ]); }); -test('ToolRegistry_register_Bad_rejects_duplicate_tool_names', function (): void { - $registry = new ToolRegistry; +test('AgentToolRegistry_register_Bad_rejects_duplicate_tool_names', function (): void { + $registry = new AgentToolRegistry; $registry->register(mcpToolRegistryFixture('session_start')); $registry->register(mcpToolRegistryFixture('session_start')); })->throws(InvalidArgumentException::class, 'Tool [session_start] is already registered.'); - -test('ToolRegistry_register_Ugly_rejects_payloads_without_a_callable_handler', function (): void { - $registry = new ToolRegistry; - - $registry->register([ - 'name' => 'broken_tool', - 'description' => 'No callable handler', - ]); -})->throws(InvalidArgumentException::class, 'A callable handler is required');