From 0e7d4ddbdfe3ed9e15e856eb6db0c24aedee36e5 Mon Sep 17 00:00:00 2001 From: Solomon Date: Fri, 8 Aug 2025 18:24:04 +0100 Subject: [PATCH] Codex MCP installation (#345) --- crates/executors/src/executors/mod.rs | 2 +- crates/local-deployment/src/container.rs | 12 +- crates/server/Cargo.toml | 1 + crates/server/src/mcp/agent_config.rs | 56 +++++ crates/server/src/mcp/mod.rs | 1 + crates/server/src/routes/config.rs | 24 +- .../DisplayConversationEntry.tsx | 3 +- frontend/src/lib/mcp-strategies.ts | 233 ++++++++++++++++++ frontend/src/pages/McpServers.tsx | 146 +++-------- 9 files changed, 343 insertions(+), 135 deletions(-) create mode 100644 crates/server/src/mcp/agent_config.rs create mode 100644 frontend/src/lib/mcp-strategies.ts diff --git a/crates/executors/src/executors/mod.rs b/crates/executors/src/executors/mod.rs index e6d6a239..c7154413 100644 --- a/crates/executors/src/executors/mod.rs +++ b/crates/executors/src/executors/mod.rs @@ -123,7 +123,7 @@ impl BaseCodingAgent { Self::Amp => Some(vec!["amp", "mcpServers"]), // Nested path for Amp Self::Gemini => Some(vec!["mcpServers"]), //ExecutorConfig::Aider => None, // Aider doesn't support MCP. https://github.com/Aider-AI/aider/issues/3314 - Self::Codex => None, // Codex uses TOML config, frontend doesn't handle TOML yet + Self::Codex => Some(vec!["mcp_servers"]), // Codex uses TOML with mcp_servers } } diff --git a/crates/local-deployment/src/container.rs b/crates/local-deployment/src/container.rs index a883b0dc..404808d1 100644 --- a/crates/local-deployment/src/container.rs +++ b/crates/local-deployment/src/container.rs @@ -351,20 +351,20 @@ impl LocalContainerService { } // Fire event when CodingAgent execution has finished - if let Some(true) = config.read().await.analytics_enabled { - if matches!( + if let Some(true) = config.read().await.analytics_enabled + && matches!( &ctx.execution_process.run_reason, ExecutionProcessRunReason::CodingAgent - ) && let Some(analytics) = &analytics - { - analytics.analytics_service.track_event(&analytics.user_id, "task_attempt_finished", Some(json!({ + ) + && let Some(analytics) = &analytics + { + analytics.analytics_service.track_event(&analytics.user_id, "task_attempt_finished", Some(json!({ "task_id": ctx.task.id.to_string(), "project_id": ctx.task.project_id.to_string(), "attempt_id": ctx.task_attempt.id.to_string(), "execution_success": matches!(ctx.execution_process.status, ExecutionProcessStatus::Completed), "exit_code": ctx.execution_process.exit_code, }))); - } } } diff --git a/crates/server/Cargo.toml b/crates/server/Cargo.toml index 7a81688b..2e2de6b3 100644 --- a/crates/server/Cargo.toml +++ b/crates/server/Cargo.toml @@ -33,6 +33,7 @@ openssl-sys = { workspace = true } rmcp = { version = "0.2.1", features = ["server", "transport-io"] } schemars = "0.8" regex = "1.11.1" +toml = "0.8" sentry = { version = "0.41.0", features = ["anyhow", "backtrace", "panic", "debug-images"] } sentry-tracing = { version = "0.41.0", features = ["backtrace"] } reqwest = { version = "0.12", features = ["json"] } diff --git a/crates/server/src/mcp/agent_config.rs b/crates/server/src/mcp/agent_config.rs new file mode 100644 index 00000000..a92a55ad --- /dev/null +++ b/crates/server/src/mcp/agent_config.rs @@ -0,0 +1,56 @@ +//! Utilities for reading and writing external agent config files (not the server's own config). +//! +//! These helpers abstract over JSON vs TOML formats used by different agents. + +use executors::executors::BaseCodingAgent; +use serde_json::Value; +use tokio::fs; + +/// Determine if the agent's config file is TOML-based. +fn is_toml_config(agent: &BaseCodingAgent) -> bool { + matches!(agent, BaseCodingAgent::Codex) +} + +/// Read an agent's external config file (JSON or TOML) and normalize it to serde_json::Value. +pub async fn read_agent_config( + config_path: &std::path::Path, + agent: &BaseCodingAgent, +) -> Result> { + let file_content = fs::read_to_string(config_path).await.unwrap_or_else(|_| { + if is_toml_config(agent) { + "".to_string() + } else { + "{}".to_string() + } + }); + + if is_toml_config(agent) { + // Parse TOML then convert to JSON Value + if file_content.trim().is_empty() { + return Ok(serde_json::json!({})); + } + let toml_val: toml::Value = toml::from_str(&file_content)?; + let json_string = serde_json::to_string(&toml_val)?; + Ok(serde_json::from_str(&json_string)?) + } else { + Ok(serde_json::from_str(&file_content)?) + } +} + +/// Write an agent's external config (as serde_json::Value) back to disk in the agent's format (JSON or TOML). +pub async fn write_agent_config( + config_path: &std::path::Path, + agent: &BaseCodingAgent, + config: &Value, +) -> Result<(), Box> { + if is_toml_config(agent) { + // Convert JSON Value back to TOML + let toml_value: toml::Value = serde_json::from_str(&serde_json::to_string(config)?)?; + let toml_content = toml::to_string_pretty(&toml_value)?; + fs::write(config_path, toml_content).await?; + } else { + let json_content = serde_json::to_string_pretty(config)?; + fs::write(config_path, json_content).await?; + } + Ok(()) +} diff --git a/crates/server/src/mcp/mod.rs b/crates/server/src/mcp/mod.rs index 8420256b..14379252 100644 --- a/crates/server/src/mcp/mod.rs +++ b/crates/server/src/mcp/mod.rs @@ -1 +1,2 @@ +pub mod agent_config; pub mod task_server; diff --git a/crates/server/src/routes/config.rs b/crates/server/src/routes/config.rs index 4a553ea5..2779b386 100644 --- a/crates/server/src/routes/config.rs +++ b/crates/server/src/routes/config.rs @@ -17,7 +17,11 @@ use tokio::fs; use ts_rs::TS; use utils::{assets::config_path, response::ApiResponse}; -use crate::{error::ApiError, DeploymentImpl}; +use crate::{ + error::ApiError, + mcp::agent_config::{read_agent_config, write_agent_config}, + DeploymentImpl, +}; pub fn router() -> Router { Router::new() @@ -210,11 +214,8 @@ async fn update_mcp_servers_in_config( fs::create_dir_all(parent).await?; } - // Read existing config file or create empty object if it doesn't exist - let file_content = fs::read_to_string(config_path) - .await - .unwrap_or_else(|_| "{}".to_string()); - let mut config: Value = serde_json::from_str(&file_content)?; + // Read existing config (JSON or TOML depending on agent) + let mut config = read_agent_config(config_path, agent).await?; let mcp_path = agent.mcp_attribute_path().unwrap(); @@ -224,9 +225,8 @@ async fn update_mcp_servers_in_config( // Set the MCP servers using the correct attribute path set_mcp_servers_in_config_path(agent, &mut config, &mcp_path, &new_servers)?; - // Write the updated config back to file - let updated_content = serde_json::to_string_pretty(&config)?; - fs::write(config_path, updated_content).await?; + // Write the updated config back to file (JSON or TOML depending on agent) + write_agent_config(config_path, agent, &config).await?; let new_count = new_servers.len(); let message = match (old_servers, new_count) { @@ -246,10 +246,8 @@ async fn read_mcp_servers_from_config( config_path: &std::path::Path, agent: &BaseCodingAgent, ) -> Result, Box> { - let file_content = fs::read_to_string(config_path) - .await - .unwrap_or_else(|_| "{}".to_string()); - let raw_config: Value = serde_json::from_str(&file_content)?; + // Read config in appropriate format (JSON or TOML) and normalize to serde_json::Value + let raw_config = read_agent_config(config_path, agent).await?; let mcp_path = agent.mcp_attribute_path().unwrap(); let servers = get_mcp_servers_from_config_path(agent, &raw_config, &mcp_path); Ok(servers) diff --git a/frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx b/frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx index 29ff03ff..da0acfd9 100644 --- a/frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx +++ b/frontend/src/components/NormalizedConversation/DisplayConversationEntry.tsx @@ -146,7 +146,8 @@ const shouldRenderMarkdown = (entryType: NormalizedEntryType) => { entryType.tool_name.toLowerCase() === 'search' || entryType.tool_name.toLowerCase() === 'webfetch' || entryType.tool_name.toLowerCase() === 'web_fetch' || - entryType.tool_name.toLowerCase() === 'task')) + entryType.tool_name.toLowerCase() === 'task' || + entryType.tool_name.toLowerCase().startsWith('mcp_'))) ); }; diff --git a/frontend/src/lib/mcp-strategies.ts b/frontend/src/lib/mcp-strategies.ts new file mode 100644 index 00000000..73153a82 --- /dev/null +++ b/frontend/src/lib/mcp-strategies.ts @@ -0,0 +1,233 @@ +// Strategy pattern implementation for MCP server configuration handling +// across different base coding agents (Claude, Amp, Gemini, Opencode, Codex) + +import { BaseCodingAgent } from 'shared/types'; + +export interface McpConfigStrategy { + // Get the default empty configuration structure for this executor (as JSON string for textarea) + getDefaultConfig(): string; + + // Create the full configuration structure from servers data + createFullConfig(servers: Record): Record; + + // Validate the full configuration structure + validateFullConfig(config: Record): void; + + // Extract the servers object from the full configuration for API calls + extractServersForApi(fullConfig: Record): Record; + + // Create the vibe-kanban MCP server configuration for this executor + createVibeKanbanConfig(): Record; + + // Add vibe-kanban configuration to existing config + addVibeKanbanToConfig( + existingConfig: Record, + vibeKanbanConfig: Record + ): Record; +} + +/** + * Standard MCP configuration strategy for Claude, Gemini, etc. + * Uses JSON with top-level "mcpServers" + */ +export class StandardMcpStrategy implements McpConfigStrategy { + getDefaultConfig(): string { + return '{\n "mcpServers": {\n }\n}'; + } + + createFullConfig(servers: Record): Record { + return { mcpServers: servers }; + } + + validateFullConfig(config: Record): void { + if (!config.mcpServers || typeof config.mcpServers !== 'object') { + throw new Error('Configuration must contain an "mcpServers" object'); + } + } + + extractServersForApi(fullConfig: Record): Record { + return fullConfig.mcpServers; + } + + createVibeKanbanConfig(): Record { + return { + command: 'npx', + args: ['-y', 'vibe-kanban', '--mcp'], + }; + } + + addVibeKanbanToConfig( + existingConfig: Record, + vibeKanbanConfig: Record + ): Record { + return { + ...existingConfig, + mcpServers: { + ...(existingConfig.mcpServers || {}), + vibe_kanban: vibeKanbanConfig, + }, + }; + } +} + +/** + * AMP-specific MCP configuration strategy + * Uses flat key "amp.mcpServers" in JSON + */ +export class AmpMcpStrategy implements McpConfigStrategy { + getDefaultConfig(): string { + return '{\n "amp.mcpServers": {\n }\n}'; + } + + createFullConfig(servers: Record): Record { + return { 'amp.mcpServers': servers }; + } + + validateFullConfig(config: Record): void { + if ( + !config['amp.mcpServers'] || + typeof config['amp.mcpServers'] !== 'object' + ) { + throw new Error( + 'AMP configuration must contain an "amp.mcpServers" object' + ); + } + } + + extractServersForApi(fullConfig: Record): Record { + return fullConfig['amp.mcpServers']; + } + + createVibeKanbanConfig(): Record { + return { + command: 'npx', + args: ['-y', 'vibe-kanban', '--mcp'], + }; + } + + addVibeKanbanToConfig( + existingConfig: Record, + vibeKanbanConfig: Record + ): Record { + return { + ...existingConfig, + 'amp.mcpServers': { + ...(existingConfig['amp.mcpServers'] || {}), + vibe_kanban: vibeKanbanConfig, + }, + }; + } +} + +/** + * Opencode (SST Opencode)-specific MCP configuration strategy + * Uses JSON with top-level "mcp" plus $schema + */ +export class OpencodeMcpStrategy implements McpConfigStrategy { + getDefaultConfig(): string { + return '{\n "mcp": {\n }, "$schema": "https://opencode.ai/config.json"\n}'; + } + + createFullConfig(servers: Record): Record { + return { + mcp: servers, + $schema: 'https://opencode.ai/config.json', + }; + } + + validateFullConfig(config: Record): void { + if (!config.mcp || typeof config.mcp !== 'object') { + throw new Error('Configuration must contain an "mcp" object'); + } + } + + extractServersForApi(fullConfig: Record): Record { + return fullConfig.mcp; + } + + createVibeKanbanConfig(): Record { + return { + type: 'local', + command: ['npx', '-y', 'vibe-kanban', '--mcp'], + enabled: true, + }; + } + + addVibeKanbanToConfig( + existingConfig: Record, + vibeKanbanConfig: Record + ): Record { + return { + ...existingConfig, + mcp: { + ...(existingConfig.mcp || {}), + vibe_kanban: vibeKanbanConfig, + }, + }; + } +} + +/** + * Codex-specific MCP configuration strategy + * Frontend works with JSON using key "mcp_servers"; backend converts to TOML. + */ +export class CodexMcpStrategy implements McpConfigStrategy { + getDefaultConfig(): string { + // Although Codex uses TOML on disk, the frontend textarea is JSON. + return '{\n "mcp_servers": {\n }\n}'; + } + + createFullConfig(servers: Record): Record { + return { mcp_servers: servers }; + } + + validateFullConfig(config: Record): void { + if (!config.mcp_servers || typeof config.mcp_servers !== 'object') { + throw new Error('Configuration must contain an "mcp_servers" object'); + } + } + + extractServersForApi(fullConfig: Record): Record { + return fullConfig.mcp_servers; + } + + createVibeKanbanConfig(): Record { + return { + command: 'npx', + args: ['-y', 'vibe-kanban', '--mcp'], + }; + } + + addVibeKanbanToConfig( + existingConfig: Record, + vibeKanbanConfig: Record + ): Record { + return { + ...existingConfig, + mcp_servers: { + ...(existingConfig.mcp_servers || {}), + vibe_kanban: vibeKanbanConfig, + }, + }; + } +} + +/** + * Factory to get the appropriate MCP strategy for a BaseCodingAgent + */ +export function getMcpStrategyByAgent( + agent: BaseCodingAgent +): McpConfigStrategy { + switch (agent) { + case BaseCodingAgent.AMP: + return new AmpMcpStrategy(); + case BaseCodingAgent.OPENCODE: + return new OpencodeMcpStrategy(); + case BaseCodingAgent.CODEX: + return new CodexMcpStrategy(); + case BaseCodingAgent.CLAUDE_CODE: + case BaseCodingAgent.GEMINI: + default: + return new StandardMcpStrategy(); + } +} diff --git a/frontend/src/pages/McpServers.tsx b/frontend/src/pages/McpServers.tsx index e255991f..bc388144 100644 --- a/frontend/src/pages/McpServers.tsx +++ b/frontend/src/pages/McpServers.tsx @@ -18,9 +18,10 @@ import { Label } from '@/components/ui/label'; import { Alert, AlertDescription } from '@/components/ui/alert'; import { Textarea } from '@/components/ui/textarea'; import { Loader2 } from 'lucide-react'; -import { BaseCodingAgent, AgentProfile } from 'shared/types'; +import { AgentProfile } from 'shared/types'; import { useUserSystem } from '@/components/config-provider'; import { mcpServersApi } from '../lib/api'; +import { getMcpStrategyByAgent } from '../lib/mcp-strategies'; export function McpServers() { const { config, profiles } = useUserSystem(); @@ -55,13 +56,9 @@ export function McpServers() { setMcpLoading(true); setMcpError(null); - // Set default empty config based on agent type - const defaultConfig = - profile.agent === BaseCodingAgent.AMP - ? '{\n "amp.mcpServers": {\n }\n}' - : profile.agent === BaseCodingAgent.OPENCODE - ? '{\n "mcp": {\n }, "$schema": "https://opencode.ai/config.json"\n}' - : '{\n "mcpServers": {\n }\n}'; + // Set default empty config based on agent type using strategy + const strategy = getMcpStrategyByAgent(profile.agent); + const defaultConfig = strategy.getDefaultConfig(); setMcpServers(defaultConfig); setMcpConfigPath(''); @@ -73,21 +70,9 @@ export function McpServers() { const servers = data.servers || {}; const configPath = data.config_path || ''; - // Create the full configuration structure based on agent type - let fullConfig; - if (profile.agent === BaseCodingAgent.AMP) { - // For AMP, use the amp.mcpServers structure - fullConfig = { 'amp.mcpServers': servers }; - } else if (profile.agent === BaseCodingAgent.OPENCODE) { - fullConfig = { - mcp: servers, - $schema: 'https://opencode.ai/config.json', - }; - } else { - // For other executors, use the standard mcpServers structure - fullConfig = { mcpServers: servers }; - } - + // Create the full configuration structure using strategy + const strategy = getMcpStrategyByAgent(profile.agent); + const fullConfig = strategy.createFullConfig(servers); const configJson = JSON.stringify(fullConfig, null, 2); setMcpServers(configJson); setMcpConfigPath(configPath); @@ -116,27 +101,15 @@ export function McpServers() { if (value.trim() && selectedProfile) { try { const config = JSON.parse(value); - // Validate that the config has the expected structure based on agent type - if (selectedProfile.agent === BaseCodingAgent.AMP) { - if ( - !config['amp.mcpServers'] || - typeof config['amp.mcpServers'] !== 'object' - ) { - setMcpError( - 'AMP configuration must contain an "amp.mcpServers" object' - ); - } - } else if (selectedProfile.agent === BaseCodingAgent.OPENCODE) { - if (!config.mcp || typeof config.mcp !== 'object') { - setMcpError('Configuration must contain an "mcp" object'); - } - } else { - if (!config.mcpServers || typeof config.mcpServers !== 'object') { - setMcpError('Configuration must contain an "mcpServers" object'); - } - } + // Validate that the config has the expected structure using strategy + const strategy = getMcpStrategyByAgent(selectedProfile.agent); + strategy.validateFullConfig(config); } catch (err) { - setMcpError('Invalid JSON format'); + if (err instanceof SyntaxError) { + setMcpError('Invalid JSON format'); + } else { + setMcpError(err instanceof Error ? err.message : 'Validation error'); + } } } }; @@ -148,46 +121,15 @@ export function McpServers() { // Parse existing configuration const existingConfig = mcpServers.trim() ? JSON.parse(mcpServers) : {}; - // Always use production MCP installation instructions - const vibeKanbanConfig = - selectedProfile.agent === BaseCodingAgent.OPENCODE - ? { - type: 'local', - command: ['npx', '-y', 'vibe-kanban', '--mcp'], - enabled: true, - } - : { - command: 'npx', - args: ['-y', 'vibe-kanban', '--mcp'], - }; + // Use strategy to create vibe-kanban configuration + const strategy = getMcpStrategyByAgent(selectedProfile.agent); + const vibeKanbanConfig = strategy.createVibeKanbanConfig(); - // Add vibe_kanban to the existing configuration - let updatedConfig; - if (selectedProfile.agent === BaseCodingAgent.AMP) { - updatedConfig = { - ...existingConfig, - 'amp.mcpServers': { - ...(existingConfig['amp.mcpServers'] || {}), - vibe_kanban: vibeKanbanConfig, - }, - }; - } else if (selectedProfile.agent === BaseCodingAgent.OPENCODE) { - updatedConfig = { - ...existingConfig, - mcp: { - ...(existingConfig.mcp || {}), - vibe_kanban: vibeKanbanConfig, - }, - }; - } else { - updatedConfig = { - ...existingConfig, - mcpServers: { - ...(existingConfig.mcpServers || {}), - vibe_kanban: vibeKanbanConfig, - }, - }; - } + // Add vibe_kanban to the existing configuration using strategy + const updatedConfig = strategy.addVibeKanbanToConfig( + existingConfig, + vibeKanbanConfig + ); // Update the textarea with the new configuration const configJson = JSON.stringify(updatedConfig, null, 2); @@ -211,37 +153,12 @@ export function McpServers() { try { const fullConfig = JSON.parse(mcpServers); - // Validate that the config has the expected structure based on agent type - let mcpServersConfig; - if (selectedProfile.agent === BaseCodingAgent.AMP) { - if ( - !fullConfig['amp.mcpServers'] || - typeof fullConfig['amp.mcpServers'] !== 'object' - ) { - throw new Error( - 'AMP configuration must contain an "amp.mcpServers" object' - ); - } - // Extract just the inner servers object for the API - backend will handle nesting - mcpServersConfig = fullConfig['amp.mcpServers']; - } else if (selectedProfile.agent === BaseCodingAgent.OPENCODE) { - if (!fullConfig.mcp || typeof fullConfig.mcp !== 'object') { - throw new Error('Configuration must contain an "mcp" object'); - } - // Extract just the mcp part for the API - mcpServersConfig = fullConfig.mcp; - } else { - if ( - !fullConfig.mcpServers || - typeof fullConfig.mcpServers !== 'object' - ) { - throw new Error( - 'Configuration must contain an "mcpServers" object' - ); - } - // Extract just the mcpServers part for the API - mcpServersConfig = fullConfig.mcpServers; - } + // Use strategy to validate and extract servers config + const strategy = getMcpStrategyByAgent(selectedProfile.agent); + strategy.validateFullConfig(fullConfig); + + // Extract just the servers object for the API - backend will handle nesting/format + const mcpServersConfig = strategy.extractServersForApi(fullConfig); await mcpServersApi.save(selectedProfile.agent, mcpServersConfig); @@ -349,7 +266,8 @@ export function McpServers() {

{mcpError}

To use MCP servers, please select a different profile - (Claude, Amp, or Gemini) above. + that supports MCP (Claude, Amp, Gemini, Codex, or + Opencode) above.