Skip to content

[refactor] Semantic Function Clustering Analysis #5841

@github-actions

Description

@github-actions

Overview

Analysis of 126 non-test Go files across 23 packages (~804 function definitions) in internal/. The codebase is overall well-organized with clear package boundaries and consistent naming conventions. This report surfaces the most actionable refactoring opportunities found through semantic clustering and naming-pattern analysis.

Summary: 3 meaningful findings identified — one misnamed file, one tracing-specific function slightly out of place, and a cross-cutting concern in mcp/collaborator_permission.go.

Full Report

Function Inventory

By Package (file count)

Package Files Primary Purpose
internal/server 18 HTTP server, routing, sessions, auth
internal/logger 16 Debug + file logging framework
internal/cmd 13 Cobra CLI, flag definitions
internal/config 12 TOML/JSON config parsing + validation
internal/guard 11 Security guards (WASM, NoopGuard)
internal/mcp 10 MCP protocol types + connection
internal/difc 9 DIFC label evaluation
internal/proxy 7 GitHub API filtering proxy
internal/strutil 4 String utilities
internal/launcher 4 Backend process management
internal/envutil 4 Environment variable helpers
Others 16 Various utilities

Identified Issues

1. Misleading File Name: guard/init.go ≠ Package Initializer

Severity: Low — Naming Clarity

File: internal/guard/init.go
Content: Contains a single exported function RunLabelAgent(...) — a substantive orchestration function that runs the WASM label agent pipeline.

Problem: In Go, init.go conventionally contains package-level init() functions or package initialization logic. This file contains neither; RunLabelAgent is a domain function called from internal/server/guard_init.go and internal/proxy/proxy.go. The naming creates a false expectation for readers.

Recommendation: Rename guard/init.goguard/label_agent.go to match the function's actual purpose.

# Current
internal/guard/init.go          → RunLabelAgent(...)

# Proposed
internal/guard/label_agent.go   → RunLabelAgent(...)

Estimated effort: < 30 minutes (rename + verify build).
Risk: Minimal — no API surface change.


2. Tracing-Specific Expansion in Generic config/expand.go

Severity: Low — Cohesion

File: internal/config/expand.go
Outlier function: expandTracingVariables(cfg *TracingConfig) error

Problem: expand.go contains generic variable-expansion helpers (expandVariablesCore, expandVariables, ExpandRawJSONVariables, expandEnvVariables) that operate on raw bytes and maps. The function expandTracingVariables is a domain-specific wrapper on *TracingConfig — a tracing concern that mirrors the structure of config_tracing.go.

Comparison:

// expand.go — generic expansion utilities
func expandVariables(value, jsonPath string) (string, error) { ... }
func ExpandRawJSONVariables(data []byte) ([]byte, error) { ... }
func expandEnvVariables(env map[string]string, ...) (map[string]string, error) { ... }

// expand.go — tracing-specific (outlier)
func expandTracingVariables(cfg *TracingConfig) error { ... }  // ← belongs in config_tracing.go

Recommendation: Move expandTracingVariables to internal/config/config_tracing.go where the rest of the tracing config logic lives (TracingConfig, validateOpenTelemetryConfig, etc.).

Estimated effort: ~15 minutes.
Risk: Minimal — unexported function, only called within the config package.


3. Mixed Concerns in mcp/collaborator_permission.go

Severity: Low — Single Responsibility

File: internal/mcp/collaborator_permission.go
Functions:

  • ParseCollaboratorPermissionArgs(...) — argument parsing (pure data)
  • LogAndWrapCollaboratorPermission(...) — logging + response wrapping (side effect + response construction)

Problem: This file mixes two distinct concerns. The comment on LogAndWrapCollaboratorPermission notes it exists to avoid duplicated parse/log/wrap logic between server and proxy packages — a valid motivation. However, the logging side-effect is injected via a logPrintf func(...) parameter, an unusual pattern that adds indirection. Additionally, BuildMCPTextResponse (called inside this function) is defined in mcp/tool_result.go, while the wrapper lives in collaborator_permission.go — response-building concern crosses file boundaries within the same package.

Recommendation: Consider whether LogAndWrapCollaboratorPermission belongs in mcp/tool_result.go alongside BuildMCPTextResponse, or whether the logging concern should be left to callers, reducing collaborator_permission.go to just the parsing function.

Estimated effort: ~1 hour.
Risk: Low — narrow usage, only called from proxy and server packages.


What Was Checked but Found Well-Organized

Area Finding
logger package (16 files) Intentional split: LogInfo* / LogInfoToServer* / LogInfoToMarkdown* are per-sink, not duplicates
parseRateLimitResetHeader vs parseRateLimitResetFromText Parse different formats (Unix timestamp header vs free-text error message) — intentional
config/validation*.go split Clean separation: field validation, env validation, schema validation
guard/wasm_parse.go / wasm_validate.go Proper sub-splitting of a large domain concern
strutil utilities Well-centralized; no scattered duplicates found
difc package (9 files) Good single-purpose file naming throughout

Refactoring Recommendations (Priority Order)

Priority 1: Quick Wins (< 1 hour total)

  1. Rename guard/init.goguard/label_agent.go

    • Effort: < 30 min
    • Benefit: Eliminates misleading filename that suggests package init logic
  2. Move expandTracingVariables to config_tracing.go

    • Effort: ~15 min
    • Benefit: expand.go becomes a pure generic expansion utility

Priority 2: Medium Effort

  1. Consolidate mcp/collaborator_permission.go
    • Evaluate whether response-building belongs with tool_result.go
    • Or simplify by moving logging concern to callers
    • Effort: ~1 hour

Implementation Checklist

  • Rename internal/guard/init.gointernal/guard/label_agent.go
  • Move expandTracingVariables from internal/config/expand.go to internal/config/config_tracing.go
  • Evaluate mcp/collaborator_permission.go concern separation
  • Run make agent-finished to verify all tests pass after changes

Analysis Metadata

Metric Value
Total Go files analyzed 126
Total functions cataloged 804
Packages analyzed 23
Outliers found 2 (misleading filename, misplaced function)
Duplicates detected 0 (apparent duplicates reviewed and found intentional)
Detection method Naming-pattern clustering + cross-reference analysis
Analysis date 2026-05-16

References: §25972993412

Generated by Semantic Function Refactoring · ● 1.1M ·

Metadata

Metadata

Assignees

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