-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Executive Summary
Analysis of 50 non-test Go files across 16 packages in the internal/ directory identified 5 high-priority refactoring opportunities focused on eliminating code duplication, improving file organization, and reducing file complexity. The analysis reveals duplicate initialization patterns, oversized files with mixed concerns, and opportunities to consolidate scattered functionality.
Key Metrics:
- Total Go files analyzed: 50
- Total functions: 344
- Packages analyzed: 16
- Largest file: internal/mcp/connection.go (948 lines, 28 functions)
- Most fragmented package: logger (12 files)
Priority 1: Critical Refactoring Opportunities
1. ⚠️ Logger Package - Duplicate Init/Close Patterns (HIGH IMPACT)
Issue: Six nearly identical functions with 100% duplicate logic, differing only by type
Location: internal/logger/global_helpers.go
Duplicate Functions:
// Pattern 1: initGlobalFileLogger (lines 23-31)
func initGlobalFileLogger(logger *FileLogger) {
globalLoggerMu.Lock()
defer globalLoggerMu.Unlock()
if globalFileLogger != nil {
globalFileLogger.Close()
}
globalFileLogger = logger
}
// Pattern 2: initGlobalJSONLLogger (lines 49-57)
func initGlobalJSONLLogger(logger *JSONLLogger) {
globalJSONLMu.Lock()
defer globalJSONLMu.Unlock()
if globalJSONLLogger != nil {
globalJSONLLogger.Close()
}
globalJSONLLogger = logger
}
// Pattern 3: initGlobalMarkdownLogger (lines 75-83)
func initGlobalMarkdownLogger(logger *MarkdownLogger) {
globalMarkdownMu.Lock()
defer globalMarkdownMu.Unlock()
if globalMarkdownLogger != nil {
globalMarkdownLogger.Close()
}
globalMarkdownLogger = logger
}Plus three matching closeGlobal*Logger functions with identical patterns.
Code Similarity: 100% - Only type parameters differ
Recommendation: Use Go generics to consolidate into generic init/close functions:
// Generic logger interface for constraint
type Closer interface {
Close() error
}
// Generic helper for initializing global loggers
func initGlobalLogger[T Closer](mu *sync.Mutex, current **T, newLogger *T) {
mu.Lock()
defer mu.Unlock()
if *current != nil {
(*current).Close()
}
*current = newLogger
}
// Generic helper for closing global loggers
func closeGlobalLogger[T Closer](mu *sync.Mutex, logger **T) error {
mu.Lock()
defer mu.Unlock()
if *logger != nil {
err := (*logger).Close()
*logger = nil
return err
}
return nil
}Impact:
- Reduces ~90 lines of duplicate code to ~20 lines
- Eliminates six functions, replacing with two generic functions
- Type-safe with Go generics (requires Go 1.18+)
- Easier maintenance - single source of truth for init/close logic
- Estimated effort: 3-4 hours
2. ⚠️ Large File - MCP Connection (948 lines, 28 functions)
Issue: Single file handling too many distinct concerns, making it difficult to navigate and test
Location: internal/mcp/connection.go
Current Structure - Functions grouped by concern:
HTTP Transport Negotiation (3 functions):
tryStreamableHTTPTransport(line 317)trySSETransport(line 346)tryPlainJSONTransport(line 375)
Response Parsing (1 function):
parseSSEResponse(line 25)
Request Building (3 functions):
createJSONRPCRequest(line 104)ensureToolCallArguments(line 115)setupHTTPRequest(line 136)
Connection Initialization (4 functions):
newMCPClient(line 75)newHTTPConnection(line 83)NewConnection(line 155)NewHTTPConnection(line 245)
HTTP Session/Request Handling (2 functions):
initializeHTTPSession(line 495)sendHTTPRequest(line 603)
MCP Method Implementations (6 functions):
listTools(line 736)callTool(line 757)listResources(line 800)readResource(line 821)listPrompts(line 852)getPrompt(line 873)
Core Methods (4 functions):
IsHTTP,GetHTTPURL,GetHTTPHeaders(lines 401-411)SendRequest(line 417)SendRequestWithServerID(line 423)callSDKMethod(line 475)Close(line 942)
Utility Functions (2 functions):
expandDockerEnvArgs(line 908)containsEqual(line 932)
Recommended Split:
connection.go (12-14 functions) - Core connection lifecycle, public API
├── NewConnection, NewHTTPConnection
├── IsHTTP, GetHTTPURL, GetHTTPHeaders
├── SendRequest, SendRequestWithServerID
├── callSDKMethod, Close
└── newMCPClient, newHTTPConnection
http_transport.go (8-10 functions) - HTTP transport negotiation & session
├── tryStreamableHTTPTransport
├── trySSETransport
├── tryPlainJSONTransport
├── initializeHTTPSession
└── sendHTTPRequest
request_builder.go (5-6 functions) - Request construction
├── createJSONRPCRequest
├── setupHTTPRequest
├── ensureToolCallArguments
├── expandDockerEnvArgs
└── containsEqual
response_parser.go (3-4 functions) - Response parsing
└── parseSSEResponse
mcp_methods.go (6 functions) - MCP protocol method implementations
├── listTools
├── callTool
├── listResources
├── readResource
├── listPrompts
└── getPrompt
Benefits:
- Clearer separation of concerns
- Easier testing of specific functionality (e.g., transport negotiation vs parsing)
- Reduced cognitive load per file (~12-14 functions vs 28)
- Better code navigation and discoverability
- Estimated effort: 6-8 hours
3. ⚠️ Large File - Unified Server (896 lines, 26 functions)
Issue: Single file handling multiple distinct server concerns
Location: internal/server/unified.go
Current Structure - Functions grouped by concern:
Tool Registration (4 functions):
registerAllTools(line 145)registerToolsFromBackend(line 174)registerSysTools(line 309)RegisterTestTool(line 875) - testing support
Guard Management (2 functions):
registerGuard(line 447)guardBackendCaller.CallTool(line 463)
Tool Calling (3 functions):
callBackendTool(line 546)convertToCallToolResult(line 502)parseToolArguments(line 695)
Session Management (3 functions):
getSessionID(line 709)requireSession(line 723)getSessionKeys(line 762)
Server Lifecycle (4 functions):
NewUnified(line 103)Run(line 688)Close(line 834)InitiateShutdown(line 848)
Getters/Status (4 functions):
GetServerIDs(line 774)GetServerStatus(line 779)GetToolsForBackend(line 801)GetToolHandler(line 822)
Testing/Utilities (4 functions):
SetGatewayVersion(line 34)NewSession(line 54)SetTestMode(line 883)ShouldExit(line 889)IsDIFCEnabled(line 894)IsShutdown(line 840)
Recommended Split:
unified.go (12-14 functions) - Core server lifecycle, configuration
├── NewUnified
├── Run, Close, InitiateShutdown
├── SetGatewayVersion, NewSession
├── IsShutdown, ShouldExit, IsDIFCEnabled
├── SetTestMode
└── GetServerIDs, GetServerStatus
unified_tools.go (8-9 functions) - Tool registration & handling
├── registerAllTools
├── registerToolsFromBackend
├── registerSysTools
├── callBackendTool
├── convertToCallToolResult
├── parseToolArguments
├── GetToolsForBackend
├── GetToolHandler
└── RegisterTestTool
unified_sessions.go (3 functions) - Session management
├── getSessionID
├── requireSession
└── getSessionKeys
unified_guards.go (2-3 functions) - Guard/DIFC integration
├── registerGuard
└── guardBackendCaller.CallTool
Benefits:
- Better organization by feature area
- Easier to find session-related or tool-related functionality
- Reduced file size (from 896 lines to ~250-300 lines per file)
- Clearer boundaries between concerns
- Estimated effort: 6-8 hours
4. 📋 Validation Logic - Naming Inconsistency
Issue: Validation files use inconsistent naming conventions
Current Files:
internal/config/validation.go(5 functions) - General config validationinternal/config/env_validation.go(15 functions) - Environment validationinternal/config/schema_validation.go(7 functions) - JSON schema validationinternal/config/rules/rules.go(9 functions) - Validation error constructorsinternal/auth/header.go(includesValidateAPIKey) - Auth validation mixed with parsing
Analysis:
The naming is inconsistent - env_validation.go and schema_validation.go follow a pattern, but validation.go breaks it. The auth validation function is also mixed into a header parsing file.
Recommendation: Adopt consistent naming and extract auth validation
internal/config/
├── validation.go - Core config validation (keep name for backward compatibility)
├── validation_env.go - Environment validation (rename from env_validation.go)
├── validation_schema.go - JSON schema validation (rename from schema_validation.go)
└── rules/
└── rules.go - Validation error constructors (keep as is)
internal/auth/
├── header.go - Header parsing only
└── validation.go - Auth validation (NEW - extract ValidateAPIKey)
Benefits:
- Consistent naming pattern (
validation_*.gofor specialized validation) - Auth concerns properly separated (parsing vs validation)
- Easier to locate all validation logic
- Estimated effort: 2-3 hours
5. 🔍 Single-Function Files - Consolidation Opportunities
Issue: Several files contain only 1 function, increasing file navigation overhead
Files Identified:
-
internal/logger/error_formatting.go(1 function: ExtractErrorMessage, 48 lines)- Recommendation: Merge into
internal/logger/rpc_helpers.go - Rationale: Both contain helper functions for log/message processing
- Impact: Reduces logger package fragmentation (12 → 11 files)
- Recommendation: Merge into
-
internal/mcp/schema_normalizer.go(1 function: NormalizeInputSchema, 60 lines)- Recommendation: Merge into
internal/mcp/types.go - Rationale: Schema normalization is a type-related utility, types.go currently has 0 functions
- Impact: Better organization of MCP type utilities
- Recommendation: Merge into
-
internal/timeutil/format.go(1 function: FormatDuration)- Recommendation: Keep as is
- Rationale: Time utilities may grow, package name matches purpose well
Benefits:
- Reduced file count without losing clarity
- Related utilities grouped together
- Easier code navigation (fewer files to search through)
- Estimated effort: 1-2 hours
Priority 2: Package Organization Observations
6. Logger Package Fragmentation
Observation: Logger package has 12 files (11 Go files + 1 subpackage)
Current Structure:
internal/logger/
├── common.go (2 functions)
├── error_formatting.go (1 function)
├── file_logger.go (9 functions)
├── global_helpers.go (6 functions)
├── jsonl_logger.go (5 functions)
├── logger.go (7 functions)
├── markdown_logger.go (11 functions)
├── rpc_formatter.go (3 functions)
├── rpc_helpers.go (4 functions)
├── rpc_logger.go (4 functions)
├── slog_adapter.go (9 functions)
└── sanitize/
└── sanitize.go (4 functions)
Analysis:
- High fragmentation (12 files for 65 functions)
- Average: 5.4 functions per file
- Single-function files exist (error_formatting.go)
- Clear separation by logger type (file, jsonl, markdown) is good
- RPC-related functions split across 3 files (formatter, helpers, logger)
Recommendation: Consider consolidating RPC-related functionality in future refactoring:
- Could merge
rpc_formatter.go,rpc_helpers.go,rpc_logger.gointo singlerpc_logging.go - Would reduce 3 files (16 functions) to 1 file
- Priority 2 - not critical but worth considering
Detailed Function Clusters
Package-by-Package Function Distribution
internal/auth (1 file, 3 functions)
- header.go (3): ParseAuthHeader, ValidateAPIKey, ExtractAgentID
- Organization: ✅ Well-organized (but see Issue Lpcox/add difc #4 about extracting validation)
internal/cmd (2 files, 9 functions)
- completion.go (1): Shell completion
- root.go (8): CLI commands, initialization
- Organization: ✅ Clear separation
internal/config (4 files, 30 functions)
- config.go (3): Config loading
- validation.go (5): Server/gateway validation
- env_validation.go (15): Environment checks
- schema_validation.go (7): JSON schema validation
- Organization:
⚠️ See Issue Lpcox/add difc #4 (naming inconsistency)
internal/config/rules (1 file, 9 functions)
- rules.go (9): Validation error constructors
- Organization: ✅ Well-defined purpose
internal/difc (5 files, 66 functions)
- agent.go (18): Agent registry
- labels.go (20): Label types and operations
- evaluator.go (9): Policy evaluation
- resource.go (11): Resource tracking
- capabilities.go (8): Capability definitions
- Organization: ✅ Excellent - well-organized by feature
internal/guard (4 files, 19 functions)
- registry.go (10): Guard registration
- context.go (5): Guard context
- noop.go (4): No-op guard implementation
- guard.go (0): Interface definitions only
- Organization: ✅ Clear separation of concerns
internal/launcher (2 files, 18 functions)
- connection_pool.go (13): Connection pooling
- launcher.go (5): Process launching
- Organization: ✅ Clear separation by functionality
internal/logger (11 files, 61 functions)
- See detailed analysis in Issue Updated Dockerfile to run 'go mod tidy' #6
- Organization:
⚠️ High fragmentation (see Issue Configure as a Go CLI tool #1 and Updated Dockerfile #5)
internal/logger/sanitize (1 file, 4 functions)
- sanitize.go (4): Secret sanitization
- Organization: ✅ Well-organized subpackage
internal/mcp (3 files, 29 functions)
- connection.go (28): MCP connections
- schema_normalizer.go (1): Schema normalization
- types.go (0): Type definitions
- Organization:
⚠️ See Issue Lpcox/initial implementation #2 (connection.go too large) and Issue Updated Dockerfile #5
internal/middleware (1 file, 6 functions)
- jqschema.go (6): Schema middleware
- Organization: ✅ Appropriate size
internal/server (6 files, 51 functions)
- unified.go (26): Unified server mode
- server.go (11): Core server
- transport.go (8): Transport negotiation
- routed.go (4): Routed server mode
- sdk_logging.go (5): SDK logging middleware
- health.go (2): Health endpoint
- auth.go (2): Auth middleware
- Organization:
⚠️ See Issue Lpcox/initial implementation #3 (unified.go too large)
internal/sys (1 file, 6 functions)
- sys.go (6): System utilities
- Organization: ✅ Appropriate size
internal/testutil/mcptest (4 files, 22 functions)
- driver.go (7): Test driver
- validator.go (7): Validation client
- server.go (4): Test server
- config.go (4): Test config
- Organization: ✅ Well-organized test utilities
internal/timeutil (1 file, 1 function)
- format.go (1): Duration formatting
- Organization: ✅ Simple, focused (may grow)
internal/tty (2 files, 3 functions)
- tty.go (2): TTY detection
- container.go (1): Container TTY detection
- Organization: ✅ Clear separation
Implementation Roadmap
Phase 1: Quick Wins (1-2 days)
Priority: High Impact, Low Effort
- Close existing refactor issues
- Issue Updated Dockerfile #5: Consolidate single-function files (1-2 hours)
- Merge error_formatting.go into rpc_helpers.go
- Merge schema_normalizer.go into types.go
- Update imports
- Run tests:
make test-all
Phase 2: Code Deduplication (2-3 days)
Priority: High Impact, Medium Effort
- Issue Configure as a Go CLI tool #1: Implement generic logger initialization (3-4 hours)
- Create generic initGlobalLogger and closeGlobalLogger functions
- Update FileLogger to use generic helpers
- Update JSONLLogger to use generic helpers
- Update MarkdownLogger to use generic helpers
- Remove old duplicate functions
- Run tests:
make test-all
Phase 3: Validation Reorganization (2-3 days)
Priority: Medium Impact, Low-Medium Effort
- Issue Lpcox/add difc #4: Standardize validation file naming (2-3 hours)
- Rename env_validation.go → validation_env.go
- Rename schema_validation.go → validation_schema.go
- Extract ValidateAPIKey to auth/validation.go
- Update all imports
- Run tests:
make test-all
Phase 4: Large File Splitting (1-2 weeks)
Priority: High Impact, High Effort
-
Issue Lpcox/initial implementation #2: Split mcp/connection.go (6-8 hours)
- Create http_transport.go (8-10 functions)
- Create request_builder.go (5-6 functions)
- Create response_parser.go (3-4 functions)
- Create mcp_methods.go (6 functions)
- Keep core in connection.go (12-14 functions)
- Update internal package references
- Run tests:
make test-all
-
Issue Lpcox/initial implementation #3: Split server/unified.go (6-8 hours)
- Create unified_tools.go (8-9 functions)
- Create unified_sessions.go (3 functions)
- Create unified_guards.go (2-3 functions)
- Keep core in unified.go (12-14 functions)
- Update internal package references
- Run tests:
make test-all
Phase 5: Optional Improvements (Future)
Priority: Lower Priority
- Issue Updated Dockerfile to run 'go mod tidy' #6: Consider consolidating RPC logging files
- Evaluate merging rpc_formatter.go, rpc_helpers.go, rpc_logger.go
- If merged, update imports and test
Analysis Metadata
- Analysis Date: 2026-01-20
- Repository: githubnext/gh-aw-mcpg
- Commit: [Current HEAD]
- Go Version: 1.25.0
- Total Files Analyzed: 50 (excluding tests)
- Total Functions: 344
- Packages Analyzed: 16
- Detection Method:
- Function signature extraction via grep
- Manual code review of large files
- Pattern matching for duplicates
- File organization analysis
- Function clustering by naming patterns
Success Criteria
Each refactoring task is complete when:
- ✅ Code changes implemented with minimal modifications
- ✅ All existing tests pass (
make test-all) - ✅ No functionality changes - pure refactoring
- ✅ Code formatted with
make format - ✅ Linter passes with
make lint - ✅ Imports updated correctly
- ✅ Documentation updated if needed
Notes
What This Analysis Does NOT Include:
- Test file organization (excluded by design)
- Performance optimization opportunities
- API design improvements
- Security enhancements
- External dependency consolidation
Focus: Code organization, maintainability, and reducing duplication through semantic function clustering.
Philosophy: All suggested refactorings preserve functionality while improving code structure. No behavioral changes are proposed - only structural improvements for better maintainability.
Go Best Practices Applied:
- Prefer small, focused files (Go convention)
- Use generics to eliminate type-based duplication (Go 1.18+)
- Organize by feature/concern, not by type
- Keep related functionality together
- Make code easy to find and understand
References:
- §21173491633 - Analysis workflow run
AI generated by Semantic Function Refactoring