Skip to content

Refactor update job builders to use shared helper pattern#5789

Merged
pelikhan merged 3 commits intomainfrom
copilot/refactor-safe-output-job-builders
Dec 7, 2025
Merged

Refactor update job builders to use shared helper pattern#5789
pelikhan merged 3 commits intomainfrom
copilot/refactor-safe-output-job-builders

Conversation

Copy link
Contributor

Copilot AI commented Dec 7, 2025

The update-issue, update-pull-request, and update-release job builders duplicated ~120 lines of identical scaffolding: guard clauses, environment variable construction, SafeOutputJobConfig wiring, and condition building.

Changes

  • Created update_entity_helpers.go with parameterized builders following the close_entity_helpers.go pattern:

    • UpdateEntityJobParams struct to specify job-specific details (permissions, outputs, scripts)
    • buildUpdateEntityJob() consolidates the common job construction logic
    • parseUpdateEntityConfig() handles shared config parsing with type-specific field callbacks
  • Refactored concrete builders to use helpers:

    • Each builder now constructs params and delegates to the shared implementation
    • Type-specific config structs embed UpdateEntityConfig for common fields
    • Custom fields (e.g., UpdateIssuesConfig.Status) remain in concrete types

Example

Before (repeated 3x with minor variations):

func (c *Compiler) buildCreateOutputUpdateIssueJob(...) (*Job, error) {
    // 50+ lines of boilerplate
    return c.buildSafeOutputJob(data, SafeOutputJobConfig{
        JobName: "update_issue",
        // 10+ common fields...
    })
}

After:

func (c *Compiler) buildCreateOutputUpdateIssueJob(...) (*Job, error) {
    params := UpdateEntityJobParams{
        EntityType: UpdateEntityIssue,
        JobName: "update_issue",
        // Only job-specific details
    }
    return c.buildUpdateEntityJob(data, mainJobName, &cfg.UpdateEntityConfig, params, log)
}

Future update types (e.g., update-discussion) can reuse the same pattern with minimal code.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/user
    • Triggering command: /usr/bin/gh gh api user --jq .login it/ref/tags/v5 GO111MODULE ps GOINSECURE GOMOD GOMODCACHE ps -go.�� -go.git GO111MODULE e/git GOINSECURE GOMOD GOMODCACHE e/git (http block)
    • Triggering command: /usr/bin/gh gh api user --jq .login .git .git /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/node GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api user --jq .login b-script.git b-script.git ec431a0c15fe40a1d0c375aea5dfcee66bc/log.json GOINSECURE GOMOD GOMODCACHE 1/x64/bin/npm env -json GO111MODULE (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] Safe Output update jobs share repeated builders</issue_title>
<issue_description># 🔍 Duplicate Code Detected: Safe Output Update Jobs

Analysis of commit faacd5c

Assignee: @copilot

Summary

All of the newly added safe-output update jobs (issues, pull requests, releases) repeat the same job-builder scaffolding: guard clauses, environment construction, identical SafeOutputJobConfig wiring, and per-type output wiring. The three structs (UpdateIssuesConfig, UpdatePullRequestsConfig, UpdateReleaseConfig) also share the same parsing approach. This is 100+ lines of near-identical Go that will have to be updated in lock-step for every new update target.

Duplication Details

Pattern: Copy/pasted safe-output job builders

  • Severity: Medium
  • Occurrences: 3 (issues, pull requests, releases)
  • Locations:
    • pkg/workflow/update_issue.go:17-64
    • pkg/workflow/update_pull_request.go:16-66
    • pkg/workflow/update_release.go:13-45
  • Code Sample:
    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:          data.SafeOutputs.UpdateIssues.GitHubToken,
        TargetRepoSlug: data.SafeOutputs.UpdateIssues.TargetRepoSlug,
    })
    The same block appears in update_pull_request.go and update_release.go with only string/permission swaps.

Impact Analysis

  • Maintainability: Any change to how safe-output jobs are configured (new env vars, telemetry, permissions) requires three manual edits today and will grow with every new update type.
  • Bug Risk: Easy to forget a tweak (e.g., staged-mode env vars or condition fixes) in one copy, leading to inconsistent behavior between issue vs PR updates.
  • Code Bloat: Adds ~120 LoC of boilerplate and makes it harder to understand what is unique to each update job.

Refactoring Recommendations

  1. Introduce a data-driven helper
    • Add a helper like buildUpdateJob(job SafeOutputJobConfig, opts UpdateJobOptions) that receives the varying pieces (permissions factory, number output keys, allowed fields) and shares condition/env wiring.
    • Estimated effort: 4-6 hours to extract and update the three call sites.
    • Benefits: one implementation point for staged-mode handling, token/target repo wiring, and outputs.
  2. Unify config parsing
    • Move the shared parseBaseSafeOutputConfig pattern plus target/field parsing into reusable helpers (e.g., parseUpdateTargetConfig(map[string]any, defaults)), so future update types just supply metadata.
    • Estimated effort: 2-3 hours.

Implementation Checklist

  • Review duplication findings
  • Prioritize refactoring tasks
  • Create refactoring plan
  • Implement changes
  • Update tests
  • Verify no functionality broken

Analysis Metadata

  • Analyzed Files: 335 changed .go/.cjs files (focus on new update job scaffolding)
  • Detection Method: Serena semantic code analysis + git diff inspection
  • Commit: faacd5c
  • Analysis Date: 2025-11-30

AI generated by Duplicate Code Detector

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits December 7, 2025 21:19
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor safe output update jobs to eliminate code duplication Refactor update job builders to use shared helper pattern Dec 7, 2025
Copilot AI requested a review from mnkiefer December 7, 2025 21:31
@pelikhan pelikhan marked this pull request as ready for review December 7, 2025 21:54
@pelikhan pelikhan merged commit c1cb03a into main Dec 7, 2025
5 checks passed
@pelikhan pelikhan deleted the copilot/refactor-safe-output-job-builders branch December 7, 2025 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Safe Output update jobs share repeated builders

3 participants