Split logs_download.go into focused files (flatten, zip, artifacts) by Copilot · Pull Request #50052 · github/gh-aw

Copilot · 2026-08-03T17:26:22Z

pkg/cli/logs_download.go had grown to 1151 lines mixing four distinct concerns (zip extraction, directory flattening, artifact listing/filtering, and download orchestration), making it hard to navigate and test in isolation.

File splits

logs_download_flatten.go (321 lines): post-download directory normalization — flattenSingleFileArtifacts, findArtifactDir, flattenArtifactTree, flattenUnifiedArtifact, flattenActivationArtifact, flattenAgentOutputsArtifact, flattenSafeOutputsItemsArtifact, flattenDownloadedArtifacts
logs_download_zip.go (115 lines): zip archive extraction with path-traversal/decompression-bomb protections — unzipFile, extractZipFile
logs_download_artifacts.go (263 lines): artifact discovery, filtering, and per-name download/retry — listArtifacts, isNonZipArtifactError, isCaseCollisionArtifactError, isDockerBuildArtifact, listRunArtifactNames, downloadArtifactsByName, retryCriticalArtifacts, logVerboseDownloadSummary
logs_download.go (522 lines, retained): top-level orchestration — isUsageOnlyArtifactFilter, shouldDownloadWorkflowRunLogs, downloadWorkflowRunLogs, downloadRunArtifacts, ensureUsageAwInfoFallback

Reducing `downloadRunArtifacts` complexity

This function alone was ~390 lines, mixing cache-checks, bulk/individual download strategies, and post-processing. Its post-download tail was extracted into two named helpers so the core download logic is easier to follow:

if err := flattenDownloadedArtifacts(ctx, opts); err != nil {
    return err
}
...
if opts.verbose {
    logVerboseDownloadSummary(opts)
}

downloadRunArtifacts is now ~322 lines.

All extractions are code moves — no logic changes, no public API changes.

Fixes [file-diet] Refactor: pkg/cli/logs_download.go exceeds 800-line threshold (1151 lines) #49999

Branch refresh requested by PR Sous Chef.
Run: https://github.com/github/gh-aw/actions/runs/30851084299

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 4.22 AIC · ⌖ 6.83 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13.7 AIC · ⌖ 4.79 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

github-actions · 2026-08-03T17:35:36Z

Thanks for getting this refactoring underway, @app/copilot-swe-agent! 🎯

This PR tackles the file-diet issue head-on — breaking up the 1151-line logs_download.go into four focused modules (flatten, zip, artifacts, and orchestration). The strategy is well-aligned with the project's preference for many small, single-responsibility files.

A few notes as this moves forward:

Status: This is a WIP draft with no changes committed yet. When you're ready to push code, the refactored files should land in this PR.
Tests: Ensure each new file has corresponding test coverage (>80% target per the issue plan), especially logs_download_zip_test.go for the security-sensitive path-traversal and case-collision cases.
Validation: Once code is pushed, make sure make test-unit and make lint pass — those are the gatekeeping checks here.

The refactoring plan is thorough and the split boundaries are clean. Looking forward to seeing the implementation!

Generated by ✅ Contribution Check · auto · 48.7 AIC · ⌖ 5.57 AIC · ⊞ 8.8K · ◷

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

github-actions · 2026-08-03T18:55:23Z

PR Triage: #50052

Category: refactor
Risk: medium
Score: 38/100 (Impact 20 + Urgency 8 + Quality 10)
Recommended action: batch_review
Batch: pr-batch:go-refactor

Draft, large pure code-organization refactor splitting logs_download.go (702+/632-). No behavior change expected; batch with other refactor PRs.

Generated by 🔧 PR Triage Agent · auto · 51.1 AIC · ⌖ 4.08 AIC · ⊞ 8K · ◷

Copilot

Pull request overview

Splits artifact download logic into focused modules while preserving existing APIs and behavior.

Changes:

Separates ZIP extraction, artifact flattening, and artifact discovery/download logic.
Extracts post-download flattening and verbose-summary helpers.
Retains top-level orchestration in logs_download.go.

Show a summary per file

File	Description
`pkg/cli/logs_download.go`	Retains download orchestration and delegates extracted operations.
`pkg/cli/logs_download_zip.go`	Contains protected ZIP extraction logic.
`pkg/cli/logs_download_flatten.go`	Normalizes downloaded artifact directory structures.
`pkg/cli/logs_download_artifacts.go`	Handles artifact discovery, filtering, retries, and summaries.

Review details

Tip

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

Files reviewed: 4/4 changed files
Comments generated: 2
Review effort level: Balanced

+
+	// Copy the content with size limit enforcement
+	// Use LimitReader to prevent reading more than declared size
+	limitedReader := io.LimitReader(srcFile, int64(maxFileSize))


pelikhan · 2026-08-03T20:07:54Z

/matt

github-actions · 2026-08-03T20:08:39Z

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

github-actions

Skills-Based Review 🧠

Applied /codebase-design — pure code-move refactor, no logic changes. Two issues worth addressing; nothing blocking.

📋 Key Themes & Highlights

Issues Found

Dead post-condition check in extractZipFile (zip.go:109): uint64(written) > maxFileSize is unreachable because io.LimitReader already prevents reading beyond the limit. It gives false assurance without providing real protection. See inline comment.
Duplicated repoFlag construction (artifacts.go): the 6-line hostname/owner/repo assembly is copy-pasted between downloadArtifactsByName and retryCriticalArtifacts. Should be a shared helper. See inline comment.

Positive Highlights

✅ Clean file split with well-named concerns — flatten, zip, artifacts, orchestration.
✅ Good existing security annotations (#nosec with explanation, decompression-bomb guard).
✅ extractErr named-return pattern for safe close-error propagation is correct and well-commented.
✅ All extractions are pure moves; no behaviour changes.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 47.9 AIC · ⌖ 10.9 AIC · ⊞ 7.1K
_{Comment /matt to run again}

gh-aw-bot · 2026-08-03T21:11:59Z

@copilot This PR still needs another pass before maintainers can investigate quickly.

Review feedback remains from Matt Pocock Skills Reviewer:
- dead post-condition check in ZIP extraction: Split logs_download.go into focused files (flatten, zip, artifacts) #50052 (review)
- duplicated repoFlag assembly in artifact retry/download paths: Split logs_download.go into focused files (flatten, zip, artifacts) #50052 (review)
Please address the remaining review feedback, refresh the branch if needed, then run the pr-finisher skill and post a concise merge-readiness summary for the current head.

Generated by PR Sous Chef · run: https://github.com/github/gh-aw/actions/runs/30853138313

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 13.7 AIC · ⌖ 4.79 AIC · ⊞ 8.3K · ◷
_{Comment /souschef to run again}

- logs_download_zip.go: use LimitReader(maxFileSize+1) so the written>maxFileSize check is now reachable and correctly rejects oversized content; also remove the inaccurate 'case-collision' claim from the file comment. - logs_download_artifacts.go: extract duplicated owner/repo/hostname → repoFlag assembly into buildRepoFlag helper and replace both call sites. Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Copilot · 2026-08-03T21:24:08Z

@copilot This PR still needs another pass before maintainers can investigate quickly.

Review feedback remains from Matt Pocock Skills Reviewer:...

Both issues addressed in commit Fix unreachable size check in zip extractor and extract repoFlag helper:

Dead post-condition check (logs_download_zip.go): LimitReader limit raised to maxFileSize+1 — now if the reader yields that many bytes the content exceeds the limit, making the written > int64(maxFileSize) guard reachable and effective. Also removed the inaccurate "case-collision" claim from the file comment.
Duplicated repoFlag assembly (logs_download_artifacts.go): extracted into a buildRepoFlag(owner, repo, hostname) helper; both downloadArtifactsByName and retryCriticalArtifacts now call it.

github-actions · 2026-08-04T18:04:06Z

🎉 This pull request is included in a new release.

Release: v0.84.4

Initial plan

e5d156b

Copilot AI assigned Copilot and pelikhan Aug 3, 2026

Copilot AI linked an issue Aug 3, 2026 that may be closed by this pull request

[file-diet] Refactor: pkg/cli/logs_download.go exceeds 800-line threshold (1151 lines) #49999

Closed

8 tasks

Copilot started work on behalf of pelikhan August 3, 2026 17:27 View session

github-actions Bot mentioned this pull request Aug 3, 2026

[Contribution Check Report] Contribution Check — 2026-08-03 #49876

Closed

Copilot AI and others added 2 commits August 3, 2026 17:38

Split logs_download.go into focused files (flatten, zip, artifacts)

0f8c8f8

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Further split downloadRunArtifacts into flatten/summary helpers

7dc4814

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Refactor pkg/cli/logs_download.go for better maintainability~~ Split logs_download.go into focused files (flatten, zip, artifacts) Aug 3, 2026

Copilot finished work on behalf of pelikhan August 3, 2026 17:46

Copilot AI requested a review from pelikhan August 3, 2026 17:46

github-actions Bot added pr-action:batch_review pr-agent:github-copilot pr-batch:go-refactor pr-priority:low pr-risk:medium pr-type:refactor labels Aug 3, 2026

github-actions Bot mentioned this pull request Aug 3, 2026

[PR Triage Report] PR Triage Report — 2026-08-03 (14 PRs) #50066

Closed

pelikhan marked this pull request as ready for review August 3, 2026 19:19

Copilot AI review requested due to automatic review settings August 3, 2026 19:19

Copilot started reviewing on behalf of pelikhan August 3, 2026 19:19 View session

Copilot AI reviewed Aug 3, 2026

View reviewed changes

Comment thread pkg/cli/logs_download_zip.go Outdated

// Copy the content with size limit enforcement

// Use LimitReader to prevent reading more than declared size

limitedReader := io.LimitReader(srcFile, int64(maxFileSize))

Comment thread pkg/cli/logs_download_zip.go Outdated

Merge branch 'main' into copilot/file-diet-refactor-logs-download

ad606c1

github-actions Bot reviewed Aug 3, 2026

View reviewed changes

Merge branch 'main' into copilot/file-diet-refactor-logs-download

7274eb5

github-actions Bot mentioned this pull request Aug 3, 2026

[pr-sous-chef] PR Sous Chef run report - 2026-08-03T00:15:52Z #49872

Closed

Merge branch 'main' into copilot/file-diet-refactor-logs-download

8ffc701

Copilot started work on behalf of gh-aw-bot August 3, 2026 21:12 View session

Copilot AI requested a review from gh-aw-bot August 3, 2026 21:24

Copilot finished work on behalf of gh-aw-bot August 3, 2026 21:24

Merge branch 'main' into copilot/file-diet-refactor-logs-download

075bfee

pelikhan merged commit 93170cc into main Aug 3, 2026
28 checks passed

pelikhan deleted the copilot/file-diet-refactor-logs-download branch August 3, 2026 22:08

Navigation Menu

Uh oh!

Split logs_download.go into focused files (flatten, zip, artifacts) - #50052

Split logs_download.go into focused files (flatten, zip, artifacts)#50052
pelikhan merged 8 commits into
mainfrom
copilot/file-diet-refactor-logs-download

Copilot AI commented Aug 3, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

pelikhan commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026 •

edited

Loading

Uh oh!

github-actions Bot left a comment

Uh oh!

gh-aw-bot commented Aug 3, 2026

Uh oh!

Copilot AI commented Aug 3, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Copilot AI commented Aug 3, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

File splits

Reducing downloadRunArtifacts complexity

Uh oh!

github-actions Bot commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026

PR Triage: #50052

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Review details

Uh oh!

Uh oh!

pelikhan commented Aug 3, 2026

Uh oh!

github-actions Bot commented Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Skills-Based Review 🧠

Issues Found

Positive Highlights

Uh oh!

gh-aw-bot commented Aug 3, 2026

Uh oh!

Copilot AI commented Aug 3, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Aug 3, 2026 •

edited by github-actions Bot

Loading

Reducing `downloadRunArtifacts` complexity

github-actions Bot commented Aug 3, 2026 •

edited

Loading