[refactor] Semantic Function Clustering Analysis: Refactoring Opportunities

[github-actions]

Overview

Static analysis of 117 non-test Go source files containing 756 functions/methods across the internal/ directory. The codebase is generally well-organized with clear single-purpose files. Four concrete refactoring opportunities were identified, ranging from near-duplicate helpers to a structural pattern that could benefit from generics.

---

Function Inventory

Package	Files	Primary Purpose
internal/logger	17	Multi-target logging (file, JSONL, markdown, tools, RPC)
internal/server	17	HTTP server, routing, session, auth, DIFC enforcement
internal/difc	9	Decentralized Information Flow Control labels/evaluation
internal/config	12	TOML/JSON config parsing and validation
internal/cmd	11	CLI commands and flags (Cobra)
internal/mcp	10	MCP protocol connection and types
internal/guard	7	Security guard interface, WASM, noop, write-sink
internal/proxy	7	GitHub API filtering proxy with DIFC
internal/launcher	4	Backend process management, health monitoring
internal/testutil/mcptest	4	Test utilities
Other packages	19	auth, envutil, httputil, middleware, oidc, strutil, syncutil, sys, tracing, tty, version

---

Identified Issues

1. marshalAndSanitizeForLog in Wrong Package

Severity: Low-medium
File: internal/server/tool_registry.go (line 352)

// server/tool_registry.go
func marshalAndSanitizeForLog(value interface{}) string {
    resultJSON, _ := json.Marshal(value)
    return sanitize.SanitizeString(string(resultJSON))
}

Issue: This unexported helper combines json.Marshal + sanitize.SanitizeString. A related function already lives in the logger package:

// logger/marshal_debug.go
func LogMarshaledForDebug(value interface{}, onMarshalSuccess func(string), onMarshalFailure func(error)) {
    resultJSON, err := json.Marshal(value)
    // ...only marshals, does NOT sanitize
}

marshalAndSanitizeForLog is used in two places in tool_registry.go for operational logging, but the marshal+sanitize pattern could be promoted to a public function in internal/logger or internal/logger/sanitize so other packages don't re-implement it.

Recommendation: Add MarshalAndSanitize(value interface{}) string to internal/logger/sanitize/sanitize.go and use it from both tool_registry.go and any future callers.
Estimated effort: ~30 min
Files affected: internal/server/tool_registry.go, internal/logger/sanitize/sanitize.go

---

2. generateRandomID() Duplicates strutil.RandomHex Pattern Without Centralizing Fallback

Severity: Low
File: internal/middleware/jqschema.go (line 116)

// middleware/jqschema.go
func generateRandomID() string {
    id, err := strutil.RandomHex(16)
    if err != nil {
        // Fallback to a process- and time-based ID if random generation fails.
        return fmt.Sprintf("fallback-%d-%d", os.Getpid(), time.Now().UnixNano())
    }
    return id
}

strutil.RandomHex and auth.GenerateRandomAPIKey both call crypto/rand but propagate the error. The middleware wrapper swallows the error with a deterministic fallback — a pattern not available in strutil.

Recommendation: Add RandomHexWithFallback(n int) string to internal/strutil/random_hex.go that encapsulates the fallback logic. The middleware can then remove its local wrapper:

// strutil/random_hex.go (proposed)
func RandomHexWithFallback(n int) string {
    s, err := RandomHex(n)
    if err != nil {
        return fmt.Sprintf("fallback-%d-%d", os.Getpid(), time.Now().UnixNano())
    }
    return s
}

This makes the fallback pattern reusable and removes the middleware's dependency on os and time just for ID generation.
Estimated effort: ~20 min
Files affected: internal/strutil/random_hex.go, internal/middleware/jqschema.go

---

3. makeLevelLogger / makeServerLevelLogger Are Near-Identical Structural Patterns

Severity: Low (internal package detail)
File: internal/logger/common.go (lines 232–248)

func makeLevelLogger(
    dispatch func(level LogLevel, category, format string, args ...interface{}),
    level LogLevel,
) func(category, format string, args ...interface{}) {
    return func(category, format string, args ...interface{}) {
        dispatch(level, category, format, args...)
    }
}

func makeServerLevelLogger(
    dispatch func(serverID string, level LogLevel, category, format string, args ...interface{}),
    level LogLevel,
) func(serverID, category, format string, args ...interface{}) {
    return func(serverID, category, format string, args ...interface{}) {
        dispatch(serverID, level, category, format, args...)
    }
}

These two functions are structurally identical closures that fix the level parameter — the only difference is the presence of a leading serverID string argument. With Go 1.18+ generics already used elsewhere in the codebase (e.g., syncutil/cache.go, logger/global_helpers.go), this pattern is a candidate for a generic level-binder.

Recommendation: This is low priority because both functions are unexported and only used internally. Consider addressing during a dedicated logger refactor pass.
Estimated effort: ~1 hour (with full test coverage verification)

---

4. Logger Setup Pattern Repeated Across Four Files

Severity: Low (well-structured but verbose)
Files: internal/logger/file_logger.go, jsonl_logger.go, markdown_logger.go, tools_logger.go

Each logger follows an identical 3-function scaffold:

• setup<X>Logger(file *os.File, logDir, fileName string) (*XLogger, error)
• handle<X>LoggerError(err error, ...) (*XLogger, error)
• Init<X>Logger(logDir, fileName string) error

The generic initLogger[T closableLogger] in common.go already partially unifies this. However, setup* and handle* still contain per-type boilerplate that varies only in the returned concrete type.

Note: This pattern is intentional and correct for separate log targets. The initLogger generic already handles the shared lifecycle. This is more of an observation than a blocker.

Recommendation: Consider defining a LoggerFactory[T closableLogger] interface or using function-value composition to further reduce boilerplate in future logger additions.
Estimated effort: ~2–3 hours
Files affected: All files in internal/logger/

---

Summary of Recommendations

Priority	Issue	Effort	Impact
1	Promote marshalAndSanitizeForLog to sanitize package	~30 min	Prevents future re-implementation in other packages
2	Add RandomHexWithFallback to strutil	~20 min	Centralizes fallback pattern
3	Unify makeLevelLogger/makeServerLevelLogger with generics	~1 hr	Reduces internal boilerplate
4	Reduce logger-setup boilerplate via factory pattern	~2–3 hr	Simplifies adding new log targets

Implementation Checklist

• Add sanitize.MarshalAndSanitize(value interface{}) string to internal/logger/sanitize/sanitize.go
• Update internal/server/tool_registry.go to use the new function
• Add strutil.RandomHexWithFallback(n int) string to internal/strutil/random_hex.go
• Update internal/middleware/jqschema.go to use strutil.RandomHexWithFallback
• Evaluate generic consolidation of makeLevelLogger/makeServerLevelLogger
• Evaluate logger factory pattern for new logger additions

Analysis Metadata

Metric	Value
Total Go files analyzed	117
Total functions/methods cataloged	756
Packages examined	22
Outliers (functions in wrong files)	1 (marshalAndSanitizeForLog)
Near-duplicates detected	2 (generateRandomID wrapping RandomHex; makeLevelLogger/makeServerLevelLogger)
Structural patterns flagged	1 (logger setup boilerplate)
Detection method	Static grep analysis + naming pattern clustering
Analysis date	2026-04-28

References: §25048716374

Generated by Semantic Function Refactoring · ● 1.6M · ◷

[github-actions]

🤖 This is an automated response from Repo Assist.

I verified all four recommendations against the current codebase. Findings:

Priority 1: marshalAndSanitizeForLog → sanitize.MarshalAndSanitize ✅ Accurate. The function in internal/server/tool_registry.go (line 352) is unexported and used in two places for logging sanitization. internal/logger/sanitize/sanitize.go already exports SanitizeString and SanitizeJSON but not the marshal+sanitize combination. Adding MarshalAndSanitize(value interface{}) string there is a natural fit. Effort is genuinely ~30 min.

Priority 2: RandomHexWithFallback in strutil ✅ Accurate. I've implemented this as a separate PR today — it adds strutil.RandomHexWithFallback(n int) string with 4 unit tests (including the fallback path triggered by negative n), and simplifies generateRandomID in jqschema.go to a one-liner.

Priority 3: Generic makeLevelLogger/makeServerLevelLogger ✅ Accurate. Both are unexported closures in internal/logger/common.go that differ only by a leading serverID string arg. The pattern is a genuine generics candidate. Agreed this is low priority given both functions are unexported and already have tests.

Priority 4: Logger setup boilerplate ✅ Accurate as an observation. The initLogger[T closableLogger] generic in common.go already unifies most of the lifecycle. Further factoring would mainly help when adding a new log target.

For Priority 1, I can create a follow-up PR extracting marshalAndSanitizeForLog to sanitize.MarshalAndSanitize if that would be useful.

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 · ● 7.5M · ◷

To install this agentic workflow, run

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