Skip to content

[refactor] Semantic Function Clustering Analysis — 7 Unresolved Items (Carry-Forward) #1799

Closed
Closed
[refactor] Semantic Function Clustering Analysis — 7 Unresolved Items (Carry-Forward)#1799
Labels
automated-analysiscode-qualityrefactoring
@github-actions

Description

@github-actions
github-actions
bot
opened on Mar 11, 2026

Overview

Analysis of 75 non-test Go files across 22 packages catalogued ~529 top-level functions and methods. Since the previous run (§22924130376), one commit landed: #1792 (DIFC package refactoring — extracted checkFlowHelper and newEmptyEvaluationResult helpers, resolving ~71 lines of duplication from the separate #1719 patterns). This is a positive structural improvement but does not affect the 7 carry-forward items below.

All 7 findings from the prior analysis remain unresolved. The codebase is generally well-organized; open items are low-risk cleanups: two micro-file merges, one micro-package removal, one outlier function relocation, one file rename, and two thin-file consolidations.

Full Report

Analysis Metadata

  • Total Go Files Analyzed: 75 (72 in internal/, 3 at root: main.go, tools.go, version.go)
  • Total Functions Catalogued: ~529
  • Packages Analyzed: 22 internal packages
  • Outliers Found: 3 (items 3, 5 below)
  • Micro-files / Micro-packages Found: 5 (items 1, 2, 6, 7)
  • Misleading Filenames: 1 (item 4)
  • Detection Method: Naming pattern analysis + function signature review + cross-package usage analysis
  • Analysis Date: 2026-03-11
  • Previous Issue: [refactor] Semantic Function Clustering Analysis — Findings Carry Forward (7 Unresolved Items) #1748 (closed — all findings remain unresolved)
  • Commits Since Last Analysis: 1 (#1792 — DIFC refactor extracting checkFlowHelper/newEmptyEvaluationResult, no structural impact on items below)

Identified Issues

1. 🔴 internal/tty/ — Two Single-Function Micro-files

Files: internal/tty/tty.go (15 lines) and internal/tty/container.go (35 lines)

Both live in package tty and both detect environment/terminal characteristics. There is no reason for separate files.

File Function Lines
tty.go IsStderrTerminal() bool 15
container.go IsRunningInContainer() bool 35

Recommendation: Merge container.go into tty.go. Total file stays under 55 lines.

Estimated Effort: 10 minutes
Impact: Eliminates one file; both terminal-detection functions are discoverable in one place.

2. 🔴 internal/dockerutil/env.go — Single-Function Micro-Package

File: internal/dockerutil/env.go (41 lines)
Package: dockerutil
Sole Function: ExpandEnvArgs(args []string) []string
Only Caller: internal/mcp/connection.go:128

// internal/dockerutil/env.go — sole function in package
func ExpandEnvArgs(args []string) []string {
    // Converts "-e VAR_NAME" to "-e VAR_NAME=value" by reading process environment
}

Problem: An entire package for a single utility function used in one place adds unnecessary fragmentation.

Options:

  1. Move to internal/config/docker_helpers.go — consolidates all Docker utilities in one place.
  2. Inline as private helper in mcp/connection.go — it has one caller and is ~20 lines of logic.

Recommendation: Option 2 (inline as expandEnvArgs in mcp/connection.go) — single caller, no independent importability needed.

Estimated Effort: 20 minutes
Impact: Removes one package, clarifies ownership.

3. 🟡 internal/auth/header.go:TruncateSessionID — Outlier Utility in Auth Package

File: internal/auth/header.go:172
Function:

func TruncateSessionID(sessionID string) string {
    if sessionID == "" {
        return "(none)"
    }
    if len(sessionID) <= 8 {
        return sessionID
    }
    return sessionID[:8] + "..."
}

Callers: internal/server/sdk_logging.go (5 call sites — all logging calls)

Problem: This is a string utility (truncate for safe logging), not an authentication concern. The repository already has internal/strutil/truncate.go with Truncate() and TruncateWithSuffix().

Recommendation: Move TruncateSessionID to internal/strutil/truncate.go. Update sdk_logging.go to import strutil instead of auth for this function.

Estimated Effort: 20 minutes
Impact: Single source of truth for truncation; auth package stays focused on authentication logic.

4. 🟡 internal/server/transport.go — Misleading Filename

File: internal/server/transport.go (54 lines)
Sole Public Function: CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey string) *http.Server

Problem: The name "transport" implies a low-level transport protocol concern. Compare the asymmetry:

File Function Mode
transport.go CreateHTTPServerForMCP Unified
routed.go CreateHTTPServerForRoutedMode Routed

Both are HTTP server factory functions. The actual MCP transport layer is in internal/mcp/http_transport.go, making server/transport.go doubly confusing.

Recommendation: Rename transport.go → unified_server.go (or http_server_unified.go) to mirror routed.go. The logger variable logTransport would need renaming — use logHTTPServer to avoid conflict with logUnified in unified.go.

Estimated Effort: 15 minutes
Impact: Consistent naming across server factory files; easier discovery.

5. 🟡 internal/cmd/root.go — Config Output Functions Are Outliers

File: internal/cmd/root.go (~587 lines)
Misplaced Functions:

  • writeGatewayConfigToStdout(cfg, listenAddr, mode string) error
  • writeGatewayConfig(cfg, listenAddr, mode string, w io.Writer) error (~90 lines)
  • loadEnvFile(path string) error (~35 lines)

Problem: root.go's responsibility is CLI wiring. writeGatewayConfig is a ~90-line config serialization routine that builds and writes JSON per spec §5.4 — a serialization concern, not a CLI lifecycle concern.

Recommendation: Extract to internal/cmd/config_output.go. This mirrors the existing pattern of split responsibility files in internal/cmd/ (flags_core.go, flags_logging.go, flags_difc.go, flags_launch.go).

Estimated Effort: 30 minutes
Impact: Reduces root.go from ~587 → ~460 lines; config output logic is more discoverable.

6. 🟢 internal/cmd/flags_launch.go — Single-Flag Micro-file

File: internal/cmd/flags_launch.go (21 lines)
Content: One constant (defaultSequentialLaunch), one variable (sequentialLaunch), and an init() block.

Context: The flags_*.go pattern is intentional for feature-specific flag grouping. However, flags_launch.go has only a single flag compared to flags_core.go (47 lines), flags_logging.go (55 lines), and flags_difc.go (236 lines).

Recommendation: Merge into internal/cmd/flags.go (99 lines → ~120 lines). Alternatively, keep if launch-specific flags are expected to grow.

Estimated Effort: 10 minutes
Impact: Minor — reduces file count by one.

7. 🟢 internal/config/config_payload.go — Thin Constants File

File: internal/config/config_payload.go (30 lines)
Content: 3 exported constants (DefaultPayloadDir, DefaultLogDir, DefaultPayloadSizeThreshold) + 1 init() function (10 lines) registering a DefaultsSetter.

Recommendation: Merge constants and init() into internal/config/config_core.go (307 lines → ~337 lines, still manageable). The constants define operational defaults, not a distinct feature concern.

Estimated Effort: 15 minutes
Impact: Reduces file count; groups operational defaults with core config definitions.

Well-Organized Patterns (No Changes Needed)

  • internal/logger/: 13 files with clear separation (file logger, slog adapter, sanitize, RPC formatters). global_helpers.go correctly centralizes init/close.
  • internal/difc/: DIFC labels/evaluator/resource split is semantically clear. #1792 further improved it by extracting checkFlowHelper and newEmptyEvaluationResult. ✓
  • internal/config/: The config_*.go feature-per-file naming pattern is consistently applied.
  • internal/mcp/: connection.go + http_transport.go split correctly isolates HTTP-specific transport complexity.
  • internal/cmd/flags_*.go: The getDefault*() helper pattern is intentional (documented in flags.go).
  • internal/launcher/log_helpers.go: 126-line logging helper file is tightly coupled to launcher state — appropriate placement.

Summary of Recommendations

Priority Finding File(s) Effort
🔴 High Merge tty/ micro-files container.go → tty.go 10 min
🔴 High Remove single-function dockerutil package dockerutil/env.go → inline in mcp/connection.go 20 min
🟡 Medium Move TruncateSessionID to strutil auth/header.go:172 → strutil/truncate.go 20 min
🟡 Medium Rename transport.go server/transport.go → unified_server.go 15 min
🟡 Medium Extract config output from root.go root.go → cmd/config_output.go 30 min
🟢 Low Merge flags_launch.go into flags.go cmd/flags_launch.go 10 min
🟢 Low Merge config_payload.go into config_core.go config/config_payload.go 15 min

Total Estimated Effort: ~2 hours

Implementation Checklist

  • Merge internal/tty/container.go into internal/tty/tty.go
  • Move dockerutil.ExpandEnvArgs inline to internal/mcp/connection.go; delete internal/dockerutil/ package
  • Move TruncateSessionID from internal/auth/header.go to internal/strutil/truncate.go; update callers
  • Rename internal/server/transport.go → unified_server.go (rename logger var to logHTTPServer)
  • Extract writeGatewayConfig* from internal/cmd/root.go to internal/cmd/config_output.go
  • Merge internal/cmd/flags_launch.go into internal/cmd/flags.go
  • Merge internal/config/config_payload.go constants into config_core.go
  • Run make agent-finished after each change to verify no regressions

References:

Generated by Semantic Function Refactoring · ◷

Metadata

Metadata

Assignees

No one assigned

    Labels

    automated-analysiscode-qualityrefactoring

    Type

    No type

    Fields

    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