Skip to content

refactor: eliminate duplicate extractArgs closures in proxy/router.go#7210

Merged
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-fix-extractargs
Jun 8, 2026
Merged

refactor: eliminate duplicate extractArgs closures in proxy/router.go#7210
lpcox merged 5 commits into
mainfrom
copilot/duplicate-code-fix-extractargs

Conversation

Copilot AI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

internal/proxy/router.go had ~40 lines of repeated extractArgs boilerplate across 23 route entries — identical anonymous functions wrapping the same map literals.

Changes

Adds four shared helpers alongside the existing repoArgs function:

  • prArgs(owner, repo, pullNumber, method string) — builds PR args map; replaces 4 inline literals
  • issueArgs(owner, repo, issueNumber string, method ...string) — builds issue args map with optional method; replaces 3 inline literals
  • emptyExtractArgs — shared closure returning map[string]interface{}{}; replaces 6 identical anonymous funcs
  • repoArgsExtractor — shared closure returning repoArgs(m[1], m[2]); replaces 10 identical anonymous func wrappers

Routes that previously needed a wrapper closure just to call repoArgs or return an empty map now reference the shared variable directly:

// Before
{
    pattern:  regexp.MustCompile(`^/repos/([^/]+)/([^/]+)/issues$`),
    toolName: "list_issues",
    extractArgs: func(m []string) map[string]interface{} {
        return repoArgs(m[1], m[2])
    },
},

// After
{
    pattern:     regexp.MustCompile(`^/repos/([^/]+)/([^/]+)/issues$`),
    toolName:    "list_issues",
    extractArgs: repoArgsExtractor,
},

Routes that build richer maps (PR, issue) now delegate to typed helpers instead of open-coded map literals:

// Before
extractArgs: func(m []string) map[string]interface{} {
    return map[string]interface{}{"owner": m[1], "repo": m[2], "pullNumber": m[3], "method": "get_files"}
},

// After
extractArgs: func(m []string) map[string]interface{} {
    return prArgs(m[1], m[2], m[3], "get_files")
},

Note: The "pullNumber" / "issue_number" key name asymmetry is pre-existing and intentional — these names are dictated by the downstream MCP tool interfaces and cannot be changed without coordinated guard updates.

Add prArgs, issueArgs helpers and emptyExtractArgs, repoArgsExtractor
shared closure variables. Update all 23 affected route entries to use
the new helpers, removing ~40 lines of repeated boilerplate.

Closes #7191
Copilot AI changed the title [WIP] Fix duplicate code pattern in proxy/router.go refactor: eliminate duplicate extractArgs closures in proxy/router.go Jun 8, 2026
Copilot AI requested a review from lpcox June 8, 2026 14:06
Copilot finished work on behalf of lpcox June 8, 2026 14:06
@lpcox lpcox marked this pull request as ready for review June 8, 2026 14:36
Copilot AI review requested due to automatic review settings June 8, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 internal/proxy/router.go to remove repeated extractArgs anonymous functions across many REST route entries by introducing shared helper functions/closures for common argument maps.

Changes:

  • Added prArgs(...) and issueArgs(...) helpers to centralize construction of common PR/issue argument maps.
  • Introduced shared extractArgs implementations for “no args” routes and “owner/repo only” routes, and reused them across route entries.
  • Updated affected routes to reference these helpers instead of repeating inline map literals/closures.
Show a summary per file
File Description
internal/proxy/router.go Deduplicates repeated extractArgs closures/map literals via shared helpers and shared extractors in the route table.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread internal/proxy/router.go
Comment thread internal/proxy/router.go Outdated
lpcox and others added 2 commits June 8, 2026 08:51
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lpcox

lpcox commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

Copilot AI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@copilot resolve the merge conflicts in this pull request

Merged main into the PR branch. The conflict was in the helper function block — both branches added new helpers in the same area. The resolution keeps all of them: prArgs, issueArgs, emptyExtractArgs, and repoArgsExtractor from this PR, plus repoMethodArgs and repoMethodResourceArgs from main. Build and tests pass.

GitHub Advanced Security started work on behalf of lpcox June 8, 2026 15:56 View session
Copilot finished work on behalf of lpcox June 8, 2026 15:56
GitHub Advanced Security finished work on behalf of lpcox June 8, 2026 15:56
@lpcox lpcox merged commit ab5eec4 into main Jun 8, 2026
17 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-fix-extractargs branch June 8, 2026 16:36
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 Pattern: Repeated extractArgs Closure Groups in proxy/router.go

3 participants