Skip to content

Fix getMcpServerConfig to merge all MCP config keys and validate by transport type#309

Merged
sroussey merged 2 commits intoclaude/add-mcp-task-registry-y9KGEfrom
copilot/sub-pr-307
Mar 14, 2026
Merged

Fix getMcpServerConfig to merge all MCP config keys and validate by transport type#309
sroussey merged 2 commits intoclaude/add-mcp-task-registry-y9KGEfrom
copilot/sub-pr-307

Conversation

Copy link
Contributor

Copilot AI commented Mar 13, 2026

getMcpServerConfig() only merged a hardcoded subset of keys (transport, server_url, command, args, env), silently dropping all auth fields when using a registry server. It also lacked transport-aware validation, causing runtime crashes via ! assertions inside createMcpClient().

Changes

  • Merge all MCP config keys: Replace the hardcoded key list with Object.keys(mcpServerConfigSchema.properties), which includes all auth fields (auth_type, auth_token, auth_client_id, auth_client_secret, auth_private_key, auth_algorithm, auth_jwt_bearer_assertion, auth_redirect_url, auth_scope, auth_client_name, auth_jwt_lifetime_seconds) since mcpServerConfigSchema.properties already spreads mcpAuthConfigSchema.properties.

  • Transport-based validation: Throw descriptive errors before handing off to mcpClientFactory.create():

    • stdio → requires command
    • sse / streamable-http → requires server_url
// Before: silently dropped auth fields, crashed at runtime on missing command/server_url
for (const key of ["transport", "server_url", "command", "args", "env"] as const) { ... }

// After: merges all schema-defined keys, validates per transport
for (const key of Object.keys(mcpServerConfigSchema.properties)) { ... }
if (merged.transport === "stdio" && !merged.command) {
  throw new Error("MCP server command is required for stdio transport");
}
if ((merged.transport === "sse" || merged.transport === "streamable-http") && !merged.server_url) {
  throw new Error("MCP server URL is required for sse and streamable-http transports");
}

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…getMcpServerConfig

Co-authored-by: sroussey <127349+sroussey@users.noreply.github.com>
Copilot AI changed the title [WIP] [WIP] Address feedback on MCP server registry and server references Fix getMcpServerConfig to merge all MCP config keys and validate by transport type Mar 13, 2026
Copilot AI requested a review from sroussey March 13, 2026 22:46
@sroussey sroussey marked this pull request as ready for review March 14, 2026 00:02
@sroussey sroussey merged commit 7dea402 into claude/add-mcp-task-registry-y9KGE Mar 14, 2026
@sroussey sroussey deleted the copilot/sub-pr-307 branch March 14, 2026 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants