-
Notifications
You must be signed in to change notification settings - Fork 31
Closed
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-64pkg/workflow/update_pull_request.go:16-66pkg/workflow/update_release.go:13-45
- Code Sample:
The same block appears in
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, })
update_pull_request.goandupdate_release.gowith 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
- 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.
- Add a helper like
- Unify config parsing
- Move the shared
parseBaseSafeOutputConfigpattern 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.
- Move the shared
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/.cjsfiles (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
Copilot