Add error return to MCPConfigProvider.RenderMCPConfig interface

pkg/workflow/agentic_engine.go

@@ -152,7 +152,7 @@ type WorkflowExecutor interface {
 // Engines that support MCP servers should implement this
 type MCPConfigProvider interface {
 	// RenderMCPConfig renders the MCP configuration for this engine to the given YAML builder
-	RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData)
+	RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error
 }
 
 // LogParser handles parsing and analyzing engine logs
@@ -311,8 +311,9 @@ func (e *BaseEngine) GetLogParserScriptId() string {
 
 // RenderMCPConfig provides a default no-op implementation for MCP configuration
 // Engines can override this to provide custom MCP server configuration
-func (e *BaseEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) {
+func (e *BaseEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error {
 	// Default implementation does nothing - engines that support MCP should override this
+	return nil
 }
 
 // convertStepToYAML converts a step map to YAML string - uses proper YAML serialization

pkg/workflow/agentic_engine_interfaces_test.go

@@ -105,8 +105,8 @@ func TestInterfaceSegregation(t *testing.T) {
 				ParsedTools: &ToolsConfig{},
 				Tools:       map[string]any{},
 			}
-			// Should not panic
-			engine.RenderMCPConfig(&yaml, map[string]any{}, []string{}, workflowData)
+			// Should not panic or error
+			require.NoError(t, engine.RenderMCPConfig(&yaml, map[string]any{}, []string{}, workflowData))
 		}
 	})
 

pkg/workflow/agentic_workflow_test.go

@@ -110,7 +110,8 @@ func TestAgenticWorkflowsMCPConfigGeneration(t *testing.T) {
 			var yaml strings.Builder
 			mcpTools := []string{"agentic-workflows"}
 
-			e.engine.RenderMCPConfig(&yaml, workflowData.Tools, mcpTools, workflowData)
+			err := e.engine.RenderMCPConfig(&yaml, workflowData.Tools, mcpTools, workflowData)
+			require.NoError(t, err)
 			result := yaml.String()
 
 			// Verify the MCP config contains agentic-workflows
@@ -143,7 +144,7 @@ func TestAgenticWorkflowsInstallStepIncludesGHToken(t *testing.T) {
 	var yaml strings.Builder
 	engine := NewCopilotEngine()
 
-	c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData)
+	require.NoError(t, c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData))
 	result := yaml.String()
 
 	// Verify the install step is present
@@ -178,7 +179,7 @@ func TestAgenticWorkflowsInstallStepSkippedWithImport(t *testing.T) {
 	var yaml strings.Builder
 	engine := NewCopilotEngine()
 
-	c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData)
+	require.NoError(t, c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData))
 	result := yaml.String()
 
 	// Verify the install step is NOT present when import exists
@@ -203,7 +204,7 @@ func TestAgenticWorkflowsInstallStepPresentWithoutImport(t *testing.T) {
 	var yaml strings.Builder
 	engine := NewCopilotEngine()
 
-	c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData)
+	require.NoError(t, c.generateMCPSetup(&yaml, workflowData.Tools, engine, workflowData))
 	result := yaml.String()
 
 	// Verify the install step IS present when no import exists

pkg/workflow/claude_mcp.go

@@ -9,7 +9,7 @@ import (
 var claudeMCPLog = logger.New("workflow:claude_mcp")
 
 // RenderMCPConfig renders the MCP configuration for Claude engine
-func (e *ClaudeEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) {
+func (e *ClaudeEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error {
 	claudeMCPLog.Printf("Rendering MCP config for Claude: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools))
 
 	// Create unified renderer with Claude-specific options
@@ -29,7 +29,7 @@ func (e *ClaudeEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]a
 	gatewayConfig := buildMCPGatewayConfig(workflowData)
 
 	// Use shared JSON MCP config renderer with unified renderer methods
-	_ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
+	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
 		GatewayConfig: gatewayConfig,
 		Renderers: MCPToolRenderers{

pkg/workflow/codex_engine_test.go

@@ -343,7 +343,9 @@ func TestCodexEngineRenderMCPConfig(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			var yaml strings.Builder
 			workflowData := &WorkflowData{Name: "test-workflow"}
-			engine.RenderMCPConfig(&yaml, tt.tools, tt.mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&yaml, tt.tools, tt.mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 
 			result := yaml.String()
 			lines := strings.Split(strings.TrimSpace(result), "\n")
@@ -416,7 +418,9 @@ func TestCodexEngineUserAgentIdentifierConversion(t *testing.T) {
 			tools := map[string]any{"github": map[string]any{}}
 			mcpTools := []string{"github"}
 
-			engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 
 			result := yaml.String()
 			expectedUserAgentLine := "user_agent = \"" + tt.expectedUA + "\""
@@ -487,7 +491,9 @@ func TestCodexEngineRenderMCPConfigUserAgentFromConfig(t *testing.T) {
 			tools := map[string]any{"github": map[string]any{}}
 			mcpTools := []string{"github"}
 
-			engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 
 			result := yaml.String()
 			expectedUserAgentLine := "user_agent = \"" + tt.expectedUA + "\""
@@ -604,7 +610,9 @@ func TestCodexEngineRenderMCPConfigUserAgentWithHyphen(t *testing.T) {
 			tools := map[string]any{"github": map[string]any{}}
 			mcpTools := []string{"github"}
 
-			engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&yaml, tools, mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 
 			result := yaml.String()
 			expectedUserAgentLine := "user_agent = \"" + tt.expectedUA + "\""
@@ -734,7 +742,9 @@ func TestCodexEngineHttpMCPServerRendered(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			var yaml strings.Builder
 			workflowData := &WorkflowData{Name: "test-workflow"}
-			engine.RenderMCPConfig(&yaml, tt.tools, tt.mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&yaml, tt.tools, tt.mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 
 			result := yaml.String()
 

pkg/workflow/codex_mcp.go

@@ -10,7 +10,7 @@ import (
 var codexMCPLog = logger.New("workflow:codex_mcp")
 
 // RenderMCPConfig generates MCP server configuration for Codex
-func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) {
+func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error {
 	if codexMCPLog.Enabled() {
 		codexMCPLog.Printf("Rendering MCP config for Codex: mcp_tools=%v, tool_count=%d", mcpTools, len(tools))
 	}
@@ -120,7 +120,7 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
 		})
 	}
 
-	_ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
+	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
 		GatewayConfig: gatewayConfig,
 		Renderers: MCPToolRenderers{

pkg/workflow/compiler_yaml_main_job.go

@@ -224,7 +224,9 @@ func (c *Compiler) generateMainJobSteps(yaml *strings.Builder, data *WorkflowDat
 	c.generateGitHubMCPAppTokenMintingStep(yaml, data)
 
 	// Add MCP setup
-	c.generateMCPSetup(yaml, data.Tools, engine, data)
+	if err := c.generateMCPSetup(yaml, data.Tools, engine, data); err != nil {
+		return fmt.Errorf("failed to generate MCP setup: %w", err)
+	}
 
 	// Stop-time safety checks are now handled by a dedicated job (stop_time_check)
 	// No longer generated in the main job steps

pkg/workflow/copilot_mcp.go

@@ -9,7 +9,7 @@ import (
 var copilotMCPLog = logger.New("workflow:copilot_mcp")
 
 // RenderMCPConfig generates MCP server configuration for Copilot CLI
-func (e *CopilotEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) {
+func (e *CopilotEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error {
 	copilotMCPLog.Printf("Rendering MCP config for Copilot engine: mcpTools=%d", len(mcpTools))
 
 	// Create the directory first
@@ -77,7 +77,7 @@ func (e *CopilotEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]
 		},
 	}
 
-	_ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, options)
+	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, options)
 }
 
 // renderCopilotMCPConfigWithContext generates custom MCP server configuration for Copilot CLI

pkg/workflow/copilot_mcp_http_integration_test.go

@@ -38,7 +38,9 @@ func TestCopilotEngine_HTTPMCPWithHeaderSecrets_Integration(t *testing.T) {
 	// Test MCP config rendering
 	var mcpConfig strings.Builder
 	mcpTools := []string{"datadog"}
-	engine.RenderMCPConfig(&mcpConfig, workflowData.Tools, mcpTools, workflowData)
+	if err := engine.RenderMCPConfig(&mcpConfig, workflowData.Tools, mcpTools, workflowData); err != nil {
+		t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+	}
 
 	mcpOutput := mcpConfig.String()
 
@@ -203,7 +205,9 @@ func TestCopilotEngine_HTTPMCPWithoutSecrets_Integration(t *testing.T) {
 	// Test MCP config rendering
 	var mcpConfig strings.Builder
 	mcpTools := []string{"custom"}
-	engine.RenderMCPConfig(&mcpConfig, workflowData.Tools, mcpTools, workflowData)
+	if err := engine.RenderMCPConfig(&mcpConfig, workflowData.Tools, mcpTools, workflowData); err != nil {
+		t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+	}
 
 	mcpOutput := mcpConfig.String()
 

pkg/workflow/gemini_mcp.go

@@ -9,7 +9,7 @@ import (
 var geminiMCPLog = logger.New("workflow:gemini_mcp")
 
 // RenderMCPConfig renders MCP server configuration for Gemini CLI
-func (e *GeminiEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) {
+func (e *GeminiEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]any, mcpTools []string, workflowData *WorkflowData) error {
 	geminiMCPLog.Printf("Rendering MCP config for Gemini: tool_count=%d, mcp_tool_count=%d", len(tools), len(mcpTools))
 
 	// Create unified renderer with Gemini-specific options
@@ -24,7 +24,7 @@ func (e *GeminiEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]a
 	}
 
 	// Use shared JSON MCP config renderer
-	_ = RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
+	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
 		GatewayConfig: buildMCPGatewayConfig(workflowData),
 		Renderers: MCPToolRenderers{

pkg/workflow/mcp_api_key_masking_test.go

@@ -26,7 +26,7 @@ func TestSafeOutputsAPIKeyImmediateMasking(t *testing.T) {
 	mockEngine := NewClaudeEngine()
 
 	var yaml strings.Builder
-	compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+	require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 	output := yaml.String()
 
 	// Find the Safe Outputs config generation section
@@ -93,7 +93,7 @@ func TestSafeInputsAPIKeyImmediateMasking(t *testing.T) {
 	mockEngine := NewClaudeEngine()
 
 	var yaml strings.Builder
-	compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+	require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 	output := yaml.String()
 
 	// Find the Safe Inputs config generation section
@@ -149,7 +149,7 @@ func TestMCPGatewayAPIKeyImmediateMasking(t *testing.T) {
 	mockEngine := NewClaudeEngine()
 
 	var yaml strings.Builder
-	compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+	require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 	output := yaml.String()
 
 	// Find the MCP gateway API key generation
@@ -206,7 +206,7 @@ func TestAPIKeyMaskingNoEmptyDeclaration(t *testing.T) {
 	mockEngine := NewClaudeEngine()
 
 	var yaml strings.Builder
-	compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+	require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 	output := yaml.String()
 
 	// Verify no empty API key declarations before assignment

pkg/workflow/mcp_setup_generator.go

@@ -73,14 +73,14 @@ import (
 var mcpSetupGeneratorLog = logger.New("workflow:mcp_setup_generator")
 
 // generateMCPSetup generates the MCP server configuration setup
-func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, engine CodingAgentEngine, workflowData *WorkflowData) {
+func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any, engine CodingAgentEngine, workflowData *WorkflowData) error {
 	mcpSetupGeneratorLog.Print("Generating MCP server configuration setup")
 	// Collect tools that need MCP server configuration
 	var mcpTools []string
 
 	// Check if workflowData is valid before accessing its fields
 	if workflowData == nil {
-		return
+		return nil
 	}
 
 	workflowTools := workflowData.Tools
@@ -138,7 +138,7 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
 	// If no MCP tools, no configuration needed
 	if len(mcpTools) == 0 {
 		mcpSetupGeneratorLog.Print("No MCP tools configured, skipping MCP setup")
-		return
+		return nil
 	}
 
 	// Install gh-aw extension if agentic-workflows tool is enabled
@@ -732,5 +732,5 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
 
 	// Render MCP config - this will pipe directly to the gateway script
 	// The MCP gateway is always enabled, even when agent sandbox is disabled
-	engine.RenderMCPConfig(yaml, tools, mcpTools, workflowData)
+	return engine.RenderMCPConfig(yaml, tools, mcpTools, workflowData)
 }

pkg/workflow/mcp_setup_generator_test.go

@@ -63,7 +63,7 @@ func TestSafeInputsStepCodeGenerationStability(t *testing.T) {
 
 	for i := 0; i < iterations; i++ {
 		var yaml strings.Builder
-		compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+		require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 		outputs[i] = yaml.String()
 	}
 
@@ -219,7 +219,7 @@ func TestMCPGatewayVersionFromFrontmatter(t *testing.T) {
 			var yaml strings.Builder
 			mockEngine := NewClaudeEngine()
 
-			compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData)
+			require.NoError(t, compiler.generateMCPSetup(&yaml, workflowData.Tools, mockEngine, workflowData))
 			setupOutput := yaml.String()
 
 			// The setup output should contain the container image with the correct version

pkg/workflow/tools_timeout_test.go

@@ -137,7 +137,9 @@ func TestCodexEngineWithToolsTimeout(t *testing.T) {
 			// Render MCP config
 			var configBuilder strings.Builder
 			mcpTools := []string{"github"}
-			engine.RenderMCPConfig(&configBuilder, workflowData.Tools, mcpTools, workflowData)
+			if err := engine.RenderMCPConfig(&configBuilder, workflowData.Tools, mcpTools, workflowData); err != nil {
+				t.Fatalf("RenderMCPConfig returned unexpected error: %v", err)
+			}
 			configContent := configBuilder.String()
 
 			if !strings.Contains(configContent, tt.expectedTimeout) {