fix: expose safe-outputs.actions custom action tools to agent MCP toolset (#26291)

Author: Copilot

.github/workflows/smoke-codex.lock.yml

@@ -11,6 +11,7 @@ import (
 	"github.com/github/gh-aw/pkg/stringutil"
 
 	"github.com/github/gh-aw/pkg/testutil"
+	"github.com/stretchr/testify/require"
 )
 
 func TestAllowedLabelsConfigParsing(t *testing.T) {
@@ -302,7 +303,8 @@ This workflow tests that allowed-labels are included in safe outputs config JSON
 	}
 
 	// Generate safe outputs config
-	configJSON := generateSafeOutputsConfig(workflowData)
+	configJSON, err := generateSafeOutputsConfig(workflowData)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 
 	// Verify that allowed_labels is in the config
 	if !strings.Contains(configJSON, `"allowed_labels"`) {

pkg/workflow/compile_outputs_allowed_labels_test.go

@@ -478,11 +478,12 @@ func TestDispatchRepositoryConfigSerialization(t *testing.T) {
 		},
 	}
 
-	configJSON := generateSafeOutputsConfig(workflowData)
+	configJSON, err := generateSafeOutputsConfig(workflowData)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, configJSON, "Config JSON should not be empty")
 
 	var config map[string]any
-	err := json.Unmarshal([]byte(configJSON), &config)
+	err = json.Unmarshal([]byte(configJSON), &config)
 	require.NoError(t, err, "Config JSON should be valid")
 
 	dispatchRepo, ok := config["dispatch_repository"].(map[string]any)

pkg/workflow/dispatch_repository_test.go

@@ -381,7 +381,8 @@ This workflow dispatches to different workflow types.
 		"yml-test should use .yml extension")
 
 	// Generate safe outputs config to verify workflow_files is included
-	configJSON := generateSafeOutputsConfig(workflowData)
+	configJSON, err := generateSafeOutputsConfig(workflowData)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, configJSON, "Config JSON should not be empty")
 
 	// Parse config to verify workflow_files is present

pkg/workflow/dispatch_workflow_test.go

@@ -129,7 +129,11 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
 	// Generate safe-outputs configuration once to avoid duplicate computation
 	var safeOutputConfig string
 	if HasSafeOutputsEnabled(workflowData.SafeOutputs) {
-		safeOutputConfig = generateSafeOutputsConfig(workflowData)
+		var err error
+		safeOutputConfig, err = generateSafeOutputsConfig(workflowData)
+		if err != nil {
+			return fmt.Errorf("failed to generate safe outputs config: %w", err)
+		}
 	}
 
 	// Sort tools to ensure stable code generation

pkg/workflow/mcp_setup_generator.go

@@ -5,6 +5,8 @@ package workflow
 import (
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseSafeJobsConfig(t *testing.T) {
@@ -392,7 +394,8 @@ func TestSafeJobsInSafeOutputsConfig(t *testing.T) {
 		},
 	}
 
-	configJSON := generateSafeOutputsConfig(workflowData)
+	configJSON, err := generateSafeOutputsConfig(workflowData)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 
 	if configJSON == "" {
 		t.Fatal("Expected safe-outputs config JSON to be generated")

pkg/workflow/safe_jobs_test.go

@@ -25,10 +25,10 @@ import (
 // generateSafeOutputsConfig generates the JSON configuration for the safe-outputs
 // MCP server. Standard handler configs are sourced from handlerRegistry to ensure
 // they stay in sync with GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG.
-func generateSafeOutputsConfig(data *WorkflowData) string {
+func generateSafeOutputsConfig(data *WorkflowData) (string, error) {
 	if data.SafeOutputs == nil {
 		safeOutputsConfigLog.Print("No safe outputs configuration found, returning empty config")
-		return ""
+		return "", nil
 	}
 	safeOutputsConfigLog.Print("Generating safe outputs configuration for workflow")
 
@@ -109,6 +109,26 @@ func generateSafeOutputsConfig(data *WorkflowData) string {
 		}
 	}
 
+	// Safe-actions configuration: custom GitHub Actions exposed as safe output tools.
+	// The normalized action names are added as config keys so both MCP server implementations
+	// recognise them as enabled tools (the tool schema is already in tools.json via
+	// tools_meta.json; the MCP server just needs to see the name in config.json).
+	if len(data.SafeOutputs.Actions) > 0 {
+		safeOutputsConfigLog.Printf("Processing %d safe action configurations", len(data.SafeOutputs.Actions))
+		for actionName := range data.SafeOutputs.Actions {
+			normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName)
+			if _, exists := safeOutputsConfig[normalizedName]; exists {
+				return "", fmt.Errorf(
+					"safe-outputs action %q has a normalized name %q that conflicts with an existing safe outputs config entry; rename the action to avoid the conflict",
+					actionName,
+					normalizedName,
+				)
+			}
+			safeOutputsConfigLog.Printf("Adding safe action to config: %s (normalized: %s)", actionName, normalizedName)
+			safeOutputsConfig[normalizedName] = true
+		}
+	}
+
 	// Mentions configuration: controls which @mentions are allowed in AI output.
 	// This is consumed by the ingestion step, not by standard handlers.
 	if data.SafeOutputs.Mentions != nil {
@@ -165,11 +185,11 @@ func generateSafeOutputsConfig(data *WorkflowData) string {
 	}
 
 	if len(safeOutputsConfig) == 0 {
-		return ""
+		return "", nil
 	}
 	configJSON, _ := json.Marshal(safeOutputsConfig)
 	safeOutputsConfigLog.Printf("Safe outputs config generation complete: %d tool types configured", len(safeOutputsConfig))
-	return string(configJSON)
+	return string(configJSON), nil
 }
 
 // generateCustomJobToolDefinition creates an MCP tool definition for a custom safe-output job.

pkg/workflow/safe_outputs_config_generation.go

@@ -43,7 +43,8 @@ jobs:
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -59,6 +60,70 @@ jobs:
 	assert.Equal(t, ".lock.yml", workflowFiles["ci"], "ci should map to .lock.yml")
 }
 
+// TestGenerateSafeOutputsConfigActions tests that generateSafeOutputsConfig includes custom
+// action tool names as enabled keys so both MCP server implementations register them.
+func TestGenerateSafeOutputsConfigActions(t *testing.T) {
+	data := &WorkflowData{
+		SafeOutputs: &SafeOutputsConfig{
+			Actions: map[string]*SafeOutputActionConfig{
+				"upload_report": {
+					Uses:        "actions/upload-artifact@v4",
+					Description: "Upload the report",
+				},
+				"publish-results": {
+					Uses:        "owner/action@v1",
+					Description: "Publish results",
+				},
+			},
+		},
+	}
+
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
+	require.NotEmpty(t, result, "Expected non-empty config")
+
+	var parsed map[string]any
+	require.NoError(t, json.Unmarshal([]byte(result), &parsed), "Result must be valid JSON")
+
+	// Each action tool should appear as a truthy key in config.json so the MCP server
+	// registers it. Names are normalized (hyphens converted to underscores).
+	uploadVal, hasUploadReport := parsed["upload_report"]
+	assert.True(t, hasUploadReport, "Expected upload_report key in config")
+	assert.Equal(t, true, uploadVal, "upload_report value should be true")
+
+	publishVal, hasPublishResults := parsed["publish_results"]
+	assert.True(t, hasPublishResults, "Expected publish_results key in config (hyphen normalized to underscore)")
+	assert.Equal(t, true, publishVal, "publish_results value should be true")
+}
+
+// TestGenerateSafeOutputsConfigActionsCollisionReturnsError tests that a custom action
+// whose normalized name collides with an existing built-in handler key returns an error.
+func TestGenerateSafeOutputsConfigActionsCollisionReturnsError(t *testing.T) {
+	trueVal := "true"
+	data := &WorkflowData{
+		SafeOutputs: &SafeOutputsConfig{
+			// add_labels is a built-in handler that produces a real config object.
+			AddLabels: &AddLabelsConfig{
+				Allowed: []string{"bug"},
+			},
+			// A custom action whose normalized name matches the built-in "add_labels" key.
+			Actions: map[string]*SafeOutputActionConfig{
+				"add-labels": {
+					Uses:        "owner/some-action@v1",
+					Description: "Should trigger a collision error",
+				},
+			},
+			// Ensure at least one handler is set to make config non-empty.
+			NoOp: &NoOpConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: &trueVal}},
+		},
+	}
+
+	_, err := generateSafeOutputsConfig(data)
+	require.Error(t, err, "Expected an error when a custom action name collides with a built-in handler key")
+	assert.Contains(t, err.Error(), "add-labels", "Error should mention the conflicting action name")
+	assert.Contains(t, err.Error(), "add_labels", "Error should mention the conflicting normalized name")
+}
+
 // TestGenerateSafeOutputsConfigMissingToolWithIssue tests the missing_tool config.
 // The legacy create_missing_tool_issue sub-key is no longer generated; only missing_tool is present.
 func TestGenerateSafeOutputsConfigMissingToolWithIssue(t *testing.T) {
@@ -73,7 +138,8 @@ func TestGenerateSafeOutputsConfigMissingToolWithIssue(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -105,7 +171,8 @@ func TestGenerateSafeOutputsConfigMentions(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -347,7 +414,8 @@ func TestGenerateSafeOutputsConfigAddLabelsBlocked(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -386,7 +454,8 @@ func TestGenerateSafeOutputsConfigCreatePullRequestTargetRepo(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -429,7 +498,8 @@ func TestGenerateSafeOutputsConfigCreatePullRequestBackwardCompat(t *testing.T)
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -462,7 +532,8 @@ func TestGenerateSafeOutputsConfigCreatePullRequestAutoCloseIssue(t *testing.T)
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -487,7 +558,8 @@ func TestGenerateSafeOutputsConfigCreatePullRequestAutoCloseIssueExpression(t *t
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -510,7 +582,8 @@ func TestGenerateSafeOutputsConfigCreatePullRequestAutoCloseIssueOmittedByDefaul
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -546,7 +619,8 @@ func TestGenerateSafeOutputsConfigRepoMemory(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -586,7 +660,8 @@ func TestGenerateSafeOutputsConfigNoRepoMemory(t *testing.T) {
 		RepoMemoryConfig: nil,
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -609,7 +684,8 @@ func TestGenerateSafeOutputsConfigEmptyRepoMemory(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -632,7 +708,8 @@ func TestGenerateSafeOutputsConfigReplyToPullRequestReviewComment(t *testing.T)
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -661,7 +738,8 @@ func TestGenerateSafeOutputsConfigReplyToPullRequestReviewCommentWithTarget(t *t
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -705,7 +783,8 @@ func TestGenerateSafeOutputsConfigClosePullRequest(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any
@@ -743,7 +822,8 @@ func TestGenerateSafeOutputsConfigClosePullRequestStaged(t *testing.T) {
 		},
 	}
 
-	result := generateSafeOutputsConfig(data)
+	result, err := generateSafeOutputsConfig(data)
+	require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 	require.NotEmpty(t, result, "Expected non-empty config")
 
 	var parsed map[string]any

pkg/workflow/safe_outputs_config_generation_test.go

@@ -5,6 +5,8 @@ package workflow
 import (
 	"encoding/json"
 	"testing"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestParseMentionsConfig_Boolean(t *testing.T) {
@@ -216,9 +218,10 @@ func TestGenerateSafeOutputsConfig_WithMentions(t *testing.T) {
 				},
 			}
 
-			configJSON := generateSafeOutputsConfig(data)
+			configJSON, err := generateSafeOutputsConfig(data)
+			require.NoError(t, err, "generateSafeOutputsConfig should not return an error")
 			var parsed map[string]any
-			err := json.Unmarshal([]byte(configJSON), &parsed)
+			err = json.Unmarshal([]byte(configJSON), &parsed)
 			if err != nil {
 				t.Fatalf("Failed to parse config JSON: %v", err)
 			}