From 9e2b7cf3e75493475635059169662fc923e886b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 12:57:44 +0000 Subject: [PATCH 1/2] Re-download truncated e2e-bin archives once download-artifact v4 can finish "successfully" with a truncated file. Run 38059192473 attempt 1 (PR #1345) logged "Starting download" and nothing more, then the unpack step failed with zstd "Read error (39) : premature end" and the hatch e2e_vex_build leg went red. A re-run on the same SHA passed. In the merge queue the same blip evicts the entry and costs a full rebuild, and every merge group downloads e2e-bin once per e2e and cargo-vex leg. Add a composite download-artifact wrapper, alongside the upload one, that runs a caller-supplied check on the downloaded files. When the download or the check fails it clears the path and downloads once more; a second failure fails the job. The two e2e-bin consumers check the archive with `zstd -t`. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KdaGJqi1VLtSWMnHY4hP6g --- .github/actions/download-artifact/action.yml | 71 ++++++++++++++++++++ .github/workflows/ci.yml | 8 ++- scripts/tests/test_ci_e2e_archive.py | 39 +++++++++++ 3 files changed, 116 insertions(+), 2 deletions(-) create mode 100644 .github/actions/download-artifact/action.yml diff --git a/.github/actions/download-artifact/action.yml b/.github/actions/download-artifact/action.yml new file mode 100644 index 000000000..0bb1b7312 --- /dev/null +++ b/.github/actions/download-artifact/action.yml @@ -0,0 +1,71 @@ +name: Download artifact with a verified retry +description: Re-download once when the download fails or leaves files that fail a check +inputs: + pattern: + description: Glob of artifact names to download + required: true + path: + description: Directory to download into (cleared before the retry) + required: true + merge-multiple: + description: Extract every matching artifact into the same directory + default: 'false' + verify: + description: >- + Bash script, run from the workspace, that exits non-zero when the + downloaded files are incomplete or corrupt (e.g. `zstd -t `) + default: 'true' +runs: + using: composite + steps: + # download-artifact v4 can finish "successfully" with a truncated file: + # run 38059192473 attempt 1 logged "Starting download" and nothing more, + # then `zstd -d` failed with "Read error (39) : premature end" in the + # next step and the leg went red. A merge-queue leg that does that + # evicts the entry, so check the files and fetch them once more. + - name: Download (attempt 1) + id: first + if: ${{ !cancelled() }} + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + continue-on-error: true + with: + pattern: ${{ inputs.pattern }} + merge-multiple: ${{ inputs.merge-multiple }} + path: ${{ inputs.path }} + + - name: Verify (attempt 1) + id: first-check + if: ${{ !cancelled() && steps.first.outcome == 'success' }} + continue-on-error: true + shell: bash + env: + VERIFY: ${{ inputs.verify }} + run: bash --noprofile --norc -euo pipefail -c "$VERIFY" + + - name: Clear the failed download + if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + shell: bash + env: + DOWNLOAD_PATH: ${{ inputs.path }} + run: | + set -euo pipefail + echo "::warning::Artifact download or check failed; downloading again" + rm -rf -- "$DOWNLOAD_PATH" + sleep 10 + + # Neither step below continues on error: a second bad download is not + # a blip, and the job must fail rather than test corrupt files. + - name: Download (attempt 2) + if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + with: + pattern: ${{ inputs.pattern }} + merge-multiple: ${{ inputs.merge-multiple }} + path: ${{ inputs.path }} + + - name: Verify (attempt 2) + if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + shell: bash + env: + VERIFY: ${{ inputs.verify }} + run: bash --noprofile --norc -euo pipefail -c "$VERIFY" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 20caf9438..ffccdf6d8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1288,11 +1288,13 @@ jobs: persist-credentials: false - name: Download the e2e binaries - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + # Re-downloads once if the archive arrives truncated. + uses: ./.github/actions/download-artifact with: pattern: e2e-bin-${{ matrix.os }}* merge-multiple: true path: target/e2e-archive + verify: zstd -q -t target/e2e-archive/e2e-bin.tar.zst - name: Unpack the e2e binaries shell: bash @@ -2261,11 +2263,13 @@ jobs: with: persist-credentials: false - name: Download the e2e binaries - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 + # Re-downloads once if the archive arrives truncated. + uses: ./.github/actions/download-artifact with: pattern: e2e-bin-${{ matrix.os }}* merge-multiple: true path: target/e2e-archive + verify: zstd -q -t target/e2e-archive/e2e-bin.tar.zst - name: Unpack the e2e binaries shell: bash # Decompress to a file, not into `| tar -xf -`: a tar that exits at diff --git a/scripts/tests/test_ci_e2e_archive.py b/scripts/tests/test_ci_e2e_archive.py index 3ebbbc5d4..d9291855e 100644 --- a/scripts/tests/test_ci_e2e_archive.py +++ b/scripts/tests/test_ci_e2e_archive.py @@ -119,6 +119,45 @@ def test_corrupt_archive_fails_the_consumer_step(self): (archive_dir / "e2e-bin.tar.zst").write_bytes(b"not a zstd archive") self.assertNotEqual(run(consumer, work).returncode, 0) + def test_download_retries_when_the_archive_arrives_truncated(self): + # download-artifact v4 once left a truncated e2e-bin.tar.zst and + # still passed (run 38059192473): each consumer's verify check must + # reject that file so the wrapper downloads it again. + checks = [] + for job in JOBS.values(): + for step_name, step in rows.steps(job): + if step_name == "Download the e2e binaries": + self.assertIn("uses: ./.github/actions/download-artifact", step) + checks.append(re.search(r"(?m)^ verify: (.+)$", step)[1]) + self.assertTrue(checks) + for check in checks: + with tempfile.TemporaryDirectory(prefix="e2e-truncated-") as directory: + work = Path(directory) + archive_dir = work / "target/e2e-archive" + archive_dir.mkdir(parents=True) + archive = archive_dir / "e2e-bin.tar.zst" + payload = work / "payload" + payload.write_bytes(os.urandom(1 << 20)) + subprocess.run(["zstd", "-q", str(payload), "-o", str(archive)], check=True) + self.assertEqual(run(check, work).returncode, 0, check) + archive.write_bytes(archive.read_bytes()[: archive.stat().st_size // 2]) + self.assertNotEqual(run(check, work).returncode, 0, check) + archive.unlink() + self.assertNotEqual(run(check, work).returncode, 0, "a missing archive passed " + check) + + def test_download_wrapper_retries_a_failed_check_and_fails_on_the_second(self): + action = (ROOT / ".github/actions/download-artifact/action.yml").read_text() + retry = "steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure'" + steps = action.split(" - name: ")[1:] + names = [step.split("\n", 1)[0] for step in steps] + self.assertEqual(names, ["Download (attempt 1)", "Verify (attempt 1)", "Clear the failed download", + "Download (attempt 2)", "Verify (attempt 2)"]) + for step in steps[:2]: + self.assertIn("continue-on-error: true", step) + for step in steps[2:]: + self.assertIn(retry, step) + self.assertNotIn("continue-on-error", step) + if __name__ == "__main__": unittest.main() From cd71fbf5600204197a23c2b6e2a912d3c3ec19a6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 11 Oct 2026 13:01:33 +0000 Subject: [PATCH 2/2] Require verify and gate retry checks precisely The second check now runs only after the second download succeeds, so a failed retry download reports its own error rather than a missing file. `verify` is required: a caller without a check would get no retry. The test pins every step's exact `if:` gate, which catches a broken gate that a substring match missed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01KdaGJqi1VLtSWMnHY4hP6g --- .github/actions/download-artifact/action.yml | 5 ++-- scripts/tests/test_ci_e2e_archive.py | 26 ++++++++++++-------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/.github/actions/download-artifact/action.yml b/.github/actions/download-artifact/action.yml index 0bb1b7312..862030a6a 100644 --- a/.github/actions/download-artifact/action.yml +++ b/.github/actions/download-artifact/action.yml @@ -14,7 +14,7 @@ inputs: description: >- Bash script, run from the workspace, that exits non-zero when the downloaded files are incomplete or corrupt (e.g. `zstd -t `) - default: 'true' + required: true runs: using: composite steps: @@ -56,6 +56,7 @@ runs: # Neither step below continues on error: a second bad download is not # a blip, and the job must fail rather than test corrupt files. - name: Download (attempt 2) + id: second if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4 with: @@ -64,7 +65,7 @@ runs: path: ${{ inputs.path }} - name: Verify (attempt 2) - if: ${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }} + if: ${{ !cancelled() && steps.second.outcome == 'success' }} shell: bash env: VERIFY: ${{ inputs.verify }} diff --git a/scripts/tests/test_ci_e2e_archive.py b/scripts/tests/test_ci_e2e_archive.py index d9291855e..ae2f9f6a1 100644 --- a/scripts/tests/test_ci_e2e_archive.py +++ b/scripts/tests/test_ci_e2e_archive.py @@ -128,7 +128,9 @@ def test_download_retries_when_the_archive_arrives_truncated(self): for step_name, step in rows.steps(job): if step_name == "Download the e2e binaries": self.assertIn("uses: ./.github/actions/download-artifact", step) - checks.append(re.search(r"(?m)^ verify: (.+)$", step)[1]) + verify = re.search(r"(?m)^ verify: (.+)$", step) + self.assertIsNotNone(verify, "an e2e-bin download has no verify check") + checks.append(verify[1]) self.assertTrue(checks) for check in checks: with tempfile.TemporaryDirectory(prefix="e2e-truncated-") as directory: @@ -147,16 +149,20 @@ def test_download_retries_when_the_archive_arrives_truncated(self): def test_download_wrapper_retries_a_failed_check_and_fails_on_the_second(self): action = (ROOT / ".github/actions/download-artifact/action.yml").read_text() - retry = "steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure'" + retry = "${{ !cancelled() && (steps.first.outcome == 'failure' || steps.first-check.outcome == 'failure') }}" steps = action.split(" - name: ")[1:] - names = [step.split("\n", 1)[0] for step in steps] - self.assertEqual(names, ["Download (attempt 1)", "Verify (attempt 1)", "Clear the failed download", - "Download (attempt 2)", "Verify (attempt 2)"]) - for step in steps[:2]: - self.assertIn("continue-on-error: true", step) - for step in steps[2:]: - self.assertIn(retry, step) - self.assertNotIn("continue-on-error", step) + gates = {step.split("\n", 1)[0]: re.search(r"(?m)^ if: (.+)$", step)[1] for step in steps} + self.assertEqual(gates, { + "Download (attempt 1)": "${{ !cancelled() }}", + "Verify (attempt 1)": "${{ !cancelled() && steps.first.outcome == 'success' }}", + "Clear the failed download": retry, + "Download (attempt 2)": retry, + "Verify (attempt 2)": "${{ !cancelled() && steps.second.outcome == 'success' }}", + }) + # Only the first attempt may fail softly; the second must fail the job. + for step in steps: + name = step.split("\n", 1)[0] + self.assertEqual("continue-on-error: true" in step, name.endswith("(attempt 1)"), name) if __name__ == "__main__":