[Repo Assist] refactor(envutil): move ExpandEnvArgs from config to envutil

internal/config/docker_helpers.go

@@ -129,33 +129,3 @@ func checkLogDirMounted(containerID, logDir string) bool {
 	logDockerHelpers.Printf("Log dir mount check: containerID=%s, logDir=%s, mounted=%v", containerID, logDir, mounted)
 	return mounted
 }
-
-// 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 {
-	logDockerHelpers.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]
-
-		// Check if this is a -e flag
-		if arg == "-e" && i+1 < len(args) {
-			nextArg := args[i+1]
-			// If next arg doesn't contain '=', it's a variable reference
-			if len(nextArg) > 0 && !strings.Contains(nextArg, "=") {
-				// Look up the variable in the environment
-				if value, exists := os.LookupEnv(nextArg); exists {
-					logDockerHelpers.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
-				}
-				logDockerHelpers.Printf("Env var not found in process environment: name=%s", nextArg)
-			}
-		}
-		result = append(result, arg)
-	}
-	logDockerHelpers.Printf("Env args expansion complete: output_count=%d", len(result))
-	return result
-}

internal/config/docker_helpers_and_env_test.go

@@ -119,150 +119,6 @@ func TestValidateContainerID_SecurityCritical(t *testing.T) {
 	}
 }
 
-// TestExpandEnvArgs tests ExpandEnvArgs with various -e flag combinations.
-func TestExpandEnvArgs(t *testing.T) {
-	tests := []struct {
-		name    string
-		args    []string
-		envVars map[string]string
-		want    []string
-	}{
-		{
-			name: "nil input returns empty slice",
-			args: nil,
-			want: []string{},
-		},
-		{
-			name: "empty input returns empty slice",
-			args: []string{},
-			want: []string{},
-		},
-		{
-			name: "args without -e flag pass through unchanged",
-			args: []string{"run", "--rm", "-i", "ghcr.io/org/image:latest"},
-			want: []string{"run", "--rm", "-i", "ghcr.io/org/image:latest"},
-		},
-		{
-			name:    "-e VAR_NAME is expanded when env var is set",
-			args:    []string{"-e", "MY_TOKEN"},
-			envVars: map[string]string{"MY_TOKEN": "secret-value"},
-			want:    []string{"-e", "MY_TOKEN=secret-value"},
-		},
-		{
-			name: "-e VAR_NAME is left unchanged when env var is not set",
-			args: []string{"-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
-			want: []string{"-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
-		},
-		{
-			name:    "-e VAR=VALUE (already has =) is passed through unchanged",
-			args:    []string{"-e", "MY_VAR=already-set"},
-			envVars: map[string]string{"MY_VAR": "other-value"},
-			want:    []string{"-e", "MY_VAR=already-set"},
-		},
-		{
-			name: "-e at end of args (no following value) is passed through",
-			args: []string{"run", "-e"},
-			want: []string{"run", "-e"},
-		},
-		{
-			name:    "multiple -e flags are all expanded",
-			args:    []string{"-e", "TOKEN_A", "-e", "TOKEN_B"},
-			envVars: map[string]string{"TOKEN_A": "val-a", "TOKEN_B": "val-b"},
-			want:    []string{"-e", "TOKEN_A=val-a", "-e", "TOKEN_B=val-b"},
-		},
-		{
-			name:    "mix of set and unset env vars in -e flags",
-			args:    []string{"-e", "SET_VAR", "-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
-			envVars: map[string]string{"SET_VAR": "value"},
-			want:    []string{"-e", "SET_VAR=value", "-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
-		},
-		{
-			name:    "realistic docker run command with env var expansion",
-			args:    []string{"run", "--rm", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN", "-i", "ghcr.io/github/github-mcp-server:latest"},
-			envVars: map[string]string{"GITHUB_PERSONAL_ACCESS_TOKEN": "ghp_abc123"},
-			want:    []string{"run", "--rm", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_abc123", "-i", "ghcr.io/github/github-mcp-server:latest"},
-		},
-		{
-			name: "-e VAR where VAR is empty string is passed through unchanged",
-			args: []string{"-e", ""},
-			want: []string{"-e", ""},
-		},
-		{
-			name:    "env var with empty value is expanded with empty value",
-			args:    []string{"-e", "EMPTY_VAR"},
-			envVars: map[string]string{"EMPTY_VAR": ""},
-			want:    []string{"-e", "EMPTY_VAR="},
-		},
-		{
-			name:    "env var value containing = is expanded correctly",
-			args:    []string{"-e", "URL_VAR"},
-			envVars: map[string]string{"URL_VAR": "https://example.com?key=val"},
-			want:    []string{"-e", "URL_VAR=https://example.com?key=val"},
-		},
-		{
-			name:    "non -e flags between expanded flags are preserved",
-			args:    []string{"-e", "VAR_A", "--name", "mycontainer", "-e", "VAR_B"},
-			envVars: map[string]string{"VAR_A": "alpha", "VAR_B": "beta"},
-			want:    []string{"-e", "VAR_A=alpha", "--name", "mycontainer", "-e", "VAR_B=beta"},
-		},
-		{
-			name:    "same var referenced multiple times is expanded each time",
-			args:    []string{"-e", "SHARED_VAR", "-e", "SHARED_VAR"},
-			envVars: map[string]string{"SHARED_VAR": "shared"},
-			want:    []string{"-e", "SHARED_VAR=shared", "-e", "SHARED_VAR=shared"},
-		},
-		{
-			name: "-e immediately followed by another -e flag where first -e var is unset",
-			args: []string{"-e", "-e"},
-			// The first "-e" tries to expand the second "-e" as a var name.
-			// Since no env var named "-e" is set, the expansion is skipped.
-			// Both "-e" args are emitted as-is.
-			want: []string{"-e", "-e"},
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			for k, v := range tt.envVars {
-				t.Setenv(k, v)
-			}
-
-			got := ExpandEnvArgs(tt.args)
-
-			require.NotNil(t, got, "ExpandEnvArgs should never return nil")
-			assert.Equal(t, tt.want, got)
-		})
-	}
-}
-
-// TestExpandEnvArgs_DoesNotMutateInput verifies that the original args slice
-// is not modified by ExpandEnvArgs.
-func TestExpandEnvArgs_DoesNotMutateInput(t *testing.T) {
-	t.Setenv("MY_SECRET", "secret-value")
-
-	original := []string{"-e", "MY_SECRET", "--rm"}
-	// Make a copy to compare against after the call
-	copyOfOriginal := make([]string, len(original))
-	copy(copyOfOriginal, original)
-
-	ExpandEnvArgs(original)
-
-	assert.Equal(t, copyOfOriginal, original, "ExpandEnvArgs must not mutate the input slice")
-}
-
-// TestExpandEnvArgs_OutputIsIndependentOfInput verifies that modifications to
-// the returned slice do not affect the original input.
-func TestExpandEnvArgs_OutputIsIndependentOfInput(t *testing.T) {
-	t.Setenv("SOME_VAR", "value")
-
-	args := []string{"run", "-e", "SOME_VAR"}
-	result := ExpandEnvArgs(args)
-
-	// Modifying the result should not affect the original
-	result[0] = "MODIFIED"
-	assert.Equal(t, "run", args[0], "Modifying result should not affect original slice")
-}
-
 // TestGetGatewayPortFromEnv tests the env-based gateway port parsing.
 func TestGetGatewayPortFromEnv_Comprehensive(t *testing.T) {
 	tests := []struct {

internal/envutil/expand_env_args.go

@@ -0,0 +1,42 @@
+package envutil
+
+import (
+	"fmt"
+	"os"
+	"strings"
+
+	"github.com/github/gh-aw-mcpg/internal/logger"
+)
+
+var logExpand = logger.New("envutil:expand")
+
+// ExpandEnvArgs expands Docker -e flags that reference environment variables.
+// Converts "-e VAR_NAME" to "-e VAR_NAME=value" by reading from the process environment.
+// If the variable is not set, the flag is passed through unchanged.
+func ExpandEnvArgs(args []string) []string {
+	logExpand.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]
+
+		// Check if this is a -e flag
+		if arg == "-e" && i+1 < len(args) {
+			nextArg := args[i+1]
+			// If next arg doesn't contain '=', it's a variable reference
+			if len(nextArg) > 0 && !strings.Contains(nextArg, "=") {
+				// Look up the variable in the environment
+				if value, exists := os.LookupEnv(nextArg); exists {
+					logExpand.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
+				}
+				logExpand.Printf("Env var not found in process environment: name=%s", nextArg)
+			}
+		}
+		result = append(result, arg)
+	}
+	logExpand.Printf("Env args expansion complete: output_count=%d", len(result))
+	return result
+}

internal/envutil/expand_env_args_test.go

@@ -0,0 +1,152 @@
+package envutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestExpandEnvArgs tests ExpandEnvArgs with various -e flag combinations.
+func TestExpandEnvArgs(t *testing.T) {
+	tests := []struct {
+		name    string
+		args    []string
+		envVars map[string]string
+		want    []string
+	}{
+		{
+			name: "nil input returns empty slice",
+			args: nil,
+			want: []string{},
+		},
+		{
+			name: "empty input returns empty slice",
+			args: []string{},
+			want: []string{},
+		},
+		{
+			name: "args without -e flag pass through unchanged",
+			args: []string{"run", "--rm", "-i", "ghcr.io/org/image:latest"},
+			want: []string{"run", "--rm", "-i", "ghcr.io/org/image:latest"},
+		},
+		{
+			name:    "-e VAR_NAME is expanded when env var is set",
+			args:    []string{"-e", "MY_TOKEN"},
+			envVars: map[string]string{"MY_TOKEN": "secret-value"},
+			want:    []string{"-e", "MY_TOKEN=secret-value"},
+		},
+		{
+			name: "-e VAR_NAME is left unchanged when env var is not set",
+			args: []string{"-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
+			want: []string{"-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
+		},
+		{
+			name:    "-e VAR=VALUE (already has =) is passed through unchanged",
+			args:    []string{"-e", "MY_VAR=already-set"},
+			envVars: map[string]string{"MY_VAR": "other-value"},
+			want:    []string{"-e", "MY_VAR=already-set"},
+		},
+		{
+			name: "-e at end of args (no following value) is passed through",
+			args: []string{"run", "-e"},
+			want: []string{"run", "-e"},
+		},
+		{
+			name:    "multiple -e flags are all expanded",
+			args:    []string{"-e", "TOKEN_A", "-e", "TOKEN_B"},
+			envVars: map[string]string{"TOKEN_A": "val-a", "TOKEN_B": "val-b"},
+			want:    []string{"-e", "TOKEN_A=val-a", "-e", "TOKEN_B=val-b"},
+		},
+		{
+			name:    "mix of set and unset env vars in -e flags",
+			args:    []string{"-e", "SET_VAR", "-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
+			envVars: map[string]string{"SET_VAR": "value"},
+			want:    []string{"-e", "SET_VAR=value", "-e", "_DOCKERENV_TEST_TRULY_UNSET_XYZ999"},
+		},
+		{
+			name:    "realistic docker run command with env var expansion",
+			args:    []string{"run", "--rm", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN", "-i", "ghcr.io/github/github-mcp-server:latest"},
+			envVars: map[string]string{"GITHUB_PERSONAL_ACCESS_TOKEN": "ghp_abc123"},
+			want:    []string{"run", "--rm", "-e", "GITHUB_PERSONAL_ACCESS_TOKEN=ghp_abc123", "-i", "ghcr.io/github/github-mcp-server:latest"},
+		},
+		{
+			name: "-e VAR where VAR is empty string is passed through unchanged",
+			args: []string{"-e", ""},
+			want: []string{"-e", ""},
+		},
+		{
+			name:    "env var with empty value is expanded with empty value",
+			args:    []string{"-e", "EMPTY_VAR"},
+			envVars: map[string]string{"EMPTY_VAR": ""},
+			want:    []string{"-e", "EMPTY_VAR="},
+		},
+		{
+			name:    "env var value containing = is expanded correctly",
+			args:    []string{"-e", "URL_VAR"},
+			envVars: map[string]string{"URL_VAR": "https://example.com?key=val"},
+			want:    []string{"-e", "URL_VAR=https://example.com?key=val"},
+		},
+		{
+			name:    "non -e flags between expanded flags are preserved",
+			args:    []string{"-e", "VAR_A", "--name", "mycontainer", "-e", "VAR_B"},
+			envVars: map[string]string{"VAR_A": "alpha", "VAR_B": "beta"},
+			want:    []string{"-e", "VAR_A=alpha", "--name", "mycontainer", "-e", "VAR_B=beta"},
+		},
+		{
+			name:    "same var referenced multiple times is expanded each time",
+			args:    []string{"-e", "SHARED_VAR", "-e", "SHARED_VAR"},
+			envVars: map[string]string{"SHARED_VAR": "shared"},
+			want:    []string{"-e", "SHARED_VAR=shared", "-e", "SHARED_VAR=shared"},
+		},
+		{
+			name: "-e immediately followed by another -e flag where first -e var is unset",
+			args: []string{"-e", "-e"},
+			// The first "-e" tries to expand the second "-e" as a var name.
+			// Since no env var named "-e" is set, the expansion is skipped.
+			// Both "-e" args are emitted as-is.
+			want: []string{"-e", "-e"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			for k, v := range tt.envVars {
+				t.Setenv(k, v)
+			}
+
+			got := ExpandEnvArgs(tt.args)
+
+			require.NotNil(t, got, "ExpandEnvArgs should never return nil")
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestExpandEnvArgs_DoesNotMutateInput verifies that the original args slice
+// is not modified by ExpandEnvArgs.
+func TestExpandEnvArgs_DoesNotMutateInput(t *testing.T) {
+	t.Setenv("MY_SECRET", "secret-value")
+
+	original := []string{"-e", "MY_SECRET", "--rm"}
+	// Make a copy to compare against after the call
+	copyOfOriginal := make([]string, len(original))
+	copy(copyOfOriginal, original)
+
+	ExpandEnvArgs(original)
+
+	assert.Equal(t, copyOfOriginal, original, "ExpandEnvArgs must not mutate the input slice")
+}
+
+// TestExpandEnvArgs_OutputIsIndependentOfInput verifies that modifications to
+// the returned slice do not affect the original input.
+func TestExpandEnvArgs_OutputIsIndependentOfInput(t *testing.T) {
+	t.Setenv("SOME_VAR", "value")
+
+	args := []string{"run", "-e", "SOME_VAR"}
+	result := ExpandEnvArgs(args)
+
+	// Modifying the result should not affect the original
+	result[0] = "MODIFIED"
+	assert.Equal(t, "run", args[0], "Modifying result should not affect original slice")
+}

internal/mcp/connection.go

@@ -14,8 +14,8 @@ import (
 	"sync"
 	"time"
 
-	"github.com/github/gh-aw-mcpg/internal/config"
 	"github.com/github/gh-aw-mcpg/internal/difc"
+	"github.com/github/gh-aw-mcpg/internal/envutil"
 	"github.com/github/gh-aw-mcpg/internal/logger"
 	"github.com/github/gh-aw-mcpg/internal/logger/sanitize"
 	"github.com/github/gh-aw-mcpg/internal/oidc"
@@ -104,7 +104,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 := config.ExpandEnvArgs(args)
+	expandedArgs := envutil.ExpandEnvArgs(args)
 	logConn.Printf("Expanded args for Docker env: %v", sanitize.SanitizeArgs(expandedArgs))
 
 	// Create command transport