[refactor] Semantic Function Clustering Analysis — Outliers & Refactoring Opportunities

[github-actions]

Overview

Analysis of 122 non-test Go source files across the internal/ packages identified several function placement outliers, a notable env-config pattern inconsistency, and a large-scale structural near-duplication in the logger package. The codebase is generally well-organised, but three actionable issues were found.

Summary:

• 122 Go source files (excl. test files) · ~779 functions/methods catalogued
• 1 clear outlier function (misplaced config env reader)
• 1 structural near-duplication pattern in logger/ (4 logger types with identical setup scaffolding)
• 1 minor cmd complexity concern (root.go embeds config-writing helpers)

---

Issue 1 — getSessionTimeout() is a config-env reader living in the wrong package

File: internal/server/routed.go:127
Function:

func getSessionTimeout() time.Duration {
    return envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour)
}

Problem: This function reads the MCP_GATEWAY_SESSION_TIMEOUT environment variable and belongs to the same family as the GetGateway*FromEnv() functions in internal/config/config_env.go:

// config/config_env.go
func GetGatewayPortFromEnv() (int, error) { ... }
func GetGatewayDomainFromEnv() string     { ... }
func GetGatewayAPIKeyFromEnv() string     { ... }
func GetGatewayToolTimeoutFromEnv() (int, bool, error) { ... }

The function comment even acknowledges it is "shared by both routed mode and unified (transport) mode", which implies it belongs at a higher abstraction layer. Its placement in server/routed.go means:

• Consumers in internal/server/transport.go must import from a sibling file instead of the config package
• Adding tests for env-var wiring requires importing the server package instead of config
• The MCP_GATEWAY_SESSION_TIMEOUT env var is documented at the config level but implemented in the server layer

Recommendation: Move getSessionTimeout() to internal/config/config_env.go as a public GetGatewaySessionTimeoutFromEnv() function, consistent with the existing pattern.

Estimated effort: ~30 min · Low risk

---

Issue 2 — Logger package: four logger types share identical setup scaffolding without full abstraction

The logger package contains four concrete logger types (FileLogger, MarkdownLogger, JSONLLogger, ToolsLogger), each implemented in its own file. All four follow an identical five-function scaffolding pattern:

Pattern	FileLogger	MarkdownLogger	JSONLLogger	ToolsLogger
setup*Logger(file, logDir, fileName)	✅	✅	✅	✅
handle*LoggerError(err, logDir, fileName)	✅	✅	✅	✅
Init*Logger(logDir, fileName)	✅	✅	✅	✅
(l *XLogger) Close() error	✅	✅	✅	✅
Close*Logger() error (global close)	✅	✅	✅	✅

common.go provides generic infrastructure (initLogger[T], closeLogFile, initLogFile, etc.) but the four concrete Init*Logger / handle*LoggerError functions still repeat the same ~15-line call-through pattern:

// file_logger.go
func InitFileLogger(logDir, fileName string) error {
    return initLogger(logDir, fileName, 0, &globalFileLoggerMu, &globalFileLogger,
        setupFileLogger, handleFileLoggerError)
}

// markdown_logger.go  (identical structure, different types)
func InitMarkdownLogger(logDir, fileName string) error {
    return initLogger(logDir, fileName, 0, &globalMarkdownLoggerMu, &globalMarkdownLogger,
        setupMarkdownLogger, handleMarkdownLoggerError)
}
// ... same for jsonl_logger.go, tools_logger.go

The handle*LoggerError functions are also nearly identical, differing only in whether they fall back to stdout or a noop logger:

// file_logger.go — falls back to stdout
func handleFileLoggerError(err error, logDir, fileName string) (*FileLogger, error) {
    logFallbackWarnings(err, ...)
    return setupFileLogger(os.Stdout, logDir, fileName)
}

// jsonl_logger.go — returns a noop logger
func handleJSONLLoggerError(err error, _ string, _ string) (*JSONLLogger, error) {
    return &JSONLLogger{}, nil
}

Problem: The generic infrastructure in common.go was clearly designed to eliminate this duplication (the loggerSetupFunc[T] and loggerErrorHandlerFunc[T] type aliases and the initLogger generic function are already there), but the four concrete loggers still each carry scaffolding that could be eliminated or further collapsed.

Recommendation: Evaluate whether the handle*LoggerError functions add enough distinct value to justify their existence, or whether the fallback behaviour can be encoded as a parameter to the existing initLogger generic. At minimum, add a comment in common.go explaining the design rationale so future contributors do not add a fifth logger type with the same scaffolding.

Estimated effort: 2–4 hours · Medium complexity

---

Issue 3 — writeGatewayConfig / resolveGuardPolicyOverride inflate cmd/root.go

File: internal/cmd/root.go

root.go contains two groups of logic that are noticeably heavier than typical command-wiring code:

1. writeGatewayConfig / writeGatewayConfigToStdout (~50 lines): Serialises and emits a JSON representation of the resolved gateway config. This is startup-specific output logic, not CLI wiring.

2. resolveGuardPolicyOverride (~40 lines): Parses CLI flags into a *config.GuardPolicy using the config package's parsing functions. This is policy-resolution logic that could live closer to where other guard-policy parsing happens (internal/config/guard_policy_parse.go).

Both groups are currently called only from within root.go, so the coupling is low. However, as the command grows, root.go risks becoming a catch-all.

Recommendation:

• Extract writeGatewayConfig* into internal/cmd/output.go or internal/cmd/startup_output.go
• Consider moving resolveGuardPolicyOverride to internal/config/guard_policy_parse.go as it is pure config-parsing logic (though it does inspect cobra.Command flag state — a wrapper could pass the resolved flag values instead)

Estimated effort: 1–2 hours · Low risk

---

Analysis Metadata

Item	Value
Total Go files analysed	122 (excl. test files)
Total functions/methods catalogued	~779
Packages examined	19 (auth, cmd, config, difc, envutil, guard, httputil, launcher, logger, mcp, middleware, oidc, proxy, server, strutil, syncutil, sys, tracing, tty, version)
Outliers found	1 (getSessionTimeout in wrong package)
Near-duplicate patterns	1 (logger setup scaffolding)
Cmd complexity concerns	1 (root.go growth)
Detection method	Naming-pattern analysis + manual cross-file inspection

References:

• §25372004791

Generated by Semantic Function Refactoring · ● 1.1M · ◷

[github-actions]

🤖 This is an automated response from Repo Assist.

Thanks for the detailed analysis! All three issues identified here are actionable. A few notes on each:

Issue 1 — getSessionTimeout() placement: Agreed — this function belongs in internal/config/config_env.go alongside its siblings. This was also flagged in #5071. Moving it (and renaming to GetGatewaySessionTimeoutFromEnv()) is low-risk since the function is unexported and called only from server/routed.go and server/transport.go. The main benefit is testability: consumers of the config package can then test session-timeout wiring without importing the server package.

Issue 2 — Logger scaffolding duplication: The common.go generic infrastructure is doing most of the heavy lifting already. The handle*LoggerError functions do have meaningfully different fallback behaviour (stdout vs. noop), so full consolidation would need a callback parameter. At minimum, documenting the design intent in common.go as suggested would prevent future copy-paste. Full collapse is medium complexity but the value is real — worth picking up as a future improvement.

Issue 3 — root.go growth: The writeGatewayConfig* extraction to cmd/output.go is the easiest starting point. resolveGuardPolicyOverride has a cobra dependency that complicates the move, but passing the resolved flag values (rather than *cobra.Command) to a pure config function would cleanly separate concerns.

I'll track Issue 1 for a future fix run.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• proxy.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Repo Assist · ● 3.2M · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@851905c06e905bf362a9f6cc54f912e3df747d55

[github-actions]

Closing this issue as a new semantic function refactoring analysis is being performed.