From 07198b6e98ac8dcb991308fccdca491eaa102022 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 20:50:43 +0000 Subject: [PATCH 1/3] Initial plan From 5b0aed38060bba1d2de5943a70e96099fde95093 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 21:04:13 +0000 Subject: [PATCH 2/3] Fix: include custom jobs, scripts, and actions in prompt block The buildSafeOutputsSections function now includes custom jobs (SafeOutputsConfig.Jobs), scripts (SafeOutputsConfig.Scripts), and actions (SafeOutputsConfig.Actions) in the compiled prompt section, preventing silent drift between the runtime configuration surface and the agent-facing compiled instructions. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/c4791b62-2306-4194-adbd-6a5c94ba32ef Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .../safe_outputs_prompt_tools_test.go | 224 ++++++++++++++++++ pkg/workflow/unified_prompt_step.go | 37 +++ 2 files changed, 261 insertions(+) create mode 100644 pkg/workflow/safe_outputs_prompt_tools_test.go diff --git a/pkg/workflow/safe_outputs_prompt_tools_test.go b/pkg/workflow/safe_outputs_prompt_tools_test.go new file mode 100644 index 00000000000..98ddef77d13 --- /dev/null +++ b/pkg/workflow/safe_outputs_prompt_tools_test.go @@ -0,0 +1,224 @@ +//go:build !integration + +package workflow + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestBuildSafeOutputsSectionsCustomTools verifies that custom jobs, scripts, and actions +// defined in safe-outputs are included in the compiled prompt block. +// This prevents silent drift between the runtime configuration surface and the +// agent-facing compiled instructions. +func TestBuildSafeOutputsSectionsCustomTools(t *testing.T) { + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectedTools []string + expectNil bool + }{ + { + name: "nil safe outputs returns nil", + expectNil: true, + }, + { + name: "empty safe outputs returns nil", + safeOutputs: &SafeOutputsConfig{}, + expectNil: true, + }, + { + name: "custom job appears in tools list", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "deploy": {Description: "Deploy to production"}, + }, + }, + expectedTools: []string{"noop", "deploy"}, + }, + { + name: "custom job name with dashes is normalized to underscores", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "send-notification": {Description: "Send a notification"}, + }, + }, + expectedTools: []string{"noop", "send_notification"}, + }, + { + name: "multiple custom jobs are sorted and appear in tools list", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "zebra-job": {}, + "alpha-job": {}, + }, + }, + expectedTools: []string{"noop", "alpha_job", "zebra_job"}, + }, + { + name: "custom script appears in tools list", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Scripts: map[string]*SafeScriptConfig{ + "my-script": {Description: "Run my script"}, + }, + }, + expectedTools: []string{"noop", "my_script"}, + }, + { + name: "custom action appears in tools list", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Actions: map[string]*SafeOutputActionConfig{ + "my-action": {Description: "Run my custom action"}, + }, + }, + expectedTools: []string{"noop", "my_action"}, + }, + { + name: "custom jobs are listed even without predefined tools", + safeOutputs: &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "custom-deploy": {}, + }, + }, + expectedTools: []string{"noop", "custom_deploy"}, + }, + { + name: "mix of predefined tools and custom jobs both appear in tools list", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + AddComments: &AddCommentsConfig{}, + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "deploy": {}, + }, + Scripts: map[string]*SafeScriptConfig{ + "notify": {}, + }, + }, + expectedTools: []string{"add_comment", "create_issue", "noop", "deploy", "notify"}, + }, + { + name: "mix of predefined tools, custom jobs, scripts, and actions all appear", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{}, + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "custom-job": {}, + }, + Scripts: map[string]*SafeScriptConfig{ + "custom-script": {}, + }, + Actions: map[string]*SafeOutputActionConfig{ + "custom-action": {}, + }, + }, + expectedTools: []string{"create_issue", "noop", "custom_job", "custom_script", "custom_action"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sections := buildSafeOutputsSections(tt.safeOutputs) + + if tt.expectNil { + assert.Nil(t, sections, "Expected nil sections for empty/nil config") + return + } + + require.NotNil(t, sections, "Expected non-nil sections") + + // Find the opening section with the Tools: list + var toolsLine string + for _, section := range sections { + if !section.IsFile && strings.HasPrefix(section.Content, "") { + toolsLine = section.Content + break + } + } + + require.NotEmpty(t, toolsLine, "Expected to find opening section") + + // Extract tool names from "Tools: tool1, tool2, ..." line + lines := strings.Split(toolsLine, "\n") + require.GreaterOrEqual(t, len(lines), 2, "Expected at least two lines in tools section") + + toolsListLine := lines[1] + assert.True(t, strings.HasPrefix(toolsListLine, "Tools: "), "Second line should start with 'Tools: '") + + toolsList := strings.TrimPrefix(toolsListLine, "Tools: ") + actualToolNames := strings.Split(toolsList, ", ") + + // Strip any max budget annotations like "noop(max:5)" → "noop" + for i, t := range actualToolNames { + if name, _, found := strings.Cut(t, "("); found { + actualToolNames[i] = name + } + } + + assert.ElementsMatch(t, tt.expectedTools, actualToolNames, + "Tool names in should match expected set") + }) + } +} + +// TestBuildSafeOutputsSectionsCustomToolsConsistency verifies that every custom +// tool type registered in the runtime configuration has a corresponding entry in +// the compiled prompt block — preventing silent drift. +func TestBuildSafeOutputsSectionsCustomToolsConsistency(t *testing.T) { + config := &SafeOutputsConfig{ + NoOp: &NoOpConfig{}, + Jobs: map[string]*SafeJobConfig{ + "job-alpha": {Description: "Alpha job"}, + "job-beta": {Description: "Beta job"}, + }, + Scripts: map[string]*SafeScriptConfig{ + "script-one": {Description: "Script one"}, + }, + Actions: map[string]*SafeOutputActionConfig{ + "action-x": {Description: "Action X"}, + }, + } + + sections := buildSafeOutputsSections(config) + require.NotNil(t, sections, "Expected non-nil sections") + + // Concatenate all non-file section content to find the tools block + var toolsBuilder strings.Builder + for _, section := range sections { + if !section.IsFile { + toolsBuilder.WriteString(section.Content) + toolsBuilder.WriteString("\n") + } + } + toolsContent := toolsBuilder.String() + + // Every custom job name (normalized) must appear in the tools list + for jobName := range config.Jobs { + normalizedName := strings.ReplaceAll(jobName, "-", "_") + assert.Contains(t, toolsContent, normalizedName, + "Custom job %q (normalized: %q) should appear in ", jobName, normalizedName) + } + + // Every custom script name (normalized) must appear in the tools list + for scriptName := range config.Scripts { + normalizedName := strings.ReplaceAll(scriptName, "-", "_") + assert.Contains(t, toolsContent, normalizedName, + "Custom script %q (normalized: %q) should appear in ", scriptName, normalizedName) + } + + // Every custom action name (normalized) must appear in the tools list + for actionName := range config.Actions { + normalizedName := strings.ReplaceAll(actionName, "-", "_") + assert.Contains(t, toolsContent, normalizedName, + "Custom action %q (normalized: %q) should appear in ", actionName, normalizedName) + } +} diff --git a/pkg/workflow/unified_prompt_step.go b/pkg/workflow/unified_prompt_step.go index 4393356c332..a64ffd75f1e 100644 --- a/pkg/workflow/unified_prompt_step.go +++ b/pkg/workflow/unified_prompt_step.go @@ -7,6 +7,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" ) var unifiedPromptLog = logger.New("workflow:unified_prompt_step") @@ -663,6 +664,42 @@ func buildSafeOutputsSections(safeOutputs *SafeOutputsConfig) []PromptSection { tools = append(tools, toolWithMaxBudget("noop", safeOutputs.NoOp.Max)) } + // Add custom job tools from SafeOutputs.Jobs (sorted for deterministic output). + if len(safeOutputs.Jobs) > 0 { + jobNames := make([]string, 0, len(safeOutputs.Jobs)) + for jobName := range safeOutputs.Jobs { + jobNames = append(jobNames, jobName) + } + sort.Strings(jobNames) + for _, jobName := range jobNames { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(jobName)) + } + } + + // Add custom script tools from SafeOutputs.Scripts (sorted for deterministic output). + if len(safeOutputs.Scripts) > 0 { + scriptNames := make([]string, 0, len(safeOutputs.Scripts)) + for scriptName := range safeOutputs.Scripts { + scriptNames = append(scriptNames, scriptName) + } + sort.Strings(scriptNames) + for _, scriptName := range scriptNames { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(scriptName)) + } + } + + // Add custom action tools from SafeOutputs.Actions (sorted for deterministic output). + if len(safeOutputs.Actions) > 0 { + actionNames := make([]string, 0, len(safeOutputs.Actions)) + for actionName := range safeOutputs.Actions { + actionNames = append(actionNames, actionName) + } + sort.Strings(actionNames) + for _, actionName := range actionNames { + tools = append(tools, stringutil.NormalizeSafeOutputIdentifier(actionName)) + } + } + if len(tools) == 0 { return nil } From bac5ffa690b4e91d2415ee472cb3c18fbdcef4de Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 21:34:15 +0000 Subject: [PATCH 3/3] refactor: improve test assertions for safe-output-tools prompt tests - Use assert.Equal (order-sensitive) instead of assert.ElementsMatch so sorting/determinism test cases actually validate emission order - Rewrite consistency test to parse the exact Tools: line into a set of identifiers and assert membership, using stringutil.NormalizeSafeOutputIdentifier instead of strings.ReplaceAll to avoid false positives from substring matches - Extract helper extractToolNamesFromSections to avoid duplication Agent-Logs-Url: https://github.com/github/gh-aw/sessions/88d2ab26-99ab-40e0-8b1f-04dd925ae6a1 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- .github/workflows/daily-choice-test.lock.yml | 2 +- .../workflows/notion-issue-summary.lock.yml | 2 +- .../safe_outputs_prompt_tools_test.go | 107 ++++++++++-------- 3 files changed, 60 insertions(+), 51 deletions(-) diff --git a/.github/workflows/daily-choice-test.lock.yml b/.github/workflows/daily-choice-test.lock.yml index 5d86d660064..2baf46e263f 100644 --- a/.github/workflows/daily-choice-test.lock.yml +++ b/.github/workflows/daily-choice-test.lock.yml @@ -141,7 +141,7 @@ jobs: cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" cat << 'GH_AW_PROMPT_d8cfcd1ac4379461_EOF' - Tools: missing_tool, missing_data, noop + Tools: missing_tool, missing_data, noop, test_environment The following GitHub context information is available for this workflow: diff --git a/.github/workflows/notion-issue-summary.lock.yml b/.github/workflows/notion-issue-summary.lock.yml index 0775ec51670..cb6050abe0f 100644 --- a/.github/workflows/notion-issue-summary.lock.yml +++ b/.github/workflows/notion-issue-summary.lock.yml @@ -148,7 +148,7 @@ jobs: cat "${RUNNER_TEMP}/gh-aw/prompts/safe_outputs_prompt.md" cat << 'GH_AW_PROMPT_4451a47783df5d92_EOF' - Tools: missing_tool, missing_data, noop + Tools: missing_tool, missing_data, noop, notion_add_comment The following GitHub context information is available for this workflow: diff --git a/pkg/workflow/safe_outputs_prompt_tools_test.go b/pkg/workflow/safe_outputs_prompt_tools_test.go index 98ddef77d13..4592ef4a257 100644 --- a/pkg/workflow/safe_outputs_prompt_tools_test.go +++ b/pkg/workflow/safe_outputs_prompt_tools_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/github/gh-aw/pkg/stringutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -136,36 +137,10 @@ func TestBuildSafeOutputsSectionsCustomTools(t *testing.T) { require.NotNil(t, sections, "Expected non-nil sections") - // Find the opening section with the Tools: list - var toolsLine string - for _, section := range sections { - if !section.IsFile && strings.HasPrefix(section.Content, "") { - toolsLine = section.Content - break - } - } - - require.NotEmpty(t, toolsLine, "Expected to find opening section") - - // Extract tool names from "Tools: tool1, tool2, ..." line - lines := strings.Split(toolsLine, "\n") - require.GreaterOrEqual(t, len(lines), 2, "Expected at least two lines in tools section") + actualToolNames := extractToolNamesFromSections(t, sections) - toolsListLine := lines[1] - assert.True(t, strings.HasPrefix(toolsListLine, "Tools: "), "Second line should start with 'Tools: '") - - toolsList := strings.TrimPrefix(toolsListLine, "Tools: ") - actualToolNames := strings.Split(toolsList, ", ") - - // Strip any max budget annotations like "noop(max:5)" → "noop" - for i, t := range actualToolNames { - if name, _, found := strings.Cut(t, "("); found { - actualToolNames[i] = name - } - } - - assert.ElementsMatch(t, tt.expectedTools, actualToolNames, - "Tool names in should match expected set") + assert.Equal(t, tt.expectedTools, actualToolNames, + "Tool names in should match expected order and set") }) } } @@ -191,34 +166,68 @@ func TestBuildSafeOutputsSectionsCustomToolsConsistency(t *testing.T) { sections := buildSafeOutputsSections(config) require.NotNil(t, sections, "Expected non-nil sections") - // Concatenate all non-file section content to find the tools block - var toolsBuilder strings.Builder - for _, section := range sections { - if !section.IsFile { - toolsBuilder.WriteString(section.Content) - toolsBuilder.WriteString("\n") - } + actualToolNames := extractToolNamesFromSections(t, sections) + actualToolSet := make(map[string]bool, len(actualToolNames)) + for _, name := range actualToolNames { + actualToolSet[name] = true } - toolsContent := toolsBuilder.String() - // Every custom job name (normalized) must appear in the tools list + // Every custom job name (normalized) must appear as an exact tool identifier. for jobName := range config.Jobs { - normalizedName := strings.ReplaceAll(jobName, "-", "_") - assert.Contains(t, toolsContent, normalizedName, - "Custom job %q (normalized: %q) should appear in ", jobName, normalizedName) + normalized := stringutil.NormalizeSafeOutputIdentifier(jobName) + assert.True(t, actualToolSet[normalized], + "Custom job %q (normalized: %q) should appear as an exact tool identifier in ", jobName, normalized) } - // Every custom script name (normalized) must appear in the tools list + // Every custom script name (normalized) must appear as an exact tool identifier. for scriptName := range config.Scripts { - normalizedName := strings.ReplaceAll(scriptName, "-", "_") - assert.Contains(t, toolsContent, normalizedName, - "Custom script %q (normalized: %q) should appear in ", scriptName, normalizedName) + normalized := stringutil.NormalizeSafeOutputIdentifier(scriptName) + assert.True(t, actualToolSet[normalized], + "Custom script %q (normalized: %q) should appear as an exact tool identifier in ", scriptName, normalized) } - // Every custom action name (normalized) must appear in the tools list + // Every custom action name (normalized) must appear as an exact tool identifier. for actionName := range config.Actions { - normalizedName := strings.ReplaceAll(actionName, "-", "_") - assert.Contains(t, toolsContent, normalizedName, - "Custom action %q (normalized: %q) should appear in ", actionName, normalizedName) + normalized := stringutil.NormalizeSafeOutputIdentifier(actionName) + assert.True(t, actualToolSet[normalized], + "Custom action %q (normalized: %q) should appear as an exact tool identifier in ", actionName, normalized) + } +} + +// extractToolNamesFromSections parses the opening section and returns +// the list of tool names in the order they appear, stripping any max-budget annotations +// (e.g. "noop(max:5)" → "noop"). +func extractToolNamesFromSections(t *testing.T, sections []PromptSection) []string { + t.Helper() + + var toolsLine string + for _, section := range sections { + if !section.IsFile && strings.HasPrefix(section.Content, "") { + toolsLine = section.Content + break + } + } + + require.NotEmpty(t, toolsLine, "Expected to find opening section") + + lines := strings.Split(toolsLine, "\n") + require.GreaterOrEqual(t, len(lines), 2, "Expected at least two lines in tools section") + + toolsListLine := lines[1] + require.True(t, strings.HasPrefix(toolsListLine, "Tools: "), + "Second line should start with 'Tools: ', got: %q", toolsListLine) + + toolsList := strings.TrimPrefix(toolsListLine, "Tools: ") + toolEntries := strings.Split(toolsList, ", ") + + names := make([]string, 0, len(toolEntries)) + for _, entry := range toolEntries { + // Strip optional budget annotation: "noop(max:5)" → "noop" + if name, _, found := strings.Cut(entry, "("); found { + names = append(names, name) + } else { + names = append(names, entry) + } } + return names }