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

[github-actions]

Overview

Automated semantic analysis of 106 non-test Go files across 24 packages in internal/ identified 8 actionable refactoring opportunities. All findings from the prior analysis (#4381) remain unresolved.

Analysis date: 2026-04-24 · Run: §24885452160

---

Identified Issues

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

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

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

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

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

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

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.

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

internal/tracing/http.go provides a general WrapHTTPHandler function. internal/server/http_helpers.go still contains a near-identical WithOTELTracing. Both perform W3C context extraction and span creation with ~80% shared implementation. server.WithOTELTracing additionally adds server-specific session ID tracking after the handler returns.

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

---

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

internal/proxy/proxy.go imports the Go stdlib "log" package and uses it in two places (lines 106 and 210), while logProxy = logger.New("proxy:proxy") is already defined in the same file and used everywhere else.

// 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 both stdlib log.Printf calls with logProxy.Printf(...) and 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	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	106 (non-test, internal/)
Packages analyzed	24
Findings	8 (all carried over from prior analysis — unresolved)
Detection method	Naming pattern analysis + semantic clustering + code inspection

References:

• §24885452160
• Prior analysis: #4381 (closed)

Generated by Semantic Function Refactoring · ● 455K · ◷