Implement per-case MCP overrides and related documentation#147
Open
roark47 wants to merge 6 commits into
Open
Conversation
Add CaseConfig.MCP field, MergeCaseMCP helper (whole-entry replace by name, mocked-only MVP, deep copy) and case-level MCP validation. Covers SUP-0003.
Add provisionMCPConfigForCase: cases without overrides reuse the eval-only sync.Once fast path; cases with overrides merge and provision fresh so mocked fixtures stay isolated under parallelism. Covers SUP-0003.
Two cases share the project-mgmt server and get_project tool but override the fixture per case (OPEN/CLOSED). Gated by the e2e build tag and full-E2E env. Covers SUP-0003.
Add per-case mocked MCP override sections to the English and Chinese writing-evals guides, the skill-upper eval-yaml reference, and CHANGELOG. Covers SUP-0003.
There was a problem hiding this comment.
Pull request overview
This PR adds per-case overrides for mocked MCP servers so each case can swap mocked fixtures (via config_ref) while keeping server/tool names stable, enabling isolated behavior across cases (including parallel execution).
Changes:
- Introduces merge semantics for case-level
mcp.serverson top of eval-level MCP config (replace-by-name, append new). - Updates evaluator runtime preparation to provision MCP per case when overrides are present, while preserving the cached fast-path when not.
- Adds validator coverage, unit/e2e tests, fixtures, and documentation/changelog entries describing the feature and constraints.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/skill-upper/references/eval-yaml.md | Documents case-level mocked MCP override behavior in the Skill reference. |
| internal/mcp/provisioner_test.go | Adds unit test proving same server name can embed different mocked fixtures via different config_ref. |
| internal/evaluator/evaluator.go | Switches to per-case MCP provisioning when case-level overrides are present. |
| internal/evaluator/evaluator_test.go | Adds tests validating override application and rejecting case-level mode: real. |
| internal/config/validator.go | Adds case-level MCP validation logic for MVP constraints. |
| internal/config/validator_test.go | Extends validator test matrix for valid/invalid case-level MCP overrides. |
| internal/config/schema.go | Adds mcp block to CaseConfig schema with documented merge semantics. |
| internal/config/mcp_merge.go | Adds MergeCaseMCP implementation for eval/case MCP server merging. |
| internal/config/mcp_merge_test.go | Adds unit tests for merge semantics, immutability, and error cases. |
| internal/config/loader_test.go | Adds loader tests ensuring case-level mcp is parsed and defaults behave. |
| e2e/testdata/mcp-case-overrides/SKILL.md | Adds a smoke skill used by e2e to exercise case overrides. |
| e2e/testdata/mcp-case-overrides/evals/fixtures/mcp/project-open.yaml | Adds OPEN fixture for per-case override. |
| e2e/testdata/mcp-case-overrides/evals/fixtures/mcp/project-default.yaml | Adds default fixture (UNKNOWN) for eval-level baseline. |
| e2e/testdata/mcp-case-overrides/evals/fixtures/mcp/project-closed.yaml | Adds CLOSED fixture for per-case override. |
| e2e/testdata/mcp-case-overrides/evals/eval.yaml | Adds eval config wiring default server + two case files. |
| e2e/testdata/mcp-case-overrides/evals/cases/project-open.yaml | Adds case file overriding fixture to OPEN. |
| e2e/testdata/mcp-case-overrides/evals/cases/project-closed.yaml | Adds case file overriding fixture to CLOSED. |
| e2e/mcp_test.go | Adds end-to-end test that runs two cases with per-case mocked overrides. |
| docs/zh/guide/writing-evals.md | Documents per-case mocked MCP overrides (Chinese). |
| docs/guide/writing-evals.md | Documents per-case mocked MCP overrides (English). |
| CHANGELOG.md | Adds changelog entry describing SUP-0003 behavior and constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+186
to
+194
| if server.Transport != "" && server.Transport != "stdio" { | ||
| errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) mocked mode only supports stdio transport", i, server.Name)) | ||
| } | ||
| if server.Endpoint != "" { | ||
| errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) mocked mode does not support endpoint", i, server.Name)) | ||
| } | ||
| if server.Command != "" || len(server.Args) > 0 { | ||
| errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) mocked mode does not support command/args", i, server.Name)) | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Good catch, this is reasonable. I added validation so case-level mocked MCP servers require config_ref unless they use the built-in filesystem mock, and added validator coverage for both the missing config_ref case and the filesystem exception. Verified with go test -race ./internal/config, make verify, and make test.
Comment on lines
+30
to
+36
| indexByName := make(map[string]int, len(merged.Servers)) | ||
| for i, server := range merged.Servers { | ||
| if _, exists := indexByName[server.Name]; exists { | ||
| return MCPConfig{}, fmt.Errorf("duplicate eval-level mcp server name %q", server.Name) | ||
| } | ||
| indexByName[server.Name] = i | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Agreed, using an empty eval-level server name as a merge key would make the effective MCP config confusing. I added a merge-time guard for empty eval-level names and a regression test for that path. I also added the same case-level config_ref guard in MergeCaseMCP so non-validator callers get a clear error too. Verified with go test -race ./internal/config, make verify, and make test.
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.
This pull request introduces support for per-case mocked MCP server overrides, allowing each test case to specify its own mocked MCP fixture while keeping server and tool names consistent. This enables more flexible and isolated testing scenarios, especially when running cases in parallel. The implementation includes configuration merging logic, schema and validation updates, documentation, and comprehensive tests.
Per-case mocked MCP server overrides
Added support for case-level
mcp.serversoverrides in test case YAML files. Case-level servers merge with eval-level servers by name: a matching name replaces the eval-level entry, and new names are appended. Each case can now specify its own mocked fixture, with strict validation to ensure isolation and correctness. [1] [2] [3]Documented the new per-case MCP override feature in both the English and Chinese guides, including configuration examples, merging rules, and constraints. [1] [2]
Testing and validation
Example and fixture files
Changelog