feat(mcp): serve the agent resources surface that only dappcore/mcp had - #20
Merged
Conversation
The agent MCP server answered resources/list with a hardcoded
['resources' => []] and implemented no resources/read at all. The real
surface — plans, phase checklists, plan state, session context — lived only
in dappcore/mcp's McpAgentServerCommand, which is also where the dependency
cycle came from: a shared package importing Core\Mod\Agentic\Models\*.
This ports the capability across so mcp's copy can be deleted afterwards
rather than before. Nothing is missing from both at once.
Five URI shapes, one class each, matching how Mcp\Tools\Agent already splits
tools out rather than carrying them as methods on a 2000-line command:
plans://all AllPlansResource
plans://{slug} PlanDocumentResource
plans://{slug}/phases/{order} PhaseChecklistResource
plans://{slug}/state/{key} StateValueResource
sessions://{id}/context SessionContextResource
AgentResourceRegistry lists and routes them, the counterpart to
AgentToolRegistry. Resources contribute their own list entries, so the plan
document resource enumerates every non-archived plan while the unbounded
per-key ones advertise nothing and stay directly addressable.
Two deliberate departures from the code being ported.
A missing target now answers a JSON-RPC -32602 instead of returning the
string "Plan not found: {slug}" as the resource body — a client could not
distinguish that from a real document whose text happens to say so.
And initialize advertises the resources capability. Implementing the surface
without declaring it would leave it undiscoverable: a client that is not told
the server has resources never issues resources/list, so it would have been
shipped and never called, which is the failure this whole sweep exists to
remove.
Registered as a container singleton in register(), not hung off an event like
onMcpTools. There is no McpResourcesRegistering to listen for, and $listens
is populated by ModuleScanner from app/Core|Mod|Website only — dead once this
package sits under vendor/ — so a binding is what actually resolves in a host
application.
Ten new tests: six over the registry (routing, declining near-miss shapes,
reading, dynamic listing, and returning null rather than a document for a
missing target) and four driving the real stdio command end to end for the
initialize advertisement, resources/list, resources/read and its error path.
Suite: 156 failed, 1165 passed, from 156 failed, 1155 passed — the same
failures, plus exactly these ten.
Co-Authored-By: Virgil <virgil@lethean.io>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughChangesAgent resource support
Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant McpAgentServerCommand
participant AgentResourceRegistry
participant AgentResource
MCPClient->>McpAgentServerCommand: resources/list or resources/read
McpAgentServerCommand->>AgentResourceRegistry: entries() or read(uri)
AgentResourceRegistry->>AgentResource: matches(uri) and read(uri)
AgentResource-->>AgentResourceRegistry: metadata or Markdown content
AgentResourceRegistry-->>McpAgentServerCommand: resource response data
McpAgentServerCommand-->>MCPClient: JSON-RPC result or -32602 error
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! |
Snider
added a commit
to dAppCore/mcp
that referenced
this pull request
Aug 8, 2026
…dy owns (#19) This package imported Core\Mod\Agentic\Models\* in four files while dappcore/agent is meant to depend on it — a library importing the thing that imports it. AX-8: the arrow points one way. Three files go because agent already owns them, verified rather than assumed: AgentToolRegistry agent's has all 13 shared methods plus 6 more, none only here. Unreferenced in this package — even McpApiController reaches for agent's FQCN, not this. AgentSessionService agent's has all 20 shared plus replay and getReplayContext, none only here. Zero references. McpAgentServerCommand 2064 lines. Its 29 tool methods each map to an extracted class in agent's Mcp\Tools\Agent, and agent's Boot::onMcpTools registers every one — checked for registration, not just for the files existing. Its five resource handlers were the one thing agent did NOT have: agent's server answered resources/list with a hardcoded empty array and had no resources/read. Deleting this first would have silently dropped plans://all, plans://{slug}, plans://{slug}/phases/{order}, plans://{slug}/state/{key} and sessions://{id}/context. They were ported to agent first, in dAppCore/agent#20, and only then is this removed. McpApiController carried a third copy of the same rendering, and that is what made the cycle a compile-time one. The plan and session renderers are replaced by AgentResourceProvider, an interface this package owns and the module that owns the data implements, resolved from the container and absent without complaint when agent is not installed. Both model imports go with them. Interim worth stating plainly: nothing is bound to AgentResourceProvider yet, so this package's HTTP plans:// and sessions:// endpoints answer "not found" until agent binds its registry to it. Agent implementing the interface needs agent to depend on dappcore/mcp, which is still blocked on the Core\Mcp\ PSR-4 root being claimed by both packages. The data itself is not lost — agent's own MCP server serves all five URIs as of #20; it is the HTTP mirror here that is degraded until that lands. Two soft couplings remain by design, both container lookups by string with no import and a guard: AgentToolRegistry in executeTool and AgentApiKeyService in McpAuthenticate. Neither is a compile-time dependency and neither creates a composer cycle; both would be better behind interfaces, and are left for when the namespace collision is resolved. Suite unchanged: 21 failed, 299 passed. 2786 deletions, 102 insertions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR 1 of 2 in the dependency-cycle break. This ports the capability into agent; mcp's copy is deleted in PR 2, after this merges — so nothing is missing from both at once.
The gap
The agent MCP server answered
resources/listwith a hardcoded['resources' => []]and implemented noresources/readat all. The real surface lived only indappcore/mcp'sMcpAgentServerCommand— which is also where the cycle came from: a shared package importingCore\Mod\Agentic\Models\*.The 46-method diff that gated this found the 29
tool*methods fully covered by agent's extracted tool classes and reachable (Boot::onMcpToolsregisters all of them). The 5resource*methods were covered by nothing. A straight delete would have lost them silently.What lands
Five URI shapes, one class each — matching how
Mcp\Tools\Agentalready splits tools out rather than carrying them as methods on a 2000-line command:AgentResourceRegistrylists and routes them — the counterpart toAgentToolRegistry. Resources contribute their own list entries, so the plan document resource enumerates every non-archived plan, while the unbounded per-key ones (phase checklists, state values) advertise nothing and stay directly addressable.Two deliberate departures from the ported code
A missing target answers
-32602, not the string"Plan not found: {slug}"returned as the resource body. A client could not distinguish that from a real document whose text happens to say so.initializeadvertises the resources capability. Implementing the surface without declaring it leaves it undiscoverable — a client that isn't told the server has resources never issuesresources/list. It would have been shipped and never called, which is the exact failure this sweep exists to remove.Registration
A container singleton in
register(), not hung off an event the wayonMcpToolsis. There's noMcpResourcesRegisteringto listen for, and$listensis populated by ModuleScanner fromapp/Core|Mod|Websiteonly — dead once this package sits undervendor/— so a binding is what actually resolves in a host application.Verification
Same failures as
main, plus exactly these ten. The one failing test whose name contains "Resource" isContentResource, a pre-existing failure in a file this branch does not touch.🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
New Features
Tests