diff --git a/.github/workflows/ci-doctor.lock.yml b/.github/workflows/ci-doctor.lock.yml index 903c41a021..182ff9226c 100644 --- a/.github/workflows/ci-doctor.lock.yml +++ b/.github/workflows/ci-doctor.lock.yml @@ -166,7 +166,7 @@ jobs: writeMessage(res); } function isToolEnabled(name) { - return safeOutputsConfig[name] && safeOutputsConfig[name].enabled; + return safeOutputsConfig[name]; } function appendSafeOutput(entry) { if (!outputFile) throw new Error("No output file configured"); diff --git a/.github/workflows/dev.lock.yml b/.github/workflows/dev.lock.yml index d59ba61057..1fad30d46f 100644 --- a/.github/workflows/dev.lock.yml +++ b/.github/workflows/dev.lock.yml @@ -345,7 +345,7 @@ jobs: writeMessage(res); } function isToolEnabled(name) { - return safeOutputsConfig[name] && safeOutputsConfig[name].enabled; + return safeOutputsConfig[name]; } function appendSafeOutput(entry) { if (!outputFile) throw new Error("No output file configured"); diff --git a/docs/src/content/docs/index.mdx b/docs/src/content/docs/index.mdx index 4a859c56e1..69d2377f15 100644 --- a/docs/src/content/docs/index.mdx +++ b/docs/src/content/docs/index.mdx @@ -1,9 +1,9 @@ --- title: GitHub Agentic Workflows -description: AI-powered repository automation, run safely in GitHub Actions +description: AI-powered repository automation, run safely on GitHub Actions template: splash hero: - tagline: AI-powered repository automation, run safely in GitHub Actions + tagline: AI-powered repository automation, run safely on GitHub Actions image: file: ../../assets/houston.webp actions: diff --git a/pkg/workflow/claude_engine.go b/pkg/workflow/claude_engine.go index 94541440d0..577f89dbf5 100644 --- a/pkg/workflow/claude_engine.go +++ b/pkg/workflow/claude_engine.go @@ -560,19 +560,7 @@ func (e *ClaudeEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]a case "cache-memory": e.renderCacheMemoryMCPConfig(yaml, isLast, workflowData) case "safe-outputs": - yaml.WriteString(" \"safe_outputs\": {\n") - yaml.WriteString(" \"command\": \"node\",\n") - yaml.WriteString(" \"args\": [\"/tmp/safe-outputs/mcp-server.cjs\"],\n") - yaml.WriteString(" \"env\": {\n") - yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS\": \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\",\n") - yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\": ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }}\n") - yaml.WriteString(" }\n") - serverCount++ - if serverCount < totalServers { - yaml.WriteString(" },\n") - } else { - yaml.WriteString(" }\n") - } + e.renderSafeOutputsMCPConfig(yaml, isLast) default: // Handle custom MCP tools (those with MCP-compatible type) if toolConfig, ok := tools[toolName].(map[string]any); ok { @@ -683,6 +671,23 @@ func (e *ClaudeEngine) renderCacheMemoryMCPConfig(yaml *strings.Builder, isLast } } +// renderSafeOutputsMCPConfig generates the Safe Outputs MCP server configuration +func (e *ClaudeEngine) renderSafeOutputsMCPConfig(yaml *strings.Builder, isLast bool) { + yaml.WriteString(" \"safe_outputs\": {\n") + yaml.WriteString(" \"command\": \"node\",\n") + yaml.WriteString(" \"args\": [\"/tmp/safe-outputs/mcp-server.cjs\"],\n") + yaml.WriteString(" \"env\": {\n") + yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS\": \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\",\n") + yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\": ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }}\n") + yaml.WriteString(" }\n") + + if isLast { + yaml.WriteString(" }\n") + } else { + yaml.WriteString(" },\n") + } +} + // ParseLogMetrics implements engine-specific log parsing for Claude func (e *ClaudeEngine) ParseLogMetrics(logContent string, verbose bool) LogMetrics { var metrics LogMetrics diff --git a/pkg/workflow/codex_engine.go b/pkg/workflow/codex_engine.go index e61026fb6b..844b76206c 100644 --- a/pkg/workflow/codex_engine.go +++ b/pkg/workflow/codex_engine.go @@ -88,10 +88,16 @@ func (e *CodexEngine) GetExecutionSteps(workflowData *WorkflowData, logFile stri } } - // Use model from engineConfig if available, otherwise default to o4-mini - model := "o4-mini" + // Build model parameter only if specified in engineConfig + var modelParam string if workflowData.EngineConfig != nil && workflowData.EngineConfig.Model != "" { - model = workflowData.EngineConfig.Model + modelParam = fmt.Sprintf("-c model=%s ", workflowData.EngineConfig.Model) + } + + // Build search parameter if web-search tool is present + var searchParam string + if _, hasWebSearch := workflowData.Tools["web-search"]; hasWebSearch { + searchParam = "--search " } command := fmt.Sprintf(`set -o pipefail @@ -102,9 +108,7 @@ export CODEX_HOME=/tmp/mcp-config mkdir -p /tmp/aw-logs # Run codex with log capture - pipefail ensures codex exit code is preserved -codex exec \ - -c model=%s \ - --full-auto "$INSTRUCTION" 2>&1 | tee %s`, model, logFile) +codex exec %s%s--full-auto "$INSTRUCTION" 2>&1 | tee %s`, modelParam, searchParam, logFile) env := map[string]string{ "OPENAI_API_KEY": "${{ secrets.OPENAI_API_KEY }}", @@ -220,17 +224,7 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an playwrightTool := expandedTools["playwright"] e.renderPlaywrightCodexMCPConfig(yaml, playwrightTool, workflowData.NetworkPermissions) case "safe-outputs": - // Add safe-outputs MCP server if safe-outputs are configured - hasSafeOutputs := workflowData != nil && workflowData.SafeOutputs != nil && HasSafeOutputsEnabled(workflowData.SafeOutputs) - if hasSafeOutputs { - yaml.WriteString(" \n") - yaml.WriteString(" [mcp_servers.safe_outputs]\n") - yaml.WriteString(" command = \"node\"\n") - yaml.WriteString(" args = [\n") - yaml.WriteString(" \"/tmp/safe-outputs/mcp-server.cjs\",\n") - yaml.WriteString(" ]\n") - yaml.WriteString(" env = { \"GITHUB_AW_SAFE_OUTPUTS\" = \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\", \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\" = ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }} }\n") - } + e.renderSafeOutputsCodexMCPConfig(yaml, workflowData) default: // Handle custom MCP tools (those with MCP-compatible type) if toolConfig, ok := expandedTools[toolName].(map[string]any); ok { @@ -501,6 +495,21 @@ func (e *CodexEngine) renderCodexMCPConfig(yaml *strings.Builder, toolName strin return nil } +// renderSafeOutputsCodexMCPConfig generates the Safe Outputs MCP server configuration for codex config.toml +func (e *CodexEngine) renderSafeOutputsCodexMCPConfig(yaml *strings.Builder, workflowData *WorkflowData) { + // Add safe-outputs MCP server if safe-outputs are configured + hasSafeOutputs := workflowData != nil && workflowData.SafeOutputs != nil && HasSafeOutputsEnabled(workflowData.SafeOutputs) + if hasSafeOutputs { + yaml.WriteString(" \n") + yaml.WriteString(" [mcp_servers.safe_outputs]\n") + yaml.WriteString(" command = \"node\"\n") + yaml.WriteString(" args = [\n") + yaml.WriteString(" \"/tmp/safe-outputs/mcp-server.cjs\",\n") + yaml.WriteString(" ]\n") + yaml.WriteString(" env = { \"GITHUB_AW_SAFE_OUTPUTS\" = \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\", \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\" = ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }} }\n") + } +} + // GetLogParserScriptId returns the JavaScript script name for parsing Codex logs func (e *CodexEngine) GetLogParserScriptId() string { return "parse_codex_log" diff --git a/pkg/workflow/codex_test.go b/pkg/workflow/codex_test.go index ce09e06496..8e1228ffee 100644 --- a/pkg/workflow/codex_test.go +++ b/pkg/workflow/codex_test.go @@ -121,10 +121,6 @@ This is a test workflow. if !strings.Contains(lockContent, "codex exec") { t.Errorf("Expected lock file to contain 'codex exec' command but it didn't.\nContent:\n%s", lockContent) } - // Check for correct model based on AI setting - if !strings.Contains(lockContent, "model=o4-mini") { - t.Errorf("Expected lock file to contain 'model=o4-mini' for codex but it didn't.\nContent:\n%s", lockContent) - } if !strings.Contains(lockContent, "OPENAI_API_KEY") { t.Errorf("Expected lock file to contain 'OPENAI_API_KEY' for codex but it didn't.\nContent:\n%s", lockContent) } diff --git a/pkg/workflow/custom_engine.go b/pkg/workflow/custom_engine.go index b956e65ca8..82bf621ef4 100644 --- a/pkg/workflow/custom_engine.go +++ b/pkg/workflow/custom_engine.go @@ -151,19 +151,7 @@ func (e *CustomEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]a case "cache-memory": e.renderCacheMemoryMCPConfig(yaml, isLast, workflowData) case "safe-outputs": - yaml.WriteString(" \"safe_outputs\": {\n") - yaml.WriteString(" \"command\": \"node\",\n") - yaml.WriteString(" \"args\": [\"/tmp/safe-outputs/mcp-server.cjs\"],\n") - yaml.WriteString(" \"env\": {\n") - yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS\": \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\",\n") - yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\": ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }}\n") - yaml.WriteString(" }\n") - serverCount++ - if serverCount < totalServers { - yaml.WriteString(" },\n") - } else { - yaml.WriteString(" }\n") - } + e.renderSafeOutputsMCPConfig(yaml, isLast) default: // Handle custom MCP tools (those with MCP-compatible type) if toolConfig, ok := tools[toolName].(map[string]any); ok { @@ -284,6 +272,23 @@ func (e *CustomEngine) renderCacheMemoryMCPConfig(yaml *strings.Builder, isLast } } +// renderSafeOutputsMCPConfig generates the Safe Outputs MCP server configuration +func (e *CustomEngine) renderSafeOutputsMCPConfig(yaml *strings.Builder, isLast bool) { + yaml.WriteString(" \"safe_outputs\": {\n") + yaml.WriteString(" \"command\": \"node\",\n") + yaml.WriteString(" \"args\": [\"/tmp/safe-outputs/mcp-server.cjs\"],\n") + yaml.WriteString(" \"env\": {\n") + yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS\": \"${{ env.GITHUB_AW_SAFE_OUTPUTS }}\",\n") + yaml.WriteString(" \"GITHUB_AW_SAFE_OUTPUTS_CONFIG\": ${{ toJSON(env.GITHUB_AW_SAFE_OUTPUTS_CONFIG) }}\n") + yaml.WriteString(" }\n") + + if isLast { + yaml.WriteString(" }\n") + } else { + yaml.WriteString(" },\n") + } +} + // ParseLogMetrics implements basic log parsing for custom engine // For custom engines, try both Claude and Codex parsing approaches to extract turn information func (e *CustomEngine) ParseLogMetrics(logContent string, verbose bool) LogMetrics { diff --git a/pkg/workflow/js/safe_outputs_mcp_server.cjs b/pkg/workflow/js/safe_outputs_mcp_server.cjs index 926cd3ab22..9c6c9db700 100644 --- a/pkg/workflow/js/safe_outputs_mcp_server.cjs +++ b/pkg/workflow/js/safe_outputs_mcp_server.cjs @@ -99,7 +99,7 @@ function replyError(id, code, message, data) { } function isToolEnabled(name) { - return safeOutputsConfig[name] && safeOutputsConfig[name].enabled; + return safeOutputsConfig[name]; } function appendSafeOutput(entry) { diff --git a/pkg/workflow/mcp-config.go b/pkg/workflow/mcp-config.go index 76b4fb50c1..6f7e27317e 100644 --- a/pkg/workflow/mcp-config.go +++ b/pkg/workflow/mcp-config.go @@ -73,7 +73,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma // Render properties based on format for propIndex, property := range existingProperties { - isLastProperty := propIndex == len(existingProperties)-1 + isLast := propIndex == len(existingProperties)-1 switch property { case "command": @@ -83,7 +83,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma fmt.Fprintf(yaml, "%scommand = \"%s\"\n", renderer.IndentLevel, cmdStr) } else { comma := "," - if isLastProperty { + if isLast { comma = "" } fmt.Fprintf(yaml, "%s\"command\": \"%s\"%s\n", renderer.IndentLevel, cmdStr, comma) @@ -103,7 +103,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma fmt.Fprintf(yaml, "%s]\n", renderer.IndentLevel) } else { comma := "," - if isLastProperty { + if isLast { comma = "" } fmt.Fprintf(yaml, "%s\"args\": [\n", renderer.IndentLevel) @@ -138,7 +138,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma yaml.WriteString(" }\n") } else { comma := "," - if isLastProperty { + if isLast { comma = "" } fmt.Fprintf(yaml, "%s\"env\": {\n", renderer.IndentLevel) @@ -163,7 +163,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma if url, exists := mcpConfig["url"]; exists { if urlStr, ok := url.(string); ok { comma := "," - if isLastProperty { + if isLast { comma = "" } fmt.Fprintf(yaml, "%s\"url\": \"%s\"%s\n", renderer.IndentLevel, urlStr, comma) @@ -173,7 +173,7 @@ func renderSharedMCPConfig(yaml *strings.Builder, toolName string, toolConfig ma if headers, exists := mcpConfig["headers"]; exists { if headersMap, ok := headers.(map[string]any); ok { comma := "," - if isLastProperty { + if isLast { comma = "" } fmt.Fprintf(yaml, "%s\"headers\": {\n", renderer.IndentLevel) diff --git a/pkg/workflow/safe_outputs_mcp_server_test.go b/pkg/workflow/safe_outputs_mcp_server_test.go index 33c0fd33e9..f73d903775 100644 --- a/pkg/workflow/safe_outputs_mcp_server_test.go +++ b/pkg/workflow/safe_outputs_mcp_server_test.go @@ -113,9 +113,9 @@ func TestSafeOutputsMCPServer_ListTools(t *testing.T) { defer os.Remove(tempFile) config := map[string]interface{}{ - "create-issue": map[string]interface{}{"enabled": true}, - "create-discussion": map[string]interface{}{"enabled": true}, - "missing-tool": map[string]interface{}{"enabled": true}, + "create-issue": true, + "create-discussion": true, + "missing-tool": true, } client := NewMCPTestClient(t, tempFile, config) @@ -238,49 +238,12 @@ func TestSafeOutputsMCPServer_MissingTool(t *testing.T) { t.Log("missing-tool executed successfully using Go MCP SDK") } -func TestSafeOutputsMCPServer_DisabledTool(t *testing.T) { - tempFile := createTempOutputFile(t) - defer os.Remove(tempFile) - - config := map[string]interface{}{ - "create-issue": map[string]interface{}{ - "enabled": false, // Explicitly disabled - }, - "missing-tool": map[string]interface{}{ - "enabled": true, // Keep one enabled so server can start - }, - } - - client := NewMCPTestClient(t, tempFile, config) - defer client.Close() - - // Try to call disabled tool - should return an error - ctx := context.Background() - _, err := client.CallTool(ctx, "create-issue", map[string]any{ - "title": "This should fail", - "body": "Tool is disabled", - }) - - // Should get an error - if err == nil { - t.Fatalf("Expected error for disabled tool, got success") - } - - if !strings.Contains(err.Error(), "create-issue safe-output is not enabled") && - !strings.Contains(err.Error(), "Tool 'create-issue' failed") && - !strings.Contains(err.Error(), "Tool not found: create-issue") { - t.Errorf("Expected error about disabled tool, got: %s", err.Error()) - } - - t.Log("Disabled tool correctly rejected using Go MCP SDK") -} - func TestSafeOutputsMCPServer_UnknownTool(t *testing.T) { tempFile := createTempOutputFile(t) defer os.Remove(tempFile) config := map[string]interface{}{ - "create-issue": map[string]interface{}{"enabled": true}, + "create-issue": true, } client := NewMCPTestClient(t, tempFile, config) @@ -307,8 +270,8 @@ func TestSafeOutputsMCPServer_MultipleTools(t *testing.T) { defer os.Remove(tempFile) config := map[string]interface{}{ - "create-issue": map[string]interface{}{"enabled": true}, - "add-issue-comment": map[string]interface{}{"enabled": true}, + "create-issue": true, + "add-issue-comment": true, } client := NewMCPTestClient(t, tempFile, config)