feat: native PHP MCP server via laravel/mcp#134
Conversation
Replaces the fragile knowledge-server.js (shelled out to CLI) with a native laravel/mcp integration using stdio transport. 5 intent-based tools: - recall: semantic search via TieredSearchService - remember: capture knowledge with write gate, dedup, auto git context - correct: supersession-based correction propagation - context: load project entries grouped by category, ranked by usage - stats: entry counts across all project namespaces Key decisions: - Custom McpServiceProvider for Laravel Zero (no illuminate/routing) - Manual request validation (no illuminate/validation in Zero) - Mcp::local() stdio transport for CLI and Desktop integration - Context tool shows available_projects on empty results Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a new MCP KnowledgeServer and five MCP tools (Recall, Remember, Correct, Context, Stats), integrates a McpServiceProvider and route, registers laravel/mcp, and includes unit tests for each tool. Tools integrate with Qdrant, project detection, metadata, correction, write-gate, and enhancement services. Changes
Sequence DiagramssequenceDiagram
participant Client
participant RememberTool
participant ProjectDetector
participant GitContext
participant WriteGate
participant Qdrant
participant EnhancementQueue
Client->>RememberTool: handle(title, content, tags, category, ...)
RememberTool->>RememberTool: validate inputs
RememberTool->>ProjectDetector: detect project
ProjectDetector-->>RememberTool: project
RememberTool->>GitContext: fetch git metadata
GitContext-->>RememberTool: git metadata
RememberTool->>WriteGate: evaluate write permission
WriteGate-->>RememberTool: allow/deny
alt WriteGate Denied
RememberTool-->>Client: error response
else WriteGate Allowed
RememberTool->>Qdrant: upsert entry (duplicate detection)
alt Duplicate Detected
Qdrant-->>RememberTool: duplicate exception / existing id
RememberTool-->>Client: duplicate response
else Entry Stored
Qdrant-->>RememberTool: success
RememberTool->>EnhancementQueue: enqueue for enhancements
RememberTool-->>Client: success response
end
end
sequenceDiagram
participant Client
participant RecallTool
participant ProjectDetector
participant TieredSearch
participant Qdrant
participant EntryMetadata
Client->>RecallTool: handle(query, project?, category?, tag?, limit, global?)
RecallTool->>RecallTool: validate query
RecallTool->>ProjectDetector: detect project (if needed)
ProjectDetector-->>RecallTool: project
alt Global Search
RecallTool->>Qdrant: list/search all knowledge_* collections
Qdrant-->>RecallTool: per-collection results
RecallTool->>EntryMetadata: augment results
else Per-Project Search
RecallTool->>TieredSearch: perform tiered search with filters
TieredSearch-->>RecallTool: ranked results
RecallTool->>EntryMetadata: augment results
end
RecallTool-->>Client: formatted results + meta
sequenceDiagram
participant Client
participant ContextTool
participant ProjectDetector
participant Qdrant
participant EntryMetadata
Client->>ContextTool: handle(project?, categories?, max_tokens?)
ContextTool->>ProjectDetector: detect project (if needed)
ProjectDetector-->>ContextTool: project
ContextTool->>Qdrant: fetch entries (by category or global, limit)
Qdrant-->>ContextTool: entries
alt No entries found
ContextTool->>Qdrant: list collections -> available projects
ContextTool-->>Client: empty entries + available_projects
else Entries found
ContextTool->>ContextTool: rank entries (usage + freshness)
ContextTool->>ContextTool: group by category and truncate to token budget
ContextTool->>EntryMetadata: format entries
ContextTool-->>Client: categorized entries + counts
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
📊 Coverage Report
Files Below Threshold
🏆 Synapse Sentinel Gate |
🔧 Synapse Sentinel: 1 check need attentionThe following issues must be resolved before this PR can be merged: All tests passed.--- Quick Reference:
🤖 Generated by Synapse Sentinel - View Run |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/Mcp/Tools/ContextTool.php`:
- Around line 47-50: The $categories, $maxTokens and $limit inputs must be
validated and normalized before any scroll() calls: ensure $categories is either
null or an array of non-empty strings (e.g., if
is_array($request->get('categories')) then filter/validate elements and set to
null when empty or any non-string entries removed) and use that sanitized
$categories when calling scroll(); ensure $maxTokens and $limit are numeric,
coerce/cast to int, enforce minimums of 1 and then apply the existing maxima
(min(...,16000) for $maxTokens and min(...,100) for $limit), and replace the
current is_int checks with this normalization so non-positive or non-numeric
values default to safe minimums; apply the same validation/normalization logic
wherever $categories/$maxTokens/$limit are set around the scroll() usage (the
block that populates and passes these to scroll()).
In `@app/Mcp/Tools/CorrectTool.php`:
- Around line 26-28: The ID validation in CorrectTool.php currently disallows
non-string IDs but CorrectionService::correct() accepts string|int; update the
check in the method that returns "Provide the ID..." to allow integers and
numeric strings (e.g., accept is_int($id) or (is_string($id) && $id !== '' ) or
use is_numeric for numeric strings), then ensure you pass a properly typed value
to CorrectionService::correct() (cast to int when numeric or to string
otherwise) so numeric entry IDs are accepted; reference the conditional that
currently calls Response::error and the call to CorrectionService::correct() to
locate where to change the validation and any necessary casting.
In `@app/Mcp/Tools/RecallTool.php`:
- Line 41: The current $limit assignment in RecallTool (the line setting $limit
from $request->get('limit')) allows 0 or negative values; change the logic to
coerce the request value to an integer, clamp it to the range 1..20, and fall
back to 5 when no valid value is provided—i.e., ensure $limit is an int, apply a
minimum of 1 and maximum of 20 before it is used in tiered search or aggregation
flows so no non-positive limits propagate.
In `@app/Mcp/Tools/RememberTool.php`:
- Line 56: Clamp the incoming confidence value to the 0–100 range before
storing: when building the array in RememberTool (the line that sets the
'confidence' key based on $request->get('confidence')), coerce to int then apply
min(max($value, 0), 100) so out-of-range values become 0 or 100; also apply the
same clamping where the schema/description is produced (the code around the
lines that describe the confidence field) to ensure stored and documented values
are consistent.
- Around line 46-60: The $entry payload contains nullable/mixed values that
break strict contracts consumed by upsert() and queue(); normalize every field
to expected non-null types before those calls: ensure 'tags' is always an array
of strings (cast/filter values), 'category' and 'evidence' are non-null strings
(use '' as default), 'priority' is a string with default 'medium', 'confidence'
is an int with default 50, and 'last_verified' is a string timestamp; update the
construction of $entry (the array built in RememberTool) so each key is
explicitly coerced/validated to the correct type before calling upsert() and
queue().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b75c95c0-e02f-43a5-a40a-de1d26ee21f1
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
app/Mcp/Servers/KnowledgeServer.phpapp/Mcp/Tools/ContextTool.phpapp/Mcp/Tools/CorrectTool.phpapp/Mcp/Tools/RecallTool.phpapp/Mcp/Tools/RememberTool.phpapp/Mcp/Tools/StatsTool.phpapp/Providers/McpServiceProvider.phpbootstrap/providers.phpcomposer.jsonroutes/ai.php
| $categories = is_array($request->get('categories')) ? $request->get('categories') : null; | ||
| $maxTokens = is_int($request->get('max_tokens')) ? min($request->get('max_tokens'), 16000) : 4000; | ||
| $limit = is_int($request->get('limit')) ? min($request->get('limit'), 100) : 50; | ||
|
|
There was a problem hiding this comment.
Validate categories and enforce positive bounds before scroll() calls.
Line 47 currently permits mixed arrays; those values are forwarded as category filters on Line 111. Also, Line 48/49 allow non-positive max_tokens/limit.
Suggested fix
- /** `@var` array<string>|null $categories */
- $categories = is_array($request->get('categories')) ? $request->get('categories') : null;
- $maxTokens = is_int($request->get('max_tokens')) ? min($request->get('max_tokens'), 16000) : 4000;
- $limit = is_int($request->get('limit')) ? min($request->get('limit'), 100) : 50;
+ $rawCategories = $request->get('categories');
+ $categories = is_array($rawCategories)
+ ? array_values(array_filter(
+ $rawCategories,
+ static fn (mixed $category): bool => is_string($category) && $category !== ''
+ ))
+ : null;
+ $maxTokens = is_int($request->get('max_tokens'))
+ ? max(1, min($request->get('max_tokens'), 16000))
+ : 4000;
+ $limit = is_int($request->get('limit'))
+ ? max(1, min($request->get('limit'), 100))
+ : 50;Also applies to: 105-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/ContextTool.php` around lines 47 - 50, The $categories,
$maxTokens and $limit inputs must be validated and normalized before any
scroll() calls: ensure $categories is either null or an array of non-empty
strings (e.g., if is_array($request->get('categories')) then filter/validate
elements and set to null when empty or any non-string entries removed) and use
that sanitized $categories when calling scroll(); ensure $maxTokens and $limit
are numeric, coerce/cast to int, enforce minimums of 1 and then apply the
existing maxima (min(...,16000) for $maxTokens and min(...,100) for $limit), and
replace the current is_int checks with this normalization so non-positive or
non-numeric values default to safe minimums; apply the same
validation/normalization logic wherever $categories/$maxTokens/$limit are set
around the scroll() usage (the block that populates and passes these to
scroll()).
| if (! is_string($id) || $id === '') { | ||
| return Response::error('Provide the ID of the entry to correct.'); | ||
| } |
There was a problem hiding this comment.
Allow numeric IDs for correction requests.
Line 26 currently rejects non-string IDs, but CorrectionService::correct() accepts string|int. This blocks correction when entries use numeric IDs.
Suggested fix
- if (! is_string($id) || $id === '') {
+ if ((! is_string($id) && ! is_int($id)) || (is_string($id) && $id === '')) {
return Response::error('Provide the ID of the entry to correct.');
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/CorrectTool.php` around lines 26 - 28, The ID validation in
CorrectTool.php currently disallows non-string IDs but
CorrectionService::correct() accepts string|int; update the check in the method
that returns "Provide the ID..." to allow integers and numeric strings (e.g.,
accept is_int($id) or (is_string($id) && $id !== '' ) or use is_numeric for
numeric strings), then ensure you pass a properly typed value to
CorrectionService::correct() (cast to int when numeric or to string otherwise)
so numeric entry IDs are accepted; reference the conditional that currently
calls Response::error and the call to CorrectionService::correct() to locate
where to change the validation and any necessary casting.
| } | ||
|
|
||
| $project = is_string($request->get('project')) ? $request->get('project') : $this->projectDetector->detect(); | ||
| $limit = is_int($request->get('limit')) ? min($request->get('limit'), 20) : 5; |
There was a problem hiding this comment.
Clamp limit to a positive range before querying.
Line 41 allows 0/negative values through. That can propagate invalid limits to tiered search and global aggregation flow.
Suggested fix
- $limit = is_int($request->get('limit')) ? min($request->get('limit'), 20) : 5;
+ $limit = is_int($request->get('limit'))
+ ? max(1, min($request->get('limit'), 20))
+ : 5;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $limit = is_int($request->get('limit')) ? min($request->get('limit'), 20) : 5; | |
| $limit = is_int($request->get('limit')) | |
| ? max(1, min($request->get('limit'), 20)) | |
| : 5; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/RecallTool.php` at line 41, The current $limit assignment in
RecallTool (the line setting $limit from $request->get('limit')) allows 0 or
negative values; change the logic to coerce the request value to an integer,
clamp it to the range 1..20, and fall back to 5 when no valid value is
provided—i.e., ensure $limit is an int, apply a minimum of 1 and maximum of 20
before it is used in tiered search or aggregation flows so no non-positive
limits propagate.
| /** @var array<string>|null $tags */ | ||
| $tags = is_array($request->get('tags')) ? $request->get('tags') : []; | ||
|
|
||
| $entry = [ | ||
| 'id' => Str::uuid()->toString(), | ||
| 'title' => $title, | ||
| 'content' => $content, | ||
| 'category' => is_string($request->get('category')) ? $request->get('category') : null, | ||
| 'tags' => $tags, | ||
| 'priority' => is_string($request->get('priority')) ? $request->get('priority') : 'medium', | ||
| 'confidence' => is_int($request->get('confidence')) ? $request->get('confidence') : 50, | ||
| 'status' => 'draft', | ||
| 'evidence' => is_string($request->get('evidence')) ? $request->get('evidence') : null, | ||
| 'last_verified' => now()->toIso8601String(), | ||
| ]; |
There was a problem hiding this comment.
Normalize entry shape before upsert()/queue() to satisfy strict contracts.
Line 53/54 and Line 58 can produce nullable/mixed payload values that conflict with the required parameter shapes used at Line 79 and Line 91. PHPStan already flags this.
Suggested fix
- /** `@var` array<string>|null $tags */
- $tags = is_array($request->get('tags')) ? $request->get('tags') : [];
+ $rawTags = $request->get('tags');
+ $tags = is_array($rawTags)
+ ? array_values(array_filter(
+ $rawTags,
+ static fn (mixed $tag): bool => is_string($tag) && $tag !== ''
+ ))
+ : [];
+
+ $rawCategory = $request->get('category');
+ $category = is_string($rawCategory) && $rawCategory !== '' ? $rawCategory : null;
+ $rawEvidence = $request->get('evidence');
- $entry = [
+ /** `@var` array{id: string, title: string, content: string, tags: array<string>, priority: string, confidence: int, status: string, last_verified: string, category?: string, evidence?: string} $entry */
+ $entry = [
'id' => Str::uuid()->toString(),
'title' => $title,
'content' => $content,
- 'category' => is_string($request->get('category')) ? $request->get('category') : null,
'tags' => $tags,
'priority' => is_string($request->get('priority')) ? $request->get('priority') : 'medium',
'confidence' => is_int($request->get('confidence')) ? $request->get('confidence') : 50,
'status' => 'draft',
- 'evidence' => is_string($request->get('evidence')) ? $request->get('evidence') : null,
'last_verified' => now()->toIso8601String(),
];
+
+ if ($category !== null) {
+ $entry['category'] = $category;
+ }
+
+ if (is_string($rawEvidence) && $rawEvidence !== '') {
+ $entry['evidence'] = $rawEvidence;
+ }As per coding guidelines: "Adhere to PHPStan level 8 with strict rules for static analysis".
Also applies to: 79-91
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/RememberTool.php` around lines 46 - 60, The $entry payload
contains nullable/mixed values that break strict contracts consumed by upsert()
and queue(); normalize every field to expected non-null types before those
calls: ensure 'tags' is always an array of strings (cast/filter values),
'category' and 'evidence' are non-null strings (use '' as default), 'priority'
is a string with default 'medium', 'confidence' is an int with default 50, and
'last_verified' is a string timestamp; update the construction of $entry (the
array built in RememberTool) so each key is explicitly coerced/validated to the
correct type before calling upsert() and queue().
| 'category' => is_string($request->get('category')) ? $request->get('category') : null, | ||
| 'tags' => $tags, | ||
| 'priority' => is_string($request->get('priority')) ? $request->get('priority') : 'medium', | ||
| 'confidence' => is_int($request->get('confidence')) ? $request->get('confidence') : 50, |
There was a problem hiding this comment.
Clamp confidence to the documented 0–100 range.
Line 56 accepts any integer, but the schema description on Line 123 advertises 0–100. Clamp before storing to keep data consistent.
Suggested fix
- 'confidence' => is_int($request->get('confidence')) ? $request->get('confidence') : 50,
+ 'confidence' => is_int($request->get('confidence'))
+ ? max(0, min($request->get('confidence'), 100))
+ : 50,Also applies to: 122-124
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/Mcp/Tools/RememberTool.php` at line 56, Clamp the incoming confidence
value to the 0–100 range before storing: when building the array in RememberTool
(the line that sets the 'confidence' key based on $request->get('confidence')),
coerce to int then apply min(max($value, 0), 100) so out-of-range values become
0 or 100; also apply the same clamping where the schema/description is produced
(the code around the lines that describe the confidence field) to ensure stored
and documented values are consistent.
26 tests covering RecallTool, RememberTool, CorrectTool, ContextTool, and StatsTool. Tests validate input validation, error handling, project detection, and response formatting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🏆 Sentinel Certified✅ Tests & Coverage: 0 tests passed Add this badge to your README: [](https://github.com/conduit-ui/knowledge/actions/workflows/gate.yml) |
Summary
knowledge-server.jswrapper with nativelaravel/mcpv0.6.0 integrationrecall,remember,correct,context,statsMcpServiceProviderfor Laravel Zero compatibility (no routing/validation deps)Mcp::local()for Claude Code and Claude DesktopBugs fixed from initial testing
RememberToolreferenced undefined$validated['evidence']— now uses$request->get('evidence')patterns,decisions,gotchas)ContextToolempty results now showavailable_projectslist to help users find the right namespaceClaude Desktop config
{ "knowledge": { "command": "php", "args": ["/path/to/knowledge/know", "mcp:start", "knowledge"] } }Test plan
tools/listreturns all 5 tools with schemasrecallreturns ranked results with confidence/freshnessstatsreturns entry counts across all projectsremembercaptures entries with git context and dedup🤖 Generated with Claude Code
Summary by CodeRabbit