From 9f6f3cddca49c35ea640ca0595f508c2b4df0dec Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:17:29 +0000 Subject: [PATCH 1/4] Initial plan From 11d5730cfbd9f9153572e05b9b64a5de00c390f3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:35:43 +0000 Subject: [PATCH 2/4] fix: expose safe-outputs.actions custom action tools to agent MCP toolset Add custom action tool names to config.json in generateSafeOutputsConfig. Both MCP server implementations gate tool registration through config.json; custom action names were missing from it, causing the tools to be skipped. Fixes: safe-outputs.actions tools not available at runtime (#issue) Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c182d7e9-a465-4d83-a124-c5e4a2709a76 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/smoke-codex.lock.yml | 2 +- .../safe_outputs_config_generation.go | 13 +++++++ .../safe_outputs_config_generation_test.go | 35 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/.github/workflows/smoke-codex.lock.yml b/.github/workflows/smoke-codex.lock.yml index 574f3270e04..219a9a4326f 100644 --- a/.github/workflows/smoke-codex.lock.yml +++ b/.github/workflows/smoke-codex.lock.yml @@ -537,7 +537,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > "${RUNNER_TEMP}/gh-aw/safeoutputs/config.json" << 'GH_AW_SAFE_OUTPUTS_CONFIG_7fd0a6e0f1cb4a80_EOF' - {"add_comment":{"hide_older_comments":true,"max":2},"add_labels":{"allowed":["smoke-codex"]},"create_issue":{"close_older_issues":true,"close_older_key":"smoke-codex","expires":2,"labels":["automation","testing"],"max":1},"create_report_incomplete_issue":{},"hide_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"remove_labels":{"allowed":["smoke"]},"report_incomplete":{},"unassign_from_user":{"allowed":["githubactionagent"],"max":1}} + {"add_comment":{"hide_older_comments":true,"max":2},"add_labels":{"allowed":["smoke-codex"]},"add_smoked_label":true,"create_issue":{"close_older_issues":true,"close_older_key":"smoke-codex","expires":2,"labels":["automation","testing"],"max":1},"create_report_incomplete_issue":{},"hide_comment":{"max":5},"missing_data":{},"missing_tool":{},"noop":{"max":1,"report-as-issue":"true"},"remove_labels":{"allowed":["smoke"]},"report_incomplete":{},"unassign_from_user":{"allowed":["githubactionagent"],"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_7fd0a6e0f1cb4a80_EOF - name: Write Safe Outputs Tools env: diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index fde86bf8019..3e6a1f5f116 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -109,6 +109,19 @@ 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) + 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 { diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index d02e9f38007..7d850a9f7c2 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -59,6 +59,41 @@ 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 := generateSafeOutputsConfig(data) + 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") +} + // 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) { From 15d5f9242e41fe448a9ea121fda975a9b12886ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 21:55:16 +0000 Subject: [PATCH 3/4] fix: guard against config key collision when adding safe-actions to config.json Skip writing a custom action's normalized name if it collides with an existing config entry (e.g. built-in handlers, safe-scripts), so the original object config is never overwritten by a bare `true`. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/d7f3ab8d-cd82-40a9-8baa-6b6ad5127fdd Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../safe_outputs_config_generation.go | 8 +++++ .../safe_outputs_config_generation_test.go | 35 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 3e6a1f5f116..214e3abeaf2 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -117,6 +117,14 @@ func generateSafeOutputsConfig(data *WorkflowData) string { 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 { + safeOutputsConfigLog.Printf( + "Skipping safe action %q because normalized name %q conflicts with an existing safe outputs config entry", + actionName, + normalizedName, + ) + continue + } safeOutputsConfigLog.Printf("Adding safe action to config: %s (normalized: %s)", actionName, normalizedName) safeOutputsConfig[normalizedName] = true } diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index 7d850a9f7c2..8b501b92e23 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.go @@ -94,6 +94,41 @@ func TestGenerateSafeOutputsConfigActions(t *testing.T) { assert.Equal(t, true, publishVal, "publish_results value should be true") } +// TestGenerateSafeOutputsConfigActionsNoConflict tests that a custom action whose normalized +// name collides with an existing built-in handler key does not overwrite it. +func TestGenerateSafeOutputsConfigActionsNoConflict(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 not overwrite add_labels config", + }, + }, + // Ensure at least one handler is set to make config non-empty. + NoOp: &NoOpConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: &trueVal}}, + }, + } + + result := generateSafeOutputsConfig(data) + 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") + + // The built-in add_labels config (an object with "allowed") must be preserved. + addLabelsVal, ok := parsed["add_labels"].(map[string]any) + require.True(t, ok, "add_labels should remain a map (not overwritten by custom action)") + allowed, _ := addLabelsVal["allowed"].([]any) + assert.Contains(t, allowed, "bug", "add_labels.allowed should still contain the configured label") +} + // 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) { From 9b4709eab13cf7c6335d6e2474091fd078c9bfee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Apr 2026 22:30:40 +0000 Subject: [PATCH 4/4] fix: return error on safe-action name collision instead of silently skipping Change generateSafeOutputsConfig signature to (string, error) and return an error when a custom action's normalized name conflicts with an existing config entry (built-in handler or safe-script), instead of silently skipping the action. Update all callers to handle the new error return. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/695d9ff3-cbaa-4619-8376-6189a6d97001 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../compile_outputs_allowed_labels_test.go | 4 +- pkg/workflow/dispatch_repository_test.go | 5 +- pkg/workflow/dispatch_workflow_test.go | 3 +- pkg/workflow/mcp_setup_generator.go | 6 +- pkg/workflow/safe_jobs_test.go | 5 +- .../safe_outputs_config_generation.go | 13 ++-- .../safe_outputs_config_generation_test.go | 74 +++++++++++-------- pkg/workflow/safe_outputs_mentions_test.go | 7 +- pkg/workflow/safe_outputs_test.go | 4 +- 9 files changed, 73 insertions(+), 48 deletions(-) diff --git a/pkg/workflow/compile_outputs_allowed_labels_test.go b/pkg/workflow/compile_outputs_allowed_labels_test.go index 9771f29e3d7..fbdd3da8abe 100644 --- a/pkg/workflow/compile_outputs_allowed_labels_test.go +++ b/pkg/workflow/compile_outputs_allowed_labels_test.go @@ -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"`) { diff --git a/pkg/workflow/dispatch_repository_test.go b/pkg/workflow/dispatch_repository_test.go index 6ac742b44ef..1b77838511c 100644 --- a/pkg/workflow/dispatch_repository_test.go +++ b/pkg/workflow/dispatch_repository_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) diff --git a/pkg/workflow/dispatch_workflow_test.go b/pkg/workflow/dispatch_workflow_test.go index c7498485a71..063d43cb223 100644 --- a/pkg/workflow/dispatch_workflow_test.go +++ b/pkg/workflow/dispatch_workflow_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 diff --git a/pkg/workflow/mcp_setup_generator.go b/pkg/workflow/mcp_setup_generator.go index 575086354e8..a069176d781 100644 --- a/pkg/workflow/mcp_setup_generator.go +++ b/pkg/workflow/mcp_setup_generator.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 diff --git a/pkg/workflow/safe_jobs_test.go b/pkg/workflow/safe_jobs_test.go index e3114aabba8..0b5c2f526bc 100644 --- a/pkg/workflow/safe_jobs_test.go +++ b/pkg/workflow/safe_jobs_test.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") diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index 214e3abeaf2..50695b278a5 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.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") @@ -118,12 +118,11 @@ func generateSafeOutputsConfig(data *WorkflowData) string { for actionName := range data.SafeOutputs.Actions { normalizedName := stringutil.NormalizeSafeOutputIdentifier(actionName) if _, exists := safeOutputsConfig[normalizedName]; exists { - safeOutputsConfigLog.Printf( - "Skipping safe action %q because normalized name %q conflicts with an existing safe outputs config entry", + 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, ) - continue } safeOutputsConfigLog.Printf("Adding safe action to config: %s (normalized: %s)", actionName, normalizedName) safeOutputsConfig[normalizedName] = true @@ -186,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. diff --git a/pkg/workflow/safe_outputs_config_generation_test.go b/pkg/workflow/safe_outputs_config_generation_test.go index 8b501b92e23..61e4aa36a66 100644 --- a/pkg/workflow/safe_outputs_config_generation_test.go +++ b/pkg/workflow/safe_outputs_config_generation_test.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 @@ -77,7 +78,8 @@ func TestGenerateSafeOutputsConfigActions(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 @@ -94,9 +96,9 @@ func TestGenerateSafeOutputsConfigActions(t *testing.T) { assert.Equal(t, true, publishVal, "publish_results value should be true") } -// TestGenerateSafeOutputsConfigActionsNoConflict tests that a custom action whose normalized -// name collides with an existing built-in handler key does not overwrite it. -func TestGenerateSafeOutputsConfigActionsNoConflict(t *testing.T) { +// 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{ @@ -108,7 +110,7 @@ func TestGenerateSafeOutputsConfigActionsNoConflict(t *testing.T) { Actions: map[string]*SafeOutputActionConfig{ "add-labels": { Uses: "owner/some-action@v1", - Description: "Should not overwrite add_labels config", + Description: "Should trigger a collision error", }, }, // Ensure at least one handler is set to make config non-empty. @@ -116,17 +118,10 @@ func TestGenerateSafeOutputsConfigActionsNoConflict(t *testing.T) { }, } - result := generateSafeOutputsConfig(data) - 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") - - // The built-in add_labels config (an object with "allowed") must be preserved. - addLabelsVal, ok := parsed["add_labels"].(map[string]any) - require.True(t, ok, "add_labels should remain a map (not overwritten by custom action)") - allowed, _ := addLabelsVal["allowed"].([]any) - assert.Contains(t, allowed, "bug", "add_labels.allowed should still contain the configured label") + _, 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. @@ -143,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 @@ -175,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 @@ -417,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 @@ -456,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 @@ -499,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 @@ -532,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 @@ -557,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 @@ -580,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 @@ -616,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 @@ -656,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 @@ -679,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 @@ -702,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 @@ -731,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 @@ -775,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 @@ -813,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 diff --git a/pkg/workflow/safe_outputs_mentions_test.go b/pkg/workflow/safe_outputs_mentions_test.go index ead41ea63ba..5d414c0762e 100644 --- a/pkg/workflow/safe_outputs_mentions_test.go +++ b/pkg/workflow/safe_outputs_mentions_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) } diff --git a/pkg/workflow/safe_outputs_test.go b/pkg/workflow/safe_outputs_test.go index 456a5224fc5..6646c3fe248 100644 --- a/pkg/workflow/safe_outputs_test.go +++ b/pkg/workflow/safe_outputs_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/github/gh-aw/pkg/stringutil" + "github.com/stretchr/testify/require" ) // ======================================== @@ -661,7 +662,8 @@ func TestGenerateSafeOutputsConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := generateSafeOutputsConfig(tt.workflowData) + result, err := generateSafeOutputsConfig(tt.workflowData) + require.NoError(t, err, "generateSafeOutputsConfig should not return an error") if tt.expectEmpty { if result != "" {