Skip to content

[refactor] Semantic Function Clustering Analysis — Refactoring Opportunities #10083

Description

@github-actions

Overview

Automated semantic function clustering analysis of 999 functions across 149 non-test Go source files in internal/. The codebase is well-structured overall, with packages appropriately decomposed into focused files. This report highlights the most actionable refactoring opportunities found.

Summary: 149 files analyzed · 999 functions cataloged · 6 function clusters identified · 3 outlier patterns · 4 duplicate/near-duplicate candidates

Full Report

Function Inventory by Package

Package Files Functions Notes
internal/config 20 143 Well-split by concern (validation_*, guard_policy_*)
internal/server 20 132 Large unified.go (828 lines, 26 funcs)
internal/difc 11 124 Good file separation
internal/logger 15 122 Parallel LogXxx, LogXxxToServer, LogXxxToMarkdown families
internal/guard 11 80 wasm_lifecycle.go is large (623 lines)
internal/mcp 9 72 Connection well-split across 4 files
internal/proxy 6 51 Good organization
internal/cmd 11 40 flags_* split is appropriate
internal/launcher 4 40 Reasonable
internal/tracing 6 38 Fine
internal/util 7 22 Appropriately scattered utilities
internal/middleware 2 21 Fine
Other (11 pkgs) 12 94 Small, focused

Identified Issues

1. internal/server/unified.go — Oversized File with Mixed Concerns

File: internal/server/unified.go
Size: 828 lines, 26 top-level functions/methods
Issue: This single file contains the UnifiedServer constructor, backend calling logic, guard integration, DIFC enforcement, tool call lifecycle (6 phases), rate limiting, and lifecycle management. While this is a central orchestrator, several logical sub-concerns could be extracted to improve navigability.

Functions that could move to existing or new companion files:

  • executeBackendRequest[T] and executeBackendToolCall — backend dispatch logic, could move to a backend_dispatch.go companion
  • guardBackendCaller type and its methods (CallTool, callCollaboratorPermission) — guard-specific caller could live in guard_init.go alongside internal/server/guard_init.go
  • enforceToolCallLimit — already in rate_limit.go conceptually; consider moving

Estimated effort: Low — just file splits, no logic changes
Impact: Easier navigation and review of the main unified.go orchestration path


2. Duplicate-Named writeToFile Methods on Different Logger Types

Files: internal/logger/tools_logger.go:89 and internal/logger/observed_url_domains_logger.go:105
Pattern: Both ToolsLogger and ObservedURLDomainsLogger define a private writeToFile() error method with identical signatures. While these are method receivers on different types (not free-function duplicates), both simply delegate to writeJSON:

// tools_logger.go
func (tl *ToolsLogger) writeToFile() error {
    return tl.writeJSON(tl.data, 0644)
}

// observed_url_domains_logger.go
func (l *ObservedURLDomainsLogger) writeToFile() error {
    serialized := make(map[string][]string, len(l.data))
    for serverID, domains := range l.data {
        serialized[serverID] = util.SortedSetKeys(domains)
    }
    return l.writeJSON(serialized, 0600)
}

Assessment: These are not true duplicates — they wrap different logic. However, the writeToFile naming is misleading since both loggers already have writeJSON from the embedded jsonFileSink. The intermediate writeToFile methods add a thin wrapper that could be inlined into the callers (LogTools and LogDomains), reducing a layer of indirection.

Recommendation: Inline writeToFile calls at call sites and remove the methods, or rename to something that expresses the transform (e.g., serializeAndWrite).
Estimated effort: Trivial (< 1 hour)
Impact: Reduced indirection in the logger package


3. internal/envutil/expand_env_args.go vs internal/config/expand.go — Related Expansion Logic in Different Packages

Files: internal/envutil/expand_env_args.go (Docker -e flag expansion) and internal/config/expand.go (${VAR_NAME} template expansion)
Concern: Both files implement environment-variable expansion but with deliberately different semantics:

  • envutil: expands Docker -e VAR_NAME-e VAR_NAME=value, silently skips missing vars
  • config: expands ${VAR_NAME} in config strings, fails hard on undefined vars

The config/expand.go file has a prominent comment explaining why it does not reuse envutil. This is well-documented intentional design.

Assessment: Not a refactoring issue per se, but the similar purpose could confuse maintainers.
Recommendation: Add a cross-reference comment in envutil/expand_env_args.go pointing to config/expand.go noting the different error semantics, to complete the documentation picture.
Estimated effort: Trivial (5 minutes)


4. TLS Helpers Split Across internal/proxy/tls.go and internal/httputil/tls.go

Files: internal/proxy/tls.go (certificate generation) and internal/httputil/tls.go (protocol-level helpers)
Pattern: Each file has a package-level var logTLS = logger.ForFile() with the same name but in different packages.

Assessment: This split is correctly documented in the package-level doc comment of both files and follows the design principle: proxy owns cert generation, httputil owns cert loading and configuration. This is correct organization, not an outlier.

Recommendation: No change needed. The existing doc comments are good.


5. internal/server/response_writer.go Extends internal/httputil/response_writer.go

Files: internal/server/response_writer.go and internal/httputil/response_writer.go
Pattern: server.responseWriter embeds httputil.BaseResponseWriter and adds body buffering. This is the intended use of the BaseResponseWriter embedding pattern (the doc comment on BaseResponseWriter explicitly says "Embed BaseResponseWriter in package-specific types to avoid duplicating this status-capture boilerplate").

Assessment: This is correct OO-style composition, not duplication.
Recommendation: No change needed.


6. Scattered Format/Duration Helpers

Functions found in multiple files:

  • internal/util/format.go: FormatDuration, FormatFutureTime, FormatSessionIDForLog
  • internal/logger/global_state.go: formatLogLine
  • internal/logger/slog_adapter.go: formatSlogValue
  • internal/logger/rpc_format.go: 6 RPC-specific format functions

Assessment: The util package appropriately owns general-purpose formatters. Logger-specific formatters live in the logger package. There is some overlap in "format a duration" functionality:

  • util.FormatDuration — human-readable duration formatting for session display
  • logger.formatLogLine — log-line formatting

These serve different consumers and have different signatures. No consolidation needed unless FormatDuration is reimplemented elsewhere.

Recommendation: Audit call sites of FormatDuration to confirm no equivalent exists inside logger package; if there is, consolidate.
Estimated effort: Low


Clustering Results Summary

Cluster A: Validation Functions (internal/config/validation_*.go)

6 files, 50 functions. Well-organized — each file targets a specific validation domain:

  • validation_env.go — environment variable validation
  • validation_errors.go — error construction helpers
  • validation_gateway.go — gateway config validation
  • validation_rules.go — reusable validation rule primitives
  • validation_schema.go — JSON schema validation
  • validation_server.go — per-server config validation
  • validation_tracing.go — tracing config validation

✓ No outliers detected.

Cluster B: Guard Functions (internal/guard/wasm_*.go)

5 files, 39 functions. wasm_lifecycle.go at 623 lines handles WASM module initialization, teardown, and reconnect logic. This is large but all within the same concern domain.

Potential split: The WASM module loading/compilation phase (lines ~1–250) vs. the session lifecycle management phase (lines ~250–623) could be separated into wasm_compile.go and wasm_session.go for easier navigation.

Cluster C: Logger Infrastructure (internal/logger/)

15 files, 122 functions. The global_helpers.go file has 23 functions that are the public API shim for all logger types. This is the intended design (dispatcher pattern).

Potential improvement: global_helpers.go mixes logger initialization helpers with public LogXxx API functions. Consider splitting into api.go (public-facing LogInfo, LogWarn, etc.) and keeping init helpers in global_helpers.go.

Cluster D: DIFC Label Types (internal/difc/)

11 files, 124 functions. difc/agent.go has 25 functions — the largest file in the package. It handles agent label management including CRUD operations for all label types.

Assessment: Given the complexity of the DIFC model, this is acceptable. No critical outliers.

Cluster E: Proxy (internal/proxy/)

6 files, 51 functions. proxy/router.go has 14 functions — routing, auth token extraction, and URL construction. This is manageable.

Cluster F: Connection (internal/mcp/connection*.go)

4 files, 29 functions for Connection methods alone. The split into connection.go (core), connection_logging.go (logging), connection_methods.go (SDK method wrappers), connection_transport.go (transport reconnect) is clean.


Refactoring Recommendations

Priority 1: High Impact

1.1 — Split internal/server/unified.go (~828 lines)

Extract the guard-specific guardBackendCaller type and its methods to internal/server/guard_init.go (or a new guard_caller.go), and move executeBackendRequest/executeBackendToolCall (backend dispatch) to a new internal/server/backend_dispatch.go. This would reduce unified.go to ~650 lines focused on server lifecycle and tool handler registration.

  • Estimated effort: 2–3 hours
  • Risk: Low (pure file moves, no logic changes)
  • Benefit: Easier code review and onboarding

1.2 — Inline or rename writeToFile in logger types

Remove the writeToFile indirection layer in tools_logger.go and observed_url_domains_logger.go by either inlining the writeJSON calls at call sites or renaming to reflect the transform (e.g., persistToDisk).

  • Estimated effort: <1 hour
  • Risk: Minimal
  • Benefit: Cleaner logger internals

Priority 2: Medium Impact

2.1 — Split internal/guard/wasm_lifecycle.go (~623 lines)

Consider separating WASM compilation/loading (loadWasmGuard, compileWasm) from session lifecycle management. A proposed split: wasm_compile.go (compilation + loading, ~250 lines) and wasm_lifecycle.go (session management, ~370 lines).

  • Estimated effort: 1–2 hours
  • Risk: Low
  • Benefit: Easier navigation

2.2 — Split internal/logger/global_helpers.go (23 functions)

Separate the public logging API (LogInfo, LogWarn, LogError, LogDebug families) into a new internal/logger/api.go file, keeping initialization helpers in global_helpers.go. This makes the public API surface immediately discoverable.

  • Estimated effort: 30 minutes
  • Risk: Minimal
  • Benefit: Better discoverability

Priority 3: Low Impact / Maintenance

3.1 — Cross-reference comment in envutil/expand_env_args.go

Add a comment pointing to config/expand.go and explaining the different error semantics (soft skip vs. hard fail). Prevents future confusion when the two files' purposes overlap.

  • Estimated effort: 5 minutes
  • Risk: None
  • Benefit: Documentation improvement

Implementation Checklist

  • Review Priority 1.1: plan unified.go split and identify all internal references
  • Review Priority 1.2: inline writeToFile wrappers in logger types
  • Review Priority 2.1: plan wasm_lifecycle.go split
  • Review Priority 2.2: plan global_helpers.go public API extraction
  • Add cross-reference comment in envutil/expand_env_args.go
  • Verify no behavior changes after each file split with make test-all

Analysis Metadata

  • Total Go Files Analyzed: 149 (excluding test files and internal/testutil/)
  • Total Functions Cataloged: 999
  • Function Clusters Identified: 6
  • Outlier Patterns Found: 3 (all assessed as intentional design or minor)
  • True Duplicates Detected: 0 (all apparent duplicates serve distinct purposes)
  • Near-Duplicates (thin wrappers): 1 (writeToFile in two logger types)
  • Detection Method: Grep-based function extraction + cross-file name analysis + semantic pattern review
  • Analysis Date: 2026-07-25T21:15:21Z

Generated by Semantic Function Refactoring · sonnet46 · 77.1 AIC · ⊞ 9.9K ·

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions