[refactor] Semantic Function Clustering Analysis — Outliers, Near-Duplicates, and Organization Issues

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg
Workflow run: §29368807559

Executive Summary

Analyzed 148 non-test Go source files across 26 packages in internal/. The codebase is generally well-organized with clear single-responsibility files. This report identifies 3 outlier/misplacement patterns, 2 near-duplicate response writer implementations (intentionally related via embedding), and 1 TLS helper scattering pattern. Most findings are low-severity organizational issues. The highest-priority item is TLS configuration helpers scattered across three separate files/packages with overlapping concerns.

Total functions catalogued: ~650+ across the internal/ tree. Clusters are well-formed. No exact duplicate implementations were detected.

Full Report

Function Inventory

By Package (non-test files only)

Package	Files	Primary Purpose
internal/auth	1	Auth header parsing
internal/cmd	11	CLI commands (Cobra)
internal/config	13	Config parsing & validation
internal/difc	10	Decentralized info-flow control
internal/envutil	4	Env var utilities
internal/githubhttp	4	GitHub API HTTP helpers
internal/guard	12	Security guards (Noop/Wasm/WriteSink)
internal/httputil	4	Generic HTTP utilities
internal/launcher	4	Backend process management
internal/logger	13	Logging framework
internal/mcp	9	MCP protocol types & connections
internal/middleware	2	HTTP middleware
internal/proxy	7	Filtering HTTP proxy
internal/sanitize	1	Data redaction
internal/server	21	HTTP server
internal/syncutil	2	Concurrency utilities
internal/sys	2	System utilities
internal/tracing	7	OpenTelemetry tracing
internal/util	5	String/formatting/randomness
(others)	8	Various

Identified Issues

1. TLS Configuration Scatter (Three Separate Files)

Severity: Medium
Issue: TLS-related helper functions are spread across three files in different packages:

internal/httputil/tls.go

• TLSTrustEnvKeys() []string
• NewServerTLSConfig(cert tls.Certificate) *tls.Config
• NewClientTLSConfig() *tls.Config
• ConfigureTLSTrustEnvironment(caCertPath string) error

internal/server/gateway_tls.go

• LoadGatewayTLS(certPath, keyPath, caPath string) (*tls.Config, error)

internal/proxy/tls.go

• GenerateSelfSignedTLS(dir string) (*TLSConfig, error)
• randomSerial() (*big.Int, error) (private helper)
• writePEM(path, blockType string, derBytes []byte, perm os.FileMode) error (private helper)

Analysis: LoadGatewayTLS in internal/server/gateway_tls.go combines cert/key loading and optional mTLS CA configuration — overlapping in intent with httputil.NewServerTLSConfig and httputil.ConfigureTLSTrustEnvironment. The proxy/tls.go self-signed cert generation is legitimately proxy-specific (local dev only) and is acceptable in-place.

Recommendation: Consider moving LoadGatewayTLS to internal/httputil/tls.go since it has no server-package-specific dependencies, or add clear documentation explaining the split (httputil = reusable TLS helpers, server/gateway_tls = gateway-specific loader, proxy/tls = dev-cert generation).

Estimated effort: 1–2 hours
Files: internal/httputil/tls.go, internal/server/gateway_tls.go, internal/proxy/tls.go

---

2. Near-Duplicate Response Writer Implementations

Severity: Low (already resolved via embedding)
Issue: Both internal/server/response_writer.go and internal/httputil/response_writer.go implement WriteHeader and Write.

internal/httputil/response_writer.go — BaseResponseWriter

type BaseResponseWriter struct {
    http.ResponseWriter
    StatusCode  int
    wroteHeader bool
}

internal/server/response_writer.go — responseWriter

type responseWriter struct {
    httputil.BaseResponseWriter  // correctly embeds the above
    body bytes.Buffer
}

Analysis: This is the correct pattern — server.responseWriter intentionally embeds httputil.BaseResponseWriter and extends it with body buffering for debug logging. Not a true duplicate. The embedding relationship is sound.

Recommendation: No action needed. Mark as reviewed.

---

3. FormatSessionIDForLog in util/format_duration.go

Severity: Low
Issue: internal/util/format_duration.go is named for duration formatting but also contains FormatSessionIDForLog, a string-truncation helper unrelated to durations.

Functions in file:

• FormatSessionIDForLog(sessionID string) string — session ID truncation for logs
• FormatFutureTime(t time.Time) string — time formatting
• FormatDuration(d time.Duration) string — duration formatting

Recommendation: Either:

1. Rename format_duration.go → format_helpers.go to reflect its mixed content, or
2. Move FormatSessionIDForLog to internal/util/util.go

Estimated effort: <30 minutes
Files: internal/util/format_duration.go

---

4. guard/validation.go — Single-Function File

Severity: Low
Issue: internal/guard/validation.go contains only one private function:

func validateIntegrityField(fieldName string, raw interface{}) error

Analysis: A single-function unexported-only file may have been created anticipating additional validation functions that never materialized. Given the guard package already has well-split files by concern, this creates minor noise.

Recommendation: Merge validateIntegrityField into wasm_payload.go (its primary call site) unless additional guard validation functions are planned.

Estimated effort: <30 minutes
Files: internal/guard/validation.go, internal/guard/wasm_payload.go

---

5. Rate-Limit Split Between githubhttp and server (Informational)

Severity: Informational
Issue: Rate-limit parsing spans two packages:

• internal/githubhttp/rate_limit.go: ParseRateLimitResetHeader, ParseRateLimitResetFromText
• internal/server/rate_limit.go: extractRateLimitErrorText, isRateLimitToolResult, isRateLimitText

Analysis: The split is intentional — githubhttp handles HTTP-level GitHub API rate limit headers, while server detects rate-limiting in MCP tool results. The server package correctly calls into githubhttp for parsing. Dependency direction is correct.

Recommendation: Add a short comment to internal/server/rate_limit.go documenting the split (MCP-level detection vs. HTTP-level parsing) for future developers.

Estimated effort: <15 minutes

---

Clustering Summary

Well-Organized Clusters ✅

Cluster	Package	Notes
Config validation	internal/config/validation_*.go	Excellent split by validation domain
DIFC labels/evaluator	internal/difc/	Clear separation of concerns
Guard implementations	internal/guard/	Well-split by guard type
Connection management	internal/mcp/connection*.go	Good split of connection types
Logger implementations	internal/logger/	Each logger type has its own file
Tracing	internal/tracing/	Good separation of provider, spans, config
Session management	internal/server/session*.go	Well-separated auto-init vs. core
Proxy routing	internal/proxy/router.go	Arg-builder helpers correctly colocated

Clusters with Minor Issues ⚠️

Cluster	Issue	Priority
TLS configuration	Split across 3 files/packages	Medium
Format helpers	FormatSessionIDForLog in wrong-named file	Low
Guard validation	Single-function file	Low
Rate-limit parsing	Intentionally split — needs documentation	Informational

Refactoring Recommendations

Priority 1: Medium Impact

1. Consolidate or document TLS helper split
   • Move LoadGatewayTLS to internal/httputil/tls.go or add clear documentation on intended ownership
   • Estimated effort: 1–2 hours

Priority 2: Low Impact (Housekeeping)

2. Rename util/format_duration.go to format_helpers.go

   • Or extract FormatSessionIDForLog to a better-named file
   • Estimated effort: <30 minutes

3. Add clarifying comment to server/rate_limit.go

   • Document the MCP-level vs HTTP-level rate limit parsing split
   • Estimated effort: <15 minutes

4. Merge or expand guard/validation.go

   • Merge the single private function into its call site, or add more validation functions
   • Estimated effort: <30 minutes

Implementation Checklist

• Review TLS helper split and decide on consolidation approach
• Rename util/format_duration.go to better reflect its mixed content
• Add clarifying comments to server/rate_limit.go on rate-limit parsing split
• Evaluate guard/validation.go — expand or merge
• Update any affected tests after file renames (import paths unchanged for same-package renames)

Analysis Metadata

• Total Go Files Analyzed: 148 (non-test)
• Packages Analyzed: 26
• Function Clusters Identified: 12 major clusters
• Outliers Found: 1 (FormatSessionIDForLog in wrong-named file)
• Near-Duplicates: 1 pair (response writers — correctly using embedding)
• TLS Scatter: 3 files across 3 packages
• Single-function files: 1 (guard/validation.go)
• Detection Method: Static analysis via grep + semantic review of function signatures and call patterns

References:

• §29368807559

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Semantic Function Refactoring · 76.5 AIC · ⊞ 8.5K · ◷