Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 6, 2025

CustomEngine.renderGitHubMCPConfig duplicated 28 lines of YAML-writing logic already provided by the shared RenderGitHubMCPDockerConfig helper, creating maintenance burden and missing toolsets support.

Changes

  • Refactored CustomEngine.renderGitHubMCPConfig to call shared helper

    • Reduced from 28 to 9 lines (68% reduction)
    • Added toolsets support via getGitHubToolsets()
    • Follows Claude engine pattern (no "type" field, shell variable syntax)
  • Added regression tests covering read-only mode, toolsets, custom args, and combinations

Before/After

// Before: 28 lines of duplicated YAML writing
yaml.WriteString("                \"command\": \"docker\",\n")
yaml.WriteString("                \"args\": [\n")
yaml.WriteString("                  \"run\",\n")
// ... 20+ more lines

// After: 9 lines using shared helper
RenderGitHubMCPDockerConfig(yaml, GitHubMCPDockerOptions{
    ReadOnly:           readOnly,
    Toolsets:           toolsets,  // Now supported
    DockerImageVersion: githubDockerImageVersion,
    CustomArgs:         customArgs,
    IncludeTypeField:   false,
    AllowedTools:       nil,
    EffectiveToken:     "",
})

Ensures single source of truth for GitHub MCP Docker configuration across all engines.

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected: Custom Engine GitHub MCP Docker Config</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Custom Engine GitHub MCP Docker Config

Analysis of commit ${GH_AW_EXPR_0C1909E0}

Assignee: @copilot

Summary

CustomEngine.renderGitHubMCPConfig reimplements the same GitHub MCP Docker configuration that RenderGitHubMCPDockerConfig already provides. The two implementations drift slightly (missing toolset handling and token switching) but remain largely identical, leaving over 20 lines of copy-pasted YAML emission logic.

Duplication Details

Pattern: GitHub MCP Docker YAML writer duplicated in custom engine

  • Severity: Medium
  • Occurrences: 2
  • Locations:
    • pkg/workflow/custom_engine.go:165
    • pkg/workflow/engine_helpers.go:226
  • Code Sample:
// pkg/workflow/custom_engine.go:165-190
yaml.WriteString("              \"github\": {\n")
yaml.WriteString("                \"command\": \"docker\",\n")
yaml.WriteString("                \"args\": [\n")
yaml.WriteString("                  \"run\",\n")
yaml.WriteString("                  \"-i\",\n")
yaml.WriteString("                  \"--rm\",\n")
yaml.WriteString("                  \"-e\",\n")
yaml.WriteString("                  \"GITHUB_PERSONAL_ACCESS_TOKEN\",\n")
if readOnly {
  yaml.WriteString("                  \"-e\",\n")
  yaml.WriteString("                  \"GITHUB_READ_ONLY=1\",\n")
}
yaml.WriteString("                  \"ghcr.io/github/github-mcp-server:" + githubDockerImageVersion + "\"")
...

Impact Analysis

  • Maintainability: Future changes to GitHub MCP Docker semantics must be updated in two places, risking drift and inconsistent behavior across engines.
  • Bug Risk: The custom engine path already omits toolset configuration and Copilot-specific token handling covered by the shared helper, making copy/paste errors likely as features evolve.
  • Code Bloat: The duplicated block adds ~25 lines of boilerplate that the new shared helper was meant to centralize.

Refactoring Recommendations

  1. Reuse shared helper in custom engine

    • Extract options from githubTool and pass them into RenderGitHubMCPDockerConfig.
    • Estimated effort: 1-2 hours (needs option wiring + verification).
    • Benefits: Single source of truth for Docker configuration; ensures future enhancements reach all engines.
  2. Add regression tests around custom engine GitHub MCP config

    • Cover read-only and custom-args cases to validate helper usage.
    • Estimated effort: 1 hour.
    • Benefits: Guards against helper regressions and mismatch with other engines.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 2
  • Detection Method: Serena semantic code analysis
  • Commit: ${GH_AW_EXPR_0C1909E0}
  • Analysis Date: 2025-02-14T00:00:00Z

AI generated by Duplicate Code Detector</issue_description>

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


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

Copilot AI and others added 2 commits November 6, 2025 04:21
- Replace duplicate YAML-writing code in CustomEngine.renderGitHubMCPConfig with call to RenderGitHubMCPDockerConfig
- Add toolsets support to custom engine (previously missing)
- Add comprehensive regression tests for custom engine GitHub MCP config
- Tests cover basic config, read-only mode, custom toolsets, custom args, and combinations
- Ensures custom engine follows same patterns as Claude engine (no type field, shell var syntax)

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix duplicate GitHub MCP Docker config code Eliminate duplicate GitHub MCP Docker config in CustomEngine Nov 6, 2025
Copilot AI requested a review from pelikhan November 6, 2025 04:27
@pelikhan pelikhan marked this pull request as ready for review November 6, 2025 04:36
Copilot AI review requested due to automatic review settings November 6, 2025 04:36
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 refactors the Custom Engine's GitHub MCP Docker configuration to use a shared helper function, adding support for GitHub toolsets and ensuring consistency with other engines. The changes extract duplicated Docker configuration code and consolidate it into the shared RenderGitHubMCPDockerConfig helper.

  • Refactored renderGitHubMCPConfig in Custom Engine to use shared RenderGitHubMCPDockerConfig helper
  • Added GitHub toolsets support to Custom Engine configuration
  • Added comprehensive test coverage for GitHub MCP Docker configuration scenarios

Reviewed Changes

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

File Description
pkg/workflow/custom_engine.go Replaced inline Docker configuration code with shared RenderGitHubMCPDockerConfig helper function, adding toolsets parameter
pkg/workflow/custom_engine_test.go Added comprehensive test cases covering GitHub MCP Docker config scenarios including read-only mode, toolsets, and custom args
.github/workflows/issue-classifier.lock.yml Updated generated workflow to include GITHUB_TOOLSETS environment variable with default value

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

@pelikhan pelikhan merged commit e5c50dc into main Nov 6, 2025
58 of 62 checks passed
@pelikhan pelikhan deleted the copilot/fix-duplicate-code-github-mcp-docker branch November 6, 2025 04:39
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: Custom Engine GitHub MCP Docker Config

2 participants