From de5600b85594929309660ea2695d00114e4b23d6 Mon Sep 17 00:00:00 2001 From: Snider Date: Sat, 8 Aug 2026 12:25:03 +0100 Subject: [PATCH] fix(mcp): roll the dependency contracts, so the tool classes can be built MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AgentTool does `use ValidatesDependencies` and `implements HasDependencies`, and neither symbol has ever existed in this repository — they live in dappcore/mcp, which this package deliberately does not depend on. Every class extending AgentTool was therefore fatal on load: php -r 'new \Core\Mod\Agentic\Mcp\Tools\Agent\Brain\BrainRemember();' FATAL: Trait "Core\Mcp\Tools\Concerns\ValidatesDependencies" not found All forty registered tool classes, unconstructible in every environment. Not "unregistered" — unbuildable. The dead $listens event registration that meant nothing ever tried to instantiate one was anaesthetic, not a second bug: make registration live without this and the suite goes from 156 failures to 1321. The design is a local copy of Core\Mcp, not a dependency on it, so the fix is to finish the copy. Three contracts roll in unchanged from dappcore/mcp, which now holds the canonical versions: DependencyType, ToolDependency and HasDependencies. Each gains the SPDX header this repo requires. Two could NOT be rolled verbatim, because upstream's and this repo's ToolDependencyService share a name and not an API. MissingDependencyException upstream takes (string $toolName, array $missingDependencies, array $suggestedOrder) and composes its own message. This service raises it as `new $exceptionClass($message)` — one argument. Copying the upstream signature would have replaced a class-not-found with an ArgumentCountError the first time a dependency went unmet. It is written message-first, with the detail as optional arguments. ValidatesDependencies upstream calls checkDependencies() and getMissingDependencies(), which do not exist here — the equivalents are canExecute() and missing() — and passes named arguments to a method declared `validateDependencies(mixed ...$arguments)`, where they bind to nothing. It is written against the API this repo actually has, positionally, and returns the service's own {tool, type, key, message} rows rather than pretending to hand back ToolDependency objects. Two supporting fixes fall out. AgentToolRegistry imported Core\Mcp\Services\ToolDependencyService, which resolves to this repo's php/Mcp/Services/ToolDependencyService.php — a file declaring Core\Mod\Agentic\Mcp\Services\ToolDependencyService — so autoloading it raised "Cannot redeclare". It now imports the class that is actually there. And normaliseDependency() met ToolDependency objects with get_object_vars(), which hands the array branch a DependencyType enum where it casts to string, and loses the text because the object calls it description where the service reads message; it now goes through toArray() and maps the field across. Sixteen tools declare real dependencies via ToolDependency::contextExists(), so this path is live, not hypothetical. Receipts: all 40 tool classes construct, verified by instantiating every one. Suite 131 failed / 1190 passed, from 156 / 1165 — exactly the 25-test ValidatesDependencies cluster fixed, zero new failures, confirmed by diffing failing test names either side. Co-Authored-By: Virgil --- php/Mcp/Dependencies/DependencyType.php | 59 +++++++ php/Mcp/Dependencies/HasDependencies.php | 23 +++ php/Mcp/Dependencies/ToolDependency.php | 136 +++++++++++++++ .../Exceptions/MissingDependencyException.php | 62 +++++++ php/Mcp/Services/ToolDependencyService.php | 16 ++ .../Tools/Concerns/ValidatesDependencies.php | 158 ++++++++++++++++++ php/Services/AgentToolRegistry.php | 5 +- 7 files changed, 457 insertions(+), 2 deletions(-) create mode 100644 php/Mcp/Dependencies/DependencyType.php create mode 100644 php/Mcp/Dependencies/HasDependencies.php create mode 100644 php/Mcp/Dependencies/ToolDependency.php create mode 100644 php/Mcp/Exceptions/MissingDependencyException.php create mode 100644 php/Mcp/Tools/Concerns/ValidatesDependencies.php diff --git a/php/Mcp/Dependencies/DependencyType.php b/php/Mcp/Dependencies/DependencyType.php new file mode 100644 index 00000000..243a2734 --- /dev/null +++ b/php/Mcp/Dependencies/DependencyType.php @@ -0,0 +1,59 @@ + 'Tool must be called first', + self::SESSION_STATE => 'Session state required', + self::CONTEXT_EXISTS => 'Context value required', + self::ENTITY_EXISTS => 'Entity must exist', + self::CUSTOM => 'Custom condition', + }; + } +} diff --git a/php/Mcp/Dependencies/HasDependencies.php b/php/Mcp/Dependencies/HasDependencies.php new file mode 100644 index 00000000..2f5d1e65 --- /dev/null +++ b/php/Mcp/Dependencies/HasDependencies.php @@ -0,0 +1,23 @@ + + */ + public function dependencies(): array; +} diff --git a/php/Mcp/Dependencies/ToolDependency.php b/php/Mcp/Dependencies/ToolDependency.php new file mode 100644 index 00000000..a9503241 --- /dev/null +++ b/php/Mcp/Dependencies/ToolDependency.php @@ -0,0 +1,136 @@ +type, + key: $this->key, + description: $this->description, + optional: true, + metadata: $this->metadata, + ); + } + + /** + * Convert to array representation. + */ + public function toArray(): array + { + return [ + 'type' => $this->type->value, + 'key' => $this->key, + 'description' => $this->description, + 'optional' => $this->optional, + 'metadata' => $this->metadata, + ]; + } + + /** + * Create from array representation. + */ + public static function fromArray(array $data): self + { + return new self( + type: DependencyType::from($data['type']), + key: $data['key'], + description: $data['description'] ?? null, + optional: $data['optional'] ?? false, + metadata: $data['metadata'] ?? [], + ); + } +} diff --git a/php/Mcp/Exceptions/MissingDependencyException.php b/php/Mcp/Exceptions/MissingDependencyException.php new file mode 100644 index 00000000..04ae0115 --- /dev/null +++ b/php/Mcp/Exceptions/MissingDependencyException.php @@ -0,0 +1,62 @@ + $missingDependencies + * The rows ToolDependencyService::missing() returns — arrays, not + * ToolDependency objects, because that is what this service produces. + * @param array $suggestedOrder Tools worth calling first. + */ + public function __construct( + string $message, + public readonly array $missingDependencies = [], + public readonly string $toolName = '', + public readonly array $suggestedOrder = [], + ) { + parent::__construct($message); + } + + /** + * The dependency keys that were not satisfied. + * + * @return array + * + * @example + * $exception->missingKeys(); // ['workspace_id'] + */ + public function missingKeys(): array + { + return array_values(array_filter(array_map( + static fn (array $dependency): string => (string) ($dependency['key'] ?? ''), + $this->missingDependencies, + ))); + } +} diff --git a/php/Mcp/Services/ToolDependencyService.php b/php/Mcp/Services/ToolDependencyService.php index bc2ea2bf..7dd36cee 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\Mcp\Dependencies\ToolDependency; use Illuminate\Container\Container; use InvalidArgumentException; use RuntimeException; @@ -272,6 +273,21 @@ private function normaliseDependency(mixed $dependency): array ]; } + if ($dependency instanceof ToolDependency) { + // toArray(), not get_object_vars(): the object holds $type as a + // DependencyType enum and names its text $description. The array + // branch below casts type to string — which fatals on an enum — and + // looks for 'message', so a raw property dump both breaks and + // silently loses the description. + $fields = $dependency->toArray(); + $fields['message'] = $fields['description'] ?? null; + + return $this->normaliseDependency(array_filter( + $fields, + static fn (mixed $value): bool => $value !== null, + )); + } + if (is_object($dependency)) { $vars = get_object_vars($dependency); diff --git a/php/Mcp/Tools/Concerns/ValidatesDependencies.php b/php/Mcp/Tools/Concerns/ValidatesDependencies.php new file mode 100644 index 00000000..eb462a41 --- /dev/null +++ b/php/Mcp/Tools/Concerns/ValidatesDependencies.php @@ -0,0 +1,158 @@ + + * + * @example + * return [ToolDependency::contextExists('workspace_id')]; + */ + public function dependencies(): array + { + return []; + } + + /** + * Throw unless every declared dependency is satisfied. + * + * Positional, not named: the service takes mixed ...$arguments and parses + * them by position and type. + * + * @throws MissingDependencyException + * + * @example + * $this->validateDependencies(['session_id' => 'abc'], $args); + */ + protected function validateDependencies(array $context = [], array $args = []): void + { + app(ToolDependencyService::class)->validateDependencies( + $this->dependencySessionId($context), + $this->name(), + $context, + $args, + ); + } + + /** + * Whether every declared dependency is satisfied, without throwing. + * + * @example + * if (! $this->dependenciesMet($context, $args)) { ... } + */ + protected function dependenciesMet(array $context = [], array $args = []): bool + { + return app(ToolDependencyService::class)->canExecute( + $this->name(), + $context, + $args, + $this->dependencySessionId($context), + ); + } + + /** + * The dependencies that are not satisfied. + * + * Returns the service's own row shape — arrays of + * {tool, type, key, message} — rather than ToolDependency objects, because + * that is what it produces. + * + * @return array + * + * @example + * $missing = $this->getMissingDependencies($context, $args); + */ + protected function getMissingDependencies(array $context = [], array $args = []): array + { + return app(ToolDependencyService::class)->missing( + $this->name(), + $context, + $args, + $this->dependencySessionId($context), + ); + } + + /** + * Record this call so later tools can depend on it having happened. + * + * @example + * $this->recordToolCall($context, $args); + */ + protected function recordToolCall(array $context = [], array $args = []): void + { + app(ToolDependencyService::class)->recordToolCall( + $this->dependencySessionId($context), + $this->name(), + $args, + ); + } + + /** + * Shape an unmet-dependency failure as a tool error response. + * + * @example + * return $this->dependencyError($exception); + */ + protected function dependencyError(MissingDependencyException $exception): array + { + return [ + 'error' => 'dependency_not_met', + 'message' => $exception->getMessage(), + 'missing' => array_map( + static fn (array $dependency): array => [ + 'type' => $dependency['type'] ?? 'unknown', + 'key' => $dependency['key'] ?? '', + 'description' => $dependency['message'] ?? '', + ], + $exception->missingDependencies, + ), + 'suggested_order' => $exception->suggestedOrder, + ]; + } + + /** + * The session a dependency check is scoped to. + */ + private function dependencySessionId(array $context): string + { + $sessionId = $context['session_id'] ?? null; + + return is_string($sessionId) && $sessionId !== '' ? $sessionId : 'anonymous'; + } +} diff --git a/php/Services/AgentToolRegistry.php b/php/Services/AgentToolRegistry.php index 1db01d03..0dbbcd3d 100644 --- a/php/Services/AgentToolRegistry.php +++ b/php/Services/AgentToolRegistry.php @@ -6,7 +6,8 @@ use Core\Api\Models\ApiKey; use Core\Mcp\Dependencies\HasDependencies; -use Core\Mcp\Services\ToolDependencyService; +use Core\Mcp\Exceptions\MissingDependencyException; +use Core\Mod\Agentic\Mcp\Services\ToolDependencyService; use Core\Mod\Agentic\Mcp\Tools\Agent\Contracts\AgentToolInterface; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; @@ -181,7 +182,7 @@ public function apiKeyCanAccessTool(ApiKey $apiKey, string $toolName): bool * * @throws \InvalidArgumentException If tool not found * @throws \RuntimeException If permission denied - * @throws \Core\Mcp\Exceptions\MissingDependencyException If dependencies not met + * @throws MissingDependencyException If dependencies not met */ public function execute( string $name,