-
Notifications
You must be signed in to change notification settings - Fork 1
Convert MCP servers from slice to map structure #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
| // MCPServers maps server names to their configurations | ||
| // Does not filter rules, metadata only | ||
| MCPServers []MCPServerConfig `yaml:"mcp_servers,omitempty" json:"mcp_servers,omitempty"` | ||
| MCPServers map[string]MCPServerConfig `yaml:"mcp_servers,omitempty" json:"mcp_servers,omitempty"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please add a new type:
type MCPServerConfigs map[string]MCPServerConfigThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added MCPServerConfigs type alias and updated all usages throughout the codebase. (f6ec4d7)
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR converts the MCP servers configuration from a slice-based structure to a map-based structure, where server names map to their configurations. This change improves the API by allowing servers to be referenced by name and enables task-level server configurations to override rule-level configurations with the same name.
- Changed
MCPServersfield from[]MCPServerConfigtoMCPServerConfigs(a type alias formap[string]MCPServerConfig) in bothTaskFrontMatterandRuleFrontMatter - Updated the
Result.MCPServers()method to merge servers from rules and tasks, with task servers taking precedence over rule servers with the same name - Added comprehensive test coverage including a new test case for override behavior
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/codingcontext/mcp_server_config.go | Adds MCPServerConfigs type alias for map[string]MCPServerConfig |
| pkg/codingcontext/task_frontmatter.go | Changes MCPServers field from slice to map using new type alias |
| pkg/codingcontext/rule_frontmatter.go | Changes MCPServers field from slice to map using new type alias |
| pkg/codingcontext/result.go | Updates merge logic to use map iteration and task-override-rule precedence |
| pkg/codingcontext/task_frontmatter_test.go | Updates test data to use map structure instead of slices |
| pkg/codingcontext/rule_frontmatter_test.go | Updates test data to use map structure instead of slices |
| pkg/codingcontext/result_test.go | Updates all test cases to use maps, adds override test case, updates assertions to iterate over map |
| pkg/codingcontext/context_test.go | Updates test YAML strings and assertions to use map format |
| examples/agents/tasks/example-with-standard-fields.md | Updates example frontmatter to use proper map YAML syntax with server names as keys |
| docs/reference/file-formats.md | Updates documentation to describe map structure and provides comprehensive examples |
Plan: Change MCP Servers from Slice to Map
TaskFrontMatter.MCPServersfrom[]MCPServerConfigtomap[string]MCPServerConfigRuleFrontMatter.MCPServersfrom[]MCPServerConfigtomap[string]MCPServerConfigResult.MCPServers()method to return map instead of sliceSummary
Successfully changed MCP servers from a slice to a map structure and addressed all feedback.
Latest Changes (addressing PR feedback):
MCPServerConfigsas an alias formap[string]MCPServerConfigThe new type is defined in
mcp_server_config.goand used consistently across:TaskFrontMatter.MCPServersRuleFrontMatter.MCPServersResult.MCPServers()return typeAll tests pass, lint succeeds, and the build completes successfully.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.