Skip to content

[refactor] Semantic Function Clustering Analysis: One Remaining Outlier + Completed Refactors #3635

@github-actions

Description

@github-actions

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

Overview

Analysis of 103 non-test Go source files across 23 packages, cataloging 706 functions. The codebase is well-structured and two of the three actionable findings from the previous analysis have since been implemented. One outlier function remains: validateGuardPolicies lives in guard_policy.go instead of validation.go. No new duplicates or outliers were detected in the latest changes.

Progress Since Last Analysis

Finding Status
Move validateGuardPolicies to validation.go Still open
Add strutil.TruncateRunes and delegate from truncateForLog FixedTruncateRunes added to internal/strutil/truncate.go; truncateForLog now calls it
Inline lookupEnrichmentToken alias in unified.go Fixed — removed; call sites use envutil.LookupGitHubToken() directly

Function Inventory

By Package

Package Files ~Functions Primary Purpose
internal/auth 2 6 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, 706 functions cataloged

Identified Issues

1. Outlier Function: validateGuardPolicies in Wrong File (Unchanged)

Issue: validateGuardPolicies is a config-level validation function that lives in guard_policy.go but belongs alongside the other config validators in validation.go.

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

  • 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 alongside the other validate* functions. The logGuardPolicy debug logger reference will need to be updated (either use the existing logger in validation.go or keep a reference).

  • Estimated effort: ~30 minutes (move function body; no interface changes; no callers outside config package)

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

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

Issue: 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: This is intentionally documented in internal/logger/common.go as necessary for consistent mutex handling. The withMutexLock helper already centralizes the locking logic itself. Go's lack of method inheritance makes a single shared method impractical without significant restructuring.

Recommendation: No change required. If the number of logger types grows significantly beyond 5–6, revisit embedding a baseLogger struct.

Refactoring Recommendations

Priority 1: High Impact, Low Effort

Move validateGuardPolicies to validation.go

  • From: internal/config/guard_policy.go:692
  • To: internal/config/validation.go (alongside other validate* functions)
  • Effort: ~30 minutes
  • Benefits: All config-level validators in one canonical file; easier to find for contributors

Implementation Checklist

  • Move validateGuardPolicies from guard_policy.go to validation.go (update logger reference as needed)
  • 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: 706
  • Packages Examined: 23
  • Outliers Found: 1 (validateGuardPolicies in wrong file)
  • Near-Duplicate Patterns: 1 (withLock ×4 — documented intentional)
  • Exact Duplicate Implementations: 0
  • New Issues vs Prior Run: 0 (all prior fixes confirmed; one prior finding still open)
  • Detection Method: Naming pattern analysis, cross-package grep, semantic review
  • Analysis Date: 2026-04-12

References:

Generated by Semantic Function Refactoring · ● 729.3K ·

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions