Skip to content

Commit da82deb

Browse files
committed
Retry transient download failures and keep testing healthy binaries
The v7.9.0 release run lost one step to a one-off CERTIFICATE_VERIFY_FAILED from a Windows runner's TLS-intercepting proxy while the tool runner fetched gh, which skipped the windows-x64 attestation sidecar upload; the binary itself shipped and its attestation is registered server-side. The verification seam now retries network-level failures (TLS errors, timeouts, truncated bodies) up to 3 times before surfacing. The same run also exposed test-binaries skipping wholesale when a sibling build cell fails, leaving every healthy binary untested: the job now gates with always() and fail-fast off, so only the failed target's cell errors out on its missing artifact. The v7.9.0 changelog section records the missing sidecar.
1 parent 4381fde commit da82deb

5 files changed

Lines changed: 101 additions & 8 deletions

File tree

.github/workflows/_release-engine.yaml

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -387,12 +387,19 @@ jobs:
387387
# job re-validates the artifact on a pristine VM, through the same
388388
# upload/download round-trip a user's binary takes. Also skipped for
389389
# branches that don't affect code (e.g., update-mailmap, format-markdown).
390+
# always() keeps it running when a sibling build failed: without it, one
391+
# red build cell skips this whole job and every healthy binary ships with
392+
# no runtime test at all (v7.9.0 shipped exactly that way).
390393
if: >
391-
fromJSON(needs.metadata.outputs.metadata).nuitka_matrix
394+
always()
395+
&& fromJSON(needs.metadata.outputs.metadata).nuitka_matrix
392396
&& !(fromJSON(needs.metadata.outputs.metadata).skip_binary_build || false)
393397
&& fromJSON(needs.metadata.outputs.metadata).release_commits_matrix
394398
strategy:
395-
# Advisory matrix (default fail-fast): see docs/workflows.md "Matrix fail-fast strategy".
399+
# Info-gathering matrix: see docs/workflows.md "Matrix fail-fast strategy".
400+
# fail-fast off so the cell of a target whose build failed (its artifact
401+
# download errors out) does not cancel the healthy targets' tests.
402+
fail-fast: false
396403
# Fall back to an empty `{"include":[]}` matrix when nuitka_matrix is null
397404
# (binary builds disabled via nuitka.enabled = false). GitHub resolves
398405
# strategy.matrix during job setup even when `if:` will skip the job, and a

changelog.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,14 @@
66
> This version is **not released yet** and is under active development.
77
88
- Teach the `repomatic-ship` skill to reconcile bundled skills and agents as a third pass, judge a false-positive autofix PR against current `main` before writing a lint rule for it, and tell a superseded intra-cycle measurement from a genuine contradiction.
9+
- The tool runner now retries a download up to 3 times on transient network failures, instead of failing the job on a one-off TLS or truncation error.
10+
- Standalone binary tests now run for every healthy target when a sibling build fails, instead of being skipped wholesale.
911

1012
## [`7.9.0` (2026-08-10)](https://github.com/kdeldycke/repomatic/compare/v7.8.0...v7.9.0)
1113

14+
> [!WARNING]
15+
> The `windows-x64` binary shipped without its `.attestation.json` sidecar: a transient TLS failure on the runner skipped the upload, and immutable releases lock the asset list. The attestation itself is registered, so `gh attestation verify repomatic-7.9.0-windows-x64.exe --repo kdeldycke/repomatic` still verifies against GitHub's attestation service.
16+
1217
- **Breaking:** `REPOMATIC_PAT` now requires `Administration: Read-only`, and `lint-repo` fails without it. Regenerate the token with the setup guide's pre-filled link; steps that cannot be verified now say so instead of vanishing.
1318
- New `[tool.repomatic] nuitka.dev-targets` option: an ordinary push now compiles binaries only for a canary subset, `["linux-arm64"]` by default. Release commits, a new weekly schedule and manual dispatches build the full 6-target fleet.
1419
- `lint-repo` now also verifies versionless `releases/latest/download` URLs in `docs/install.md`, catching a renamed asset that leaves the guide pointing at a 404.

docs/workflows.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -801,8 +801,8 @@ For how to *choose* what the matrix tests (covering the shipped config broadly w
801801
Whether a matrix job overrides the default `fail-fast: true` depends on what the cells produce, not on which workflow they live in. Three categories:
802802

803803
1. **Asset-producing matrices that feed an immutable downstream artifact.** Each cell builds something the next job ships and cannot retroactively fix. Override to `fail-fast: false` so a transient runner crash on one cell does not cancel siblings whose output was already valid: shipping partial coverage is strictly better than shipping nothing. Downstream gates must accept `result != 'skipped'` (not `== 'success'`) so partial-success runs still flow through. **Applies to:** `compile-binaries` (binaries attached to the draft release before [§ Immutable releases](#immutable-releases) locks them).
804-
2. **Info-gathering matrices.** Each cell collects diagnostic data and the value of the run scales with how many cells reported. Override to `fail-fast: false` so a single failure does not erase the rest of the snapshot. **Applies to:** `tests` (per-cell `continue-on-error` already decides what fails the workflow), `dump-context`.
805-
3. **Advisory or single-cell matrices.** Tests that do not gate publication, validations, or matrices that typically run with one cell. Keep the default `fail-fast: true`: cancelling siblings on the first failure saves runner minutes, and a real regression is resolved by fixing the underlying code (then re-running) or, for already-published releases, by skipping that version (see [§ Immutable releases](#immutable-releases)) rather than by exhaustively diagnosing every platform up front. **Applies to:** `test-binaries`, `validate-arch`, and the single-cell publish-pipeline matrices (`build-package`, `create-tag`, `publish-pypi`, `create-release`, `publish-release`, `scan-virustotal`).
804+
2. **Info-gathering matrices.** Each cell collects diagnostic data and the value of the run scales with how many cells reported. Override to `fail-fast: false` so a single failure does not erase the rest of the snapshot. **Applies to:** `tests` (per-cell `continue-on-error` already decides what fails the workflow), `dump-context`, and `test-binaries` (gated with `always()` besides, so one failed build cell neither skips nor cancels the healthy targets' tests: its own cell fails on the missing artifact, which the advisory nature tolerates).
805+
3. **Advisory or single-cell matrices.** Tests that do not gate publication, validations, or matrices that typically run with one cell. Keep the default `fail-fast: true`: cancelling siblings on the first failure saves runner minutes, and a real regression is resolved by fixing the underlying code (then re-running) or, for already-published releases, by skipping that version (see [§ Immutable releases](#immutable-releases)) rather than by exhaustively diagnosing every platform up front. **Applies to:** `validate-arch` and the single-cell publish-pipeline matrices (`build-package`, `create-tag`, `publish-pypi`, `create-release`, `publish-release`, `scan-virustotal`).
806806

807807
GitHub resolves a job's `strategy.matrix` during setup even when the job's `if:` guard will skip it, so a matrix expression that resolves to an empty or null value can abort the entire run with `Unexpected value ''` before `if:` is ever checked. This surfaces when a project disables binary builds (`nuitka.enabled = false` makes `nuitka_matrix` null), turning every non-release push red. Two triggers exist: a `workflow_call` output read as a bare string (an empty `release_commits_matrix` becomes `fromJSON('')`), and a matrix-derived `runs-on: ${{ matrix.os }}` that cannot resolve against an absent matrix. The fix is a fallback to a valid empty matrix. `matrix: ${{ ... || fromJSON('{"include":[]}') }}` expands the job to zero runs, so it skips cleanly instead of failing the workflow. `compile-binaries`, `test-binaries`, and the caller's `publish-pypi` job carry this fallback; a job that pins a static `runs-on` and has no other matrix-derived fields (like `create-tag`) already skips cleanly on a null matrix and needs none.
808808

repomatic/tool_runner.py

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import sys
4545
import tarfile
4646
import tempfile
47+
import time
4748
import zipfile
4849
from contextlib import nullcontext
4950
from dataclasses import replace
@@ -305,6 +306,20 @@ def resolve_config(
305306
for small JSON API responses.
306307
"""
307308

309+
_DOWNLOAD_ATTEMPTS = 3
310+
"""Attempts at downloading one artifact before giving up.
311+
312+
```{note}
313+
Runner egress is flaky in ways that clear on the next connection: `v7.9.0`
314+
lost its windows-x64 attestation sidecar to a one-off
315+
`CERTIFICATE_VERIFY_FAILED: self-signed certificate` from a Windows runner's
316+
TLS-intercepting proxy, and short bodies surface as the truncation guard's
317+
`OSError`. One artifact download aborting a whole release job over a
318+
transient is the wrong trade, so the verification seam retries with a short
319+
pause and lets only a repeated failure surface.
320+
```
321+
"""
322+
308323
_DOWNLOAD_CHUNK_SIZE = 65536
309324
"""Read size for streaming downloads and incremental hashing."""
310325

@@ -386,15 +401,33 @@ def _download_and_verify(
386401
) -> None:
387402
"""Download a file and verify its SHA-256 checksum.
388403
404+
Network-level failures (TLS errors, timeouts, truncated bodies: all
405+
`OSError` subclasses, `URLError` included) are retried up to
406+
{data}`_DOWNLOAD_ATTEMPTS` times; a checksum mismatch on a complete body
407+
is not, since identical bytes would fail identically.
408+
389409
:param url: URL to download.
390410
:param expected_sha256: Expected lowercase hex SHA-256 digest.
391411
`None` skips verification (logs the computed digest for reference).
392412
:param dest_path: Where to write the downloaded file.
393413
:param label: Progress bar label. Defaults to the destination filename.
394-
:raises OSError: If the body is shorter than the advertised `Content-Length`.
414+
:raises OSError: If every attempt fails at the network level, including a
415+
body shorter than the advertised `Content-Length`.
395416
:raises ValueError: If the checksum does not match.
396417
"""
397-
actual = download_to(url, dest_path, label=label)
418+
for attempt in range(1, _DOWNLOAD_ATTEMPTS + 1):
419+
try:
420+
actual = download_to(url, dest_path, label=label)
421+
break
422+
except OSError as ex:
423+
dest_path.unlink(missing_ok=True)
424+
if attempt == _DOWNLOAD_ATTEMPTS:
425+
raise
426+
logging.warning(
427+
f"Download attempt {attempt}/{_DOWNLOAD_ATTEMPTS} of {url}"
428+
f" failed ({ex}); retrying."
429+
)
430+
time.sleep(2 * attempt)
398431
if expected_sha256 is None:
399432
logging.info("SHA-256 of %s: %s (not verified).", url, actual)
400433
return

tests/test_tool_runner.py

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
from itertools import combinations
3232
from pathlib import Path
3333
from unittest.mock import MagicMock, patch
34+
from urllib.error import URLError
3435

3536
import pytest
3637
import tomlrt
@@ -732,22 +733,69 @@ def test_download_and_verify_mismatch(tmp_path):
732733
assert not dest.exists()
733734

734735

736+
def test_download_and_verify_retries_transient_failure(tmp_path):
737+
"""A transient network failure is retried and the next attempt succeeds."""
738+
content = b"hello binary world"
739+
expected = hashlib.sha256(content).hexdigest()
740+
dest = tmp_path / "downloaded"
741+
742+
with (
743+
patch(
744+
"repomatic.tool_runner.urlopen",
745+
side_effect=[
746+
URLError("certificate verify failed: self-signed certificate"),
747+
_urlopen_response(content),
748+
],
749+
) as fake_urlopen,
750+
patch("repomatic.tool_runner.time.sleep") as fake_sleep,
751+
):
752+
_download_and_verify("https://example.com/file", expected, dest)
753+
754+
assert fake_urlopen.call_count == 2
755+
assert fake_sleep.call_count == 1
756+
assert dest.read_bytes() == content
757+
758+
759+
def test_download_and_verify_gives_up_after_attempts(tmp_path):
760+
"""A persistent network failure surfaces after the last attempt."""
761+
dest = tmp_path / "downloaded"
762+
763+
with (
764+
patch(
765+
"repomatic.tool_runner.urlopen",
766+
side_effect=URLError("connection reset"),
767+
) as fake_urlopen,
768+
patch("repomatic.tool_runner.time.sleep"),
769+
pytest.raises(URLError, match="connection reset"),
770+
):
771+
_download_and_verify("https://example.com/file", "0" * 64, dest)
772+
773+
assert fake_urlopen.call_count == 3
774+
assert not dest.exists()
775+
776+
735777
def test_download_and_verify_truncated(tmp_path):
736778
"""A body shorter than Content-Length raises OSError, not a mismatch.
737779
738780
A truncated transfer (proxy hiccup, dropped connection) hashes to a wrong
739781
digest, so it used to be reported as a SHA-256 mismatch: that reads as a
740782
stale pin or a tampered artifact when the registry checksum is correct.
783+
Truncation is retried like any network failure, so the error only
784+
surfaces once every attempt came up short.
741785
"""
742786
content = b"hello binary world"
743787
dest = tmp_path / "downloaded"
744788

745789
with (
746790
patch(
747791
"repomatic.tool_runner.urlopen",
748-
# Advertise more bytes than the body delivers.
749-
return_value=_urlopen_response(content, advertised_length=len(content) + 7),
792+
# Advertise more bytes than the body delivers, on every attempt.
793+
side_effect=[
794+
_urlopen_response(content, advertised_length=len(content) + 7)
795+
for _ in range(3)
796+
],
750797
),
798+
patch("repomatic.tool_runner.time.sleep"),
751799
pytest.raises(OSError, match="Truncated download .* got 18 of 25 bytes"),
752800
):
753801
_download_and_verify(

0 commit comments

Comments
 (0)