fix(mcp): delete the McpContext copy nothing here can load - #24
Conversation
php/Mcp/Transport/McpContext.php and its McpToolHandler contract declared
Core\Front\Mcp\McpContext and Core\Front\Mcp\Contracts\McpToolHandler — a
namespace this package maps nowhere. composer.json declares Core\Mcp\,
Core\Mod\Agentic\ and Core\Service\Agentic\, and nothing else, so
class_exists('Core\Front\Mcp\McpContext') is false here and always has been.
Their tests only ran because they hand-loaded the files:
mcpRequire('Mcp/Transport/McpContext.php');
which is the third manual-require-around-broken-autoload found in this repo
today, after loadAgenticPhpClass() and LivewireTestCase::livewireComponent().
The pattern is reliable evidence that a file cannot be reached the normal way.
dappcore/mcp declares the same two FQCNs and, unlike this package, actually
maps them — so in any consumer application mcp's copies are what load. Two
packages owning one class name is settled by autoload order, silently, and this
side lost twice over: shadowed where both were installed, unloadable where only
this one was.
The reference check found nothing else touching them. The fifty-odd other
toMcpContext hits in this repo are a model method of the same name on
AgentPlan, BrainMemory and friends — unrelated, and worth stating because a
careless grep would have read them as callers.
Agent's copy carried getScopes() and hasScope() that mcp's lacked. Those, and
every test in both files, moved to dAppCore/mcp#22 first, so the behaviour
exists in one place before it stops existing here. That PR also fixes a
property_exists() visibility bug in the scope helpers, which is why four of the
eight McpContextTest cases removed here were failing rather than passing.
Suite 127 failed / 1190 passed, from 131 / 1197 on a freshly measured main —
eleven tests removed, of which four were failing, and no other change.
Co-Authored-By: Virgil <virgil@lethean.io>
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Pairs with dAppCore/mcp#22, which takes the methods and every test first. Behaviour exists in one place before it stops existing here.
The reference check
php/Mcp/Transport/McpContext.phpand itsMcpToolHandlercontract declareCore\Front\Mcp\McpContextandCore\Front\Mcp\Contracts\McpToolHandler— a namespace this package maps nowhere.composer.jsondeclaresCore\Mcp\,Core\Mod\Agentic\andCore\Service\Agentic\, and nothing else:Their tests only ran because they hand-loaded the files:
That is the third manual-require-around-broken-autoload found in this repo today, after
loadAgenticPhpClass()andLivewireTestCase::livewireComponent(). The pattern is reliable evidence that a file cannot be reached the normal way.dappcore/mcpdeclares the same two FQCNs and, unlike this package, actually maps them — so in any consumer application mcp's copies are what load. Two packages owning one class name is settled by autoload order, silently, and this side lost twice over: shadowed where both were installed, unloadable where only this one was.Nothing else touches them. Worth stating: the fifty-odd other
toMcpContexthits in this repo are a model method of the same name onAgentPlan,BrainMemoryand friends — unrelated, and a careless grep would have read them as callers.What moved before this deletes
Agent's copy carried
getScopes()andhasScope()that mcp's lacked. Those and every test in both files went to mcp#22 — 12 tests, where that package previously had none for this class.That PR also fixes a
property_exists()visibility bug in the scope helpers, which is why four of the eightMcpContextTestcases removed here were failing rather than passing.Verification
Baseline taken fresh rather than reused — an earlier stale-baseline comparison in this sequence produced a wrong claim I had to correct, so the number here is from a run of
maintoday.🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io