refactor: extract generic parseConfigScaffold helper to eliminate safe-output parser duplication

pkg/workflow/add_labels.go

@@ -16,23 +16,14 @@ type AddLabelsConfig struct {
 
 // parseAddLabelsConfig handles add-labels configuration
 func (c *Compiler) parseAddLabelsConfig(outputMap map[string]any) *AddLabelsConfig {
-	// Check if the key exists
-	if _, exists := outputMap["add-labels"]; !exists {
-		return nil
-	}
-
-	addLabelsLog.Print("Parsing add-labels configuration")
-
-	// Unmarshal into typed config struct
-	var config AddLabelsConfig
-	if err := unmarshalConfig(outputMap, "add-labels", &config, addLabelsLog); err != nil {
+	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)
 	}
-
-	addLabelsLog.Printf("Parsed configuration: allowed_count=%d, blocked_count=%d, target=%s", len(config.Allowed), len(config.Blocked), config.Target)
-
-	return &config
+	return config
 }

pkg/workflow/assign_milestone.go

@@ -15,25 +15,15 @@ type AssignMilestoneConfig struct {
 
 // parseAssignMilestoneConfig handles assign-milestone configuration
 func (c *Compiler) parseAssignMilestoneConfig(outputMap map[string]any) *AssignMilestoneConfig {
-	// Check if the key exists
-	if _, exists := outputMap["assign-milestone"]; !exists {
-		assignMilestoneLog.Print("No assign-milestone configuration found")
-		return nil
-	}
-
-	assignMilestoneLog.Print("Parsing assign-milestone configuration")
-
-	// Unmarshal into typed config struct
-	var config AssignMilestoneConfig
-	if err := unmarshalConfig(outputMap, "assign-milestone", &config, assignMilestoneLog); err != nil {
+	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))
 	}
-
-	assignMilestoneLog.Printf("Parsed milestone config: target=%s, allowed_count=%d",
-		config.Target, len(config.Allowed))
-
-	return &config
+	return config
 }

pkg/workflow/assign_to_agent.go

@@ -23,13 +23,11 @@ type AssignToAgentConfig struct {
 
 // parseAssignToAgentConfig handles assign-to-agent configuration
 func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToAgentConfig {
-	// Check if the key exists
+	// Check key existence first so we can preprocess templatable fields before YAML unmarshaling
 	if _, exists := outputMap["assign-to-agent"]; !exists {
 		return nil
 	}
 
-	assignToAgentLog.Print("Parsing assign-to-agent configuration")
-
 	// Get config data for pre-processing before YAML unmarshaling
 	configData, _ := outputMap["assign-to-agent"].(map[string]any)
 
@@ -39,12 +37,13 @@ func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToA
 		return nil
 	}
 
-	// Unmarshal into typed config struct
-	var config AssignToAgentConfig
-	if err := unmarshalConfig(outputMap, "assign-to-agent", &config, assignToAgentLog); err != 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
@@ -55,5 +54,5 @@ func (c *Compiler) parseAssignToAgentConfig(outputMap map[string]any) *AssignToA
 	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
+	return config
 }

pkg/workflow/assign_to_user.go

@@ -17,13 +17,11 @@ type AssignToUserConfig struct {
 
 // parseAssignToUserConfig handles assign-to-user configuration
 func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUserConfig {
-	// Check if the key exists
+	// Check key existence first so we can preprocess templatable fields before YAML unmarshaling
 	if _, exists := outputMap["assign-to-user"]; !exists {
 		return nil
 	}
 
-	assignToUserLog.Print("Parsing assign-to-user configuration")
-
 	// Get config data for pre-processing before YAML unmarshaling
 	configData, _ := outputMap["assign-to-user"].(map[string]any)
 
@@ -39,15 +37,16 @@ func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUs
 		return nil
 	}
 
-	// Unmarshal into typed config struct
-	var config AssignToUserConfig
-	if err := unmarshalConfig(outputMap, "assign-to-user", &config, assignToUserLog); err != 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")
-		config = AssignToUserConfig{
+		return &AssignToUserConfig{
 			BaseSafeOutputConfig: BaseSafeOutputConfig{Max: defaultIntStr(1)},
 		}
+	})
+	if config == nil {
+		return nil
 	}
 
 	// Set default max if not specified
@@ -57,5 +56,5 @@ func (c *Compiler) parseAssignToUserConfig(outputMap map[string]any) *AssignToUs
 
 	assignToUserLog.Printf("Parsed configuration: allowed_count=%d, target=%s", len(config.Allowed), config.Target)
 
-	return &config
+	return config
 }

pkg/workflow/config_helpers.go

@@ -31,6 +31,9 @@
 // Configuration Integer Parsing:
 //   - parseExpiresFromConfig() - Extract expiration time
 //   - parseRelativeTimeSpec() - Parse relative time specifications
+//
+// Parser Scaffold:
+//   - parseConfigScaffold() - Generic safe-output config parser scaffold
 
 package workflow
 
@@ -241,3 +244,48 @@ func unmarshalConfig(m map[string]any, key string, target any, log *logger.Logge
 
 	return nil
 }
+
+// parseConfigScaffold is the generic parser scaffold for safe-output handler config parsers.
+// It implements the common four-step pattern shared across safe-output handlers:
+//  1. Key existence check – returns nil immediately if the key is absent in outputMap
+//  2. Entry log: "Parsing <key> configuration"
+//  3. Typed unmarshal via unmarshalConfig into a zero-value T
+//  4. On unmarshal error: delegate to the onError callback, which handles
+//     additional logging and returns the appropriate fallback (or nil to disable)
+//
+// The caller is responsible for any preprocessing (e.g. preprocessIntFieldAsString)
+// that must happen before YAML unmarshaling, and for any postprocessing (e.g. setting
+// default max values or computing derived fields) after a successful parse.
+//
+// Example – empty-config fallback:
+//
+//	config := parseConfigScaffold(outputMap, "add-labels", addLabelsLog,
+//	    func(err error) *AddLabelsConfig {
+//	        addLabelsLog.Printf("Failed to unmarshal config: %v", err)
+//	        addLabelsLog.Print("Using empty configuration (allows any labels)")
+//	        return &AddLabelsConfig{}
+//	    })
+//
+// Example – disable-on-error:
+//
+//	config := parseConfigScaffold(outputMap, "set-issue-type", setIssueTypeLog,
+//	    func(err error) *SetIssueTypeConfig {
+//	        setIssueTypeLog.Printf("Failed to unmarshal config, disabling handler: %v", err)
+//	        return nil
+//	    })
+func parseConfigScaffold[T any](
+	outputMap map[string]any,
+	key string,
+	log *logger.Logger,
+	onError func(err error) *T,
+) *T {
+	if _, exists := outputMap[key]; !exists {
+		return nil
+	}
+	log.Printf("Parsing %s configuration", key)
+	var config T
+	if err := unmarshalConfig(outputMap, key, &config, log); err != nil {
+		return onError(err)
+	}
+	return &config
+}

pkg/workflow/config_scaffold_helpers_test.go

@@ -0,0 +1,136 @@
+//go:build !integration
+
+package workflow
+
+import (
+	"testing"
+
+	"github.com/github/gh-aw/pkg/logger"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// testScaffoldConfig is a minimal config type used only in scaffold helper tests.
+type testScaffoldConfig struct {
+	Name    string   `yaml:"name,omitempty"`
+	Allowed []string `yaml:"allowed,omitempty"`
+}
+
+var testScaffoldLog = logger.New("workflow:test_scaffold")
+
+func TestParseConfigScaffold_KeyAbsent(t *testing.T) {
+	outputMap := map[string]any{
+		"other-key": map[string]any{"name": "value"},
+	}
+
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			t.Error("onError should not be called when key is absent")
+			return nil
+		})
+
+	assert.Nil(t, result, "should return nil when key is absent")
+}
+
+func TestParseConfigScaffold_KeyPresentNullValue(t *testing.T) {
+	// Null YAML value (nil) should unmarshal into a zero-value struct successfully
+	outputMap := map[string]any{
+		"my-key": nil,
+	}
+
+	onErrorCalled := false
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			onErrorCalled = true
+			return nil
+		})
+
+	assert.False(t, onErrorCalled, "onError should not be called for nil (null) config value")
+	require.NotNil(t, result, "should return non-nil zero-value config for null config")
+	assert.Empty(t, result.Name, "zero-value Name should be empty string")
+}
+
+func TestParseConfigScaffold_ValidConfig(t *testing.T) {
+	outputMap := map[string]any{
+		"my-key": map[string]any{
+			"name":    "test-name",
+			"allowed": []any{"foo", "bar"},
+		},
+	}
+
+	onErrorCalled := false
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			onErrorCalled = true
+			return nil
+		})
+
+	assert.False(t, onErrorCalled, "onError should not be called for valid config")
+	require.NotNil(t, result, "should return parsed config")
+	assert.Equal(t, "test-name", result.Name, "should parse name field")
+	assert.Equal(t, []string{"foo", "bar"}, result.Allowed, "should parse allowed array")
+}
+
+func TestParseConfigScaffold_EmptyConfigFallback(t *testing.T) {
+	// A non-map value causes unmarshal to fail → onError must return empty fallback
+	outputMap := map[string]any{
+		"my-key": "not-a-map",
+	}
+
+	fallback := &testScaffoldConfig{Name: "fallback"}
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			assert.Error(t, err, "onError should receive the unmarshal error")
+			return fallback
+		})
+
+	assert.Equal(t, fallback, result, "should return the fallback from onError")
+}
+
+func TestParseConfigScaffold_DisableOnError(t *testing.T) {
+	// A non-map value causes unmarshal to fail → onError returns nil (disable handler)
+	outputMap := map[string]any{
+		"my-key": "not-a-map",
+	}
+
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			return nil
+		})
+
+	assert.Nil(t, result, "should return nil when onError disables the handler")
+}
+
+func TestParseConfigScaffold_OnErrorReceivesError(t *testing.T) {
+	outputMap := map[string]any{
+		"my-key": "not-a-map",
+	}
+
+	var capturedErr error
+	parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			capturedErr = err
+			return nil
+		})
+
+	require.Error(t, capturedErr, "onError should receive a non-nil error")
+}
+
+func TestParseConfigScaffold_EmptyMap(t *testing.T) {
+	// An empty map should unmarshal into a zero-value struct successfully
+	outputMap := map[string]any{
+		"my-key": map[string]any{},
+	}
+
+	onErrorCalled := false
+	result := parseConfigScaffold(outputMap, "my-key", testScaffoldLog,
+		func(err error) *testScaffoldConfig {
+			onErrorCalled = true
+			return nil
+		})
+
+	assert.False(t, onErrorCalled, "onError should not be called for empty map config")
+	require.NotNil(t, result, "should return non-nil zero-value config for empty map")
+	assert.Empty(t, result.Name, "zero-value Name should be empty string")
+	assert.Nil(t, result.Allowed, "zero-value Allowed should be nil")
+}

pkg/workflow/mark_pull_request_as_ready_for_review.go

@@ -15,38 +15,31 @@ type MarkPullRequestAsReadyForReviewConfig struct {
 
 // parseMarkPullRequestAsReadyForReviewConfig handles mark-pull-request-as-ready-for-review configuration
 func (c *Compiler) parseMarkPullRequestAsReadyForReviewConfig(outputMap map[string]any) *MarkPullRequestAsReadyForReviewConfig {
-	// Check if the key exists
-	if _, exists := outputMap["mark-pull-request-as-ready-for-review"]; !exists {
-		markPullRequestAsReadyForReviewLog.Print("No configuration found for mark-pull-request-as-ready-for-review")
+	config := parseConfigScaffold(outputMap, "mark-pull-request-as-ready-for-review", markPullRequestAsReadyForReviewLog,
+		func(err error) *MarkPullRequestAsReadyForReviewConfig {
+			return nil
+		})
+	if config == nil {
 		return nil
 	}
 
-	markPullRequestAsReadyForReviewLog.Print("Parsing mark-pull-request-as-ready-for-review configuration")
-
-	// Get the configuration map
+	// 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 {
-			// Handle null case - use empty config
 			configMap = make(map[string]any)
 		}
 	}
 
-	// Unmarshal into typed config struct
-	var config MarkPullRequestAsReadyForReviewConfig
-	if err := unmarshalConfig(outputMap, "mark-pull-request-as-ready-for-review", &config, markPullRequestAsReadyForReviewLog); err != nil {
-		return nil
-	}
-
-	// Parse common target configuration (target, target-repo)
 	targetConfig, _ := ParseTargetConfig(configMap)
 	config.SafeOutputTargetConfig = targetConfig
 
-	// Parse filter configuration (required-labels, required-title-prefix)
 	filterConfig := ParseFilterConfig(configMap)
 	config.SafeOutputFilterConfig = filterConfig
 
-	return &config
+	return config
 }