Full Report
Analysis Metadata
- Total Go files analyzed: 74 (excluding test files)
- Total functions cataloged: ~541 top-level functions and methods
- Files with most functions:
internal/server/unified.go (49), internal/guard/wasm.go (33), internal/difc/agent.go (27)
- Outliers found: 7 free functions in wrong packages
- Near-duplicates found: 1 (
TruncateSessionID vs strutil.TruncateWithSuffix)
- Thin single-function packages: 1 (
dockerutil)
- Detection method: Naming-pattern analysis + cross-reference of package boundaries + usage tracing
Finding 1 — Policy-parsing free functions in server/unified.go (High Impact)
File: internal/server/unified.go:597, 1185, 1269, 1309
Four free (non-method) functions that parse and normalize guard policies live in the server package despite operating purely on config types and raw map[string]interface{} data:
| Function |
Line |
What it does |
hasServerGuardPolicies(cfg *config.Config) bool |
597 |
Reports whether any server has per-server guard policies |
normalizeScopeKind(policy map[string]interface{}) |
1185 |
Lowercases / trims the scope_kind field |
parseServerGuardPolicy(serverID string, raw map[string]interface{}) |
1269 |
Resolves a guard policy from a raw JSON map |
parsePolicyMap(raw map[string]interface{}) |
1309 |
Parses allow-only / write-sink policy structure |
These are called only from unified.go's guard-registration pipeline, but they import and return *config.GuardPolicy / *config.Config types and have no dependency on UnifiedServer state. Moving them to internal/config/guard_policy.go (alongside ParseGuardPolicyJSON and ValidateGuardPolicy) or a new internal/config/guard_policy_parse.go would:
- Co-locate all policy-parsing logic in one package
- Make
unified.go solely responsible for orchestration
- Allow these helpers to be reused by other consumers of
config without importing server
Example of the coupling mismatch:
// internal/server/unified.go:1309 — parses config types, belongs in config package
func parsePolicyMap(raw map[string]interface{}) (*config.GuardPolicy, error) {
// ...calls config.ParseGuardPolicyJSON and config.ValidateGuardPolicy...
}
Recommendation: Move parseServerGuardPolicy, parsePolicyMap, normalizeScopeKind, and hasServerGuardPolicies to internal/config/guard_policy.go. Export them (or keep as package-private) as appropriate for the config package's API surface.
Estimated effort: ~1 hour. No logic changes required.
Finding 2 — DIFC tag conversion utilities in server/unified.go (Medium Impact)
File: internal/server/unified.go:1166–1183
Two free functions convert between []string and []difc.Tag:
func toDIFCTags(values []string) []difc.Tag { ... } // line 1166
func tagsToStrings(tags []difc.Tag) []string { ... } // line 1177
These operate exclusively on difc.Tag (a type alias for string) and have zero dependency on UnifiedServer or any server-package type. They belong in internal/difc/ as peer utilities alongside NewSecrecyLabel, NewIntegrityLabel, etc.
Currently both functions are package-private and used only in unified.go, but they would be natural exports from the difc package (e.g., difc.TagsFromStrings / difc.TagsToStrings).
Recommendation: Move to internal/difc/labels.go or a new internal/difc/convert.go, exporting them.
Estimated effort: ~30 min.
Finding 3 — findServerWASMGuardFile in server/unified.go (Medium Impact)
File: internal/server/unified.go:728–755
func findServerWASMGuardFile(serverID string) (string, bool, error) {
// scans MCP_GATEWAY_WASM_GUARDS_DIR/(serverID)/ for .wasm files
}
This function performs filesystem discovery of WASM guard binaries. The internal/guard/ package already owns WASM guard creation (wasm.go) and the guard registry (registry.go). Guard file discovery is a guard concern, not a server concern.
Recommendation: Move to internal/guard/wasm.go or a new internal/guard/discovery.go.
Estimated effort: ~30 min.
Finding 4 — TruncateSessionID partially duplicates strutil.TruncateWithSuffix (Low Impact)
Files: internal/auth/header.go:172–182, internal/strutil/truncate.go:26–34
TruncateSessionID in the auth package implements its own 8-char truncation logic:
func TruncateSessionID(sessionID string) string {
if sessionID == "" { return "(none)" }
if len(sessionID) <= 8 { return sessionID }
return sessionID[:8] + "..."
}
strutil.TruncateWithSuffix(s, 8, "...") already covers the truncation part. The only difference is the "(none)" empty-string sentinel, which is a logging presentation detail.
The function could be simplified to:
func TruncateSessionID(sessionID string) string {
if sessionID == "" { return "(none)" }
return strutil.TruncateWithSuffix(sessionID, 8, "...")
}
This removes duplicated slicing logic and makes auth consistently use the strutil utility.
Recommendation: Refactor TruncateSessionID to delegate truncation to strutil.TruncateWithSuffix.
Estimated effort: ~10 min.
Finding 5 — internal/dockerutil/ is a single-function package (Low Impact)
File: internal/dockerutil/env.go
The entire dockerutil package contains one exported function:
func ExpandEnvArgs(args []string) []string { ... }
It is imported only by internal/mcp/connection.go. The function expands Docker -e KEY style argument pairs from environment variables — which is an environment-variable utility concern, not a Docker-specific concern.
Options:
- Merge into
internal/envutil/ as envutil.ExpandDockerEnvArgs
- Rename the package to
internal/launcher/dockerenv/ to better signal its scope
- Keep as-is if future Docker utilities are planned
Recommendation: Merge into internal/envutil/envutil.go since the function is purely about env-var expansion and has no Docker API dependencies.
Estimated effort: ~20 min.
Summary Table
| Finding |
File |
Functions |
Package Mismatch |
Priority |
| Policy-parsing free functions |
server/unified.go |
parseServerGuardPolicy, parsePolicyMap, normalizeScopeKind, hasServerGuardPolicies |
Should be in config |
High |
| DIFC tag conversions |
server/unified.go |
toDIFCTags, tagsToStrings |
Should be in difc |
Medium |
| WASM guard file discovery |
server/unified.go |
findServerWASMGuardFile |
Should be in guard |
Medium |
| Truncation duplication |
auth/header.go |
TruncateSessionID |
Should use strutil |
Low |
| Single-function package |
dockerutil/env.go |
ExpandEnvArgs |
Should merge into envutil |
Low |
What Was NOT Flagged
- The split between
config/docker_helpers.go (config-time Docker validation) and dockerutil/env.go (runtime env expansion) is intentional and appropriate — different concerns.
logger/rpc_helpers.go:truncateAndSanitize is a private thin wrapper that is co-located with its callers — acceptable.
mcp/connection.go:SetDIFCSinkServerIDs / isDIFCSinkServerID — global state management tied to the connection lifecycle. Moving is possible but lower value.
- All
init() functions across cmd/flags_*.go — standard Go flag-registration pattern.
- Free functions in
server/routed.go (rejectIfShutdown, newFilteredServerCache, CreateHTTPServerForRoutedMode, createFilteredServer) — these all operate on server types and belong in the server package.
Implementation Checklist
Overview
Analysis of 74 non-test Go source files (~541 functions) in
internal/confirms thatinternal/server/unified.go(1,679 lines, 49+ functions) continues to house several free functions whose natural home is another package. Additionally,internal/auth/header.gocontains truncation logic that partially duplicatesinternal/strutil/truncate.go, andinternal/dockerutil/remains a single-function package that could be absorbed. No exact cross-package duplicates were found beyond these cases.All findings below are verified against the current codebase (
HEADmain branch) and represent genuinely actionable improvements — not style nits.Full Report
Analysis Metadata
internal/server/unified.go(49),internal/guard/wasm.go(33),internal/difc/agent.go(27)TruncateSessionIDvsstrutil.TruncateWithSuffix)dockerutil)Finding 1 — Policy-parsing free functions in
server/unified.go(High Impact)File:
internal/server/unified.go:597, 1185, 1269, 1309Four free (non-method) functions that parse and normalize guard policies live in the server package despite operating purely on
configtypes and rawmap[string]interface{}data:hasServerGuardPolicies(cfg *config.Config) boolnormalizeScopeKind(policy map[string]interface{})scope_kindfieldparseServerGuardPolicy(serverID string, raw map[string]interface{})parsePolicyMap(raw map[string]interface{})These are called only from
unified.go's guard-registration pipeline, but they import and return*config.GuardPolicy/*config.Configtypes and have no dependency onUnifiedServerstate. Moving them tointernal/config/guard_policy.go(alongsideParseGuardPolicyJSONandValidateGuardPolicy) or a newinternal/config/guard_policy_parse.gowould:unified.gosolely responsible for orchestrationconfigwithout importingserverExample of the coupling mismatch:
Recommendation: Move
parseServerGuardPolicy,parsePolicyMap,normalizeScopeKind, andhasServerGuardPoliciestointernal/config/guard_policy.go. Export them (or keep as package-private) as appropriate for the config package's API surface.Estimated effort: ~1 hour. No logic changes required.
Finding 2 — DIFC tag conversion utilities in
server/unified.go(Medium Impact)File:
internal/server/unified.go:1166–1183Two free functions convert between
[]stringand[]difc.Tag:These operate exclusively on
difc.Tag(a type alias forstring) and have zero dependency onUnifiedServeror any server-package type. They belong ininternal/difc/as peer utilities alongsideNewSecrecyLabel,NewIntegrityLabel, etc.Currently both functions are package-private and used only in
unified.go, but they would be natural exports from thedifcpackage (e.g.,difc.TagsFromStrings/difc.TagsToStrings).Recommendation: Move to
internal/difc/labels.goor a newinternal/difc/convert.go, exporting them.Estimated effort: ~30 min.
Finding 3 —
findServerWASMGuardFileinserver/unified.go(Medium Impact)File:
internal/server/unified.go:728–755This function performs filesystem discovery of WASM guard binaries. The
internal/guard/package already owns WASM guard creation (wasm.go) and the guard registry (registry.go). Guard file discovery is a guard concern, not a server concern.Recommendation: Move to
internal/guard/wasm.goor a newinternal/guard/discovery.go.Estimated effort: ~30 min.
Finding 4 —
TruncateSessionIDpartially duplicatesstrutil.TruncateWithSuffix(Low Impact)Files:
internal/auth/header.go:172–182,internal/strutil/truncate.go:26–34TruncateSessionIDin theauthpackage implements its own 8-char truncation logic:strutil.TruncateWithSuffix(s, 8, "...")already covers the truncation part. The only difference is the"(none)"empty-string sentinel, which is a logging presentation detail.The function could be simplified to:
This removes duplicated slicing logic and makes
authconsistently use thestrutilutility.Recommendation: Refactor
TruncateSessionIDto delegate truncation tostrutil.TruncateWithSuffix.Estimated effort: ~10 min.
Finding 5 —
internal/dockerutil/is a single-function package (Low Impact)File:
internal/dockerutil/env.goThe entire
dockerutilpackage contains one exported function:It is imported only by
internal/mcp/connection.go. The function expands Docker-e KEYstyle argument pairs from environment variables — which is an environment-variable utility concern, not a Docker-specific concern.Options:
internal/envutil/asenvutil.ExpandDockerEnvArgsinternal/launcher/dockerenv/to better signal its scopeRecommendation: Merge into
internal/envutil/envutil.gosince the function is purely about env-var expansion and has no Docker API dependencies.Estimated effort: ~20 min.
Summary Table
server/unified.goparseServerGuardPolicy,parsePolicyMap,normalizeScopeKind,hasServerGuardPoliciesconfigserver/unified.gotoDIFCTags,tagsToStringsdifcserver/unified.gofindServerWASMGuardFileguardauth/header.goTruncateSessionIDstrutildockerutil/env.goExpandEnvArgsenvutilWhat Was NOT Flagged
config/docker_helpers.go(config-time Docker validation) anddockerutil/env.go(runtime env expansion) is intentional and appropriate — different concerns.logger/rpc_helpers.go:truncateAndSanitizeis a private thin wrapper that is co-located with its callers — acceptable.mcp/connection.go:SetDIFCSinkServerIDs/isDIFCSinkServerID— global state management tied to the connection lifecycle. Moving is possible but lower value.init()functions acrosscmd/flags_*.go— standard Go flag-registration pattern.server/routed.go(rejectIfShutdown,newFilteredServerCache,CreateHTTPServerForRoutedMode,createFilteredServer) — these all operate on server types and belong in the server package.Implementation Checklist
hasServerGuardPolicies,normalizeScopeKind,parseServerGuardPolicy,parsePolicyMapfromserver/unified.gotoconfig/guard_policy.gotoDIFCTags,tagsToStringsfromserver/unified.gotodifc/labels.goordifc/convert.go(exported)findServerWASMGuardFilefromserver/unified.gotoguard/wasm.goorguard/discovery.goTruncateSessionIDinauth/header.goto delegate tostrutil.TruncateWithSuffixdockerutil.ExpandEnvArgsintoenvutiland remove thedockerutilpackagemake agent-finishedto verify no regressionsReferences: