diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 605160e0f0d..8a07a5702e9 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -10,455 +10,8 @@ func (c *Compiler) addHandlerManagerConfigEnvVar(steps *[]string, data *Workflow return } - config := make(map[string]map[string]any) - - // Add config for each enabled safe output type with their options - // Presence in config = enabled, so no need for "enabled": true field - if data.SafeOutputs.CreateIssues != nil { - cfg := data.SafeOutputs.CreateIssues - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if len(cfg.AllowedLabels) > 0 { - handlerConfig["allowed_labels"] = cfg.AllowedLabels - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - if cfg.Expires > 0 { - handlerConfig["expires"] = cfg.Expires - } - // Add labels, title_prefix to config - if len(cfg.Labels) > 0 { - handlerConfig["labels"] = cfg.Labels - } - if cfg.TitlePrefix != "" { - handlerConfig["title_prefix"] = cfg.TitlePrefix - } - // Add assignees to config - if len(cfg.Assignees) > 0 { - handlerConfig["assignees"] = cfg.Assignees - } - // Add target-repo to config - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - config["create_issue"] = handlerConfig - } - - if data.SafeOutputs.AddComments != nil { - cfg := data.SafeOutputs.AddComments - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if cfg.HideOlderComments { - handlerConfig["hide_older_comments"] = true - } - // Add target-repo to config - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["add_comment"] = handlerConfig - } - - if data.SafeOutputs.CreateDiscussions != nil { - cfg := data.SafeOutputs.CreateDiscussions - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Category != "" { - handlerConfig["category"] = cfg.Category - } - if cfg.TitlePrefix != "" { - handlerConfig["title_prefix"] = cfg.TitlePrefix - } - if len(cfg.Labels) > 0 { - handlerConfig["labels"] = cfg.Labels - } - if len(cfg.AllowedLabels) > 0 { - handlerConfig["allowed_labels"] = cfg.AllowedLabels - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - if cfg.CloseOlderDiscussions { - handlerConfig["close_older_discussions"] = true - } - if cfg.Expires > 0 { - handlerConfig["expires"] = cfg.Expires - } - // Add target-repo to config - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - config["create_discussion"] = handlerConfig - } - - if data.SafeOutputs.CloseIssues != nil { - cfg := data.SafeOutputs.CloseIssues - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if len(cfg.RequiredLabels) > 0 { - handlerConfig["required_labels"] = cfg.RequiredLabels - } - if cfg.RequiredTitlePrefix != "" { - handlerConfig["required_title_prefix"] = cfg.RequiredTitlePrefix - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["close_issue"] = handlerConfig - } - - if data.SafeOutputs.CloseDiscussions != nil { - cfg := data.SafeOutputs.CloseDiscussions - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if len(cfg.RequiredLabels) > 0 { - handlerConfig["required_labels"] = cfg.RequiredLabels - } - if cfg.RequiredTitlePrefix != "" { - handlerConfig["required_title_prefix"] = cfg.RequiredTitlePrefix - } - if cfg.RequiredCategory != "" { - handlerConfig["required_category"] = cfg.RequiredCategory - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["close_discussion"] = handlerConfig - } - - if data.SafeOutputs.AddLabels != nil { - cfg := data.SafeOutputs.AddLabels - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if len(cfg.Allowed) > 0 { - handlerConfig["allowed"] = cfg.Allowed - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["add_labels"] = handlerConfig - } - - if data.SafeOutputs.UpdateIssues != nil { - cfg := data.SafeOutputs.UpdateIssues - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - // Boolean pointer fields indicate which fields can be updated - if cfg.Status != nil { - handlerConfig["allow_status"] = true - } - if cfg.Title != nil { - handlerConfig["allow_title"] = true - } - if cfg.Body != nil { - handlerConfig["allow_body"] = true - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["update_issue"] = handlerConfig - } - - if data.SafeOutputs.UpdateDiscussions != nil { - cfg := data.SafeOutputs.UpdateDiscussions - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - // Boolean pointer fields indicate which fields can be updated - if cfg.Title != nil { - handlerConfig["allow_title"] = true - } - if cfg.Body != nil { - handlerConfig["allow_body"] = true - } - if cfg.Labels != nil { - handlerConfig["allow_labels"] = true - } - if len(cfg.AllowedLabels) > 0 { - handlerConfig["allowed_labels"] = cfg.AllowedLabels - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["update_discussion"] = handlerConfig - } - - if data.SafeOutputs.LinkSubIssue != nil { - cfg := data.SafeOutputs.LinkSubIssue - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if len(cfg.ParentRequiredLabels) > 0 { - handlerConfig["parent_required_labels"] = cfg.ParentRequiredLabels - } - if cfg.ParentTitlePrefix != "" { - handlerConfig["parent_title_prefix"] = cfg.ParentTitlePrefix - } - if len(cfg.SubRequiredLabels) > 0 { - handlerConfig["sub_required_labels"] = cfg.SubRequiredLabels - } - if cfg.SubTitlePrefix != "" { - handlerConfig["sub_title_prefix"] = cfg.SubTitlePrefix - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["link_sub_issue"] = handlerConfig - } - - if data.SafeOutputs.UpdateRelease != nil { - cfg := data.SafeOutputs.UpdateRelease - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - config["update_release"] = handlerConfig - } - - if data.SafeOutputs.CreatePullRequestReviewComments != nil { - cfg := data.SafeOutputs.CreatePullRequestReviewComments - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Side != "" { - handlerConfig["side"] = cfg.Side - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["create_pull_request_review_comment"] = handlerConfig - } - - if data.SafeOutputs.CreatePullRequests != nil { - cfg := data.SafeOutputs.CreatePullRequests - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.TitlePrefix != "" { - handlerConfig["title_prefix"] = cfg.TitlePrefix - } - if len(cfg.Labels) > 0 { - handlerConfig["labels"] = cfg.Labels - } - if cfg.Draft != nil { - handlerConfig["draft"] = *cfg.Draft - } - if cfg.IfNoChanges != "" { - handlerConfig["if_no_changes"] = cfg.IfNoChanges - } - if cfg.AllowEmpty { - handlerConfig["allow_empty"] = cfg.AllowEmpty - } - if cfg.Expires > 0 { - handlerConfig["expires"] = cfg.Expires - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - // Add base branch (required for git operations) - handlerConfig["base_branch"] = "${{ github.ref_name }}" - // Add max patch size - maxPatchSize := 1024 // default 1024 KB - if data.SafeOutputs.MaximumPatchSize > 0 { - maxPatchSize = data.SafeOutputs.MaximumPatchSize - } - handlerConfig["max_patch_size"] = maxPatchSize - config["create_pull_request"] = handlerConfig - } - - if data.SafeOutputs.PushToPullRequestBranch != nil { - cfg := data.SafeOutputs.PushToPullRequestBranch - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if cfg.TitlePrefix != "" { - handlerConfig["title_prefix"] = cfg.TitlePrefix - } - if len(cfg.Labels) > 0 { - handlerConfig["labels"] = cfg.Labels - } - if cfg.IfNoChanges != "" { - handlerConfig["if_no_changes"] = cfg.IfNoChanges - } - if cfg.CommitTitleSuffix != "" { - handlerConfig["commit_title_suffix"] = cfg.CommitTitleSuffix - } - // Add base branch (required for git operations) - handlerConfig["base_branch"] = "${{ github.ref_name }}" - // Add max patch size - maxPatchSize := 1024 // default 1024 KB - if data.SafeOutputs.MaximumPatchSize > 0 { - maxPatchSize = data.SafeOutputs.MaximumPatchSize - } - handlerConfig["max_patch_size"] = maxPatchSize - config["push_to_pull_request_branch"] = handlerConfig - } - - if data.SafeOutputs.UpdatePullRequests != nil { - cfg := data.SafeOutputs.UpdatePullRequests - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - // Boolean pointer fields indicate which fields can be updated - // Default to true if not specified (backward compatibility) - if cfg.Title != nil { - handlerConfig["allow_title"] = *cfg.Title - } else { - handlerConfig["allow_title"] = true - } - if cfg.Body != nil { - handlerConfig["allow_body"] = *cfg.Body - } else { - handlerConfig["allow_body"] = true - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["update_pull_request"] = handlerConfig - } - - if data.SafeOutputs.ClosePullRequests != nil { - cfg := data.SafeOutputs.ClosePullRequests - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.Target != "" { - handlerConfig["target"] = cfg.Target - } - if len(cfg.RequiredLabels) > 0 { - handlerConfig["required_labels"] = cfg.RequiredLabels - } - if cfg.RequiredTitlePrefix != "" { - handlerConfig["required_title_prefix"] = cfg.RequiredTitlePrefix - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["close_pull_request"] = handlerConfig - } - - if data.SafeOutputs.HideComment != nil { - cfg := data.SafeOutputs.HideComment - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if len(cfg.AllowedReasons) > 0 { - handlerConfig["allowed_reasons"] = cfg.AllowedReasons - } - if cfg.TargetRepoSlug != "" { - handlerConfig["target-repo"] = cfg.TargetRepoSlug - } - if len(cfg.AllowedRepos) > 0 { - handlerConfig["allowed_repos"] = cfg.AllowedRepos - } - config["hide_comment"] = handlerConfig - } - - if data.SafeOutputs.DispatchWorkflow != nil { - cfg := data.SafeOutputs.DispatchWorkflow - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if len(cfg.Workflows) > 0 { - handlerConfig["workflows"] = cfg.Workflows - } - config["dispatch_workflow"] = handlerConfig - } - - if data.SafeOutputs.CreateProjectStatusUpdates != nil { - cfg := data.SafeOutputs.CreateProjectStatusUpdates - handlerConfig := make(map[string]any) - if cfg.Max > 0 { - handlerConfig["max"] = cfg.Max - } - if cfg.GitHubToken != "" { - handlerConfig["github-token"] = cfg.GitHubToken - } - config["create_project_status_update"] = handlerConfig - } + // Use the new generic builder instead of manual duplication + config := buildSafeOutputConfigs(data.SafeOutputs) // Only add the env var if there are handlers to configure if len(config) > 0 { diff --git a/pkg/workflow/safe_output_config_builder.go b/pkg/workflow/safe_output_config_builder.go new file mode 100644 index 00000000000..3f801b585c4 --- /dev/null +++ b/pkg/workflow/safe_output_config_builder.go @@ -0,0 +1,256 @@ +package workflow + +import ( + "reflect" + + "github.com/githubnext/gh-aw/pkg/logger" +) + +var safeOutputConfigBuilderLog = logger.New("workflow:safe_output_config_builder") + +// fieldMapping defines how struct field names map to handler config keys +var fieldMapping = map[string]string{ + "Max": "max", + "GitHubToken": "github-token", + "TitlePrefix": "title_prefix", + "Labels": "labels", + "AllowedLabels": "allowed_labels", + "Assignees": "assignees", + "TargetRepoSlug": "target-repo", + "AllowedRepos": "allowed_repos", + "Expires": "expires", + "Target": "target", + "HideOlderComments": "hide_older_comments", + "Category": "category", + "CloseOlderDiscussions": "close_older_discussions", + "RequiredLabels": "required_labels", + "RequiredTitlePrefix": "required_title_prefix", + "RequiredCategory": "required_category", + "Allowed": "allowed", + "Status": "allow_status", + "Title": "allow_title", + "Body": "allow_body", + "ParentRequiredLabels": "parent_required_labels", + "ParentTitlePrefix": "parent_title_prefix", + "SubRequiredLabels": "sub_required_labels", + "SubTitlePrefix": "sub_title_prefix", + "Side": "side", + "Draft": "draft", + "IfNoChanges": "if_no_changes", + "AllowEmpty": "allow_empty", + "CommitTitleSuffix": "commit_title_suffix", + "AllowedReasons": "allowed_reasons", + "Workflows": "workflows", + "Reviewers": "reviewers", + "DefaultAgent": "default_agent", + "Discussion": "discussion", +} + +// buildHandlerConfig builds a handler configuration map from a config struct using reflection. +// It automatically handles common patterns like: +// - Skipping zero values (empty strings, 0, nil slices, nil pointers) +// - Converting boolean pointers to their presence (for allow_* fields) +// - Mapping Go struct field names to handler config keys +// - Processing embedded structs recursively +func buildHandlerConfig(configStruct any) map[string]any { + handlerConfig := make(map[string]any) + + v := reflect.ValueOf(configStruct) + if v.Kind() == reflect.Ptr { + if v.IsNil() { + return handlerConfig + } + v = v.Elem() + } + + if v.Kind() != reflect.Struct { + safeOutputConfigBuilderLog.Printf("Expected struct, got %v", v.Kind()) + return handlerConfig + } + + buildHandlerConfigRecursive(v, handlerConfig) + return handlerConfig +} + +// buildHandlerConfigRecursive processes struct fields recursively, handling embedded structs +func buildHandlerConfigRecursive(v reflect.Value, handlerConfig map[string]any) { + t := v.Type() + + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + fieldType := t.Field(i) + + // Skip unexported fields + if !fieldType.IsExported() { + continue + } + + // Handle embedded structs by recursing into them + if fieldType.Anonymous { + if field.Kind() == reflect.Struct { + buildHandlerConfigRecursive(field, handlerConfig) + } + continue + } + + // Get the field name and look up the mapping + fieldName := fieldType.Name + configKey, exists := fieldMapping[fieldName] + if !exists { + // If no mapping exists, skip this field + continue + } + + // Handle different field types + switch field.Kind() { + case reflect.String: + str := field.String() + if str != "" { + handlerConfig[configKey] = str + } + + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + intVal := field.Int() + if intVal > 0 { + handlerConfig[configKey] = intVal + } + + case reflect.Bool: + boolVal := field.Bool() + if boolVal { + handlerConfig[configKey] = true + } + + case reflect.Slice: + if !field.IsNil() && field.Len() > 0 { + // Convert slice to []any for JSON marshaling + slice := make([]any, field.Len()) + for j := 0; j < field.Len(); j++ { + slice[j] = field.Index(j).Interface() + } + handlerConfig[configKey] = slice + } + + case reflect.Ptr: + if !field.IsNil() { + // For pointer fields, check the type they point to + elem := field.Elem() + switch elem.Kind() { + case reflect.Bool: + // Boolean pointers indicate "allow_*" fields + // For these, we just set the key to true if the pointer exists + handlerConfig[configKey] = true + default: + // For other pointer types, use their underlying value + handlerConfig[configKey] = elem.Interface() + } + } + + default: + safeOutputConfigBuilderLog.Printf("Unhandled field type %v for field %s", field.Kind(), fieldName) + } + } +} + +// safeOutputRegistry maps safe output types to their handler names +type safeOutputHandler struct { + fieldName string + handlerName string + customizer func(config any, handlerConfig map[string]any) // Optional customizer for special cases +} + +var safeOutputHandlers = []safeOutputHandler{ + {fieldName: "CreateIssues", handlerName: "create_issue"}, + {fieldName: "AddComments", handlerName: "add_comment"}, + {fieldName: "CreateDiscussions", handlerName: "create_discussion"}, + {fieldName: "CloseIssues", handlerName: "close_issue"}, + {fieldName: "CloseDiscussions", handlerName: "close_discussion"}, + {fieldName: "AddLabels", handlerName: "add_labels"}, + {fieldName: "UpdateIssues", handlerName: "update_issue"}, + {fieldName: "UpdateDiscussions", handlerName: "update_discussion"}, + {fieldName: "LinkSubIssue", handlerName: "link_sub_issue"}, + {fieldName: "UpdateRelease", handlerName: "update_release"}, + {fieldName: "CreatePullRequestReviewComments", handlerName: "create_pull_request_review_comment"}, + { + fieldName: "CreatePullRequests", + handlerName: "create_pull_request", + customizer: func(config any, handlerConfig map[string]any) { + // Add base branch for git operations + handlerConfig["base_branch"] = "${{ github.ref_name }}" + }, + }, + { + fieldName: "PushToPullRequestBranch", + handlerName: "push_to_pull_request_branch", + customizer: func(config any, handlerConfig map[string]any) { + // Add base branch for git operations + handlerConfig["base_branch"] = "${{ github.ref_name }}" + }, + }, + {fieldName: "UpdatePullRequests", handlerName: "update_pull_request", customizer: updatePullRequestCustomizer}, + {fieldName: "ClosePullRequests", handlerName: "close_pull_request"}, + {fieldName: "HideComment", handlerName: "hide_comment"}, + {fieldName: "DispatchWorkflow", handlerName: "dispatch_workflow"}, + {fieldName: "CreateProjectStatusUpdates", handlerName: "create_project_status_update"}, +} + +// updatePullRequestCustomizer handles special defaulting logic for update_pull_request +func updatePullRequestCustomizer(config any, handlerConfig map[string]any) { + // For update_pull_request, default to true if not specified + if _, exists := handlerConfig["allow_title"]; !exists { + handlerConfig["allow_title"] = true + } + if _, exists := handlerConfig["allow_body"]; !exists { + handlerConfig["allow_body"] = true + } +} + +// buildSafeOutputConfigs builds handler configuration for all safe output types using reflection. +// This replaces the massive duplication in addHandlerManagerConfigEnvVar. +func buildSafeOutputConfigs(safeOutputs *SafeOutputsConfig) map[string]map[string]any { + config := make(map[string]map[string]any) + + if safeOutputs == nil { + return config + } + + v := reflect.ValueOf(safeOutputs).Elem() + + // Process each registered handler + for _, handler := range safeOutputHandlers { + field := v.FieldByName(handler.fieldName) + if !field.IsValid() || field.IsNil() { + continue + } + + // Build the handler config using reflection + handlerConfig := buildHandlerConfig(field.Interface()) + + // Apply any custom logic + if handler.customizer != nil { + handler.customizer(field.Interface(), handlerConfig) + } + + // Add even if empty - the presence of the config pointer indicates the handler is enabled + config[handler.handlerName] = handlerConfig + } + + // Handle max_patch_size for PR-related handlers + if safeOutputs.MaximumPatchSize > 0 { + for _, handlerName := range []string{"create_pull_request", "push_to_pull_request_branch"} { + if cfg, exists := config[handlerName]; exists { + cfg["max_patch_size"] = safeOutputs.MaximumPatchSize + } + } + } else if safeOutputs.CreatePullRequests != nil || safeOutputs.PushToPullRequestBranch != nil { + // Add default max_patch_size + defaultSize := 1024 + for _, handlerName := range []string{"create_pull_request", "push_to_pull_request_branch"} { + if cfg, exists := config[handlerName]; exists { + cfg["max_patch_size"] = defaultSize + } + } + } + + return config +} diff --git a/pkg/workflow/safe_output_config_builder_test.go b/pkg/workflow/safe_output_config_builder_test.go new file mode 100644 index 00000000000..9e1b332e63d --- /dev/null +++ b/pkg/workflow/safe_output_config_builder_test.go @@ -0,0 +1,314 @@ +package workflow + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildHandlerConfig(t *testing.T) { + tests := []struct { + name string + input any + expected map[string]any + }{ + { + name: "CreateIssuesConfig with all fields", + input: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 5, + }, + TitlePrefix: "[AI] ", + Labels: []string{"bug", "ai"}, + AllowedLabels: []string{"bug", "feature"}, + Assignees: []string{"user1", "user2"}, + TargetRepoSlug: "owner/repo", + AllowedRepos: []string{"org/repo1", "org/repo2"}, + Expires: 72, + }, + expected: map[string]any{ + "max": int64(5), + "title_prefix": "[AI] ", + "labels": []any{"bug", "ai"}, + "allowed_labels": []any{"bug", "feature"}, + "assignees": []any{"user1", "user2"}, + "target-repo": "owner/repo", + "allowed_repos": []any{"org/repo1", "org/repo2"}, + "expires": int64(72), + }, + }, + { + name: "AddCommentsConfig with target and hide older", + input: &AddCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 3, + }, + Target: "issue", + HideOlderComments: true, + TargetRepoSlug: "owner/repo", + AllowedRepos: []string{"owner/repo2"}, + }, + expected: map[string]any{ + "max": int64(3), + "target": "issue", + "hide_older_comments": true, + "target-repo": "owner/repo", + "allowed_repos": []any{"owner/repo2"}, + }, + }, + { + name: "Config with zero values omitted", + input: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 0, // Should be omitted + }, + TitlePrefix: "", // Should be omitted + Labels: []string{}, // Should be omitted + }, + expected: map[string]any{}, + }, + { + name: "UpdateIssuesConfig with boolean pointers", + input: &UpdateIssuesConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 1, + }, + SafeOutputTargetConfig: SafeOutputTargetConfig{ + Target: "issue", + }, + }, + Status: testBoolPtr(true), + Title: testBoolPtr(true), + Body: testBoolPtr(true), + }, + expected: map[string]any{ + "max": int64(1), + "target": "issue", + "allow_status": true, + "allow_title": true, + "allow_body": true, + }, + }, + { + name: "CloseEntityConfig with required fields", + input: &CloseEntityConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 10, + }, + SafeOutputTargetConfig: SafeOutputTargetConfig{ + Target: "issue", + }, + SafeOutputFilterConfig: SafeOutputFilterConfig{ + RequiredLabels: []string{"stale"}, + RequiredTitlePrefix: "[OLD]", + }, + }, + expected: map[string]any{ + "max": int64(10), + "target": "issue", + "required_labels": []any{"stale"}, + "required_title_prefix": "[OLD]", + }, + }, + { + name: "Nil pointer returns empty config", + input: (*CreateIssuesConfig)(nil), + expected: map[string]any{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := buildHandlerConfig(tt.input) + assert.Equal(t, tt.expected, result, "Handler config should match expected") + }) + } +} + +func TestBuildSafeOutputConfigs(t *testing.T) { + tests := []struct { + name string + safeOutputs *SafeOutputsConfig + expectedKeys []string + validateField func(t *testing.T, config map[string]map[string]any) + }{ + { + name: "Single handler - CreateIssues", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 5, + }, + TitlePrefix: "[AI] ", + AllowedLabels: []string{"bug"}, + }, + }, + expectedKeys: []string{"create_issue"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + issueConfig, exists := config["create_issue"] + require.True(t, exists, "create_issue should exist") + assert.Equal(t, int64(5), issueConfig["max"]) + assert.Equal(t, "[AI] ", issueConfig["title_prefix"]) + assert.Equal(t, []any{"bug"}, issueConfig["allowed_labels"]) + }, + }, + { + name: "Multiple handlers", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 5, + }, + }, + AddComments: &AddCommentsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 3, + }, + Target: "issue", + }, + AddLabels: &AddLabelsConfig{ + Allowed: []string{"bug", "feature"}, + }, + }, + expectedKeys: []string{"create_issue", "add_comment", "add_labels"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + assert.Len(t, config, 3, "Should have 3 handlers") + + issueConfig := config["create_issue"] + assert.Equal(t, int64(5), issueConfig["max"]) + + commentConfig := config["add_comment"] + assert.Equal(t, int64(3), commentConfig["max"]) + assert.Equal(t, "issue", commentConfig["target"]) + + labelConfig := config["add_labels"] + assert.Equal(t, []any{"bug", "feature"}, labelConfig["allowed"]) + }, + }, + { + name: "CreatePullRequests with base_branch customization", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 1, + }, + TitlePrefix: "[PR] ", + }, + }, + expectedKeys: []string{"create_pull_request"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + prConfig, exists := config["create_pull_request"] + require.True(t, exists) + assert.Equal(t, "${{ github.ref_name }}", prConfig["base_branch"], "Should add base_branch") + assert.Equal(t, 1024, prConfig["max_patch_size"], "Should add default max_patch_size") + }, + }, + { + name: "UpdatePullRequests with default allow fields", + safeOutputs: &SafeOutputsConfig{ + UpdatePullRequests: &UpdatePullRequestsConfig{ + UpdateEntityConfig: UpdateEntityConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 1, + }, + SafeOutputTargetConfig: SafeOutputTargetConfig{ + Target: "pr", + }, + }, + }, + }, + expectedKeys: []string{"update_pull_request"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + prConfig, exists := config["update_pull_request"] + require.True(t, exists) + assert.Equal(t, true, prConfig["allow_title"], "Should default to true") + assert.Equal(t, true, prConfig["allow_body"], "Should default to true") + }, + }, + { + name: "Custom MaximumPatchSize", + safeOutputs: &SafeOutputsConfig{ + CreatePullRequests: &CreatePullRequestsConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 1, + }, + }, + MaximumPatchSize: 2048, + }, + expectedKeys: []string{"create_pull_request"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + prConfig := config["create_pull_request"] + assert.Equal(t, 2048, prConfig["max_patch_size"], "Should use custom max_patch_size") + }, + }, + { + name: "Nil SafeOutputsConfig returns empty", + safeOutputs: nil, + expectedKeys: []string{}, + validateField: func(t *testing.T, config map[string]map[string]any) { + assert.Empty(t, config) + }, + }, + { + name: "Empty configs are included (handler enabled by presence)", + safeOutputs: &SafeOutputsConfig{ + CreateIssues: &CreateIssuesConfig{ + // All zero values + }, + }, + expectedKeys: []string{"create_issue"}, + validateField: func(t *testing.T, config map[string]map[string]any) { + assert.Contains(t, config, "create_issue", "Empty config should still be included") + issueConfig := config["create_issue"] + assert.Empty(t, issueConfig, "Config should be empty map") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := buildSafeOutputConfigs(tt.safeOutputs) + + // Check expected keys + for _, key := range tt.expectedKeys { + assert.Contains(t, result, key, "Should contain key %s", key) + } + + // Additional validation + if tt.validateField != nil { + tt.validateField(t, result) + } + }) + } +} + +func TestBuildHandlerConfig_EdgeCases(t *testing.T) { + t.Run("Non-struct input", func(t *testing.T) { + result := buildHandlerConfig("not a struct") + assert.Empty(t, result, "Should return empty for non-struct") + }) + + t.Run("Struct with no mapped fields", func(t *testing.T) { + type UnknownConfig struct { + UnmappedField string + } + result := buildHandlerConfig(&UnknownConfig{UnmappedField: "value"}) + assert.Empty(t, result, "Should skip unmapped fields") + }) + + t.Run("Struct with embedded BaseSafeOutputConfig", func(t *testing.T) { + config := &CreateIssuesConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{ + Max: 10, + GitHubToken: "token-value", + }, + TitlePrefix: "[Test]", + } + result := buildHandlerConfig(config) + assert.Equal(t, int64(10), result["max"], "Should extract Max from embedded struct") + assert.Equal(t, "token-value", result["github-token"], "Should extract GitHubToken from embedded struct") + assert.Equal(t, "[Test]", result["title_prefix"], "Should extract direct fields") + }) +}