Add GitHubMCPMode string enum and replace githubTool any with map[string]any

docs/adr/38945-type-github-mcp-mode-enum-and-narrow-tool-config.md

@@ -0,0 +1,40 @@
+# ADR-38945: Type GitHub MCP Mode as an Enum and Narrow Tool Config to `map[string]any`
+
+**Date**: 2026-06-12
+**Status**: Draft
+
+## Context
+
+The GitHub MCP configuration code in `pkg/workflow/mcp_github_config.go` had grown around two weakly-typed conventions. First, roughly 19 helper functions accepted the GitHub tool config as `githubTool any` and each immediately re-asserted it to `map[string]any` internally, scattering the same `.(map[string]any)` type assertion (and its failure handling) across nearly every function and its callers. Second, the MCP "mode" of the GitHub server (`local`, `remote`, `gh-proxy`, `cli`) was modelled as a bare `string`, so the compiler relied on string literals like `== "remote"` and `!= "gh-proxy"` with no compile-time protection against typos or invalid values. Both patterns made the code harder to read and gave the type system no leverage to catch mistakes in a security-sensitive area (proxy policy, lockdown, token selection).
+
+## Decision
+
+We will introduce a named `GitHubMCPMode` string type in `tools_types.go` with constants `GitHubMCPModeLocal`, `GitHubMCPModeRemote`, `GitHubMCPModeGHProxy`, and `GitHubMCPModeCLI`, change `GitHubToolConfig.Mode` from `string` to `GitHubMCPMode`, and cast the parsed value once at the single ingestion point in `tools_parser.go`. We will also narrow every `githubTool any` parameter across `mcp_github_config.go` and its callers to `map[string]any`, performing the single `.(map[string]any)` assertion once at each call site (when reading `tools["github"]`) rather than inside every helper. `normalizeGitHubType`/`getGitHubType` return `GitHubMCPMode`, and all bare string comparisons are replaced with the typed constants. We chose this because pushing the type assertion to the boundary and naming the mode values makes the security-relevant configuration paths self-documenting and compiler-checked, with no behavioral change (nil maps remain safe since Go returns zero values on reads).
+
+## Alternatives Considered
+
+### Alternative 1: Keep `githubTool any` and the bare-string mode
+Leave the existing signatures unchanged and continue asserting to `map[string]any` inside each helper. Rejected because it perpetuates ~19 duplicated assertions and offers no compile-time defense for mode values; the duplication and stringly-typed comparisons are exactly the maintenance and correctness hazard this PR exists to remove.
+
+### Alternative 2: Model the tool config as a concrete struct end-to-end
+Replace the `map[string]any` representation entirely with a fully-typed `GitHubToolConfig` struct threaded through all helpers. Rejected (for now) as too large a change: the tool config is parsed from open-ended frontmatter and shared with generic MCP rendering paths that operate on `map[string]any`, so a full struct migration would ripple far beyond this PR. Narrowing to `map[string]any` plus a typed mode enum captures most of the type-safety benefit at a fraction of the blast radius.
+
+## Consequences
+
+### Positive
+- The single `.(map[string]any)` assertion moves to each call site, eliminating ~19 duplicated assertions and their inconsistent failure handling inside helpers.
+- `GitHubMCPMode` constants give the compiler authority over mode values, preventing typo'd or invalid mode strings in security-sensitive logic (proxy, lockdown, token selection).
+- Helper signatures (`map[string]any`, `GitHubMCPMode`) now document intent directly, improving readability for future contributors.
+
+### Negative
+- Wide, mechanical change surface: ~30 non-test files plus 11 test files touched, increasing review effort and rebase/merge-conflict risk against concurrent work in `pkg/workflow`.
+- Test inputs that previously exercised non-map values (e.g. `"invalid"`, `false`, `"not a map"`) are replaced with `nil`/empty maps, so the "wrong type passed in" branch is no longer covered at the helper level — that contract now lives at the call-site assertion.
+- The type assertion at each call site silently yields a `nil` map on mismatch (`, _`), so a malformed `tools["github"]` value degrades to zero-value behavior rather than an explicit error.
+
+### Neutral
+- `getGitHubReadOnly()` loses its unused parameter and now takes no arguments, since it unconditionally returns `true`.
+- The change is intended to be behavior-preserving; the GitHub MCP server remains unconditionally read-only and nil-map reads continue to return zero values.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27446718826) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/workflow/agent_validation.go

@@ -199,7 +199,8 @@ func (c *Compiler) validatePiEngineRequirements(tools *ToolsConfig, engine Codin
 		return nil
 	}
 
-	if tools == nil || tools.GitHub == nil || tools.GitHub.Mode != "gh-proxy" {
+	if tools == nil || tools.GitHub == nil ||
+		(tools.GitHub.Mode != GitHubMCPModeGHProxy && tools.GitHub.Mode != GitHubMCPModeCLI) {
 		return errors.New("engine 'pi' requires tools.github.mode: gh-proxy")
 	}
 

pkg/workflow/claude_tools.go

@@ -322,7 +322,7 @@ func appendGitHubMCPTools(allowedTools []string, toolName string, toolValue any,
 	}
 	githubMode := getGitHubType(mcpConfig)
 	defaultTools := constants.DefaultGitHubToolsLocal
-	if githubMode == "remote" {
+	if githubMode == GitHubMCPModeRemote {
 		defaultTools = constants.DefaultGitHubToolsRemote
 	}
 	for _, defaultTool := range defaultTools {

pkg/workflow/codex_mcp.go

@@ -56,7 +56,7 @@ func (e *CodexEngine) RenderMCPConfig(yaml *strings.Builder, tools map[string]an
 		renderer := createRenderer(false) // isLast is always false in TOML format
 		switch toolName {
 		case "github":
-			githubTool := expandedTools["github"]
+			githubTool, _ := expandedTools["github"].(map[string]any)
 			renderer.RenderGitHubMCP(&mcpConfigContent, githubTool, workflowData)
 		case "playwright":
 			playwrightTool := expandedTools["playwright"]

pkg/workflow/compiler_difc_proxy.go

@@ -83,7 +83,8 @@ func hasDIFCGuardsConfigured(data *WorkflowData) bool {
 	if !hasGitHub || githubTool == false {
 		return false
 	}
-	return len(getGitHubGuardPolicies(githubTool)) > 0
+	toolConfig, _ := githubTool.(map[string]any)
+	return len(getGitHubGuardPolicies(toolConfig)) > 0
 }
 
 // isIntegrityProxyEnabled returns true unless the user has explicitly disabled the DIFC proxy
@@ -167,20 +168,15 @@ func hasPreAgentStepsWithGHToken(data *WorkflowData) bool {
 // endorser-min-integrity) are also included in the proxy policy.
 //
 // Returns an empty string if no guard policy fields are found.
-func getDIFCProxyPolicyJSON(githubTool any, data *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig) string {
-	toolConfig, ok := githubTool.(map[string]any)
-	if !ok {
-		return ""
-	}
-
+func getDIFCProxyPolicyJSON(githubTool map[string]any, data *WorkflowData, gatewayConfig *MCPGatewayRuntimeConfig) string {
 	policy := make(map[string]any)
 
 	// Support both 'allowed-repos' (preferred) and deprecated 'repos'
-	repos, hasRepos := toolConfig["allowed-repos"]
+	repos, hasRepos := githubTool["allowed-repos"]
 	if !hasRepos {
-		repos, hasRepos = toolConfig["repos"]
+		repos, hasRepos = githubTool["repos"]
 	}
-	integrity, hasIntegrity := toolConfig["min-integrity"]
+	integrity, hasIntegrity := githubTool["min-integrity"]
 
 	if !hasRepos && !hasIntegrity {
 		return ""
@@ -198,7 +194,7 @@ func getDIFCProxyPolicyJSON(githubTool any, data *WorkflowData, gatewayConfig *M
 	}
 
 	// Inject reaction fields when the feature flag is enabled and MCPG supports it.
-	injectIntegrityReactionFields(policy, toolConfig, data, gatewayConfig)
+	injectIntegrityReactionFields(policy, githubTool, data, gatewayConfig)
 
 	guardPolicy := map[string]any{
 		"allow-only": policy,
@@ -229,16 +225,16 @@ func resolveProxyContainerImage(gatewayConfig *MCPGatewayRuntimeConfig) string {
 func (c *Compiler) buildStartDIFCProxyStepYAML(data *WorkflowData) string {
 	difcProxyLog.Print("Building Start DIFC proxy step YAML")
 
-	githubTool := data.Tools["github"]
+	githubToolConfig, _ := data.Tools["github"].(map[string]any)
 
 	// Get MCP server token (same token the gateway uses for the GitHub MCP server)
-	customGitHubToken := getGitHubToken(githubTool)
+	customGitHubToken := getGitHubToken(githubToolConfig)
 	effectiveToken := getEffectiveGitHubToken(customGitHubToken)
 
 	// Build the simplified guard policy JSON (static fields only)
 	// (plus reaction fields when integrity-reactions feature flag is enabled)
 	ensureDefaultMCPGatewayConfig(data)
-	policyJSON := getDIFCProxyPolicyJSON(githubTool, data, data.SandboxConfig.MCP)
+	policyJSON := getDIFCProxyPolicyJSON(githubToolConfig, data, data.SandboxConfig.MCP)
 	if policyJSON == "" {
 		difcProxyLog.Print("Could not build DIFC proxy policy JSON, skipping proxy start")
 		return ""
@@ -498,18 +494,18 @@ const defaultCliProxyPolicyJSON = `{"allow-only":{"repos":"all","min-integrity":
 func (c *Compiler) buildStartCliProxyStepYAML(data *WorkflowData) string {
 	difcProxyLog.Print("Building Start CLI proxy step YAML")
 
-	githubTool := data.Tools["github"]
+	githubToolConfig, _ := data.Tools["github"].(map[string]any)
 
 	// Get token for the proxy
-	customGitHubToken := getGitHubToken(githubTool)
+	customGitHubToken := getGitHubToken(githubToolConfig)
 	effectiveToken := getEffectiveGitHubToken(customGitHubToken)
 
 	// Build the guard policy JSON (static fields only, plus reaction fields when enabled).
 	// The CLI proxy requires a policy to forward requests — without one, all API
 	// calls return HTTP 503 ("proxy enforcement not configured"). Use the default
 	// permissive policy when no guard policy is configured in the frontmatter.
 	ensureDefaultMCPGatewayConfig(data)
-	policyJSON := getDIFCProxyPolicyJSON(githubTool, data, data.SandboxConfig.MCP)
+	policyJSON := getDIFCProxyPolicyJSON(githubToolConfig, data, data.SandboxConfig.MCP)
 	if policyJSON == "" {
 		policyJSON = defaultCliProxyPolicyJSON
 		difcProxyLog.Print("No guard policy configured, using default CLI proxy policy")

pkg/workflow/compiler_difc_proxy_test.go

@@ -210,7 +210,7 @@ func TestHasPreAgentStepsWithGHToken(t *testing.T) {
 func TestGetDIFCProxyPolicyJSON(t *testing.T) {
 	tests := []struct {
 		name             string
-		githubTool       any
+		githubTool       map[string]any
 		expectedContains []string
 		expectedAbsent   []string
 		expectEmpty      bool
@@ -221,8 +221,8 @@ func TestGetDIFCProxyPolicyJSON(t *testing.T) {
 			expectEmpty: true,
 		},
 		{
-			name:        "non-map tool",
-			githubTool:  false,
+			name:        "empty map tool",
+			githubTool:  map[string]any{},
 			expectEmpty: true,
 		},
 		{

pkg/workflow/compiler_github_mcp_steps.go

@@ -26,10 +26,11 @@ func (c *Compiler) generateGitHubMCPLockdownDetectionStep(yaml *strings.Builder,
 		githubConfigLog.Print("Skipping GitHub MCP lockdown detection step: GitHub tool not enabled")
 		return
 	}
+	githubToolMap, _ := githubTool.(map[string]any)
 
 	// Skip when guard policy is already fully configured in the workflow.
 	// The step is only needed to auto-configure guard policies for public repos.
-	if len(getGitHubGuardPolicies(githubTool)) > 0 {
+	if len(getGitHubGuardPolicies(githubToolMap)) > 0 {
 		githubConfigLog.Print("Guard policy already configured in workflow, skipping automatic guard policy determination")
 		return
 	}
@@ -168,9 +169,10 @@ func (c *Compiler) generateParseGuardVarsStep(yaml *strings.Builder, data *Workf
 		githubConfigLog.Print("Skipping parse-guard-vars step: GitHub tool not enabled")
 		return
 	}
+	githubToolMap, _ := githubTool.(map[string]any)
 
 	// Only generate the step when guard policies are configured.
-	if len(getGitHubGuardPolicies(githubTool)) == 0 {
+	if len(getGitHubGuardPolicies(githubToolMap)) == 0 {
 		githubConfigLog.Print("Skipping parse-guard-vars step: no explicit guard policies configured")
 		return
 	}

pkg/workflow/compiler_yaml.go

@@ -933,12 +933,15 @@ func (c *Compiler) generateCreateAwInfo(yaml *strings.Builder, data *WorkflowDat
 	// Include lockdown validation env vars when lockdown is explicitly enabled.
 	// validateLockdownRequirements is called from generate_aw_info.cjs and uses these vars.
 	githubTool, hasGitHub := data.Tools["github"]
-	if hasGitHub && githubTool != false && hasGitHubLockdownExplicitlySet(githubTool) && getGitHubLockdown(githubTool) {
-		yaml.WriteString("          GITHUB_MCP_LOCKDOWN_EXPLICIT: \"true\"\n")
-		yaml.WriteString("          GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }}\n")
-		yaml.WriteString("          GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }}\n")
-		if customToken := getGitHubToken(githubTool); customToken != "" {
-			fmt.Fprintf(yaml, "          CUSTOM_GITHUB_TOKEN: %s\n", customToken)
+	if hasGitHub && githubTool != false {
+		toolConfig, _ := githubTool.(map[string]any)
+		if hasGitHubLockdownExplicitlySet(toolConfig) && getGitHubLockdown(toolConfig) {
+			yaml.WriteString("          GITHUB_MCP_LOCKDOWN_EXPLICIT: \"true\"\n")
+			yaml.WriteString("          GH_AW_GITHUB_TOKEN: ${{ secrets.GH_AW_GITHUB_TOKEN }}\n")
+			yaml.WriteString("          GH_AW_GITHUB_MCP_SERVER_TOKEN: ${{ secrets.GH_AW_GITHUB_MCP_SERVER_TOKEN }}\n")
+			if customToken := getGitHubToken(toolConfig); customToken != "" {
+				fmt.Fprintf(yaml, "          CUSTOM_GITHUB_TOKEN: %s\n", customToken)
+			}
 		}
 	}
 	// Embed custom token weights only when custom model multipliers are configured.

pkg/workflow/copilot_engine_execution.go

@@ -604,12 +604,14 @@ touch %s
 		if workflowData.ParsedTools != nil && workflowData.ParsedTools.GitHub != nil && workflowData.ParsedTools.GitHub.GitHubApp != nil {
 			tokenExpression := "${{ steps.github-mcp-app-token.outputs.token }}"
 			if workflowData.ParsedTools.GitHub.GitHubApp.shouldIgnoreMissingKey() {
-				customGitHubToken := getGitHubToken(workflowData.Tools["github"])
+				githubToolConfig, _ := workflowData.Tools["github"].(map[string]any)
+				customGitHubToken := getGitHubToken(githubToolConfig)
 				tokenExpression = combineTokenExpressions(tokenExpression, getEffectiveGitHubToken(customGitHubToken))
 			}
 			env["GITHUB_MCP_SERVER_TOKEN"] = tokenExpression
 		} else {
-			customGitHubToken := getGitHubToken(workflowData.Tools["github"])
+			githubToolConfig, _ := workflowData.Tools["github"].(map[string]any)
+			customGitHubToken := getGitHubToken(githubToolConfig)
 			// Use effective token with precedence: custom > default
 			effectiveToken := getEffectiveGitHubToken(customGitHubToken)
 			env["GITHUB_MCP_SERVER_TOKEN"] = effectiveToken

pkg/workflow/copilot_engine_test.go

@@ -1444,7 +1444,7 @@ func TestCopilotEnginePromptFilePath(t *testing.T) {
 func TestCopilotEngineRenderGitHubMCPConfig(t *testing.T) {
 	tests := []struct {
 		name         string
-		githubTool   any
+		githubTool   map[string]any
 		isLast       bool
 		expectedStrs []string
 	}{

pkg/workflow/copilot_github_mcp_test.go

@@ -13,7 +13,7 @@ import (
 func TestRenderGitHubCopilotMCPConfig_AllowedTools(t *testing.T) {
 	tests := []struct {
 		name              string
-		githubTool        any
+		githubTool        map[string]any
 		isLast            bool
 		expectedContent   []string
 		unexpectedContent []string
@@ -122,7 +122,7 @@ func TestRenderGitHubCopilotMCPConfig_AllowedTools(t *testing.T) {
 func TestGetGitHubAllowedTools(t *testing.T) {
 	tests := []struct {
 		name       string
-		githubTool any
+		githubTool map[string]any
 		expected   []string
 	}{
 		{
@@ -169,8 +169,8 @@ func TestGetGitHubAllowedTools(t *testing.T) {
 			expected: []string{"issue_read:1", "list_labels"},
 		},
 		{
-			name:       "Not a map",
-			githubTool: "invalid",
+			name:       "Nil tool",
+			githubTool: nil,
 			expected:   nil,
 		},
 	}
@@ -208,7 +208,7 @@ func TestGetGitHubAllowedTools(t *testing.T) {
 func TestGetGitHubGuardPoliciesToolCallLimits(t *testing.T) {
 	tests := []struct {
 		name       string
-		githubTool any
+		githubTool map[string]any
 		expected   map[string]any
 	}{
 		{

pkg/workflow/docker.go

@@ -20,15 +20,19 @@ func collectDockerImages(tools map[string]any, workflowData *WorkflowData, actio
 	imageSet := make(map[string]bool) // Use a set to avoid duplicates
 
 	// Check for GitHub tool (uses Docker image)
-	if githubTool, hasGitHub := tools["github"]; hasGitHub {
-		githubType := getGitHubType(githubTool)
-		// Only add if using local (Docker) mode
-		if githubType == "local" {
-			githubDockerImageVersion := getGitHubDockerImageVersion(githubTool)
-			image := "ghcr.io/github/github-mcp-server:" + githubDockerImageVersion
-			if !imageSet[image] {
-				images = append(images, image)
-				imageSet[image] = true
+	if rawGithubTool, hasGitHub := tools["github"]; hasGitHub {
+		// Only proceed when the value is an actual config map; a boolean false
+		// means the tool is explicitly disabled.
+		if githubTool, ok := rawGithubTool.(map[string]any); ok {
+			githubType := getGitHubType(githubTool)
+			// Only add if using local (Docker) mode
+			if githubType == GitHubMCPModeLocal {
+				githubDockerImageVersion := getGitHubDockerImageVersion(githubTool)
+				image := "ghcr.io/github/github-mcp-server:" + githubDockerImageVersion
+				if !imageSet[image] {
+					images = append(images, image)
+					imageSet[image] = true
+				}
 			}
 		}
 	}

pkg/workflow/engine_helpers_shared_test.go

@@ -367,7 +367,7 @@ func TestRenderJSONMCPConfig(t *testing.T) {
 			options: JSONMCPConfigOptions{
 				ConfigPath: "/tmp/test-config.json",
 				Renderers: MCPToolRenderers{
-					RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
+					RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) {
 						yaml.WriteString("              \"github\": { \"test\": true }")
 						if !isLast {
 							yaml.WriteString(",")
@@ -406,7 +406,7 @@ func TestRenderJSONMCPConfig(t *testing.T) {
 			options: JSONMCPConfigOptions{
 				ConfigPath: "/tmp/filtered-config.json",
 				Renderers: MCPToolRenderers{
-					RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
+					RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) {
 						yaml.WriteString("              \"github\": { \"filtered\": true }")
 						if !isLast {
 							yaml.WriteString(",")
@@ -442,7 +442,7 @@ func TestRenderJSONMCPConfig(t *testing.T) {
 			options: JSONMCPConfigOptions{
 				ConfigPath: "/tmp/debug-config.json",
 				Renderers: MCPToolRenderers{
-					RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
+					RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) {
 						yaml.WriteString("              \"github\": {}\n")
 					},
 					RenderPlaywright:       func(yaml *strings.Builder, playwrightTool any, isLast bool) {},
@@ -511,7 +511,7 @@ func TestRenderJSONMCPConfig_IsLastHandling(t *testing.T) {
 	options := JSONMCPConfigOptions{
 		ConfigPath: "/tmp/test.json",
 		Renderers: MCPToolRenderers{
-			RenderGitHub: func(yaml *strings.Builder, githubTool any, isLast bool, workflowData *WorkflowData) {
+			RenderGitHub: func(yaml *strings.Builder, githubTool map[string]any, isLast bool, workflowData *WorkflowData) {
 				callOrder = append(callOrder, "github")
 				isLastValues = append(isLastValues, isLast)
 			},

pkg/workflow/github_lockdown_test.go

@@ -54,7 +54,7 @@ func TestGitHubLockdownField(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			githubTool := tt.toolsMap["github"]
+			githubTool, _ := tt.toolsMap["github"].(map[string]any)
 			result := getGitHubLockdown(githubTool)
 			if result != tt.expected {
 				t.Errorf("getGitHubLockdown() = %v, want %v", result, tt.expected)