[refactor] Semantic Function Clustering Analysis — 3 Targeted Refactoring Opportunities

[github-actions]

Analysis of repository: github/gh-aw-mcpg

Overview

Analysis of 126 non-test Go source files across 23 packages in internal/, cataloging 807 functions/methods. The codebase is generally well-organized with a clear package-per-concern structure. No exact function duplicates were found. Three targeted refactoring opportunities were identified: a cohesion gap in the cmd/ package, three trivially inlineable private logging helpers, and a utility function that would be more discoverable in a shared package.

Function Inventory

By Package (files → approx. functions)

Package	Files	Notes
internal/server	18	Largest package; well decomposed by concern
internal/logger	16	Intentional multi-file design with shared patterns
internal/cmd	13	5 of 13 files are flag registration files
internal/config	12	Config split well by feature area
internal/guard	11	WASM + policy logic
internal/difc	9	Labels, evaluator, capabilities, agents
internal/mcp	10	Protocol types + transport
internal/proxy	7	Handler, router, transforms
internal/strutil	4	Generic string utilities
internal/envutil	4	Environment variable utilities
internal/launcher	4	Process lifecycle

---

Identified Issues

1. detectGuardWasm() and containerGuardWasmPath placed in proxy.go instead of wasm_cache.go

File: internal/cmd/proxy.go (lines 58–73)
Issue: The constant containerGuardWasmPath and the function detectGuardWasm() are WASM guard detection concerns, yet they live in proxy.go alongside general server startup logic. The file internal/cmd/wasm_cache.go already exists specifically to house WASM-related setup (defaultWasmCacheDir, resolveWasmCacheDir, configureWasmCompilationCache).

Current placement (proxy.go):

// containerGuardWasmPath is the baked-in guard path in the container image.
const containerGuardWasmPath = "/guards/github/00-github-guard.wasm"

func detectGuardWasm() string {
    logProxyCmd.Printf("Checking for baked-in guard at %s", containerGuardWasmPath)
    if _, err := os.Stat(containerGuardWasmPath); err == nil {
        logProxyCmd.Printf("Auto-detected baked-in guard: %s", containerGuardWasmPath)
        return containerGuardWasmPath
    }
    logProxyCmd.Print("Baked-in guard not found, --guard-wasm flag required")
    return ""
}

Recommendation: Move containerGuardWasmPath and detectGuardWasm() into internal/cmd/wasm_cache.go. This consolidates all WASM-related CLI setup in one file and reduces cognitive scatter in the already-large proxy.go.

Estimated Impact: Minor — improves cohesion; no behavioral change.

---

2. Three private 1-line logging helpers in config/validation.go that could be inlined

File: internal/config/validation.go (lines 22–35)
Issue: Three private functions exist solely to wrap single logValidation.Printf(...) calls:

func logValidateServerStart(name, serverType string) {
    logValidation.Printf("Validating server config: name=%s, type=%s", name, serverType)
}
func logValidateServerPassed(name, serverType string) {
    logValidation.Printf("Server config validation passed: name=%s, type=%s", name, serverType)
}
func logValidateServerFailed(name, serverType, reason string) {
    logValidation.Printf("Validation failed: %s, name=%s, type=%s", reason, name, serverType)
}

These are called exactly once each in validateServerConfigWithCustomSchemas. The indirection adds a layer without meaningful benefit — the log message formats are fixed and not reused.

Recommendation: Inline the logValidation.Printf(...) calls at the three call sites and remove the helpers. This shrinks the file's function count by 3 and makes the validation flow directly readable.

Estimated Impact: Minor — reduces function count, improves readability of the validation flow.

---

3. deepCloneJSON generic utility buried in proxy/response_transform.go

File: internal/proxy/response_transform.go (lines 141–157)
Issue: deepCloneJSON is a general-purpose recursive deep-clone for interface{} JSON values. It has no proxy-specific logic and is only used internally within response_transform.go, but as a utility it would be more discoverable in internal/strutil or a future internal/jsonutil package if needed elsewhere.

func deepCloneJSON(v interface{}) interface{} {
    switch v := v.(type) {
    case map[string]interface{}:
        clone := make(map[string]interface{}, len(v))
        for k, v := range v { clone[k] = deepCloneJSON(v) }
        return clone
    case []interface{}:
        clone := make([]interface{}, len(v))
        for i, v := range v { clone[i] = deepCloneJSON(v) }
        return clone
    default:
        return v
    }
}

Note: Since it is currently only called within response_transform.go, moving it is low priority unless similar patterns appear elsewhere.

Recommendation: If a second caller ever emerges, move to internal/strutil or a new internal/jsonutil package.

Estimated Impact: Low — only relevant if the function is needed in other packages.

---

What Was Checked and Found Well-Organized

• auth.TruncateSessionID in internal/auth/header.go: Widely used in server/ for safe session ID logging. Location is correct — auth-domain concern.
• config/expand.go vs envutil/expand_env_args.go: Different scopes — JSON config variable substitution vs CLI argument expansion. Not duplicates.
• logger/sanitize.TruncateSecret: Correctly uses strutil.TruncateWithSuffix rather than duplicating truncation logic.
• cmd/guard_policy.go: Thin Cobra-aware wrapper over config.ResolveGuardPolicyOverride. Appropriate split.
• logWithLevel vs logWithLevelAndServer: Intentional parallel structure, documented in logger/common.go with the "Log-Level Quad-Function Pattern" comment.
• repoArgs in proxy/router.go: Used 10+ times within the same file; extraction is correct.

---

Refactoring Recommendations

Priority	Action	File(s)	Effort
High	Move detectGuardWasm() + containerGuardWasmPath to wasm_cache.go	cmd/proxy.go → cmd/wasm_cache.go	15 min
Medium	Inline 3 private logValidateServer* helpers	config/validation.go	10 min
Low	Consider moving deepCloneJSON to strutil if reuse emerges	proxy/response_transform.go	Future

Implementation Checklist

• Move containerGuardWasmPath constant and detectGuardWasm() from cmd/proxy.go to cmd/wasm_cache.go
• Update the logger reference in detectGuardWasm to use an appropriate logger in wasm_cache.go
• Inline the three logValidate* call sites in validateServerConfigWithCustomSchemas and delete the helper functions
• Run make agent-finished to verify build + tests pass after changes

Analysis Metadata

• Total Go Files Analyzed: 126 (non-test, internal/ directory)
• Total Functions Cataloged: 807
• Packages Analyzed: 23
• Outliers Found: 1 (cohesion) + 2 minor
• Exact Duplicates Detected: 0
• Detection Method: Static function inventory + naming pattern clustering + call-site analysis
• Analysis Date: 2026-05-15T21:20:27Z

Generated by Semantic Function Refactoring · ● 1.1M · ◷