[refactor] Semantic Function Clustering Analysis — Code Organization & Duplication

[github-actions]

Overview

Automated semantic analysis of 97 non-test Go files across 22 packages in internal/ identified 8 actionable refactoring opportunities. All 6 findings from the prior analysis (#4330) remain unresolved. Two new findings were identified in the tracing and proxy packages added since the prior run.

Analysis date: 2026-04-23 · Run: §24830913327

---

Identified Issues

1. 🔴 Duplicate JSON-RPC Types (server/sdk_logging.go ↔ mcp/types.go) (unchanged)

server/sdk_logging.go defines three types that are structurally identical to types already in mcp/types.go:

server/sdk_logging.go	mcp/types.go	JSON fields
JSONRPCRequest	mcp.Request	jsonrpc, id, method, params
JSONRPCResponse	mcp.Response	jsonrpc, id, result, error
JSONRPCError	mcp.ResponseError	code, message, data

The local types in sdk_logging.go are used exclusively within that file. There is no structural difference from the mcp package types.

Recommendation: Replace JSONRPCRequest, JSONRPCResponse, JSONRPCError in server/sdk_logging.go with mcp.Request, mcp.Response, and mcp.ResponseError. Estimated effort: ~30 min.

---

2. 🔴 Docker Helpers Misplaced in config Package (unchanged)

internal/config/docker_helpers.go contains 6 Docker inspection/validation functions that execute shell commands (docker inspect, docker info):

Function	Purpose
checkDockerAccessible()	Verifies Docker daemon reachability
validateContainerID()	Validates container ID hex format
runDockerInspect()	Executes docker inspect with a format template
checkPortMapping()	Verifies port bindings
checkStdinInteractive()	Checks -i flag presence
checkLogDirMounted()	Checks volume mounts

These are about runtime system inspection, not configuration parsing. internal/sys/container.go already handles container detection — this is the natural home.

Recommendation: Move docker_helpers.go to internal/sys/docker.go and update the import in config/validation_env.go. Estimated effort: ~1 hour.

---

3. 🔴 sessionSuffix Pattern Duplicated Across Packages (unchanged)

launcher/log_helpers.go defines:

func sessionSuffix(sessionID string) string {
    if sessionID == "" {
        return ""
    }
    return fmt.Sprintf(" for session '%s'", sessionID)
}

mcp/errors.go inlines the identical pattern:

suffix := ""
if errCtx.SessionID != "" {
    suffix = " for session '" + errCtx.SessionID + "'"
}

Recommendation: Promote sessionSuffix to internal/strutil (e.g., strutil.SessionSuffix) so both packages share one implementation. Estimated effort: ~30 min.

---

4. 🟡 server/sdk_logging.go Will Become a Single-Function File After Fix #1 (unchanged)

After removing the three duplicate type definitions (Finding 1), sdk_logging.go reduces to a single exported function WithSDKLogging. http_helpers.go in the same package already consolidates HTTP middleware concerns and is the natural home.

Recommendation: After removing the duplicate types, move WithSDKLogging into http_helpers.go and delete sdk_logging.go. Estimated effort: ~30 min (after #1).

---

5. 🟡 isEffectivelyEmpty Lives in the Wrong File (unchanged)

internal/logger/rpc_helpers.go defines isEffectivelyEmpty but it is only ever called from rpc_formatter.go (line 75). Having it in a separate helper file creates cross-file dependencies that make each file non-self-contained.

Recommendation: Move isEffectivelyEmpty from rpc_helpers.go to rpc_formatter.go. Estimated effort: ~15 min.

---

6. 🟢 formatResetAt Could Use strutil.FormatDuration (unchanged)

internal/server/circuit_breaker.go:

func formatResetAt(t time.Time) string {
    if t.IsZero() {
        return "unknown"
    }
    return fmt.Sprintf("%s (in %s)", t.UTC().Format(time.RFC3339), time.Until(t).Round(time.Second))
}

The inner time.Until(t).Round(time.Second) uses Go's default duration formatting, while strutil.FormatDuration provides the codebase-standard human-readable format. Consistent duration display is the standard across all log output.

Recommendation: Replace the inner duration formatting with strutil.FormatDuration(time.Until(t).Round(time.Second)). Estimated effort: ~10 min.

---

7. 🟡 Near-Duplicate OTel HTTP Wrapping: server.WithOTELTracing ↔ tracing.WrapHTTPHandler (new)

internal/tracing/http.go (added recently) provides a general WrapHTTPHandler function. internal/server/http_helpers.go still contains a near-identical WithOTELTracing:

server.WithOTELTracing (server-specific):

func WithOTELTracing(next http.Handler, tag string) http.Handler {
    t := tracing.Tracer()
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        ctx := otel.GetTextMapPropagator().Extract(r.Context(), ...)
        ctx, span := t.Start(ctx, "gateway.request",
            oteltrace.WithAttributes(
                semconv.HTTPRequestMethodKey.String(r.Method),
                semconv.URLPathKey.String(r.URL.Path),
                attribute.String("gateway.tag", tag),
            ), ...)
        defer span.End()
        req := r.WithContext(ctx)
        next.ServeHTTP(w, req)
        // Post-request session ID enrichment
        sessionID := SessionIDFromContext(req.Context())
        span.SetAttributes(attribute.String("session.id", auth.TruncateSessionID(sessionID)))
    })
}

tracing.WrapHTTPHandler (general, in tracing package):

func WrapHTTPHandler(next http.Handler, spanName string, extraAttrs ...attribute.KeyValue) http.Handler {
    // identical W3C context extraction + span creation with variadic attrs
}

The two functions share ~80% of their implementation. server.WithOTELTracing adds server-specific session ID tracking after the handler returns.

Recommendation: Refactor server.WithOTELTracing to delegate the W3C extraction + span creation to tracing.WrapHTTPHandler, then enrich the span with session ID after returning. This eliminates the duplicated OTel boilerplate. Estimated effort: ~45 min.

---

8. 🟢 proxy.go Uses Stdlib log Instead of Project Logger (new)

internal/proxy/proxy.go imports the Go stdlib "log" package and uses it in two places, while every other file in the proxy package (and the entire codebase) uses logger.New(...):

// Line 106
log.Printf("[proxy] WARNING: invalid DIFC mode %q, defaulting to filter", cfg.DIFCMode)

// Line 210
log.Printf("[proxy] Guard initialized: mode=%s, secrecy=%v, integrity=%v", ...)

This bypasses the structured debug logging system (DEBUG=proxy:*) and produces inconsistent log output.

Recommendation: Replace stdlib log.Printf calls with logProxy.Printf(...) where logProxy = logger.New("proxy:proxy") (already defined in the file). Remove the "log" import. Estimated effort: ~10 min.

---

Refactoring Recommendations (Prioritized)

Priority 1 — Quick Wins (< 1 hour each)

#	Action	Files Affected	Impact
1	Replace duplicate JSON-RPC types in sdk_logging.go with mcp.Request/Response/ResponseError	server/sdk_logging.go, mcp/types.go	Eliminates 3 duplicate type definitions
2	Move isEffectivelyEmpty() from rpc_helpers.go to rpc_formatter.go	logger/rpc_helpers.go, logger/rpc_formatter.go	Co-locates function with its sole caller
3	Use strutil.FormatDuration in formatResetAt	server/circuit_breaker.go	Consistent duration formatting
4	Fix proxy.go stdlib log → project logProxy	proxy/proxy.go	Consistent structured logging

Priority 2 — Medium Effort (1–2 hours each)

#	Action	Files Affected	Impact
5	Move config/docker_helpers.go → sys/docker.go	config/docker_helpers.go, sys/container.go, config/validation_env.go	Co-locates all system/container utilities
6	Promote sessionSuffix to strutil package	launcher/log_helpers.go, mcp/errors.go, strutil/	Single source of truth for session log formatting
7	Refactor server.WithOTELTracing to delegate to tracing.WrapHTTPHandler	server/http_helpers.go, tracing/http.go	Eliminates ~80% code duplication in OTel wrapping
8	Merge sdk_logging.go into http_helpers.go (after #1)	server/sdk_logging.go, server/http_helpers.go	Consolidates HTTP middleware, removes orphan file

---

Implementation Checklist

• Replace duplicate JSON-RPC types in server/sdk_logging.go (P1 Configure as a Go CLI tool #1)
• Move isEffectivelyEmpty() to rpc_formatter.go (P1 Lpcox/initial implementation #2)
• Use strutil.FormatDuration in formatResetAt (P1 Lpcox/initial implementation #3)
• Fix proxy.go stdlib log calls (P1 Lpcox/add difc #4)
• Move config/docker_helpers.go to sys/docker.go (P2 Updated Dockerfile #5)
• Promote sessionSuffix to strutil package (P2 Updated Dockerfile to run 'go mod tidy' #6)
• Refactor server.WithOTELTracing to delegate to tracing.WrapHTTPHandler (P2 sys___init requirement breaks standard MCP client compatibility #7)
• Merge sdk_logging.go into http_helpers.go (P2 Make DIFC disabled by default with --enable-difc flag for standard MCP client compatibility #8)
• Update tests to reflect changes
• Run make agent-finished to verify build + lint + tests pass

---

Analysis Metadata

Metric	Value
Go files analyzed	97 (non-test, internal/)
Packages analyzed	22
Findings carried over (unresolved)	6
New findings	2
Detection method	Naming pattern analysis + semantic clustering + code inspection

References:

• §24830913327
• Prior analysis: #4330 (closed)

Generated by Semantic Function Refactoring · ● 436.9K · ◷

[github-actions]

Closing this issue as a new semantic function refactoring analysis is being performed.