[refactor] Semantic Function Clustering Analysis — Refactoring Opportunities

[github-actions]

Overview

Analysis of 114 non-test Go source files across internal/ (765 total functions) surfaced three concrete refactoring opportunities with meaningful impact on code organisation and discoverability. No duplicate function implementations were found — the existing split into focused packages (e.g. strutil, httputil, envutil) is working well. The issues below are about misplaced functions and thin wrappers, not logic duplication.

---

Finding 1 — Session helpers living in server/http_helpers.go instead of server/session.go

File: internal/server/http_helpers.go (371 lines)
Related file: internal/server/session.go (112 lines)

http_helpers.go has a clear mixed-concern problem. Four functions deal exclusively with session lifecycle but live in the HTTP-helpers file rather than the dedicated session file:

Function	What it does
extractAndValidateSession	Extracts & validates session ID from the Authorization header
injectSessionContext	Stores session ID / backend ID into the request context
setupSessionCallback	Combines the two above + logs new connection
logHTTPRequestBody	Logs the request body tagged with session/backend IDs

The remaining eight functions in http_helpers.go are genuine HTTP/middleware concerns (writeErrorResponse, rejectRequest, withResponseLogging, WithOTELTracing, wrapWithMiddleware, WithSDKLogging, logRuntimeError, peekRequestBody) and belong where they are.

Recommendation: Move the four session-related functions to internal/server/session.go.
Estimated effort: ~30 min (mechanical move + verify no circular imports)
Impact: session.go becomes the single place to look for all session lifecycle logic; http_helpers.go shrinks from 371 → ~260 lines.

---

Finding 2 — buildCircuitBreakers, buildAllowedToolSets, and hasWildcard in server/unified.go instead of their dedicated files

File: internal/server/unified.go (855 lines)

Three constructor/helper functions live in unified.go that logically belong in the files named after the feature they initialise:

Function	Current location	Natural home
buildCircuitBreakers	unified.go:332	circuit_breaker.go
buildAllowedToolSets	unified.go:365	tool_registry.go
hasWildcard	unified.go:389	tool_registry.go

unified.go is already the largest file in the server package (855 lines). The dedicated files exist precisely to absorb feature-specific logic:

• circuit_breaker.go (325 lines) — owns all circuit-breaker types and behaviour
• tool_registry.go (450 lines) — owns all tool registration and filtering

Recommendation: Move buildCircuitBreakers to circuit_breaker.go and buildAllowedToolSets + hasWildcard to tool_registry.go.
Estimated effort: ~30 min (mechanical move + verify compile)
Impact: unified.go shrinks by ~60 lines; feature-specific construction logic lives next to feature-specific types.

---

Finding 3 — writeErrorResponse in server/http_helpers.go is a one-line passthrough wrapper

File: internal/server/http_helpers.go:61

func writeErrorResponse(w http.ResponseWriter, statusCode int, code, message string) {
    httputil.WriteErrorResponse(w, statusCode, code, message)
}

This private function adds zero logic — it is a rename of httputil.WriteErrorResponse. All callers within server/ could call httputil.WriteErrorResponse directly (the package is already imported wherever this is used).

Recommendation: Delete writeErrorResponse and replace the ~5 call sites with httputil.WriteErrorResponse directly.
Estimated effort: ~15 min
Impact: Removes one layer of indirection; makes the dependency on httputil explicit at each call site.

---

Bonus observation — cmd/tracing_helpers.go and cmd/flags_tracing.go could be one file

These two files total only 75 lines and cover a single feature (tracing in the CLI layer):

• tracing_helpers.go — 52 lines (registerTracingFlags, initTracingProviderWithFallback, shutdownTracingProviderWithTimeout)
• flags_tracing.go — 23 lines (init() registering the flags)

Recommendation: Merge into a single cmd/tracing.go.
Estimated effort: ~10 min
Impact: Eliminates a file; all tracing CLI wiring lives in one place.

---

Analysis Metadata

Metric	Value
Go source files analysed	114 (non-test, internal/)
Total functions cataloged	765
Exact/near duplicates found	0
Outlier functions found	7
Thin wrapper functions	1
Candidate file merges	1

Implementation Checklist

• Move extractAndValidateSession, injectSessionContext, setupSessionCallback, logHTTPRequestBody from server/http_helpers.go → server/session.go
• Move buildCircuitBreakers from server/unified.go → server/circuit_breaker.go
• Move buildAllowedToolSets + hasWildcard from server/unified.go → server/tool_registry.go
• Remove writeErrorResponse wrapper; replace call sites with httputil.WriteErrorResponse
• Merge cmd/tracing_helpers.go + cmd/flags_tracing.go → cmd/tracing.go
• Run make agent-finished after each change to verify no regressions

References: §25211549765

Generated by Semantic Function Refactoring · ● 1.1M · ◷

[github-actions]

🤖 This is an automated response from Repo Assist.

I verified all four findings against the current codebase — they're accurate:

Finding 1 (session helpers in http_helpers.go): Confirmed. extractAndValidateSession, injectSessionContext, setupSessionCallback, and logHTTPRequestBody are all in http_helpers.go:84–200, not in session.go.

Finding 2 (buildCircuitBreakers, buildAllowedToolSets, hasWildcard in unified.go): Confirmed. All three are at unified.go:333–400. Both circuit_breaker.go and tool_registry.go exist as natural homes.

Finding 3 (writeErrorResponse thin wrapper): Confirmed. http_helpers.go:51–53 is literally a one-liner httputil.WriteErrorResponse passthrough. About 5 call sites across the server package.

Bonus (cmd/tracing_helpers.go + cmd/flags_tracing.go): Confirmed — 52 + 23 lines for a single feature.

All four are low-risk mechanical moves. The writeErrorResponse removal might touch the most files but is the most clearly beneficial. Happy to implement any of these if maintainers want a PR — Finding 3 is ~15 minutes of effort with high clarity.

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

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.