[refactor] Semantic Function Clustering Analysis: Outliers, Patterns & Consolidation Opportunities

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg

Overview

This report analyzes all 155 non-test Go source files across 26 packages in internal/, cataloging 966 top-level function and method declarations. The codebase is generally well-organized, with clear package-level separation of concerns. Most findings are refinement opportunities rather than significant problems. Three clusters of interest emerged: scattered but thin format-helper patterns, a composable logging architecture that works correctly but has visible structural layering, and a few functions that could be consolidated or co-located more naturally.

Function Inventory

Packages by File Count

Package	Files	Primary Purpose
internal/server	20	HTTP server, routing, sessions, tools
internal/config	18	Config parsing, validation, expansion
internal/logger	16	Structured and debug logging
internal/cmd	14	CLI flags and commands (Cobra)
internal/mcp	11	MCP protocol, connections, transport
internal/difc	10	DIFC labels, evaluation, pipeline
internal/guard	12	Security guards, WASM lifecycle
internal/proxy	7	GitHub API proxy, DIFC enforcement
internal/tracing	7	OpenTelemetry setup
internal/envutil	4	Environment variable utilities
internal/githubhttp	4	GitHub API HTTP helpers
internal/httputil	4	Generic HTTP helpers
internal/launcher	4	Backend process management
internal/testutil	4	Test utilities
internal/util	5	String, format, random utilities
Other (11 pkgs)	1–2 each	Focused single-purpose packages

Identified Issues

1. Outlier: config/expand.go — Env Expansion Duplicates envutil Patterns

Observation: internal/config/expand.go implements its own ${VAR_NAME} environment variable expansion using os.LookupEnv directly. Meanwhile, internal/envutil/ already provides a general-purpose env-var toolkit (GetEnvString, HasEnvVar, etc.).

• File: internal/config/expand.go
• Functions: expandVariablesCore, expandVariables, ExpandRawJSONVariables, expandEnvVariables, expandMapInPlace, expandTracingVariables
• Overlap: The regex-based ${VAR} substitution pattern is config-specific (fail-on-undefined semantics), so it is intentionally different from envutil's defaults. This is acceptable but worth documenting.
• Recommendation: Add a package-level comment in config/expand.go explaining why it doesn't reuse envutil (different error semantics: undefined vars are fatal in config context). This prevents future contributors from questioning the apparent duplication.

---

2. Near-Duplicate: truncateAndSanitize in logger/rpc_format.go Is a Thin Wrapper

Observation: internal/logger/rpc_format.go defines:

func truncateAndSanitize(payload string, maxLength int) string {
    sanitized := sanitize.SanitizeString(payload)
    return util.Truncate(sanitized, maxLength)
}

This is a two-line composition of sanitize.SanitizeString + util.Truncate. It is called within rpc_format.go only.

• File: internal/logger/rpc_format.go (line 28)
• Issue: Private wrapper adds an indirection layer with no additional logic. Callers could inline the two calls.
• Recommendation: Inline the two calls at each usage site, or if the combination is needed elsewhere, promote it to internal/sanitize as sanitize.TruncateAndSanitize(s string, maxLen int) string.
• Estimated Impact: Minor — reduces one private function, improves clarity.

---

3. Structural Pattern: Three Parallel logWithLevel* Dispatcher Functions

Observation: The logger package has three parallel dispatcher functions following the same structural pattern:

Function	File	Pattern
logWithLevel(level, category, format, args)	file_logger.go	dispatches to FileLogger.Log
logWithLevelAndServer(serverID, level, category, format, args)	server_file_logger.go	dispatches to ServerFileLogger.Log + unified log
logWithMarkdown(level, category, format, args)	markdown_logger.go	dispatches to logFuncs[level] + MarkdownLogger.Log

All three are used to build per-level exported functions (LogInfo, LogWarn, LogError, LogDebug and their variants) via newLevelLoggerFuncs / newServerLevelLoggerFuncs. The global_state.go comment notes this pattern explicitly.

• Status: ✅ Already partially refactored — the shared newLogFuncSet generic eliminates structural duplication. The three dispatcher functions themselves are intentionally distinct (different signatures and side effects).
• Recommendation: No immediate refactoring needed. Consider adding a single comment in global_state.go cross-referencing all three dispatchers to help future maintainers navigate the architecture.

---

4. Outlier: rejectRequest (server) and rejectProxyRequest (proxy) — Parallel Rejection Patterns

Observation: Two similar rejection helper functions exist in separate packages:

// internal/server/http_helpers.go
func rejectRequest(w http.ResponseWriter, r *http.Request, status int, code, msg, logCategory, runtimeErrType, runtimeDetail string) {
    logger.LogErrorToMarkdown(logCategory, "Request rejected: ...")
    logRuntimeError(runtimeErrType, runtimeDetail, r, nil)
    httputil.WriteErrorResponse(w, status, code, msg)
}

// internal/proxy/handler.go
func rejectProxyRequest(w http.ResponseWriter, span oteltrace.Span, status int, code, msg string, err error) {
    logger.LogError("proxy", "Request rejected: ...")
    if span != nil { tracing.RecordSpanError(span, err, msg) }
    httputil.WriteErrorResponse(w, status, code, msg)
}

Both ultimately call httputil.WriteErrorResponse, but with different signatures and side-effect responsibilities (server logs to markdown + runtime error; proxy also records a span). They are not consolidation candidates — the divergence is intentional and both are package-private.

• Status: Acceptable divergence.
• Recommendation: None required. The shared httputil.WriteErrorResponse already captures the common denominator.

---

5. Observation: internal/config/config_env.go Gateway Accessors Are Co-located with Parsing Logic

Observation: config_env.go contains both low-level env-parsing (parseAndValidateIntEnv) and high-level gateway config accessors (GetGatewayPortFromEnv, GetGatewayDomainFromEnv, etc.).

• File: internal/config/config_env.go
• Issue: The private parsing helper parseAndValidateIntEnv is used only by the public GetGateway* functions in the same file — this is fine. However, the mix of low-level parsing and high-level accessors in one file may confuse new contributors.
• Recommendation: Consider renaming the file gateway_env.go to better reflect that it contains gateway-specific env var accessors, separating it conceptually from expand.go which handles JSON config expansion.
• Estimated Impact: Cosmetic — improves discoverability.

---

6. Cluster: Three TLS Files Across Packages

Observation: TLS-related functions are spread across three files in different packages:

File	Functions	Purpose
internal/httputil/tls.go	TLSTrustEnvKeys, NewServerTLSConfig, NewClientTLSConfig, ConfigureTLSTrustEnvironment	Generic TLS config builders
internal/server/gateway_tls.go	LoadGatewayTLS	Loads cert/key/CA for gateway mTLS
internal/proxy/tls.go	GenerateSelfSignedTLS, randomSerial, writePEM	Self-signed cert generation for proxy

• Status: ✅ Well-organized. Each file is cohesive within its package — httputil/tls.go has reusable builders, server/gateway_tls.go has the gateway-specific loader, proxy/tls.go has proxy-specific cert generation. No consolidation needed.
• Recommendation: None required.

---

7. Cluster: Format Functions in difc/labels.go

Observation: internal/difc/labels.go (31 functions — the largest file) contains two private formatting helpers:

• formatIntegrityLevel(tags []Tag) string (line 435)
• formatSecrecyLevel(tags []Tag) string (line 468)

These are used only within FormatViolationError in the same file. They are reasonably placed.

• Status: ✅ Acceptable. The functions are cohesive with their only caller.
• Recommendation: If labels.go continues to grow, consider extracting FormatViolationError and its helpers into a dedicated violation_error.go file. Not urgent at current size.

---

Refactoring Recommendations

Priority 1: Low-effort, High-clarity

1. Add explanatory comment to config/expand.go explaining the intentional separation from envutil (different undefined-variable semantics). Prevents future confusion.

   • Effort: ~5 minutes

2. Consider renaming config/config_env.go → config/gateway_env.go to better signal that this file contains gateway-specific environment variable accessors.

   • Effort: ~10 minutes (rename + update any references)

Priority 2: Medium-term

3. Inline or promote truncateAndSanitize in logger/rpc_format.go. Either inline the two calls or move the composite to internal/sanitize if it becomes reused.

   • Effort: ~15 minutes

4. Add navigation comment in global_state.go cross-referencing the three logWith* dispatcher functions so maintainers can quickly find the trio.

   • Effort: ~5 minutes

Priority 3: Future Consideration

5. Extract FormatViolationError + helpers from difc/labels.go into difc/violation_error.go if the file continues to grow beyond its current 31 functions.
   • Effort: ~30 minutes when needed

Analysis Metadata

Metric	Value
Total Go files analyzed	155
Total functions cataloged	966
Packages analyzed	26
Outliers found	2 (config/expand.go semantics, config_env.go naming)
Near-duplicates detected	1 (truncateAndSanitize thin wrapper)
Structural pattern clusters	3 (TLS, logWith*, rejectRequest)
Detection method	Naming pattern analysis + code inspection
Analysis date	2026-07-13

References: §29285489446

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 · 64.4 AIC · ⊞ 8.5K · ◷