Skip to content

[refactor] Semantic function clustering: thin files and one outlier function to consolidate #6810

Description

@github-actions

Analysis of repository: github/gh-aw-mcpg

Executive Summary

Analyzed 137 non-test Go source files across internal/ containing 847 functions. The overall codebase is well-structured with consistent naming conventions and clear package responsibilities. The analysis identified a small number of actionable refactoring opportunities, primarily around thin single-function files that could be consolidated and one utility function that is an outlier in its current file.

No significant duplicate function implementations were found — the codebase is generally well-factored. The main findings are 3 consolidation opportunities and 1 outlier function to relocate.


Function Inventory Summary

Package Files Functions Notes
internal/difc/ 8 ~100 Well-split by concern
internal/logger/ 14 ~80 Many small files
internal/config/ 14 ~85 Well-organized by concern
internal/guard/ 9 ~65 Wasm files well-split
internal/server/ 18 ~110 A few thin files
internal/mcp/ 7 ~65 Good separation
internal/cmd/ 11 ~35 Mostly flag registration
internal/proxy/ 7 ~40 Good separation
internal/strutil/ 6 ~15 Several single-function files
internal/envutil/ 4 ~15 Clean
Others 19 ~40 Various

Identified Issues

1. internal/server/transport.go — Single-Function Thin Wrapper

Issue: transport.go contains exactly one function (CreateHTTPServerForMCP) that is a thin wrapper calling buildMCPHTTPServer from http_server.go. It adds routing logic specific to streamable HTTP transport but the file boundary creates unnecessary indirection.

File: internal/server/transport.go
Function: CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey, hmacSecret string) *http.Server

Related file: internal/server/http_server.go contains buildMCPHTTPServer (the common scaffold) and newHTTPServer. routed.go also calls buildMCPHTTPServer directly without a dedicated wrapper file.

Code:

// transport.go — entire functional content
func CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey, hmacSecret string) *http.Server {
    return buildMCPHTTPServer(addr, unifiedServer, apiKey, hmacSecret, func(mux *http.ServeMux, sessionTimeout time.Duration) {
        // Registers /mcp and /mcp/ handlers
        ...
    })
}

Recommendation: Merge CreateHTTPServerForMCP into http_server.go alongside the other server-construction functions. Alternatively, move it into unified.go since it's the streamable-HTTP entry point used alongside routed.go.

Estimated Effort: 30 minutes
Impact: Reduced file count, improved co-location of related HTTP server construction logic


2. internal/logger/startup.go — Two-Function Wrapper File

Issue: startup.go contains only two thin passthrough functions (StartupInfo and StartupWarn) that wrap the global file logger. These functions are used by internal/cmd/ for startup-phase messages but don't warrant a standalone file.

File: internal/logger/startup.go
Functions:

func StartupInfo(format string, args ...interface{}) { ... }
func StartupWarn(format string, args ...interface{}) { ... }

Recommendation: Move StartupInfo and StartupWarn into internal/logger/global_helpers.go, which already houses generic helper functions for managing global logger state. The startup.go file can then be removed.

Estimated Effort: 15 minutes
Impact: Fewer files, startup helpers co-located with other global logger helpers


3. clientAddr() Utility Function in internal/cmd/proxy.go — Outlier Location

Issue: proxy.go contains clientAddr(), a pure string manipulation utility that converts a listener address to a client-friendly form by substituting wildcard hosts with localhost. This function has no dependency on proxy-specific types and its logic is generic enough to belong in internal/strutil/.

File: internal/cmd/proxy.go (line 326)
Function:

// clientAddr returns a client-friendly address from a listener address.
// When the host is a wildcard (0.0.0.0, ::, or empty), substitutes "localhost".
func clientAddr(addr string) string {
    host, port, err := net.SplitHostPort(addr)
    ...
}

Context: This function is called once in runProxy() for display purposes. It is tested in proxy_test.go.

Recommendation: Move to internal/strutil/ as NormalizeListenAddr(addr string) string (or keep as an unexported helper in a dedicated internal/cmd/addr.go file if the move to strutil is too broad). If moved, update the test accordingly.

Note: configureTLSTrustEnvironment() in the same file is command-specific (sets environment variables for TLS trust) and appropriately lives in the cmd package.

Estimated Effort: 30 minutes
Impact: Improves discoverability of the address-normalization utility, separates concerns in proxy.go


4. internal/strutil/ — Several Single-Function Files (Low Priority)

Issue: The strutil package has several files each containing a single exported function:

  • map.goGetStringFromMap(m map[string]interface{}, key string) string
  • deep_clone_json.goDeepCloneJSON(v interface{}) interface{}
  • deduplicate.goDeduplicateStrings(input []string, sorted bool) []string

Each of these is tiny (~10–20 lines). This is acceptable Go style, but given they are all general-purpose utilities, consolidating them into a single util.go or grouping by concern (e.g., collections.go for deduplicate/map, json.go for deep clone) would reduce the file count.

Recommendation: Low priority — Go's one-concept-per-file approach is a valid pattern. Only consolidate if the strutil package grows significantly. The random_hex.go and truncate.go files have sufficient depth to stand alone.

Estimated Effort: 20 minutes (if pursued)
Impact: Marginally reduced file count in strutil/


What Was NOT Found (Positives)

  • No duplicate function implementations across packages — similar utility patterns (log helpers, validate functions) are properly scoped to their respective types/packages
  • response_writer.go duplication is intentionalhttputil.BaseResponseWriter provides a shared base type; server.responseWriter correctly embeds it and adds body buffering
  • Guard wasm_*.go split is well-organizedwasm.go (runtime), wasm_parse.go (response parsing), wasm_payload.go (payload construction), wasm_validate.go (input validation) are clearly separated
  • server/auth.go correctly delegates to the generic applyIfConfigured in middleware.go — no duplication
  • launcher/log_helpers.go and mcp/connection_logging.go — analogous patterns in separate packages, appropriate

Refactoring Recommendations by Priority

Priority 1: Quick Wins (< 1 hour total)

  1. Merge server/transport.go into server/http_server.go or server/unified.go

    • Move CreateHTTPServerForMCP function
    • Update any callers
    • Delete transport.go
  2. Merge logger/startup.go into logger/global_helpers.go

    • Move StartupInfo and StartupWarn
    • Delete startup.go

Priority 2: Medium Impact (1–2 hours)

  1. Relocate clientAddr() from cmd/proxy.go
    • Option A: Move to internal/strutil/ as NormalizeListenAddr
    • Option B: Extract to internal/cmd/addr.go for command-specific addr utils
    • Update test file location accordingly

Priority 3: Long-term / Low Priority

  1. Consolidate strutil/ single-function files if the package grows further

Implementation Checklist

  • Merge server/transport.goserver/http_server.go or server/unified.go
  • Merge logger/startup.gologger/global_helpers.go
  • Relocate clientAddr() from cmd/proxy.go to strutil or cmd/addr.go
  • Run make agent-finished to verify no regressions
  • Consider strutil/ consolidation as follow-up

Analysis Metadata

  • Total Go Files Analyzed: 137 (non-test, internal/)
  • Total Functions Cataloged: 847
  • Outlier Functions Found: 1 (clientAddr in cmd/proxy.go)
  • Thin Files (≤2 functions): 2 (server/transport.go, logger/startup.go)
  • Duplicate Implementations Detected: 0
  • Scattered Utility Patterns: 1 (strutil/ single-function files, low priority)
  • Detection Method: Static grep-based function extraction + naming pattern analysis + cross-reference analysis
  • Analysis Date: 2026-05-31

Generated by Semantic Function Refactoring · sonnet46 5M ·

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions