Skip to content

refactor: extract shared flattenArtifactTree helper to remove duplicate flatten logic#21172

Merged
pelikhan merged 2 commits intomainfrom
copilot/fix-duplicate-flattening-logic
Mar 16, 2026
Merged

refactor: extract shared flattenArtifactTree helper to remove duplicate flatten logic#21172
pelikhan merged 2 commits intomainfrom
copilot/fix-duplicate-flattening-logic

Conversation

Copy link
Contributor

Copilot AI commented Mar 16, 2026

Three functions in pkg/cli/logs_download.go (flattenUnifiedArtifact, flattenActivationArtifact, flattenAgentOutputsArtifact) contained ~150 lines of near-identical walk → move → cleanup code, making any behavioral fix a triple-apply problem.

Changes

  • New helper flattenArtifactTree(sourceDir, artifactDir, outputDir, label string, verbose bool) error — encapsulates the filepath.Walk traversal, os.MkdirAll/os.Rename per entry, and warn-only os.RemoveAll cleanup; label drives all log and user-facing messages
  • flattenUnifiedArtifact — retains old/new structure detection (tmp/gh-aw prefix check), then delegates with sourceDir != artifactDir when the old layout is present
  • flattenActivationArtifact / flattenAgentOutputsArtifact — reduced to path resolution + single delegate call
  • New tests for flattenArtifactTree in logs_flatten_test.go covering: basic move, nested dirs, detached sourceDir/artifactDir (old-structure case), empty source, multiple files, verbose mode
// Before: ~50 lines each, copy-pasted 3×
func flattenActivationArtifact(outputDir string, verbose bool) error {
    activationDir := findArtifactDir(outputDir, "activation", "")
    // ... 45 lines of walk/move/cleanup ...
}

// After: meaningful logic only
func flattenActivationArtifact(outputDir string, verbose bool) error {
    activationDir := findArtifactDir(outputDir, "activation", "")
    if activationDir == "" {
        return nil
    }
    logsDownloadLog.Printf("Flattening activation artifact directory: %s", activationDir)
    return flattenArtifactTree(activationDir, activationDir, outputDir, "activation artifact", verbose)
}

Copilot AI and others added 2 commits March 16, 2026 04:15
…icate flattening logic

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI linked an issue Mar 16, 2026 that may be closed by this pull request
5 tasks
@pelikhan pelikhan marked this pull request as ready for review March 16, 2026 04:53
Copilot AI review requested due to automatic review settings March 16, 2026 04:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors artifact-flattening in the logs download flow by extracting the shared walk/move/cleanup logic into a single helper, reducing duplication and adding direct unit tests for the helper.

Changes:

  • Added flattenArtifactTree(...) helper to centralize traversal, file moves, and cleanup for artifact directories.
  • Updated unified/activation/agent_outputs flatteners to delegate to the shared helper (while preserving unified old/new structure detection).
  • Added focused unit tests for flattenArtifactTree covering common flatten scenarios.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
pkg/cli/logs_download.go Extracts shared flatten logic into flattenArtifactTree and updates callers to use it.
pkg/cli/logs_flatten_test.go Adds direct unit tests for flattenArtifactTree and introduces testify assertions for the new cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.


if err != nil {
return fmt.Errorf("failed to flatten unified artifact: %w", err)
return fmt.Errorf("failed to flatten %s: %w", label, err)
@pelikhan pelikhan merged commit 3a8a9b7 into main Mar 16, 2026
93 checks passed
@pelikhan pelikhan deleted the copilot/fix-duplicate-flattening-logic branch March 16, 2026 05:05
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 flattening logic across artifact handlers in pkg/cli/logs_download.go

3 participants