Duplicate Code: Repeated safe-output parser scaffold in pkg/workflow handlers

[github-actions]

Overview

Recent duplication analysis on commit 5626426d25eab5baf5c4a2c1bb154c740556b519 found a repeated parser scaffold across multiple safe-output handler config parsers in pkg/workflow.

The repeated pattern includes key existence checks, near-identical logging, typed config unmarshaling, fallback/default handling, and final summary logging. This appears in enough places to warrant extraction into a shared parser helper.

Critical Information

• Severity: Medium
• Pattern type: Structural duplication (copy-paste with minor field differences)
• Occurrences identified: 8+ parser methods (threshold met: 3+)
• Typical duplicated size: ~12-20 lines per method (threshold met: >10 lines)

Duplication Details

Pattern: Safe-output config parser scaffold

• Common repeated flow:

  1. Check outputMap["<key>"] existence
  2. Emit Parsing ... configuration log
  3. var config <Type> + unmarshalConfig(...)
  4. On error, fallback/disable/default behavior
  5. Final parsed-summary log + return &config

• Representative locations:

  • pkg/workflow/add_labels.go:17
  • pkg/workflow/remove_labels.go:17
  • pkg/workflow/set_issue_type.go:16
  • pkg/workflow/unassign_from_user.go:17
  • pkg/workflow/assign_milestone.go:16
  • pkg/workflow/assign_to_user.go:18
  • pkg/workflow/assign_to_agent.go:24
  • pkg/workflow/mark_pull_request_as_ready_for_review.go:16

Code Sample (Representative)

// add_labels.go
if _, exists := outputMap["add-labels"]; !exists {
    return nil
}
addLabelsLog.Print("Parsing add-labels configuration")
var config AddLabelsConfig
if err := unmarshalConfig(outputMap, "add-labels", &config, addLabelsLog); err != nil {
    addLabelsLog.Printf("Failed to unmarshal config: %v", err)
    return &AddLabelsConfig{}
}
return &config

// remove_labels.go
if _, exists := outputMap["remove-labels"]; !exists {
    return nil
}
removeLabelsLog.Print("Parsing remove-labels configuration")
var config RemoveLabelsConfig
if err := unmarshalConfig(outputMap, "remove-labels", &config, removeLabelsLog); err != nil {
    removeLabelsLog.Printf("Failed to unmarshal config: %v", err)
    return &RemoveLabelsConfig{}
}
return &config

// set_issue_type.go
if _, exists := outputMap["set-issue-type"]; !exists {
    return nil
}
setIssueTypeLog.Print("Parsing set-issue-type configuration")
var config SetIssueTypeConfig
if err := unmarshalConfig(outputMap, "set-issue-type", &config, setIssueTypeLog); err != nil {
    setIssueTypeLog.Printf("Failed to unmarshal set-issue-type config, disabling handler: %v", err)
    return nil
}
return &config

Impact Analysis

• Maintainability: Shared behavior updates require touching many files.
• Bug risk: Error/fallback behavior can drift between handlers over time.
• Code bloat: Boilerplate obscures handler-specific logic.

Refactoring Recommendations

1. Extract a generic parser scaffold helper

   • Suggested location: pkg/workflow/config_helpers.go
   • Inputs: config key, typed destination, logger, optional preprocess hook, optional fallback hook, optional postprocess hook.
   • Estimated effort: Medium (3-6 hours).
   • Benefit: Centralizes repeated flow and reduces drift risk.

2. Group default/fallback behaviors into reusable policies

   • Example policies: disableOnError, emptyConfigOnError, defaultMaxOnError.
   • Estimated effort: Small-Medium.
   • Benefit: Preserves per-handler semantics while removing duplicate control flow.

3. Add focused tests around shared helper semantics

   • Validate null config, invalid type, fallback behavior, and logging invariants.
   • Estimated effort: Small.
   • Benefit: Safe migration of existing handlers.

Implementation Checklist

• Define and implement shared parser scaffold helper
• Migrate add-labels and remove-labels first (lowest-risk pair)
• Migrate set-issue-type and unassign-from-user
• Migrate remaining safe-output parser methods with same shape
• Add/adjust unit tests for fallback/default behaviors
• Verify no behavior regressions in compiled workflow output

Analysis Metadata

• Detection method: Serena semantic analysis (get_symbols_overview, find_symbol, find_referencing_symbols, search_for_pattern)
• Scope: .go files under pkg/ (excluding tests)
• Analyzed commit: 5626426d25eab5baf5c4a2c1bb154c740556b519
• Workflow run: 23539689258

References:

• §23539689258

Generated by Duplicate Code Detector · ◷