Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 15, 2025

Currently, MCP configuration rendering is duplicated across 4 engines (Claude, Copilot, Codex, Custom) with ~85% code similarity, creating maintenance burden and increasing bug surface.

Changes

New unified renderer

  • pkg/workflow/mcp_renderer.go - MCPConfigRendererUnified with configurable rendering for all MCP server types
  • pkg/workflow/mcp_renderer_test.go - 20 test cases covering all engines and format combinations

Options struct

MCPRendererOptions controls engine-specific variations:

  • IncludeCopilotFields bool - Adds "type": "local" and "tools": ["*"] for Copilot
  • InlineArgs bool - Single-line args (Copilot) vs multi-line (Claude/Custom)
  • Format string - Supports "json" and "toml"
  • IsLast bool - Controls trailing comma placement

Unified methods

All wrap existing rendering functions for backward compatibility:

  • RenderGitHubMCP() - Local (Docker) and remote modes
  • RenderPlaywrightMCP() - JSON/TOML with configurable arg formatting
  • RenderSafeOutputsMCP() - JSON/TOML with proper env var escaping
  • RenderAgenticWorkflowsMCP() - JSON/TOML support

Usage

// Copilot engine
renderer := NewMCPConfigRenderer(MCPRendererOptions{
    IncludeCopilotFields: true,
    InlineArgs:           true,
    Format:               "json",
    IsLast:               false,
})

// Claude engine
renderer := NewMCPConfigRenderer(MCPRendererOptions{
    IncludeCopilotFields: false,
    InlineArgs:           false,
    Format:               "json",
    IsLast:               true,
})

// Codex engine
renderer := NewMCPConfigRenderer(MCPRendererOptions{
    Format: "toml",
})

Existing engine code remains unchanged; this provides a cleaner API for future refactoring.

Original prompt

This section details on the original issue you should resolve

<issue_title>[task] Create unified MCPConfigRenderer with base implementations</issue_title>
<issue_description>## Objective
Create a new pkg/workflow/mcp_renderer.go file with a unified MCP configuration renderer that eliminates duplication across 20+ similar rendering functions.

Context

Currently, MCP configuration rendering is duplicated across 4 engine implementations (Claude, Copilot, Codex, Custom) with ~85% code similarity. This creates maintenance burden and increases bug surface. Related to #3942.

Approach

  1. Create new file pkg/workflow/mcp_renderer.go
  2. Define MCPRendererOptions struct with engine-specific configuration options:
    • IncludeCopilotFields bool
    • InlineArgs bool
    • Format string (yaml/toml)
    • IsLast bool
  3. Implement MCPConfigRenderer struct with unified rendering methods:
    • RenderGitHubMCP(yaml *strings.Builder, githubTool any, workflowData *WorkflowData)
    • RenderPlaywrightMCP(yaml *strings.Builder, playwrightTool any)
    • RenderSafeOutputsMCP(yaml *strings.Builder)
    • RenderAgenticWorkflowsMCP(yaml *strings.Builder)
  4. Add constructor NewMCPConfigRenderer(opts MCPRendererOptions) *MCPConfigRenderer

Files to Create

  • pkg/workflow/mcp_renderer.go - New unified renderer implementation

Acceptance Criteria

  • MCPConfigRenderer struct created with unified rendering logic
  • MCPRendererOptions supports all engine-specific variations
  • All 4 MCP types have unified Render* methods
  • Constructor NewMCPConfigRenderer() properly initializes options
  • Code compiles without errors
  • Unit tests created for the new renderer in pkg/workflow/mcp_renderer_test.go
  • Tests verify YAML and TOML output formats
  • Tests cover all option combinations
    Related to [refactor] 🔧 Semantic Function Clustering Analysis: Refactoring Opportunities #3942

AI generated by Plan Command for #3942</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

- Create pkg/workflow/mcp_renderer.go with MCPConfigRendererUnified struct
- Implement MCPRendererOptions with IncludeCopilotFields, InlineArgs, Format, IsLast
- Add NewMCPConfigRenderer constructor
- Implement unified rendering methods: RenderGitHubMCP, RenderPlaywrightMCP, RenderSafeOutputsMCP, RenderAgenticWorkflowsMCP
- Support both JSON and TOML output formats
- Handle engine-specific variations (Copilot vs Claude/Custom vs Codex)
- Create comprehensive test suite in mcp_renderer_test.go covering all option combinations
- All tests pass (make test-unit)
- All linting passes (make lint)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Create unified MCPConfigRenderer with base implementations Add unified MCPConfigRenderer to eliminate duplication across engines Nov 15, 2025
Copilot AI requested a review from pelikhan November 15, 2025 07:02
Copilot finished work on behalf of pelikhan November 15, 2025 07:02
@pelikhan pelikhan marked this pull request as ready for review November 15, 2025 13:02
Copilot AI review requested due to automatic review settings November 15, 2025 13:02
Copilot finished reviewing on behalf of pelikhan November 15, 2025 13:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a unified MCPConfigRenderer to eliminate code duplication across four engine implementations (Claude, Copilot, Codex, Custom) that were rendering MCP server configurations with ~85% code similarity. The refactoring consolidates common MCP rendering logic into a single, configurable renderer.

Key changes:

  • New MCPConfigRendererUnified struct with engine-agnostic rendering methods
  • MCPRendererOptions struct to configure engine-specific behavior (Copilot fields, inline args, format, trailing commas)
  • Four unified render methods: RenderGitHubMCP, RenderPlaywrightMCP, RenderSafeOutputsMCP, RenderAgenticWorkflowsMCP

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
pkg/workflow/mcp_renderer.go New unified renderer implementation with configurable options for all MCP server types (175 lines)
pkg/workflow/mcp_renderer_test.go Comprehensive test suite with 20 test cases covering all engines and format combinations (523 lines)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pelikhan pelikhan merged commit 0c5d175 into main Nov 15, 2025
92 of 98 checks passed
@pelikhan pelikhan deleted the copilot/create-unified-mcp-config-renderer branch November 15, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[task] Create unified MCPConfigRenderer with base implementations

2 participants