-
Notifications
You must be signed in to change notification settings - Fork 3
Description
Executive Summary
Analysis of 48 non-test Go files in the internal/ directory identified 5 high-priority refactoring opportunities focused on code organization, function clustering, and reducing duplication. The analysis reveals patterns in logger initialization, validation logic distribution, and file size imbalances that could be improved for better maintainability.
Key Metrics:
- Total Go files analyzed: 48
- Total functions: ~322
- Packages analyzed: 16
- Most functions in single file: 28 (internal/mcp/connection.go)
- Largest logger fragmentation: 11 files
Priority 1: High-Impact Refactoring Opportunities
1. Logger Package - Duplicate Initialization Patterns ⚠️
Issue: Three nearly identical initialization helper functions with duplicate mutex locking logic
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
}Similarity: 100% identical logic with different types
Recommendation: Use Go generics to consolidate into a single generic function:
// Generic helper for initializing global loggers
func initGlobalLogger[T io.Closer](mu *sync.Mutex, current *T, newLogger T) {
mu.Lock()
defer mu.Unlock()
if current != nil && *current != nil {
(*current).Close()
}
*current = &newLogger
}Impact:
- Reduces code from ~54 lines to ~15 lines
- Eliminates duplicate maintenance burden
- Type-safe with generics (Go 1.18+)
- Estimated effort: 2-3 hours
2. Large Single-File Concerns - MCP Connection ⚠️
Issue: Single file with 28 functions handling multiple concerns
Location: internal/mcp/connection.go (28 functions)
Analysis: File handles multiple distinct concerns:
- SSE response parsing (parseSSEResponse)
- HTTP transport negotiation (tryStreamableHTTPTransport, trySSETransport, tryPlainJSONTransport)
- Connection initialization (newMCPClient, newHTTPConnection)
- Request handling (createJSONRPCRequest, setupHTTPRequest)
- Error handling and logging
Current Structure:
connection.go (28 functions)
├── SSE parsing
├── HTTP transport detection
├── Connection lifecycle
├── Request/response handling
└── Error formatting
Recommended Split:
connection.go (10-12 functions) - Core connection lifecycle
http_transport.go (8-10 functions) - HTTP transport negotiation & handling
request_builder.go (5-6 functions) - JSON-RPC request construction
response_parser.go (3-4 functions) - SSE and response parsing
Benefits:
- Clearer separation of concerns
- Easier testing of specific functionality
- Reduced cognitive load per file
- Estimated effort: 4-6 hours
3. Validation Logic Fragmentation 📋
Issue: Validation functions scattered across 4+ files and packages
Current Distribution:
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(ValidateAPIKey) - Auth validation
Analysis:
While some separation is reasonable (environment vs schema), the distribution makes it hard to discover all validation logic. The rules package contains error constructors but no actual validation logic.
Validation Function Types:
- Schema validation: JSON schema compliance
- Environment validation: Docker, container, port checks
- Config validation: Server config, mount formats
- Auth validation: API key validation
Recommendation: Consolidate related validation:
internal/config/
├── validation.go - Core config validation (server, gateway)
├── validation_schema.go - JSON schema validation (rename from schema_validation.go)
├── validation_env.go - Environment validation (rename from env_validation.go)
└── rules/
└── rules.go - Validation error constructors (keep as is)
internal/auth/
└── validation.go - Auth-specific validation (extract from header.go)
Benefits:
- Clearer naming convention (validation_*.go pattern)
- Auth validation moved to dedicated file
- Easier to locate all validation logic
- Estimated effort: 3-4 hours
4. Server Package - Unified.go Size ⚠️
Issue: Second largest file with 26 functions handling unified server mode
Location: internal/server/unified.go (26 functions)
Current Structure: Mix of:
- Session management (4 functions)
- Tool registration and handling (8 functions)
- Resource management (3 functions)
- Lifecycle methods (5 functions)
- Backend communication (6 functions)
Recommended Split:
unified.go (12-14 functions) - Core server, lifecycle, DIFC
unified_sessions.go (4-5 functions) - Session management
unified_tools.go (6-7 functions) - Tool registration & handling
unified_resources.go (3-4 functions) - Resource management
Benefits:
- Better organization by feature area
- Easier to test specific functionality
- Reduced file size (from 26 to ~12-14 functions per file)
- Estimated effort: 5-6 hours
5. Single-Function Files 🔍
Issue: Several files contain only 1-2 functions, increasing file navigation overhead
Files:
internal/timeutil/format.go(1 function: FormatDuration)internal/logger/error_formatting.go(1 function: ExtractErrorMessage)internal/mcp/schema_normalizer.go(1 function: normalizeLocalType)
Analysis:
While single-purpose files can be appropriate for large functions, these are small utility functions that could be better organized.
Recommendations:
-
timeutil/format.go → Keep as is
- Time utilities may grow
- Package name matches function purpose
-
logger/error_formatting.go → Merge into
logger/rpc_helpers.go- Both contain helper functions for log processing
- ExtractErrorMessage is a helper function
- Reduces logger package fragmentation (11 → 10 files)
-
mcp/schema_normalizer.go → Merge into
mcp/types.go- Schema normalization is a type-related utility
- types.go currently has 0 functions, suitable for utilities
Benefits:
- Reduced file count without losing clarity
- Related utilities grouped together
- Estimated effort: 1-2 hours
Priority 2: Code Quality Improvements
6. Error Wrapping Patterns
Observation: 80+ instances of fmt.Errorf("... %w", err) pattern
Analysis: Consistent error wrapping is good practice. Current usage is appropriate and follows Go 1.13+ standards.
Recommendation: No changes needed. Pattern is correct and consistent.
7. Logger Initialization Pattern
Observation: Consistent pattern across codebase: var log = logger.New("pkg:filename")
Count: 18 package-level logger variables
Examples:
var logRouted = logger.New("server:routed") // internal/server/routed.go
var logUnified = logger.New("server:unified") // internal/server/unified.go
var logConn = logger.New("mcp:connection") // internal/mcp/connection.go
var log = logger.New("auth:header") // internal/auth/header.goAnalysis:
- Consistent naming convention (good)
- Package-level variables (acceptable for loggers)
- Follows debug logging namespace convention
Recommendation: Keep pattern as is. It's a well-established Go practice for package-level loggers.
Priority 3: Documentation Improvements
8. Config Validation Documentation
Observation: Validation logic is well-commented but could benefit from centralized documentation
Recommendation: Add package-level documentation to internal/config/ explaining:
- Which file handles which validation type
- Variable expansion rules
- Error reporting strategy
Detailed Function Clusters
Package-by-Package Function Distribution
internal/auth (1 file, 3 functions)
- header.go: ParseAuthHeader, ValidateAPIKey, ExtractAgentID
- Organization: ✅ Well-organized
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:
⚠️ Could benefit from naming consistency (see Issue Lpcox/initial implementation #3)
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: ✅ 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
- Organization: ✅ Clear separation
internal/launcher (1 file, 4 functions)
- launcher.go (4): Process launching
- Organization: ✅ Appropriate size
internal/logger (11 files, 61 functions)
- file_logger.go (9): File logging
- logger.go (7): Core logger
- markdown_logger.go (11): Markdown logging
- slog_adapter.go (9): slog integration
- rpc_logger.go (4): RPC message logging
- jsonl_logger.go (5): JSONL logging
- global_helpers.go (6): Global logger helpers
- rpc_formatter.go (3): RPC formatting
- rpc_helpers.go (4): RPC processing helpers
- common.go (2): Shared utilities
- error_formatting.go (1): Error extraction
- 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:
⚠️ connection.go is too large (see Issue Lpcox/initial implementation #2 and Updated Dockerfile #5)
internal/middleware (1 file, 5 functions)
- jqschema.go (5): 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 (2): Routed server mode
- health.go (2): Health endpoint
- auth.go (2): Auth middleware
- Organization:
⚠️ unified.go is large (see Issue Lpcox/add difc #4)
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
internal/tty (2 files, 3 functions)
- tty.go (2): TTY detection
- container.go (1): Container TTY
- Organization: ✅ Clear separation
Implementation Checklist
Phase 1: Quick Wins (1-2 days)
- Issue Updated Dockerfile #5: Consolidate single-function files
- Merge error_formatting.go into rpc_helpers.go
- Merge schema_normalizer.go into types.go
- Update imports
- Run tests
Phase 2: Logger Improvements (2-3 days)
- Issue Configure as a Go CLI tool #1: Implement generic logger initialization
- Create generic initGlobalLogger function
- Update three init functions to use generic version
- Update close functions similarly
- Run tests
Phase 3: Validation Reorganization (3-4 days)
- Issue Lpcox/initial implementation #3: Rename and reorganize validation files
- Rename schema_validation.go → validation_schema.go
- Rename env_validation.go → validation_env.go
- Extract ValidateAPIKey to auth/validation.go
- Update imports
- Run tests
Phase 4: Large File Splitting (1-2 weeks)
-
Issue Lpcox/initial implementation #2: Split mcp/connection.go
- Create http_transport.go
- Create request_builder.go
- Create response_parser.go
- Move functions and update tests
-
Issue Lpcox/add difc #4: Split server/unified.go
- Create unified_sessions.go
- Create unified_tools.go
- Create unified_resources.go
- Move functions and update tests
Analysis Metadata
- Analysis Date: 2026-01-19
- Repository: githubnext/gh-aw-mcpg
- Branch: Analyzed working tree
- Go Version: 1.25.0
- Total Files Analyzed: 48 (excluding tests)
- Total Functions: ~322
- Packages: 16
- Detection Method:
- Function signature extraction via grep
- Manual code review of key files
- Pattern matching for duplicates
- File size and complexity analysis
Notes
What This Analysis Does NOT Include:
- Test file organization (excluded by design)
- Performance optimization opportunities
- API design improvements
- Security enhancements
- External package consolidation
Focus: Code organization, maintainability, and reducing duplication through semantic function clustering.
Philosophy: These refactorings preserve all functionality while improving code organization. No behavioral changes are suggested, only structural improvements.
AI generated by Semantic Function Refactoring