[refactor] Semantic Function Clustering Analysis - Code Organization Improvements

[github-actions]

Analysis of repository: githubnext/gh-aw-mcpg - Workflow run §21315883783

Executive Summary

Analyzed 52 non-test Go files across 18 packages containing approximately 357 exported and private functions. The codebase demonstrates excellent organization with effective use of Go generics and interface patterns. Most apparent "duplication" is actually proper interface implementation or domain-specific specialization.

Key Findings:

• ✅ Well-organized areas: Logger package (excellent generics usage), Config validation (clear separation of concerns), DIFC package
• ⚠️ 3 minor refactoring opportunities identified in server package (1-2 hours effort)
• 📊 No significant code duplication - most duplicate names are standard Go interface methods

Function Inventory Summary

By Package

Package	Files	Functions	Primary Purpose
internal/auth	1	4	Authentication header parsing
internal/cmd	2	10	CLI commands (Cobra)
internal/config	5	45	Configuration parsing & validation
internal/difc	5	62	DIFC security labels
internal/guard	4	22	Security guards
internal/launcher	2	18	Backend process management
internal/logger	11	61	Logging framework
internal/mcp	2	29	MCP protocol types
internal/middleware	1	4	Request middleware
internal/server	9	63	HTTP server
internal/sys	1	6	System utilities
internal/testutil/mcptest	4	22	Testing utilities
internal/timeutil	1	1	Time formatting
internal/tty	2	3	TTY detection

Total: 52 files, ~357 functions

Semantic Clustering Results

Major Function Patterns Identified:

Pattern	Count	Primary Packages
New* constructors	31	difc (13), logger (4), server (3)
Get* accessors	31	difc (9), config (3), launcher (4)
Log* functions	15	logger (15) - well-organized
Close() methods	11	Multiple - proper interface pattern
Private helpers	116	server (31), config (25), logger (23), mcp (20)

Identified Issues

1. 🟡 Duplicate Shutdown Check Functions (Medium Priority)

Issue: Two nearly identical middleware functions for checking shutdown state

Occurrences:

• internal/server/routed.go:21-33 - rejectIfShutdown()
• internal/server/transport.go:18-30 - rejectIfShutdownUnified()

Code Comparison:

// routed.go:21
func rejectIfShutdown(unifiedServer *UnifiedServer, next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if unifiedServer.IsShutdown() {
            logRouted.Printf("Rejecting request during shutdown: remote=%s, method=%s, path=%s", r.RemoteAddr, r.Method, r.URL.Path)
            logger.LogWarn("shutdown", "Request rejected during shutdown, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
            w.Header().Set("Content-Type", "application/json")
            w.WriteHeader(http.StatusServiceUnavailable)
            w.Write([]byte(shutdownErrorJSON))
            return
        }
        next.ServeHTTP(w, r)
    })
}

// transport.go:18 - NEARLY IDENTICAL (only logger variable name differs)
func rejectIfShutdownUnified(unifiedServer *UnifiedServer, next http.Handler) http.Handler {
    return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        if unifiedServer.IsShutdown() {
            logTransport.Printf("Rejecting request during shutdown: remote=%s, method=%s, path=%s", r.RemoteAddr, r.Method, r.URL.Path)
            logger.LogWarn("shutdown", "Request rejected during shutdown, remote=%s, path=%s", r.RemoteAddr, r.URL.Path)
            w.Header().Set("Content-Type", "application/json")
            w.WriteHeader(http.StatusServiceUnavailable)
            w.Write([]byte(shutdownErrorJSON))
            return
        }
        next.ServeHTTP(w, r)
    })
}

Similarity: ~95% identical (only debug logger variable differs)

Recommendation:

1. Consolidate into single rejectIfShutdown function
2. Pass logger as parameter or use single logger instance
3. Consider creating internal/server/middleware.go for shared middleware

Estimated Impact: Eliminates 13 lines of duplicate code, improves maintainability

---

2. 🟡 Session Truncation Helper in Wrong Package (Medium Priority)

Issue: Session ID truncation utility is in server package but should be in auth package

Occurrence:

• internal/server/sdk_logging.go:144-153 - truncateSession()

Code:

// truncateSession returns a truncated session ID for logging (first 8 chars)
func truncateSession(s string) string {
    if s == "" {
        return "(none)"
    }
    if len(s) <= 8 {
        return s
    }
    return s[:8] + "..."
}

Problem: Session manipulation utility is in wrong package, not reusable

Recommendation:

1. Move to internal/auth/header.go as public TruncateSessionID() function
2. Update imports in sdk_logging.go to use auth.TruncateSessionID()

Benefits:

• Makes utility discoverable and reusable
• Centralizes session-related operations in auth package
• Follows "one file per feature" principle

Estimated Impact: Improved code organization, enables reuse

---

3. 🟢 Deprecated Function Still Present (Low Priority)

Issue: Function marked as deprecated but not yet removed

Occurrence:

• internal/server/auth.go:45-53 - extractSessionFromAuth()

Code:

// extractSessionFromAuth extracts session ID from Authorization header.
// This function delegates to auth.ExtractSessionID for consistent session ID extraction.
// Per spec 7.1: When API key is configured, Authorization contains plain API key.
// When API key is not configured, supports Bearer token for backward compatibility.
//
// Deprecated: Use auth.ExtractSessionID directly instead.
func extractSessionFromAuth(authHeader string) string {
    return auth.ExtractSessionID(authHeader)
}

Current Usage:

• internal/server/sdk_logging.go:48 - Direct call to auth.ExtractSessionID() ✅ (correct)

Recommendation:

1. Verify no other usages with: grep -r "extractSessionFromAuth" internal/
2. Remove deprecated wrapper function
3. Update any remaining call sites to use auth.ExtractSessionID directly

Estimated Impact: Code cleanup, removes technical debt

---

Well-Organized Areas ✅

Logger Package - Exemplary Use of Generics

Files: internal/logger/global_helpers.go, internal/logger/common.go

The logger package demonstrates excellent use of Go generics to eliminate code duplication:

// Generic helper for initializing any logger type
func initGlobalLogger[T closableLogger](mu *sync.RWMutex, current *T, newLogger T) {
    mu.Lock()
    defer mu.Unlock()
    if *current != nil {
        (*current).Close()
    }
    *current = newLogger
}

// Generic helper for closing any logger type  
func closeGlobalLogger[T closableLogger](mu *sync.RWMutex, logger *T) error {
    mu.Lock()
    defer mu.Unlock()
    if *logger != nil {
        err := (*logger).Close()
        var zero T
        *logger = zero
        return err
    }
    return nil
}

Impact: Eliminates duplication across 3 logger types (File, JSONL, Markdown) with type-safe generics

Config Package - Clear Separation of Concerns

Files:

• internal/config/validation.go - General validation logic (10 functions)
• internal/config/validation_env.go - Environment-specific validation (15 functions)
• internal/config/validation_schema.go - JSON schema validation (8 functions)

Organization: Well-structured with clear boundaries between general, environment, and schema validation

DIFC Package - Excellent Modular Design

Files:

• internal/difc/agent.go - Agent label management (18 functions)
• internal/difc/capabilities.go - Capability operations (8 functions)
• internal/difc/evaluator.go - Flow evaluation (5 functions)
• internal/difc/labels.go - Label operations (20 functions)
• internal/difc/resource.go - Resource labeling (11 functions)

Organization: Clear separation by responsibility, each file has a single, well-defined purpose

---

Analysis of "Duplicate" Function Names

The analysis identified several function names that appear multiple times, but examination reveals these are not problematic duplicates:

Standard Go Interface Methods (Expected)

Method	Occurrences	Explanation
Close()	8	Standard io.Closer interface - proper pattern ✅
Get()	3	Standard accessor pattern on different types ✅
Add()	2	Standard collection operation on different types ✅
Remove()	3	Standard collection operation on different types ✅
Start()	2	Standard lifecycle method on different types ✅
Stop()	3	Standard lifecycle method on different types ✅

Method Overloading on Same Type (Go Idiom)

Method	File	Explanation
Clone()	difc/labels.go	Multiple types in same file implement Clone() ✅
Overall()	difc/resource.go	Multiple types in same file implement Overall() ✅
ToResult()	difc/resource.go	Multiple types in same file implement ToResult() ✅
CanFlowTo()	difc/labels.go	Multiple types implement flow check ✅

Conclusion: These are appropriate uses of Go's type system and interface patterns, not code duplication.

---

Refactoring Recommendations

Priority 1: Quick Wins (1-2 hours)

1. Consolidate Duplicate Shutdown Middleware

   • Merge rejectIfShutdown and rejectIfShutdownUnified into single function
   • Optionally create internal/server/middleware.go for middleware functions
   • Estimated effort: 30 minutes
   • Benefits: Eliminates 13 lines of duplicate code

2. Move Session Truncation to Auth Package

   • Move truncateSession to internal/auth/header.go as TruncateSessionID()
   • Update import in sdk_logging.go
   • Estimated effort: 15 minutes
   • Benefits: Better organization, reusable utility

3. Remove Deprecated Wrapper

   • Verify no usage of extractSessionFromAuth
   • Remove deprecated function from auth.go
   • Estimated effort: 10 minutes
   • Benefits: Code cleanup

Total Estimated Effort: 1 hour

Priority 2: Optional Enhancements (2-4 hours)

4. Create Server Middleware File

   • Extract middleware functions to internal/server/middleware.go:
     • authMiddleware
     • rejectIfShutdown (consolidated)
     • withResponseLogging
   • Benefits: Clearer organization, middleware functions in one place

5. Review Server Package Private Helpers

   • Review 31 private helpers in server package
   • Look for additional consolidation opportunities
   • Benefits: Potential for further code organization improvements

---

Implementation Checklist

• Issue Configure as a Go CLI tool #1: Consolidate rejectIfShutdown and rejectIfShutdownUnified
• Issue Lpcox/initial implementation #2: Move truncateSession to internal/auth/header.go
• Issue Lpcox/initial implementation #3: Remove deprecated extractSessionFromAuth wrapper
• Run tests after each change: make test
• Verify no functionality broken: make test-all
• Optional: Create internal/server/middleware.go for middleware functions

---

Analysis Metadata

• Total Go Files Analyzed: 52 (excluding test files)
• Total Functions Cataloged: ~357
• Function Clusters Identified: 8 major patterns
• Duplicate Code Issues: 1 (shutdown middleware)
• Organizational Issues: 2 (session truncation placement, deprecated wrapper)
• Well-Organized Packages: Logger, Config, DIFC
• Detection Method: Static analysis + semantic clustering + manual code review
• Analysis Date: 2026-01-24
• Overall Assessment: 🟢 Excellent - Minor improvements available, but codebase is well-structured

---

Conclusion

This codebase demonstrates strong engineering practices with excellent use of:

• Go generics for eliminating duplication (logger package)
• Clear separation of concerns (config validation, DIFC)
• Appropriate interface patterns (Close, Get, Add methods)

The identified refactoring opportunities are minor organizational improvements rather than critical issues. The suggested changes will improve discoverability and reduce maintenance burden, but the code is already in good shape.

Recommendation: Implement Priority 1 items (1 hour effort) for quick wins. Priority 2 items are optional enhancements.

References:

• Workflow Run: §21315883783

AI generated by Semantic Function Refactoring