diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 62a0313854..844a027762 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,6 +47,10 @@ jobs: - name: Rebuild lock files run: make recompile + - name: Check for changes in generated files + run: | + git diff .github/workflows/*.lock.yml + - name: Check for uncommitted workflow changes run: | if [ -n "$(git status --porcelain .github/workflows/*.yml)" ]; then diff --git a/.github/workflows/test-codex.lock.yml b/.github/workflows/test-codex.lock.yml index 096a240ded..494d003c11 100644 --- a/.github/workflows/test-codex.lock.yml +++ b/.github/workflows/test-codex.lock.yml @@ -75,6 +75,8 @@ jobs: run: | mkdir -p /tmp/mcp-config cat > /tmp/mcp-config/config.toml << EOF + [history] + persistence = "none" [mcp_servers.github] command = "docker" @@ -87,6 +89,18 @@ jobs: "ghcr.io/github/github-mcp-server:sha-45e90ae" ] env = { "GITHUB_PERSONAL_ACCESS_TOKEN" = "${{ secrets.GITHUB_TOKEN }}" } + + [mcp_servers.time] + command = "docker" + args = [ + "run", + "--rm", + "-i", + "-e", + "LOCAL_TIMEZONE", + "mcp/time", + ] + env = { "LOCAL_TIMEZONE" = "${LOCAL_TIMEZONE}" } EOF - name: Create prompt run: | @@ -98,6 +112,8 @@ jobs: You are a code review assistant powered by Codex. Your task is to analyze the changes in this pull request and provide a comprehensive summary. + **First, get the current time using the get_current_time tool to timestamp your analysis.** + ### Analysis Tasks 1. **Review the Pull Request Details** @@ -118,6 +134,7 @@ jobs: **Branch:** `${{ github.head_ref }}` **Files Changed:** [number] files + **Analysis Time:** [current timestamp from get_current_time] #### 📋 Change Overview - Brief description of what this PR accomplishes @@ -242,8 +259,8 @@ jobs: -c model=o4-mini \ --full-auto "$INSTRUCTION" 2>&1 | tee /tmp/test-codex.log env: - OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} GITHUB_STEP_SUMMARY: ${{ env.GITHUB_STEP_SUMMARY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} - name: Check if workflow-complete.txt exists, if so upload it id: check_file run: | diff --git a/.github/workflows/test-codex.md b/.github/workflows/test-codex.md index a5c2ddec70..fcc14e8d8d 100644 --- a/.github/workflows/test-codex.md +++ b/.github/workflows/test-codex.md @@ -13,6 +13,14 @@ permissions: pull-requests: write issues: read actions: read +tools: + time: + mcp: + type: stdio + container: "mcp/time" + env: + LOCAL_TIMEZONE: "${LOCAL_TIMEZONE}" + allowed: ["get_current_time"] --- # Test Codex @@ -21,6 +29,8 @@ permissions: You are a code review assistant powered by Codex. Your task is to analyze the changes in this pull request and provide a comprehensive summary. +**First, get the current time using the get_current_time tool to timestamp your analysis.** + ### Analysis Tasks 1. **Review the Pull Request Details** @@ -41,6 +51,7 @@ You are a code review assistant powered by Codex. Your task is to analyze the ch **Branch:** `${{ github.head_ref }}` **Files Changed:** [number] files + **Analysis Time:** [current timestamp from get_current_time] #### 📋 Change Overview - Brief description of what this PR accomplishes diff --git a/pkg/cli/commands.go b/pkg/cli/commands.go index 6e4eb3e19c..682b9e1da9 100644 --- a/pkg/cli/commands.go +++ b/pkg/cli/commands.go @@ -157,11 +157,11 @@ func listAgenticEngines(verbose bool) error { fmt.Println(console.FormatListHeader("========================")) if verbose { - fmt.Printf("%-15s %-20s %-12s %-8s %-8s %s\n", "ID", "Display Name", "Status", "MCP", "Node.js", "Description") - fmt.Printf("%-15s %-20s %-12s %-8s %-8s %s\n", "--", "------------", "------", "---", "-------", "-----------") + fmt.Printf("%-15s %-20s %-12s %-8s %-12s %s\n", "ID", "Display Name", "Status", "MCP", "HTTP Transport", "Description") + fmt.Printf("%-15s %-20s %-12s %-8s %-12s %s\n", "--", "------------", "------", "---", "-------------", "-----------") } else { - fmt.Printf("%-15s %-20s %-12s %-8s %-8s\n", "ID", "Display Name", "Status", "MCP", "Node.js") - fmt.Printf("%-15s %-20s %-12s %-8s %-8s\n", "--", "------------", "------", "---", "-------") + fmt.Printf("%-15s %-20s %-12s %-8s %-12s\n", "ID", "Display Name", "Status", "MCP", "HTTP Transport") + fmt.Printf("%-15s %-20s %-12s %-8s %-12s\n", "--", "------------", "------", "---", "-------------") } for _, engineID := range engines { @@ -185,20 +185,28 @@ func listAgenticEngines(verbose bool) error { mcpSupport = "Yes" } + // HTTP transport support + httpTransport := "No" + if engine.SupportsHTTPTransport() { + httpTransport = "Yes" + } + if verbose { - fmt.Printf("%-15s %-20s %-12s %-8s %s\n", + fmt.Printf("%-15s %-20s %-12s %-8s %-12s %s\n", engine.GetID(), engine.GetDisplayName(), status, mcpSupport, + httpTransport, engine.GetDescription()) } else { - fmt.Printf("%-15s %-20s %-12s %-8s\n", + fmt.Printf("%-15s %-20s %-12s %-8s %-12s\n", engine.GetID(), engine.GetDisplayName(), status, - mcpSupport) + mcpSupport, + httpTransport) } } diff --git a/pkg/workflow/agentic_engine.go b/pkg/workflow/agentic_engine.go index 5021cc3eaa..76646c4b4d 100644 --- a/pkg/workflow/agentic_engine.go +++ b/pkg/workflow/agentic_engine.go @@ -25,6 +25,9 @@ type AgenticEngine interface { // SupportsToolsWhitelist returns true if this engine supports MCP tool allow-listing SupportsToolsWhitelist() bool + // SupportsHTTPTransport returns true if this engine supports HTTP transport for MCP servers + SupportsHTTPTransport() bool + // GetInstallationSteps returns the GitHub Actions steps needed to install this engine GetInstallationSteps(engineConfig *EngineConfig) []GitHubActionStep @@ -60,6 +63,7 @@ type BaseEngine struct { description string experimental bool supportsToolsWhitelist bool + supportsHTTPTransport bool } func (e *BaseEngine) GetID() string { @@ -82,6 +86,10 @@ func (e *BaseEngine) SupportsToolsWhitelist() bool { return e.supportsToolsWhitelist } +func (e *BaseEngine) SupportsHTTPTransport() bool { + return e.supportsHTTPTransport +} + // EngineRegistry manages available agentic engines type EngineRegistry struct { engines map[string]AgenticEngine diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index a31114cb04..a7802e29e0 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -18,6 +18,7 @@ func NewClaudeEngine() *ClaudeEngine { description: "Uses Claude Code with full MCP tool support and allow-listing", experimental: false, supportsToolsWhitelist: true, + supportsHTTPTransport: true, // Claude supports both stdio and HTTP transport }, } } diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index a639f35248..faf30b562c 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -15,9 +15,10 @@ func NewCodexEngine() *CodexEngine { BaseEngine: BaseEngine{ id: "codex", displayName: "Codex", - description: "Uses OpenAI Codex CLI (experimental)", + description: "Uses OpenAI Codex CLI with MCP server support", experimental: true, - supportsToolsWhitelist: false, + supportsToolsWhitelist: true, + supportsHTTPTransport: false, // Codex only supports stdio transport }, } } @@ -74,6 +75,10 @@ codex exec \ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string) { yaml.WriteString(" cat > /tmp/mcp-config/config.toml << EOF\n") + // Add history configuration to disable persistence + yaml.WriteString(" [history]\n") + yaml.WriteString(" persistence = \"none\"\n") + // Generate [mcp_servers] section for _, toolName := range mcpTools { switch toolName { diff --git a/pkg/workflow/codex_engine_test.go b/pkg/workflow/codex_engine_test.go index 709ae9a92c..462b037a41 100644 --- a/pkg/workflow/codex_engine_test.go +++ b/pkg/workflow/codex_engine_test.go @@ -21,8 +21,8 @@ func TestCodexEngine(t *testing.T) { t.Error("Codex engine should be experimental") } - if engine.SupportsToolsWhitelist() { - t.Error("Codex engine should not support MCP tools") + if !engine.SupportsToolsWhitelist() { + t.Error("Codex engine should support MCP tools") } // Test installation steps diff --git a/pkg/workflow/codex_test.go b/pkg/workflow/codex_test.go index 3129bb7666..45ac0b3f77 100644 --- a/pkg/workflow/codex_test.go +++ b/pkg/workflow/codex_test.go @@ -139,6 +139,13 @@ This is a test workflow. if !strings.Contains(lockContent, "[mcp_servers.github]") { t.Errorf("Expected lock file to contain '[mcp_servers.github]' section in config.toml but it didn't.\nContent:\n%s", lockContent) } + // Check that history configuration is present + if !strings.Contains(lockContent, "[history]") { + t.Errorf("Expected lock file to contain '[history]' section in config.toml but it didn't.\nContent:\n%s", lockContent) + } + if !strings.Contains(lockContent, "persistence = \"none\"") { + t.Errorf("Expected lock file to contain 'persistence = \"none\"' in config.toml but it didn't.\nContent:\n%s", lockContent) + } // Ensure it does NOT contain mcp-servers.json if strings.Contains(lockContent, "mcp-servers.json") { t.Errorf("Expected lock file to NOT contain 'mcp-servers.json' when using codex.\nContent:\n%s", lockContent) @@ -289,6 +296,27 @@ tools: expectMcpServersJson: true, expectCodexHome: false, }, + { + name: "codex with custom MCP tools generates config.toml", + frontmatter: `--- +engine: codex +tools: + github: + allowed: [get_issue, create_issue] + custom-server: + mcp: + type: stdio + command: "python" + args: ["-m", "my_server"] + env: + API_KEY: "${{ secrets.API_KEY }}" + allowed: ["*"] +---`, + expectedAI: "codex", + expectConfigToml: true, + expectMcpServersJson: false, + expectCodexHome: true, + }, } for _, tt := range tests { @@ -332,6 +360,18 @@ This is a test workflow for MCP configuration with different AI engines. if !strings.Contains(lockContent, "command = \"docker\"") { t.Errorf("Expected docker command in config.toml but didn't find it in:\n%s", lockContent) } + // Check for custom MCP server if test includes it + if strings.Contains(tt.name, "custom MCP") { + if !strings.Contains(lockContent, "[mcp_servers.custom-server]") { + t.Errorf("Expected [mcp_servers.custom-server] section but didn't find it in:\n%s", lockContent) + } + if !strings.Contains(lockContent, "command = \"python\"") { + t.Errorf("Expected python command for custom server but didn't find it in:\n%s", lockContent) + } + if !strings.Contains(lockContent, "\"API_KEY\" = \"${{ secrets.API_KEY }}\"") { + t.Errorf("Expected API_KEY env var for custom server but didn't find it in:\n%s", lockContent) + } + } // Should NOT have services section (services mode removed) if strings.Contains(lockContent, "services:") { t.Errorf("Expected NO services section in workflow but found it in:\n%s", lockContent) diff --git a/pkg/workflow/compiler.go b/pkg/workflow/compiler.go index c457b01794..6b853967ce 100644 --- a/pkg/workflow/compiler.go +++ b/pkg/workflow/compiler.go @@ -481,6 +481,11 @@ func (c *Compiler) parseWorkflowFile(markdownPath string) (*WorkflowData, error) return nil, fmt.Errorf("invalid MCP configuration: %w", err) } + // Validate HTTP transport support for the current engine + if err := c.validateHTTPTransportSupport(tools, agenticEngine); err != nil { + return nil, fmt.Errorf("HTTP transport not supported: %w", err) + } + // Apply default GitHub MCP tools (only for engines that support MCP) if agenticEngine.SupportsToolsWhitelist() { tools = c.applyDefaultGitHubMCPTools(tools) @@ -1928,7 +1933,15 @@ func (c *Compiler) generateEngineExecutionSteps(yaml *strings.Builder, data *Wor // Add environment variables if len(executionConfig.Environment) > 0 { yaml.WriteString(" env:\n") - for key, value := range executionConfig.Environment { + // Sort environment keys for consistent output + envKeys := make([]string, 0, len(executionConfig.Environment)) + for key := range executionConfig.Environment { + envKeys = append(envKeys, key) + } + sort.Strings(envKeys) + + for _, key := range envKeys { + value := executionConfig.Environment[key] yaml.WriteString(fmt.Sprintf(" %s: %s\n", key, value)) } } @@ -1978,3 +1991,22 @@ func (c *Compiler) generateEngineExecutionSteps(yaml *strings.Builder, data *Wor yaml.WriteString(" touch " + logFile + "\n") } } + +// validateHTTPTransportSupport validates that HTTP MCP servers are only used with engines that support HTTP transport +func (c *Compiler) validateHTTPTransportSupport(tools map[string]any, engine AgenticEngine) error { + if engine.SupportsHTTPTransport() { + // Engine supports HTTP transport, no validation needed + return nil + } + + // Engine doesn't support HTTP transport, check for HTTP MCP servers + for toolName, toolConfig := range tools { + if config, ok := toolConfig.(map[string]any); ok { + if hasMcp, mcpType := hasMCPConfig(config); hasMcp && mcpType == "http" { + return fmt.Errorf("tool '%s' uses HTTP transport which is not supported by engine '%s' (only stdio transport is supported)", toolName, engine.GetID()) + } + } + } + + return nil +} diff --git a/pkg/workflow/gemini_engine.go b/pkg/workflow/gemini_engine.go index 658082cbae..d0428a2777 100644 --- a/pkg/workflow/gemini_engine.go +++ b/pkg/workflow/gemini_engine.go @@ -17,6 +17,7 @@ func NewGeminiEngine() *GeminiEngine { description: "Uses Google Gemini CLI with GitHub integration and tool support", experimental: false, supportsToolsWhitelist: true, + supportsHTTPTransport: false, // Gemini CLI does not support custom MCP HTTP servers }, } }