Skip to content

Refactor: Extract duplicate artifact download code into shared helper#1956

Merged
pelikhan merged 3 commits intomainfrom
copilot/remove-duplicate-code-patterns
Oct 18, 2025
Merged

Refactor: Extract duplicate artifact download code into shared helper#1956
pelikhan merged 3 commits intomainfrom
copilot/remove-duplicate-code-patterns

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Oct 18, 2025

  • Analyze the duplicate code issue
  • Understand the three locations with duplicate artifact download steps
  • Create a shared helper function for artifact downloads with configurable paths
  • Update safe_jobs.go to use the shared helper
  • Update threat_detection.go to use the shared helper
  • Run tests to ensure no regressions
  • Run build and lint checks
  • Run make agent-finish for complete validation
  • Move artifact download helpers to artifacts.go per code review feedback

Summary

Successfully eliminated duplicate artifact download code and organized it into a dedicated file.

Latest Changes (per code review)

Moved to new file pkg/workflow/artifacts.go:

  • ArtifactDownloadConfig struct
  • buildArtifactDownloadSteps() function

Kept in pkg/workflow/safe_output_helpers.go:

  • buildAgentOutputDownloadSteps() wrapper function (specific to safe-outputs)

This improves code organization by separating generic artifact download functionality into its own file, making it easier to find and maintain.

Testing

✅ All unit tests pass
✅ Build succeeds
✅ Linter passes
✅ All artifact download functionality still works correctly

Original prompt

This section details on the original issue you should resolve

<issue_title>[duplicate-code] 🔍 Duplicate Code Detected</issue_title>
<issue_description># 🔍 Duplicate Code Detected

Analysis of commit 9bebf8a

Assignee: @copilot

Summary

Repeated logic for downloading and wiring up the agent output artifact appears across safe output helpers, safe-jobs, and threat-detection jobs. Additionally, frontmatter extraction helpers duplicate the same map-conversion boilerplate.

Duplication Details

Pattern 1: Agent output artifact download steps repeated

  • Severity: Medium
  • Occurrences: 3
  • Locations:
    • pkg/workflow/safe_output_helpers.go:79
    • pkg/workflow/safe_jobs.go:269
    • pkg/workflow/threat_detection.go:142
  • Code Sample:
    steps = append(steps, "      - name: Download agent output artifact\n")
    steps = append(steps, "        continue-on-error: true\n")
    steps = append(steps, "        uses: actions/download-artifact@v5\n")
    steps = append(steps, "        with:\n")
    steps = append(steps, fmt.Sprintf("          name: %s\n", constants.AgentOutputArtifactName))
    steps = append(steps, "          path: <variant-specific path>\n")
    // ... environment bootstrap steps duplicate the same pattern per job

Pattern 2: Frontmatter map extraction helpers duplicated

  • Severity: Low
  • Occurrences: 3
  • Locations:
    • pkg/workflow/tools.go:144
    • pkg/workflow/tools.go:158
    • pkg/workflow/tools.go:168
  • Code Sample:
    func extractXFromFrontmatter(frontmatter map[string]any) map[string]any {
        if raw, exists := frontmatter["x"]; exists {
            if asMap, ok := raw.(map[string]any); ok {
                return asMap
            }
        }
        return make(map[string]any)
    }

Impact Analysis

  • Maintainability: Repeating download/setup logic makes future path or artifact-name changes brittle and scatters responsibility across multiple helpers.
  • Bug Risk: Divergent updates between the copies could break individual safe jobs or detection workflows.
  • Code Bloat: Extra boilerplate inflates workflow compiler complexity and hides the intent of each job builder.

Refactoring Recommendations

  1. Extract reusable artifact download helper

    • Move common “download agent output” + env bootstrap logic into a single utility that accepts the target directory and environment variable options (e.g., buildAgentOutputDownloadSteps(basePath string)), then reuse it in safe output helpers, safe jobs, and threat detection.
    • Estimated effort: 2-3 hours
    • Benefits: Single point of truth for artifact handling and consistent behavior across jobs.
  2. Generalize frontmatter map extraction

    • Introduce a shared helper like getMap(frontmatter, key string) to remove triplicated map-type checks, or inline it where used.
    • Estimated effort: <1 hour
    • Benefits: Less boilerplate and fewer chances for inconsistent nil-handling.

Implementation Checklist

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

Analysis Metadata

  • Analyzed Files: 6
  • Detection Method: Serena semantic code analysis
  • Commit: 9bebf8a
  • Analysis Date: 2025-10-18

AI generated by Duplicate Code Detector</issue_description>

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

Fixes #1955


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: dsyme <7204669+dsyme@users.noreply.github.com>
Copilot AI changed the title [WIP] Remove duplicated code for agent output artifact download Refactor: Extract duplicate artifact download code into shared helper Oct 18, 2025
Copilot AI requested a review from dsyme October 18, 2025 18:03
Comment thread pkg/workflow/safe_output_helpers.go Outdated
// and set the GITHUB_AW_AGENT_OUTPUT environment variable
func buildAgentOutputDownloadSteps() []string {
// ArtifactDownloadConfig holds configuration for building artifact download steps
type ArtifactDownloadConfig struct {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot move to artifacts.go

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Moved ArtifactDownloadConfig and buildArtifactDownloadSteps() to artifacts.go in commit a40cdf6.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan October 18, 2025 18:23
@pelikhan pelikhan marked this pull request as ready for review October 18, 2025 18:28
@github-actions
Copy link
Copy Markdown
Contributor

Agentic Changeset Generator triggered by this pull request.

@pelikhan pelikhan merged commit aa1535f into main Oct 18, 2025
9 of 10 checks passed
@pelikhan pelikhan deleted the copilot/remove-duplicate-code-patterns branch October 18, 2025 18:32
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] 🔍 Duplicate Code Detected

3 participants