[refactor] Semantic Function Clustering: Outliers and Refactoring Opportunities

[github-actions]

Overview

Analysis of 75 non-test Go files across 17 packages in internal/ cataloged 549 function/method definitions. The codebase is generally well-organized, but four meaningful refactoring opportunities were identified: guard-policy parsing logic is scattered across packages, a validation helper sits in the wrong package, DIFC/sink state is embedded in a transport file, and a factory function for building guard policies lives in the CLI layer rather than the config layer.

No critical architectural flaws were found. The logger package (11 files) has good factoring via generics in global_helpers.go. The difc, guard, and config packages are each well-scoped. Recommendations below are ranked by impact.

Full Report

Analysis Metadata

Metric	Value
Go files analyzed	75
Total functions/methods cataloged	549
Packages analyzed	17 (auth, cmd, config, config/rules, difc, envutil, guard, launcher, logger, logger/sanitize, mcp, middleware, server, strutil, sys, timeutil, tty, version)
Outliers found	3
Near-duplicate clusters	1
Detection method	Naming pattern analysis + cross-file usage inspection
Analysis date	2026-03-15

---

Identified Issues

Issue 1 — Guard Policy Parsing Logic Split Between config and server (HIGH)

Problem: The config package defines and owns GuardPolicy, AllowOnlyPolicy, WriteSinkPolicy, ParseGuardPolicyJSON, ValidateGuardPolicy, and NormalizeGuardPolicy. However, three additional policy-parsing functions live inside internal/server/unified.go even though they have no server-specific state dependencies:

Function	File	Lines
parseServerGuardPolicy(serverID string, raw map[string]interface{}) (*config.GuardPolicy, error)	internal/server/unified.go:1127	Parses raw map → *config.GuardPolicy
parsePolicyMap(raw map[string]interface{}) (*config.GuardPolicy, error)	internal/server/unified.go:1167	Dispatches to config.ParseGuardPolicyJSON
normalizeScopeKind(policy map[string]interface{}) map[string]interface{}	internal/server/unified.go:1058	Normalizes scope/scopeKind key names in policy map

Both parseServerGuardPolicy and parsePolicyMap ultimately call config.ParseGuardPolicyJSON. They have no receiver, no server state, and no imports beyond config. Their natural home is config/guard_policy.go or a new config/guard_policy_parser.go.

resolveGuardPolicy and resolveWriteSinkPolicy do reference us *UnifiedServer fields and should remain in server/unified.go.

Recommendation: Move parseServerGuardPolicy, parsePolicyMap, and normalizeScopeKind to internal/config/guard_policy.go. Export them or keep them unexported depending on callers. Update the call sites in unified.go to use the config-package versions.

Estimated effort: 1–2 hours
Benefits: Single source of truth for policy parsing; config package is self-contained; easier unit testing without server setup

---

Issue 2 — isValidAllowOnlyRepos Validator Stranded in guard/wasm.go (MEDIUM)

Problem: guard/wasm.go contains a standalone validator:

// internal/guard/wasm.go:395
func isValidAllowOnlyRepos(repos interface{}) bool {
    switch value := repos.(type) {
    case string:
        trimmed := strings.TrimSpace(strings.ToLower(value))
        return trimmed == "all" || trimmed == "public"
    case []interface{}:
        ...
    }
}
```

This function validates the `repos` field of an `AllowOnlyPolicy` — a `config` package type. The `config` package already has `isValidRepoScope`, `isValidRepoOwner`, `isValidRepoName`, and `isValidTokenString` for related validation. The WASM guard should not duplicate or own policy schema knowledge.

**Recommendation**: Move `isValidAllowOnlyRepos` to `internal/config/guard_policy.go` (rename to `IsValidAllowOnlyRepos` or keep unexported and expose via a helper). Update `wasm.go` to call the config-package version. This eliminates the guard layer's knowledge of policy format.

**Estimated effort**: 30–60 minutes  
**Benefits**: Policy validation is centralized; guard package no longer encodes policy schema

---

### Issue 3 — DIFC Sink State and Context Helpers Embedded in `mcp/connection.go` (MEDIUM)

**Problem**: `internal/mcp/connection.go` is a transport-layer file (stdio/HTTP connection management). It also contains unrelated DIFC sink configuration:

```
SetDIFCSinkServerIDs(serverIDs []string)          // package-level state setter
isDIFCSinkServerID(serverID string) bool          // checks sink membership
GetAgentTagsSnapshotFromContext(ctx) (*Snapshot, bool)  // context extraction
getAgentTagsSnapshotFromContext(ctx) (*Snapshot, bool)  // private alias of above
AgentTagsSnapshot struct                           // data type
AgentTagsSnapshotContextKey const                 // context key
difcSinkServerIDs / difcSinkServerIDsMu            // package-level state

These six symbols are about DIFC metadata enrichment for RPC logging — not about transport. connection.go is already 580+ lines; the DIFC concern adds cognitive overhead when reading transport logic.

Recommendation: Extract these into a new file internal/mcp/difc.go. No API changes needed — same package, same exports. Remove the getAgentTagsSnapshotFromContext private alias (callers can use the exported form directly).

Estimated effort: 30 minutes (file split, no API change)
Benefits: connection.go becomes purely a transport concern; DIFC-related code is discoverable in one place

---

Issue 4 — buildAllowOnlyPolicy Factory in CLI Layer (LOW–MEDIUM)

Problem: internal/cmd/flags_difc.go contains:

func buildAllowOnlyPolicy(public bool, owner, repo, minIntegrity string) (*config.GuardPolicy, error)

This function constructs a *config.GuardPolicy with an AllowOnlyPolicy from CLI flag values. It encodes business logic (e.g., "repo requires owner", "exactly one scope variant"), calls config.ValidateGuardPolicy, and knows the internal structure of config.AllowOnlyPolicy. This is a factory/constructor for a config type and belongs in the config package.

Recommendation: Move to internal/config/guard_policy.go as an exported function — e.g., NewAllowOnlyGuardPolicy(public bool, owner, repo, minIntegrity string) (*GuardPolicy, error). Update cmd/root.go and cmd/flags_difc.go to call config.NewAllowOnlyGuardPolicy(...).

Estimated effort: 45–60 minutes
Benefits: Config package becomes a complete source for policy creation; CLI layer is thinner; function is independently testable without CLI scaffolding

---

Minor: mcp/dockerenv.go Is a Single-Function File (LOW)

internal/mcp/dockerenv.go (41 lines) contains only ExpandEnvArgs, which expands Docker -e flags. It is only called from connection.go:133. This isolation is arguably fine for testability, but the function is simple enough to live in connection.go or a shared mcp/env.go. This is purely cosmetic — no action required unless the team prefers to reduce file count.

---

Function Clusters — Summary

Package	Primary Purpose	Files	Notable Patterns
config	Config parsing, validation, policy	11 files	Well-organized by concern; policy parsing partially leaked to server
logger	Multi-target structured logging	11 files	Generic helpers in global_helpers.go avoid duplication; clean design
difc	DIFC label and evaluation	7 files	Each file has clear single responsibility
server	HTTP server + MCP routing	9 files	unified.go (1500+ lines) is large; policy parsing belongs in config
guard	Security guards (WASM, noop, write-sink)	6 files	wasm.go contains one misplaced policy validator
mcp	MCP protocol connection/transport	6 files	DIFC state co-located with transport in connection.go
cmd	CLI (Cobra flags + root)	7 files	buildAllowOnlyPolicy factory belongs in config
launcher	Backend process management	3 files	log_helpers.go cleanly separates logging helpers from core logic ✓

---

Refactoring Checklist

• [HIGH] Move parseServerGuardPolicy, parsePolicyMap, normalizeScopeKind from server/unified.go → config/guard_policy.go
• [MEDIUM] Move isValidAllowOnlyRepos from guard/wasm.go → config/guard_policy.go
• [MEDIUM] Extract DIFC sink state and context helpers from mcp/connection.go → new mcp/difc.go
• [LOW-MEDIUM] Move buildAllowOnlyPolicy from cmd/flags_difc.go → config/guard_policy.go as NewAllowOnlyGuardPolicy
• For each move: update callers, verify all tests pass (make test-all)

References:

• §23119230896

Generated by Semantic Function Refactoring · ◷