-
Notifications
You must be signed in to change notification settings - Fork 19
[refactor] Semantic Function Clustering: Refactoring Opportunities in Go Source Files #2913
Description
Overview
Automated semantic function clustering analysis of 91 non-test Go files across 23 packages (652 function definitions). The analysis identified 15 oversized files violating single-responsibility principle, 3 misplaced functions, and 1 pure wrapper duplicate. The highest-priority finding is internal/guard/wasm.go at 1,142 lines combining five distinct responsibilities.
1. Duplicate / Redundant Wrapper Function
server/http_helpers.go:writeJSONResponse — pure passthrough wrapper
internal/server/http_helpers.go defines a private one-liner that simply delegates to httputil.WriteJSONResponse:
// internal/server/http_helpers.go
func writeJSONResponse(w http.ResponseWriter, statusCode int, body interface{}) {
httputil.WriteJSONResponse(w, statusCode, body) // pure passthrough, no added behaviour
}4 call sites inside internal/server/ use the wrapper instead of the public function directly (health.go, routed.go, handlers.go ×2).
Recommendation: Remove writeJSONResponse from http_helpers.go and update the 4 callers to call httputil.WriteJSONResponse directly. Zero behaviour change, eliminates one indirection layer.
Effort: < 30 min
2. Misplaced Functions
2a. logRuntimeError in server/auth.go
internal/server/auth.go has two auth-focused functions (authMiddleware, applyAuthIfConfigured) plus a logging helper that builds structured JSON runtime-error log entries:
// internal/server/auth.go:14 — logging helper, not auth logic
func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) { ... }Recommendation: Move logRuntimeError to internal/server/http_helpers.go (already the home for server-scoped HTTP utilities) or a new internal/server/logging.go.
Effort: < 30 min
2b. Generic JSON map helpers in server/difc_log.go
internal/server/difc_log.go (purpose: log DIFC-filtered items) contains three domain-agnostic JSON traversal helpers at lines 64–119:
func getStringField(m map[string]interface{}, fields ...string) string { ... }
func extractAuthorLogin(m map[string]interface{}) string { ... }
func extractNumberField(m map[string]interface{}) string { ... }These are general-purpose helpers, not specific to DIFC logging. The same pattern is independently reinvented in proxy/handler.go.
Recommendation (minimal): Add a comment marking them as local helpers.
Recommendation (better): Extract to a new internal/maputil/ package so any package can import them, preventing further reinvention.
Effort: 1–2 h (if extracting to maputil)
2c. ExpandEnvArgs in config/docker_helpers.go
internal/config/docker_helpers.go:135 exports ExpandEnvArgs — a function that expands environment variables in command argument slices. Its only caller is internal/mcp/connection.go:106 (not the config package):
// internal/mcp/connection.go
expandedArgs := config.ExpandEnvArgs(args)Recommendation: Move ExpandEnvArgs to internal/envutil/ (already exists for env-var utilities) to eliminate the mcp → config dependency for a non-config concern.
Effort: 30–60 min
3. Oversized Files — Candidates for Decomposition
The following files combine multiple distinct responsibilities, making navigation and focused testing harder. All suggested splits stay within the same package; no exported API changes.
3a. internal/guard/wasm.go — 1,142 lines ⚠️ CRITICAL
Five distinct responsibilities in one file:
| Responsibility | Key Functions | Suggested File |
|---|---|---|
| Guard lifecycle (constructor, Close) | NewWasmGuard*, Close |
keep in wasm.go |
| WASM runtime / memory management | callWasmFunction, tryCallWasmFunction, wasmAlloc, wasmDealloc, isWasmTrap |
wasm_runtime.go |
| Host function bindings | instantiateHostFunctions, hostCallBackend, hostLog |
wasm_host.go |
| Payload building | BuildLabelAgentPayload, buildStrictLabelAgentPayload, normalizePolicyPayload, isValidAllowOnlyRepos |
wasm_payload.go |
| Response parsing | parseLabelAgentResponse, parseResourceResponse, parseCollectionLabeledData, parsePathLabeledResponse, checkBoolFailure |
wasm_parse.go |
Recommendation: Split into wasm.go + wasm_runtime.go + wasm_host.go + wasm_payload.go + wasm_parse.go.
Effort: 3–4 h
3b. internal/config/guard_policy.go — 721 lines
Four distinct responsibilities:
| Responsibility | Key Functions | Suggested File |
|---|---|---|
| Core types + (un)marshaling | UnmarshalJSON ×2, MarshalJSON ×2, IsWriteSinkPolicy |
keep in guard_policy.go |
| Validation | ValidateGuardPolicy, ValidateWriteSinkPolicy, validateAcceptEntry, isValidRepo*, isScopeTokenChar, validateGuardPolicies |
guard_policy_validate.go |
| Parsing | ParseGuardPolicyJSON, ParsePolicyMap, ParseServerGuardPolicy, BuildAllowOnlyPolicy |
guard_policy_parse.go |
| Normalization | NormalizeGuardPolicy, normalizeAndValidateScopeArray, NormalizeScopeKind |
guard_policy_normalize.go |
Effort: 2–3 h
3c. internal/server/unified.go — 714 lines
Combines core server setup, tool execution, DIFC integration, backend calling, lifecycle/shutdown, and enrichment token lookup:
| Responsibility | Key Functions | Suggested File |
|---|---|---|
| Core server + lifecycle | NewUnified, Run, Close, IsShutdown, InitiateShutdown, ShouldExit, SetHTTPShutdown, GetHTTPShutdown |
keep in unified.go |
| Tool execution + backend calling | callBackendTool, executeBackendToolCall, newErrorCallToolResult, guardBackendCaller |
unified_tools.go |
| Enrichment / env lookups | lookupEnrichmentToken, lookupGitHubAPIBaseURL |
unified_env.go |
| Status / introspection | GetServerIDs, GetServerStatus, GetToolsForBackend, GetToolHandler, GetPayloadSizeThreshold, IsDIFCEnabled, RegisterTestTool, SetTestMode |
unified_status.go |
Effort: 3–4 h
3d. internal/mcp/connection.go — 669 lines
Mixes connection construction, reconnection, session management, and MCP method wrappers:
| Responsibility | Suggested File |
|---|---|
| Core connection lifecycle | keep in connection.go |
| Send / reconnect logic | connection_send.go |
MCP method wrappers (listTools, callTool, listResources, etc.) |
connection_methods.go |
Effort: 2–3 h
3e. internal/mcp/http_transport.go — 622 lines
Mixes client construction, transport probing (streamable/SSE/plain-JSON), and request/response handling:
| Responsibility | Suggested File |
|---|---|
| Client construction + RoundTrip | keep in http_transport.go |
Transport probing (trySDKTransport, tryStreamableHTTPTransport, trySSETransport, tryPlainJSONTransport) |
http_transport_probe.go |
| Request/response execution | http_transport_request.go |
Effort: 2–3 h
3f. internal/config/validation_schema.go — 551 lines
Mixes HTTP schema fetching, JSON-Schema compilation/validation, and multi-level error formatting:
| Responsibility | Suggested File |
|---|---|
| Schema fetching + HTTP retry | validation_schema_fetch.go |
| Schema compilation + validation | keep in validation_schema.go |
Error formatting (formatSchemaError, formatValidationErrorRecursive, formatErrorContext) |
validation_schema_format.go |
Effort: 1–2 h
3g. internal/config/config_stdin.go — 515 lines
Mixes JSON parsing, type conversion/normalization, field stripping, and variable expansion:
| Responsibility | Suggested File |
|---|---|
| JSON parsing + top-level loading | keep in config_stdin.go |
| Type conversion + normalization | config_stdin_convert.go |
Effort: 1–2 h
3h. internal/config/validation.go — 465 lines
Mixes variable expansion, mount validation, server config validation, auth validation, gateway validation, and trusted bots validation:
| Responsibility | Suggested File |
|---|---|
| Variable expansion + core dispatch | keep in validation.go |
| Server mount validation | validation_mounts.go |
| Auth + gateway + trusted-bots validation | validation_auth.go |
Effort: 2–3 h
3i. Other large files (moderate priority)
| File | Lines | Suggested Action |
|---|---|---|
internal/server/guard_init.go |
408 | Split guard registration / policy resolution / WASM discovery |
internal/server/tool_registry.go |
406 | Split tool registration from parallel/sequential launch strategies |
internal/proxy/handler.go |
531 | Split HTTP handler from response restructuring helpers |
internal/proxy/router.go |
444 | Split routing logic from middleware chain assembly |
internal/middleware/jqschema.go |
430 | Split schema transform from file I/O for payloads |
internal/difc/evaluator.go |
449 | Split flow evaluation from label propagation logic |
4. Intentional Patterns (No Action Needed)
The following appear repetitive but are correctly structured:
withLockon each logger type — identical body per type; correct because each is on a different receiver and Go has no mixins.setup*Logger/handle*LoggerError— different fallback strategies per logger type (stdout, silent, strict, unified); differentiation is intentional.Log{Info,Warn,Error,Debug}[WithServer]families — three public APIs with distinct signatures; one-liner wrappers are idiomatic Go.- Session ID extraction split (
extractAndValidateSessionvsSessionIDFromContext) — different extraction points (header vs. context); not a duplicate.
Implementation Checklist
Quick wins (< 1 hour each)
- Remove
server/http_helpers.go:writeJSONResponsewrapper; update 4 callers to usehttputil.WriteJSONResponsedirectly - Move
server/auth.go:logRuntimeErrortoserver/http_helpers.goor newserver/logging.go - Move
config/docker_helpers.go:ExpandEnvArgstointernal/envutil/
Medium effort — split large files (no API breakage, all internal)
- Split
guard/wasm.go→wasm.go+wasm_runtime.go+wasm_host.go+wasm_payload.go+wasm_parse.go - Split
config/guard_policy.go→ core +_validate.go+_parse.go+_normalize.go - Split
server/unified.go→ core +unified_tools.go+unified_env.go+unified_status.go - Split
mcp/connection.go→ core +connection_send.go+connection_methods.go - Split
mcp/http_transport.go→ core +http_transport_probe.go+http_transport_request.go - Split
config/validation_schema.go→ core +_fetch.go+_format.go - Split
config/config_stdin.go→ parser +_convert.go - Split
config/validation.go→ core +validation_mounts.go+validation_auth.go
Optional / longer term
- Extract
getStringField/extractAuthorLogin/extractNumberFieldtointernal/maputil/to prevent pattern reinvention - Review and split remaining >400-line files in
server/,proxy/,middleware/,difc/
Analysis Metadata
| Metric | Value |
|---|---|
| Go files analyzed (non-test) | 91 |
| Function definitions cataloged | 652 |
| Packages covered | 23 |
| Clear duplicate wrappers | 1 (writeJSONResponse) |
| Misplaced functions | 3 |
| Files recommended for split (critical) | 8 |
| Files recommended for split (moderate) | 6 |
| Analysis date | 2026-03-31 |
References: §23792958258
Generated by Semantic Function Refactoring · ◷