Refactor create-* config parsing to shared helper and unify string→array coercion

pkg/workflow/config_parsing_helpers_test.go

@@ -255,6 +255,77 @@ func TestParsePullRequestsConfigWithHelpers(t *testing.T) {
 	}
 }
 
+func TestParseIssuesConfigWithSingleStringAssignee(t *testing.T) {
+	compiler := &Compiler{}
+	outputMap := map[string]any{
+		"create-issue": map[string]any{
+			"assignees": "single-assignee",
+		},
+	}
+
+	result := compiler.parseCreateIssuesConfig(outputMap)
+	if result == nil {
+		t.Fatal("expected non-nil result")
+	}
+
+	if len(result.Assignees) != 1 {
+		t.Fatalf("expected 1 assignee, got %d", len(result.Assignees))
+	}
+	if result.Assignees[0] != "single-assignee" {
+		t.Errorf("expected assignee 'single-assignee', got %q", result.Assignees[0])
+	}
+}
+
+func TestParsePullRequestsConfigWithSingleStringReviewerAndTeamReviewer(t *testing.T) {
+	compiler := &Compiler{}
+	outputMap := map[string]any{
+		"create-pull-request": map[string]any{
+			"reviewers":      "single-reviewer",
+			"team-reviewers": "single-team",
+		},
+	}
+
+	result := compiler.parseCreatePullRequestsConfig(outputMap)
+	if result == nil {
+		t.Fatal("expected non-nil result")
+	}
+
+	if len(result.Reviewers) != 1 {
+		t.Fatalf("expected 1 reviewer, got %d", len(result.Reviewers))
+	}
+	if result.Reviewers[0] != "single-reviewer" {
+		t.Errorf("expected reviewer 'single-reviewer', got %q", result.Reviewers[0])
+	}
+
+	if len(result.TeamReviewers) != 1 {
+		t.Fatalf("expected 1 team reviewer, got %d", len(result.TeamReviewers))
+	}
+	if result.TeamReviewers[0] != "single-team" {
+		t.Errorf("expected team reviewer 'single-team', got %q", result.TeamReviewers[0])
+	}
+}
+
+func TestParsePullRequestsConfigWithSingleStringAssignee(t *testing.T) {
+	compiler := &Compiler{}
+	outputMap := map[string]any{
+		"create-pull-request": map[string]any{
+			"assignees": "single-assignee",
+		},
+	}
+
+	result := compiler.parseCreatePullRequestsConfig(outputMap)
+	if result == nil {
+		t.Fatal("expected non-nil result")
+	}
+
+	if len(result.Assignees) != 1 {
+		t.Fatalf("expected 1 assignee, got %d", len(result.Assignees))
+	}
+	if result.Assignees[0] != "single-assignee" {
+		t.Errorf("expected assignee 'single-assignee', got %q", result.Assignees[0])
+	}
+}
+
 func TestParsePullRequestsConfigExpires(t *testing.T) {
 	tests := []struct {
 		name          string

pkg/workflow/create_discussion.go

@@ -30,61 +30,48 @@ type CreateDiscussionsConfig struct {
 
 // parseCreateDiscussionsConfig handles create-discussion configuration
 func (c *Compiler) parseCreateDiscussionsConfig(outputMap map[string]any) *CreateDiscussionsConfig {
-	// Check if the key exists
-	if _, exists := outputMap["create-discussion"]; !exists {
-		return nil
-	}
-
-	// Get the config data to check for special cases before unmarshaling
-	configData, _ := outputMap["create-discussion"].(map[string]any)
-
-	// Pre-process the expires field (convert to hours before unmarshaling)
-	expiresDisabled := preprocessExpiresField(configData, discussionLog)
-
-	// Pre-process templatable bool fields
-	for _, field := range []string{"close-older-discussions", "footer"} {
-		if err := preprocessBoolFieldAsString(configData, field, discussionLog); err != nil {
-			discussionLog.Printf("Invalid %s value: %v", field, err)
-			return nil
-		}
-	}
+	config := parseCreateEntityConfig(
+		outputMap,
+		"create-discussion",
+		CreateParseOptions{
+			BoolFields:    []string{"close-older-discussions", "footer"},
+			IntFields:     []string{"max"},
+			HandleExpires: true,
+		},
+		discussionLog,
+		func(err error) *CreateDiscussionsConfig {
+			discussionLog.Printf("Failed to unmarshal config: %v", err)
+			// For backward compatibility, handle nil/empty config
+			return &CreateDiscussionsConfig{}
+		},
+		nil,
+		func(_ map[string]any, config *CreateDiscussionsConfig, expiresDisabled bool) {
+			// Set default max if not specified
+			if config.Max == nil {
+				config.Max = defaultIntStr(1)
+			}
 
-	// Pre-process templatable int fields
-	if err := preprocessIntFieldAsString(configData, "max", discussionLog); err != nil {
-		discussionLog.Printf("Invalid max value: %v", err)
-		return nil
-	}
+			// Set default expires to 7 days (168 hours) if not specified and not explicitly disabled
+			if config.Expires == 0 && !expiresDisabled {
+				config.Expires = 168 // 7 days = 168 hours
+				discussionLog.Print("Using default expiration: 7 days (168 hours)")
+			} else if expiresDisabled {
+				config.Expires = 0
+				discussionLog.Print("Expiration explicitly disabled")
+			}
 
-	config := parseConfigScaffold(outputMap, "create-discussion", discussionLog, func(err error) *CreateDiscussionsConfig {
-		discussionLog.Printf("Failed to unmarshal config: %v", err)
-		// For backward compatibility, handle nil/empty config
-		return &CreateDiscussionsConfig{}
-	})
+			// Set default fallback-to-issue to true if not specified
+			if config.FallbackToIssue == nil {
+				trueVal := true
+				config.FallbackToIssue = &trueVal
+				discussionLog.Print("Using default fallback-to-issue: true")
+			}
+		},
+	)
 	if config == nil {
 		return nil
 	}
 
-	// Set default max if not specified
-	if config.Max == nil {
-		config.Max = defaultIntStr(1)
-	}
-
-	// Set default expires to 7 days (168 hours) if not specified and not explicitly disabled
-	if config.Expires == 0 && !expiresDisabled {
-		config.Expires = 168 // 7 days = 168 hours
-		discussionLog.Print("Using default expiration: 7 days (168 hours)")
-	} else if expiresDisabled {
-		config.Expires = 0
-		discussionLog.Print("Expiration explicitly disabled")
-	}
-
-	// Set default fallback-to-issue to true if not specified
-	if config.FallbackToIssue == nil {
-		trueVal := true
-		config.FallbackToIssue = &trueVal
-		discussionLog.Print("Using default fallback-to-issue: true")
-	}
-
 	// Normalize and validate category naming convention
 	config.Category = normalizeDiscussionCategory(config.Category, discussionLog, c.markdownPath)
 

pkg/workflow/create_entity_helpers.go

@@ -0,0 +1,83 @@
+package workflow
+
+import "github.com/github/gh-aw/pkg/logger"
+
+// CreateParseOptions defines common preprocessing options for create-entity parsers.
+//
+// BoolFields and IntFields list config field names that should be normalized through
+// templatable preprocessing before YAML unmarshaling.
+// HandleExpires enables shared expires normalization via preprocessExpiresField.
+type CreateParseOptions struct {
+	BoolFields    []string
+	IntFields     []string
+	HandleExpires bool
+}
+
+// parseCreateEntityConfig parses create-* config scaffolding shared by issue/discussion/PR handlers.
+//
+// Parameters:
+//   - outputMap: full safe-output map from frontmatter parsing.
+//   - configKey: create-* key to parse (for example "create-issue").
+//   - opts: shared preprocessing configuration for bool/int/expires fields.
+//   - log: logger used for preprocessing and parse diagnostics.
+//   - onError: required error handler invoked on unmarshal failures.
+//
+// Callback lifecycle:
+//   - preUnmarshal is optional (may be nil). When provided, it is invoked first with the raw
+//     config map. The map may be nil when configKey exists but is not a map; if preUnmarshal
+//     returns false, parsing is aborted.
+//   - onError is invoked when YAML unmarshaling fails and returns the fallback config behavior.
+//   - postUnmarshal is optional (may be nil). When provided, it is invoked after successful
+//     unmarshaling and receives expiresDisabled (true when expires was explicitly set to false).
+func parseCreateEntityConfig[T any](
+	outputMap map[string]any,
+	configKey string,
+	opts CreateParseOptions,
+	log *logger.Logger,
+	onError func(error) *T,
+	preUnmarshal func(map[string]any) bool,
+	postUnmarshal func(map[string]any, *T, bool),
+) *T {
+	if _, exists := outputMap[configKey]; !exists {
+		return nil
+	}
+
+	configDataAny := outputMap[configKey]
+	configData, isMap := configDataAny.(map[string]any)
+	if !isMap {
+		configData = nil
+	}
+	if preUnmarshal != nil && !preUnmarshal(configData) {
+		return nil
+	}
+
+	expiresDisabled := false
+	if opts.HandleExpires {
+		expiresDisabled = preprocessExpiresField(configData, log)
+	}
+
+	for _, field := range opts.BoolFields {
+		if err := preprocessBoolFieldAsString(configData, field, log); err != nil {
+			log.Printf("Invalid %s value: %v", field, err)
+			return nil
+		}
+	}
+
+	for _, field := range opts.IntFields {
+		if err := preprocessIntFieldAsString(configData, field, log); err != nil {
+			log.Printf("Invalid %s value: %v", field, err)
+			return nil
+		}
+	}
+
+	config := parseConfigScaffold(outputMap, configKey, log, onError)
+	if config == nil {
+		return nil
+	}
+
+	if postUnmarshal != nil {
+		postUnmarshal(configData, config, expiresDisabled)
+	}
+
+	return config
+}

pkg/workflow/create_issue.go

@@ -28,64 +28,38 @@ type CreateIssuesConfig struct {
 
 // parseCreateIssuesConfig handles create-issue configuration
 func (c *Compiler) parseCreateIssuesConfig(outputMap map[string]any) *CreateIssuesConfig {
-	// Check if the key exists
-	if _, exists := outputMap["create-issue"]; !exists {
-		return nil
-	}
-
-	// Get the config data to check for special cases before unmarshaling
-	configData, _ := outputMap["create-issue"].(map[string]any)
-
-	// Pre-process the expires field (convert to hours before unmarshaling)
-	expiresDisabled := preprocessExpiresField(configData, createIssueLog)
-
-	// Pre-process templatable bool fields: convert literal booleans to strings so that
-	// GitHub Actions expression strings (e.g. "${{ inputs.close-older-issues }}") are also accepted.
-	for _, field := range []string{"close-older-issues", "group", "footer", "group-by-day"} {
-		if err := preprocessBoolFieldAsString(configData, field, createIssueLog); err != nil {
-			createIssueLog.Printf("Invalid %s value: %v", field, err)
-			return nil
-		}
-	}
-
-	// Pre-process templatable int fields
-	if err := preprocessIntFieldAsString(configData, "max", createIssueLog); err != nil {
-		createIssueLog.Printf("Invalid max value: %v", err)
-		return nil
-	}
-
-	config := parseConfigScaffold(outputMap, "create-issue", createIssueLog, func(err error) *CreateIssuesConfig {
-		createIssueLog.Printf("Failed to unmarshal config: %v", err)
-		// For backward compatibility, handle nil/empty config
-		return &CreateIssuesConfig{}
-	})
-	if config == nil {
-		return nil
-	}
-
-	// Handle single string assignee (YAML unmarshaling won't convert string to []string)
-	if len(config.Assignees) == 0 && configData != nil {
-		if assignees, exists := configData["assignees"]; exists {
-			if assigneeStr, ok := assignees.(string); ok {
-				config.Assignees = []string{assigneeStr}
-				createIssueLog.Printf("Converted single assignee string to array: %v", config.Assignees)
+	return parseCreateEntityConfig(
+		outputMap,
+		"create-issue",
+		CreateParseOptions{
+			BoolFields:    []string{"close-older-issues", "group", "footer", "group-by-day"},
+			IntFields:     []string{"max"},
+			HandleExpires: true,
+		},
+		createIssueLog,
+		func(err error) *CreateIssuesConfig {
+			createIssueLog.Printf("Failed to unmarshal config: %v", err)
+			// For backward compatibility, handle nil/empty config
+			return &CreateIssuesConfig{}
+		},
+		func(configData map[string]any) bool {
+			coerceStringOrArrayFields(configData, []string{"assignees"}, createIssueLog)
+			return true
+		},
+		func(_ map[string]any, config *CreateIssuesConfig, expiresDisabled bool) {
+			// Set default max if not specified
+			if config.Max == nil {
+				config.Max = defaultIntStr(1)
 			}
-		}
-	}
-
-	// Set default max if not specified
-	if config.Max == nil {
-		config.Max = defaultIntStr(1)
-	}
 
-	// Log expires if configured or explicitly disabled
-	if expiresDisabled {
-		createIssueLog.Print("Issue expiration explicitly disabled")
-	} else if config.Expires > 0 {
-		createIssueLog.Printf("Issue expiration configured: %d hours", config.Expires)
-	}
-
-	return config
+			// Log expires if configured or explicitly disabled
+			if expiresDisabled {
+				createIssueLog.Print("Issue expiration explicitly disabled")
+			} else if config.Expires > 0 {
+				createIssueLog.Printf("Issue expiration configured: %d hours", config.Expires)
+			}
+		},
+	)
 }
 
 // hasCopilotAssignee checks if "copilot" is in the assignees list