🔧 Semantic Function Clustering Analysis

Analysis of repository: github/gh-aw-mcpg — internal/ directory (~180 non-test .go files)

Executive Summary

No open [refactor]-prefixed issues were found to close before starting this analysis (existing [duplicate-code]-prefixed issues belong to a separate workflow and were left untouched). I scanned every non-test .go file under internal/, extracted ~1,400 top-level function/method declarations, clustered them by name/purpose, and cross-referenced same-named functions across files/packages to look for outliers and duplication. The codebase is generally well organized (small, single-purpose files, consistent validation_*.go / flags_*.go / guard_*.go naming families). I found a handful of near-duplicate implementations worth consolidating, and no clear "function in the wrong file" outliers of significant size — most validate*/Validate* occurrences outside internal/config are legitimately package-local (auth, delegation, launcher, enclavegithub each validate their own domain types) rather than misplaced config validation.

Function Inventory (highlights)

• internal/config/: 18 files, ~90 functions, split cleanly into validation_{env,errors,gateway,rules,schema,server,tracing}.go, config_{core,feature,stdin,tracing}.go, expand.go, guard_policy*.go. Well organized ✓
• internal/guard/: 11 files implementing the Guard interface (NoopGuard, WasmGuard, WriteSinkGuard) — each guard repeats the same method set (LabelAgent, LabelResource, LabelResponse, NewSessionGuard), which is expected interface-implementation duplication, not a smell.
• internal/server/ + internal/proxy/: delegation control-plane wiring duplicated (ControlHandler, handleDelegationControl) — flagged below.
• internal/tty/: tty.go / tty_stub.go both define IsStderrTerminal — intentional Go build-tag pattern (!tty_stub vs tty_stub), not a duplicate.
• internal/delegation/identity.go: toRequest defined twice in the same file on different receivers (delegationBinding and *Identity) with near-identical field-copy bodies.

Identified Issues

1. Duplicate Delegation Control-Plane Wiring (proxy vs server)

• Files: internal/proxy/delegation.go (lines 40–65) and internal/server/delegation.go (lines 173–189)
• Functions: ControlHandler() and handleDelegationControl(w, r) are defined nearly identically on *proxy.Server and *server.UnifiedServer. Both build a delegation.ControlDeps{Store, Capability, StatePath} from local fields and call delegation.HandleControl(...) with a logger's Printf, gated by an http.NotFound prefix check on delegation.ControlPathPrefix.
• Similarity: ~90% structurally identical; only field access paths differ (h.server.delegation.* vs us.delegation.*).
• Recommendation: Extract a small shared helper in internal/delegation (e.g. delegation.NewControlHandler(deps ControlDeps, logf func(string, ...any)) http.Handler plus a ControlDepsProvider interface with a ControlDeps() ControlDeps method) so both proxy.Server and server.UnifiedServer can construct their control handler via one shared call, eliminating the duplicated http.NotFound/prefix-check/dispatch logic in both packages.
• Estimated Impact: ~25 duplicated lines removed; single source of truth for the control-plane guard logic.

2. Duplicate Struct-to-Request Conversion (toRequest)

• File: internal/delegation/identity.go
• Functions: (b delegationBinding) toRequest(...) (line 70) and (id *Identity) toRequest() (line 161)
• Issue: Both copy the same 8 fields (RunID, EnclaveBackend, EnclaveEntryID, InvocationID, Repository, ToolPolicy, SchemaHash, AdmittedDefaultBranchSHA) into a CreateOrConfirmRequest. Since Identity already embeds/derives a delegationBinding-shaped value, (*Identity).toRequest() could likely delegate to delegationBinding.toRequest(...) instead of re-listing every field.
• Recommendation: Have Identity.toRequest() build/reuse a delegationBinding and call binding.toRequest(...), removing one full field-copy block.
• Estimated Impact: Minor, but removes a silent-drift risk (a future field added to one struct but not the other).

3. Scattered Format* / formatting helpers (acceptable, monitor)

• internal/util/format.go, internal/config/error_format.go, internal/logger/rpc_format.go, internal/logger/slog_adapter.go, and internal/difc/violations.go each define their own format* helpers with no shared implementation. Each serves a distinct domain (session IDs, config errors, RPC message text, slog value stringification, DIFC violation messages), so this is not flagged as a hard duplicate — but if a generic "truncate + label" pattern keeps recurring, consider consolidating trivial string-formatting helpers into internal/util/format.go, which already exists as the shared utility home.

4. validate*/Validate* outside internal/config — reviewed, not outliers

Occurrences in internal/sys/docker.go, internal/proxy/tls.go, internal/util/collections.go (generic ValidateUnique[T]), internal/server/{guard_visibility,session,tool_registry,guard_init}.go, internal/auth/header.go, internal/delegation/{recovery,store,envelope}.go, internal/launcher/mount_policy.go, and internal/enclavegithub/{policy,capability}.go were reviewed individually. Each validates a type/concept owned by its own package (container IDs, TLS DNS names, session extraction, agent-policy tools, delegation envelopes/state, mount specs, enclave policy/claims) rather than generic config validation — correctly located, no move recommended.

Refactoring Recommendations

Priority 1

1. Consolidate delegation control-plane handler wiring (internal/proxy/delegation.go + internal/server/delegation.go) into a shared internal/delegation helper. Estimated effort: 1–2 hours. Benefit: removes ~25 duplicated lines and a maintenance hazard where the two copies could drift (e.g., one gaining a security check the other misses).

Priority 2

2. Deduplicate toRequest conversions in internal/delegation/identity.go by having Identity.toRequest() delegate to delegationBinding.toRequest(). Estimated effort: 30 minutes. Benefit: single source of truth for the request field mapping.

Priority 3 (monitor only, no action needed now)

3. Keep an eye on scattered format* helpers — no action recommended today since each is domain-specific and small.

Analysis Metadata

• Total Go files analyzed: ~180 (non-test, under internal/)
• Total functions/methods cataloged: ~1,400
• Duplicate/near-duplicate clusters flagged: 2 (delegation control-plane wiring, toRequest conversion)
• False positives ruled out: guard interface implementations (NoopGuard/WasmGuard/WriteSinkGuard), tty build-tag variants (tty.go/tty_stub.go), MatchRoute (different packages/signatures), validate* functions outside internal/config (correctly domain-scoped)
• Detection method: static grep-based function inventory + cross-file name/signature comparison (Serena MCP tools were not available in this environment's toolset; GitHub MCP + shell-based static analysis used instead)
• Analysis date: 2026-09-11

Generated by Semantic Function Refactoring · copilot · auto · 50.4 AIC · ⊞ 13.2K · ◷