Skip to content

refactor: semantic function clustering — collaborator helpers, SessionSuffix, middleware split, AgentTagsSnapshot#7633

Merged
lpcox merged 4 commits into
mainfrom
copilot/refactor-semantic-function-clustering
Jun 16, 2026
Merged

refactor: semantic function clustering — collaborator helpers, SessionSuffix, middleware split, AgentTagsSnapshot#7633
lpcox merged 4 commits into
mainfrom
copilot/refactor-semantic-function-clustering

Conversation

Copilot AI commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Automated semantic analysis identified six misplaced functions and one over-stuffed file. Addresses all four priorities.

Priority 1 — Remove server → proxy dependency

ParseCollaboratorPermissionArgs, FetchCollaboratorPermission, and LogAndWrapCollaboratorPermission were in proxy/ despite serving both proxy and server. This forced server/unified.go to import proxy, requiring an injected-logPrintf workaround to avoid circularity.

  • Moved to internal/httputil/collaborator.go (existing shared HTTP utilities package)
  • Deleted proxy/collaborator_permission.go
  • Updated callers in proxy/proxy.go and server/unified.go; removed proxy import from server/unified.go
  • Moved unit tests to internal/httputil/collaborator_test.go

Priority 2a — SessionSuffixstrutil

logger.SessionSuffix is a pure string formatter with no logger state, called only from launcher/log_helpers.go.

  • Moved to internal/strutil/session_suffix.go
  • Updated launcher/log_helpers.go import; moved test to strutil/session_suffix_test.go

Priority 2b — Split server/middleware.go (316 lines, 5 concerns)

New file Contents
middleware_auth.go applyIfConfigured, authMiddleware, rejectAuthRequest, applyAuthIfConfigured
sdk_logging.go WithSDKLogging, withResponseLogging
middleware.go (~120 lines residual) Config types, WithOTELTracing, wrapWithMiddleware, buildMCPHandler

Priority 3 — Co-locate AgentTagsSnapshot with its context key

AgentTagsSnapshot and GetAgentTagsSnapshotFromContext lived in connection_logging.go while AgentTagsSnapshotContextKey was in connection.go. Moved the type and accessor to connection.go.

GitHub Advanced Security started work on behalf of lpcox June 16, 2026 17:04 View session
GitHub Advanced Security finished work on behalf of lpcox June 16, 2026 17:05
…pers, SessionSuffix, middleware split, AgentTagsSnapshot
Copilot AI changed the title [WIP] Refactor semantic function clustering based on analysis refactor: semantic function clustering — collaborator helpers, SessionSuffix, middleware split, AgentTagsSnapshot Jun 16, 2026
GitHub Advanced Security started work on behalf of lpcox June 16, 2026 17:30 View session
Copilot finished work on behalf of lpcox June 16, 2026 17:30
Copilot AI requested a review from lpcox June 16, 2026 17:30
GitHub Advanced Security finished work on behalf of lpcox June 16, 2026 17:31
@lpcox lpcox marked this pull request as ready for review June 16, 2026 17:31
Copilot AI review requested due to automatic review settings June 16, 2026 17:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors several cross-cutting helpers to better align with package responsibilities, reduce unnecessary dependencies (notably server → proxy), and split an overgrown middleware file into focused units.

Changes:

  • Moved collaborator-permission REST helpers from proxy into shared internal/httputil, updating server/proxy call sites and relocating tests.
  • Moved SessionSuffix from logger to strutil and updated launcher call sites and tests.
  • Split internal/server/middleware.go into smaller files and co-located AgentTagsSnapshot with its context key in internal/mcp/connection.go.
Show a summary per file
File Description
internal/strutil/session_suffix.go Adds strutil.SessionSuffix utility.
internal/strutil/session_suffix_test.go Moves SessionSuffix tests into strutil package.
internal/logger/common.go Removes logger.SessionSuffix after relocation.
internal/launcher/log_helpers.go Updates launcher logging to use strutil.SessionSuffix.
internal/httputil/collaborator.go Moves collaborator permission helpers into shared HTTP utilities.
internal/httputil/collaborator_test.go Relocates collaborator helper unit tests into httputil.
internal/proxy/proxy.go Switches proxy collaborator calls to httputil helpers.
internal/server/unified.go Removes proxy dependency by using httputil collaborator helpers.
internal/server/middleware.go Leaves core middleware wiring/config; removes auth + SDK logging implementations.
internal/server/middleware_auth.go New file containing API-key auth middleware and helpers.
internal/server/sdk_logging.go New file containing SDK HTTP boundary logging helpers.
internal/mcp/connection.go Moves AgentTagsSnapshot type/accessor next to context key.
internal/mcp/connection_logging.go Removes relocated AgentTagsSnapshot code from logging file.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 13/13 changed files
  • Comments generated: 2

Comment thread internal/server/sdk_logging.go
Comment thread internal/server/sdk_logging.go
lpcox and others added 2 commits June 16, 2026 11:17
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
GitHub Advanced Security started work on behalf of lpcox June 16, 2026 18:18 View session
GitHub Advanced Security started work on behalf of lpcox June 16, 2026 18:18 View session
GitHub Advanced Security finished work on behalf of lpcox June 16, 2026 18:19
GitHub Advanced Security finished work on behalf of lpcox June 16, 2026 18:20
@lpcox lpcox merged commit d6374bf into main Jun 16, 2026
27 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering branch June 16, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants