From dd294ee7f12274ee8576c4c411a9d4f76401bffe Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 04:06:41 +0000 Subject: [PATCH 1/2] Initial plan From 6738efb823ef142892b04c13c317cfb144664007 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 04:22:05 +0000 Subject: [PATCH 2/2] refactor: extract shared MCP renderer helpers to eliminate structural duplication - Add buildMCPRendererFactory() and buildStandardJSONMCPRenderers() helpers - Refactor Claude, Gemini, Copilot, Codex MCP config rendering to use helpers - Fix Codex inconsistent ActionMode (now uses GetActionModeFromWorkflowData consistently) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/claude_mcp.go | 53 +++-------------------- pkg/workflow/codex_mcp.go | 54 +++--------------------- pkg/workflow/copilot_mcp.go | 56 +++---------------------- pkg/workflow/gemini_mcp.go | 49 +++------------------- pkg/workflow/mcp_renderer_helpers.go | 63 ++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 191 deletions(-) create mode 100644 pkg/workflow/mcp_renderer_helpers.go diff --git a/pkg/workflow/claude_mcp.go b/pkg/workflow/claude_mcp.go index 111c8185cbf..032c1888e5b 100644 --- a/pkg/workflow/claude_mcp.go +++ b/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) + }), }) } diff --git a/pkg/workflow/codex_mcp.go b/pkg/workflow/codex_mcp.go index 2e25c1675b0..f7af0ba577a 100644 --- a/pkg/workflow/codex_mcp.go +++ b/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) + }), }) } diff --git a/pkg/workflow/copilot_mcp.go b/pkg/workflow/copilot_mcp.go index e794944ba77..3501f594296 100644 --- a/pkg/workflow/copilot_mcp.go +++ b/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 diff --git a/pkg/workflow/gemini_mcp.go b/pkg/workflow/gemini_mcp.go index 89e0494dc84..699575c4f17 100644 --- a/pkg/workflow/gemini_mcp.go +++ b/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) + }), }) } diff --git a/pkg/workflow/mcp_renderer_helpers.go b/pkg/workflow/mcp_renderer_helpers.go new file mode 100644 index 00000000000..65c47a46462 --- /dev/null +++ b/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, + } +}