[refactor] Semantic Function Clustering Analysis: One Persistent Outlier + One New Thin Wrapper

[github-actions]

Analysis of repository: github/gh-aw-mcpg
Workflow run: §24394563102

Overview

Analysis of 103 non-test Go source files across 23 packages, cataloging 708 functions. The codebase remains well-structured with two actionable findings: the persistent validateGuardPolicies outlier (first flagged in #3635, still unresolved) and a new thin private wrapper truncateForLog introduced in commit b950a22 (#3713).

Function Inventory

By Package

Package	Files	~Functions	Primary Purpose
internal/auth	2	7	Authentication header parsing, API key generation
internal/cmd	9	22	CLI commands (Cobra), flag registration
internal/config	12	45	Configuration parsing (TOML/JSON), validation
internal/difc	8	40	Decentralized Information Flow Control
internal/envutil	3	5	Environment variable utilities
internal/guard	6	36	Security guards (WASM, NoopGuard, WriteSink, Registry)
internal/httputil	1	1	Shared HTTP helpers
internal/launcher	4	20	Backend process/connection management
internal/logger	14	75	Multi-format logging framework
internal/mcp	6	55	MCP protocol types and connections
internal/middleware	1	8	jq schema processing middleware
internal/oidc	2	6	OIDC token provider
internal/proxy	6	40	GitHub API filtering proxy
internal/server	14	78	HTTP server (routed/unified modes)
internal/strutil	3	4	String utilities
internal/syncutil	1	3	Concurrency utilities
internal/sys	1	4	Container detection
internal/timeutil	1	1	Time formatting
internal/tracing	2	13	OpenTelemetry tracing
internal/tty	1	2	Terminal detection
internal/version	1	2	Version management

Total: 103 non-test Go files, 708 functions cataloged

Identified Issues

1. Outlier Function: validateGuardPolicies in Wrong File (Persistent — 3rd analysis)

Issue: validateGuardPolicies is a config-level validation function sitting in guard_policy.go but belongs in validation.go alongside all other top-level config validators.

• File: internal/config/guard_policy.go:771

• Function: func validateGuardPolicies(cfg *Config) error

• Body:

  func validateGuardPolicies(cfg *Config) error {
      logGuardPolicy.Printf("Validating guard policies: count=%d", len(cfg.Guards))
      for name, guardCfg := range cfg.Guards {
          if guardCfg != nil && guardCfg.Policy != nil {
              if err := ValidateGuardPolicy(guardCfg.Policy); err != nil {
                  return fmt.Errorf("invalid policy for guard '%s': %w", name, err)
              }
          }
      }
      return nil
  }

• Issue: Takes a *Config argument and validates guard policies across the entire config — the same contract as all other top-level validators in validation.go:

  • validateGatewayConfig (line 439)
  • validateTrustedBots (line 508)
  • validateTOMLStdioContainerization (line 525)
  • validateOpenTelemetryConfig (line 553)
  • validateAuthConfig (line 248)

  Both config_core.go:401 and config_stdin.go:358 call validateGuardPolicies in the same validation cascade as the validation.go functions, yet it lives in a different file.

• Recommendation: Move validateGuardPolicies to internal/config/validation.go. Replace the logGuardPolicy reference with logValidation (already present in validation.go).

• Estimated effort: ~30 minutes (move function body; no interface changes; all callers are in the same package)

• Estimated impact: All config-level validators in one canonical file; improved contributor discoverability.

---

2. Thin Private Wrapper: truncateForLog in proxy/graphql.go (New — commit b950a22)

Issue: truncateForLog is a one-line private wrapper introduced in the same commit that added strutil.TruncateRunes. It adds a semantic label but no logic, and is only called twice in the same file.

• File: internal/proxy/graphql.go:193–195
• Function:

  // truncateForLog truncates s to at most maxRunes runes, for safe debug logging.
  func truncateForLog(s string, maxRunes int) string {
      return strutil.TruncateRunes(s, maxRunes)
  }

• Call sites (both in graphql.go):

  logGraphQL.Printf("extractSearchQuery: found in variables: %q", truncateForLog(v, 80))
  logGraphQL.Printf("extractSearchQuery: found inline: %q", truncateForLog(m[1], 80))

• Issue: The wrapper provides no logic beyond strutil.TruncateRunes. The strutil.TruncateRunes function itself is already descriptive. Similar logging elsewhere (e.g., rpc_helpers.go) calls strutil.Truncate directly without a local alias.
• Recommendation: Remove truncateForLog and call strutil.TruncateRunes directly at the two call sites. This is consistent with how other files use strutil utilities.
• Estimated effort: ~5 minutes
• Estimated impact: Removes a trivial indirection; consistent with the rest of the codebase.

---

3. withLock Methods — 4 Logger Types (Informational, No Action Needed)

Four logger types each define an identical 3-line withLock method:

• internal/logger/file_logger.go:61
• internal/logger/jsonl_logger.go:72
• internal/logger/markdown_logger.go:73
• internal/logger/tools_logger.go:86

Context: Each logger type has its own mutex; Go's lack of method inheritance makes a shared generic withLock impractical without major restructuring. This is an accepted Go idiom for small method bodies. No change needed unless the logger type count grows beyond 5–6.

---

Refactoring Recommendations

Priority 1: High Impact, Low Effort

Move validateGuardPolicies to validation.go

• From: internal/config/guard_policy.go:771
• To: internal/config/validation.go (alongside other validate* functions)
• Replace logGuardPolicy with logValidation
• Effort: ~30 minutes
• Benefits: All config-level validators in one canonical file

Priority 2: Low Impact, Trivial Effort

Remove truncateForLog wrapper in proxy/graphql.go

• Replace 2 call sites of truncateForLog(x, 80) with strutil.TruncateRunes(x, 80)
• Delete the wrapper function (3 lines)
• Effort: ~5 minutes
• Benefits: Removes trivial indirection; consistent with rest of codebase

Implementation Checklist

• Move validateGuardPolicies from guard_policy.go to validation.go (use logValidation instead of logGuardPolicy)
• Remove truncateForLog wrapper; call strutil.TruncateRunes directly at 2 call sites in graphql.go
• Run make agent-finished after changes to verify build and tests pass

Analysis Metadata

• Total Go Files Analyzed: 103 (excluding test files)
• Total Functions Cataloged: 708
• Packages Examined: 23
• Outliers Found: 1 (validateGuardPolicies in wrong file — persistent from prior runs)
• Thin Wrappers Found: 1 (truncateForLog — new in commit b950a22)
• Near-Duplicate Patterns: 1 (withLock ×4 — documented intentional)
• Exact Duplicate Implementations: 0
• Detection Method: Naming pattern analysis, cross-package grep, function signature review
• Analysis Date: 2026-04-14

References:

• §24394563102
• Prior analysis (closed): github/gh-aw-mcpg#3692

Generated by Semantic Function Refactoring · ● 520.4K · ◷