Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 26, 2026

Addresses code duplication and organizational issues identified in semantic function clustering analysis: duplicate shutdown middleware (~95% identical), misplaced session truncation utility, and actively-used deprecated wrapper.

Changes

Shutdown Middleware Consolidation

  • Merged rejectIfShutdown() and rejectIfShutdownUnified() into single parameterized function
  • Added logNamespace parameter for call-site specific debug logging
  • Eliminates 13 lines of duplicate code
// Before: Two nearly identical functions
func rejectIfShutdown(server *UnifiedServer, next http.Handler) http.Handler { ... }
func rejectIfShutdownUnified(server *UnifiedServer, next http.Handler) http.Handler { ... }

// After: Single function with logger namespace parameter
func rejectIfShutdown(server *UnifiedServer, next http.Handler, logNamespace string) http.Handler {
    log := logger.New(logNamespace)
    // ...
}

// Call sites now specify their namespace
rejectIfShutdown(unifiedServer, handler, "server:routed")
rejectIfShutdown(unifiedServer, handler, "server:transport")

Session Utilities to Auth Package

  • Moved truncateSession() from internal/server/sdk_logging.go to internal/auth/header.go as exported TruncateSessionID()
  • Centralizes all session-related operations in auth package
  • Makes utility reusable across packages (7 call sites updated)

Deprecated Wrapper Removal

  • Removed extractSessionFromAuth() wrapper that delegated to auth.ExtractSessionID()
  • Updated 3 call sites to use auth.ExtractSessionID() directly
  • Eliminates unnecessary indirection layer

Impact

  • Net reduction: ~17 lines
  • Files modified: 5 (routed.go, transport.go, sdk_logging.go, auth.go, header.go)
  • No functional changes, purely organizational improvements

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • nonexistent.local
    • Triggering command: /tmp/go-build2901014057/b269/launcher.test /tmp/go-build2901014057/b269/launcher.test -test.testlogfile=/tmp/go-build2901014057/b269/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true g_.a 64/src/internal/crypto/tls ndor/bin/as user.name abis (dns block)
  • this-host-does-not-exist-12345.com
    • Triggering command: /tmp/go-build2901014057/b278/mcp.test /tmp/go-build2901014057/b278/mcp.test -test.testlogfile=/tmp/go-build2901014057/b278/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true 64/src/runtime/cgo --global ache/go/1.25.6/x64/pkg/tool/linu--64 user.name (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis - Minor Code Organization Improvements</issue_title>
<issue_description>Analysis of repository: githubnext/gh-aw-mcpg

Executive Summary

Analyzed 48 non-test Go files across 14 packages containing approximately 362 functions/methods (both exported and unexported). The codebase demonstrates excellent organization with strong use of Go generics, clear separation of concerns, and well-structured packages. Identified 3 actionable refactoring opportunities that can improve code organization and reduce duplication.

Key Findings:

  • Well-organized areas: Logger package (exemplary generics usage), Config package (clear validation separation), DIFC package (modular design)
  • ⚠️ 1 critical duplicate: Shutdown middleware duplicated across 2 files (~95% identical, 13 duplicate lines)
  • 🔧 2 organizational improvements: Session truncation utility in wrong package, deprecated function still in use (3 call sites)

Function Inventory Summary

By Package

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

Total: 48 files, 362 functions/methods

Semantic Clustering Results

Major Function Patterns Identified:

Pattern Count Observation
New* constructors 25+ Consistent constructor naming ✓
Validation functions (validate*) 10 Well-organized in config package ✓
Close() methods 8 Standard Go interface pattern ✓
Logging functions (Log*) 12 Properly centralized in logger package ✓
Format functions (format*) 6 Appropriately distributed by purpose ✓
Parse functions (parse*) 3 Appropriately distributed ✓
Middleware functions 6 In server package, one duplication found ⚠️

Identified Issues

1. 🔴 Duplicate Shutdown Middleware (Critical - High Impact)

Issue: Two nearly identical functions for shutdown rejection middleware

Occurrences:

  • File 1: internal/server/routed.go:21-33 - rejectIfShutdown()
  • File 2: 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 debug logger differs: logTransport vs logRouted)
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)
    })
}

Analysis:

  • Similarity: ~95% identical (13 duplicate lines)
  • Only difference: Debug logger variable name (logRouted vs logTransport)
  • Both functions: Check shutdown state, return HTTP 503, use same error JSON
  • Impact: Code duplication, double maintenance burden

Recommendation:

  1. Consolidate into single function with logger namespace parameter
  2. **Remove ...

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits January 26, 2026 23:50
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor code organization in Go repository Refactor: Consolidate shutdown middleware, centralize session utilities, remove deprecated wrappers Jan 26, 2026
Copilot AI requested a review from lpcox January 26, 2026 23:56
@lpcox lpcox marked this pull request as ready for review January 27, 2026 00:13
@lpcox
Copy link
Collaborator

lpcox commented Jan 27, 2026

@copilot why can't I merge this pr?

Copy link
Contributor Author

Copilot AI commented Jan 27, 2026

@copilot why can't I merge this pr?

The PR is ready to merge! According to the GitHub API, the PR is in a "clean" mergeable state with no conflicts or blocking issues. The code changes have passed all tests and checks.

You can merge it using the "Merge pull request" button on the PR page, or via the command line if you prefer. There are no technical blockers preventing the merge.

@lpcox lpcox merged commit 2cd9101 into main Jan 27, 2026
4 checks passed
@lpcox lpcox deleted the copilot/refactor-code-organization branch January 27, 2026 01:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[refactor] Semantic Function Clustering Analysis - Minor Code Organization Improvements

2 participants