Skip to content

Refactor: Extract duplicate step formatting code from Copilot and Codex engines - #1977

Merged
pelikhan merged 4 commits into
mainfrom
copilot/refactor-duplicate-code
Oct 19, 2025
Merged

pelikhan merged 4 commits into
mainfrom
copilot/refactor-duplicate-code

Conversation

Copilot AI commented Oct 19, 2025 •

Copy link
Copy Markdown
Contributor

Summary

Fixes duplicate code issue by extracting shared step formatting logic from Copilot and Codex engine execution steps into a reusable helper function.

Problem

The Copilot and Codex engines contained nearly identical ~20 line blocks for formatting GitHub Actions execution steps. This duplication meant that any changes to step formatting (such as indentation adjustments or environment variable handling) had to be applied in multiple places, increasing maintenance burden and the risk of inconsistent behavior.

Duplicate code locations:

  • pkg/workflow/copilot_engine.go lines 184-206
  • pkg/workflow/codex_engine.go lines 166-188

Both engines performed identical operations:

  1. Split commands into lines and indent them (10 spaces)
  2. Format environment variables section
  3. Sort environment keys alphabetically
  4. Apply consistent YAML indentation (8 spaces for headers, 10 for values)

Solution

Created a new shared helper function FormatStepWithCommandAndEnv() in pkg/workflow/engine_shared_helpers.go that consolidates this formatting logic. Both engines now call this single implementation.

Before:

stepLines = append(stepLines, "        run: |")

// Split command into lines and indent them properly
commandLines := strings.Split(command, "\n")
for _, line := range commandLines {
    stepLines = append(stepLines, "          "+line)
}

// Add environment variables
if len(env) > 0 {
    stepLines = append(stepLines, "        env:")
    // Sort environment keys for consistent output
    envKeys := make([]string, 0, len(env))
    for key := range env {
        envKeys = append(envKeys, key)
    }
    sort.Strings(envKeys)

    for _, key := range envKeys {
        value := env[key]
        stepLines = append(stepLines, fmt.Sprintf("          %s: %s", key, value))
    }
}

After:

// Format step with command and environment variables using shared helper
stepLines = FormatStepWithCommandAndEnv(stepLines, command, env)

Changes

  • pkg/workflow/engine_shared_helpers.go: Added FormatStepWithCommandAndEnv() helper function with full documentation
  • pkg/workflow/copilot_engine.go: Replaced 23 duplicate lines with shared helper call (91% reduction)
  • pkg/workflow/codex_engine.go: Replaced 23 duplicate lines with shared helper call (91% reduction)
  • pkg/workflow/engine_shared_helpers_test.go: Added comprehensive test coverage with 3 test functions covering various scenarios

Benefits

  1. Single Point of Maintenance: Future formatting changes only need to be made once
  2. Guaranteed Consistency: Both engines use identical formatting logic, eliminating possibility of divergent behavior
  3. Reduced Bug Risk: Fixes to step formatting will automatically apply to all engines
  4. Better Test Coverage: New comprehensive tests validate correct behavior for edge cases
  5. Improved Readability: Engine code is now more focused on engine-specific logic

Testing

  • ✅ All existing unit and integration tests pass
  • ✅ Added comprehensive tests for the new helper function:
    • Simple and multi-line commands
    • Commands with and without environment variables
    • Environment variable alphabetical sorting
    • YAML indentation validation
    • Preservation of existing step lines
  • ✅ make agent-finish completes successfully
  • ✅ Compiled workflows maintain identical format
  • ✅ CodeQL security scan: 0 vulnerabilities

Code Metrics

  • Duplicate code eliminated: ~42 lines
  • New shared helper: 40 lines (reusable)
  • Test coverage added: 217 lines
  • Net impact: Reduced code duplication while improving test coverage and maintainability
Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected</issue_title>
<issue_description># 🔍 Duplicate Code Detected

Analysis of commit 82fcbb2

Assignee: @copilot

Summary

The Copilot and Codex engine execution steps share a nearly identical block that formats GitHub Actions steps, including command splitting and environment serialization. This duplication spans ~20 lines in each file and should be consolidated into a shared helper to avoid divergence when behavior changes.

Duplication Details

Pattern 1: Shared environment formatting for engine execution steps

  • Severity: Medium
  • Occurrences: 2
  • Locations:
    • pkg/workflow/copilot_engine.go:170-208
    • pkg/workflow/codex_engine.go:162-190
  • Code Sample:
    stepLines = append(stepLines, "        run: |")
    commandLines := strings.Split(command, "\n")
    for _, line := range commandLines {
        stepLines = append(stepLines, "          "+line)
    }
    
    if len(env) > 0 {
        stepLines = append(stepLines, "        env:")
        envKeys := make([]string, 0, len(env))
        for key := range env {
            envKeys = append(envKeys, key)
        }
        sort.Strings(envKeys)
        for _, key := range envKeys {
            value := env[key]
            stepLines = append(stepLines, fmt.Sprintf("          %s: %s", key, value))
        }
    }

Impact Analysis

  • Maintainability: Updating step formatting or environment handling requires touching each engine separately, increasing the odds of inconsistent fixes.
  • Bug Risk: Future fixes (e.g., escaping adjustments) risk being applied to only one engine, leading to hard-to-diagnose differences in workflow behavior.
  • Code Bloat: Duplicated logic adds ~40 lines overall and hides the shared intent behind per-engine noise.

Refactoring Recommendations

  1. Extract helper for step + env serialization
    • Extract common functionality to: pkg/workflow/engine_shared_helpers.go
    • Estimated effort: 2-3 hours (review and test Copilot/Codex flows)
    • Benefits: Single point for formatting changes; reduces diff noise and error surface

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 3
  • Detection Method: Serena semantic code analysis
  • Commit: 82fcbb2
  • Analysis Date: 2025-10-19 11:08:51 UTC

AI generated by Duplicate Code Detector</issue_description>

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

Fixes #1976


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot AI and others added 2 commits October 19, 2025 11:15
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Copilot AI changed the title [WIP] Consolidate duplicate code in GitHub Actions formatting Refactor: Extract duplicate step formatting code from Copilot and Codex engines Oct 19, 2025
Copilot AI requested a review from dsyme October 19, 2025 11:26
@pelikhan
pelikhan marked this pull request as ready for review October 19, 2025 16:03
@github-actions

Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

@pelikhan
pelikhan merged commit 26f6175 into main Oct 19, 2025
4 checks passed
@pelikhan
pelikhan deleted the copilot/refactor-duplicate-code branch October 19, 2025 16:10
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.

[duplicate-code] 🔍 Duplicate Code Detected

3 participants