Skip to content

refactor: semantic function clustering — absorb dockerutil, reduce duplication, move outlier free functions#1933

Merged
lpcox merged 2 commits intomainfrom
copilot/refactor-semantic-function-clustering-again
Mar 15, 2026
Merged

refactor: semantic function clustering — absorb dockerutil, reduce duplication, move outlier free functions#1933
lpcox merged 2 commits intomainfrom
copilot/refactor-semantic-function-clustering-again

Conversation

Copy link
Contributor

Copilot AI commented Mar 15, 2026

Summary

Addresses the actionable improvements identified in issue #1827 (semantic function clustering analysis).

Changes

1. Absorb internal/dockerutil into internal/mcp

The internal/dockerutil package contained a single function (ExpandEnvArgs) used exclusively by internal/mcp/connection.go. Moving it eliminates the single-function package:

  • Created internal/mcp/dockerenv.go with ExpandEnvArgs
  • Updated connection.go to use the local function (no more import of dockerutil)
  • Consolidated TestExpandDockerEnvArgs in connection_test.go with the 4 additional test cases from the old dockerutil/env_test.go
  • Deleted internal/dockerutil/

2. Reduce duplication in auth.TruncateSessionID

TruncateSessionID in internal/auth/header.go duplicated the truncation logic already in internal/strutil/truncate.go. It now delegates to strutil.Truncate(sessionID, 8), keeping only the "(none)" special-case for empty strings.

3. Move toDIFCTags/tagsToStrings from unified.go to internal/difc

These free functions only deal with difc.Tag types and have no dependency on server logic. Exported as difc.StringsToTags / difc.TagsToStrings in a new internal/difc/tags.go file with unit tests.

4. Move convertToCallToolResult/parseToolArguments from unified.go to internal/mcp

These MCP SDK utility functions are generic enough to belong in the mcp package alongside other protocol helpers. Exported as mcp.ConvertToCallToolResult / mcp.ParseToolArguments in internal/mcp/tool_result.go, using logger.New("mcp:tool_result") for debug logging per project conventions.

Testing

  • All existing unit and integration tests pass (make agent-finished)
  • New unit tests added for difc.StringsToTags / difc.TagsToStrings
  • CodeQL: no alerts

Closes #1827

Copilot AI and others added 2 commits March 14, 2026 23:55
…plication, move outlier free functions

- Absorb internal/dockerutil (single-function package) into internal/mcp
  - Moved ExpandEnvArgs to internal/mcp/dockerenv.go
  - Updated connection.go and connection_test.go; consolidated test cases
  - Deleted internal/dockerutil directory

- Fix auth.TruncateSessionID duplication with strutil.Truncate
  - TruncateSessionID delegates to strutil.Truncate(sessionID, 8)

- Move toDIFCTags/tagsToStrings from unified.go to internal/difc
  - New internal/difc/tags.go with exported StringsToTags/TagsToStrings
  - Added unit tests in internal/difc/tags_test.go

- Move convertToCallToolResult/parseToolArguments from unified.go to internal/mcp
  - New internal/mcp/tool_result.go with ConvertToCallToolResult/ParseToolArguments
  - Updated unified.go and its test to use mcp.* versions
  - Uses logger.New('mcp:tool_result') for debug logging per project conventions

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI requested a review from lpcox March 15, 2026 00:08
@lpcox lpcox marked this pull request as ready for review March 15, 2026 00:10
Copilot AI review requested due to automatic review settings March 15, 2026 00:10
@lpcox lpcox merged commit 4f0bcd3 into main Mar 15, 2026
11 checks passed
@lpcox lpcox deleted the copilot/refactor-semantic-function-clustering-again branch March 15, 2026 00:10
Copy link
Contributor

Copilot AI left a comment

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 and “clusters” semantically related helper functions into the packages where they logically belong, removing a single-function package and reducing duplication across the codebase.

Changes:

  • Moved Docker -e VAR environment expansion logic into internal/mcp and removed internal/dockerutil.
  • Deduplicated auth.TruncateSessionID by delegating to strutil.Truncate.
  • Moved DIFC tag conversion helpers out of UnifiedServer into internal/difc with new unit tests.
  • Moved MCP tool result/argument parsing helpers out of UnifiedServer into internal/mcp, updating call sites and tests.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
internal/server/unified.go Switched to new mcp/difc helper functions; removed local free functions.
internal/server/call_tool_result_test.go Updated tests to call mcp.ConvertToCallToolResult.
internal/mcp/tool_result.go New home for ConvertToCallToolResult and ParseToolArguments with debug logging.
internal/mcp/dockerenv.go Absorbed Docker env expansion helper into mcp with updated logger namespace.
internal/mcp/connection.go Uses local ExpandEnvArgs instead of dockerutil.
internal/mcp/connection_test.go Consolidated/expanded Docker env arg tests; removed dockerutil import.
internal/difc/tags.go New exported DIFC tag/string conversion helpers.
internal/difc/tags_test.go Unit tests for StringsToTags / TagsToStrings.
internal/auth/header.go TruncateSessionID now delegates to strutil.Truncate.
internal/dockerutil/env_test.go Removed as part of deleting the internal/dockerutil package.

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

You can also share your feedback on Copilot code review. Take the survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants