Change mcp_servers map to single mcp_server configuration object #164
Conversation
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
|
@copilot No. Sorry. Rules contain MCP servers. These are just like tools. In the front-matter, the must specific one instance of the MCP server configuration. mcp_server:
command: python
args: ["-m", "server"]
env:
PYTHON_PATH: /usr/bin/python3
# Arbitrary nested fields below
custom_config:
host: localhost
port: 5432
ssl: true
pool:
min: 2
max: 10 |
Convert from map of server configs to single MCPServerConfig object. Each task/rule now specifies one MCP server configuration with standard fields (command, args, env, type, url, headers) plus arbitrary custom fields via the Content map. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Updated in commit 64b7225. Changed from |
There was a problem hiding this comment.
Pull request overview
This PR refactors the MCP (Model Context Protocol) server configuration from a map-based approach to a single-object configuration model. Instead of each task/rule specifying multiple named MCP servers via a mcp_servers map, each task/rule now specifies one MCP server configuration via the mcp_server field. The Result.MCPServers() method now returns a slice of all MCP server configurations collected from rules and the task.
Key changes:
- API change:
mcp_serversmap field →mcp_serversingle object field in task/rule frontmatter - Return type change:
MCPServers()returns[]MCPServerConfigslice instead ofMCPServerConfigsmap - Documentation and examples updated to reflect the new single-server-per-file model
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/codingcontext/task_frontmatter.go | Changed MCPServers field from map to single MCPServer object |
| pkg/codingcontext/rule_frontmatter.go | Changed MCPServers field from map to single MCPServer object |
| pkg/codingcontext/result.go | Refactored MCPServers() to return slice and append configs from rules and task |
| pkg/codingcontext/task_frontmatter_test.go | Updated tests to use single MCP server object with Args field |
| pkg/codingcontext/rule_frontmatter_test.go | Updated tests to use single MCP server object with Args field |
| pkg/codingcontext/result_test.go | Rewrote tests for slice-based return type and removed override test case |
| pkg/codingcontext/README.md | Updated API documentation for new slice return type |
| docs/reference/file-formats.md | Updated reference docs with single-object configuration format and examples |
| examples/agents/tasks/example-with-standard-fields.md | Updated example to show single MCP server configuration |
| examples/agents/tasks/example-mcp-arbitrary-fields.md | Simplified example to show one server with arbitrary custom fields |
pkg/codingcontext/result.go
Outdated
| // Check if the MCPServer is not empty (has at least one field set) | ||
| if rule.FrontMatter.MCPServer.Command != "" || rule.FrontMatter.MCPServer.URL != "" { | ||
| servers = append(servers, rule.FrontMatter.MCPServer) | ||
| } | ||
| } | ||
|
|
||
| // Add servers from task (overriding any from rules) | ||
| if r.Task.FrontMatter.MCPServers != nil { | ||
| for name, config := range r.Task.FrontMatter.MCPServers { | ||
| servers[name] = config | ||
| } | ||
| // Add server from task if it has one | ||
| if r.Task.FrontMatter.MCPServer.Command != "" || r.Task.FrontMatter.MCPServer.URL != "" { | ||
| servers = append(servers, r.Task.FrontMatter.MCPServer) |
There was a problem hiding this comment.
The empty server detection logic only checks Command and URL fields. However, a valid MCP server configuration could have only Args, Env, Headers, or Content fields set without Command or URL (though unlikely). Consider checking if any meaningful field is set, or implement an IsEmpty() method on MCPServerConfig for clearer semantics. For example, a config with only Type set (without Command or URL) would currently be excluded, which might be intentional but isn't clearly documented.
There was a problem hiding this comment.
@copilot remove these checks. This check will need to be done by the agent, so the check is duplicative.
There was a problem hiding this comment.
Removed in commit 6ad46e5. The MCPServers() method now returns all configs without filtering, including empty ones. The agent can perform any necessary filtering.
Remove the empty server detection logic from MCPServers() method as requested. The agent will handle filtering empty configs. Updated tests to expect empty MCPServerConfig structs in the returned slice. Co-authored-by: alexec <1142830+alexec@users.noreply.github.com>
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.