diff --git a/pkg/workflow/config_helpers.go b/pkg/workflow/config_helpers.go index e2b7154b12..34adcf41e7 100644 --- a/pkg/workflow/config_helpers.go +++ b/pkg/workflow/config_helpers.go @@ -55,19 +55,10 @@ func ParseStringArrayFromConfig(m map[string]any, key string, log *logger.Logger if log != nil { log.Printf("Parsing %s from config", key) } - if arrayValue, ok := value.([]any); ok { - var strings []string - for _, item := range arrayValue { - if strVal, ok := item.(string); ok { - strings = append(strings, strVal) - } - } + if strings := parseStringSliceAny(value, log); strings != nil { // Return the slice even if empty (to distinguish from not provided) - if strings == nil { - if log != nil { - log.Printf("No valid %s strings found, returning empty array", key) - } - return []string{} + if len(strings) == 0 && log != nil { + log.Printf("No valid %s strings found, returning empty array", key) } if log != nil { log.Printf("Parsed %d %s from config", len(strings), key) diff --git a/pkg/workflow/config_parsing_helpers_test.go b/pkg/workflow/config_parsing_helpers_test.go index bad24e1705..86413f9e7e 100644 --- a/pkg/workflow/config_parsing_helpers_test.go +++ b/pkg/workflow/config_parsing_helpers_test.go @@ -121,6 +121,13 @@ func TestParseLabelsFromConfig(t *testing.T) { }, expected: []string{"bug", "enhancement", "documentation"}, }, + { + name: "labels as []string", + input: map[string]any{ + "labels": []string{"bug", "enhancement"}, + }, + expected: []string{"bug", "enhancement"}, + }, { name: "labels as non-array type", input: map[string]any{ diff --git a/pkg/workflow/missing_data.go b/pkg/workflow/missing_data.go deleted file mode 100644 index c81eb32a15..0000000000 --- a/pkg/workflow/missing_data.go +++ /dev/null @@ -1,11 +0,0 @@ -package workflow - -import ( - "github.com/github/gh-aw/pkg/logger" -) - -var missingDataLog = logger.New("workflow:missing_data") - -func (c *Compiler) parseMissingDataConfig(outputMap map[string]any) *MissingDataConfig { - return c.parseIssueReportingConfig(outputMap, "missing-data", "[missing data]", missingDataLog) -} diff --git a/pkg/workflow/missing_issue_reporting.go b/pkg/workflow/missing_issue_reporting.go index 4abf71fcef..49ec5890b0 100644 --- a/pkg/workflow/missing_issue_reporting.go +++ b/pkg/workflow/missing_issue_reporting.go @@ -4,6 +4,10 @@ import ( "github.com/github/gh-aw/pkg/logger" ) +var missingDataLog = logger.New("workflow:missing_data") +var missingToolLog = logger.New("workflow:missing_tool") +var reportIncompleteLog = logger.New("workflow:report_incomplete") + // IssueReportingConfig holds configuration shared by safe-output types that create GitHub issues // (missing-data and missing-tool). Both types have identical fields; the yaml tags on the // parent struct fields give them their distinct YAML keys. @@ -19,6 +23,34 @@ type IssueReportingConfig struct { type MissingDataConfig = IssueReportingConfig type MissingToolConfig = IssueReportingConfig +// ReportIncompleteConfig holds configuration for the report_incomplete safe output. +// report_incomplete is a structured signal that the agent could not complete its +// assigned task due to an infrastructure or tool failure (e.g., MCP server crash, +// missing authentication, inaccessible repository). +// +// When an agent emits report_incomplete, gh-aw activates failure handling even +// when the agent process exits 0 and other safe outputs were also emitted. +// This prevents semantically-empty outputs (e.g., a comment describing tool +// failures) from being classified as a successful result. +// +// ReportIncompleteConfig is a type alias for IssueReportingConfig so that it +// supports the same create-issue, title-prefix, and labels configuration fields +// as missing-tool and missing-data. +type ReportIncompleteConfig = IssueReportingConfig + +func (c *Compiler) parseMissingDataConfig(outputMap map[string]any) *MissingDataConfig { + return c.parseIssueReportingConfig(outputMap, "missing-data", "[missing data]", missingDataLog) +} + +func (c *Compiler) parseMissingToolConfig(outputMap map[string]any) *MissingToolConfig { + return c.parseIssueReportingConfig(outputMap, "missing-tool", "[missing tool]", missingToolLog) +} + +// parseReportIncompleteConfig handles report_incomplete configuration. +func (c *Compiler) parseReportIncompleteConfig(outputMap map[string]any) *ReportIncompleteConfig { + return c.parseIssueReportingConfig(outputMap, "report-incomplete", "[incomplete]", reportIncompleteLog) +} + func (c *Compiler) parseIssueReportingConfig(outputMap map[string]any, yamlKey, defaultTitle string, log *logger.Logger) *IssueReportingConfig { configData, exists := outputMap[yamlKey] if !exists { @@ -64,17 +96,9 @@ func (c *Compiler) parseIssueReportingConfig(outputMap map[string]any, yamlKey, cfg.TitlePrefix = defaultTitle } - if labels, exists := configMap["labels"]; exists { - if labelsArray, ok := labels.([]any); ok { - var labelStrings []string - for _, label := range labelsArray { - if labelStr, ok := label.(string); ok { - labelStrings = append(labelStrings, labelStr) - } - } - cfg.Labels = labelStrings - log.Printf("labels: %v", labelStrings) - } + if _, exists := configMap["labels"]; exists { + cfg.Labels = ParseStringArrayFromConfig(configMap, "labels", log) + log.Printf("labels: %v", cfg.Labels) } else { cfg.Labels = []string{} } diff --git a/pkg/workflow/missing_tool.go b/pkg/workflow/missing_tool.go deleted file mode 100644 index 196644f53e..0000000000 --- a/pkg/workflow/missing_tool.go +++ /dev/null @@ -1,11 +0,0 @@ -package workflow - -import ( - "github.com/github/gh-aw/pkg/logger" -) - -var missingToolLog = logger.New("workflow:missing_tool") - -func (c *Compiler) parseMissingToolConfig(outputMap map[string]any) *MissingToolConfig { - return c.parseIssueReportingConfig(outputMap, "missing-tool", "[missing tool]", missingToolLog) -} diff --git a/pkg/workflow/missing_tool_test.go b/pkg/workflow/missing_tool_test.go index c7d9d60b6f..a5ee575480 100644 --- a/pkg/workflow/missing_tool_test.go +++ b/pkg/workflow/missing_tool_test.go @@ -283,6 +283,18 @@ func TestMissingToolConfigParsing(t *testing.T) { expectTitlePrefix: "[missing tool]", expectLabels: []string{"bug", "enhancement", "missing-tool"}, }, + { + name: "Custom labels as []string", + configData: map[string]any{ + "missing-tool": map[string]any{ + "labels": []string{"bug", "enhancement", "missing-tool"}, + }, + }, + expectMax: 0, + expectCreateIssue: true, + expectTitlePrefix: "[missing tool]", + expectLabels: []string{"bug", "enhancement", "missing-tool"}, + }, { name: "Full configuration", configData: map[string]any{ diff --git a/pkg/workflow/report_incomplete.go b/pkg/workflow/report_incomplete.go deleted file mode 100644 index 478ba3f0fe..0000000000 --- a/pkg/workflow/report_incomplete.go +++ /dev/null @@ -1,27 +0,0 @@ -package workflow - -import ( - "github.com/github/gh-aw/pkg/logger" -) - -var reportIncompleteLog = logger.New("workflow:report_incomplete") - -// ReportIncompleteConfig holds configuration for the report_incomplete safe output. -// report_incomplete is a structured signal that the agent could not complete its -// assigned task due to an infrastructure or tool failure (e.g., MCP server crash, -// missing authentication, inaccessible repository). -// -// When an agent emits report_incomplete, gh-aw activates failure handling even -// when the agent process exits 0 and other safe outputs were also emitted. -// This prevents semantically-empty outputs (e.g., a comment describing tool -// failures) from being classified as a successful result. -// -// ReportIncompleteConfig is a type alias for IssueReportingConfig so that it -// supports the same create-issue, title-prefix, and labels configuration fields -// as missing-tool and missing-data. -type ReportIncompleteConfig = IssueReportingConfig - -// parseReportIncompleteConfig handles report_incomplete configuration. -func (c *Compiler) parseReportIncompleteConfig(outputMap map[string]any) *ReportIncompleteConfig { - return c.parseIssueReportingConfig(outputMap, "report-incomplete", "[incomplete]", reportIncompleteLog) -} diff --git a/pkg/workflow/safe_outputs_cross_repo_config_test.go b/pkg/workflow/safe_outputs_cross_repo_config_test.go index cb7098f186..7603f41651 100644 --- a/pkg/workflow/safe_outputs_cross_repo_config_test.go +++ b/pkg/workflow/safe_outputs_cross_repo_config_test.go @@ -539,6 +539,13 @@ func TestParseAllowedReposFromConfig(t *testing.T) { }, expected: []string{"owner/repo1", "owner/repo2", "other-owner/repo3"}, }, + { + name: "allowed-repos as []string", + input: map[string]any{ + "allowed-repos": []string{"owner/repo1", "owner/repo2"}, + }, + expected: []string{"owner/repo1", "owner/repo2"}, + }, { name: "no allowed-repos key", input: map[string]any{},