[refactor] Semantic function clustering analysis: MCP content parsing duplication and StatusCode shadowing

[github-actions]

Executive Summary

Analysis of 135 non-test Go files across 24 packages in internal/, cataloguing 847 functions. Three significant refactoring opportunities were identified: (1) a repeated MCP content-item parsing pattern spread across three packages, (2) a StatusCode() method in server/response_writer.go that shadows the promoted BaseResponseWriter.StatusCode field it embeds, and (3) a handful of thin wrapper functions that could be consolidated or inlined. No critical duplicates were found; overall the codebase is well-organized.

---

Function Inventory

Package	Files	Functions
internal/auth	1	7
internal/cmd	13	34
internal/config	13	112
internal/config/rules	1	16
internal/difc	9	114
internal/envutil	4	13
internal/guard	11	69
internal/httputil	3	9
internal/jsonutil	1	1
internal/launcher	4	36
internal/logger	16	110
internal/mcp	8	70
internal/middleware	1	15
internal/oidc	1	6
internal/proxy	8	43
internal/server	19	129
internal/strutil	6	14
internal/syncutil	1	1
internal/sys	2	12
internal/testutil/mcptest	4	23
internal/tracing	6	23
internal/tty	1	1
internal/version	1	5

---

Identified Issues

1. MCP Content-Item Parsing Duplicated Across Three Packages (Medium Impact)

Issue: The same {"content":[{"type":"text","text":"..."}]} MCP tool-result format is parsed independently in three separate packages.

Location	Function	Purpose
internal/mcp/tool_result.go:61	convertMapToCallToolResult	Full conversion to SDK content types
internal/server/rate_limit.go:13	extractRateLimitErrorText	Extract text field to detect rate-limit messages
internal/server/rate_limit.go:40	isRateLimitToolResult	Detect rate-limit errors in tool results
internal/difc/path_labels.go:102	unwrapMCPResponse	Unwrap the MCP envelope to access inner JSON

Code comparison (shared pattern):

// server/rate_limit.go
contents, _ := m["content"].([]interface{})
for _, c := range contents {
    cm, ok := c.(map[string]interface{})
    if text, ok := cm["text"].(string); ok && text != "" { ... }
}

// difc/path_labels.go
content, ok := dataMap["content"].([]interface{})
first, ok := content[0].(map[string]interface{})
textType, _ := first["type"].(string)
text, _ := first["text"].(string)

Recommendation: Add a helper to internal/mcp/tool_result.go (already the canonical home of tool-result parsing):

// ExtractTextContentFromResult returns the concatenated text from all text-type
// content items in a raw MCP tool result map, or "" if none are found.
func ExtractTextContentFromResult(result map[string]interface{}) string { ... }

server/rate_limit.go and difc/path_labels.go can then call this instead of re-implementing the traversal.

Estimated effort: 1–2 hours
Benefit: Single source of truth for MCP content unwrapping; fixes immediately propagate to all consumers.

---

2. StatusCode() Method Shadows Promoted BaseResponseWriter.StatusCode Field (Low-Medium Impact)

Issue: internal/server/response_writer.go defines a StatusCode() method that shadows the promoted public field of the same name from the embedded httputil.BaseResponseWriter:

// httputil/response_writer.go
type BaseResponseWriter struct {
    http.ResponseWriter
    StatusCode  int   // ← public field
    wroteHeader bool
}

// server/response_writer.go
type responseWriter struct {
    httputil.BaseResponseWriter   // promotes StatusCode field
    body bytes.Buffer
}

func (w *responseWriter) StatusCode() int {   // ← method shadows the promoted field
    logResponseWriter.Printf("Retrieving captured status code: %d", w.BaseResponseWriter.StatusCode)
    return w.BaseResponseWriter.StatusCode
}

Result:

• rw.StatusCode → calls the method (the field is shadowed and inaccessible by short name)
• rw.BaseResponseWriter.StatusCode → only way to access the field directly

This is inconsistent with the internal/tracing/http.go usage of the same base type:

// tracing/http.go – StatusCode is accessed as a plain field
type statusResponseWriter struct { httputil.BaseResponseWriter }
srw.StatusCode   // ← field access (no shadowing method here)

The method adds only debug logging on access, which is low value for a status-code read.

Recommendation (Option A – preferred): Remove the StatusCode() method from server/response_writer.go and access the field directly via the promoted name, consistent with tracing/http.go:

// Before
lw.StatusCode()   // server/http_helpers.go, server/middleware.go

// After
lw.StatusCode     // promoted field — consistent with tracing usage

Recommendation (Option B): If the logging on access is desired, rename the field in BaseResponseWriter to the unexported statusCode and add an accessor:

type BaseResponseWriter struct {
    http.ResponseWriter
    statusCode  int
    ...
}
func (w *BaseResponseWriter) StatusCode() int { return w.statusCode }

Estimated effort: 30 minutes
Benefit: Removes ambiguity, aligns server and tracing usage patterns.

---

3. Thin Wrapper Functions That Could Be Inlined or Consolidated (Low Impact)

3a. truncateAndSanitize in logger/rpc_helpers.go

// logger/rpc_helpers.go
func truncateAndSanitize(payload string, maxLength int) string {
    sanitized := sanitize.SanitizeString(payload)
    return strutil.Truncate(sanitized, maxLength)
}

This three-line private function is a trivial combinator over two public utilities already in separate packages. Its body could be inlined at its call sites (currently only within the logger package), or the function could be moved to internal/logger/sanitize as SanitizeAndTruncate.

3b. applyAuthIfConfigured / applyHMACIfConfigured one-liner wrappers

// server/auth.go
func applyAuthIfConfigured(key string, h http.HandlerFunc) http.HandlerFunc {
    return applyIfConfigured(key, h, authMiddleware)    // ← one-liner wrapper
}

// server/hmac.go
func applyHMACIfConfigured(secret string, h http.HandlerFunc) http.HandlerFunc {
    return applyIfConfigured(secret, h, hmacMiddleware) // ← one-liner wrapper
}

These are intentional and the naming improves readability at call sites. However, the two wrapper functions could be replaced with direct calls to applyIfConfigured in wrapWithMiddleware (the only caller), reducing indirection. Current design is acceptable; this is a style preference.

Estimated effort: 30–60 minutes
Benefit: Marginal; code is already readable.

---

Semantic Clusters (Well-Organized Areas)

The following packages are well-organized and require no action:

Cluster	Files	Notes
Config validation	validation.go, validation_env.go, validation_schema.go, validation_tracing.go, guard_policy_validation.go	Each file focuses on a distinct validation domain
Config expansion	expand.go, config_stdin.go:expandMapInPlace	expandMapInPlace is a thin stdin-specific wrapper; acceptable
Logger infrastructure	common.go, global_helpers.go, init.go	Generic initLogger[T] / closeGlobalLogger[T] generics avoid repetition across per-logger-type files
Middleware stack	auth.go, hmac.go, middleware.go	Clear layering: generic applyIfConfigured + domain-specific wrappers
HTTP response writer	httputil/response_writer.go, server/response_writer.go, tracing/http.go	Correct use of embedding; see Issue #2 for the naming concern
Rate limiting layers	httputil/github_http.go (header parsing), server/rate_limit.go (tool result parsing), proxy/handler.go (HTTP injection)	Intentional separation by layer
Connection management	mcp/connection.go, mcp/connection_methods.go, mcp/connection_logging.go, mcp/http_transport.go	Clean split by concern

---

Refactoring Recommendations (Priority Order)

Priority 1 — Extract shared MCP content helper (Medium effort, meaningful benefit)

• Add ExtractTextContentFromResult (or similar) to internal/mcp/tool_result.go
• Update server/rate_limit.go and difc/path_labels.go to use it
• Estimated effort: 1–2 hours

Priority 2 — Fix StatusCode() method shadowing (Tiny effort, clarifies intent)

• Remove StatusCode() method from server/response_writer.go
• Update all callers from lw.StatusCode() to lw.StatusCode
• Estimated effort: 30 minutes

Priority 3 — Consider inlining truncateAndSanitize (Cosmetic)

• Only worthwhile if cleaning up the rpc_helpers.go file for other reasons

---

Implementation Checklist

• Investigate and implement Priority 1: ExtractTextContentFromResult helper in internal/mcp/tool_result.go
• Fix Priority 2: resolve StatusCode method/field shadowing in server/response_writer.go
• (Optional) Priority 3: inline or relocate truncateAndSanitize
• Run make agent-finished to verify all changes compile and tests pass

---

Analysis Metadata

Item	Value
Total Go files analyzed	135 (non-test)
Total functions catalogued	847
Packages analyzed	24
Function clusters identified	23 (one per package)
Outliers found	0 (all functions appropriate to their files)
Duplicates detected	1 significant (MCP content parsing, 3 sites)
Naming ambiguity issues	1 (StatusCode method/field shadow)
Detection method	Static grep-based function inventory + semantic pattern matching
Analysis date	2026-06-04T21:32:21Z

Generated by Semantic Function Refactoring · sonnet46 6.5M · ◷