Skip to content

Commit 967125c

Browse files
committed
fix(config): validate per-case mcp overrides
1 parent 212c014 commit 967125c

4 files changed

Lines changed: 59 additions & 7 deletions

File tree

internal/config/mcp_merge.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,12 @@ package config
33
import (
44
"errors"
55
"fmt"
6+
"strings"
7+
)
8+
9+
const (
10+
mcpModeMocked = "mocked"
11+
builtinFilesystemMCPServer = "filesystem"
612
)
713

814
// MergeCaseMCP computes the effective MCP configuration for a case by merging
@@ -29,6 +35,9 @@ func MergeCaseMCP(evalMCP MCPConfig, caseMCP MCPConfig) (MCPConfig, error) {
2935

3036
indexByName := make(map[string]int, len(merged.Servers))
3137
for i, server := range merged.Servers {
38+
if strings.TrimSpace(server.Name) == "" {
39+
return MCPConfig{}, errors.New("eval-level mcp server name is required")
40+
}
3241
if _, exists := indexByName[server.Name]; exists {
3342
return MCPConfig{}, fmt.Errorf("duplicate eval-level mcp server name %q", server.Name)
3443
}
@@ -45,9 +54,12 @@ func MergeCaseMCP(evalMCP MCPConfig, caseMCP MCPConfig) (MCPConfig, error) {
4554
}
4655
seenCaseNames[server.Name] = struct{}{}
4756

48-
if server.Mode != "mocked" {
57+
if server.Mode != mcpModeMocked {
4958
return MCPConfig{}, fmt.Errorf("case-level mcp server %q must use mode: mocked", server.Name)
5059
}
60+
if server.Name != builtinFilesystemMCPServer && strings.TrimSpace(server.ConfigRef) == "" {
61+
return MCPConfig{}, fmt.Errorf("case-level mcp server %q mocked mode requires config_ref", server.Name)
62+
}
5163

5264
cloned := cloneMCPServer(server)
5365
if idx, ok := indexByName[server.Name]; ok {

internal/config/mcp_merge_test.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ func TestMergeCaseMCP_Errors(t *testing.T) {
110110
name: "duplicate case server names",
111111
evalMCP: MCPConfig{},
112112
caseMCP: MCPConfig{Servers: []MCPServer{
113-
{Name: "svc", Mode: "mocked"},
114-
{Name: "svc", Mode: "mocked"},
113+
{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/open.yaml"},
114+
{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/closed.yaml"},
115115
}},
116116
errMsg: "duplicate case-level mcp server name",
117117
},
@@ -121,13 +121,27 @@ func TestMergeCaseMCP_Errors(t *testing.T) {
121121
caseMCP: MCPConfig{Servers: []MCPServer{{Name: "svc", Mode: "real"}}},
122122
errMsg: "must use mode: mocked",
123123
},
124+
{
125+
name: "case server missing config ref",
126+
evalMCP: MCPConfig{},
127+
caseMCP: MCPConfig{Servers: []MCPServer{{Name: "svc", Mode: "mocked"}}},
128+
errMsg: "mocked mode requires config_ref",
129+
},
130+
{
131+
name: "empty eval server name",
132+
evalMCP: MCPConfig{Servers: []MCPServer{
133+
{Mode: "mocked", ConfigRef: "evals/fixtures/mcp/default.yaml"},
134+
}},
135+
caseMCP: MCPConfig{Servers: []MCPServer{{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/open.yaml"}}},
136+
errMsg: "eval-level mcp server name is required",
137+
},
124138
{
125139
name: "duplicate eval server names",
126140
evalMCP: MCPConfig{Servers: []MCPServer{
127141
{Name: "svc", Mode: "mocked"},
128142
{Name: "svc", Mode: "mocked"},
129143
}},
130-
caseMCP: MCPConfig{Servers: []MCPServer{{Name: "svc", Mode: "mocked"}}},
144+
caseMCP: MCPConfig{Servers: []MCPServer{{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/open.yaml"}}},
131145
errMsg: "duplicate eval-level mcp server name",
132146
},
133147
}

internal/config/validator.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,12 @@ func validateCaseMCP(mcpCfg MCPConfig) []string {
180180
}
181181
seen[server.Name] = struct{}{}
182182

183-
if server.Mode != "mocked" {
183+
if server.Mode != mcpModeMocked {
184184
errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) must use mode: mocked at case level", i, server.Name))
185185
}
186+
if server.Mode == mcpModeMocked && server.Name != builtinFilesystemMCPServer && strings.TrimSpace(server.ConfigRef) == "" {
187+
errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) mocked mode requires config_ref", i, server.Name))
188+
}
186189
if server.Transport != "" && server.Transport != "stdio" {
187190
errs = append(errs, fmt.Sprintf("mcp.servers[%d] (%q) mocked mode only supports stdio transport", i, server.Name))
188191
}

internal/config/validator_test.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,17 @@ func TestValidator_ValidateCaseConfig(t *testing.T) {
920920
},
921921
wantErr: false,
922922
},
923+
{
924+
name: "valid case-level filesystem mocked MCP override without config_ref",
925+
cfg: &CaseConfig{
926+
ID: "test-case",
927+
Input: Input{Prompt: "Say hello"},
928+
MCP: MCPConfig{Servers: []MCPServer{
929+
{Name: "filesystem", Mode: "mocked"},
930+
}},
931+
},
932+
wantErr: false,
933+
},
923934
{
924935
name: "case-level agent_judge with valid context",
925936
cfg: &CaseConfig{
@@ -969,13 +980,25 @@ func TestValidator_ValidateCaseConfig(t *testing.T) {
969980
errMsg: "mcp.servers[0].name is required",
970981
},
971982
{
972-
name: "case-level MCP duplicate names",
983+
name: "case-level MCP missing config_ref",
973984
cfg: &CaseConfig{
974985
ID: "test-case",
975986
Input: Input{Prompt: "Say hello"},
976987
MCP: MCPConfig{Servers: []MCPServer{
977988
{Name: "svc", Mode: "mocked"},
978-
{Name: "svc", Mode: "mocked"},
989+
}},
990+
},
991+
wantErr: true,
992+
errMsg: "mocked mode requires config_ref",
993+
},
994+
{
995+
name: "case-level MCP duplicate names",
996+
cfg: &CaseConfig{
997+
ID: "test-case",
998+
Input: Input{Prompt: "Say hello"},
999+
MCP: MCPConfig{Servers: []MCPServer{
1000+
{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/open.yaml"},
1001+
{Name: "svc", Mode: "mocked", ConfigRef: "evals/fixtures/mcp/closed.yaml"},
9791002
}},
9801003
},
9811004
wantErr: true,

0 commit comments

Comments
 (0)