refactor: move outlier functions to semantically correct files#6152
Conversation
…and server helpers - Move ConnectionErrorContext + LogConnectionError from mcp/errors.go → mcp/connection_logging.go, delete errors.go - Extract validateOpenTelemetryConfig + regex patterns from config/config_tracing.go → new config/validation_tracing.go - Move LogAndWrapCollaboratorPermission from mcp/tool_result.go → mcp/collaborator_permission.go - Split server/http_helpers.go: middleware functions → new server/middleware.go Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors code layout by relocating a few “outlier” helpers into files whose names and responsibilities better match their semantics (server middleware composition, MCP connection diagnostics, tracing validation, collaborator permission helpers), without changing behavior.
Changes:
- Extracts MCP HTTP middleware composition + OTEL/SDK logging wrappers into
internal/server/middleware.go, leaving request/response utilities inhttp_helpers.go. - Moves MCP connection failure diagnostic types/helpers into
internal/mcp/connection_logging.goand deletes the now-redundanterrors.go. - Extracts OpenTelemetry validation into
internal/config/validation_tracing.goand co-locates collaborator permission logging with its argument parsing helper.
Show a summary per file
| File | Description |
|---|---|
| internal/server/middleware.go | New file containing MCP handler construction and standard middleware stack (OTEL/auth/HMAC/shutdown/SDK logging). |
| internal/server/http_helpers.go | Removes middleware-related code, keeping only HTTP helper utilities. |
| internal/mcp/tool_result.go | Removes collaborator-permission logging/wrapping helper from tool result utilities. |
| internal/mcp/errors.go | Deletes file after relocating the only remaining connection-diagnostic helpers. |
| internal/mcp/connection_logging.go | Adds connection failure diagnostic context + logging helpers alongside other connection logging. |
| internal/mcp/collaborator_permission.go | Adds LogAndWrapCollaboratorPermission next to collaborator permission arg parsing. |
| internal/config/validation_tracing.go | New validation file for OpenTelemetry config (W3C ID regex + endpoint rules). |
| internal/config/config_tracing.go | Removes embedded validation/regex; keeps tracing config + variable expansion/default registration. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 0
|
@copilot merge main |
Done — merged main into the branch (c108941). There was one conflict in |
Automated semantic clustering of 811 functions across 14 packages identified 3 misplaced functions and 1 over-broad file. This PR moves each to where a developer would naturally look for it.
Changes
mcp/errors.go→mcp/connection_logging.go:ConnectionErrorContext+LogConnectionErrorbelonged with the other connection diagnostic helpers (logReconnectStart,logOutboundRPCRequest, etc.), not in a file conventionally reserved for error type definitions.errors.godeleted.config/config_tracing.go→ newconfig/validation_tracing.go:validateOpenTelemetryConfigand its 4 W3C regex patterns extracted into a dedicated validation file, consistent with the existingvalidation*.goconvention in this package.mcp/tool_result.go→mcp/collaborator_permission.go:LogAndWrapCollaboratorPermissionmoved alongsideParseCollaboratorPermissionArgs— all collaborator permission logic now co-located.server/http_helpers.go→ split intoserver/middleware.go: Middleware composition (wrapWithMiddleware,applyIfConfigured,WithOTELTracing,buildMCPHandler,WithSDKLogging,mcpHandlerConfig) extracted into a dedicated file; request/response utilities remain inhttp_helpers.go.No logic changes — pure file reorganization.