diff --git a/docs/adr/52158-deduplicate-safe-output-parser-wrappers-with-post-process-helper.md b/docs/adr/52158-deduplicate-safe-output-parser-wrappers-with-post-process-helper.md new file mode 100644 index 00000000000..096a8d843a6 --- /dev/null +++ b/docs/adr/52158-deduplicate-safe-output-parser-wrappers-with-post-process-helper.md @@ -0,0 +1,47 @@ +# ADR-52158: Deduplicate Safe-Output Parser Wrappers with a Post-Process Helper + +**Date**: 2026-08-11 +**Status**: Proposed — pending maintainer acceptance on merge +**Deciders**: pelikhan (via copilot-swe-agent, PR #52158) + +--- + +### Context + +The `pkg/workflow` package provides safe-output handlers for GitHub Actions workflow steps. Each handler calls the shared `parseConfigScaffold` helper to unmarshal its YAML config, but then repeats the same boilerplate: check whether the returned pointer is nil, apply default field values, and emit a post-parse debug log line. This pattern appeared identically across 13+ non-test files (`add_comment.go`, `add_labels.go`, `add_reviewer.go`, `assign_milestone.go`, `assign_to_agent.go`, `assign_to_user.go`, `close_entity_helpers.go`, `mark_pull_request_as_ready_for_review.go`, `remove_labels.go`, `replace_label.go`, `set_issue_field.go`, `set_issue_type.go`, `unassign_from_user.go`). Because each copy was hand-maintained, defaulting rules and log formatting could drift silently when one handler received a fix that peers did not. + +### Decision + +We will introduce `parseConfigScaffoldWithPostProcess[T any]` in `pkg/workflow/config_helpers.go`. This generic wrapper calls `parseConfigScaffold`, and if the result is non-nil and a `postProcess` callback is provided, invokes that callback before returning. All 13+ existing callers are refactored to use the new helper, moving their nil-check, default-value assignment, and post-parse logging into the `postProcess` closure. Handlers with genuinely unique pre-parse logic (e.g., `create_entity_helpers.go`) are left unchanged. No behavioral changes are introduced; existing defaults, error messages, and log formats are preserved exactly. + +The callback contract is: `postProcess` runs for **every** non-nil result, including a non-nil fallback returned by `onError`, and is skipped when the result is nil (key absent, or `onError` returned nil to disable the handler). This keeps defaulting and logging consistent between successfully parsed configs and error fallbacks. The contract is pinned down by direct tests in `config_scaffold_helpers_test.go` covering valid config, non-nil error fallback, nil error fallback, absent key, and a nil callback. + +### Alternatives Considered + +#### Alternative 1: Extend `parseConfigScaffold` directly with an optional postProcess parameter + +Add a `postProcess func(config *T)` parameter to the existing `parseConfigScaffold` signature so all callers can supply post-processing inline without a new function. + +This was not chosen because changing `parseConfigScaffold`'s signature would require updating every existing call site (including those that pass `nil` or do not need post-processing), and would complicate the function's existing contract around nil handling and error fallback. A thin wrapper preserves backward compatibility and keeps `parseConfigScaffold` focused. + +#### Alternative 2: Define a `PostProcessable` interface on each config type + +Give each config struct a `PostProcess()` method implementing a shared interface, and have `parseConfigScaffold` detect and call it via a type assertion or a separate generic constraint. + +This was not chosen because it would scatter post-processing logic across many struct definitions rather than co-locating it with the handler's parse function. It also increases coupling between the generic scaffold and the concrete config types, making the scaffold harder to reason about in isolation. + +### Consequences + +#### Positive +- The nil-check-and-callback wrapper is written once and reused across all handlers, eliminating 13+ nearly identical code blocks. +- Default-value policy and post-parse logging conventions can be changed in a single `postProcess` closure per handler rather than tracked across many files. +- New handlers automatically follow the pattern by passing a `postProcess` closure instead of copying boilerplate. +- Handler files are shorter and easier to read. + +#### Negative +- `config_helpers.go` grows by one additional generic function, adding to the surface area readers must understand when onboarding. +- Handlers with unusual post-processing (e.g., extracting nested map keys as in `mark_pull_request_as_ready_for_review.go`) must still write non-trivial closures, so the helper provides less benefit for those cases. + +#### Neutral +- `create_entity_helpers.go` is explicitly excluded from the refactor because it already owns a distinct `parseCreateEntityConfig` scaffold with pre- and post-hooks; the two scaffolds coexist without conflict. +- The refactor is purely internal to `pkg/workflow`; no public API or configuration schema changes. diff --git a/pkg/workflow/add_comment.go b/pkg/workflow/add_comment.go index e12e0f6e223..14d72768177 100644 --- a/pkg/workflow/add_comment.go +++ b/pkg/workflow/add_comment.go @@ -69,19 +69,18 @@ func (c *Compiler) parseCommentsConfig(outputMap map[string]any) *AddCommentsCon return nil } - config := parseConfigScaffold(outputMap, "add-comment", addCommentLog, func(err error) *AddCommentsConfig { - addCommentLog.Printf("Failed to unmarshal config: %v", err) - // For backward compatibility, handle nil/empty config - return &AddCommentsConfig{} - }) - if config == nil { - return nil - } - - // Set default max if not specified - if config.Max == nil { - config.Max = defaultIntStr(1) - } + config := parseConfigScaffoldWithPostProcess(outputMap, "add-comment", addCommentLog, + func(err error) *AddCommentsConfig { + addCommentLog.Printf("Failed to unmarshal config: %v", err) + // For backward compatibility, handle nil/empty config + return &AddCommentsConfig{} + }, + func(config *AddCommentsConfig) { + // Set default max if not specified + if config.Max == nil { + config.Max = defaultIntStr(1) + } + }) return config } diff --git a/pkg/workflow/add_labels.go b/pkg/workflow/add_labels.go index 6eea6b7fa4c..d488a60b9d8 100644 --- a/pkg/workflow/add_labels.go +++ b/pkg/workflow/add_labels.go @@ -20,16 +20,16 @@ type AddLabelsConfig struct { // parseAddLabelsConfig handles add-labels configuration func (c *Compiler) parseAddLabelsConfig(outputMap map[string]any) *AddLabelsConfig { - config := parseConfigScaffold(outputMap, "add-labels", addLabelsLog, func(err error) *AddLabelsConfig { - addLabelsLog.Printf("Failed to unmarshal config: %v", err) - // Handle null case: create empty config (allows any labels) - addLabelsLog.Print("Using empty configuration (allows any labels)") - return &AddLabelsConfig{} - }) - if config != nil { - addLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "add-labels", addLabelsLog, + func(err error) *AddLabelsConfig { + addLabelsLog.Printf("Failed to unmarshal config: %v", err) + // Handle null case: create empty config (allows any labels) + addLabelsLog.Print("Using empty configuration (allows any labels)") + return &AddLabelsConfig{} + }, + func(config *AddLabelsConfig) { + addLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) + }) } // buildAddLabelsPermissions computes the permissions for add_labels based on config. diff --git a/pkg/workflow/add_reviewer.go b/pkg/workflow/add_reviewer.go index 19705b1a667..47fb32b4c73 100644 --- a/pkg/workflow/add_reviewer.go +++ b/pkg/workflow/add_reviewer.go @@ -51,29 +51,28 @@ func (c *Compiler) parseAddReviewerConfig(outputMap map[string]any) *AddReviewer return nil } - config := parseConfigScaffold(outputMap, "add-reviewer", addReviewerLog, func(err error) *AddReviewerConfig { - addReviewerLog.Printf("Failed to unmarshal config: %v", err) - // For backward compatibility, handle nil/empty config - return &AddReviewerConfig{} - }) - if config == nil { - return nil - } - - // Set default max if not specified - if config.Max == nil { - config.Max = defaultIntStr(3) - } + config := parseConfigScaffoldWithPostProcess(outputMap, "add-reviewer", addReviewerLog, + func(err error) *AddReviewerConfig { + addReviewerLog.Printf("Failed to unmarshal config: %v", err) + // For backward compatibility, handle nil/empty config + return &AddReviewerConfig{} + }, + func(config *AddReviewerConfig) { + // Set default max if not specified + if config.Max == nil { + config.Max = defaultIntStr(3) + } - // Fallback from deprecated field names to preferred names - if len(config.AllowedReviewers) == 0 { - config.AllowedReviewers = config.Reviewers - } - if len(config.AllowedTeamReviewers) == 0 { - config.AllowedTeamReviewers = config.TeamReviewers - } + // Fallback from deprecated field names to preferred names + if len(config.AllowedReviewers) == 0 { + config.AllowedReviewers = config.Reviewers + } + if len(config.AllowedTeamReviewers) == 0 { + config.AllowedTeamReviewers = config.TeamReviewers + } - addReviewerLog.Printf("Parsed add-reviewer config: allowed_reviewers=%d, target=%s", len(config.AllowedReviewers), config.Target) + addReviewerLog.Printf("Parsed add-reviewer config: allowed_reviewers=%d, target=%s", len(config.AllowedReviewers), config.Target) + }) return config } diff --git a/pkg/workflow/assign_milestone.go b/pkg/workflow/assign_milestone.go index c8f97e61bb3..dd36c1bebd8 100644 --- a/pkg/workflow/assign_milestone.go +++ b/pkg/workflow/assign_milestone.go @@ -17,15 +17,15 @@ type AssignMilestoneConfig struct { // parseAssignMilestoneConfig handles assign-milestone configuration func (c *Compiler) parseAssignMilestoneConfig(outputMap map[string]any) *AssignMilestoneConfig { - config := parseConfigScaffold(outputMap, "assign-milestone", assignMilestoneLog, func(err error) *AssignMilestoneConfig { - assignMilestoneLog.Printf("Failed to unmarshal config: %v", err) - // Handle null case: create empty config (allows any milestones) - assignMilestoneLog.Print("Null milestone config, allowing any milestones") - return &AssignMilestoneConfig{} - }) - if config != nil { - assignMilestoneLog.Printf("Parsed milestone config: target=%s, allowed_count=%d", - config.Target, len(config.Allowed)) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "assign-milestone", assignMilestoneLog, + func(err error) *AssignMilestoneConfig { + assignMilestoneLog.Printf("Failed to unmarshal config: %v", err) + // Handle null case: create empty config (allows any milestones) + assignMilestoneLog.Print("Null milestone config, allowing any milestones") + return &AssignMilestoneConfig{} + }, + func(config *AssignMilestoneConfig) { + assignMilestoneLog.Printf("Parsed milestone config: target=%s, allowed_count=%d", + config.Target, len(config.Allowed)) + }) } diff --git a/pkg/workflow/assign_to_agent.go b/pkg/workflow/assign_to_agent.go index 82daafaac53..5a661c0be59 100644 --- a/pkg/workflow/assign_to_agent.go +++ b/pkg/workflow/assign_to_agent.go @@ -37,22 +37,21 @@ func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToA return nil } - config := parseConfigScaffold(outputMap, "assign-to-agent", assignToAgentLog, func(err error) *AssignToAgentConfig { - assignToAgentLog.Printf("Failed to unmarshal config: %v", err) - // Handle null case: create empty config - return &AssignToAgentConfig{} - }) - if config == nil { - return nil - } - - // Set default max if not specified - if config.Max == nil { - config.Max = defaultIntStr(1) - } - - assignToAgentLog.Printf("Parsed assign-to-agent config: default_agent=%s, default_model=%s, default_custom_agent=%s, allowed_count=%d, target=%s, max=%s, pull_request_repo=%s, base_branch=%s", - config.DefaultAgent, config.DefaultModel, config.DefaultCustomAgent, len(config.Allowed), config.Target, *config.Max, config.PullRequestRepoSlug, config.BaseBranch) + config := parseConfigScaffoldWithPostProcess(outputMap, "assign-to-agent", assignToAgentLog, + func(err error) *AssignToAgentConfig { + assignToAgentLog.Printf("Failed to unmarshal config: %v", err) + // Handle null case: create empty config + return &AssignToAgentConfig{} + }, + func(config *AssignToAgentConfig) { + // Set default max if not specified + if config.Max == nil { + config.Max = defaultIntStr(1) + } + + assignToAgentLog.Printf("Parsed assign-to-agent config: default_agent=%s, default_model=%s, default_custom_agent=%s, allowed_count=%d, target=%s, max=%s, pull_request_repo=%s, base_branch=%s", + config.DefaultAgent, config.DefaultModel, config.DefaultCustomAgent, len(config.Allowed), config.Target, *config.Max, config.PullRequestRepoSlug, config.BaseBranch) + }) return config } diff --git a/pkg/workflow/assign_to_user.go b/pkg/workflow/assign_to_user.go index a48c3ac2465..a9e951ba685 100644 --- a/pkg/workflow/assign_to_user.go +++ b/pkg/workflow/assign_to_user.go @@ -37,24 +37,23 @@ func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUs return nil } - config := parseConfigScaffold(outputMap, "assign-to-user", assignToUserLog, func(err error) *AssignToUserConfig { - assignToUserLog.Printf("Failed to unmarshal config: %v", err) - // For backward compatibility, use defaults - assignToUserLog.Print("Using default configuration") - return &AssignToUserConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: defaultIntStr(1)}, - } - }) - if config == nil { - return nil - } - - // Set default max if not specified - if config.Max == nil { - config.Max = defaultIntStr(1) - } - - assignToUserLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + config := parseConfigScaffoldWithPostProcess(outputMap, "assign-to-user", assignToUserLog, + func(err error) *AssignToUserConfig { + assignToUserLog.Printf("Failed to unmarshal config: %v", err) + // For backward compatibility, use defaults + assignToUserLog.Print("Using default configuration") + return &AssignToUserConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: defaultIntStr(1)}, + } + }, + func(config *AssignToUserConfig) { + // Set default max if not specified + if config.Max == nil { + config.Max = defaultIntStr(1) + } + + assignToUserLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + }) return config } diff --git a/pkg/workflow/close_entity_helpers.go b/pkg/workflow/close_entity_helpers.go index fb82d4cf829..82bf534c079 100644 --- a/pkg/workflow/close_entity_helpers.go +++ b/pkg/workflow/close_entity_helpers.go @@ -129,27 +129,26 @@ func (c *Compiler) parseCloseEntityConfig(outputMap map[string]any, params Close } } - config := parseConfigScaffold(outputMap, params.ConfigKey, logger, func(err error) *CloseEntityConfig { - logger.Printf("Failed to unmarshal config: %v", err) - // For backward compatibility, handle nil/empty config - return &CloseEntityConfig{} - }) - if config == nil { - return nil - } - - // Set default max if not specified - if config.Max == nil { - config.Max = defaultIntStr(1) - logger.Printf("Set default max to 1 for %s", params.ConfigKey) - } + config := parseConfigScaffoldWithPostProcess(outputMap, params.ConfigKey, logger, + func(err error) *CloseEntityConfig { + logger.Printf("Failed to unmarshal config: %v", err) + // For backward compatibility, handle nil/empty config + return &CloseEntityConfig{} + }, + func(config *CloseEntityConfig) { + // Set default max if not specified + if config.Max == nil { + config.Max = defaultIntStr(1) + logger.Printf("Set default max to 1 for %s", params.ConfigKey) + } - // Backward compatibility: map deprecated title-prefix to required-title-prefix. - if config.RequiredTitlePrefix == "" && config.TitlePrefix != "" { - config.RequiredTitlePrefix = config.TitlePrefix - } + // Backward compatibility: map deprecated title-prefix to required-title-prefix. + if config.RequiredTitlePrefix == "" && config.TitlePrefix != "" { + config.RequiredTitlePrefix = config.TitlePrefix + } - logger.Printf("Parsed %s configuration: max=%s, target=%s", params.ConfigKey, *config.Max, config.Target) + logger.Printf("Parsed %s configuration: max=%s, target=%s", params.ConfigKey, *config.Max, config.Target) + }) return config } diff --git a/pkg/workflow/config_helpers.go b/pkg/workflow/config_helpers.go index bd83ad70dd7..3b953a44d4e 100644 --- a/pkg/workflow/config_helpers.go +++ b/pkg/workflow/config_helpers.go @@ -31,6 +31,8 @@ // // Parser Scaffold: // - parseConfigScaffold() - Generic safe-output config parser scaffold +// - parseConfigScaffoldWithPostProcess() - parseConfigScaffold plus an optional +// post-parse callback for defaulting/logging, avoiding repeated nil-check wrappers package workflow @@ -278,3 +280,40 @@ func parseConfigScaffold[T any]( } return &config } + +// parseConfigScaffoldWithPostProcess wraps parseConfigScaffold and additionally invokes the +// optional postProcess callback for every non-nil result. This includes non-nil fallback +// configs returned by onError, not just successfully unmarshalled ones, so that defaults and +// logging apply consistently to fallbacks. postProcess is skipped when the result is nil +// (key absent, or onError returned nil to disable the handler). +// +// This removes the repeated "if config == nil { return nil } ... return config" wrapper that +// most safe-output handlers need around parseConfigScaffold for applying default values and/or +// emitting a post-parse debug log line. +// +// postProcess may be nil if no post-processing is required. +// +// Example: +// +// return parseConfigScaffoldWithPostProcess(outputMap, "assign-milestone", assignMilestoneLog, +// func(err error) *AssignMilestoneConfig { +// assignMilestoneLog.Printf("Failed to unmarshal config: %v", err) +// return &AssignMilestoneConfig{} +// }, +// func(config *AssignMilestoneConfig) { +// assignMilestoneLog.Printf("Parsed milestone config: target=%s, allowed_count=%d", +// config.Target, len(config.Allowed)) +// }) +func parseConfigScaffoldWithPostProcess[T any]( + outputMap map[string]any, + key string, + debugLog *logger.Logger, + onError func(err error) *T, + postProcess func(config *T), +) *T { + config := parseConfigScaffold(outputMap, key, debugLog, onError) + if config != nil && postProcess != nil { + postProcess(config) + } + return config +} diff --git a/pkg/workflow/config_scaffold_helpers_test.go b/pkg/workflow/config_scaffold_helpers_test.go index ea9ff1401d8..473748596b9 100644 --- a/pkg/workflow/config_scaffold_helpers_test.go +++ b/pkg/workflow/config_scaffold_helpers_test.go @@ -134,3 +134,93 @@ func TestParseConfigScaffold_EmptyMap(t *testing.T) { assert.Empty(t, result.Name, "zero-value Name should be empty string") assert.Nil(t, result.Allowed, "zero-value Allowed should be nil") } + +func TestParseConfigScaffoldWithPostProcess_ValidConfig(t *testing.T) { + outputMap := map[string]any{ + "my-key": map[string]any{"name": "test-name"}, + } + + result := parseConfigScaffoldWithPostProcess(outputMap, "my-key", testScaffoldLog, + func(err error) *testScaffoldConfig { + t.Error("onError should not be called for a valid config") + return nil + }, + func(config *testScaffoldConfig) { + config.Allowed = []string{"default"} + }) + + require.NotNil(t, result, "should return the parsed config") + assert.Equal(t, "test-name", result.Name, "parsed field should be preserved") + assert.Equal(t, []string{"default"}, result.Allowed, "postProcess should apply defaults") +} + +func TestParseConfigScaffoldWithPostProcess_NonNilErrorFallback(t *testing.T) { + // A non-map value causes unmarshal to fail → onError returns a non-nil fallback, + // which must still be passed through postProcess. + outputMap := map[string]any{ + "my-key": "not-a-map", + } + + postProcessCalled := false + result := parseConfigScaffoldWithPostProcess(outputMap, "my-key", testScaffoldLog, + func(err error) *testScaffoldConfig { + return &testScaffoldConfig{} + }, + func(config *testScaffoldConfig) { + postProcessCalled = true + config.Name = "defaulted" + }) + + assert.True(t, postProcessCalled, "postProcess should run for a non-nil error fallback") + require.NotNil(t, result, "should return the fallback config") + assert.Equal(t, "defaulted", result.Name, "postProcess should mutate the fallback config") +} + +func TestParseConfigScaffoldWithPostProcess_KeyAbsent(t *testing.T) { + outputMap := map[string]any{ + "other-key": map[string]any{"name": "value"}, + } + + result := parseConfigScaffoldWithPostProcess(outputMap, "my-key", testScaffoldLog, + func(err error) *testScaffoldConfig { + t.Error("onError should not be called when key is absent") + return nil + }, + func(config *testScaffoldConfig) { + t.Error("postProcess should not be called when key is absent") + }) + + assert.Nil(t, result, "should return nil when key is absent") +} + +func TestParseConfigScaffoldWithPostProcess_NilErrorFallback(t *testing.T) { + outputMap := map[string]any{ + "my-key": "not-a-map", + } + + result := parseConfigScaffoldWithPostProcess(outputMap, "my-key", testScaffoldLog, + func(err error) *testScaffoldConfig { + return nil + }, + func(config *testScaffoldConfig) { + t.Error("postProcess should not be called when onError returns nil") + }) + + assert.Nil(t, result, "should return nil when onError disables the handler") +} + +func TestParseConfigScaffoldWithPostProcess_NilPostProcess(t *testing.T) { + outputMap := map[string]any{ + "my-key": map[string]any{"name": "test-name"}, + } + + result := parseConfigScaffoldWithPostProcess(outputMap, "my-key", testScaffoldLog, + func(err error) *testScaffoldConfig { + t.Error("onError should not be called for a valid config") + return nil + }, + nil) + + require.NotNil(t, result, "should return the parsed config with a nil postProcess") + assert.Equal(t, "test-name", result.Name, "parsed field should be preserved") +} diff --git a/pkg/workflow/mark_pull_request_as_ready_for_review.go b/pkg/workflow/mark_pull_request_as_ready_for_review.go index cf9b368bbc5..4dd5d93160c 100644 --- a/pkg/workflow/mark_pull_request_as_ready_for_review.go +++ b/pkg/workflow/mark_pull_request_as_ready_for_review.go @@ -16,32 +16,30 @@ type MarkPullRequestAsReadyForReviewConfig struct { // parseMarkPullRequestAsReadyForReviewConfig handles mark-pull-request-as-ready-for-review configuration func (c *Compiler) parseMarkPullRequestAsReadyForReviewConfig(outputMap map[string]any) *MarkPullRequestAsReadyForReviewConfig { markPullRequestAsReadyForReviewLog.Print("Parsing mark-pull-request-as-ready-for-review config") - config := parseConfigScaffold(outputMap, "mark-pull-request-as-ready-for-review", markPullRequestAsReadyForReviewLog, + config := parseConfigScaffoldWithPostProcess(outputMap, "mark-pull-request-as-ready-for-review", markPullRequestAsReadyForReviewLog, func(err error) *MarkPullRequestAsReadyForReviewConfig { return nil + }, + func(config *MarkPullRequestAsReadyForReviewConfig) { + // Postprocess: parse common target configuration (target, target-repo) and + // filter configuration (required-labels, required-title-prefix) from the raw map, + // as these fields require additional extraction beyond YAML unmarshaling. + var configMap map[string]any + if configVal, exists := outputMap["mark-pull-request-as-ready-for-review"]; exists { + if cfgMap, ok := configVal.(map[string]any); ok { + configMap = cfgMap + } else { + configMap = make(map[string]any) + } + } + + targetConfig, _ := ParseTargetConfig(configMap) + config.SafeOutputTargetConfig = targetConfig + + filterConfig := ParseFilterConfig(configMap) + config.SafeOutputFilterConfig = filterConfig + + markPullRequestAsReadyForReviewLog.Printf("Parsed mark-pull-request-as-ready-for-review config: target=%s", config.Target) }) - if config == nil { - return nil - } - - // Postprocess: parse common target configuration (target, target-repo) and - // filter configuration (required-labels, required-title-prefix) from the raw map, - // as these fields require additional extraction beyond YAML unmarshaling. - var configMap map[string]any - if configVal, exists := outputMap["mark-pull-request-as-ready-for-review"]; exists { - if cfgMap, ok := configVal.(map[string]any); ok { - configMap = cfgMap - } else { - configMap = make(map[string]any) - } - } - - targetConfig, _ := ParseTargetConfig(configMap) - config.SafeOutputTargetConfig = targetConfig - - filterConfig := ParseFilterConfig(configMap) - config.SafeOutputFilterConfig = filterConfig - - markPullRequestAsReadyForReviewLog.Printf("Parsed mark-pull-request-as-ready-for-review config: target=%s", config.Target) return config } diff --git a/pkg/workflow/remove_labels.go b/pkg/workflow/remove_labels.go index 39c55807610..e1e9bdb4d54 100644 --- a/pkg/workflow/remove_labels.go +++ b/pkg/workflow/remove_labels.go @@ -16,14 +16,14 @@ type RemoveLabelsConfig struct { // parseRemoveLabelsConfig handles remove-labels configuration func (c *Compiler) parseRemoveLabelsConfig(outputMap map[string]any) *RemoveLabelsConfig { - config := parseConfigScaffold(outputMap, "remove-labels", removeLabelsLog, func(err error) *RemoveLabelsConfig { - removeLabelsLog.Printf("Failed to unmarshal config: %v", err) - // Handle null case: create empty config (allows any labels) - removeLabelsLog.Print("Using empty configuration (allows any labels)") - return &RemoveLabelsConfig{} - }) - if config != nil { - removeLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "remove-labels", removeLabelsLog, + func(err error) *RemoveLabelsConfig { + removeLabelsLog.Printf("Failed to unmarshal config: %v", err) + // Handle null case: create empty config (allows any labels) + removeLabelsLog.Print("Using empty configuration (allows any labels)") + return &RemoveLabelsConfig{} + }, + func(config *RemoveLabelsConfig) { + removeLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target) + }) } diff --git a/pkg/workflow/replace_label.go b/pkg/workflow/replace_label.go index 95f49725696..e4eeeec0f12 100644 --- a/pkg/workflow/replace_label.go +++ b/pkg/workflow/replace_label.go @@ -28,15 +28,15 @@ type ReplaceLabelConfig struct { // parseReplaceLabelConfig handles replace-label configuration func (c *Compiler) parseReplaceLabelConfig(outputMap map[string]any) *ReplaceLabelConfig { - config := parseConfigScaffold(outputMap, "replace-label", replaceLabelLog, func(err error) *ReplaceLabelConfig { - replaceLabelLog.Printf("Failed to unmarshal config: %v", err) - // Handle null case: create empty config (allows any labels) - replaceLabelLog.Print("Using empty configuration (allows any labels)") - return &ReplaceLabelConfig{} - }) - if config != nil { - replaceLabelLog.Printf("Parsed configuration: allowed_add_count=%d, allowed_remove_count=%d, blocked_count=%d, allowed_transitions_count=%d, target=%s", - len(config.AllowedAdd), len(config.AllowedRemove), len(config.Blocked), len(config.AllowedTransitions), config.Target) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "replace-label", replaceLabelLog, + func(err error) *ReplaceLabelConfig { + replaceLabelLog.Printf("Failed to unmarshal config: %v", err) + // Handle null case: create empty config (allows any labels) + replaceLabelLog.Print("Using empty configuration (allows any labels)") + return &ReplaceLabelConfig{} + }, + func(config *ReplaceLabelConfig) { + replaceLabelLog.Printf("Parsed configuration: allowed_add_count=%d, allowed_remove_count=%d, blocked_count=%d, allowed_transitions_count=%d, target=%s", + len(config.AllowedAdd), len(config.AllowedRemove), len(config.Blocked), len(config.AllowedTransitions), config.Target) + }) } diff --git a/pkg/workflow/set_issue_field.go b/pkg/workflow/set_issue_field.go index 1cc5a800d30..4fea31c2af7 100644 --- a/pkg/workflow/set_issue_field.go +++ b/pkg/workflow/set_issue_field.go @@ -14,12 +14,12 @@ type SetIssueFieldConfig struct { // parseSetIssueFieldConfig handles set-issue-field configuration. func (c *Compiler) parseSetIssueFieldConfig(outputMap map[string]any) *SetIssueFieldConfig { - config := parseConfigScaffold(outputMap, "set-issue-field", setIssueFieldLog, func(err error) *SetIssueFieldConfig { - setIssueFieldLog.Printf("Failed to unmarshal set-issue-field config, disabling handler: %v", err) - return nil - }) - if config != nil { - setIssueFieldLog.Printf("Parsed configuration: target=%s", config.Target) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "set-issue-field", setIssueFieldLog, + func(err error) *SetIssueFieldConfig { + setIssueFieldLog.Printf("Failed to unmarshal set-issue-field config, disabling handler: %v", err) + return nil + }, + func(config *SetIssueFieldConfig) { + setIssueFieldLog.Printf("Parsed configuration: target=%s", config.Target) + }) } diff --git a/pkg/workflow/set_issue_type.go b/pkg/workflow/set_issue_type.go index 2f0ff7d902c..845ae4db048 100644 --- a/pkg/workflow/set_issue_type.go +++ b/pkg/workflow/set_issue_type.go @@ -16,12 +16,12 @@ type SetIssueTypeConfig struct { // parseSetIssueTypeConfig handles set-issue-type configuration func (c *Compiler) parseSetIssueTypeConfig(outputMap map[string]any) *SetIssueTypeConfig { - config := parseConfigScaffold(outputMap, "set-issue-type", setIssueTypeLog, func(err error) *SetIssueTypeConfig { - setIssueTypeLog.Printf("Failed to unmarshal set-issue-type config, disabling handler: %v", err) - return nil - }) - if config != nil { - setIssueTypeLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) - } - return config + return parseConfigScaffoldWithPostProcess(outputMap, "set-issue-type", setIssueTypeLog, + func(err error) *SetIssueTypeConfig { + setIssueTypeLog.Printf("Failed to unmarshal set-issue-type config, disabling handler: %v", err) + return nil + }, + func(config *SetIssueTypeConfig) { + setIssueTypeLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + }) } diff --git a/pkg/workflow/unassign_from_user.go b/pkg/workflow/unassign_from_user.go index 7189ee35be1..270752a948e 100644 --- a/pkg/workflow/unassign_from_user.go +++ b/pkg/workflow/unassign_from_user.go @@ -16,24 +16,23 @@ type UnassignFromUserConfig struct { // parseUnassignFromUserConfig handles unassign-from-user configuration func (c *Compiler) parseUnassignFromUserConfig(outputMap map[string]any) *UnassignFromUserConfig { - config := parseConfigScaffold(outputMap, "unassign-from-user", unassignFromUserLog, func(err error) *UnassignFromUserConfig { - unassignFromUserLog.Printf("Failed to unmarshal config: %v", err) - // For backward compatibility, use defaults - unassignFromUserLog.Print("Using default configuration") - return &UnassignFromUserConfig{ - BaseSafeOutputConfig: BaseSafeOutputConfig{Max: defaultIntStr(1)}, - } - }) - if config == nil { - return nil - } + config := parseConfigScaffoldWithPostProcess(outputMap, "unassign-from-user", unassignFromUserLog, + func(err error) *UnassignFromUserConfig { + unassignFromUserLog.Printf("Failed to unmarshal config: %v", err) + // For backward compatibility, use defaults + unassignFromUserLog.Print("Using default configuration") + return &UnassignFromUserConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: defaultIntStr(1)}, + } + }, + func(config *UnassignFromUserConfig) { + // 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) - } - - unassignFromUserLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + unassignFromUserLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target) + }) return config }