[refactor] Semantic Function Clustering Analysis — Refactoring Opportunities

[github-actions]

Overview

Static analysis of all 65 non-test Go files in internal/ identified four actionable refactoring opportunities. Two large files exceed 1000 lines and mix distinct concerns (mcp/connection.go, server/unified.go). One package-wide inconsistency exists in the logger/ package where global helpers for ToolsLogger are misplaced. One utility function (registerFlagCompletions) is in the wrong file in cmd/.

Overall code organization is good — packages are purposefully structured, test utilities are separated, and the config, difc, guard, and auth packages are well-organized. The issues below are high-signal, actionable improvements.

---

Analysis Metadata

Field	Value
Total Go files analyzed	65 (non-test, internal/ + root)
Total functions cataloged	~280
Function clusters identified	12
Outliers found	3
Duplicates/inconsistencies detected	2
Analysis date	2026-02-20

---

Identified Issues

1. 🔴 Outlier: initGlobalToolsLogger / closeGlobalToolsLogger in tools_logger.go

Priority: High — This breaks an established, documented pattern.

Issue: Every other global logger init/close helper pair lives in global_helpers.go:

• initGlobalFileLogger / closeGlobalFileLogger → global_helpers.go
• initGlobalJSONLLogger / closeGlobalJSONLLogger → global_helpers.go
• initGlobalMarkdownLogger / closeGlobalMarkdownLogger → global_helpers.go
• initGlobalServerFileLogger / closeGlobalServerFileLogger → global_helpers.go
• ❌ initGlobalToolsLogger / closeGlobalToolsLogger → tools_logger.go ← outlier

The file common.go even explicitly documents this pattern and lists all helpers expected to be in global_helpers.go (lines 164–167), but ToolsLogger's helpers are missing from that list.

Recommendation: Move initGlobalToolsLogger and closeGlobalToolsLogger from tools_logger.go to global_helpers.go, and add them to the documentation list in common.go. Estimated effort: 15 minutes.

---

2. 🔴 Monolith: mcp/connection.go (1010 lines, 5 distinct concerns)

Priority: High — A single file handles five distinct responsibilities.

Responsibilities currently mixed in connection.go:

Responsibility	Functions
SSE/HTTP response parsing	parseSSEResponse, parseJSONRPCResponseWithSSE, isHTTPConnectionError
Connection factory / transport negotiation	NewConnection, NewHTTPConnection, newMCPClient, newHTTPConnection, trySDKTransport, tryStreamableHTTPTransport, trySSETransport, tryPlainJSONTransport
HTTP request building & execution	createJSONRPCRequest, ensureToolCallArguments, setupHTTPRequest, executeHTTPRequest, sendHTTPRequest, initializeHTTPSession
MCP protocol dispatch	listTools, callTool, listResources, readResource, listPrompts, getPrompt
Docker env utilities	expandDockerEnvArgs, containsEqual

Specific outlier: expandDockerEnvArgs and containsEqual process Docker --env argument formatting. They are conceptually Docker launch utilities, not connection logic. They would fit better in the launcher/ package or in a dedicated internal/mcp/docker.go file.

Recommendation:

• Extract Docker env utilities to a new internal/mcp/docker_env.go (or move to launcher/)
• Consider splitting HTTP request infrastructure into internal/mcp/http_request.go
• Estimated effort: 2–3 hours

---

3. 🟡 Monolith: server/unified.go (1037 lines, 4 distinct concerns)

Priority: Medium — A single file handles session management, tool registration, tool calling, and server lifecycle.

Responsibilities currently mixed in unified.go:

Responsibility	Functions
Session management	NewSession, getSessionID, requireSession, getSessionKeys, ensureSessionDirectory
Tool registration	registerAllTools, registerAllToolsSequential, registerAllToolsParallel, registerToolsFromBackend, registerSysTools, registerGuard
Tool call dispatch	callBackendTool, convertToCallToolResult, newErrorCallToolResult, parseToolArguments
Server lifecycle & status	NewUnified, Run, Close, IsShutdown, InitiateShutdown, GetServerIDs, GetServerStatus, GetToolsForBackend, GetToolHandler, GetPayloadSizeThreshold, IsDIFCEnabled
Test helpers	RegisterTestTool, SetTestMode, ShouldExit

Recommendation:

• Extract session management to internal/server/session.go (the Session struct and its lifecycle functions already form a natural unit)
• Extract tool registration to internal/server/tool_registry.go
• Estimated effort: 3–4 hours

---

4. 🟡 Outlier: registerFlagCompletions in cmd/root.go

Priority: Low — Minor organizational inconsistency.

Issue: registerFlagCompletions(cmd *cobra.Command) is defined in root.go but is entirely about flag completion behavior. The cmd/ package already has a dedicated flags.go file with RegisterFlag and registerAllFlags — flag-related logic lives there. registerFlagCompletions is an outlier in root.go.

Recommendation: Move registerFlagCompletions from root.go to flags.go. Estimated effort: 10 minutes.

---

Refactoring Recommendations by Priority

Priority 1: Quick Wins (< 30 min each)

1. Move initGlobalToolsLogger/closeGlobalToolsLogger to global_helpers.go

   • Update common.go documentation list
   • No behavioral change, pure reorganization

2. Move registerFlagCompletions to flags.go

   • Pure function move, no behavioral change

Priority 2: Medium Effort (2–4 hours each)

3. Extract Docker env utilities from mcp/connection.go

   • Move expandDockerEnvArgs and containsEqual to internal/mcp/docker_env.go or launcher/
   • Update any callers (within the same package, so likely zero external impact)

4. Extract session management from server/unified.go

   • Move Session struct, NewSession, and session-related methods to internal/server/session.go
   • This file already exists as a conceptual entity (referenced in handlers/auth)

Priority 3: Larger Refactoring (3–6 hours)

5. Split mcp/connection.go by HTTP vs stdio concerns

   • HTTP request infrastructure → internal/mcp/http_request.go
   • Transport negotiation → internal/mcp/transport.go

6. Split server/unified.go tool registration

   • Tool registration → internal/server/tool_registry.go

---

Well-Organized Areas (No Action Needed)

The following packages are well-structured and should serve as models:

• ✅ internal/auth/ — Single file, clear purpose
• ✅ internal/config/rules/ — Validation error constructors cleanly isolated
• ✅ internal/difc/ — Labels, evaluator, agent, capabilities each in own file
• ✅ internal/launcher/ — Launcher + connection pool + log helpers cleanly separated
• ✅ internal/guard/ — Guard interface, noop, registry, context each in own file
• ✅ internal/logger/sanitize/ — Sanitization isolated in sub-package

Implementation Checklist

• Move initGlobalToolsLogger/closeGlobalToolsLogger to global_helpers.go + update docs
• Move registerFlagCompletions to flags.go
• Extract expandDockerEnvArgs/containsEqual from mcp/connection.go
• Extract Session + session management from server/unified.go to session.go
• Consider splitting mcp/connection.go HTTP infrastructure
• Consider splitting server/unified.go tool registration

Generated by Semantic Function Refactoring

[github-actions]

🍪 Issue Monster has assigned this to Copilot!

I've identified this issue as a good candidate for automated resolution and assigned it to the Copilot agent.

The Copilot agent will analyze the issue and create a pull request with the fix.

Om nom nom! 🍪

🍪 Om nom nom by Issue Monster