-
Notifications
You must be signed in to change notification settings - Fork 267
Closed
Description
🔍 Duplicate Code Detected: Safe-Output Update Job Builders
Analysis of commit bc2ada90cdc66c1e44df93ecd0ef877b72ea8343
Assignee: @copilot
Summary
All of the Go builders for update-type safe outputs (update_issue, update_pull_request, update_release, etc.) manually repeat the same boilerplate: validate configuration, construct a customEnvVars slice, append the shared environment block, build identical outputs maps, and call buildSafeOutputJob. The only differences are the env var names, script, permissions, and output keys. The duplication spans ~40 lines per function and already diverges (e.g., some build job conditions, others don’t), making the code hard to maintain.
Duplication Details
Pattern: Repeated buildSafeOutputJob scaffolding
- Severity: Medium
- Occurrences: 3 primary builders in this commit (
update_issue,update_pull_request,update_release) plus other update jobs with the same shape. - Locations:
pkg/workflow/update_issue.go:17-63pkg/workflow/update_pull_request.go:16-65pkg/workflow/update_release.go:13-56
- Code Sample:
if data.SafeOutputs == nil || data.SafeOutputs.UpdateIssues == nil {
return nil, fmt.Errorf("safe-outputs.update-issue configuration is required")
}
customEnvVars := []string{
fmt.Sprintf(" GH_AW_UPDATE_STATUS: %t\n", data.SafeOutputs.UpdateIssues.Status != nil),
fmt.Sprintf(" GH_AW_UPDATE_TITLE: %t\n", data.SafeOutputs.UpdateIssues.Title != nil),
}
customEnvVars = append(customEnvVars, c.buildStandardSafeOutputEnvVars(data, data.SafeOutputs.UpdateIssues.TargetRepoSlug)...)
outputs := map[string]string{
"issue_number": "${{ steps.update_issue.outputs.issue_number }}",
"issue_url": "${{ steps.update_issue.outputs.issue_url }}",
}
return c.buildSafeOutputJob(data, SafeOutputJobConfig{
JobName: "update_issue",
StepName: "Update Issue",
Script: getUpdateIssueScript(),
Permissions: NewPermissionsContentsReadIssuesWrite(),
Outputs: outputs,
Token: data.SafeOutputs.UpdateIssues.GitHubToken,
TargetRepoSlug:data.SafeOutputs.UpdateIssues.TargetRepoSlug,
})Impact Analysis
- Maintainability: Adding a new update-type safe output currently requires copying the entire builder, increasing the chance of missed fields (e.g., job conditions present in
update_issuebut not others). - Bug Risk: Fixes to shared behavior (logging, staged env vars, permission hardening) must be made in every function; inconsistencies are already starting to appear.
- Code Bloat: Each builder repeats ~40 lines of boilerplate that could live in a single helper, inflating
pkg/workflow.
Refactoring Recommendations
- Create a parametric helper (
buildUpdateSafeOutputJob)- Accepts the job name, safe-output config, env var overrides, output key map, permissions, and optional condition builder. The helper can assemble env vars, call
buildSafeOutputJob, and handle token/target-slug wiring once. - Estimated effort: Medium; requires introducing a struct with the variable fields and updating existing builders to call it.
- Benefits: Centralized logic for env var construction, permissions, and job conditions; easier to audit.
- Accepts the job name, safe-output config, env var overrides, output key map, permissions, and optional condition builder. The helper can assemble env vars, call
- Unify job-condition handling
- Let the helper take an optional
ConditionBuilderso that only the differing predicate (issue vs PR) is provided, preventing future drift where some builders forget to guard against missing context. - Estimated effort: Low once helper exists.
- Let the helper take an optional
Implementation Checklist
- Review duplication findings
- Prioritize refactoring tasks
- Create refactoring plan
- Implement changes
- Update tests
- Verify no functionality broken
Analysis Metadata
- Analyzed Files: 3 Go builders (issue, pull request, release) plus related update jobs
- Detection Method: Serena semantic code analysis (
find_symbol, manual diffing) - Commit:
bc2ada90cdc66c1e44df93ecd0ef877b72ea8343 - Analysis Date: 2025-11-28
AI generated by Duplicate Code Detector
Reactions are currently unavailable