fix: artifact pagination bugs and configurable artifact count limits #2165
Conversation
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
Co-authored-by: Copilot <[email protected]>
- Fix off-by-one error in pagination loop (< to <=) that prevented fetching last page - Add Math.ceil() to maxNumberOfPages calculation for proper limit handling - Replace hardcoded 2000 limit with configurable getMaxArtifactListCount() - Add pagination test for multi-page artifact listing - Add environment variable test for ACTIONS_ARTIFACT_MAX_ARTIFACT_COUNT - Add comprehensive test coverage for getMaxArtifactListCount() function Fixes compound bug where pagination and limit logic capped results at 900 artifacts instead of intended 1000.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes critical pagination bugs in artifact listing and adds configurability for artifact count limits. The changes ensure all artifacts are fetched when paginating through results and allow users to override the default 1000-artifact limit via environment variable.
Key Changes:
- Fixed off-by-one error in pagination loop (changed
<to<=) to fetch the last page - Corrected
maxNumberOfPagescalculation usingMath.ceil()for proper rounding - Replaced hardcoded limit with configurable
getMaxArtifactListCount()function
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/artifact/src/internal/shared/config.ts | Adds new getMaxArtifactListCount() function to support configurable artifact limits via environment variable |
| packages/artifact/src/internal/find/list-artifacts.ts | Fixes pagination logic and replaces hardcoded artifact limit with configurable function |
| packages/artifact/tests/list-artifacts.test.ts | Adds comprehensive tests for multi-page pagination and environment variable override behavior |
| packages/artifact/tests/config.test.ts | Adds unit tests for getMaxArtifactListCount() covering default, override, and error cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Contributor
|
Lgtm |
Link-
reviewed
Oct 22, 2025
Link-
reviewed
Oct 22, 2025
Link-
approved these changes
Oct 24, 2025
mwasilew
added a commit
to mwasilew/meta-qcom
that referenced
this pull request
Nov 6, 2025
v6 release fixes an off-by-one bug when there are more than 100 artifacts to download. More details in actions/toolkit PR#2165 actions/toolkit#2165 The problem manifests as downloading only 100 artifacts when there are more than 100 and fewer than 200. As default page size is 100, only 1st page is retrieved. It affected test result reporting in meta-qcom Signed-off-by: Milosz Wasilewski <[email protected]>
lumag
added a commit
to qualcomm-linux/meta-qcom
that referenced
this pull request
Nov 7, 2025
v6 release fixes an off-by-one bug when there are more than 100 artifacts to download. More details in actions/toolkit PR#2165 actions/toolkit#2165 The problem manifests as downloading only 100 artifacts when there are more than 100 and fewer than 200. As default page size is 100, only 1st page is retrieved. It affected test result reporting in meta-qcom
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
This pull request addresses critical pagination bugs in artifact listing, improves configurability for artifact limits, and adds comprehensive test coverage to prevent regressions. The changes ensure that all intended artifacts are fetched and that environment variable overrides are respected.
Pagination and Limit Handling Improvements
<to<=, ensuring the last page of artifacts is fetched.maxNumberOfPagesto useMath.ceil()for correct pagination when the artifact count is not a multiple of the page size.Configurability Enhancements
getMaxArtifactListCount()function, allowing configuration via theACTIONS_ARTIFACT_MAX_ARTIFACT_COUNTenvironment variable.Testing Improvements
ACTIONS_ARTIFACT_MAX_ARTIFACT_COUNTenvironment variable in artifact listing.getMaxArtifactListCount()to verify default behavior, environment variable overrides, and error handling for invalid values.