[file-diet] Refactor Large Go File: pkg/workflow/mcp-config.go

[agentic-workflows-dev]

Overview

The file pkg/workflow/mcp-config.go has grown to 1,301 lines, making it difficult to maintain and test. This file handles all MCP (Model Context Protocol) server configuration rendering across multiple engines and formats. It contains 22 functions covering diverse responsibilities that should be split into focused modules.

Current State

• File: pkg/workflow/mcp-config.go
• Size: 1,301 lines (163% over the 800-line healthy threshold)
• Functions: 22 total functions
• Test Coverage: No test file exists - this is a critical gap
• Complexity: High - handles multiple MCP server types (Playwright, Serena, Safe Outputs, Agentic Workflows, custom), multiple output formats (JSON, TOML), and multiple engines (Copilot, Claude, Custom)

Refactoring Strategy

Proposed File Splits

Based on semantic analysis, split the file into the following modules:

1. mcp-config-playwright.go

• Functions:
  • renderPlaywrightMCPConfig
  • renderPlaywrightMCPConfigWithOptions
  • renderPlaywrightMCPConfigTOML
  • Helper functions: generatePlaywrightDockerArgs, getPlaywrightCustomArgs, extractExpressionsFromPlaywrightArgs, replaceExpressionsInPlaywrightArgs
• Responsibility: Playwright MCP server configuration rendering (Docker-based browser automation)
• Estimated LOC: ~250 lines
• Rationale: Playwright configuration is self-contained with domain-specific logic for browser automation

2. mcp-config-serena.go

• Functions:
  • selectSerenaContainer
  • renderSerenaMCPConfigWithOptions
  • Helper functions: getSerenaCustomArgs
• Responsibility: Serena MCP server configuration (local vs Docker modes, language-specific container selection)
• Estimated LOC: ~180 lines
• Rationale: Serena has unique container selection logic based on language support

3. mcp-config-builtin.go

• Functions:
  • renderSafeOutputsMCPConfig
  • renderSafeOutputsMCPConfigWithOptions
  • renderSafeOutputsMCPConfigTOML
  • renderAgenticWorkflowsMCPConfigWithOptions
  • renderAgenticWorkflowsMCPConfigTOML
• Responsibility: Built-in MCP servers (Safe Outputs, Agentic Workflows) - GitHub-specific tools
• Estimated LOC: ~200 lines
• Rationale: These are standard GitHub workflow tools with fixed configurations

4. mcp-config-custom.go

• Functions:
  • renderCustomMCPConfigWrapper
  • renderCustomMCPConfigWrapperWithContext
  • renderSharedMCPConfig
  • getMCPConfig (MCP config parser)
  • hasMCPConfig
  • collectHTTPMCPHeaderSecrets
• Responsibility: Custom/user-defined MCP server configuration parsing and rendering
• Estimated LOC: ~400 lines (largest module due to general parsing logic)
• Rationale: Handles arbitrary user-defined MCP servers with stdio/http types

5. mcp-config-types.go

• Types:
  • WellKnownContainer
  • MCPConfigRenderer
  • ToolConfig (interface)
  • MapToolConfig (implementation with methods: GetString, GetStringArray, GetStringMap, GetAny)
• Functions:
  • getWellKnownContainer
• Responsibility: Shared type definitions and interfaces for MCP configuration
• Estimated LOC: ~80 lines
• Rationale: Common types used across all MCP modules

6. mcp-config-utils.go

• Functions:
  • rewriteLocalhostToDockerHost
  • YAML writing helpers: writeArgsToYAML, writeArgsToYAMLInline (if they exist)
• Responsibility: Shared utility functions for MCP configuration (URL rewriting, YAML formatting)
• Estimated LOC: ~100 lines
• Rationale: Generic utilities reused by multiple MCP config modules

Shared Utilities

The original mcp-config.go should be removed after extracting all functionality. If any shared initialization code remains (like the logger), consider:

• Keep logger declaration: var mcpLog = logger.New("workflow:mcp-config") should exist in each new file with appropriate namespace (e.g., "workflow:mcp-config-playwright")

Interface Abstractions

Consider introducing interfaces to reduce coupling:

• MCPRenderer interface: Already exists (MCPConfigRenderer struct), but could be converted to an interface with implementations per engine
• MCPConfigParser interface: Separate parsing logic from rendering logic
• Factory pattern: NewMCPConfig(toolName, toolConfig) (MCPConfig, error) for cleaner instantiation

Test Coverage Plan

Add comprehensive tests for each new file - this is critical as no tests currently exist:

1. mcp-config-playwright_test.go

• Test cases:
  • Playwright config with default settings
  • Playwright config with custom args
  • Playwright config with allowed domains
  • JSON vs TOML format rendering
  • Copilot fields inclusion/exclusion
  • Expression extraction and replacement
• Target coverage: >80%

2. mcp-config-serena_test.go

• Test cases:
  • Container selection based on language support
  • Local vs Docker mode rendering
  • Custom args handling
  • Default vs Oraios container fallback
• Target coverage: >80%

3. mcp-config-builtin_test.go

• Test cases:
  • Safe Outputs configuration (JSON + TOML)
  • Agentic Workflows configuration (JSON + TOML)
  • Copilot fields inclusion
• Target coverage: >80%

4. mcp-config-custom_test.go

• Test cases:
  • stdio MCP servers (with command, with container)
  • HTTP MCP servers (with/without headers)
  • Auto-containerization for well-known commands (npx, uvx)
  • Container + version combination
  • Header secret extraction
  • Invalid configurations (missing required fields)
  • URL localhost rewriting for Docker
• Target coverage: >80%

5. mcp-config-types_test.go

• Test cases:
  • MapToolConfig methods (GetString, GetStringArray, GetStringMap, GetAny)
  • Well-known container lookup
  • Type conversion edge cases
• Target coverage: >90% (simpler code)

6. mcp-config-utils_test.go

• Test cases:
  • Localhost to Docker host rewriting (various URL formats)
  • YAML array formatting (inline vs multi-line)
• Target coverage: >90% (utility functions)

Implementation Guidelines

1. Preserve Behavior: Ensure all existing functionality works identically
2. Maintain Exports: Keep public API unchanged (all render* functions remain exported)
3. Add Tests First: Write tests for each new file before refactoring (TDD approach recommended)
4. Incremental Changes: Split one module at a time in this order:
   • Start with mcp-config-types.go (no dependencies)
   • Then mcp-config-utils.go (depends only on types)
   • Then domain-specific modules (playwright, serena, builtin, custom)
   • Delete original file last
5. Run Tests Frequently: Verify make test-unit passes after each split
6. Update Imports: Ensure all import paths are correct across the codebase
7. Document Changes: Add package-level comments explaining module boundaries
8. Logger Updates: Use specific logger namespaces for each module (e.g., workflow:mcp-config-playwright)

Acceptance Criteria

• Original file is split into 6 focused files
• Each new file is under 500 lines (target: 100-400 lines per file)
• All existing tests pass (make test-unit - verify no regressions)
• New test files created with ≥80% coverage for each module
• No breaking changes to public API (all render* functions work identically)
• Code passes linting (make lint)
• Build succeeds (make build)
• All workflow compilation tests pass (make recompile)
• Documentation updated (add package comments to each new file)

Additional Context

• Repository Guidelines: Follow patterns in AGENTS.md and specs/testing.md
• Code Organization: Prefer many small files grouped by functionality (this aligns with repo philosophy)
• Testing: Match existing test patterns in pkg/workflow/*_test.go
• Console Formatting: Use console.FormatWarningMessage for user-facing output (already present in code)
• Type Safety: The ToolConfig interface provides good abstraction - maintain this pattern

Why This Matters

Current risks:

• No test coverage: A 1,301-line file with 22 functions has zero tests - any changes risk breaking production workflows
• Difficult to maintain: Finding and modifying specific MCP server logic requires scanning 1,300+ lines
• Coupling: Changes to Playwright config might accidentally affect Serena or custom MCP servers
• Onboarding friction: New contributors face a steep learning curve understanding this monolithic file

Benefits after refactoring:

• Testability: Smaller modules are easier to test in isolation
• Maintainability: Find and modify specific MCP server logic quickly
• Safety: Test coverage prevents regressions when adding new features
• Clarity: Each file has a single, clear responsibility

---

Priority: High (1,301 lines is 63% larger than threshold, critical test coverage gap)
Effort: Large (6 new files + comprehensive test suite)
Expected Impact: Significantly improved maintainability, test coverage, and reduced risk of regressions

AI generated by Daily File Diet

[github-actions]

🍪 Issue Monster has assigned this to Copilot!

I've identified this issue as a good candidate for automated resolution and assigned it to the Copilot agent.

The Copilot agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

AI generated by Issue Monster