fix: await archiver import before traversing in folders:download --zip - #721
Merged
Conversation
Coverage Report for CI Build 34595979007Coverage at 84.916% (no base build to compare)Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
arjankowski
changed the title
folders:download --ziparchiver import before traversing in folders:download --zip
KwiatkowskiML
previously approved these changes
Sep 11, 2026
Since the archiver 8 upgrade, _setupZip() awaits a dynamic import before assigning this.zip, but run() started folder traversal without waiting for it. When the first folder listing returned before the module loaded, items took the non-zip path, the archive was never finalized and the .zip was left empty (or the command hung). Load the module up front and keep _setupZip() synchronous so this.zip exists before the first item is processed. Add a regression test that asserts the archive is initialized before _getItems() runs, and verify the entries of the produced archive in the existing --zip test. Co-authored-by: Artur Jankowski <ajankowski@box.com>
arjankowski
force-pushed
the
fix/folders-download-zip-race
branch
from
September 11, 2026 11:50
b48162f to
70b5b47
Compare
congminh1254
approved these changes
Sep 11, 2026
Contributor
|
@arjankowski Awesome, thank you. How long until a new version is cut? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: This is a cherry-pick of #719 by @dgilman-perplexity. The original PR was submitted from a fork, which means GitHub does not expose repository secrets to CI — causing the FOSSA license scan to fail. This PR re-applies the same commit from an internal branch so all CI checks can pass.
Since the archiver 8 upgrade, _setupZip() awaits a dynamic import before assigning this.zip, but run() started folder traversal without waiting for it. When the first folder listing returned before the module loaded, items took the non-zip path, the archive was never finalized and the .zip was left empty (or the command hung).
Load the module up front and keep _setupZip() synchronous so this.zip exists before the first item is processed. Add a regression test that asserts the archive is initialized before _getItems() runs, and verify the entries of the produced archive in the existing --zip test.
Fix for #718