Refactor: Extract shared helpers for safe output config parsing

pkg/workflow/add_comment.go

@@ -120,23 +120,18 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
 	if configData, exists := outputMap["add-comment"]; exists {
 		addCommentLog.Print("Parsing add-comment configuration")
 		commentsConfig := &AddCommentsConfig{}
-		commentsConfig.Max = 1 // Default max is 1
 
 		if configMap, ok := configData.(map[string]any); ok {
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &commentsConfig.BaseSafeOutputConfig)
-
 			// Parse target
 			if target, exists := configMap["target"]; exists {
 				if targetStr, ok := target.(string); ok {
 					commentsConfig.Target = targetStr
 				}
 			}
 
-			// Parse target-repo using shared helper
-			targetRepoSlug := parseTargetRepoFromConfig(configMap)
-			// Validate that target-repo is not "*" - only definite strings are allowed
-			if targetRepoSlug == "*" {
+			// Parse target-repo using shared helper with validation
+			targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+			if isInvalid {
 				return nil // Invalid configuration, return nil to cause validation error
 			}
 			commentsConfig.TargetRepoSlug = targetRepoSlug
@@ -151,6 +146,13 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon
 					commentsConfig.Discussion = &discussionBool
 				}
 			}
+
+			// Parse common base fields with default max of 1
+			c.parseBaseSafeOutputConfig(configMap, &commentsConfig.BaseSafeOutputConfig, 1)
+		} else {
+			// If configData is nil or not a map (e.g., "add-comment:" with no value),
+			// still set the default max
+			commentsConfig.Max = 1
 		}
 
 		return commentsConfig

pkg/workflow/config_helpers.go

@@ -58,3 +58,17 @@ func parseTitlePrefixFromConfig(configMap map[string]any) string {
 func parseTargetRepoFromConfig(configMap map[string]any) string {
 	return extractStringFromMap(configMap, "target-repo", configHelpersLog)
 }
+
+// parseTargetRepoWithValidation extracts the target-repo value from a config map and validates it.
+// Returns the target repository slug as a string, or empty string if not present or invalid.
+// Returns an error (indicated by the second return value being true) if the value is "*" (wildcard),
+// which is not allowed for safe output target repositories.
+func parseTargetRepoWithValidation(configMap map[string]any) (string, bool) {
+	targetRepoSlug := parseTargetRepoFromConfig(configMap)
+	// Validate that target-repo is not "*" - only definite strings are allowed
+	if targetRepoSlug == "*" {
+		configHelpersLog.Print("Invalid target-repo: wildcard '*' is not allowed")
+		return "", true // Return true to indicate validation error
+	}
+	return targetRepoSlug, false
+}

pkg/workflow/config_parsing_helpers_test.go

@@ -455,3 +455,63 @@ func TestParsePRReviewCommentsConfigWithWildcardTargetRepo(t *testing.T) {
 		t.Errorf("expected nil for wildcard target-repo, got %+v", result)
 	}
 }
+
+func TestParseTargetRepoWithValidation(t *testing.T) {
+	tests := []struct {
+		name          string
+		input         map[string]any
+		expectedSlug  string
+		expectedError bool
+	}{
+		{
+			name: "valid target-repo",
+			input: map[string]any{
+				"target-repo": "owner/repo",
+			},
+			expectedSlug:  "owner/repo",
+			expectedError: false,
+		},
+		{
+			name: "empty target-repo",
+			input: map[string]any{
+				"target-repo": "",
+			},
+			expectedSlug:  "",
+			expectedError: false,
+		},
+		{
+			name:          "missing target-repo",
+			input:         map[string]any{},
+			expectedSlug:  "",
+			expectedError: false,
+		},
+		{
+			name: "wildcard target-repo (invalid)",
+			input: map[string]any{
+				"target-repo": "*",
+			},
+			expectedSlug:  "",
+			expectedError: true,
+		},
+		{
+			name: "target-repo with special characters",
+			input: map[string]any{
+				"target-repo": "github-next/gh-aw",
+			},
+			expectedSlug:  "github-next/gh-aw",
+			expectedError: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			slug, isInvalid := parseTargetRepoWithValidation(tt.input)
+			if slug != tt.expectedSlug {
+				t.Errorf("expected slug %q, got %q", tt.expectedSlug, slug)
+			}
+			if isInvalid != tt.expectedError {
+				t.Errorf("expected error %v, got %v", tt.expectedError, isInvalid)
+			}
+		})
+	}
+}

pkg/workflow/create_agent_task.go

@@ -20,7 +20,6 @@ func (c *Compiler) parseAgentTaskConfig(outputMap map[string]any) *CreateAgentTa
 	if configData, exists := outputMap["create-agent-task"]; exists {
 		createAgentTaskLog.Print("Parsing create-agent-task configuration")
 		agentTaskConfig := &CreateAgentTaskConfig{}
-		agentTaskConfig.Max = 1 // Default max is 1
 
 		if configMap, ok := configData.(map[string]any); ok {
 			// Parse base branch
@@ -30,16 +29,19 @@ func (c *Compiler) parseAgentTaskConfig(outputMap map[string]any) *CreateAgentTa
 				}
 			}
 
-			// Parse target-repo using shared helper
-			targetRepoSlug := parseTargetRepoFromConfig(configMap)
-			// Validate that target-repo is not "*" - only definite strings are allowed
-			if targetRepoSlug == "*" {
+			// Parse target-repo using shared helper with validation
+			targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+			if isInvalid {
 				return nil // Invalid configuration, return nil to cause validation error
 			}
 			agentTaskConfig.TargetRepoSlug = targetRepoSlug
 
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &agentTaskConfig.BaseSafeOutputConfig)
+			// Parse common base fields with default max of 1
+			c.parseBaseSafeOutputConfig(configMap, &agentTaskConfig.BaseSafeOutputConfig, 1)
+		} else {
+			// If configData is nil or not a map (e.g., "create-agent-task:" with no value),
+			// still set the default max
+			agentTaskConfig.Max = 1
 		}
 
 		return agentTaskConfig

pkg/workflow/create_code_scanning_alert.go

@@ -101,18 +101,21 @@ func (c *Compiler) parseCodeScanningAlertsConfig(outputMap map[string]any) *Crea
 	createCodeScanningAlertLog.Print("Parsing create-code-scanning-alert configuration")
 	configData := outputMap["create-code-scanning-alert"]
 	securityReportsConfig := &CreateCodeScanningAlertsConfig{}
-	securityReportsConfig.Max = 0 // Default max is 0 (unlimited)
 
 	if configMap, ok := configData.(map[string]any); ok {
-		// Parse common base fields
-		c.parseBaseSafeOutputConfig(configMap, &securityReportsConfig.BaseSafeOutputConfig)
-
 		// Parse driver
 		if driver, exists := configMap["driver"]; exists {
 			if driverStr, ok := driver.(string); ok {
 				securityReportsConfig.Driver = driverStr
 			}
 		}
+
+		// Parse common base fields with default max of 0 (unlimited)
+		c.parseBaseSafeOutputConfig(configMap, &securityReportsConfig.BaseSafeOutputConfig, 0)
+	} else {
+		// If configData is nil or not a map (e.g., "create-code-scanning-alert:" with no value),
+		// still set the default max (0 = unlimited)
+		securityReportsConfig.BaseSafeOutputConfig.Max = 0
 	}
 
 	return securityReportsConfig

pkg/workflow/create_discussion.go

@@ -16,12 +16,8 @@ type CreateDiscussionsConfig struct {
 func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscussionsConfig {
 	if configData, exists := outputMap["create-discussion"]; exists {
 		discussionsConfig := &CreateDiscussionsConfig{}
-		discussionsConfig.Max = 1 // Default max is 1
 
 		if configMap, ok := configData.(map[string]any); ok {
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &discussionsConfig.BaseSafeOutputConfig)
-
 			// Parse title-prefix using shared helper
 			discussionsConfig.TitlePrefix = parseTitlePrefixFromConfig(configMap)
 
@@ -39,13 +35,19 @@ func (c *Compiler) parseDiscussionsConfig(outputMap map[string]any) *CreateDiscu
 				}
 			}
 
-			// Parse target-repo using shared helper
-			targetRepoSlug := parseTargetRepoFromConfig(configMap)
-			// Validate that target-repo is not "*" - only definite strings are allowed
-			if targetRepoSlug == "*" {
+			// Parse target-repo using shared helper with validation
+			targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+			if isInvalid {
 				return nil // Invalid configuration, return nil to cause validation error
 			}
 			discussionsConfig.TargetRepoSlug = targetRepoSlug
+
+			// Parse common base fields with default max of 1
+			c.parseBaseSafeOutputConfig(configMap, &discussionsConfig.BaseSafeOutputConfig, 1)
+		} else {
+			// If configData is nil or not a map (e.g., "create-discussion:" with no value),
+			// still set the default max
+			discussionsConfig.Max = 1
 		}
 
 		return discussionsConfig

pkg/workflow/create_issue.go

@@ -23,7 +23,6 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf
 	if configData, exists := outputMap["create-issue"]; exists {
 		createIssueLog.Print("Parsing create-issue configuration")
 		issuesConfig := &CreateIssuesConfig{}
-		issuesConfig.Max = 1 // Default max is 1
 
 		if configMap, ok := configData.(map[string]any); ok {
 			// Parse title-prefix using shared helper
@@ -49,16 +48,19 @@ func (c *Compiler) parseIssuesConfig(outputMap map[string]any) *CreateIssuesConf
 				}
 			}
 
-			// Parse target-repo using shared helper
-			targetRepoSlug := parseTargetRepoFromConfig(configMap)
-			// Validate that target-repo is not "*" - only definite strings are allowed
-			if targetRepoSlug == "*" {
+			// Parse target-repo using shared helper with validation
+			targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+			if isInvalid {
 				return nil // Invalid configuration, return nil to cause validation error
 			}
 			issuesConfig.TargetRepoSlug = targetRepoSlug
 
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &issuesConfig.BaseSafeOutputConfig)
+			// Parse common base fields with default max of 1
+			c.parseBaseSafeOutputConfig(configMap, &issuesConfig.BaseSafeOutputConfig, 1)
+		} else {
+			// If configData is nil or not a map (e.g., "create-issue:" with no value),
+			// still set the default max
+			issuesConfig.Max = 1
 		}
 
 		return issuesConfig

pkg/workflow/create_pr_review_comment.go

@@ -98,12 +98,8 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any
 
 	configData := outputMap["create-pull-request-review-comment"]
 	prReviewCommentsConfig := &CreatePullRequestReviewCommentsConfig{Side: "RIGHT"} // Default side is RIGHT
-	prReviewCommentsConfig.Max = 10                                                 // Default max is 10
 
 	if configMap, ok := configData.(map[string]any); ok {
-		// Parse common base fields
-		c.parseBaseSafeOutputConfig(configMap, &prReviewCommentsConfig.BaseSafeOutputConfig)
-
 		// Parse side
 		if side, exists := configMap["side"]; exists {
 			if sideStr, ok := side.(string); ok {
@@ -121,13 +117,19 @@ func (c *Compiler) parsePullRequestReviewCommentsConfig(outputMap map[string]any
 			}
 		}
 
-		// Parse target-repo using shared helper
-		targetRepoSlug := parseTargetRepoFromConfig(configMap)
-		// Validate that target-repo is not "*" - only definite strings are allowed
-		if targetRepoSlug == "*" {
+		// Parse target-repo using shared helper with validation
+		targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+		if isInvalid {
 			return nil // Invalid configuration, return nil to cause validation error
 		}
 		prReviewCommentsConfig.TargetRepoSlug = targetRepoSlug
+
+		// Parse common base fields with default max of 10
+		c.parseBaseSafeOutputConfig(configMap, &prReviewCommentsConfig.BaseSafeOutputConfig, 10)
+	} else {
+		// If configData is nil or not a map (e.g., "create-pull-request-review-comment:" with no value),
+		// still set the default max
+		prReviewCommentsConfig.Max = 10
 	}
 
 	return prReviewCommentsConfig

pkg/workflow/create_pull_request.go

@@ -155,7 +155,6 @@ func (c *Compiler) parsePullRequestsConfig(outputMap map[string]any) *CreatePull
 	createPRLog.Print("Parsing create-pull-request configuration")
 	configData := outputMap["create-pull-request"]
 	pullRequestsConfig := &CreatePullRequestsConfig{}
-	pullRequestsConfig.Max = 1 // Always max 1 for pull requests
 
 	if configMap, ok := configData.(map[string]any); ok {
 		// Parse title-prefix using shared helper
@@ -195,23 +194,23 @@ func (c *Compiler) parsePullRequestsConfig(outputMap map[string]any) *CreatePull
 			}
 		}
 
-		// Parse target-repo using shared helper
-		targetRepoSlug := parseTargetRepoFromConfig(configMap)
-		// Validate that target-repo is not "*" - only definite strings are allowed
-		if targetRepoSlug == "*" {
+		// Parse target-repo using shared helper with validation
+		targetRepoSlug, isInvalid := parseTargetRepoWithValidation(configMap)
+		if isInvalid {
 			return nil // Invalid configuration, return nil to cause validation error
 		}
 		pullRequestsConfig.TargetRepoSlug = targetRepoSlug
 
-		// Parse github-token (max is always 1 for pull requests)
-		if githubToken, exists := configMap["github-token"]; exists {
-			if githubTokenStr, ok := githubToken.(string); ok {
-				pullRequestsConfig.GitHubToken = githubTokenStr
-			}
-		}
+		// Parse common base fields (github-token, max if specified by user)
+		c.parseBaseSafeOutputConfig(configMap, &pullRequestsConfig.BaseSafeOutputConfig, -1)
 
 		// Note: max parameter is not supported for pull requests (always limited to 1)
-		// If max is specified, it will be ignored as pull requests are singular only
+		// Override any user-specified max value to enforce the limit
+		pullRequestsConfig.Max = 1
+	} else {
+		// If configData is nil or not a map (e.g., "create-pull-request:" with no value),
+		// still set the default max (always 1 for pull requests)
+		pullRequestsConfig.Max = 1
 	}
 
 	return pullRequestsConfig

pkg/workflow/missing_tool.go

@@ -74,8 +74,8 @@ func (c *Compiler) parseMissingToolConfig(outputMap map[string]any) *MissingTool
 		}
 
 		if configMap, ok := configData.(map[string]any); ok {
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &missingToolConfig.BaseSafeOutputConfig)
+			// Parse common base fields with default max of 0 (no limit)
+			c.parseBaseSafeOutputConfig(configMap, &missingToolConfig.BaseSafeOutputConfig, 0)
 		}
 
 		return missingToolConfig

pkg/workflow/publish_assets.go

@@ -57,8 +57,8 @@ func (c *Compiler) parseUploadAssetConfig(outputMap map[string]any) *UploadAsset
 				}
 			}
 
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig)
+			// Parse common base fields with default max of 0 (no limit)
+			c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 0)
 		} else if configData == nil {
 			// Handle null case: create config with defaults
 			return config

pkg/workflow/push_to_pull_request_branch.go

@@ -194,8 +194,8 @@ func (c *Compiler) parsePushToPullRequestBranchConfig(outputMap map[string]any)
 				}
 			}
 
-			// Parse common base fields
-			c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig)
+			// Parse common base fields with default max of 0 (no limit)
+			c.parseBaseSafeOutputConfig(configMap, &pushToBranchConfig.BaseSafeOutputConfig, 0)
 		}
 
 		return pushToBranchConfig

pkg/workflow/safe_output_config.go

@@ -1,8 +1,15 @@
 package workflow
 
-// parseBaseSafeOutputConfig parses common fields (max, github-token) from a config map
-func (c *Compiler) parseBaseSafeOutputConfig(configMap map[string]any, config *BaseSafeOutputConfig) {
-	// Parse max
+// parseBaseSafeOutputConfig parses common fields (max, github-token) from a config map.
+// If defaultMax is provided (>= 0), it will be set as the default value for config.Max
+// before parsing the max field from configMap.
+func (c *Compiler) parseBaseSafeOutputConfig(configMap map[string]any, config *BaseSafeOutputConfig, defaultMax int) {
+	// Set default max if provided
+	if defaultMax >= 0 {
+		config.Max = defaultMax
+	}
+
+	// Parse max (this will override the default if present in configMap)
 	if max, exists := configMap["max"]; exists {
 		if maxInt, ok := parseIntValue(max); ok {
 			config.Max = maxInt