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()) + }) })