-
Notifications
You must be signed in to change notification settings - Fork 19
[refactor] Semantic Function Clustering: Refactoring Opportunities in Go Source Files #3113
Description
Overview
Automated semantic function clustering analysis of 92 non-test Go files across 23 packages (662 function definitions). Since the previous analysis (§23896173526), one new compliance function was added (isMalformedAuthHeader in server/auth.go, correctly placed) and one new crypto utility (generateRandomAPIKey in cmd/root.go, candidate for relocation). All findings from the prior analysis remain open.
Progress Since Last Analysis
| Item | Status |
|---|---|
Remove writeJSONResponse passthrough wrapper |
✅ Done |
Add Config.EnsureGatewayDefaults() |
✅ Done |
Add isMalformedAuthHeader for HTTP 400 compliance |
✅ Done (correctly placed in server/auth.go) |
Move logRuntimeError out of server/auth.go |
⬜ Open |
Move ExpandEnvArgs out of config/docker_helpers.go |
⬜ Open |
Extract generic JSON helpers from server/difc_log.go |
⬜ Open |
| Split oversized files (8 critical, 7 moderate) | ⬜ Open |
1. Misplaced Functions
1a. logRuntimeError in server/auth.go
internal/server/auth.go contains auth-focused functions (authMiddleware, applyAuthIfConfigured, isMalformedAuthHeader) plus a logging helper that builds structured runtime-error log entries:
// internal/server/auth.go:14 — logging helper, not auth logic
func logRuntimeError(errorType, detail string, r *http.Request, serverName *string) { ... }Its only caller is rejectRequest in internal/server/http_helpers.go:41. The function has no conceptual tie to authentication.
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
1b. 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 non-test caller is internal/mcp/connection.go:107:
// internal/mcp/connection.go
expandedArgs := config.ExpandEnvArgs(args)This creates an mcp → config dependency for a non-config concern. All other functions in docker_helpers.go are Docker-specific validators — ExpandEnvArgs is a generic env-util.
Recommendation: Move ExpandEnvArgs to internal/envutil/ (already exists for env-var utilities) to eliminate the cross-package dependency.
Effort: 30–60 min
1c. generateRandomAPIKey in cmd/root.go (New)
internal/cmd/root.go:593 contains generateRandomAPIKey(), a cryptographic utility that generates a 32-byte random hex-encoded key. This is buried inside the 614-line CLI root file, far from the auth-related code it supports.
func generateRandomAPIKey() (string, error) {
bytes := make([]byte, 32)
if _, err := rand.Read(bytes); err != nil { ... }
return hex.EncodeToString(bytes), nil
}A similar (but distinct) function randomSerial() exists in internal/proxy/tls.go for TLS cert serial numbers — both are crypto-random utilities in unrelated packages.
Recommendation: Move generateRandomAPIKey to internal/auth/ (the auth package already handles key parsing/validation) to group all auth-key utilities together.
Effort: < 30 min
2. Scattered Generic Helpers
2a. Generic JSON map helpers in server/difc_log.go
internal/server/difc_log.go:64–103 contains three domain-agnostic JSON traversal helpers:
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. internal/proxy/handler.go independently operates on map[string]interface{} values, confirming the risk of further reinvention.
Recommendation (minimal): Add a comment marking them as local helpers to deter copy-paste.
Recommendation (better): Extract to a new internal/maputil/ package so any package can import them.
Effort: 1–2 h (if extracting to maputil)
3. Oversized Files — Candidates for Decomposition
Files combining multiple distinct responsibilities. All suggested splits stay within the same package; no exported API changes since all are under internal/.
3a. internal/guard/wasm.go — 1,168 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*, 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 — 680 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 — 643 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/cmd/root.go — 614 lines
Mixes CLI lifecycle (run, preRun, postRun), flag-resolution helpers, env-file loading, config writing, and now crypto key generation:
| Responsibility | Key Functions | Suggested File |
|---|---|---|
| Core CLI lifecycle | init, preRun, run, postRun, Execute, SetVersion |
keep in root.go |
| Helper utilities | resolveGuardPolicyOverride, writeGatewayConfigToStdout, writeGatewayConfig, loadEnvFile, generateRandomAPIKey |
cmd/run_helpers.go |
Effort: 1–2 h
3g. internal/config/validation_schema.go — 550 lines
Mixes HTTP schema fetching, JSON-Schema compilation/validation, and multi-level error formatting:
| Responsibility | Suggested File |
|---|---|
Schema fetching + HTTP retry (isTransientHTTPError, fetchAndFixSchema, fixSchemaBytes) |
validation_schema_fetch.go |
Schema compilation + validation (getOrCompileSchema, validateJSONSchema) |
keep in validation_schema.go |
Error formatting (formatSchemaError, formatValidationErrorRecursive, formatErrorContext) |
validation_schema_format.go |
Effort: 1–2 h
3h. internal/config/config_stdin.go — 517 lines
Mixes JSON parsing, type conversion/normalization, field stripping, and variable expansion:
| Responsibility | Suggested File |
|---|---|
JSON parsing + top-level loading (LoadFromStdin, UnmarshalJSON, stripExtensionFieldsForValidation) |
keep in config_stdin.go |
Type conversion + normalization (convertStdinConfig, convertStdinServerConfig, buildStdioServerConfig, normalizeLocalType) |
config_stdin_convert.go |
Effort: 1–2 h
3i. 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
3j. Other large files (moderate priority)
| File | Lines | Suggested Action |
|---|---|---|
internal/proxy/handler.go |
534 | Split HTTP handler from response restructuring helpers (rewrapSearchResponse, rebuildGraphQLResponse, replaceNodesArray, deepCloneJSON) |
internal/difc/evaluator.go |
449 | Split flow evaluation from label propagation logic |
internal/proxy/router.go |
444 | Split routing table from MatchRoute/StripGHHostPrefix functions |
internal/middleware/jqschema.go |
456 | Split schema transform from file I/O for payloads |
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/proxy.go |
416 | Split proxy core from middleware/transport configuration |
4. Intentional Patterns (No Action Needed)
withLockon each logger type — identical body per type; correct because each is on a different receiver.setup*Logger/handle*LoggerError— different fallback strategies per logger type; 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. paginateAll[T]()— generic pagination helper, correctly placed inconnection.go.applyGatewayDefaults(private) +EnsureGatewayDefaults(public) — clean encapsulation; both inconfig_core.go. Well-structured.isMalformedAuthHeader— new function (compliance fix), correctly placed inserver/auth.go.randomSerial()vsgenerateRandomAPIKey()— similar crypto-random patterns but serving distinct purposes (TLS cert serial vs API key generation); not true duplicates.
Implementation Checklist
Quick wins (< 1 hour each)
- Move
server/auth.go:logRuntimeErrortoserver/http_helpers.goor newserver/logging.go - Move
config/docker_helpers.go:ExpandEnvArgstointernal/envutil/ - Move
cmd/root.go:generateRandomAPIKeytointernal/auth/(groups with key parsing/validation)
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
cmd/root.go→ core +cmd/run_helpers.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
proxy/,middleware/,difc/,server/
Analysis Metadata
| Metric | Value |
|---|---|
| Go files analyzed (non-test) | 92 |
| Function definitions cataloged | 662 |
| Packages covered | 23 |
| Resolved since last analysis | 3 (writeJSONResponse, EnsureGatewayDefaults, isMalformedAuthHeader placement) |
| Misplaced functions | 3 (logRuntimeError, ExpandEnvArgs, generateRandomAPIKey) |
| Scattered helpers | 3 (getStringField, extractAuthorLogin, extractNumberField) |
| Files recommended for split (critical) | 9 |
| Files recommended for split (moderate) | 7 |
| Analysis date | 2026-04-03 |
References: §23943157099
Generated by Semantic Function Refactoring · ◷