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

## 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

Author: lpcox

internal/auth/header.go

@@ -40,6 +40,7 @@ import (
 
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
+	"github.com/github/gh-aw-mcpg/internal/strutil"
 )
 
 var log = logger.New("auth:header")
@@ -173,8 +174,5 @@ func TruncateSessionID(sessionID string) string {
 	if sessionID == "" {
 		return "(none)"
 	}
-	if len(sessionID) <= 8 {
-		return sessionID
-	}
-	return sessionID[:8] + "..."
+	return strutil.Truncate(sessionID, 8)
 }

internal/difc/tags.go

@@ -0,0 +1,25 @@
+package difc
+
+import "strings"
+
+// StringsToTags converts a slice of strings to a slice of Tags,
+// trimming whitespace and skipping empty values.
+func StringsToTags(values []string) []Tag {
+	tags := make([]Tag, 0, len(values))
+	for _, value := range values {
+		trimmed := strings.TrimSpace(value)
+		if trimmed != "" {
+			tags = append(tags, Tag(trimmed))
+		}
+	}
+	return tags
+}
+
+// TagsToStrings converts a slice of Tags to a slice of strings.
+func TagsToStrings(tags []Tag) []string {
+	values := make([]string, 0, len(tags))
+	for _, tag := range tags {
+		values = append(values, string(tag))
+	}
+	return values
+}

internal/difc/tags_test.go

@@ -0,0 +1,89 @@
+package difc
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestStringsToTags(t *testing.T) {
+	tests := []struct {
+		name     string
+		values   []string
+		expected []Tag
+	}{
+		{
+			name:     "empty slice",
+			values:   []string{},
+			expected: []Tag{},
+		},
+		{
+			name:     "nil slice",
+			values:   nil,
+			expected: []Tag{},
+		},
+		{
+			name:     "single value",
+			values:   []string{"private:owner"},
+			expected: []Tag{"private:owner"},
+		},
+		{
+			name:     "multiple values",
+			values:   []string{"private:owner", "private:owner/repo"},
+			expected: []Tag{"private:owner", "private:owner/repo"},
+		},
+		{
+			name:     "trims whitespace",
+			values:   []string{"  private:owner  ", "\tprivate:repo\t"},
+			expected: []Tag{"private:owner", "private:repo"},
+		},
+		{
+			name:     "skips empty strings",
+			values:   []string{"private:owner", "", "  ", "private:repo"},
+			expected: []Tag{"private:owner", "private:repo"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := StringsToTags(tt.values)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}
+
+func TestTagsToStrings(t *testing.T) {
+	tests := []struct {
+		name     string
+		tags     []Tag
+		expected []string
+	}{
+		{
+			name:     "empty slice",
+			tags:     []Tag{},
+			expected: []string{},
+		},
+		{
+			name:     "nil slice",
+			tags:     nil,
+			expected: []string{},
+		},
+		{
+			name:     "single tag",
+			tags:     []Tag{"private:owner"},
+			expected: []string{"private:owner"},
+		},
+		{
+			name:     "multiple tags",
+			tags:     []Tag{"private:owner", "private:owner/repo"},
+			expected: []string{"private:owner", "private:owner/repo"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := TagsToStrings(tt.tags)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

internal/dockerutil/env_test.go

@@ -15,7 +15,6 @@ import (
 	"sync"
 	"time"
 
-	"github.com/github/gh-aw-mcpg/internal/dockerutil"
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
 	sdk "github.com/modelcontextprotocol/go-sdk/mcp"
@@ -131,7 +130,7 @@ func NewConnection(ctx context.Context, serverID, command string, args []string,
 
 	// Expand Docker -e flags that reference environment variables
 	// Docker's `-e VAR_NAME` expects VAR_NAME to be in the environment
-	expandedArgs := dockerutil.ExpandEnvArgs(args)
+	expandedArgs := ExpandEnvArgs(args)
 	logConn.Printf("Expanded args for Docker env: %v", sanitize.SanitizeArgs(expandedArgs))
 
 	// Create command transport

internal/mcp/connection.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/github/gh-aw-mcpg/internal/dockerutil"
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -202,13 +201,13 @@ func TestExpandDockerEnvArgs(t *testing.T) {
 			expected: []string{"run", "-e", "VAR1=value1", "-e", "VAR2=fixed", "image"},
 		},
 		{
-			name:     "undefined env variable",
+			name:     "undefined env variable leaves arg unchanged",
 			args:     []string{"run", "-e", "UNDEFINED_VAR", "image"},
 			envVars:  map[string]string{},
 			expected: []string{"run", "-e", "UNDEFINED_VAR", "image"},
 		},
 		{
-			name:     "empty env variable value",
+			name:     "empty env variable value expands to key=",
 			args:     []string{"run", "-e", "EMPTY_VAR", "image"},
 			envVars:  map[string]string{"EMPTY_VAR": ""},
 			expected: []string{"run", "-e", "EMPTY_VAR=", "image"},
@@ -219,32 +218,45 @@ func TestExpandDockerEnvArgs(t *testing.T) {
 			envVars:  map[string]string{},
 			expected: []string{"run", "image", "-e"},
 		},
+		{
+			name:     "nil args returns empty slice",
+			args:     nil,
+			envVars:  map[string]string{},
+			expected: []string{},
+		},
+		{
+			name:     "empty args returns empty slice",
+			args:     []string{},
+			envVars:  map[string]string{},
+			expected: []string{},
+		},
+		{
+			name:     "-e followed by empty string arg is not expanded",
+			args:     []string{"run", "-e", "", "image"},
+			envVars:  map[string]string{},
+			expected: []string{"run", "-e", "", "image"},
+		},
+		{
+			name:     "value with equals sign in env var value",
+			args:     []string{"run", "-e", "KEY_WITH_EQUALS", "image"},
+			envVars:  map[string]string{"KEY_WITH_EQUALS": "a=b=c"},
+			expected: []string{"run", "-e", "KEY_WITH_EQUALS=a=b=c", "image"},
+		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			// Set up environment variables for test
 			for k, v := range tt.envVars {
-				os.Setenv(k, v)
+				require.NoError(t, os.Setenv(k, v))
 			}
-			// Clean up after test
 			t.Cleanup(func() {
 				for k := range tt.envVars {
 					os.Unsetenv(k)
 				}
 			})
 
-			result := dockerutil.ExpandEnvArgs(tt.args)
-
-			if len(result) != len(tt.expected) {
-				t.Fatalf("Expected %d args, got %d: %v", len(tt.expected), len(result), result)
-			}
-
-			for i := range result {
-				if result[i] != tt.expected[i] {
-					t.Errorf("Arg %d: expected '%s', got '%s'", i, tt.expected[i], result[i])
-				}
-			}
+			result := ExpandEnvArgs(tt.args)
+			assert.Equal(t, tt.expected, result)
 		})
 	}
 }

internal/mcp/connection_test.go

@@ -1,4 +1,4 @@
-package dockerutil
+package mcp
 
 import (
 	"fmt"
@@ -8,12 +8,12 @@ import (
 	"github.com/github/gh-aw-mcpg/internal/logger"
 )
 
-var logDockerutil = logger.New("dockerutil:env")
+var logDockerEnv = logger.New("mcp:dockerenv")
 
-// ExpandEnvArgs expands Docker -e flags that reference environment variables
-// Converts "-e VAR_NAME" to "-e VAR_NAME=value" by reading from the process environment
+// ExpandEnvArgs expands Docker -e flags that reference environment variables.
+// Converts "-e VAR_NAME" to "-e VAR_NAME=value" by reading from the process environment.
 func ExpandEnvArgs(args []string) []string {
-	logDockerutil.Printf("Expanding env args: input_count=%d", len(args))
+	logDockerEnv.Printf("Expanding env args: input_count=%d", len(args))
 	result := make([]string, 0, len(args))
 	for i := 0; i < len(args); i++ {
 		arg := args[i]
@@ -25,17 +25,17 @@ func ExpandEnvArgs(args []string) []string {
 			if len(nextArg) > 0 && !strings.Contains(nextArg, "=") {
 				// Look up the variable in the environment
 				if value, exists := os.LookupEnv(nextArg); exists {
-					logDockerutil.Printf("Expanding env var: name=%s", nextArg)
+					logDockerEnv.Printf("Expanding env var: name=%s", nextArg)
 					result = append(result, "-e")
 					result = append(result, fmt.Sprintf("%s=%s", nextArg, value))
 					i++ // Skip the next arg since we processed it
 					continue
 				}
-				logDockerutil.Printf("Env var not found in process environment: name=%s", nextArg)
+				logDockerEnv.Printf("Env var not found in process environment: name=%s", nextArg)
 			}
 		}
 		result = append(result, arg)
 	}
-	logDockerutil.Printf("Env args expansion complete: output_count=%d", len(result))
+	logDockerEnv.Printf("Env args expansion complete: output_count=%d", len(result))
 	return result
 }