Deduplicate error CallToolResult construction, GitHub token and API URL lookups (#3507)

Three duplication patterns across `internal/server/unified.go`,
`internal/cmd/proxy.go`, and `internal/proxy/proxy.go` — an unused
helper, divergent token lookup order, and duplicated API URL resolution.

### Inline error CallToolResult → use existing helper

3 inline `&sdk.CallToolResult{IsError: true, ...}` constructions in
`unified.go` now call the existing `newErrorCallToolResult()` helper
that was being bypassed:

```go
// before (repeated 3x with varying field order)
return &sdk.CallToolResult{
    Content: []sdk.Content{&sdk.TextContent{Text: err.Error()}},
    IsError: true,
}, nil, err

// after
return newErrorCallToolResult(deniedErr)
```

### GitHub token/API URL helpers → `internal/envutil`

- **`envutil.LookupGitHubToken()`** — canonical priority:
`GITHUB_MCP_SERVER_TOKEN` → `GITHUB_TOKEN` →
`GITHUB_PERSONAL_ACCESS_TOKEN` → `GH_TOKEN`. Fixes `cmd/proxy.go` which
previously checked `GH_TOKEN` first (inconsistent with `unified.go`).
- **`envutil.LookupGitHubAPIURL(defaultURL)`** — shared `GITHUB_API_URL`
lookup with trailing-slash trim.
- `lookupEnrichmentToken()` and `lookupGitHubAPIBaseURL()` in
`unified.go` now delegate to these.
- `DeriveGitHubAPIURL()` in `internal/proxy/proxy.go` now delegates the
`GITHUB_API_URL` check to `envutil.LookupGitHubAPIURL`, keeping the
`GITHUB_SERVER_URL` derivation as-is.

Author: lpcox

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 := strings.TrimSpace(os.Getenv("GITHUB_API_URL")); v != "" {
+		return strings.TrimRight(v, "/")
+	}
+	return strings.TrimRight(strings.TrimSpace(defaultURL), "/")
+}

internal/envutil/github_test.go

@@ -0,0 +1,97 @@
+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))
+	})
+
+	t.Run("returns default for whitespace-only env", func(t *testing.T) {
+		t.Setenv("GITHUB_API_URL", "   ")
+		assert.Equal(t, defaultURL, LookupGitHubAPIURL(defaultURL))
+	})
+
+	t.Run("trims whitespace before stripping 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/proxy/proxy.go

@@ -19,6 +19,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/logger"
 	"github.com/github/gh-aw-mcpg/internal/tracing"
@@ -43,7 +44,7 @@ const (
 //     - https://github.com → https://api.github.com
 //  3. Returns empty string if no env vars are set (caller uses DefaultGitHubAPIBase)
 func DeriveGitHubAPIURL() string {
-	if apiURL := os.Getenv("GITHUB_API_URL"); apiURL != "" {
+	if apiURL := envutil.LookupGitHubAPIURL(""); apiURL != "" {
 		logProxy.Printf("GitHub API URL from GITHUB_API_URL: %s", apiURL)
 		return apiURL
 	}

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 {