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>
This commit is contained in:
lif
2025-12-30 16:54:27 +08:00
committed by GitHub
parent 0d0ef0121c
commit 8bd7143a44
3 changed files with 75 additions and 5 deletions

View File

@@ -501,7 +501,13 @@ func (c *ModelConfig) Validate() (bool, error) {
if !re.MatchString(c.Backend) {
return false, fmt.Errorf("invalid backend name: %s", c.Backend)
}
return true, nil
}
// Validate MCP configuration if present
if c.MCP.Servers != "" || c.MCP.Stdio != "" {
if _, _, err := c.MCP.MCPConfigFromYAML(); err != nil {
return false, fmt.Errorf("invalid MCP configuration: %w", err)
}
}
return true, nil

View File

@@ -169,8 +169,10 @@ func (bcl *ModelConfigLoader) LoadMultipleModelConfigsSingleFile(file string, op
}
for _, cc := range c {
if valid, _ := cc.Validate(); valid {
if valid, err := cc.Validate(); valid {
bcl.configs[cc.Name] = *cc
} else {
xlog.Warn("skipping invalid model config", "name", cc.Name, "error", err)
}
}
return nil
@@ -184,9 +186,12 @@ func (bcl *ModelConfigLoader) ReadModelConfig(file string, opts ...ConfigLoaderO
return fmt.Errorf("ReadModelConfig cannot read config file %q: %w", file, err)
}
if valid, _ := c.Validate(); valid {
if valid, err := c.Validate(); valid {
bcl.configs[c.Name] = *c
} else {
if err != nil {
return fmt.Errorf("config is not valid: %w", err)
}
return fmt.Errorf("config is not valid")
}
@@ -364,10 +369,10 @@ func (bcl *ModelConfigLoader) LoadModelConfigsFromPath(path string, opts ...Conf
xlog.Error("LoadModelConfigsFromPath cannot read config file", "error", err, "File Name", file.Name())
continue
}
if valid, _ := c.Validate(); valid {
if valid, validationErr := c.Validate(); valid {
bcl.configs[c.Name] = *c
} else {
xlog.Error("config is not valid", "error", err, "Name", c.Name)
xlog.Error("config is not valid", "error", validationErr, "Name", c.Name)
}
}

View File

@@ -166,4 +166,63 @@ parameters:
Expect(i.HasUsecases(FLAG_COMPLETION)).To(BeTrue())
Expect(i.HasUsecases(FLAG_CHAT)).To(BeTrue())
})
It("Test Validate with invalid MCP config", func() {
tmp, err := os.CreateTemp("", "config.yaml")
Expect(err).To(BeNil())
defer os.Remove(tmp.Name())
_, err = tmp.WriteString(
`name: test-mcp
backend: "llama-cpp"
mcp:
stdio: |
{
"mcpServers": {
"ddg": {
"command": "/docker/docker",
"args": ["run", "-i"]
}
"weather": {
"command": "/docker/docker",
"args": ["run", "-i"]
}
}
}`)
Expect(err).ToNot(HaveOccurred())
config, err := readModelConfigFromFile(tmp.Name())
Expect(err).To(BeNil())
Expect(config).ToNot(BeNil())
valid, err := config.Validate()
Expect(err).To(HaveOccurred())
Expect(valid).To(BeFalse())
Expect(err.Error()).To(ContainSubstring("invalid MCP configuration"))
})
It("Test Validate with valid MCP config", func() {
tmp, err := os.CreateTemp("", "config.yaml")
Expect(err).To(BeNil())
defer os.Remove(tmp.Name())
_, err = tmp.WriteString(
`name: test-mcp-valid
backend: "llama-cpp"
mcp:
stdio: |
{
"mcpServers": {
"ddg": {
"command": "/docker/docker",
"args": ["run", "-i"]
},
"weather": {
"command": "/docker/docker",
"args": ["run", "-i"]
}
}
}`)
Expect(err).ToNot(HaveOccurred())
config, err := readModelConfigFromFile(tmp.Name())
Expect(err).To(BeNil())
Expect(config).ToNot(BeNil())
valid, err := config.Validate()
Expect(err).To(BeNil())
Expect(valid).To(BeTrue())
})
})