From 84db747d93cdfa3b70ed06564d8403f48d3e4eb7 Mon Sep 17 00:00:00 2001 From: diegosouzapw Date: Thu, 30 Jul 2026 10:47:15 -0300 Subject: [PATCH] test(ci): align the artifact-provenance guard with the gap-16 criterion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own assertion from #8953 encoded the criterion this PR deliberately removes: it required `.conclusion == "success"` on the whole CI run, which discarded a perfectly good build tree whenever any unrelated shard went red — pushing the publish into the 40-minute build the fast path exists to avoid. Inverted rather than deleted, and the replacement is strictly stronger. It now pins three things where the old one pinned one: that the loose criterion is gone, that the step actually probes for the artifact (the accurate signal, since it is only uploaded when the Build job succeeded), and that it probes MORE THAN ONE candidate run — without which a single miss still falls back to a full build. The provenance clause it was originally written to protect (head_repository.full_name == env.REPO) is untouched and still asserted above. --- .../npm-publish-artifact-provenance.test.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/tests/unit/npm-publish-artifact-provenance.test.ts b/tests/unit/npm-publish-artifact-provenance.test.ts index 392e469a5b..68e20110b1 100644 --- a/tests/unit/npm-publish-artifact-provenance.test.ts +++ b/tests/unit/npm-publish-artifact-provenance.test.ts @@ -58,7 +58,29 @@ test("the artifact fast path only trusts runs from THIS repository", () => { /head_sha=\$HEAD_SHA/, "the run must still be matched on head_sha — that is the tree-equality guarantee" ); - assert.match(step, /\.conclusion == "success"/, "only a successful run may be restored"); + // Was: `assert.match(step, /\.conclusion == "success"/)`. That assertion encoded the wrong + // criterion (gap 16) and is deliberately INVERTED here, not deleted. Requiring the whole run to + // have concluded successfully discarded a perfectly good tree whenever any unrelated shard went + // red, pushing the publish into the 40-minute build this fast path exists to avoid. The artifact + // is only uploaded when the Build job itself succeeded, so its PRESENCE is the accurate signal. + // + // The replacement is strictly stronger: it pins BOTH that the loose criterion is gone AND that + // the step actually probes candidates for the artifact, which the old assertion never checked. + assert.doesNotMatch( + step, + /\.conclusion == "success"/, + "an unrelated red shard must not discard a valid build artifact" + ); + assert.match( + step, + /gh run download .*--name next-build/, + "the criterion is now whether the run HAS the artifact — so it must try to download it" + ); + assert.match( + step, + /for candidate in \$CANDIDATES/, + "…across more than one candidate run, or a single miss still falls back to a full build" + ); }); test("REPO is passed through env, never interpolated into the script body", () => {