[refactor] Semantic Function Clustering Analysis

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg
Run: §29209100965
Date: 2026-07-12

Executive Summary

Analysis of 140 non-test Go source files across 22 packages catalogued 964 exported and unexported functions. The codebase is generally well-organized with clear package boundaries. This report highlights three meaningful areas for improvement: (1) parallel ResponseWriter wrapper types across packages that partially duplicate each other, (2) TLS-helper functions spread across four packages, and (3) small utility functions (FormatSessionIDForLog, Truncate) that live outside the package most likely to want them.

No exact duplicate implementations were found. The patterns below are near-duplicates or misplaced functions — the kind of low-risk, high-clarity refactors.

---

Function Inventory

Package	Files	Functions	Primary Purpose
internal/server	20	135	HTTP server, routed/unified modes
internal/config	18	129	Config parsing and validation
internal/difc	10	124	Decentralized Information Flow Control
internal/logger	16	112	Debug/file/RPC logging
internal/guard	12	78	Security guards (WASM, WriteSink, Noop)
internal/mcp	11	73	MCP protocol types and transport
internal/proxy	7	47	GitHub API filtering proxy
internal/launcher	4	39	Backend process management
internal/tracing	7	38	OpenTelemetry tracing
internal/cmd	14	38	CLI (Cobra commands/flags)
internal/middleware	2	23	jq schema processing
internal/util	6	18	String, format, random, JSON helpers
internal/httputil	4	13	Generic HTTP helpers
internal/envutil	4	13	Environment variable utilities
Others	~12	~62	auth, sanitize, tty, sys, oidc, ...

Total: 964 functions across 140 files

---

Identified Issues

Full Report

1. Three ResponseWriter Wrapper Types With Overlapping Purpose

Severity: Medium | Estimated effort: 1–2 hours

There are three separate http.ResponseWriter wrapper structs that capture HTTP status codes:

Type	File	Purpose
BaseResponseWriter	internal/httputil/response_writer.go	Embeddable base: captures status, buffers implicit 200
responseWriter	internal/server/response_writer.go	Extends BaseResponseWriter to also buffer body bytes for debug logging
statusResponseWriter	internal/tracing/http.go	Extends BaseResponseWriter for OTel span status

The server and tracing variants already embed httputil.BaseResponseWriter, so the core duplication is resolved. However, statusResponseWriter in internal/tracing/http.go is an unnamed, unexported struct defined inline in the file. For clarity it should either be documented or folded into BaseResponseWriter if its sole behaviour is identical (it is: no additional methods beyond the embedding).

Recommendation: Document the design intent at the top of httputil/response_writer.go stating it is the canonical base. Confirm statusResponseWriter adds no behaviour beyond the embed (it doesn't) and consider whether it can be replaced with a direct BaseResponseWriter embed at the call site in tracing/http.go.

---

2. TLS Helper Functions Scattered Across Four Packages

Severity: Medium | Estimated effort: 2–3 hours

TLS-related helper functions are spread across four distinct locations:

internal/httputil/tls.go
  NewServerTLSConfig(cert tls.Certificate) *tls.Config
  NewClientTLSConfig() *tls.Config

internal/server/gateway_tls.go
  LoadGatewayTLS(certPath, keyPath, caPath string) (*tls.Config, error)

internal/proxy/tls.go
  GenerateSelfSignedTLS(dir string) (*TLSConfig, error)
  writePEM(path, blockType string, derBytes []byte, perm os.FileMode) error

internal/cmd/proxy.go
  configureTLSTrustEnvironment(caCertPath string) error

The four locations have distinct concerns (generic config builders, gateway-level loading, self-signed cert generation, trust store configuration) but a developer looking for "where does TLS live?" will need to check four packages. The writePEM helper in proxy/tls.go is an implementation detail of GenerateSelfSignedTLS and does not belong in a shared location — that's fine. The concern is discoverability.

Recommendation: Add a package-level comment to internal/httputil/tls.go pointing to server/gateway_tls.go and proxy/tls.go for the higher-level functions. Alternatively, consider whether GenerateSelfSignedTLS belongs in httputil since it's a generic TLS utility that happens to be used by the proxy. No move is strictly required; documentation is sufficient.

---

3. FormatSessionIDForLog Lives in a Separate One-Function File

Severity: Low | Estimated effort: 30 minutes

internal/util/session.go contains a single function:

func FormatSessionIDForLog(sessionID string) string

This function calls Truncate from the same package. It is a formatting utility and fits naturally into internal/util/format_duration.go (which already houses FormatFutureTime and FormatDuration) or a new internal/util/format.go. Having a file with a single 10-line function creates unnecessary file-count overhead.

Recommendation: Move FormatSessionIDForLog into internal/util/format_duration.go (or rename that file to format.go to reflect its broader formatting scope) and delete session.go.

---

4. config/config_env.go Partially Duplicates envutil.GetEnvInt

Severity: Low | Estimated effort: 1 hour

internal/config/config_env.go defines a package-private helper:

func parseAndValidateIntEnv(envKey string, validate func(int) *ValidationError) (int, bool, error)

This is a thin wrapper over envutil.GetEnvIntRaw that adds domain-specific validation. The pattern is sound (config validation belongs in the config package). However, the function name and shape are very similar to envutil.GetEnvInt, envutil.GetEnvIntRaw, and envutil.GetEnvDuration. A developer new to the codebase may implement a fourth version elsewhere.

Recommendation: Add a comment to parseAndValidateIntEnv (and its sibling GetGatewayPortFromEnv etc.) explaining that the config-level functions intentionally wrap envutil with domain validation. This is a documentation-only change.

---

5. cmd/proxy.go::configureTLSTrustEnvironment Is an Outlier in a Command File

Severity: Low | Estimated effort: 30 minutes

internal/cmd/proxy.go contains:

func configureTLSTrustEnvironment(caCertPath string) error

This function manipulates the system TLS trust store — it's an infrastructure utility, not a CLI-command-handler concern. It does not depend on any cobra.Command state. It would be more discoverable in internal/httputil/tls.go or internal/proxy/tls.go.

Recommendation: Move configureTLSTrustEnvironment to internal/httputil/tls.go and update the call site in cmd/proxy.go. This improves discoverability and keeps the command file focused on wiring flags to behavior.

---

6. logger/fileutil.go::atomicWriteFile Could Be Shared More Broadly

Severity: Low | Estimated effort: 1 hour

internal/logger/fileutil.go defines:

func atomicWriteFile(filePath string, data []byte, perm os.FileMode) error
func writeJSONToFile(logDir, fileName string, data any, perm os.FileMode) error

These are currently unexported logger internals, but atomicWriteFile is a general-purpose utility. It is only used within the logger package. If any future package needs atomic file writes, it would either duplicate the logic or depend on internal/logger (creating an awkward dependency). Consider exporting or relocating it.

Recommendation: Export atomicWriteFile as logger.AtomicWriteFile or move it to a new internal/fileutil package if/when a second consumer appears. No immediate action required; flag for the next new use case.

---

Clustering Results: What's Well-Organized ✅

The following packages demonstrate excellent single-responsibility organization and are explicitly highlighted to confirm no action is needed:

• internal/sanitize: All 7 functions (SanitizeString, SanitizeJSON, RedactSecret, RedactSecretMap, RedactURL, SanitizeArgs, MarshalAndSanitize) are cohesively scoped to data redaction.
• internal/auth: 8 functions all directly related to authentication header parsing and session ID extraction.
• internal/envutil: Clean separation between envutil.go (typed env getters), envfile.go (file loading), expand_env_args.go (Docker arg expansion), and github.go (GitHub token lookup).
• internal/config/expand.go: Variable expansion functions are cleanly isolated from validation.
• internal/jqutil/secure.go: Correctly factors out shared gojq compiler options to prevent the internal/config and internal/middleware packages from independently defining identical options — a good example of proactive de-duplication.
• internal/util: Reasonable split across util.go (map/JSON helpers), collections.go (slice helpers), format_duration.go (time formatting), random.go (random bytes), truncate.go (string truncation), session.go (session ID formatting — though see Issue Lpcox/initial implementation #3 above).

---

Refactoring Recommendations by Priority

Priority 1 (High Impact)

1. [Issue Lpcox/initial implementation #2] Document TLS function locations — add cross-package comments so developers know where all TLS helpers live. Consider moving GenerateSelfSignedTLS to httputil.
2. [Issue Updated Dockerfile #5] Move configureTLSTrustEnvironment from cmd/proxy.go → httputil/tls.go.

Priority 2 (Medium Impact)

3. [Issue Configure as a Go CLI tool #1] Document BaseResponseWriter as the canonical embed base; verify statusResponseWriter in tracing/http.go adds no unique behaviour.

Priority 3 (Low / Housekeeping)

4. [Issue Lpcox/initial implementation #3] Merge util/session.go (single function) into util/format_duration.go or a new util/format.go.
5. [Issue Lpcox/add difc #4] Add a comment to config_env.go helpers explaining the intentional layering over envutil.
6. [Issue Updated Dockerfile to run 'go mod tidy' #6] Flag atomicWriteFile for export/relocation when a second consumer emerges.

---

Implementation Checklist

• Add cross-reference comments to TLS packages (Priority 1)
• Move configureTLSTrustEnvironment to httputil/tls.go (Priority 1)
• Confirm statusResponseWriter is a pure embed and document BaseResponseWriter intent (Priority 2)
• Merge util/session.go into util/format_duration.go (Priority 3)
• Add layering comment to config_env.go (Priority 3)
• Note atomicWriteFile as a future export candidate (Priority 3)

---

Analysis Metadata

Field	Value
Total Go Files Analyzed	140
Total Functions Catalogued	964
Function Clusters Identified	6 meaningful groupings analyzed
Outliers Found	3 (TLS scatter, configureTLSTrustEnvironment, FormatSessionIDForLog)
Duplicates Detected	0 exact; 1 near-duplicate pattern (statusResponseWriter vs BaseResponseWriter)
Detection Method	Naming-pattern grep + cross-package dependency analysis

References:

• §29209100965

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