refactor: extract shared MCP renderer helpers across engine implementations

pkg/workflow/claude_mcp.go

@@ -12,60 +12,17 @@ var claudeMCPLog = logger.New("workflow:claude_mcp")
 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
 	// Claude uses JSON format without Copilot-specific fields and multi-line args
-	createRenderer := func(isLast bool) *MCPConfigRendererUnified {
-		return NewMCPConfigRenderer(MCPRendererOptions{
-			IncludeCopilotFields:   false, // Claude doesn't use "type" and "tools" fields
-			InlineArgs:             false, // Claude uses multi-line args format
-			Format:                 "json",
-			IsLast:                 isLast,
-			ActionMode:             GetActionModeFromWorkflowData(workflowData),
-			WriteSinkGuardPolicies: deriveWriteSinkGuardPolicyFromWorkflow(workflowData),
-		})
-	}
+	createRenderer := buildMCPRendererFactory(workflowData, "json", false, false)
 
 	// Build gateway configuration for MCP config
 	// Per MCP Gateway Specification v1.0.0 section 4.1.3, the gateway section is required
-	gatewayConfig := buildMCPGatewayConfig(workflowData)
-
-	// Use shared JSON MCP config renderer with unified renderer methods
 	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
-		GatewayConfig: gatewayConfig,
-		Renderers: MCPToolRenderers{
-			RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderGitHubMCP(yaml, githubTool, workflowData)
-			},
-			RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderPlaywrightMCP(yaml, playwrightTool)
-			},
-			RenderSerena: func(yaml *strings.Builder, serenaTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSerenaMCP(yaml, serenaTool)
-			},
-			RenderCacheMemory: noOpCacheMemoryRenderer,
-			RenderAgenticWorkflows: func(yaml *strings.Builder, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderAgenticWorkflowsMCP(yaml)
-			},
-			RenderSafeOutputs: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSafeOutputsMCP(yaml, workflowData)
-			},
-			RenderMCPScripts: func(yaml *strings.Builder, mcpScripts *MCPScriptsConfig, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderMCPScriptsMCP(yaml, mcpScripts, workflowData)
-			},
-			RenderWebFetch: func(yaml *strings.Builder, isLast bool) {
-				renderMCPFetchServerConfig(yaml, "json", "              ", isLast, false, deriveWriteSinkGuardPolicyFromWorkflow(workflowData))
-			},
-			RenderCustomMCPConfig: func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
-				return e.renderClaudeMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
-			},
-		},
+		GatewayConfig: buildMCPGatewayConfig(workflowData),
+		Renderers: buildStandardJSONMCPRenderers(workflowData, createRenderer, false, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
+			return e.renderClaudeMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
+		}),
 	})
 }
 

pkg/workflow/codex_mcp.go

@@ -107,59 +107,15 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
 	gatewayConfig := buildMCPGatewayConfig(workflowData)
 
 	// Use shared JSON renderer for gateway input
-	createJSONRenderer := func(isLast bool) *MCPConfigRendererUnified {
-		actionMode := ActionModeDev // Default to dev mode
-		if workflowData != nil {
-			actionMode = workflowData.ActionMode
-		}
-		return NewMCPConfigRenderer(MCPRendererOptions{
-			IncludeCopilotFields:   false, // Gateway doesn't need Copilot fields
-			InlineArgs:             false, // Use standard multi-line format
-			Format:                 "json",
-			IsLast:                 isLast,
-			ActionMode:             actionMode,
-			WriteSinkGuardPolicies: deriveWriteSinkGuardPolicyFromWorkflow(workflowData),
-		})
-	}
+	// Gateway uses JSON format without Copilot-specific fields and multi-line args
+	createJSONRenderer := buildMCPRendererFactory(workflowData, "json", false, false)
 
 	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
 		GatewayConfig: gatewayConfig,
-		Renderers: MCPToolRenderers{
-			RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderGitHubMCP(yaml, githubTool, workflowData)
-			},
-			RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderPlaywrightMCP(yaml, playwrightTool)
-			},
-			RenderSerena: func(yaml *strings.Builder, serenaTool any, isLast bool) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderSerenaMCP(yaml, serenaTool)
-			},
-			RenderCacheMemory: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				// Cache-memory is not used as MCP server
-			},
-			RenderAgenticWorkflows: func(yaml *strings.Builder, isLast bool) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderAgenticWorkflowsMCP(yaml)
-			},
-			RenderSafeOutputs: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderSafeOutputsMCP(yaml, workflowData)
-			},
-			RenderMCPScripts: func(yaml *strings.Builder, mcpScripts *MCPScriptsConfig, isLast bool) {
-				renderer := createJSONRenderer(isLast)
-				renderer.RenderMCPScriptsMCP(yaml, mcpScripts, workflowData)
-			},
-			RenderWebFetch: func(yaml *strings.Builder, isLast bool) {
-				renderMCPFetchServerConfig(yaml, "json", "              ", isLast, false, deriveWriteSinkGuardPolicyFromWorkflow(workflowData))
-			},
-			RenderCustomMCPConfig: func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
-				return e.renderCodexJSONMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
-			},
-		},
+		Renderers: buildStandardJSONMCPRenderers(workflowData, createJSONRenderer, false, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
+			return e.renderCodexJSONMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
+		}),
 	})
 }
 

pkg/workflow/copilot_mcp.go

@@ -15,62 +15,18 @@ func (e *CopilotEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]
 	// Create the directory first
 	yaml.WriteString("          mkdir -p /home/runner/.copilot\n")
 
-	// Create unified renderer with Copilot-specific options
 	// Copilot uses JSON format with type and tools fields, and inline args
-	createRenderer := func(isLast bool) *MCPConfigRendererUnified {
-		return NewMCPConfigRenderer(MCPRendererOptions{
-			IncludeCopilotFields:   true, // Copilot uses "type" and "tools" fields
-			InlineArgs:             true, // Copilot uses inline args format
-			Format:                 "json",
-			IsLast:                 isLast,
-			ActionMode:             GetActionModeFromWorkflowData(workflowData),
-			WriteSinkGuardPolicies: deriveWriteSinkGuardPolicyFromWorkflow(workflowData),
-		})
-	}
+	createRenderer := buildMCPRendererFactory(workflowData, "json", true, true)
 
 	// Build gateway configuration for MCP config
 	// Per MCP Gateway Specification v1.0.0 section 4.1.3, the gateway section is required
-	gatewayConfig := buildMCPGatewayConfig(workflowData)
-
-	// Use shared JSON MCP config renderer with unified renderer methods
 	options := JSONMCPConfigOptions{
 		ConfigPath:    "/home/runner/.copilot/mcp-config.json",
-		GatewayConfig: gatewayConfig,
-		Renderers: MCPToolRenderers{
-			RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderGitHubMCP(yaml, githubTool, workflowData)
-			},
-			RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderPlaywrightMCP(yaml, playwrightTool)
-			},
-			RenderSerena: func(yaml *strings.Builder, serenaTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSerenaMCP(yaml, serenaTool)
-			},
-			RenderCacheMemory: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				// Cache-memory is not used for Copilot (filtered out)
-			},
-			RenderAgenticWorkflows: func(yaml *strings.Builder, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderAgenticWorkflowsMCP(yaml)
-			},
-			RenderSafeOutputs: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSafeOutputsMCP(yaml, workflowData)
-			},
-			RenderMCPScripts: func(yaml *strings.Builder, mcpScripts *MCPScriptsConfig, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderMCPScriptsMCP(yaml, mcpScripts, workflowData)
-			},
-			RenderWebFetch: func(yaml *strings.Builder, isLast bool) {
-				renderMCPFetchServerConfig(yaml, "json", "              ", isLast, true, deriveWriteSinkGuardPolicyFromWorkflow(workflowData))
-			},
-			RenderCustomMCPConfig: func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
-				return e.renderCopilotMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
-			},
-		},
+		GatewayConfig: buildMCPGatewayConfig(workflowData),
+		// webFetchIncludeTools=true: Copilot requires a tools field in the web-fetch server config
+		Renderers: buildStandardJSONMCPRenderers(workflowData, createRenderer, true, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
+			return e.renderCopilotMCPConfigWithContext(yaml, toolName, toolConfig, isLast, workflowData)
+		}),
 		FilterTool: func(toolName string) bool {
 			// Filter out cache-memory for Copilot
 			// Cache-memory is handled as a simple file share, not an MCP server

pkg/workflow/gemini_mcp.go

@@ -12,54 +12,15 @@ var geminiMCPLog = logger.New("workflow:gemini_mcp")
 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
-	createRenderer := func(isLast bool) *MCPConfigRendererUnified {
-		return NewMCPConfigRenderer(MCPRendererOptions{
-			IncludeCopilotFields:   false,
-			InlineArgs:             false,
-			Format:                 "json", // Gemini uses JSON format like Claude/Codex
-			IsLast:                 isLast,
-			ActionMode:             GetActionModeFromWorkflowData(workflowData),
-			WriteSinkGuardPolicies: deriveWriteSinkGuardPolicyFromWorkflow(workflowData),
-		})
-	}
+	// Gemini uses JSON format without Copilot-specific fields and multi-line args
+	createRenderer := buildMCPRendererFactory(workflowData, "json", false, false)
 
 	// Use shared JSON MCP config renderer
 	return RenderJSONMCPConfig(yaml, tools, mcpTools, workflowData, JSONMCPConfigOptions{
 		ConfigPath:    "/tmp/gh-aw/mcp-config/mcp-servers.json",
 		GatewayConfig: buildMCPGatewayConfig(workflowData),
-		Renderers: MCPToolRenderers{
-			RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderGitHubMCP(yaml, githubTool, workflowData)
-			},
-			RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderPlaywrightMCP(yaml, playwrightTool)
-			},
-			RenderSerena: func(yaml *strings.Builder, serenaTool any, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSerenaMCP(yaml, serenaTool)
-			},
-			RenderCacheMemory: noOpCacheMemoryRenderer,
-			RenderAgenticWorkflows: func(yaml *strings.Builder, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderAgenticWorkflowsMCP(yaml)
-			},
-			RenderSafeOutputs: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
-				renderer := createRenderer(isLast)
-				renderer.RenderSafeOutputsMCP(yaml, workflowData)
-			},
-			RenderMCPScripts: func(yaml *strings.Builder, mcpScripts *MCPScriptsConfig, isLast bool) {
-				renderer := createRenderer(isLast)
-				renderer.RenderMCPScriptsMCP(yaml, mcpScripts, workflowData)
-			},
-			RenderWebFetch: func(yaml *strings.Builder, isLast bool) {
-				renderMCPFetchServerConfig(yaml, "json", "              ", isLast, false, deriveWriteSinkGuardPolicyFromWorkflow(workflowData))
-			},
-			RenderCustomMCPConfig: func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
-				return renderCustomMCPConfigWrapperWithContext(yaml, toolName, toolConfig, isLast, workflowData)
-			},
-		},
+		Renderers: buildStandardJSONMCPRenderers(workflowData, createRenderer, false, func(yaml *strings.Builder, toolName string, toolConfig map[string]any, isLast bool) error {
+			return renderCustomMCPConfigWrapperWithContext(yaml, toolName, toolConfig, isLast, workflowData)
+		}),
 	})
 }

pkg/workflow/mcp_renderer_helpers.go

@@ -0,0 +1,63 @@
+package workflow
+
+import "strings"
+
+// buildMCPRendererFactory creates a factory function for MCPConfigRendererUnified instances.
+// The returned function accepts isLast as a parameter and creates a renderer with engine-specific
+// options derived from the provided parameters and workflowData at call time.
+func buildMCPRendererFactory(workflowData *WorkflowData, format string, includeCopilotFields, inlineArgs bool) func(bool) *MCPConfigRendererUnified {
+	return func(isLast bool) *MCPConfigRendererUnified {
+		return NewMCPConfigRenderer(MCPRendererOptions{
+			IncludeCopilotFields:   includeCopilotFields,
+			InlineArgs:             inlineArgs,
+			Format:                 format,
+			IsLast:                 isLast,
+			ActionMode:             GetActionModeFromWorkflowData(workflowData),
+			WriteSinkGuardPolicies: deriveWriteSinkGuardPolicyFromWorkflow(workflowData),
+		})
+	}
+}
+
+// buildStandardJSONMCPRenderers constructs MCPToolRenderers with the standard rendering callbacks
+// shared across JSON-format engines (Claude, Gemini, Copilot, Codex gateway).
+//
+// All eight standard tool callbacks (GitHub, Playwright, Serena, CacheMemory, AgenticWorkflows,
+// SafeOutputs, MCPScripts, WebFetch) are wired to the corresponding unified renderer methods
+// via createRenderer. Cache-memory is always a no-op for these engines.
+//
+// webFetchIncludeTools controls whether the web-fetch server includes a tools field:
+// set to true for Copilot (which uses inline args) and false for all other engines.
+//
+// renderCustom is the engine-specific handler for custom MCP tool configuration entries.
+func buildStandardJSONMCPRenderers(
+	workflowData *WorkflowData,
+	createRenderer func(bool) *MCPConfigRendererUnified,
+	webFetchIncludeTools bool,
+	renderCustom RenderCustomMCPToolConfigHandler,
+) MCPToolRenderers {
+	return MCPToolRenderers{
+		RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
+			createRenderer(isLast).RenderGitHubMCP(yaml, githubTool, workflowData)
+		},
+		RenderPlaywright: func(yaml *strings.Builder, playwrightTool any, isLast bool) {
+			createRenderer(isLast).RenderPlaywrightMCP(yaml, playwrightTool)
+		},
+		RenderSerena: func(yaml *strings.Builder, serenaTool any, isLast bool) {
+			createRenderer(isLast).RenderSerenaMCP(yaml, serenaTool)
+		},
+		RenderCacheMemory: noOpCacheMemoryRenderer,
+		RenderAgenticWorkflows: func(yaml *strings.Builder, isLast bool) {
+			createRenderer(isLast).RenderAgenticWorkflowsMCP(yaml)
+		},
+		RenderSafeOutputs: func(yaml *strings.Builder, isLast bool, workflowData *WorkflowData) {
+			createRenderer(isLast).RenderSafeOutputsMCP(yaml, workflowData)
+		},
+		RenderMCPScripts: func(yaml *strings.Builder, mcpScripts *MCPScriptsConfig, isLast bool) {
+			createRenderer(isLast).RenderMCPScriptsMCP(yaml, mcpScripts, workflowData)
+		},
+		RenderWebFetch: func(yaml *strings.Builder, isLast bool) {
+			renderMCPFetchServerConfig(yaml, "json", "              ", isLast, webFetchIncludeTools, deriveWriteSinkGuardPolicyFromWorkflow(workflowData))
+		},
+		RenderCustomMCPConfig: renderCustom,
+	}
+}