fix(plugins): discover MCP servers from mcp.json at plugin root - #2713
Open
aaronpowell wants to merge 2 commits into
Open
fix(plugins): discover MCP servers from mcp.json at plugin root#2713aaronpowell wants to merge 2 commits into
aaronpowell wants to merge 2 commits into
Conversation
…gin root MCP config was declared via an extensions.com.github.awesome-copilot.mcpServers pointer to a .mcp.json file. That namespace is stripped from the served manifest, so nothing carried the MCP declaration through materialization. Per Agent Plugins v1.0.0 the fixed location is mcp.json at the plugin root, which already ships as-is. Drop the pointer, rename both .mcp.json files, and validate mcp.json (schema, closed top-level fields, server transport variants). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
plugin
Contributor
🔒 PR Risk Scan ResultsScanned 12 changed file(s).
✅ No matching risk patterns were detected in changed files.
|
Contributor
|
🟡 Contributor Reputation Check: MEDIUM risk
Maintainers: please review this contributor before merging. |
github-actions
Bot
added
the
needs-review:MEDIUM
Contributor
There was a problem hiding this comment.
Pull request overview
Moves MCP server configuration to spec-compliant root-level mcp.json files and updates discovery, validation, and documentation.
Changes:
- Migrates two MCP configurations to the Agent Plugins schema.
- Updates website discovery and plugin validation.
- Aligns contributor guidance and CI messaging.
Show a summary per file
| File | Description |
|---|---|
plugins/context-matic/README.md |
Updates MCP configuration path. |
plugins/context-matic/mcp.json |
Adds schema and transport type. |
plugins/awesome-copilot/README.md |
Updates MCP configuration link. |
plugins/awesome-copilot/plugin.json |
Removes legacy MCP pointer. |
plugins/awesome-copilot/mcp.json |
Adds the MCP schema declaration. |
eng/validate-plugins.test.mjs |
Adds MCP validation tests. |
eng/validate-plugins.mjs |
Adds root MCP configuration validation. |
eng/generate-website-data.mjs |
Discovers MCP servers from mcp.json. |
AGENTS.md |
Documents the new MCP convention. |
.github/workflows/validate-plugins.yml |
Updates CI guidance. |
.github/skills/create-canvas-extension/SKILL.md |
Updates plugin-generation guidance. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 11/11 changed files
- Comments generated: 2
- Review effort level: Balanced
Replace the hand-rolled MCP checks with Ajv validation against the canonical Agent Plugins v1.0.0 MCP schema, so non-spec configs (empty command/url, non-string args, reserved PLUGIN_ROOT/PLUGIN_DATA env keys, invalid cwd, unknown server fields) are rejected. Per-server errors are re-derived from the matching discriminated branch to avoid unhelpful oneOf output. Also reject a top-level extensions.mcpServers placement, which slipped through because the manifest schema allows arbitrary object-valued extension keys. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 764c5bb4-2811-4dc1-b61d-56c4a5597cc9
Contributor
There was a problem hiding this comment.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
eng/validate-plugins.mjs:232
- A valid JSON value of
nullmakes this dereference throw before the top-level-object check, terminating the entire plugin validation instead of returning the intendedmcp.json must contain a top-level objecterror. Guard the parse-error sentinel before reading it.
if (parsed.parseError) {
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
Comment on lines
+263
to
+265
| if (composition?.mcpServers !== undefined) { | ||
| errors.push(`extensions["${AWESOME_COPILOT_NAMESPACE}"].mcpServers is not supported; declare MCP servers in mcp.json at the plugin root`); | ||
| } |
Comment on lines
+113
to
+115
| export function validateAgentPluginMcpConfig(config) { | ||
| if (validateMcp(config)) { | ||
| return []; |
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
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.
Pull Request Checklist
npm startand verified thatREADME.mdis up to date.mainbranch for this pull request.Description
Plugins that ship an MCP server were losing that server during publish. The root cause wasn't the copy step in
materialize-plugins.mjs, it was where the config was declared: plugins pointed at their MCP file withextensions["com.github.awesome-copilot"].mcpServers": "./.mcp.json", and that whole namespace is stripped when the served manifest is emitted. Nothing carried the MCP declaration through to the marketplace branch, so contributors had to keep re-adding the pointer and it still didn't survive.The Agent Plugins v1.0.0 spec already solves this: MCP servers are discovered from a fixed
mcp.jsonat the plugin root (§6.1), and §7.2.1 explicitly says MCP config "MUST NOT be declared inline inplugin.jsonor loaded from any alternative core path." Since that file lives at the plugin root it is committed and rsynced as-is, so it needs no materialization at all. This PR moves us onto that model and drops the pointer entirely.Changes:
plugins/awesome-copilot/.mcp.jsonandplugins/context-matic/.mcp.jsontomcp.json, adding the requiredmcp.schema.json$schemaand the explicittypefield (stdioandstreamable-httprespectively), which the old files were missing.mcpServerspointer fromplugins/awesome-copilot/plugin.json.generate-website-data.mjsnow readsmcp.jsonoff disk instead of following the manifest field, so the website keeps listing MCP items.validate-plugins.mjsgainsvalidateMcpConfig, which rejects a legacy.mcp.json, a wrong or missing$schema, extra top-level fields (the spec'smcpServersobject is closed), unknown transport types, and servers missingcommand/url. It also rejectsmcpServersappearing anywhere inplugin.json.Type of Contribution
Additional Notes
Worth a careful look: this is a breaking change for any in-flight plugin PR that declares
mcpServersin its manifest. Validation now fails loudly with a message pointing atmcp.json, which seemed better than silently ignoring the field and shipping a plugin with no MCP server, but it is a behaviour change either way.The
context-maticserver had notypefield, so I inferredstreamable-httpfrom itsurl. Please confirm that is the intended transport rather than the deprecatedsse.Validation run locally:
npm run plugin:validateandnpm run buildpass, a materialize/clean round trip is clean, andnode --test eng/validate-plugins.test.mjs eng/materialize-plugins.test.mjspasses with 5 new tests covering themcp.jsonrules. Docs updated inAGENTS.md, both plugin READMEs, the validate-plugins workflow message, and the create-canvas-extension skill.By submitting this pull request, I confirm that my contribution abides by the Code of Conduct and will be licensed under the MIT License.