Refactor: Code organization improvements from semantic clustering analysis

[Copilot]

Semantic function clustering analysis identified 4 high-priority refactoring opportunities: duplicate auth middleware application, misplaced generic utilities, schema transformation mixed with type definitions, and unnecessary wrapper indirection.

Changes

Extract auth middleware helper

• Replaced duplicate conditional auth application with applyAuthIfConfigured() helper
• Eliminates 8 lines of duplication in internal/server/transport.go

Before:

finalHandler := shutdownHandler
if apiKey != "" {
    finalHandler = authMiddleware(apiKey, shutdownHandler.ServeHTTP)
}
// ... repeated for closeHandler

After:

finalHandler := applyAuthIfConfigured(apiKey, shutdownHandler.ServeHTTP)
finalCloseHandler := applyAuthIfConfigured(apiKey, closeHandler.ServeHTTP)

Move runtime error logging to dedicated file

• Created internal/server/errors.go for error handling utilities
• Moved logRuntimeError() from auth.go (was incorrectly placed in auth-specific file)

Separate schema transformation from type definitions

• Created internal/mcp/schema.go for 86-line NormalizeInputSchema() function
• internal/mcp/types.go now contains only type definitions per Go conventions

Remove validation wrapper

• Removed thin validateServerConfig() wrapper that only passed nil
• Tests call validateServerConfigWithCustomSchemas() directly

Files Changed

• Modified: internal/server/transport.go, internal/server/auth.go, internal/mcp/types.go, internal/config/validation.go, internal/config/validation_test.go
• Added: internal/server/errors.go, internal/mcp/schema.go

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:

• example.com
  • Triggering command: /tmp/go-build1870635187/b275/launcher.test /tmp/go-build1870635187/b275/launcher.test -test.testlogfile=/tmp/go-build1870635187/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ain cfg s.test --gdwarf-5 --64 -o s.test 6525�� ache/go/1.25.7/x-s -I 86_64/git --gdwarf-5 --64 -o 256506/b165/_x00-extld=gcc (dns block)
• invalid-host-that-does-not-exist-12345.com
  • Triggering command: /tmp/go-build1870635187/b260/config.test /tmp/go-build1870635187/b260/config.test -test.testlogfile=/tmp/go-build1870635187/b260/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true d -n 10 5877973+lpcox@users.noreply.github.com> 64/pkg/tool/linux_amd64/vet -p net/netip -lang=go1.25 64/pkg/tool/linux_amd64/vet -I /opt/hostedtoolcache/go/1.25.7/x64/src/runtime/cgo 256506/b165/ x_amd64/link --gdwarf-5 --64 (dns block)
  • Triggering command: /tmp/go-build3740779677/b001/config.test /tmp/go-build3740779677/b001/config.test -test.testlogfile=/tmp/go-build3740779677/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -uns�� on-clustering-again /tmp/go-build165256506/b208/vet.cfg .12/x64/bin/bash (dns block)
• nonexistent.local
  • Triggering command: /tmp/go-build1870635187/b275/launcher.test /tmp/go-build1870635187/b275/launcher.test -test.testlogfile=/tmp/go-build1870635187/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ain cfg s.test --gdwarf-5 --64 -o s.test 6525�� ache/go/1.25.7/x-s -I 86_64/git --gdwarf-5 --64 -o 256506/b165/_x00-extld=gcc (dns block)
• slow.example.com
  • Triggering command: /tmp/go-build1870635187/b275/launcher.test /tmp/go-build1870635187/b275/launcher.test -test.testlogfile=/tmp/go-build1870635187/b275/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true ain cfg s.test --gdwarf-5 --64 -o s.test 6525�� ache/go/1.25.7/x-s -I 86_64/git --gdwarf-5 --64 -o 256506/b165/_x00-extld=gcc (dns block)
• this-host-does-not-exist-12345.com
  • Triggering command: /tmp/go-build1870635187/b284/mcp.test /tmp/go-build1870635187/b284/mcp.test -test.testlogfile=/tmp/go-build1870635187/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true rverConfigWithCustomSchemas 256506/b082/vet.cfg cfg --gdwarf-5 ut-2576538208.c -o ache/go/1.25.7/x-importcfg (dns block)
  • Triggering command: /tmp/go-build3437160695/b284/mcp.test /tmp/go-build3437160695/b284/mcp.test -test.testlogfile=/tmp/go-build3437160695/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true -c=4 -nolocalimports -importcfg /tmp/go-build1870635187/b305/importcfg -pack /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/version/version.go /home/REDACTED/work/gh-aw-mcpg/gh-aw-mcpg/internal/version/version_test.go -qui�� ain /opt/hostedtoolcache/go/1.25.7/x64/src/net x_amd64/compile /tmp/go-build165/opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/compile -imultiarch x86_64-linux-gnu/tmp/go-build1870635187/b306/_pkg_.a x_amd64/compile (dns block)
  • Triggering command: `/tmp/go-build880157664/b284/mcp.test /tmp/go-build880157664/b284/mcp.test -test.testlogfile=/tmp/go-build880157664/b284/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true C77u/Ma68ZNCRMnumNFLzC77u /tmp/go-build165256506/b065/vet.cfg k/_temp/ghcca-node/node/bin/sh go1.25.7 -c=4 -nolocalimports 256506/b258/importcfg -plu�� celain --ignore-submodules | head -n 10 nction

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> 64/bin/git -plugin-opt=-pas/usr/libexec/docker/cli-plugins/docker-buildx -plugin-opt=-pasdocker-cli-plugin-metadata -plugin-opt=-pass-through=-lpthr(create|run) /opt/hostedtoolcache/go/1.25.7/x64/pkg/tool/linux_amd64/vet` (dns block)

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

• Configure Actions setup steps to set up my environment, which run before the firewall is enabled
• Add the appropriate URLs or hosts to the custom allowlist in this repository's Copilot coding agent settings (admins only)

Original prompt

---

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis - Code Organization & Refactoring Opportunities</issue_title>
<issue_description>## Executive Summary

Analyzed 63 Go source files (excluding tests) across 16 packages containing ~450 functions to identify refactoring opportunities through semantic function clustering and duplicate detection.

Key Findings:

• ✅ Overall code quality is excellent - well-organized with strong use of Go patterns
• 🔧 8 high-priority refactoring opportunities identified (duplicate logic, misplaced functions)
• 📊 Function clusters are generally well-organized by file purpose
• ⚠️ Minor inconsistencies in naming patterns and file organization

Analysis covered:

• 16 packages in internal/ directory
• Pattern analysis using semantic clustering
• Duplicate detection across files
• File organization validation

---

Package Overview

Files by Package

Package	Files	Primary Purpose
logger	14	Debug logging framework, file/markdown/JSONL logging, RPC logging
server	10	HTTP server (routed/unified modes), handlers, middleware
config	8	Configuration parsing (TOML/JSON), validation, variable expansion
cmd	7	CLI (Cobra), flag management, completions
difc	5	Data Information Flow Control labels and evaluation
guard	4	Security guard framework and registry
testutil/mcptest	4	Test utilities for MCP integration
launcher	3	Backend process management and connection pooling
mcp	2	MCP protocol types and connections
tty	2	Terminal detection utilities
auth	1	Authentication header parsing
envutil	1	Environment variable utilities
middleware	1	HTTP middleware (jq schema processing)
sys	1	System utilities
timeutil	1	Time formatting utilities
version	1	Version management

---

Identified Issues

Priority 1: High-Impact Refactoring

1. Duplicate Auth Middleware Application (server package)

Issue: Auth middleware is applied with identical pattern in two places.

Location: internal/server/transport.go:120-122 and 137-139

Current Code Pattern:

// Pattern repeated twice
finalHandler := shutdownHandler
if apiKey != "" {
    finalHandler = authMiddleware(apiKey, shutdownHandler.ServeHTTP)
}

finalCloseHandler := closeHandler
if apiKey != "" {
    finalCloseHandler = authMiddleware(apiKey, closeHandler.ServeHTTP)
}

Recommendation: Extract to helper function

func applyAuthIfConfigured(apiKey string, handler http.HandlerFunc) http.HandlerFunc {
    if apiKey != "" {
        return authMiddleware(apiKey, handler)
    }
    return handler
}

// Usage
finalHandler := applyAuthIfConfigured(apiKey, shutdownHandler)
finalCloseHandler := applyAuthIfConfigured(apiKey, closeHandler)

Estimated Impact: Reduced duplication, easier maintenance
Effort: 30 minutes

---

2. Misplaced Generic Logging Function (server package)

Issue: Generic runtime error logging function is in auth-specific file.

Function: logRuntimeError() in internal/server/auth.go

Problem: This function logs generic runtime errors and panics, not authentication-specific errors.

Current Location: internal/server/auth.go (lines ~50-60)

Recommendation: Move to internal/server/server.go or create internal/server/errors.go for error handling utilities.

Estimated Impact: Improved code organization, clearer file purpose
Effort: 15 minutes

---

3. Large Schema Normalization Function in Types File (mcp package)

Issue: 86-line schema normalization logic mixed with type definitions.

Function: NormalizeInputSchema() in internal/mcp/types.go

Problem: types.go should contain simple type definitions, not complex transformation logic.

Current: 86 lines of schema transformation logic in types.go
Better: Extract to internal/mcp/schema.go or internal/mcp/normalize.go

Recommendation:

1. Create internal/mcp/schema.go
2. Move NormalizeInputSchema() to new file
3. Keep only type definitions in types.go

Estimated Impact: Clearer separation of concerns, easier to test
Effort: 20 minutes

---

4. Unnecessary Validation Wrapper Function (config package)

Issue: Thin wrapper function that adds no value.

Function: validateServerConfig() in internal/config/validation.go:104-106

Current Code:

func validateServerConfig(name string, srv ServerConfig, cfg *Config) error {
    return validateServerConfigWithCustomSchemas(name, srv, cfg, nil)
}

Problem: This wrapper just calls the main function with nil as the last parameter.

Recommendation: Remove wrapper, call `validateServerConfigWithCustomSchem...

• Fixes [refactor] Semantic Function Clustering Analysis - Code Organization & Refactoring Opportunities #1064

---

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

[Copilot]

Pull request overview

This PR implements code organization improvements identified through semantic clustering analysis. The changes focus on reducing code duplication, improving file organization, and removing unnecessary wrapper functions.

Changes:

• Extracted duplicate auth middleware conditional logic into applyAuthIfConfigured() helper function
• Moved generic logRuntimeError() function from auth.go to dedicated errors.go file
• Separated 86-line NormalizeInputSchema() transformation logic from types.go into new schema.go file
• Removed thin validateServerConfig() wrapper function that only passed nil parameter

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
internal/server/transport.go	Added applyAuthIfConfigured() helper to eliminate duplicate conditional auth middleware application (8 lines of duplication removed)
internal/server/errors.go	New file containing logRuntimeError() utility, properly isolating generic error handling from auth-specific logic
internal/server/auth.go	Removed logRuntimeError() function and cleaned up unused imports (log, time)
internal/mcp/schema.go	New file containing NormalizeInputSchema() function with improved documentation, separating transformation logic from type definitions
internal/mcp/types.go	Removed schema transformation logic, now contains only type definitions per Go conventions
internal/config/validation.go	Removed unnecessary validateServerConfig() wrapper function
internal/config/validation_test.go	Updated test to call validateServerConfigWithCustomSchemas() directly

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.