feat: implement getToolContext for plugins to enrich core tool responses - #387
feat: implement getToolContext for plugins to enrich core tool responses#387pikann wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Important
The feature's activation contract is misdocumented: a manifest mcp.toolContextHooks declaration is required for a hook to ever fire, yet it's absent from the manifest example and one note claims the host calls every plugin that implements getToolContext — the opposite of the shipped behavior. Plugin authors following the docs end-to-end will ship hooks that silently never run.
Reviewed changes
PluginMCPEntry.getToolContextplus a manifest-declaredmcp.toolContextHooksgate;PluginRegistryindexes owners per core tool ID once at load and fans out (Promise.all) only to declaring plugins, isolating throwing/mismatched ones (apps/mcp/src/plugin-loader.ts).- Core-tool enrichment: after a successful core call, plugin sections are joined with
\n\nand merged into the result's last text block (new text block when the last block isn't text), skipped onisErrorresults (apps/mcp/src/server.ts). - 8 Vitest cases pinning no-hook, per-tool filtering, null omission, manifest/module mismatch, and throwing-plugin behaviors. All 503 MCP tests pass; Biome clean;
tscbuild passes. - Docs section in
docs/plugins/mcp-plugin-system.md;MCPManifest.ToolContextHookson the Go side with matching camelCase JSON tag.
ℹ️ Enrichment wiring in server.ts is untested
The registry method gets thorough unit coverage, but the code that actually wires the feature into tool execution has none: the isError skip, the merge-into-last-text-block (with the push-new-block fallback), and proof that getToolContext is invoked with the core tool's name/args are all uncovered. apps/mcp has no server.test.ts at all, so once a plugin declares a hook, a regression in this glue (e.g. a mis-shapen content) would silently affect every matching core tool call.
Technical details
# Test the core-tool enrichment glue
## Affected sites
- apps/mcp/src/server.ts:186-196 — the merge-into-last-text-block / push-new-block logic
- apps/mcp/src/server.ts:172-177 — the isError skip + getToolContext invocation
## Required outcome
- Coverage that a successful core result gets plugin text merged into its last text block; an `isError: true` result never reaches `getToolContext`; a last block that isn't text gets a new text block appended; empty plugin sections leave the result untouched.
## Suggested approach (optional)
- Extract the merge into a small exported pure function (e.g. `appendPluginSections(result, sections)`) and unit-test it directly, sidestepping the need for a full `Server` instance.ℹ️ Nitpicks
apps/mcp/src/plugin-loader.ts:200-204— the manifest/module-mismatch case is logged viaconsole.erroron every matching tool call. The mismatch is static (known once the entry is imported); warn once at load time instead of per call.- Section order is nondeterministic:
Promise.all+sections.pushyields completion order, not manifest declaration order, so the joined output varies call to call when multiple plugins have a section. Sorting by declaration order (orpluginId) would make output stable. docs/plugins/mcp-plugin-system.md:154-156— "joined and appended as one additional content block" contradicts the implementation, which merges into the last text block (deliberately, per theserver.tscomment about agents missing trailing blocks).
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| - Return `null` (or `undefined`) when the plugin has nothing to contribute | ||
| for this call. The host omits the section entirely rather than rendering | ||
| empty boilerplate on every call — most calls won't touch every plugin. | ||
| - The host calls `getToolContext` for every loaded plugin that implements |
There was a problem hiding this comment.
The host only invokes getToolContext for plugins whose manifest lists the exact tool ID in mcp.toolContextHooks — PluginRegistry builds that gate at load and the "never calls a plugin that didn't declare a hook" test enforces it. Two doc defects follow: the "Plugin Manifest" example (above) still shows only remoteEntryUrl, so the required field is undiscoverable, and this bullet ("every loaded plugin that implements it") states the opposite of the shipped, manifest-gated behavior. A plugin author following the docs end-to-end gets a silently dead hook — no error, no context, ever.

Summary
Implements the host side of
getToolContext(SDK contract added in Paca-AI/plugin-sdk-mcp#1, released as@paca-ai/plugin-sdk-mcp@0.2.0): lets a plugin attach additional text to the response of any core MCP tool call, declared per-tool in its manifest.Motivating case: an AI agent calls
get_taskand has no idea a GitHub branch or PR is linked to that task unless it separately callsgithub_list_task_branches/github_list_task_prs. This lets the GitHub (or BDD, Checklist, …) plugin attach that info directly to theget_taskresponse instead.What changed
apps/mcp/src/plugin-loader.ts—PluginRegistrynow builds aMap<toolId, LoadedPlugin[]>at load time from each plugin's manifest-declaredmcp.toolContextHooks.getToolContext(toolId, args, config)is a map lookup, not a fan-out: a plugin that didn't declare a given tool ID is never invoked for it.apps/mcp/src/server.ts— single interception point in theCallToolRequestSchemahandler: after any core tool call resolves (and isn't itself an error), it asks the registry for context sections and merges them into the tool result. This covers every core tool automatically, with no per-tool wiring.services/api/internal/domain/plugin/entity.go— addedToolContextHooks []stringtoMCPManifest. Without this, the field would've been silently dropped: the plugin repository re-marshals the typed manifest struct into the DB's JSONB column rather than storing the raw JSON, so an unrecognizedplugin.jsonfield never survives an install/update round-trip.docs/plugins/mcp-plugin-system.md— documents thegetToolContextcontract, the manifest opt-in, and theget_task_by_numbercaveat (its args have notaskId, onlytaskNumber, so a hook scoped toget_taskwon't fire for it).apps/mcp/src/__tests__/plugin-loader.test.ts(new) — covers the map-based dispatch: a plugin is never called for a tool ID it didn't declare, multiple declared plugins' sections are collected in order, a throwing plugin doesn't affect others, a manifest/module mismatch (declared but not implemented) degrades gracefully.A bug found (and fixed) while verifying this end-to-end
Traced a real agent conversation's
get_taskcall against a task with a linked GitHub branch. The plugin's context was correctly generated and attached — but as a separate trailing content block, not merged into the main text. The agent didn't treat that block as part of the task's context and calledgithub_list_task_branchesanyway right after, defeating the point.Fix (in
server.ts): merge plugin-contributed text into the last existing text block instead of appending a new content array entry, soget_taskreturns one continuous passage — task detail followed directly by## GitHub— instead of a result an agent can partially ignore.Test plan
apps/mcp:tsc --noEmitclean,vitest run— 503/503 passservices/api:go build ./...,go vet ./..., andgo test ./internal/domain/plugin/... ./internal/repository/postgres/... ./internal/transport/http/...all passmcpservice, confirmed 0 compile errors, confirmed the compiledbuild/server.js(what the sandboxed agent'snode /mcp/build/index.jsactually runs) has the fixagent_conversation_eventson a real conversation thatget_taskreturns a merged## GitHubsection for a task with a linked branchDepends on
com.paca.github,com.paca.bdd, andcom.paca.checklistplugin repos are updated separately to declaretoolContextHooksin their manifests and implementgetToolContext— those changes ship independently and require@paca-ai/plugin-sdk-mcp@^0.2.0.