Skip to content

Commit 6c9ae5f

Browse files
feat(mcp): add Model Context Protocol server support
Add MCP client and provider packages that allow integrating external MCP tool servers into the review loop. Includes config commands for managing MCP servers, stdio subprocess integration tests, and comprehensive test coverage.
1 parent 52a51f8 commit 6c9ae5f

16 files changed

Lines changed: 1383 additions & 19 deletions

cmd/opencodereview/config_cmd.go

Lines changed: 121 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,23 @@ func runConfigSet(key, value string) error {
9494

9595
func runConfigUnset(key string) error {
9696
parts := strings.SplitN(key, ".", 2)
97-
if len(parts) != 2 || parts[0] != "custom_providers" || parts[1] == "" {
98-
return fmt.Errorf("unset only supports custom_providers.<name>")
97+
if len(parts) != 2 || parts[1] == "" {
98+
return fmt.Errorf("unset supports custom_providers.<name> and mcp_servers.<name>")
9999
}
100-
name := parts[1]
101100

102101
configPath, err := defaultConfigPath()
103102
if err != nil {
104103
return err
105104
}
106105

107-
return unsetCustomProvider(configPath, name)
106+
switch parts[0] {
107+
case "custom_providers":
108+
return unsetCustomProvider(configPath, parts[1])
109+
case "mcp_servers":
110+
return unsetMCPServer(configPath, parts[1])
111+
default:
112+
return fmt.Errorf("unset supports custom_providers.<name> and mcp_servers.<name>")
113+
}
108114
}
109115

110116
func unsetCustomProvider(configPath, name string) error {
@@ -130,6 +136,32 @@ func unsetCustomProvider(configPath, name string) error {
130136
return nil
131137
}
132138

139+
func unsetMCPServer(configPath, name string) error {
140+
cfg, err := loadOrCreateConfig(configPath)
141+
if err != nil {
142+
return fmt.Errorf("load config: %w", err)
143+
}
144+
145+
if cfg.MCPServers == nil {
146+
return fmt.Errorf("MCP server %q not found", name)
147+
}
148+
if _, exists := cfg.MCPServers[name]; !exists {
149+
return fmt.Errorf("MCP server %q not found", name)
150+
}
151+
152+
delete(cfg.MCPServers, name)
153+
if len(cfg.MCPServers) == 0 {
154+
cfg.MCPServers = nil
155+
}
156+
157+
if err := saveConfig(configPath, cfg); err != nil {
158+
return err
159+
}
160+
161+
fmt.Printf("Deleted MCP server %q.\n", name)
162+
return nil
163+
}
164+
133165
// deleteCustomProvider removes a custom provider from cfg in memory.
134166
// Returns true if the deleted provider was the active one.
135167
func deleteCustomProvider(cfg *Config, name string) (bool, error) {
@@ -166,15 +198,25 @@ type ProviderEntry struct {
166198
ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
167199
}
168200

201+
// MCPServerConfig holds configuration for a single MCP server (stdio transport).
202+
type MCPServerConfig struct {
203+
Command string `json:"command"`
204+
Args []string `json:"args,omitempty"`
205+
Env []string `json:"env,omitempty"`
206+
Tools []string `json:"tools,omitempty"`
207+
Setup string `json:"setup,omitempty"`
208+
}
209+
169210
// Config represents the user-level configuration file (~/.opencodereview/config.json).
170211
type Config struct {
171-
Provider string `json:"provider,omitempty"`
172-
Model string `json:"model,omitempty"`
173-
Providers map[string]ProviderEntry `json:"providers,omitempty"`
174-
CustomProviders map[string]ProviderEntry `json:"custom_providers,omitempty"`
175-
Llm LlmConfig `json:"llm,omitempty"`
176-
Language string `json:"language,omitempty"`
177-
Telemetry *TelemetryConfig `json:"telemetry,omitempty"`
212+
Provider string `json:"provider,omitempty"`
213+
Model string `json:"model,omitempty"`
214+
Providers map[string]ProviderEntry `json:"providers,omitempty"`
215+
CustomProviders map[string]ProviderEntry `json:"custom_providers,omitempty"`
216+
Llm LlmConfig `json:"llm,omitempty"`
217+
Language string `json:"language,omitempty"`
218+
Telemetry *TelemetryConfig `json:"telemetry,omitempty"`
219+
MCPServers map[string]MCPServerConfig `json:"mcp_servers,omitempty"`
178220
}
179221

180222
type LlmConfig struct {
@@ -234,6 +276,9 @@ func setConfigValue(cfg *Config, key, value string) error {
234276
if strings.HasPrefix(key, "custom_providers.") {
235277
return setCustomProviderValue(cfg, key, value)
236278
}
279+
if strings.HasPrefix(key, "mcp_servers.") {
280+
return setMCPServerValue(cfg, key, value)
281+
}
237282

238283
switch key {
239284
case "provider":
@@ -329,7 +374,7 @@ func setConfigValue(cfg *Config, key, value string) error {
329374
}
330375
cfg.Llm.ExtraBody = m
331376
default:
332-
return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, llm.url, llm.auth_token, llm.auth_header, llm.model, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, url, protocol, model, models, auth_header, extra_body, extra_headers", key)
377+
return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_header, llm.model, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, url, protocol, model, models, auth_header, extra_body, extra_headers\nMCP server fields: command, args, env, tools, setup", key)
333378
}
334379
return nil
335380
}
@@ -491,6 +536,70 @@ func setCustomProviderField(cfg *Config, name, field, key, value string) error {
491536
return nil
492537
}
493538

539+
func setMCPServerValue(cfg *Config, key, value string) error {
540+
parts := strings.SplitN(key, ".", 3)
541+
if len(parts) != 3 || parts[1] == "" || parts[2] == "" {
542+
return fmt.Errorf("invalid MCP server key %q: expected mcp_servers.<name>.<field>", key)
543+
}
544+
name, field := parts[1], parts[2]
545+
546+
if cfg.MCPServers == nil {
547+
cfg.MCPServers = make(map[string]MCPServerConfig)
548+
}
549+
entry := cfg.MCPServers[name]
550+
551+
switch field {
552+
case "command":
553+
if value == "" {
554+
return fmt.Errorf("MCP server command cannot be empty")
555+
}
556+
entry.Command = value
557+
case "args":
558+
var args []string
559+
if err := json.Unmarshal([]byte(value), &args); err != nil {
560+
return fmt.Errorf("invalid JSON array for %s: %w", key, err)
561+
}
562+
entry.Args = args
563+
case "env":
564+
var env []string
565+
if err := json.Unmarshal([]byte(value), &env); err != nil {
566+
return fmt.Errorf("invalid JSON array for %s: %w", key, err)
567+
}
568+
for _, e := range env {
569+
idx := strings.Index(e, "=")
570+
if idx <= 0 {
571+
return fmt.Errorf("invalid env entry %q: must be in KEY=VALUE format", e)
572+
}
573+
}
574+
entry.Env = env
575+
case "tools":
576+
var tools []string
577+
if err := json.Unmarshal([]byte(value), &tools); err != nil {
578+
return fmt.Errorf("invalid JSON array for %s: %w", key, err)
579+
}
580+
seen := make(map[string]struct{}, len(tools))
581+
filtered := make([]string, 0, len(tools))
582+
for _, t := range tools {
583+
if t == "" {
584+
return fmt.Errorf("tool names in %s must not be empty", key)
585+
}
586+
if _, dup := seen[t]; dup {
587+
continue
588+
}
589+
seen[t] = struct{}{}
590+
filtered = append(filtered, t)
591+
}
592+
entry.Tools = filtered
593+
case "setup":
594+
entry.Setup = value
595+
default:
596+
return fmt.Errorf("unknown MCP server field %q: supported fields are command, args, env, tools, setup", field)
597+
}
598+
599+
cfg.MCPServers[name] = entry
600+
return nil
601+
}
602+
494603
func (c *Config) ensureTelemetry() {
495604
if c.Telemetry == nil {
496605
c.Telemetry = &TelemetryConfig{}

0 commit comments

Comments
 (0)