From 8bd7143a443df914c9a2608c29cebd40dd45d0bd Mon Sep 17 00:00:00 2001
From: lif <1835304752@qq.com>
Date: Tue, 30 Dec 2025 16:54:27 +0800
Subject: [PATCH] fix: propagate validation errors (#7787)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 core/config/model_config.go        |  8 +++-
 core/config/model_config_loader.go | 13 +++++--
 core/config/model_config_test.go   | 59 ++++++++++++++++++++++++++++++
 3 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/core/config/model_config.go b/core/config/model_config.go
index 9eba1c535..9010c84e6 100644
--- a/core/config/model_config.go
+++ b/core/config/model_config.go
@@ -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
diff --git a/core/config/model_config_loader.go b/core/config/model_config_loader.go
index 43ffbe890..1a8c64230 100644
--- a/core/config/model_config_loader.go
+++ b/core/config/model_config_loader.go
@@ -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)
 		}
 	}
 
diff --git a/core/config/model_config_test.go b/core/config/model_config_test.go
index 342b10c47..a086d95f6 100644
--- a/core/config/model_config_test.go
+++ b/core/config/model_config_test.go
@@ -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())
+	})
 })