[refactor] Semantic Function Clustering Analysis: Outliers, Duplication, and Organization Opportunities

[github-actions]

🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg — July 2026

Executive Summary

This analysis examined 156 non-test Go source files across 26 packages in internal/, cataloguing ~964 functions. The codebase is generally well-organized with clear package boundaries. However, three categories of actionable refactoring opportunities were identified: (1) small single-function packages that could be consolidated with related packages, (2) a few functions that feel slightly misplaced given their file/package context, and (3) two response-writer types that have intentionally-shared base logic already centralized (good pattern worth calling out).

Full Report

Function Inventory

Package Summary

Package	Files	~Functions	Primary Purpose
internal/server	21	134	HTTP server, routing, sessions, tools
internal/config	18	130	Config parsing, validation
internal/difc	12	124	DIFC security labels
internal/logger	16	122	Debug/file/RPC logging
internal/guard	12	79	WASM guards, label agents
internal/mcp	11	73	MCP protocol, connections, transports
internal/proxy	7	52	GitHub API filtering proxy
internal/cmd	14	41	CLI (Cobra flags/commands)
internal/launcher	4	39	Docker backend process management
internal/tracing	7	38	OpenTelemetry tracing
internal/middleware	2	21	jq schema processing
internal/util	6	19	String, format, random, collection utils
internal/httputil	4	17	Generic HTTP helpers, TLS
internal/envutil	4	13	Environment variable utilities
internal/sys	2	12	Container/Docker detection
internal/githubhttp	4	10	GitHub API HTTP helpers
internal/sanitize	1	8	Secret redaction
internal/auth	2	8	Auth header parsing
internal/oidc	1	6	OIDC token provider
internal/syncutil	2	5	Concurrency utilities
internal/version	1	5	Version management
internal/urlutil	1	3	URL domain extraction
internal/mcpresult	1	2	MCP result text helpers
internal/jqutil	1	1	gojq compiler options
internal/tty	2	2	Terminal detection

Identified Issues

1. Micro-packages That Could Be Consolidated

Several single-file packages contain very few functions and serve closely related purposes to larger packages. Keeping them as separate packages adds import overhead for consumers.

internal/jqutil — 1 function, 1 file

• File: internal/jqutil/secure.go
• Functions: CompileOptsWithVariables(varNames []string) []gojq.CompilerOption
• Issue: Package comment itself notes this exists only to share gojq compile options between internal/config and internal/middleware. It is a pure leaf-utility with no domain semantics.
• Recommendation: Merge into internal/middleware (the primary consumer) and expose the SecureCompileOpts variable from there, updating the single import in internal/config/validation_server.go.
• Estimated Effort: 30 minutes
• Impact: Removes one import path; the package comment's stated purpose becomes moot.

internal/mcpresult — 2 functions, 1 file

• File: internal/mcpresult/mcpresult.go
• Functions: NormalizeContentItems, ExtractTextContent
• Issue: These are pure helpers for interpreting MCP tool-call result content. They operate directly on MCP protocol structures and belong logically in internal/mcp.
• Recommendation: Move both functions into internal/mcp/tool_result.go (which already contains ParseToolArguments and related MCP result helpers).
• Estimated Effort: 30 minutes
• Impact: Consolidates all MCP result processing in one location; removes a thin package.

internal/urlutil — 3 functions, 1 file

• File: internal/urlutil/domains.go
• Functions: ExtractURLDomainsFromValue, collectURLDomains, ExtractURLDomains
• Issue: URL domain extraction is used exclusively by internal/middleware and internal/logger for domain audit logging. This is a narrow utility tightly coupled to the middleware audit feature.
• Recommendation: Consider moving into internal/middleware (primary consumer) or keeping as-is if other consumers are anticipated. Low priority since it is already a small, clean package.
• Estimated Effort: 30 minutes
• Impact: Minor consolidation.

2. Outlier Functions (Functions in Files with Mismatched Purpose)

internal/config/validation_rules.go — MountFormat naming inconsistency

• File: internal/config/validation_rules.go
• Function: MountFormat(mount, jsonPath string, index int) *ValidationError (line 125)
• Issue: All other exported functions in validation_rules.go follow the verb-noun pattern (PortRange, TimeoutPositive, AbsolutePath, RequiredStringField, etc.). MountFormat uses noun-verb order and is called from a helper mountValidationError on line 108. The name suggests formatting, not validation.
• Recommendation: Rename to MountPath or ValidateMount to match the file's naming convention, or relocate the mount-specific logic to validation_server.go (where validateMounts already lives).
• Estimated Effort: 15 minutes
• Impact: Improved naming consistency; easier discoverability.

internal/server/difc_log.go — DIFC log helpers split from DIFC package

• File: internal/server/difc_log.go
• Functions: logFilteredItems, buildFilteredItemLogEntry, buildDIFCSingleItemFilteredError, buildDIFCFilteredNotice, difcPolicyLabel
• Issue: These are DIFC-specific log formatting helpers placed in internal/server because they reference server-level logging. They duplicate the conceptual responsibility of internal/difc/violations.go (which already has FormatViolationError, formatIntegrityLevel, formatSecrecyLevel). The split makes it harder to understand the complete picture of how DIFC violations are presented.
• Recommendation: This is an acceptable tradeoff (the functions need server-level logger references). Document the split explicitly, or consider moving the formatting helpers to internal/difc/violations.go and passing in a string-returning logger helper.
• Estimated Effort: 2–3 hours if moved
• Impact: Better cohesion of DIFC formatting logic.

internal/cmd/proxy.go — proxyForcePublicReposIfNeeded policy logic in CLI

• File: internal/cmd/proxy.go (line 348)
• Function: proxyForcePublicReposIfNeeded(ctx context.Context, policyJSON, token, apiURL string) string
• Issue: This function makes a GitHub API call to determine repository visibility and modifies a policy JSON string. It contains business logic (repo visibility + policy mutation) that lives in a CLI command file, making it harder to test independently.
• Recommendation: Move to internal/config/guard_policy_parse.go or a new internal/config/guard_policy_resolve.go, where it sits alongside ResolveGuardPolicyOverride and BuildAllowOnlyPolicy.
• Estimated Effort: 1–2 hours
• Impact: Improved testability; CLI file becomes a thin orchestrator.

3. Well-Designed Patterns Worth Preserving

httputil.BaseResponseWriter — intentional inheritance

internal/httputil/response_writer.go defines BaseResponseWriter (status-code capture), and internal/server/response_writer.go embeds it, adding body buffering. This is the correct Go embedding pattern — not a duplication. The httputil package comment explicitly says to embed BaseResponseWriter to avoid duplicating status-capture boilerplate. ✅

internal/jqutil.SecureCompileOpts — centralized security options

The jqutil package was intentionally created to prevent $ENV-disable options from drifting apart between config and middleware. The intent is sound; the question is only whether a one-function package is the right level of abstraction (see Issue 1 above).

internal/config/validation_* split — appropriate decomposition

The 8 validation_*.go files in internal/config/ are well-organized: each file owns one validation domain (errors, rules, server, gateway, env, schema, tracing, shared). This is a good example of the single-responsibility-per-file principle. ✅

Clustering Results Summary

Cluster: Validation Functions

• Primary location: internal/config/validation_*.go (8 files, ~80 functions) ✅
• Outliers: internal/sys/docker.go:ValidateContainerID, internal/server/session.go:extractAndValidateSession, internal/server/guard_visibility.go:validateSinkVisibilityExemptServers
• Assessment: Outliers are domain-appropriate (docker validation belongs in sys, server session validation belongs in server). Acceptable distribution.

Cluster: Parse Functions

• Spread: internal/mcp/, internal/auth/, internal/githubhttp/, internal/config/, internal/difc/, internal/guard/
• Assessment: All parse functions are domain-local (MCP parsing in mcp, auth parsing in auth, etc.). No consolidation needed.

Cluster: Format Functions

• Spread: internal/util/format.go, internal/difc/violations.go, internal/config/validation_errors.go, internal/config/validation_schema.go
• Assessment: Appropriately split by domain. internal/util/format.go handles generic formatting (session IDs, durations); domain-specific formatters stay with their domain.

Cluster: HTTP Response Writing

• Files: internal/httputil/response_writer.go (base), internal/server/response_writer.go (extended), internal/middleware/content_rewrite.go
• Assessment: Clean embedding hierarchy. ✅

Cluster: Context Key Management

• Files: internal/guard/context.go
• Assessment: guard/context.go manages DIFC-specific context keys (difc-agent-id, difc-request-state). Its package comment explains the intentional split from internal/auth. Acceptable as-is, though the file could be renamed difc_context.go for clarity.

Refactoring Recommendations

Priority 1: Quick Wins (< 1 hour each)

1. Merge internal/mcpresult into internal/mcp/tool_result.go

   • Move NormalizeContentItems and ExtractTextContent
   • Update all import sites
   • Removes a thin package with only 2 functions

2. Rename MountFormat → MountPath in internal/config/validation_rules.go

   • Aligns with the verb-noun convention of the file
   • Update the single call site in mountValidationError

3. Rename internal/guard/context.go → internal/guard/difc_context.go

   • Better signals this file manages DIFC-specific context keys
   • No code changes needed, only file rename

Priority 2: Medium Impact (2–4 hours)

4. Move proxyForcePublicReposIfNeeded from internal/cmd/proxy.go to internal/config/

   • Improves testability of policy resolution logic
   • CLI file becomes a thin coordinator

5. Evaluate merging internal/jqutil into internal/middleware

   • Only one external consumer (internal/config) imports jqutil
   • Can expose SecureCompileOpts from middleware instead

Priority 3: Longer-term (assess before acting)

6. Consolidate DIFC log formatting
   • Consider moving buildDIFCSingleItemFilteredError and buildDIFCFilteredNotice from internal/server/difc_log.go to internal/difc/violations.go
   • Requires decoupling from server-level logger types

Implementation Checklist

• Move NormalizeContentItems / ExtractTextContent from internal/mcpresult → internal/mcp/tool_result.go
• Rename MountFormat → MountPath in internal/config/validation_rules.go
• Rename internal/guard/context.go → internal/guard/difc_context.go
• Move proxyForcePublicReposIfNeeded from internal/cmd/proxy.go → internal/config/guard_policy_resolve.go
• Evaluate internal/jqutil merge into internal/middleware
• Review DIFC log formatting consolidation opportunity
• Run make test-all after each change

Analysis Metadata

• Total Go Files Analyzed: 156 (excl. test files and testutil/)
• Total Functions Catalogued: ~964
• Packages Reviewed: 26
• Function Clusters Identified: 6 major semantic clusters
• Outliers Found: 4 (mcpresult, jqutil micro-packages; proxyForcePublicReposIfNeeded in CLI; MountFormat naming)
• Duplicates Detected: 0 true code duplicates (the BaseResponseWriter embedding is intentional, not duplication)
• Detection Method: grep-based function inventory + cross-package naming pattern analysis + manual semantic review
• Analysis Date: 2026-07-21

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 · 74.1 AIC · ⊞ 8.7K · ◷