-
Notifications
You must be signed in to change notification settings - Fork 268
Closed
Description
🔍 Duplicate Code Detected: Close Issue vs Close PR Workflows
Analysis of commit 419149f
Assignee: @copilot
Summary
pkg/workflow/js/close_issue.cjs and pkg/workflow/js/close_pull_request.cjs each implement their own versions of the same workflow: load agent output, filter items, validate staged mode, enforce label/title filters, resolve targets (triggering, *, explicit ID), and emit almost identical staged previews. The two files share ~120 lines of near-copy-paste logic, differing only in entity naming (issue vs pull request) and which field holds the number.
Duplication Details
Pattern: Close-entity orchestration
- Severity: Medium
- Occurrences: 2 (issues and pull requests)
- Locations:
pkg/workflow/js/close_issue.cjs(lines 70-210)pkg/workflow/js/close_pull_request.cjs(lines 70-210)
- Code Sample:
const isStaged = process.env.GH_AW_SAFE_OUTPUTS_STAGED === "true"; const result = loadAgentOutput(); if (!result.success) return; const items = result.items.filter(item => item.type === TYPE); if (items.length === 0) { core.info(`No ${TYPE} items found in agent output`); return; } const requiredLabels = (process.env.REQUIRED_LABELS || "") .split(",") .map(l => l.trim()) .filter(Boolean); const target = process.env.TARGET || "triggering"; if (isStaged) { // build preview markdown, including target URLs and filters await core.summary.addRaw(summaryContent).write(); return; } if (target === "triggering" && !isContextMatch) { core.info("Target is triggering but context does not match, skipping"); return; } // iterate items for (const item of items) { const number = resolveTargetNumber(item, target, context); const entity = await fetchEntity(number); if (!passesLabelFilter(entity.labels, requiredLabels)) continue; await closeEntity(number, item.body); }
Impact Analysis
- Maintainability: Any tweak to staged previews, filter semantics, or target resolution must be edited in two places, risking divergence between issue vs PR behavior.
- Bug Risk: The files have already drifted (e.g., copy text and environment variable names), and future changes could break one path while the other continues to work, making triage harder.
- Code Bloat: The large duplicate blocks bury the entity-specific logic (REST calls) and make it harder to reason about the workflow.
Refactoring Recommendations
- Extract shared
closeEntitieshelper- Accept parameters for
itemType,numberField, staged preview strings, and callbacks to fetch & close the entity. - Benefits: One place to maintain staged-mode preview formatting, label/title filtering, and target resolution logic; simplifies both scripts.
- Accept parameters for
- Unify configuration parsing
- Provide a small config object describing env var names for labels/title prefix so the helper can parse them generically.
- Benefits: Ensures label/target validation remains consistent and reduces future copy/paste errors.
Implementation Checklist
- Introduce shared helper for staged preview + closing workflow logic
- Update
close_issue.cjsandclose_pull_request.cjsto call the helper with entity-specific callbacks - Add regression tests covering both issue and PR closure paths
- Verify staged preview output still contains entity-specific metadata
- Document the helper usage for future close-* scripts
Analysis Metadata
- Analyzed Files: 2
.cjsscripts (close_issue, close_pull_request) - Detection Method: Manual Serena-guided comparison of staged preview + target resolution blocks
- Commit: 419149f
- Analysis Date: 2025-12-03
AI generated by Duplicate Code Detector
Reactions are currently unavailable