Skip to content

Commit 8bd7143

Browse files
majiayu000claude
andauthored
fix: propagate validation errors (#7787)
fix: validate MCP configuration in model config Fixes #7334 The Validate() function was not checking if MCP configuration (mcp.stdio and mcp.remote) contains valid JSON. This caused malformed JSON with missing commas to be silently accepted. Changes: - Add MCP configuration validation to ModelConfig.Validate() - Properly report validation errors instead of discarding them - Add test cases for valid and invalid MCP configurations The fix ensures that malformed JSON in MCP config sections will now be caught and reported during validation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: majiayu000 <1835304752@qq.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 0d0ef01 commit 8bd7143

3 files changed

Lines changed: 75 additions & 5 deletions

File tree

core/config/model_config.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,13 @@ func (c *ModelConfig) Validate() (bool, error) {
501501
if !re.MatchString(c.Backend) {
502502
return false, fmt.Errorf("invalid backend name: %s", c.Backend)
503503
}
504-
return true, nil
504+
}
505+
506+
// Validate MCP configuration if present
507+
if c.MCP.Servers != "" || c.MCP.Stdio != "" {
508+
if _, _, err := c.MCP.MCPConfigFromYAML(); err != nil {
509+
return false, fmt.Errorf("invalid MCP configuration: %w", err)
510+
}
505511
}
506512

507513
return true, nil

core/config/model_config_loader.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,10 @@ func (bcl *ModelConfigLoader) LoadMultipleModelConfigsSingleFile(file string, op
169169
}
170170

171171
for _, cc := range c {
172-
if valid, _ := cc.Validate(); valid {
172+
if valid, err := cc.Validate(); valid {
173173
bcl.configs[cc.Name] = *cc
174+
} else {
175+
xlog.Warn("skipping invalid model config", "name", cc.Name, "error", err)
174176
}
175177
}
176178
return nil
@@ -184,9 +186,12 @@ func (bcl *ModelConfigLoader) ReadModelConfig(file string, opts ...ConfigLoaderO
184186
return fmt.Errorf("ReadModelConfig cannot read config file %q: %w", file, err)
185187
}
186188

187-
if valid, _ := c.Validate(); valid {
189+
if valid, err := c.Validate(); valid {
188190
bcl.configs[c.Name] = *c
189191
} else {
192+
if err != nil {
193+
return fmt.Errorf("config is not valid: %w", err)
194+
}
190195
return fmt.Errorf("config is not valid")
191196
}
192197

@@ -364,10 +369,10 @@ func (bcl *ModelConfigLoader) LoadModelConfigsFromPath(path string, opts ...Conf
364369
xlog.Error("LoadModelConfigsFromPath cannot read config file", "error", err, "File Name", file.Name())
365370
continue
366371
}
367-
if valid, _ := c.Validate(); valid {
372+
if valid, validationErr := c.Validate(); valid {
368373
bcl.configs[c.Name] = *c
369374
} else {
370-
xlog.Error("config is not valid", "error", err, "Name", c.Name)
375+
xlog.Error("config is not valid", "error", validationErr, "Name", c.Name)
371376
}
372377
}
373378

core/config/model_config_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,63 @@ parameters:
166166
Expect(i.HasUsecases(FLAG_COMPLETION)).To(BeTrue())
167167
Expect(i.HasUsecases(FLAG_CHAT)).To(BeTrue())
168168
})
169+
It("Test Validate with invalid MCP config", func() {
170+
tmp, err := os.CreateTemp("", "config.yaml")
171+
Expect(err).To(BeNil())
172+
defer os.Remove(tmp.Name())
173+
_, err = tmp.WriteString(
174+
`name: test-mcp
175+
backend: "llama-cpp"
176+
mcp:
177+
stdio: |
178+
{
179+
"mcpServers": {
180+
"ddg": {
181+
"command": "/docker/docker",
182+
"args": ["run", "-i"]
183+
}
184+
"weather": {
185+
"command": "/docker/docker",
186+
"args": ["run", "-i"]
187+
}
188+
}
189+
}`)
190+
Expect(err).ToNot(HaveOccurred())
191+
config, err := readModelConfigFromFile(tmp.Name())
192+
Expect(err).To(BeNil())
193+
Expect(config).ToNot(BeNil())
194+
valid, err := config.Validate()
195+
Expect(err).To(HaveOccurred())
196+
Expect(valid).To(BeFalse())
197+
Expect(err.Error()).To(ContainSubstring("invalid MCP configuration"))
198+
})
199+
It("Test Validate with valid MCP config", func() {
200+
tmp, err := os.CreateTemp("", "config.yaml")
201+
Expect(err).To(BeNil())
202+
defer os.Remove(tmp.Name())
203+
_, err = tmp.WriteString(
204+
`name: test-mcp-valid
205+
backend: "llama-cpp"
206+
mcp:
207+
stdio: |
208+
{
209+
"mcpServers": {
210+
"ddg": {
211+
"command": "/docker/docker",
212+
"args": ["run", "-i"]
213+
},
214+
"weather": {
215+
"command": "/docker/docker",
216+
"args": ["run", "-i"]
217+
}
218+
}
219+
}`)
220+
Expect(err).ToNot(HaveOccurred())
221+
config, err := readModelConfigFromFile(tmp.Name())
222+
Expect(err).To(BeNil())
223+
Expect(config).ToNot(BeNil())
224+
valid, err := config.Validate()
225+
Expect(err).To(BeNil())
226+
Expect(valid).To(BeTrue())
227+
})
169228
})

0 commit comments

Comments
 (0)