From 8267abdaa33abd03b7d03a10d482be8d06966a93 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 21:09:19 +0000 Subject: [PATCH 1/3] Initial plan From 484df2bef7695dc2e1b02d29486a86b9d2d75d46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 21:19:42 +0000 Subject: [PATCH 2/3] Refactor update jobs to use shared helper pattern Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- pkg/workflow/safe_outputs_tools_test.go | 6 +- pkg/workflow/update_entity_helpers.go | 96 +++++++++++++++++++++++++ pkg/workflow/update_issue.go | 78 ++++++++++---------- pkg/workflow/update_pull_request.go | 78 ++++++++++---------- pkg/workflow/update_release.go | 69 +++++++++--------- 5 files changed, 211 insertions(+), 116 deletions(-) create mode 100644 pkg/workflow/update_entity_helpers.go diff --git a/pkg/workflow/safe_outputs_tools_test.go b/pkg/workflow/safe_outputs_tools_test.go index 08f5358ab0..bae78ef1ab 100644 --- a/pkg/workflow/safe_outputs_tools_test.go +++ b/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}}, diff --git a/pkg/workflow/update_entity_helpers.go b/pkg/workflow/update_entity_helpers.go new file mode 100644 index 0000000000..235eb9b3b2 --- /dev/null +++ b/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, + }) +} diff --git a/pkg/workflow/update_issue.go b/pkg/workflow/update_issue.go index 1aa68b822d..7995acc010 100644 --- a/pkg/workflow/update_issue.go +++ b/pkg/workflow/update_issue.go @@ -2,12 +2,15 @@ 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"` + 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 @@ -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 } diff --git a/pkg/workflow/update_pull_request.go b/pkg/workflow/update_pull_request.go index 3ae670439f..d024768c16 100644 --- a/pkg/workflow/update_pull_request.go +++ b/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 } diff --git a/pkg/workflow/update_release.go b/pkg/workflow/update_release.go index a4f1d1992f..64534b3cca 100644 --- a/pkg/workflow/update_release.go +++ b/pkg/workflow/update_release.go @@ -2,12 +2,15 @@ package workflow import ( "fmt" + + "github.com/githubnext/gh-aw/pkg/logger" ) +var updateReleaseLog = logger.New("workflow:update_release") + // UpdateReleaseConfig holds configuration for updating GitHub releases from agent output type UpdateReleaseConfig struct { - BaseSafeOutputConfig `yaml:",inline"` - SafeOutputTargetConfig `yaml:",inline"` + UpdateEntityConfig `yaml:",inline"` } // buildCreateOutputUpdateReleaseJob creates the update_release job using the shared builder @@ -22,9 +25,6 @@ func (c *Compiler) buildCreateOutputUpdateReleaseJob(data *WorkflowData, mainJob // Uses buildStandardSafeOutputEnvVars for consistency with other update jobs var customEnvVars []string - // 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{ "release_id": "${{ steps.update_release.outputs.release_id }}", @@ -32,43 +32,40 @@ func (c *Compiler) buildCreateOutputUpdateReleaseJob(data *WorkflowData, mainJob "release_tag": "${{ steps.update_release.outputs.release_tag }}", } - // Use the shared builder function to create the job - return c.buildSafeOutputJob(data, SafeOutputJobConfig{ - JobName: "update_release", - StepName: "Update Release", - StepID: "update_release", - MainJobName: mainJobName, - CustomEnvVars: customEnvVars, - Script: getUpdateReleaseScript(), - Permissions: NewPermissionsContentsWrite(), - Outputs: outputs, - Token: cfg.GitHubToken, - TargetRepoSlug: cfg.TargetRepoSlug, - }) + // Build job condition - update_release doesn't have event context checks + jobCondition := BuildSafeOutputType("update_release") + + params := UpdateEntityJobParams{ + EntityType: UpdateEntityRelease, + ConfigKey: "update-release", + JobName: "update_release", + StepName: "Update Release", + ScriptGetter: getUpdateReleaseScript, + PermissionsFunc: NewPermissionsContentsWrite, + CustomEnvVars: customEnvVars, + Outputs: outputs, + Condition: jobCondition, + } + + return c.buildUpdateEntityJob(data, mainJobName, &cfg.UpdateEntityConfig, params, updateReleaseLog) } // parseUpdateReleaseConfig handles update-release configuration func (c *Compiler) parseUpdateReleaseConfig(outputMap map[string]any) *UpdateReleaseConfig { - if configData, exists := outputMap["update-release"]; exists { - updateReleaseConfig := &UpdateReleaseConfig{} - - if configMap, ok := configData.(map[string]any); ok { - // Parse target config (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 - } - updateReleaseConfig.SafeOutputTargetConfig = targetConfig + params := UpdateEntityJobParams{ + EntityType: UpdateEntityRelease, + ConfigKey: "update-release", + } - // Parse common base fields with default max of 1 - c.parseBaseSafeOutputConfig(configMap, &updateReleaseConfig.BaseSafeOutputConfig, 1) - } else { - // If configData is nil or not a map, still set the default max - updateReleaseConfig.Max = 1 - } + baseConfig := c.parseUpdateEntityConfig(outputMap, params, updateReleaseLog, nil) + if baseConfig == nil { + return nil + } - return updateReleaseConfig + // Create UpdateReleaseConfig and populate it + updateReleaseConfig := &UpdateReleaseConfig{ + UpdateEntityConfig: *baseConfig, } - return nil + return updateReleaseConfig } From 1c394dfdb6e04c35141cd0622b0057809d9c9992 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 7 Dec 2025 21:28:52 +0000 Subject: [PATCH 3/3] Complete refactoring - all tests pass and validation successful Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com> --- .github/workflows/release.lock.yml | 6 +++--- pkg/workflow/update_issue.go | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/release.lock.yml b/.github/workflows/release.lock.yml index a73e060474..160a26701e 100644 --- a/.github/workflows/release.lock.yml +++ b/.github/workflows/release.lock.yml @@ -6119,19 +6119,19 @@ jobs: - name: Download Go modules run: go mod download - name: Generate SBOM (SPDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 with: artifact-name: sbom.spdx.json format: spdx-json output-file: sbom.spdx.json - name: Generate SBOM (CycloneDX format) - uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0.20.10 + uses: anchore/sbom-action@fbfd9c6c189226748411491745178e0c2017392d # v0 with: artifact-name: sbom.cdx.json format: cyclonedx-json output-file: sbom.cdx.json - name: Upload SBOM artifacts - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5 + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 with: name: sbom-artifacts path: | diff --git a/pkg/workflow/update_issue.go b/pkg/workflow/update_issue.go index 7995acc010..94f96793ac 100644 --- a/pkg/workflow/update_issue.go +++ b/pkg/workflow/update_issue.go @@ -10,10 +10,10 @@ var updateIssueLog = logger.New("workflow:update_issue") // UpdateIssuesConfig holds configuration for updating GitHub issues from agent output type UpdateIssuesConfig struct { - 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 + 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