From 4aa8a916296d996009721ebbddb406f0a2970724 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Sun, 14 Jun 2026 17:11:38 +0000
Subject: [PATCH 1/4] Add tests for config.normalizeAgentID, effectiveAgentID,
 and validateToolResponseFilters
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three under-tested functions brought to 100% coverage:

- GatewayConfig.normalizeAgentID (80% → 100%)
  - New: nil receiver no-op path
  - New: conflict-detection path when both agent_id and api_key
    are set to different values (logs deprecation + conflict warning)
  - New: integration via LoadFromFile with TOML configs covering
    all four combinations of the two fields

- GatewayConfig.effectiveAgentID (85.7% → 100%)
  - New: nil receiver path (returns empty string)
  - New: all four resolution branches (explicit, non-explicit
    non-empty AgentID, APIKey fallback, zero-value)

- validateToolResponseFilters (86.7% → 100%)
  - New: $ENV.KEY filter forces the gojq WithEnvironLoader
    closure to execute during compilation
  - New: $undefinedVar filter parses successfully but fails at
    the gojq.Compile step (variable not defined error)

Overall internal/config package coverage: 97.3% → 97.6%
Overall across all testable packages: 98.3% → 98.4%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../config_core_agent_id_coverage_test.go     | 291 ++++++++++++++++++
 .../config/validate_helpers_coverage_test.go  |  25 ++
 2 files changed, 316 insertions(+)
 create mode 100644 internal/config/config_core_agent_id_coverage_test.go

diff --git a/internal/config/config_core_agent_id_coverage_test.go b/internal/config/config_core_agent_id_coverage_test.go
new file mode 100644
index 000000000..ad53a6bc6
--- /dev/null
+++ b/internal/config/config_core_agent_id_coverage_test.go
@@ -0,0 +1,291 @@
+package config
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestEffectiveAgentID tests all branches of GatewayConfig.effectiveAgentID.
+//
+// effectiveAgentID is an unexported helper that returns the resolved agent ID:
+//   - nil receiver → ""
+//   - agentIDExplicit set → return AgentID (even if empty)
+//   - AgentID non-empty (and not explicit) → return AgentID
+//   - fallback → return APIKey
+func TestEffectiveAgentID(t *testing.T) {
+	tests := []struct {
+		name    string
+		gateway *GatewayConfig
+		want    string
+	}{
+		{
+			name:    "nil receiver returns empty string",
+			gateway: nil,
+			want:    "",
+		},
+		{
+			name: "explicit agent_id returns AgentID",
+			gateway: &GatewayConfig{
+				AgentID:         "explicit-agent",
+				agentIDExplicit: true,
+			},
+			want: "explicit-agent",
+		},
+		{
+			name: "explicit agent_id even when empty returns empty AgentID",
+			gateway: &GatewayConfig{
+				AgentID:         "",
+				APIKey:          "legacy-key",
+				agentIDExplicit: true,
+			},
+			want: "",
+		},
+		{
+			name: "non-explicit non-empty AgentID returns AgentID",
+			gateway: &GatewayConfig{
+				AgentID:         "derived-agent",
+				agentIDExplicit: false,
+			},
+			want: "derived-agent",
+		},
+		{
+			name: "non-explicit empty AgentID falls back to APIKey",
+			gateway: &GatewayConfig{
+				AgentID:         "",
+				APIKey:          "fallback-key",
+				agentIDExplicit: false,
+			},
+			want: "fallback-key",
+		},
+		{
+			name:    "zero-value GatewayConfig returns empty string",
+			gateway: &GatewayConfig{},
+			want:    "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.gateway.effectiveAgentID()
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
+// TestNormalizeAgentID tests all branches of GatewayConfig.normalizeAgentID.
+//
+// normalizeAgentID resolves the precedence between the preferred agent_id field
+// and the deprecated api_key alias. It is called after TOML/JSON parsing with
+// boolean flags indicating which fields were explicitly set.
+func TestNormalizeAgentID(t *testing.T) {
+	tests := []struct {
+		name                string
+		gateway             *GatewayConfig
+		agentIDDefined      bool
+		legacyAPIKeyDefined bool
+		source              string
+		wantAgentID         string
+		wantAPIKey          string
+		wantExplicit        bool
+	}{
+		{
+			name:                "nil receiver is a no-op",
+			gateway:             nil,
+			agentIDDefined:      false,
+			legacyAPIKeyDefined: false,
+			source:              "test",
+			// All zero values — just verifying no panic.
+		},
+		{
+			name:                "neither field defined leaves zero values",
+			gateway:             &GatewayConfig{},
+			agentIDDefined:      false,
+			legacyAPIKeyDefined: false,
+			source:              "test",
+			wantAgentID:         "",
+			wantAPIKey:          "",
+			wantExplicit:        false,
+		},
+		{
+			name: "only agent_id defined marks explicit and sets APIKey alias",
+			gateway: &GatewayConfig{
+				AgentID: "my-agent",
+			},
+			agentIDDefined:      true,
+			legacyAPIKeyDefined: false,
+			source:              "TOML",
+			wantAgentID:         "my-agent",
+			wantAPIKey:          "my-agent",
+			wantExplicit:        true,
+		},
+		{
+			name: "only api_key defined copies api_key to AgentID",
+			gateway: &GatewayConfig{
+				APIKey: "legacy-key",
+			},
+			agentIDDefined:      false,
+			legacyAPIKeyDefined: true,
+			source:              "TOML",
+			wantAgentID:         "legacy-key",
+			wantAPIKey:          "legacy-key",
+			wantExplicit:        false,
+		},
+		{
+			name: "both defined with same value uses agent_id, marks explicit",
+			gateway: &GatewayConfig{
+				AgentID: "shared-id",
+				APIKey:  "shared-id",
+			},
+			agentIDDefined:      true,
+			legacyAPIKeyDefined: true,
+			source:              "stdin JSON",
+			wantAgentID:         "shared-id",
+			wantAPIKey:          "shared-id",
+			wantExplicit:        true,
+		},
+		{
+			name: "both defined with different values: agent_id wins, conflict logged",
+			gateway: &GatewayConfig{
+				AgentID: "new-agent-id",
+				APIKey:  "old-api-key",
+			},
+			agentIDDefined:      true,
+			legacyAPIKeyDefined: true,
+			source:              "TOML",
+			wantAgentID:         "new-agent-id",
+			wantAPIKey:          "new-agent-id",
+			wantExplicit:        true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			// nil receiver case: just confirm no panic.
+			if tt.gateway == nil {
+				assert.NotPanics(t, func() {
+					tt.gateway.normalizeAgentID(tt.agentIDDefined, tt.legacyAPIKeyDefined, tt.source)
+				})
+				return
+			}
+
+			tt.gateway.normalizeAgentID(tt.agentIDDefined, tt.legacyAPIKeyDefined, tt.source)
+
+			assert.Equal(t, tt.wantAgentID, tt.gateway.AgentID, "AgentID mismatch")
+			assert.Equal(t, tt.wantAPIKey, tt.gateway.APIKey, "APIKey mismatch")
+			assert.Equal(t, tt.wantExplicit, tt.gateway.agentIDExplicit, "agentIDExplicit mismatch")
+		})
+	}
+}
+
+// TestLoadFromFile_BothAgentIDAndAPIKeySet_DifferentValues verifies that when
+// a TOML config defines both [gateway].agent_id and [gateway].api_key with
+// different values, LoadFromFile loads successfully, uses agent_id as the
+// authoritative identifier, and marks it explicit.
+//
+// This exercises the conflict-detection branch in normalizeAgentID (the
+// "using gateway.agent_id" deprecation warning) that is not reached by any
+// other test, since all other tests set at most one of the two fields.
+func TestLoadFromFile_BothAgentIDAndAPIKeySet_DifferentValues(t *testing.T) {
+	path := writeTempTOML(t, `
+[gateway]
+agent_id = "preferred-agent-id"
+api_key  = "old-legacy-key"
+
+[servers.github]
+command = "docker"
+args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"]
+`)
+
+	cfg, err := LoadFromFile(path)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+	require.NotNil(t, cfg.Gateway)
+
+	// agent_id always wins over api_key when both are present.
+	assert.Equal(t, "preferred-agent-id", cfg.Gateway.AgentID, "agent_id should be used as AgentID")
+	assert.Equal(t, "preferred-agent-id", cfg.Gateway.APIKey, "APIKey alias should mirror AgentID")
+	assert.True(t, cfg.Gateway.agentIDExplicit, "agentIDExplicit should be true when agent_id is present")
+}
+
+// TestLoadFromFile_BothAgentIDAndAPIKeySet_SameValue verifies that when a TOML
+// config sets agent_id and api_key to the same value, LoadFromFile succeeds and
+// the gateway uses that value as the authoritative identifier.
+func TestLoadFromFile_BothAgentIDAndAPIKeySet_SameValue(t *testing.T) {
+	path := writeTempTOML(t, `
+[gateway]
+agent_id = "shared-key"
+api_key  = "shared-key"
+
+[servers.github]
+command = "docker"
+args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"]
+`)
+
+	cfg, err := LoadFromFile(path)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+	require.NotNil(t, cfg.Gateway)
+
+	assert.Equal(t, "shared-key", cfg.Gateway.AgentID)
+	assert.Equal(t, "shared-key", cfg.Gateway.APIKey)
+	assert.True(t, cfg.Gateway.agentIDExplicit)
+}
+
+// TestLoadFromFile_OnlyAgentIDSet verifies that when only [gateway].agent_id
+// is set, the gateway uses it as the identifier and marks it explicit.
+func TestLoadFromFile_OnlyAgentIDSet(t *testing.T) {
+	path := writeTempTOML(t, `
+[gateway]
+agent_id = "only-agent-id"
+
+[servers.github]
+command = "docker"
+args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"]
+`)
+
+	cfg, err := LoadFromFile(path)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+	require.NotNil(t, cfg.Gateway)
+
+	assert.Equal(t, "only-agent-id", cfg.Gateway.AgentID)
+	assert.Equal(t, "only-agent-id", cfg.Gateway.APIKey, "APIKey alias should mirror AgentID")
+	assert.True(t, cfg.Gateway.agentIDExplicit)
+}
+
+// TestLoadFromFile_OnlyAPIKeySet verifies that when only the deprecated
+// [gateway].api_key is set, its value is promoted to AgentID.
+func TestLoadFromFile_OnlyAPIKeySet(t *testing.T) {
+	path := writeTempTOML(t, `
+[gateway]
+api_key = "only-api-key"
+
+[servers.github]
+command = "docker"
+args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"]
+`)
+
+	cfg, err := LoadFromFile(path)
+	require.NoError(t, err)
+	require.NotNil(t, cfg)
+	require.NotNil(t, cfg.Gateway)
+
+	assert.Equal(t, "only-api-key", cfg.Gateway.AgentID, "api_key should be promoted to AgentID")
+	assert.Equal(t, "only-api-key", cfg.Gateway.APIKey)
+	assert.False(t, cfg.Gateway.agentIDExplicit, "agentIDExplicit should be false when only api_key is set")
+}
+
+// TestGetAgentID_ExplicitAgentID verifies that GetAgentID returns the
+// explicitly configured agent_id even when APIKey is also set.
+func TestGetAgentID_ExplicitAgentID(t *testing.T) {
+	cfg := &Config{
+		Gateway: &GatewayConfig{
+			AgentID:         "my-explicit-agent",
+			APIKey:          "my-legacy-key",
+			agentIDExplicit: true,
+		},
+	}
+	assert.Equal(t, "my-explicit-agent", cfg.GetAgentID())
+}
diff --git a/internal/config/validate_helpers_coverage_test.go b/internal/config/validate_helpers_coverage_test.go
index a09cb24ec..f05daa922 100644
--- a/internal/config/validate_helpers_coverage_test.go
+++ b/internal/config/validate_helpers_coverage_test.go
@@ -102,6 +102,31 @@ func TestValidateToolResponseFilters_DirectCall(t *testing.T) {
 			wantErr:   true,
 			errSubstr: "custom.path contains an empty tool name",
 		},
+		{
+			// $ENV.KEY is valid jq syntax and compiles successfully, but calling it
+			// forces gojq to invoke the WithEnvironLoader closure during compilation.
+			// This covers the `return nil` inside the env-loader function literal at
+			// validation.go:136 (the only statement not reached by other test cases).
+			name: "filter using $ENV calls the env loader during compilation",
+			filters: map[string]string{
+				"my_tool": "$ENV.SOME_VARIABLE",
+			},
+			jsonPath: "mcpServers.myserver.tool_response_filters",
+			wantErr:  false,
+		},
+		{
+			// $undefinedVar | . parses successfully (valid jq syntax) but fails at the
+			// gojq.Compile step because the variable $undefinedVar is never bound.
+			// This covers the `return fmt.Errorf` inside the Compile error-check at
+			// validation.go:138 — the only remaining uncovered statement.
+			name: "filter with undefined variable fails at compile step not parse step",
+			filters: map[string]string{
+				"my_tool": "$undefinedVar | .field",
+			},
+			jsonPath:  "mcpServers.myserver.tool_response_filters",
+			wantErr:   true,
+			errSubstr: "my_tool contains an invalid jq expression",
+		},
 	}
 
 	for _, tt := range tests {

From 0c54f0f3d36951270324ebeccdbbec7d8ac6785e Mon Sep 17 00:00:00 2001
From: Landon Cox <landon.cox@microsoft.com>
Date: Sun, 14 Jun 2026 10:19:28 -0700
Subject: [PATCH 2/4] Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 internal/config/validate_helpers_coverage_test.go | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/internal/config/validate_helpers_coverage_test.go b/internal/config/validate_helpers_coverage_test.go
index f05daa922..fe80d52c7 100644
--- a/internal/config/validate_helpers_coverage_test.go
+++ b/internal/config/validate_helpers_coverage_test.go
@@ -103,10 +103,9 @@ func TestValidateToolResponseFilters_DirectCall(t *testing.T) {
 			errSubstr: "custom.path contains an empty tool name",
 		},
 		{
-			// $ENV.KEY is valid jq syntax and compiles successfully, but calling it
-			// forces gojq to invoke the WithEnvironLoader closure during compilation.
-			// This covers the `return nil` inside the env-loader function literal at
-			// validation.go:136 (the only statement not reached by other test cases).
+			// $ENV.KEY is valid jq syntax and compiles successfully, but it triggers the
+			// gojq environ-loader option during compilation. This ensures the
+			// validateToolResponseFilters gojq.WithEnvironLoader closure is executed.
 			name: "filter using $ENV calls the env loader during compilation",
 			filters: map[string]string{
 				"my_tool": "$ENV.SOME_VARIABLE",

From 991f9c01c3e9fc836f78878561f7e74e85707b60 Mon Sep 17 00:00:00 2001
From: Landon Cox <landon.cox@microsoft.com>
Date: Sun, 14 Jun 2026 10:19:37 -0700
Subject: [PATCH 3/4] Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 internal/config/validate_helpers_coverage_test.go | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/internal/config/validate_helpers_coverage_test.go b/internal/config/validate_helpers_coverage_test.go
index fe80d52c7..f8f20334e 100644
--- a/internal/config/validate_helpers_coverage_test.go
+++ b/internal/config/validate_helpers_coverage_test.go
@@ -114,10 +114,9 @@ func TestValidateToolResponseFilters_DirectCall(t *testing.T) {
 			wantErr:  false,
 		},
 		{
-			// $undefinedVar | . parses successfully (valid jq syntax) but fails at the
-			// gojq.Compile step because the variable $undefinedVar is never bound.
-			// This covers the `return fmt.Errorf` inside the Compile error-check at
-			// validation.go:138 — the only remaining uncovered statement.
+			// $undefinedVar | .field parses successfully but fails at gojq.Compile because
+			// the variable is never bound. This ensures the compile-error path in
+			// validateToolResponseFilters is exercised (distinct from parse errors).
 			name: "filter with undefined variable fails at compile step not parse step",
 			filters: map[string]string{
 				"my_tool": "$undefinedVar | .field",

From a1e0fb3ae87a4ab772fec91519b167e6e21161e3 Mon Sep 17 00:00:00 2001
From: Landon Cox <landon.cox@microsoft.com>
Date: Sun, 14 Jun 2026 10:19:45 -0700
Subject: [PATCH 4/4] Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
---
 internal/config/config_core_agent_id_coverage_test.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/internal/config/config_core_agent_id_coverage_test.go b/internal/config/config_core_agent_id_coverage_test.go
index ad53a6bc6..cce9d344f 100644
--- a/internal/config/config_core_agent_id_coverage_test.go
+++ b/internal/config/config_core_agent_id_coverage_test.go
@@ -46,6 +46,7 @@ func TestEffectiveAgentID(t *testing.T) {
 			name: "non-explicit non-empty AgentID returns AgentID",
 			gateway: &GatewayConfig{
 				AgentID:         "derived-agent",
+				APIKey:          "legacy-key",
 				agentIDExplicit: false,
 			},
 			want: "derived-agent",
