Deduplicate error CallToolResult construction, GitHub token and API URL lookups

internal/cmd/proxy.go

@@ -15,6 +15,7 @@ import (
 
 	"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/proxy"
 	"github.com/github/gh-aw-mcpg/internal/tracing"
@@ -177,13 +178,7 @@ func runProxy(cmd *cobra.Command, args []string) error {
 	// Resolve GitHub token (optional — proxy forwards client auth by default)
 	token := proxyToken
 	if token == "" {
-		token = os.Getenv("GH_TOKEN")
-	}
-	if token == "" {
-		token = os.Getenv("GITHUB_TOKEN")
-	}
-	if token == "" {
-		token = os.Getenv("GITHUB_PERSONAL_ACCESS_TOKEN")
+		token = envutil.LookupGitHubToken()
 	}
 	if token != "" {
 		logger.LogInfo("startup", "Fallback GitHub token configured from flag/env")

internal/envutil/github.go

@@ -0,0 +1,39 @@
+package envutil
+
+import (
+	"os"
+	"strings"
+)
+
+// LookupGitHubToken searches environment variables for a GitHub token using
+// a canonical priority order. It returns the first non-empty (trimmed) value
+// found, or an empty string if none is set.
+//
+// Priority order:
+//  1. GITHUB_MCP_SERVER_TOKEN
+//  2. GITHUB_TOKEN
+//  3. GITHUB_PERSONAL_ACCESS_TOKEN
+//  4. GH_TOKEN
+func LookupGitHubToken() string {
+	for _, key := range []string{
+		"GITHUB_MCP_SERVER_TOKEN",
+		"GITHUB_TOKEN",
+		"GITHUB_PERSONAL_ACCESS_TOKEN",
+		"GH_TOKEN",
+	} {
+		if v := strings.TrimSpace(os.Getenv(key)); v != "" {
+			return v
+		}
+	}
+	return ""
+}
+
+// LookupGitHubAPIURL returns the GitHub API base URL from the GITHUB_API_URL
+// environment variable. If the variable is not set or empty, it returns
+// defaultURL. Any trailing slash is stripped from the result.
+func LookupGitHubAPIURL(defaultURL string) string {
+	if v := os.Getenv("GITHUB_API_URL"); v != "" {
+		return strings.TrimRight(v, "/")
+	}
+	return defaultURL
+}

internal/envutil/github_test.go

@@ -0,0 +1,87 @@
+package envutil
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestLookupGitHubToken(t *testing.T) {
+	tokenVars := []string{
+		"GITHUB_MCP_SERVER_TOKEN",
+		"GITHUB_TOKEN",
+		"GITHUB_PERSONAL_ACCESS_TOKEN",
+		"GH_TOKEN",
+	}
+	clearAll := func(t *testing.T) {
+		t.Helper()
+		for _, k := range tokenVars {
+			t.Setenv(k, "")
+		}
+	}
+
+	t.Run("prefers GITHUB_MCP_SERVER_TOKEN", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GITHUB_MCP_SERVER_TOKEN", "mcp-token")
+		t.Setenv("GITHUB_TOKEN", "gh-token")
+		t.Setenv("GH_TOKEN", "gh-cli-token")
+		assert.Equal(t, "mcp-token", LookupGitHubToken())
+	})
+
+	t.Run("falls back to GITHUB_TOKEN", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GITHUB_TOKEN", "gh-token")
+		t.Setenv("GH_TOKEN", "gh-cli-token")
+		assert.Equal(t, "gh-token", LookupGitHubToken())
+	})
+
+	t.Run("falls back to GITHUB_PERSONAL_ACCESS_TOKEN", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GITHUB_PERSONAL_ACCESS_TOKEN", "pat-token")
+		t.Setenv("GH_TOKEN", "gh-cli-token")
+		assert.Equal(t, "pat-token", LookupGitHubToken())
+	})
+
+	t.Run("falls back to GH_TOKEN", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GH_TOKEN", "gh-cli-token")
+		assert.Equal(t, "gh-cli-token", LookupGitHubToken())
+	})
+
+	t.Run("returns empty when no token set", func(t *testing.T) {
+		clearAll(t)
+		assert.Equal(t, "", LookupGitHubToken())
+	})
+
+	t.Run("trims whitespace", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GITHUB_TOKEN", "  trimmed  ")
+		assert.Equal(t, "trimmed", LookupGitHubToken())
+	})
+
+	t.Run("skips whitespace-only values", func(t *testing.T) {
+		clearAll(t)
+		t.Setenv("GITHUB_MCP_SERVER_TOKEN", "   ")
+		t.Setenv("GITHUB_TOKEN", "actual-token")
+		assert.Equal(t, "actual-token", LookupGitHubToken())
+	})
+}
+
+func TestLookupGitHubAPIURL(t *testing.T) {
+	const defaultURL = "https://api.github.com"
+
+	t.Run("returns default when env not set", func(t *testing.T) {
+		t.Setenv("GITHUB_API_URL", "")
+		assert.Equal(t, defaultURL, LookupGitHubAPIURL(defaultURL))
+	})
+
+	t.Run("returns custom URL from env", func(t *testing.T) {
+		t.Setenv("GITHUB_API_URL", "https://github.example.com/api/v3")
+		assert.Equal(t, "https://github.example.com/api/v3", LookupGitHubAPIURL(defaultURL))
+	})
+
+	t.Run("strips trailing slash", func(t *testing.T) {
+		t.Setenv("GITHUB_API_URL", "https://github.example.com/api/v3/")
+		assert.Equal(t, "https://github.example.com/api/v3", LookupGitHubAPIURL(defaultURL))
+	})
+}

internal/server/unified.go

@@ -7,8 +7,6 @@ import (
 	"io"
 	"log"
 	"net/http"
-	"os"
-	"strings"
 	"sync"
 	"time"
 
@@ -18,6 +16,7 @@ import (
 
 	"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/guard"
 	"github.com/github/gh-aw-mcpg/internal/launcher"
 	"github.com/github/gh-aw-mcpg/internal/logger"
@@ -343,26 +342,13 @@ func (g *guardBackendCaller) callCollaboratorPermission(ctx context.Context, arg
 // lookupEnrichmentToken searches environment variables for a GitHub token
 // suitable for enrichment API calls.
 func lookupEnrichmentToken() string {
-	for _, key := range []string{
-		"GITHUB_MCP_SERVER_TOKEN",
-		"GITHUB_TOKEN",
-		"GITHUB_PERSONAL_ACCESS_TOKEN",
-		"GH_TOKEN",
-	} {
-		if v := strings.TrimSpace(os.Getenv(key)); v != "" {
-			return v
-		}
-	}
-	return ""
+	return envutil.LookupGitHubToken()
 }
 
 // lookupGitHubAPIBaseURL returns the GitHub API base URL from environment
 // or defaults to https://api.github.com.
 func lookupGitHubAPIBaseURL() string {
-	if v := os.Getenv("GITHUB_API_URL"); v != "" {
-		return strings.TrimRight(v, "/")
-	}
-	return "https://api.github.com"
+	return envutil.LookupGitHubAPIURL("https://api.github.com")
 }
 
 // newErrorCallToolResult creates a standard error CallToolResult with the error message
@@ -462,12 +448,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
 		deniedErr := fmt.Errorf("tool %q is not in the allowed-tools list for this server", toolName)
 		toolSpan.RecordError(deniedErr)
 		toolSpan.SetStatus(codes.Error, "tool not allowed")
-		return &sdk.CallToolResult{
-			IsError: true,
-			Content: []sdk.Content{
-				&sdk.TextContent{Text: deniedErr.Error()},
-			},
-		}, nil, deniedErr
+		return newErrorCallToolResult(deniedErr)
 	}
 
 	// Create backend caller for the guard
@@ -534,14 +515,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
 			toolSpan.RecordError(detailedErr)
 			toolSpan.SetStatus(codes.Error, "access denied: "+result.Reason)
 			httpStatusCode = 403
-			return &sdk.CallToolResult{
-				Content: []sdk.Content{
-					&sdk.TextContent{
-						Text: detailedErr.Error(),
-					},
-				},
-				IsError: true,
-			}, nil, detailedErr
+			return newErrorCallToolResult(detailedErr)
 		}
 	} else {
 		log.Printf("[DIFC] Access ALLOWED for agent %s to %s", agentID, resource.Description)
@@ -602,14 +576,7 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
 				blockErr := fmt.Errorf("DIFC policy violation: %d of %d items in response are not accessible to agent %s",
 					filtered.GetFilteredCount(), filtered.TotalCount, agentID)
 				httpStatusCode = 403
-				return &sdk.CallToolResult{
-					Content: []sdk.Content{
-						&sdk.TextContent{
-							Text: blockErr.Error(),
-						},
-					},
-					IsError: true,
-				}, nil, blockErr
+				return newErrorCallToolResult(blockErr)
 			}
 
 			if filtered.GetFilteredCount() > 0 {