ci(tools): retry pinned downloads and name a TLS failure as one - #51
Merged
Merged
Conversation
Main went red at 2026-09-22T07:09Z on commit 4df7881 -- which touched nothing in this area -- with `curl: (60) SSL certificate problem: certificate has expired` fetching the pinned FastQC zip. That host is a single university web server; its certificate had lapsed, and the log said only "##[error]Process completed with exit code 60", which reads like a code failure until someone opens the log and knows what 60 means. curl does not retry a certificate failure without --retry-all-errors, so --retry alone would not have covered the class that bit. Skipping the tool is not available either: issue #30 exists because fastqc was silently absent from CI, and a skip is not a pass. So the three pinned archive installs now share one sourced helper that: - retries the transient class (4 attempts, 5s apart), covering TLS handshake and receive failures as well as connection errors; - passes --fail, so an HTTP error status is a failure now rather than a body whose checksum mismatches later -- which points at the artifact instead of at the URL being wrong; - refuses an empty body explicitly, rather than leaving it to the checksum to explain; - annotates WHICH failure it was: a TLS failure prints the cause, the openssl command that confirms it, and the pin file that repoints it; a network failure says so; an HTTP error says the pinned URL is wrong; - returns curl's exit code, so the step still fails, loudly. The checksum check stays hard and stays in the caller, unchanged: a retry must never turn "the artifact changed" into "the artifact was eventually accepted". Verified against real failure modes, not by inspection: a self-signed TLS server (exit 60 -> the TLS diagnosis, exit 60), an HTTP 404 (exit 22 -> the URL diagnosis), an empty body (refused, exit 1), a good download against the matching checksum (passes), and a mismatched checksum (refused by the caller's check). test_install_pins.jl guards the wiring, and that guard was mutation tested rather than assumed. Deleting the helper, reverting one install step to a bare curl, and removing --retry-all-errors from the command each fail it. The first version did NOT fail on the third mutation, because the flag's name also appears in the comment explaining it -- the same sink this file's own $FASTQC_URL note warns about -- so it now asserts against the script with its comments stripped. 123/123 pass. Refs #30
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This was referenced Sep 22, 2026
hyperpolymath
added a commit
that referenced
this pull request
Sep 22, 2026
#54) Refs #30, #51 — removes the dependency the gate fix could only make legible. ## The problem, precisely On 2026-09-22 CI went red on a commit that touched nothing near FastQC: `www.bioinformatics.babraham.ac.uk` served an **expired TLS certificate**. #51 made that retryable and self-describing, but it could not remove the dependency, because FastQC publishes **no GitHub release assets** — all six of its releases carry none — so the pinned URL could only ever point at that single host. A third party's certificate renewal was able to stop our builds, and did. ## The fix The archive is **vendored**: same bytes, served from this repository's own releases — tagged `vendored/fastqc-v0.12.1`, marked **pre-release** because it is a build input rather than a version of this software. `config/defaults/tool_versions.yml` now points at `https://github.com/hyperpolymath/MetaManifold-WebUI/releases/download/vendored/fastqc-v0.12.1/fastqc_v0.12.1.zip` with the **checksum unchanged**. CI installs FastQC over a connection to GitHub — the same host that already serves vsearch and swarm — so the FastQC host leaves the critical path for building and reproducing the pipeline. ## Provenance, stated rather than glossed The capture used `curl --insecure`, because **upstream's certificate was already invalid when the file was taken**. That is worth saying out loud, because it is the one detail a reader should not have to infer. Integrity does not rest on that transfer: the sha256 in the pin is **unchanged from the original upstream pin**, which was established while upstream was healthy, and the captured file verifies against it. I also downloaded the published asset back over an anonymous connection and confirmed it is **byte-for-byte identical** (`cmp`) to a fresh upstream copy. Nothing changed except who serves the file. Licence: the archive carries its own `FastQC/LICENSE.txt` (GPL v3) and is redistributed unmodified. ## Guarded, because the failure mode is silent A URL that quietly points back at a third party still downloads and still verifies. The new testset fails **by name** if the pin returns to its old host or stops being a release of this repository, and asserts the vendored checksum so a future bump is a decision rather than a drift. Mutation-tested: repointing the pin at `babraham.ac.uk` fails two assertions by name; with the pin correct the file passes **126/126**. `docs/compliance/vendored-archives.md` records the rule — *the checksum is the integrity claim and never the transport; never re-checksum to make a download succeed; the licence travels with the archive; repointing back at a third party is a decision* — with this archive's entry (upstream URL, sha256, licence, capture date, why) and the procedure for adding the next one. `check-format`, `check-spdx` and `lint_source.jl` are clean.
hyperpolymath
added a commit
that referenced
this pull request
Sep 22, 2026
The check that exists to catch a package used but not declared in
Project.toml had two false negatives, and `import Printf` -- added earlier on
this branch -- walked through both of them: it passed this gate and failed
precompilation in CI instead, with exactly the error this check is written to
pre-empt.
1. It exempted a list of "stdlibs that need no [deps] entry", Printf among
them. There is no such class of name. A stdlib a package uses must be
declared like any other dependency; only Base, Core and Main are bound
without a declaration. Verified by experiment against every name the old
list contained: each one fails with "does not have X in its
dependencies" when undeclared.
2. It matched one name per line, so `using JSON3, YAML` checked JSON3 and
never looked at YAML.
Both are fixed by parsing the statement instead of pattern-matching its text.
The AST has already resolved comma lists, relative imports and `using A: b, c`
selection, none of which need guessing at, and the reporting now points at
the statement rather than the file.
A gate that cannot fail is decoration, so the check carries a self-test that
replays the input which defeated it -- an undeclared stdlib, and a name after
the first in a comma list -- next to the inverses that must NOT be reported,
so a future rewrite fails the gate instead of quietly passing it. The
self-test was mutation-checked: adding Printf to the always-bound set makes
it fail and name the reason.
Verified: the gate is clean on this tree, fails on an undeclared Printf, and
fails on an undeclared second name in a comma list. Both mutations restored.
Refs #51
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
1 participant
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.
What was actually happening
Answered from the logs rather than guessed. Three things were true, and only one of
them was a defect:
The pending run finished green.
e1856af(PR feat(analysis): exact counts and rationals with an explicit numeric policy #49's merge) completed with theJulia suite in 28:19 — so fix(test): probe R once so the guard answers what the consumer asks #47's fix and feat(analysis): exact counts and rationals with an explicit numeric policy #49's module are both verified on main.
Gate triageis skipped on pushes by design, not broken. The job needs a PRnumber for every substantive step, so on a push it could only build squabbler,
run a bundled fixture and print "success" having triaged nothing. On PRs it runs:
all six PRs (fix(ci): grade every non-merge commit a PR proposes #45–docs(audit): verify the project board now that the scope is available #50) passed it, alongside
Julia tests,Repo hygieneandSonarCloud. The gate is real — a ruleset (
Optimus-Branch) requires those threechecks on the default branch, and
test_install_pins.jlalready guards thecheck-name trap that makes a required check unsatisfiable.
The genuine defect: the gate went red for a third party's TLS certificate.
Run 35696791460 (commit
4df7881— which touched nothing in this area) failed atInstall fastqcwith:The pinned FastQC zip lives on one university web server; that server's
certificate had lapsed. Nothing in the commit could cause it, and the log said
only "exit code 60".
The fix
curldoes not retry a certificate failure without--retry-all-errors, so--retryalone would not have covered the class that bit. And skipping the tool isnot on the table: issue #30 exists because fastqc was silently absent from CI, and
a skip is not a pass. So the three pinned archive installs share one sourced helper
that retries the transient class, uses
--fail(so an HTTP 404 is a failure nowrather than a checksum mismatch later, which points at the artifact instead of the
URL), refuses an empty body, and — when it still fails — says which failure it is:
a TLS failure prints the cause, the
openssl s_clientcommand that confirms it, andthe pin file that repoints it.
The checksum check is untouched and stays hard in each caller: a retry must never turn
"the artifact changed" into "the artifact was eventually accepted".
Verification
Against real failure modes, not by inspection:
--failtest_install_pins.jlnow guards the wiring, and the guard was mutation tested:deleting the helper, reverting an install step to a bare
curl, and removing--retry-all-errorsfrom the command each fail it. The first version of that guarddid not fail on the third mutation — the flag's name also appears in the comment
explaining it, the same sink this file's own
$FASTQC_URLnote warns about — so it nowasserts against the script with its comments stripped. 123/123 pass;
check-format,check-spdx,bash -nover every tracked shell script andlint_source.jlall clean.Not changed, deliberately
Neither of the two availability fixes I considered: switching FastQC to a bioconda
artifact (a different pin, and install mechanics I cannot fully exercise without Java)
or adding a mirror (FastQC publishes no GitHub release assets, and no byte-identical
mirror was verified). If the host lapses again, the annotation now says so in one line
and the pin file is the lever — better than a silent switch to different bytes.