Fix MCP server registry: auth merging, schema enforcement, config immutability#321
Draft
Copilot wants to merge 2 commits intoclaude/add-mcp-task-registry-y9KGEfrom
Draft
Fix MCP server registry: auth merging, schema enforcement, config immutability#321Copilot wants to merge 2 commits intoclaude/add-mcp-task-registry-y9KGEfrom
Copilot wants to merge 2 commits intoclaude/add-mcp-task-registry-y9KGEfrom
Conversation
- Fix McpServerSchema.ts: correct doc comment for format resolution mechanism and add allOf transport-specific constraints - Fix McpServerRepository.ts: add updateServer() method and fix addServer() to emit correct event (server_updated when overwriting) - Fix TaskRunner.ts: avoid mutating task.config by using per-run copy; restore original in finally block for re-resolution on subsequent runs - Fix getMcpServerConfig() in all 4 MCP task files: merge all MCP config keys (including auth fields) using mcpServerConfigSchema.properties, not just a subset - Fix schema validation in all 4 MCP task files: require either 'server' or 'transport' via anyOf constraint - Fix discoverSchemas() in McpToolCallTask/McpPromptGetTask: spread full serverConfig into mcpList() so auth fields are included - Cache mcpServerConfigKeys at module level to avoid repeated Object.keys() allocations Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Add MCP server registry and support server references in tasks
Fix MCP server registry: auth merging, schema enforcement, config immutability
Mar 19, 2026
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.
Review feedback on the MCP server registry PR identified several correctness issues: incomplete config merging (auth fields dropped), missing schema constraints allowing runtime-only failures,
task.configmutation persisting resolved references across runs, andserver_updatedevent never being emitted.Changes
getMcpServerConfig()— all 4 MCP task filesReplace hardcoded key list (
transport,server_url,command,args,env) with iteration overmcpServerConfigSchema.properties, picking up all auth fields (auth_type,auth_token,auth_client_id, etc.). Inline values still override registry base. Keys array cached at module level.discoverSchemas()—McpToolCallTask,McpPromptGetTaskSpread full
serverConfigintomcpList()instead of a 5-field subset, so auth config is forwarded during schema discovery for authenticated servers.Schema validation — all 4 MCP task files
Add
anyOf: [{ required: ["server"] }, { required: ["transport"] }]so configs missing both a registry reference and inline transport fail at validation, not at runtime. Removes the now-redundant top-levelif/then/else(already covered byallOf: mcpServerConfigSchema.allOf).TaskRunner—run()andrunReactive()Replace
Object.assign(task.config, resolvedConfig)with a per-run shallow copy ({ ...originalConfig, ...resolvedConfig }), restoring the original reference infinally. Registry server ID strings are now re-resolved on every run instead of being permanently overwritten with the expanded object.McpServerRepositoryaddServer()now checks existence before writing and emitsserver_updated(notserver_added) when overwriting. NewupdateServer()method provides an explicit update path that throws if the record doesn't exist.McpServerConfigSchemaAdd
allOf: mcpServerConfigSchema.allOfso persisted server records are validated against transport-specific constraints (e.g.,commandrequired for stdio,server_urlfor sse/streamable-http). Fix misleading doc comment about theformat: "mcp-server"resolution mechanism.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.