Skip to content

Duplicate code: repeated safe-output handler scaffolding across JS handlers #25464

Description

@github-actions

Overview

A large number of actions/setup/js/*.cjs handlers duplicate the same control-flow scaffold (max-limit gate, processedCount state, repeated skip/error response shape, and per-item wrapper structure) instead of sharing one composable factory. This is significant duplication and increases maintenance cost when behavior changes are needed.

Critical Information

  • Severity: Medium
  • Duplication type: Structural duplication (copy-paste with small variations)
  • Scope: 25+ handler modules with near-identical scaffolding
  • Commit analyzed: 0ce475302c05891ae61787204d3d755470a5551b
Examples of repeated pattern

Common repeated block (or near-equivalent) appears in many files:

  • let processedCount = 0;
  • if (processedCount >= maxCount) { ... error: \Max count of ${maxCount} reached` }`
  • processedCount++;
  • handler-specific logic thereafter

Concrete examples:

  • actions/setup/js/add_labels.cjs lines 39-57
  • actions/setup/js/remove_labels.cjs lines 48-67
  • actions/setup/js/assign_to_user.cjs lines 49-68
  • actions/setup/js/unassign_from_user.cjs lines 49-68
  • actions/setup/js/close_issue.cjs lines 110-124
  • actions/setup/js/create_issue.cjs lines 295-324
  • actions/setup/js/create_pull_request.cjs lines 377-403
  • actions/setup/js/dispatch_workflow.cjs lines 78 and 134-138
  • actions/setup/js/report_incomplete_handler.cjs lines 29 and 39-44

Repository-wide pattern search also found the same structure in many additional handlers (add_comment, add_reviewer, assign_milestone, create_discussion, create_project, hide_comment, link_sub_issue, resolve_pr_review_thread, set_issue_type, submit_pr_review, etc.).

Impact Analysis

  • Maintainability: Any change to limit behavior or skip-result schema requires N-file edits.
  • Bug risk: Inconsistent behavior is likely as some handlers evolve and others lag.
  • Code bloat: Repeated boilerplate obscures handler-specific logic.

Refactoring Recommendations

  1. Introduce a shared higher-order handler wrapper for count gating
  • Candidate location: actions/setup/js/handler_factory.cjs (or extend existing factories)
  • Inputs: handlerType, maxCount, handleItem callback, optional onSkip override
  • Output: standardized success/error/skipped/deferred result envelope
  1. Migrate handlers incrementally in cohorts
  • Start with low-risk pair(s): add_labels + remove_labels, assign_to_user + unassign_from_user
  • Then migrate remaining CRUD-like handlers
  • Keep behavior snapshots via current tests before each cohort migration
  1. Reuse existing abstraction points
  • actions/setup/js/update_handler_factory.cjs already centralizes similar concerns for update handlers
  • actions/setup/js/missing_issue_helpers.cjs already uses shared flow; align remaining handlers to similar pattern

Implementation Checklist

  • Define shared scaffold factory/wrapper contract
  • Migrate first cohort (add_labels, remove_labels)
  • Migrate second cohort (assign_to_user, unassign_from_user)
  • Migrate remaining count-gated handlers
  • Verify unchanged behavior with existing handler tests
  • Add one focused test for shared max-limit behavior

Analysis Metadata

  • Analyzed files: non-test .go and .cjs files in current commit scope (snapshot-style commit)
  • Detection method: Serena semantic/pattern analysis + targeted code inspection
  • Trigger: @pelikhan
  • Workflow run: §24188961842

Generated by Duplicate Code Detector ·

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

enhancementNew feature or request

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions