Extract duplicate custom engine step handling into shared helper functions

[Copilot]

Problem

Multiple workflow engine implementations contained duplicate code for handling custom steps and custom MCP tool configurations. The duplication existed across 4 engine files (copilot_engine.go, codex_engine.go, claude_engine.go, custom_engine.go) and created maintenance overhead:

• Pattern 1: Custom step injection logic duplicated in 3 engines (13 lines each)
• Pattern 2: Custom MCP tool fallback handling duplicated in 4 engines (7-9 lines each)

These duplicate blocks had to be kept in sync manually, increasing the risk of inconsistent fixes and making future changes more error-prone.

Solution

Created two shared helper functions in pkg/workflow/engine_shared_helpers.go:

1. InjectCustomEngineSteps()

Extracts the common pattern for processing custom steps from engine configuration:

// Before (repeated in 3 engines):
if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Steps) > 0 {
    for _, step := range workflowData.EngineConfig.Steps {
        stepYAML, err := e.convertStepToYAML(step)
        if err != nil {
            continue
        }
        steps = append(steps, GitHubActionStep{stepYAML})
    }
}

// After (single call):
steps := InjectCustomEngineSteps(workflowData, e.convertStepToYAML)

2. HandleCustomMCPToolInSwitch()

Extracts the common pattern for handling custom MCP tools in switch statement default cases:

// Before (repeated in 4 engines):
default:
    if toolConfig, ok := tools[toolName].(map[string]any); ok {
        if hasMcp, _ := hasMCPConfig(toolConfig); hasMcp {
            if err := e.renderEngineMCPConfig(yaml, toolName, toolConfig, isLast); err != nil {
                fmt.Printf("Error generating custom MCP configuration for %s: %v\n", toolName, err)
            }
        }
    }

// After (single call):
default:
    HandleCustomMCPToolInSwitch(yaml, toolName, tools, isLast, e.renderEngineMCPConfig)

Impact

• 55 lines of duplicate code removed across 4 engine files
• Single source of truth for custom step and MCP tool handling
• Changes now only need to be made once instead of 3-4 times
• Consistent error handling and behavior across all engines
• Comprehensive test coverage added for both helper functions

Testing

• ✅ All existing tests pass without modification (8.646s runtime)
• ✅ Added focused unit tests with multiple scenarios
• ✅ Verified with make agent-finish - all 64 workflows compile correctly
• ✅ Code formatting and linting pass

Fixes #123

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Duplicate custom engine step handling across workflow engines</issue_title>
<issue_description># 🔍 Duplicate Code Detected

Analysis of commit f093f13

Assignee: @copilot

Summary

Multiple workflow engine implementations duplicate the same logic for (1) injecting user-provided custom steps into execution pipelines and (2) rendering fallback MCP server configuration for custom tools. The repeated blocks span three or more files and stay in sync manually, raising maintenance overhead and risk of inconsistent fixes.

Duplication Details

Pattern 1: Repeated custom step injection blocks

• Severity: Medium
• Occurrences: 3
• Locations:
  • pkg/workflow/copilot_engine.go:70
  • pkg/workflow/codex_engine.go:100
  • pkg/workflow/claude_engine.go:91
• Code Sample:

  // Handle custom steps if they exist in engine config
  if workflowData.EngineConfig != nil && len(workflowData.EngineConfig.Steps) > 0 {
      for _, step := range workflowData.EngineConfig.Steps {
          stepYAML, err := e.convertStepToYAML(step)
          if err != nil {
              // Log error but continue with other steps
              continue
          }
          steps = append(steps, GitHubActionStep{stepYAML})
      }
  }

Pattern 2: Duplicated fallback MCP configuration for custom tools

• Severity: Medium
• Occurrences: 4
• Locations:
  • pkg/workflow/copilot_engine.go:255
  • pkg/workflow/claude_engine.go:619
  • pkg/workflow/codex_engine.go:256
  • pkg/workflow/custom_engine.go:175
• Code Sample:

  // Handle custom MCP tools (those with MCP-compatible type)
  if toolConfig, ok := tools[toolName].(map[string]any); ok {
      if hasMcp, _ := hasMCPConfig(toolConfig); hasMcp {
          if err := e.render<Engine>MCPConfig(yaml, toolName, toolConfig, isLast); err != nil {
              fmt.Printf("Error generating custom MCP configuration for %s: %v\n", toolName, err)
          }
      }
  }

Impact Analysis

• Maintainability: Any change to how custom steps or MCP tool rendering should behave must be edited in three to four places, increasing the chance that engines drift over time.
• Bug Risk: Fixes or enhancements (e.g., improved error handling or logging) can be applied to one engine but missed in others, causing inconsistent runtime behavior.
• Code Bloat: The repeated blocks add >40 lines across the codebase that could be centralized.

Refactoring Recommendations

1. Extract shared custom step helper

   • Create a helper (e.g., appendEngineCustomSteps(workflowData *WorkflowData, convert func(map[string]any) (string, error))) that returns []GitHubActionStep for reuse in each engine before engine-specific logic executes.
   • Estimated effort: 2-3h
   • Benefits: Single place to adjust error handling, logging, or future enhancements.

2. Unify MCP custom-tool rendering

   • Implement a shared utility that iterates mcpTools and delegates engine-specific renderers via a strategy table, while centralizing the default custom-tool block and last-item detection.
   • Estimated effort: 3-4h
   • Benefits: Reduces duplication, simplifies addition of new engines, and keeps custom-tool behavior consistent.

Implementation Checklist

• Review duplication findings
• Prioritize refactoring tasks
• Create refactoring plan
• Implement changes
• Update tests
• Verify no functionality broken

Analysis Metadata

• Analyzed Files: 4
• Detection Method: Serena semantic code analysis
• Commit: f093f13
• Analysis Date: 2025-10-16

AI generated by Duplicate Code Detector</issue_description>

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

Fixes #1800

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[github-actions]

Agentic Changeset Generator triggered by this pull request