Skip to content

[duplicate-code] Duplicate Code Pattern: Collaborator Permission Parse/Log/Wrap Block #4327

@github-actions

Description

@github-actions

Part of duplicate code analysis: #4325

Summary

The "parse permission + log + wrap in MCP envelope" block inside callCollaboratorPermission (internal/server/unified.go) and restBackendCaller (internal/proxy/proxy.go) is nearly identical — ~15 lines of JSON-unmarshal, three-way log branch, and MCP wrapping copied between two different callers that ultimately perform the same GitHub REST enrichment.

Duplication Details

Pattern: Collaborator-permission JSON parse / log / MCP-wrap block

  • Severity: Medium

  • Occurrences: 2

  • Locations:

    • internal/server/unified.go (lines ~338–354) — guardBackendCaller.callCollaboratorPermission
    • internal/proxy/proxy.go (lines ~318–339) — restBackendCaller.CallTool (get_collaborator_permission branch)
  • Duplicated Block (structure identical; only the logger variable differs):

    // Log the resolved permission level for observability
    var permResp map[string]interface{}
    if jsonErr := json.Unmarshal(body, &permResp); jsonErr == nil {
        if perm, ok := permResp["permission"].(string); ok {
            log.Printf("get_collaborator_permission: %s/%s user %s → permission=%q (HTTP %d)", ...)
        } else {
            log.Printf("get_collaborator_permission: %s/%s user %s → HTTP %d, permission field missing", ...)
        }
    } else {
        log.Printf("get_collaborator_permission: %s/%s user %s → HTTP %d, %d bytes (JSON parse failed: %v)", ...)
    }
    
    // Wrap in MCP response format
    mcpResp := map[string]interface{}{
        "content": []map[string]interface{}{
            {"type": "text", "text": string(body)},
        },
    }
    return mcpResp, nil

Impact Analysis

  • Maintainability: A logging format change or a new field in the permission response (e.g., "role_name") must be made in two places. Historically these drifted — the unified.go copy includes owner/repo context in the message while proxy.go omits it, already showing divergence.
  • Bug Risk: Low — both copies currently do the same thing, but the divergence in log messages is an early sign of drift.
  • Code Bloat: ~15 lines × 2 sites = ~30 duplicated lines.

Refactoring Recommendations

  1. Extract a logAndWrapCollaboratorPermission(body []byte, log logger, ...) interface{} helper

    • Could live in internal/server/unified.go, internal/proxy/proxy.go, or a shared internal package
    • Accepts the raw body, owner/repo/username strings, HTTP status code, and a logger function
    • Returns the mcpResp map
    • Estimated effort: 1–2 hours
  2. Alternatively, consolidate the two call paths so only one place makes the REST call for get_collaborator_permission. The guardBackendCaller in unified.go and the restBackendCaller in proxy.go appear to serve the same purpose for different server modes; a single shared implementation would remove the duplication entirely.

Implementation Checklist

  • Decide: extract helper vs. consolidate call paths
  • Implement chosen approach
  • Ensure log messages are consistent (include owner/repo/username context everywhere)
  • Run make test to verify no regressions

Parent Issue

See parent analysis report: #4325
Related to #4325

Generated by Duplicate Code Detector · ● 2.6M ·

  • expires on Apr 29, 2026, 6:13 AM UTC

Metadata

Metadata

Assignees

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions