Refactor update job builders to use shared helper pattern

pkg/workflow/safe_outputs_tools_test.go

@@ -98,7 +98,9 @@ func TestGenerateFilteredToolsJSON(t *testing.T) {
 			name: "update issues enabled",
 			safeOutputs: &SafeOutputsConfig{
 				UpdateIssues: &UpdateIssuesConfig{
-					BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3},
+					UpdateEntityConfig: UpdateEntityConfig{
+						BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3},
+					},
 				},
 			},
 			expectedTools: []string{"update_issue"},
@@ -151,7 +153,7 @@ func TestGenerateFilteredToolsJSON(t *testing.T) {
 				CreateCodeScanningAlerts:        &CreateCodeScanningAlertsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 100}},
 				AddLabels:                       &AddLabelsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}},
 				AddReviewer:                     &AddReviewerConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}},
-				UpdateIssues:                    &UpdateIssuesConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}},
+				UpdateIssues:                    &UpdateIssuesConfig{UpdateEntityConfig: UpdateEntityConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}}},
 				PushToPullRequestBranch:         &PushToPullRequestBranchConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 1}},
 				UploadAssets:                    &UploadAssetsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 10}},
 				MissingTool:                     &MissingToolConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 5}},

pkg/workflow/update_entity_helpers.go

@@ -0,0 +1,96 @@
+package workflow
+
+import (
+	"fmt"
+
+	"github.com/githubnext/gh-aw/pkg/logger"
+)
+
+// UpdateEntityType represents the type of entity being updated
+type UpdateEntityType string
+
+const (
+	UpdateEntityIssue       UpdateEntityType = "issue"
+	UpdateEntityPullRequest UpdateEntityType = "pull_request"
+	UpdateEntityRelease     UpdateEntityType = "release"
+)
+
+// UpdateEntityConfig holds the configuration for an update entity operation
+type UpdateEntityConfig struct {
+	BaseSafeOutputConfig   `yaml:",inline"`
+	SafeOutputTargetConfig `yaml:",inline"`
+	// Type-specific fields are stored in the concrete config structs
+}
+
+// UpdateEntityJobParams holds the parameters needed to build an update entity job
+type UpdateEntityJobParams struct {
+	EntityType      UpdateEntityType
+	ConfigKey       string // e.g., "update-issue", "update-pull-request"
+	JobName         string // e.g., "update_issue", "update_pull_request"
+	StepName        string // e.g., "Update Issue", "Update Pull Request"
+	ScriptGetter    func() string
+	PermissionsFunc func() *Permissions
+	CustomEnvVars   []string          // Type-specific environment variables
+	Outputs         map[string]string // Type-specific outputs
+	Condition       ConditionNode     // Job condition expression
+}
+
+// parseUpdateEntityConfig is a generic function to parse update entity configurations
+func (c *Compiler) parseUpdateEntityConfig(outputMap map[string]any, params UpdateEntityJobParams, logger *logger.Logger, parseSpecificFields func(map[string]any, *UpdateEntityConfig)) *UpdateEntityConfig {
+	if configData, exists := outputMap[params.ConfigKey]; exists {
+		logger.Printf("Parsing %s configuration", params.ConfigKey)
+		config := &UpdateEntityConfig{}
+
+		if configMap, ok := configData.(map[string]any); ok {
+			// Parse target config (target, target-repo) with validation
+			targetConfig, isInvalid := ParseTargetConfig(configMap)
+			if isInvalid {
+				logger.Print("Invalid target-repo configuration")
+				return nil
+			}
+			config.SafeOutputTargetConfig = targetConfig
+
+			// Parse type-specific fields if provided
+			if parseSpecificFields != nil {
+				parseSpecificFields(configMap, config)
+			}
+
+			// Parse common base fields with default max of 1
+			c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 1)
+		} else {
+			// If configData is nil or not a map, still set the default max
+			config.Max = 1
+		}
+
+		return config
+	}
+
+	return nil
+}
+
+// buildUpdateEntityJob is a generic function to build update entity jobs
+func (c *Compiler) buildUpdateEntityJob(data *WorkflowData, mainJobName string, config *UpdateEntityConfig, params UpdateEntityJobParams, logger *logger.Logger) (*Job, error) {
+	logger.Printf("Building %s job for workflow: %s", params.JobName, data.Name)
+
+	if config == nil {
+		return nil, fmt.Errorf("safe-outputs.%s configuration is required", params.ConfigKey)
+	}
+
+	// Add standard environment variables (metadata + staged/target repo)
+	allEnvVars := append(params.CustomEnvVars, c.buildStandardSafeOutputEnvVars(data, config.TargetRepoSlug)...)
+
+	// Use the shared builder function to create the job
+	return c.buildSafeOutputJob(data, SafeOutputJobConfig{
+		JobName:        params.JobName,
+		StepName:       params.StepName,
+		StepID:         params.JobName,
+		MainJobName:    mainJobName,
+		CustomEnvVars:  allEnvVars,
+		Script:         params.ScriptGetter(),
+		Permissions:    params.PermissionsFunc(),
+		Outputs:        params.Outputs,
+		Condition:      params.Condition,
+		Token:          config.GitHubToken,
+		TargetRepoSlug: config.TargetRepoSlug,
+	})
+}

pkg/workflow/update_issue.go

@@ -2,15 +2,18 @@ package workflow
 
 import (
 	"fmt"
+
+	"github.com/githubnext/gh-aw/pkg/logger"
 )
 
+var updateIssueLog = logger.New("workflow:update_issue")
+
 // UpdateIssuesConfig holds configuration for updating GitHub issues from agent output
 type UpdateIssuesConfig struct {
-	BaseSafeOutputConfig   `yaml:",inline"`
-	SafeOutputTargetConfig `yaml:",inline"`
-	Status                 *bool `yaml:"status,omitempty"` // Allow updating issue status (open/closed) - presence indicates field can be updated
-	Title                  *bool `yaml:"title,omitempty"`  // Allow updating issue title - presence indicates field can be updated
-	Body                   *bool `yaml:"body,omitempty"`   // Allow updating issue body - presence indicates field can be updated
+	UpdateEntityConfig `yaml:",inline"`
+	Status             *bool `yaml:"status,omitempty"` // Allow updating issue status (open/closed) - presence indicates field can be updated
+	Title              *bool `yaml:"title,omitempty"`  // Allow updating issue title - presence indicates field can be updated
+	Body               *bool `yaml:"body,omitempty"`   // Allow updating issue body - presence indicates field can be updated
 }
 
 // buildCreateOutputUpdateIssueJob creates the update_issue job
@@ -31,9 +34,6 @@ func (c *Compiler) buildCreateOutputUpdateIssueJob(data *WorkflowData, mainJobNa
 	// Pass the target configuration
 	customEnvVars = append(customEnvVars, BuildTargetEnvVar("GH_AW_UPDATE_TARGET", cfg.Target)...)
 
-	// Add standard environment variables (metadata + staged/target repo)
-	customEnvVars = append(customEnvVars, c.buildStandardSafeOutputEnvVars(data, cfg.TargetRepoSlug)...)
-
 	// Create outputs for the job
 	outputs := map[string]string{
 		"issue_number": "${{ steps.update_issue.outputs.issue_number }}",
@@ -47,35 +47,46 @@ func (c *Compiler) buildCreateOutputUpdateIssueJob(data *WorkflowData, mainJobNa
 		jobCondition = buildAnd(jobCondition, eventCondition)
 	}
 
-	// Use the shared builder function to create the job
-	return c.buildSafeOutputJob(data, SafeOutputJobConfig{
-		JobName:        "update_issue",
-		StepName:       "Update Issue",
-		StepID:         "update_issue",
-		MainJobName:    mainJobName,
-		CustomEnvVars:  customEnvVars,
-		Script:         getUpdateIssueScript(),
-		Permissions:    NewPermissionsContentsReadIssuesWrite(),
-		Outputs:        outputs,
-		Condition:      jobCondition,
-		Token:          cfg.GitHubToken,
-		TargetRepoSlug: cfg.TargetRepoSlug,
-	})
+	params := UpdateEntityJobParams{
+		EntityType:      UpdateEntityIssue,
+		ConfigKey:       "update-issue",
+		JobName:         "update_issue",
+		StepName:        "Update Issue",
+		ScriptGetter:    getUpdateIssueScript,
+		PermissionsFunc: NewPermissionsContentsReadIssuesWrite,
+		CustomEnvVars:   customEnvVars,
+		Outputs:         outputs,
+		Condition:       jobCondition,
+	}
+
+	return c.buildUpdateEntityJob(data, mainJobName, &cfg.UpdateEntityConfig, params, updateIssueLog)
 }
 
 // parseUpdateIssuesConfig handles update-issue configuration
 func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssuesConfig {
-	if configData, exists := outputMap["update-issue"]; exists {
-		updateIssuesConfig := &UpdateIssuesConfig{}
+	params := UpdateEntityJobParams{
+		EntityType: UpdateEntityIssue,
+		ConfigKey:  "update-issue",
+	}
 
-		if configMap, ok := configData.(map[string]any); ok {
-			// Parse target config (target, target-repo) with validation
-			targetConfig, isInvalid := ParseTargetConfig(configMap)
-			if isInvalid {
-				return nil // Invalid configuration (e.g., wildcard target-repo), return nil to cause validation error
-			}
-			updateIssuesConfig.SafeOutputTargetConfig = targetConfig
+	parseSpecificFields := func(configMap map[string]any, baseConfig *UpdateEntityConfig) {
+		// This will be called during parsing to handle issue-specific fields
+		// The actual UpdateIssuesConfig fields are handled separately since they're not in baseConfig
+	}
+
+	baseConfig := c.parseUpdateEntityConfig(outputMap, params, updateIssueLog, parseSpecificFields)
+	if baseConfig == nil {
+		return nil
+	}
+
+	// Create UpdateIssuesConfig and populate it
+	updateIssuesConfig := &UpdateIssuesConfig{
+		UpdateEntityConfig: *baseConfig,
+	}
 
+	// Parse issue-specific fields
+	if configData, exists := outputMap["update-issue"]; exists {
+		if configMap, ok := configData.(map[string]any); ok {
 			// Parse status - presence of the key (even if nil/empty) indicates field can be updated
 			if _, exists := configMap["status"]; exists {
 				// If the key exists, it means we can update the status
@@ -92,17 +103,8 @@ func (c *Compiler) parseUpdateIssuesConfig(outputMap map[string]any) *UpdateIssu
 			if _, exists := configMap["body"]; exists {
 				updateIssuesConfig.Body = new(bool)
 			}
-
-			// Parse common base fields with default max of 1
-			c.parseBaseSafeOutputConfig(configMap, &updateIssuesConfig.BaseSafeOutputConfig, 1)
-		} else {
-			// If configData is nil or not a map (e.g., "update-issue:" with no value),
-			// still set the default max
-			updateIssuesConfig.Max = 1
 		}
-
-		return updateIssuesConfig
 	}
 
-	return nil
+	return updateIssuesConfig
 }

pkg/workflow/update_pull_request.go

@@ -10,10 +10,9 @@ var updatePullRequestLog = logger.New("workflow:update_pull_request")
 
 // UpdatePullRequestsConfig holds configuration for updating GitHub pull requests from agent output
 type UpdatePullRequestsConfig struct {
-	BaseSafeOutputConfig   `yaml:",inline"`
-	SafeOutputTargetConfig `yaml:",inline"`
-	Title                  *bool `yaml:"title,omitempty"` // Allow updating PR title - defaults to true, set to false to disable
-	Body                   *bool `yaml:"body,omitempty"`  // Allow updating PR body - defaults to true, set to false to disable
+	UpdateEntityConfig `yaml:",inline"`
+	Title              *bool `yaml:"title,omitempty"` // Allow updating PR title - defaults to true, set to false to disable
+	Body               *bool `yaml:"body,omitempty"`  // Allow updating PR body - defaults to true, set to false to disable
 }
 
 // buildCreateOutputUpdatePullRequestJob creates the update_pull_request job
@@ -38,9 +37,6 @@ func (c *Compiler) buildCreateOutputUpdatePullRequestJob(data *WorkflowData, mai
 	// Pass the target configuration
 	customEnvVars = append(customEnvVars, BuildTargetEnvVar("GH_AW_UPDATE_TARGET", cfg.Target)...)
 
-	// Add standard environment variables (metadata + staged/target repo)
-	customEnvVars = append(customEnvVars, c.buildStandardSafeOutputEnvVars(data, cfg.TargetRepoSlug)...)
-
 	// Create outputs for the job
 	outputs := map[string]string{
 		"pull_request_number": "${{ steps.update_pull_request.outputs.pull_request_number }}",
@@ -54,36 +50,47 @@ func (c *Compiler) buildCreateOutputUpdatePullRequestJob(data *WorkflowData, mai
 		jobCondition = buildAnd(jobCondition, eventCondition)
 	}
 
-	// Use the shared builder function to create the job
-	return c.buildSafeOutputJob(data, SafeOutputJobConfig{
-		JobName:        "update_pull_request",
-		StepName:       "Update Pull Request",
-		StepID:         "update_pull_request",
-		MainJobName:    mainJobName,
-		CustomEnvVars:  customEnvVars,
-		Script:         getUpdatePullRequestScript(),
-		Permissions:    NewPermissionsContentsReadPRWrite(),
-		Outputs:        outputs,
-		Condition:      jobCondition,
-		Token:          cfg.GitHubToken,
-		TargetRepoSlug: cfg.TargetRepoSlug,
-	})
+	params := UpdateEntityJobParams{
+		EntityType:      UpdateEntityPullRequest,
+		ConfigKey:       "update-pull-request",
+		JobName:         "update_pull_request",
+		StepName:        "Update Pull Request",
+		ScriptGetter:    getUpdatePullRequestScript,
+		PermissionsFunc: NewPermissionsContentsReadPRWrite,
+		CustomEnvVars:   customEnvVars,
+		Outputs:         outputs,
+		Condition:       jobCondition,
+	}
+
+	return c.buildUpdateEntityJob(data, mainJobName, &cfg.UpdateEntityConfig, params, updatePullRequestLog)
 }
 
 // parseUpdatePullRequestsConfig handles update-pull-request configuration
 func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *UpdatePullRequestsConfig {
 	updatePullRequestLog.Print("Parsing update pull request configuration")
-	if configData, exists := outputMap["update-pull-request"]; exists {
-		updatePullRequestsConfig := &UpdatePullRequestsConfig{}
+	params := UpdateEntityJobParams{
+		EntityType: UpdateEntityPullRequest,
+		ConfigKey:  "update-pull-request",
+	}
 
-		if configMap, ok := configData.(map[string]any); ok {
-			// Parse target config (target, target-repo) with validation
-			targetConfig, isInvalid := ParseTargetConfig(configMap)
-			if isInvalid {
-				return nil // Invalid configuration (e.g., wildcard target-repo), return nil to cause validation error
-			}
-			updatePullRequestsConfig.SafeOutputTargetConfig = targetConfig
+	parseSpecificFields := func(configMap map[string]any, baseConfig *UpdateEntityConfig) {
+		// This will be called during parsing to handle PR-specific fields
+		// The actual UpdatePullRequestsConfig fields are handled separately since they're not in baseConfig
+	}
 
+	baseConfig := c.parseUpdateEntityConfig(outputMap, params, updatePullRequestLog, parseSpecificFields)
+	if baseConfig == nil {
+		return nil
+	}
+
+	// Create UpdatePullRequestsConfig and populate it
+	updatePullRequestsConfig := &UpdatePullRequestsConfig{
+		UpdateEntityConfig: *baseConfig,
+	}
+
+	// Parse PR-specific fields
+	if configData, exists := outputMap["update-pull-request"]; exists {
+		if configMap, ok := configData.(map[string]any); ok {
 			// Parse title - boolean to enable/disable (defaults to true if nil or not set)
 			if titleVal, exists := configMap["title"]; exists {
 				if titleBool, ok := titleVal.(bool); ok {
@@ -99,17 +106,8 @@ func (c *Compiler) parseUpdatePullRequestsConfig(outputMap map[string]any) *Upda
 				}
 				// If present but not a bool (e.g., null), leave as nil (defaults to enabled)
 			}
-
-			// Parse common base fields with default max of 1
-			c.parseBaseSafeOutputConfig(configMap, &updatePullRequestsConfig.BaseSafeOutputConfig, 1)
-		} else {
-			// If configData is nil or not a map (e.g., "update-pull-request:" with no value),
-			// still set the default max
-			updatePullRequestsConfig.Max = 1
 		}
-
-		return updatePullRequestsConfig
 	}
 
-	return nil
+	return updatePullRequestsConfig
 }