fix: Register MCP tools via Laravel Boost config #1
Conversation
WalkthroughPlugin tool registration moved from runtime MCP server hooks to boot-time augmentation of Laravel Boost config via a new protected method; five read‑only MCP Tool classes were added under Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as Plugin.php
participant Config as boost config
participant MCP as Laravel Boost (MCP)
participant Tool as MCP Tool class
Note over Plugin,Config: Boot-time augmentation replaces runtime provider hooks
Plugin->>Config: read 'boost.mcp.tools.include'
Plugin->>Config: merge new Tool class names (extendBoostConfiguration)
Plugin->>Config: write updated config
Note over MCP,Config: MCP reads configured tools at startup
MCP->>Config: read 'boost.mcp.tools.include'
MCP->>Tool: instantiate configured Tool classes
Tool->>MCP: register endpoints / handle incoming requests
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
classes/Tools/WinterProjectStructure.php (1)
77-89: Consider handlingglob()returning false.
glob()returnsfalseon error (e.g., permission issues), which would cause a warning when passed toforeach. Consider adding a null coalescing fallback.🔎 Proposed fix
// Controllers from this plugin $controllersPath = $pluginPath . '/controllers'; if (is_dir($controllersPath)) { - $controllerFiles = glob($controllersPath . '/*.php'); + $controllerFiles = glob($controllersPath . '/*.php') ?: []; foreach ($controllerFiles as $file) { $controllerName = basename($file, '.php'); $structure['controllers'][] = [classes/Tools/WinterViewStructure.php (2)
55-71: Consider handlingglob()returning false.Similar to the other tools,
glob()can returnfalseon error, which would cause issues withforeach.🔎 Proposed fix
// Map plugin component templates - $pluginComponentViews = glob(base_path('plugins/*/*/components/*/*.htm')); + $pluginComponentViews = glob(base_path('plugins/*/*/components/*/*.htm')) ?: []; foreach ($pluginComponentViews as $viewFile) {
73-90: Sameglob()false-return consideration.🔎 Proposed fix
// Map backend controller views - $controllerViews = glob(base_path('plugins/*/*/controllers/*/*.php')); + $controllerViews = glob(base_path('plugins/*/*/controllers/*/*.php')) ?: []; foreach ($controllerViews as $viewFile) {Plugin.php (1)
82-131: Consider making documentation version configurable.The Winter CMS documentation version (1.2) and Twig version (3.x) are hardcoded. If these need to be updated frequently, consider extracting them to a config file or constants.
The implementation is correct and functional. This is just a maintainability consideration for the future.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Plugin.phpclasses/Tools/WinterDevelopmentGuide.phpclasses/Tools/WinterProjectOverview.phpclasses/Tools/WinterProjectStructure.phpclasses/Tools/WinterScaffoldingCommands.phpclasses/Tools/WinterViewStructure.php
🧰 Additional context used
🧬 Code graph analysis (3)
classes/Tools/WinterScaffoldingCommands.php (4)
classes/Tools/WinterDevelopmentGuide.php (3)
IsReadOnly(14-63)schema(22-25)handle(27-62)classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)classes/Tools/WinterProjectStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)
classes/Tools/WinterProjectStructure.php (3)
classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)Plugin.php (1)
pluginDetails(24-32)
classes/Tools/WinterDevelopmentGuide.php (4)
classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)classes/Tools/WinterProjectStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)classes/Tools/WinterScaffoldingCommands.php (3)
IsReadOnly(14-90)schema(22-25)handle(27-89)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)
🪛 PHPMD (2.15.0)
classes/Tools/WinterProjectOverview.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterScaffoldingCommands.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterViewStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterProjectStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterDevelopmentGuide.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (10)
classes/Tools/WinterProjectOverview.php (2)
1-25: LGTM - Well-structured tool class setup.The class follows the Laravel MCP Tool pattern correctly. The
$schemaand$requestparameters are required by the parent interface, so the PHPMD warnings are false positives.
27-72: Good defensive coding with graceful fallbacks.The version detection with nested try/catch fallback to Artisan command is resilient. The
class_exists()checks ensure compatibility when optional Winter CMS modules aren't available. ThegetBuildNumberManually()method correctly retrieves build information and returns an array with the expected 'build' and 'modified' keys.classes/Tools/WinterDevelopmentGuide.php (1)
1-62: LGTM - Clean static documentation tool.The tool returns well-structured static guidance data. No external I/O or dependencies make this fast and reliable. The PHPMD warnings for unused parameters are false positives since these are interface requirements.
classes/Tools/WinterProjectStructure.php (2)
27-31: Good early exit for missing dependency.The check for
PluginManageravailability before proceeding prevents errors in non-Winter environments.
92-99: LGTM - Summary aggregation.The summary counts provide useful metadata for the response.
classes/Tools/WinterScaffoldingCommands.php (1)
1-89: LGTM - Comprehensive scaffolding command reference.The tool provides a well-structured, static guide to Winter CMS scaffolding commands. The coverage of core commands with syntax, examples, and options is thorough.
classes/Tools/WinterViewStructure.php (1)
92-99: LGTM - Conventions section is helpful.The conventions map provides clear guidance on file naming patterns.
Plugin.php (3)
9-13: LGTM - Clean imports for the new tool classes.
53-57: LGTM - Boot sequence properly initializes both tool registration and config extension.
59-80: Good approach using config augmentation instead of runtime registration.The shift to config-based tool registration via
boost.mcp.tools.includealigns with Laravel Boost v1.8.x expectations. Theclass_existscheck ensures graceful handling when MCP isn't available.
The previous implementation tried to hook into `app('mcp.server')` which
doesn't exist in Laravel Boost. Laravel Boost uses a different approach
for registering additional tools via `config('boost.mcp.tools.include')`.
Changes:
- Create proper Laravel Boost Tool classes extending Laravel\Mcp\Server\Tool
- Register tools via boost.mcp.tools.include config array
- Each tool implements schema() and handle() methods per Laravel Boost API
Tools added:
- WinterProjectOverview: Version, environment, theme, plugin/component counts
- WinterProjectStructure: Plugins, components, controllers listing
- WinterScaffoldingCommands: Guide to Winter CMS code generation commands
- WinterViewStructure: Frontend (Twig) and backend (PHP) view mapping
- WinterDevelopmentGuide: Architecture patterns and best practices
Tested with Laravel Boost v1.8.7 and Laravel MCP v0.5.1
datune
force-pushed
the
fix/mcp-tool-registration
branch
from
4963ec9 to
9dc30b6
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
classes/Tools/WinterViewStructure.php (1)
47-51: PHPglob()does not support**recursive pattern.This issue was already flagged in a previous review. The patterns
pages/**/*.htmandpartials/**/*.htmwill not match files in subdirectories as PHP'sglob()doesn't support the**recursive glob pattern. This results in undercounting pages and partials.
🧹 Nitpick comments (3)
Plugin.php (2)
53-57: Tool registration runs regardless of debug mode.The
register()method guards service provider registration withapp.debugcheck (line 39-41), butboot()unconditionally callsregisterWinterMcpTools()andextendBoostConfiguration(). This means these methods execute even when debug mode is off, though the tools won't be loaded since the MCP service provider wasn't registered.Consider adding consistency by checking debug mode in boot() as well, or moving the tool registration into register() after the service providers:
🔎 Proposed fix
public function boot(): void { + if (!$this->app->config->get('app.debug', false)) { + return; + } + $this->registerWinterMcpTools(); $this->extendBoostConfiguration(); }
91-114: Hardcoded documentation versions may become stale.The Winter CMS documentation version
1.2and Twig version3.xare hardcoded. Consider deriving these from the installed package versions or making them configurable.classes/Tools/WinterProjectStructure.php (1)
61-75: Consider logging suppressed exceptions.The catch block silently swallows exceptions from
registerComponents(). While this prevents one broken plugin from failing the entire structure scan, consider logging these failures for debugging purposes.🔎 Optional enhancement
try { $components = $plugin->registerComponents(); + if (!is_array($components)) { + continue; + } foreach ($components as $componentClass => $componentAlias) { $structure['components'][] = [ 'plugin' => $id, 'alias' => $componentAlias, 'class' => $componentClass, ]; } } catch (\Exception $e) { - // Skip if component registration fails + // Log for debugging, continue scanning other plugins + \Log::debug("Failed to register components for {$id}: " . $e->getMessage()); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Plugin.phpclasses/Tools/WinterDevelopmentGuide.phpclasses/Tools/WinterProjectOverview.phpclasses/Tools/WinterProjectStructure.phpclasses/Tools/WinterScaffoldingCommands.phpclasses/Tools/WinterViewStructure.php
🧰 Additional context used
🧬 Code graph analysis (4)
classes/Tools/WinterProjectOverview.php (4)
classes/Tools/WinterDevelopmentGuide.php (3)
IsReadOnly(14-63)schema(22-25)handle(27-62)classes/Tools/WinterProjectStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)classes/Tools/WinterScaffoldingCommands.php (3)
IsReadOnly(14-90)schema(22-25)handle(27-89)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-104)schema(22-25)handle(27-103)
classes/Tools/WinterScaffoldingCommands.php (4)
classes/Tools/WinterDevelopmentGuide.php (3)
IsReadOnly(14-63)schema(22-25)handle(27-62)classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)classes/Tools/WinterProjectStructure.php (3)
IsReadOnly(14-100)schema(22-25)handle(27-99)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-104)schema(22-25)handle(27-103)
classes/Tools/WinterProjectStructure.php (5)
classes/Tools/WinterDevelopmentGuide.php (3)
IsReadOnly(14-63)schema(22-25)handle(27-62)classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)classes/Tools/WinterScaffoldingCommands.php (3)
IsReadOnly(14-90)schema(22-25)handle(27-89)classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-104)schema(22-25)handle(27-103)Plugin.php (1)
pluginDetails(24-32)
classes/Tools/WinterViewStructure.php (4)
classes/Tools/WinterDevelopmentGuide.php (1)
handle(27-62)classes/Tools/WinterProjectOverview.php (1)
handle(27-72)classes/Tools/WinterProjectStructure.php (1)
handle(27-99)classes/Tools/WinterScaffoldingCommands.php (1)
handle(27-89)
🪛 PHPMD (2.15.0)
classes/Tools/WinterProjectOverview.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterScaffoldingCommands.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterDevelopmentGuide.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterProjectStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterViewStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (10)
classes/Tools/WinterProjectOverview.php (3)
1-25: LGTM - Clean tool class structure.The class setup follows the Laravel MCP Tool pattern correctly with proper imports, the
#[IsReadOnly]attribute, and required method signatures. The unused$schemaparameter is required by the parent interface.
54-72: LGTM - Proper defensive checks for optional subsystems.The
class_exists()guards for Theme, PluginManager, and ComponentManager are appropriate since these subsystems may not be available in all Winter CMS configurations.
35-51:getBuildNumberManually()API verified as correct.The method name and return structure are confirmed by Winter CMS API documentation. The code correctly expects
'build'and'modified'keys in the returned array, with'confident'as an additional available key. The defensive fallback toArtisan::call('winter:version')is appropriate.Plugin.php (1)
62-80: LGTM - Config-based tool registration.The approach of merging Winter tools into
boost.mcp.tools.includeis the correct pattern for Laravel Boost v1.8.x. Theclass_existsguard ensures graceful degradation when the MCP package isn't available.classes/Tools/WinterDevelopmentGuide.php (1)
1-62: LGTM - Well-structured static documentation tool.This is a clean implementation of a read-only documentation tool. The static guide content is well-organized and provides useful Winter CMS development guidance. The unused
$schemaand$requestparameters are required by the parent interface.classes/Tools/WinterViewStructure.php (2)
55-73: LGTM - Plugin component template enumeration.The path parsing logic correctly extracts plugin, component, and template information from the glob results. The path index comments are helpful for understanding the expected structure.
75-94: LGTM - Backend controller view enumeration.The controller view scanning and partial detection (via
_prefix) is correctly implemented.classes/Tools/WinterProjectStructure.php (2)
27-31: LGTM - Early guard for Winter CMS availability.Returning an error response when PluginManager is unavailable is the correct approach for graceful degradation.
77-98: LGTM - Controller enumeration and summary.The controller file scanning correctly uses single-level glob and the summary provides useful aggregate counts.
classes/Tools/WinterScaffoldingCommands.php (1)
1-89: LGTM - Comprehensive scaffolding commands reference.This is a well-structured static documentation tool providing a complete reference for Winter CMS scaffolding commands. The command metadata (description, syntax, example, options) follows a consistent pattern and will be useful for MCP clients.
- Fix glob() calls to add ?: [] fallback for when glob returns false - Replace str_replace(base_path(), ...) with substr() to correctly handle paths containing /winter/ multiple times (e.g., /winter/plugins/winter/pages/) - Fix array index checks: use >= 6 when accessing $pathParts[5] - Add countFilesRecursive() helper method since PHP glob() doesn't support recursive ** patterns - Standardize output format across Tool classes and WinterMcpProvider
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
classes/Tools/WinterViewStructure.php (1)
106-123: ExtractcountFilesRecursive()to eliminate duplication.This method is identical to the implementation in
WinterMcpProvider(lines 492-509). Please extract it to a shared trait or helper class as suggested in the review comment forWinterMcpProvider.php.
🧹 Nitpick comments (2)
classes/WinterMcpProvider.php (1)
492-509: Suggest extractingcountFilesRecursive()to eliminate duplication.The implementation is solid, but this exact method is duplicated in
classes/Tools/WinterViewStructure.php(lines 109-123). Consider extracting it to a shared trait or helper class to follow DRY principles.🔎 Possible refactoring approach
Create a shared trait:
<?php namespace Winter\LaravelBoost\Classes\Traits; trait CountsFilesRecursively { /** * Recursively count files matching a pattern in a directory. */ protected function countFilesRecursive(string $directory, string $pattern): int { if (!is_dir($directory)) { return 0; } $count = count(glob($directory . '/' . $pattern) ?: []); $subdirs = glob($directory . '/*', GLOB_ONLYDIR) ?: []; foreach ($subdirs as $subdir) { $count += $this->countFilesRecursive($subdir, $pattern); } return $count; } }Then use it in both classes:
use Winter\LaravelBoost\Classes\Traits\CountsFilesRecursively; class WinterMcpProvider { use CountsFilesRecursively; // ... }classes/Tools/WinterViewStructure.php (1)
55-95: Consider reducing duplication with WinterMcpProvider.Lines 58-95 contain view discovery logic that closely mirrors
WinterMcpProvider::getViewStructure()(lines 424-481). While some duplication between Tool classes and providers may be acceptable, consolidating the common path parsing and view discovery logic could improve maintainability.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
classes/Tools/WinterProjectStructure.phpclasses/Tools/WinterViewStructure.phpclasses/WinterMcpProvider.php
🧰 Additional context used
🧬 Code graph analysis (2)
classes/WinterMcpProvider.php (1)
classes/Tools/WinterViewStructure.php (1)
countFilesRecursive(109-123)
classes/Tools/WinterProjectStructure.php (5)
classes/Tools/WinterViewStructure.php (3)
IsReadOnly(14-124)schema(22-25)handle(27-104)classes/Tools/WinterDevelopmentGuide.php (3)
IsReadOnly(14-63)schema(22-25)handle(27-62)classes/Tools/WinterScaffoldingCommands.php (3)
IsReadOnly(14-90)schema(22-25)handle(27-89)classes/Tools/WinterProjectOverview.php (3)
IsReadOnly(14-73)schema(22-25)handle(27-72)Plugin.php (1)
pluginDetails(24-32)
🪛 PHPMD (2.15.0)
classes/Tools/WinterViewStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
classes/Tools/WinterProjectStructure.php
22-22: Avoid unused parameters such as '$schema'. (undefined)
(UnusedFormalParameter)
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (10)
classes/WinterMcpProvider.php (5)
412-421: Excellent improvements to theme view counting.The changes properly address the PHP
glob()limitation with recursive patterns by introducingcountFilesRecursive()helper and adding the?: []fallback for safer handling of empty results.
424-442: Good improvements to path parsing and glob safety.The switch from
str_replace(base_path(), '', $file)tosubstr($viewFile, strlen($basePath))correctly prevents issues with paths containing repeated segments. The path format comments are helpful for maintainability.
445-460: LGTM! Consistent path handling for plugin partials.The implementation follows the same improved pattern as plugin component views, with appropriate array index checks for the path structure.
463-481: Good addition ofis_partialflag.The
is_partialflag provides useful metadata about view types based on naming conventions, and the path handling improvements are consistent with other view mappings.
483-487: Excellent documentation addition.The conventions section clearly documents Winter CMS view system patterns, making the data structure more self-documenting.
classes/Tools/WinterProjectStructure.php (2)
1-25: LGTM! Clean Tool implementation.The class properly extends
Toolwith the#[IsReadOnly]annotation. The unused$schemaparameter warning from PHPMD is a false positive—it's required by the interface contract.
27-99: Well-structured plugin discovery implementation.The method properly handles Winter CMS availability checks, includes defensive error handling for component registration failures, and consistently applies the
?: []fallback pattern forglob()results. The summary counts provide useful metadata.classes/Tools/WinterViewStructure.php (3)
1-25: LGTM! Consistent Tool pattern.The class structure is consistent with other Tool implementations in the PR. The unused parameter warnings are false positives from the interface requirements.
41-53: Previous issue resolved withcountFilesRecursive().The recursive file counting implementation properly addresses the previous review concern about PHP's
glob()not supporting**patterns. The helper method correctly traverses subdirectories.
97-103: LGTM! Clear conventions documentation.The conventions section provides helpful guidance on Winter CMS view patterns, consistent with the implementation in
WinterMcpProvider.
|
@datune there's a few nitpick comments from the AI that are valid, especially removing duplicated methods from the service provider if they're actually being provided by the tool classes instead now. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
console/TestMcpTools.php (1)
54-70: Verify Request construction with tool name.The Request is constructed with
'name' => $name, where$nameis the short alias ('overview', 'structure', etc.). However, according to the PR objectives, the actual MCP tool names are 'winter-project-overview', 'winter-project-structure', etc.If the Tool classes expect the full tool name in the Request, this will cause a mismatch. Verify whether:
- The tools actually use the 'name' parameter from the Request
- If they do, whether it should be the full name (e.g., 'winter-project-overview') or the short alias
Additionally, the response parsing assumes specific structure without error handling:
🔎 Suggested defensive improvements
protected function testTool(string $name, string $toolClass): void { $this->info("=== Testing {$name} tool ==="); try { + if (!class_exists($toolClass)) { + $this->error("Tool class {$toolClass} does not exist"); + return; + } + $tool = new $toolClass(); $request = new Request('tools/call', ['name' => $name, 'arguments' => []]); $response = $tool->handle($request); // Extract the JSON content from the response - $content = $response->toArray()['content'] ?? []; + $responseArray = $response->toArray(); + if (!isset($responseArray['content']) || !is_array($responseArray['content'])) { + $this->warn("Unexpected response structure"); + return; + } + + $content = $responseArray['content']; foreach ($content as $item) { if (isset($item['text'])) { $decoded = json_decode($item['text'], true); - $this->line(json_encode($decoded, JSON_PRETTY_PRINT)); + if ($decoded !== null) { + $this->line(json_encode($decoded, JSON_PRETTY_PRINT)); + } else { + $this->line($item['text']); + } } } } catch (\Exception $e) { $this->error("Error: " . $e->getMessage()); } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdclasses/WinterMcpProvider.phpconsole/TestMcpTools.php
💤 Files with no reviewable changes (1)
- classes/WinterMcpProvider.php
🔇 Additional comments (4)
console/TestMcpTools.php (2)
6-11: LGTM: Imports align with class-based architecture.The addition of
Requestand the five Tool classes is correct and supports the refactored class-based tool invocation model.
32-38: LGTM: Clean tool mapping.The mapping of short names to Tool classes provides a convenient testing interface.
README.md (2)
11-15: LGTM: Clear and accurate tool documentation.The tool descriptions are well-written and accurately reflect the new MCP tools. The tool names correctly use the 'winter-' prefix, which aligns with the PR objectives.
75-75: LGTM: Documentation reflects architectural change.The update correctly documents the shift from a single
WinterMcpProvider.phpto multiple Tool classes in theclasses/Tools/directory, matching the refactored architecture.
datune
force-pushed
the
fix/mcp-tool-registration
branch
from
3fc4879 to
9126105
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @console/TestMcpTools.php:
- Around line 113-115: The current cast (string) $response in the
$this->line(...) call can fatal if Response isn't stringable; instead, before
calling $this->line use a defensive check: if $response is a string, an instance
of Stringable, or implements __toString use that string; else if it has a
toArray() method call that and convert to JSON; otherwise convert with
json_encode or var_export as a final fallback; update the code around the
$this->line((string) $response) use of $response to perform these checks and
produce a safe string for $this->line.
- Around line 80-92: createMockRequest currently falls back to
ReflectionClass::newInstanceWithoutConstructor which yields an uninitialized
Request that may break handle(); instead, attempt to resolve or properly
initialize a Request: first try using the container
(container()->make(Request::class, ['path' => 'tools/call', 'parameters' => []])
or app()->make(...)) to honor framework wiring, then try
ReflectionClass->newInstanceArgs with common constructor signatures, and only as
a last resort create an instance and explicitly set the minimal
properties/methods used by handle() (e.g., setMethod('GET'|'POST'), set request
URI/path and parameters) so the Request object is usable; update
createMockRequest to perform these steps and remove/newly-document the unsafe
newInstanceWithoutConstructor fallback.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdclasses/WinterMcpProvider.phpconsole/TestMcpTools.php
💤 Files with no reviewable changes (1)
- classes/WinterMcpProvider.php
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
🔇 Additional comments (3)
console/TestMcpTools.php (3)
31-34: Good defensive check for required dependency.The early exit with a helpful installation hint improves developer experience.
38-59: Clean tool mapping and execution flow.The refactor from provider methods to class-based tools is well-structured. The explicit
return 0properly signals success to the shell.
67-71: No action needed. Direct instantiation of tool classes is safe—all five Winter tool classes (WinterDevelopmentGuide,WinterProjectOverview,WinterProjectStructure,WinterScaffoldingCommands,WinterViewStructure) use default parameterless constructors and have no dependency injection requirements. Additionally, the method wraps instantiation in exception handling, providing a safety net for any unforeseen issues.Likely an incorrect or invalid review comment.
| protected function createMockRequest(): Request | ||
| { | ||
| // Create request via reflection since constructor may vary by version | ||
| $reflection = new \ReflectionClass(Request::class); | ||
|
|
||
| // Try common constructor patterns | ||
| try { | ||
| return new Request('tools/call', []); | ||
| } catch (\Throwable $e) { | ||
| // Fallback: create without constructor | ||
| return $reflection->newInstanceWithoutConstructor(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fallback newInstanceWithoutConstructor() may produce unusable Request.
Creating an object without invoking its constructor leaves properties uninitialized. If handle() accesses any property set by the constructor (e.g., method name, parameters), this will cause undefined behavior or errors.
Consider using Laravel's container if available, or documenting that this fallback path is untested:
🔎 Suggested improvement
protected function createMockRequest(): Request
{
- // Create request via reflection since constructor may vary by version
- $reflection = new \ReflectionClass(Request::class);
-
- // Try common constructor patterns
try {
return new Request('tools/call', []);
} catch (\Throwable $e) {
- // Fallback: create without constructor
- return $reflection->newInstanceWithoutConstructor();
+ // Fallback: try container resolution
+ if (app()->bound(Request::class)) {
+ return app(Request::class);
+ }
+ // Last resort - may not work correctly
+ $this->warn('Could not create proper Request instance, using uninitialized fallback');
+ return (new \ReflectionClass(Request::class))->newInstanceWithoutConstructor();
}
}🤖 Prompt for AI Agents
In @console/TestMcpTools.php around lines 80-92, createMockRequest currently
falls back to ReflectionClass::newInstanceWithoutConstructor which yields an
uninitialized Request that may break handle(); instead, attempt to resolve or
properly initialize a Request: first try using the container
(container()->make(Request::class, ['path' => 'tools/call', 'parameters' => []])
or app()->make(...)) to honor framework wiring, then try
ReflectionClass->newInstanceArgs with common constructor signatures, and only as
a last resort create an instance and explicitly set the minimal
properties/methods used by handle() (e.g., setMethod('GET'|'POST'), set request
URI/path and parameters) so the Request object is usable; update
createMockRequest to perform these steps and remove/newly-document the unsafe
newInstanceWithoutConstructor fallback.
- Delete classes/WinterMcpProvider.php as its methods are now duplicated in the Tool classes (WinterProjectOverview, etc.) - Update console/TestMcpTools.php to use the new Tool classes directly - Update README.md to reflect new Tool-based architecture Addresses CodeRabbit review feedback from LukeTowers about removing duplicate methods from the service provider. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
datune
force-pushed
the
fix/mcp-tool-registration
branch
from
9126105 to
8b2f65b
Compare
|
Thanks @LukeTowers for the feedback! I've addressed the review comments:
Tested all 5 tools locally and they're working correctly. |
2 participants
Problem
The current implementation attempts to register Winter CMS MCP tools by hooking into
app('mcp.server'), but this binding doesn't exist in Laravel Boost v1.8.x. Laravel Boost usesconfig('boost.mcp.tools.include')for registering additional tool classes.As a result, the Winter CMS tools (plugin list, theme info, etc.) don't appear in the MCP tools list.
Solution
This PR:
Laravel\Mcp\Server\Toolschema()andhandle()methods per the Laravel Boost APIboost.mcp.tools.includeconfig arrayTools Implemented
winter-project-overviewwinter-project-structurewinter-scaffolding-commandswinter-view-structurewinter-development-guideTesting
Tested with:
All 5 tools now appear in the MCP tools list and return correct data when called.
Before/After
Before: 16 tools (Laravel Boost only)
After: 21 tools (Laravel Boost + 5 Winter CMS tools)
Summary by CodeRabbit
New Features
Behavior Changes
Bug Fixes / Reliability
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.