Fix Claude .mcp.json format and align init-generated MCP config#26532
Merged
Fix Claude .mcp.json format and align init-generated MCP config#26532
Conversation
Contributor
|
Hey A couple of things to address before this is ready for review:
If you'd like a hand completing the implementation, here's a ready-to-use prompt:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5f57f095-1b2e-444f-82bd-1e2b7fd6066b Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix GitHub API Consumption Report Agent failure
Remove invalid repository-level Claude MCP config that breaks api-consumption-report runs
Apr 16, 2026
Collaborator
|
@copilot fix the .mcp.json file format to match Claude code syntax. Fix init command that generates this file as well. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5a922218-3dfb-40cd-b1b5-b260b7f9df72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5a922218-3dfb-40cd-b1b5-b260b7f9df72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5a922218-3dfb-40cd-b1b5-b260b7f9df72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5a922218-3dfb-40cd-b1b5-b260b7f9df72 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Updated. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot
AI
changed the title
Remove invalid repository-level Claude MCP config that breaks api-consumption-report runs
Fix Claude .mcp.json format and align init-generated MCP config
Apr 16, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Aligns repository and init-generated .mcp.json with Claude Code’s expected MCP config schema by switching from the legacy servers key to mcpServers, while retaining legacy parsing support.
Changes:
- Updated
.mcp.jsonschema to usemcpServersand adjusted CLI generation logic accordingly. - Added/updated tests to validate
mcpServersoutput and legacyserversparsing behavior. - Updated documentation examples to reference
mcpServers.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/mcp_config_file.go |
Writes new configs using mcpServers, keeps legacy servers field for reads, updates user instructions text. |
pkg/cli/mcp_config_file_test.go |
Updates ensure/parse/marshal tests for mcpServers and adds a legacy-key parse case. |
pkg/cli/init_mcp_test.go |
Updates init tests to assert mcpServers output. |
docs/src/content/docs/reference/gh-aw-as-mcp-server.md |
Updates .mcp.json example to use mcpServers. |
.mcp.json |
Switches repo-local config key from servers to mcpServers. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
pkg/cli/mcp_config_file.go:103
- The rendered instructions always tell users to edit the
"mcpServers"section. For repos that already have a legacy.mcp.jsonusing"servers", following these instructions may still leave the file invalid for Claude if the schema rejects unknown keys (i.e., the legacy"servers"key remains). Consider updating the messaging to explicitly instruct renaming/removing"servers"(or to migrate the config) when a legacy config is detected.
fmt.Fprintln(os.Stderr, "To enable GitHub Copilot Agent MCP server integration, please add the following")
fmt.Fprintln(os.Stderr, "to the \"mcpServers\" section of your .mcp.json file:")
- Files reviewed: 5/5 changed files
- Comments generated: 2
Comment on lines
49
to
+56
| // Parse existing config | ||
| var config MCPConfig | ||
| if err := json.Unmarshal(data, &config); err != nil { | ||
| return fmt.Errorf("failed to parse existing mcp.json: %w", err) | ||
| } | ||
|
|
||
| // Check if the server is already configured correctly | ||
| if existingConfig, exists := config.Servers[ghAwServerName]; exists { | ||
| if existingConfig, exists := config.MCPServers[ghAwServerName]; exists { |
There was a problem hiding this comment.
ensureMCPConfig only checks config.MCPServers when deciding whether the gh-aw server is already configured. If an existing .mcp.json uses the legacy servers key, the config will unmarshal into config.Servers and this logic will always fall through to rendering update instructions, even when the gh-aw server is already present/configured. Consider detecting the legacy case (e.g., config.MCPServers == nil && len(config.Servers) > 0) and tailoring behavior/message accordingly (and/or treating Servers as an alias for MCPServers for the purpose of the identical-config check).
This issue also appears on line 102 of the same file.
Comment on lines
255
to
265
| if !tt.wantErr && tt.wantValid { | ||
| if config.Servers == nil { | ||
| t.Error("Expected servers map to be initialized") | ||
| if tt.name == "legacy config key" { | ||
| if config.Servers == nil { | ||
| t.Error("Expected legacy servers map to be initialized") | ||
| } | ||
| if config.MCPServers != nil { | ||
| t.Error("Expected mcpServers map to be nil for legacy-only config") | ||
| } | ||
| } else if config.MCPServers == nil { | ||
| t.Error("Expected mcpServers map to be initialized") | ||
| } |
There was a problem hiding this comment.
This test branches on tt.name == "legacy config key" to decide which assertions to run. That makes the test fragile (renaming the case can silently change behavior). Consider adding an explicit boolean/enum field on the table (e.g., isLegacyKey) to drive the assertions instead of string-matching the test name.
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 84/100✅ Excellent
Test Classification DetailsView all 5 modified test functions
Score Breakdown
Note on Test InflationThe test-to-production lines ratio in
The inflation is not a concern from a quality standpoint, though it is noted per the rubric. Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §24489092411
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Test Quality Sentinel: 84/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 5 modified test functions verify behavioral contracts, including a new backward-compatibility test for the legacy servers JSON key.
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.
The
GitHub API Consumption Report Agentfailed because Claude loaded a repo-local.mcp.jsonthat did not match Claude’s MCP schema, producing config validation errors during execution.Root cause addressed
.mcp.jsonto Claude-compatible syntax by usingmcpServers(instead ofservers).gh aw init) to write Claude-compatible.mcp.jsonwithmcpServers..mcp.jsonfiles that still useservers.Behavioral impact
gh aw initmatches Claude Code expectations.Additional updates
mcpServers.> [!WARNING]
>
>