[duplicate-code] Duplicate Code Pattern: Numeric Argument Extraction in restBackendCaller

[github-actions]

🔍 Duplicate Code Pattern: Numeric Argument Extraction in restBackendCaller

Part of duplicate code analysis: #6728

Summary

In internal/proxy/proxy.go, the restBackendCaller.CallTool function contains two nearly-identical ~10-line blocks for extracting a numeric resource ID from JSON args — one for pull_request_read and one for issue_read. Both blocks extract owner, repo, and a numeric ID, with identical fallback logic for float64 JSON numbers.

Duplication Details

Pattern: Float64/string numeric ID extraction

• Severity: Medium
• Occurrences: 2 instances (expandable as more REST tools are added)
• Locations:
  • internal/proxy/proxy.go (lines ~225–236, pull_request_read case)
  • internal/proxy/proxy.go (lines ~238–249, issue_read case)
• Code Sample:

  // pull_request_read case:
  owner, _ := argsMap["owner"].(string)
  repo, _ := argsMap["repo"].(string)
  number, _ := argsMap["pullNumber"].(string)
  if number == "" {
      if n, ok := argsMap["pullNumber"].(float64); ok {
          number = fmt.Sprintf("%d", int(n))
      }
  }
  if owner == "" || repo == "" || number == "" {
      return nil, fmt.Errorf("pull_request_read: missing owner/repo/pullNumber")
  }
  apiPath = fmt.Sprintf("/repos/%s/%s/pulls/%s", owner, repo, number)
  
  // issue_read case (nearly identical):
  owner, _ := argsMap["owner"].(string)
  repo, _ := argsMap["repo"].(string)
  number, _ := argsMap["issue_number"].(string)
  if number == "" {
      if n, ok := argsMap["issue_number"].(float64); ok {
          number = fmt.Sprintf("%d", int(n))
      }
  }
  if owner == "" || repo == "" || number == "" {
      return nil, fmt.Errorf("issue_read: missing owner/repo/issue_number")
  }
  apiPath = fmt.Sprintf("/repos/%s/%s/issues/%s", owner, repo, number)

Impact Analysis

• Maintainability: Adding new resource-type tools (e.g., discussion_read) will likely copy-paste this pattern again
• Bug Risk: A fix to the float64 fallback logic must be applied in both places — easy to miss one
• Code Bloat: Minor now; grows with each new tool added to the switch

Refactoring Recommendations

1. Extract a helper extractOwnerRepoNumber(argsMap, ownerKey, repoKey, numberKey string) (owner, repo, number string, err error) that handles the float64 fallback and missing-field error:

   func extractOwnerRepoNumber(argsMap map[string]interface{}, ownerKey, repoKey, numberKey, toolName string) (owner, repo, number string, err error) {
       owner, _ = argsMap[ownerKey].(string)
       repo, _ = argsMap[repoKey].(string)
       number, _ = argsMap[numberKey].(string)
       if number == "" {
           if n, ok := argsMap[numberKey].(float64); ok {
               number = fmt.Sprintf("%d", int(n))
           }
       }
       if owner == "" || repo == "" || number == "" {
           err = fmt.Errorf("%s: missing %s/%s/%s", toolName, ownerKey, repoKey, numberKey)
       }
       return
   }

   • Estimated effort: 30 minutes
   • Benefits: Single fix point for float64 fallback logic, cleaner switch cases

Implementation Checklist

• Review duplication findings
• Extract extractOwnerRepoNumber helper in internal/proxy/proxy.go or internal/proxy/collaborator_permission.go
• Refactor pull_request_read and issue_read cases to use the helper
• Update tests
• Verify no functionality broken

Parent Issue

See parent analysis report: #6728
Related to #6728

Generated by Duplicate Code Detector · sonnet46 1.7M · ◷

• expires on Jun 6, 2026, 3:39 AM UTC