Skip to content

Commit 11dc81a

Browse files
committed
ci: give back the independent execution the folded matrices took
Three places in `cli-package-validation.yml` where collapsing a matrix into one job or one step made a first failure hide everything behind it. None of them fails silently, so each costs a rerun rather than protection — but the rerun is of the most expensive job in the workflow. The predecessor now resolves on `state-root-qualification`, the only job that reads it. Registry flakiness used to forfeit `build` and everything downstream; it still costs no runner of its own, and callers are unaffected because a reusable workflow publishes its outputs only once every job has finished. `current-nightly-predecessor-to-candidate` runs before the two frozen transitions. It is the only one a pull request can influence, and all three share one `set -e`, so a flaky `curl` on either frozen tarball meant it never executed. `Preserve the qualification reports` drops to `if-no-files-found: warn` on an already-failed job, where a `curl` that failed before any `tee` added a second, unrelated red; a green run still treats an empty directory as the broken path it is. The two supported Node versions no longer depend on each other: the first smoke is `continue-on-error` and its outcome is re-raised after the second has run, which is what the matrix's `fail-fast: false` used to buy. All four are contracts in `release-cli-workflow-policy.test.mjs` now, each verified by constructing the regression it forbids.
1 parent fecc601 commit 11dc81a

2 files changed

Lines changed: 107 additions & 36 deletions

File tree

.github/workflows/cli-package-validation.yml

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@ on:
6666
value: ${{ jobs.build.outputs.release_candidate_run_attempt }}
6767
release_predecessor_version:
6868
description: Exact npm Nightly version qualified against this candidate
69-
value: ${{ jobs.build.outputs.release_predecessor_version }}
69+
value: ${{ jobs.state-root-qualification.outputs.release_predecessor_version }}
7070
release_predecessor_tarball_url:
7171
description: Exact npm Nightly tarball qualified against this candidate
72-
value: ${{ jobs.build.outputs.release_predecessor_tarball_url }}
72+
value: ${{ jobs.state-root-qualification.outputs.release_predecessor_tarball_url }}
7373
release_predecessor_integrity:
7474
description: npm SHA-512 integrity of the Nightly tarball qualified against this candidate
75-
value: ${{ jobs.build.outputs.release_predecessor_integrity }}
75+
value: ${{ jobs.state-root-qualification.outputs.release_predecessor_integrity }}
7676
workflow_dispatch:
7777

7878
permissions:
@@ -192,9 +192,6 @@ jobs:
192192
outputs:
193193
release_candidate_artifact_id: ${{ steps.release-candidate.outputs.artifact-id }}
194194
release_candidate_run_attempt: ${{ github.run_attempt }}
195-
release_predecessor_version: ${{ steps.predecessor.outputs.version }}
196-
release_predecessor_tarball_url: ${{ steps.predecessor.outputs.tarball_url }}
197-
release_predecessor_integrity: ${{ steps.predecessor.outputs.integrity }}
198195
steps:
199196
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
200197
with:
@@ -206,12 +203,6 @@ jobs:
206203
cache: npm
207204
- name: Select the release npm toolchain
208205
run: npm install --global --no-audit --no-fund "$(node -p 'require("./package.json").packageManager')"
209-
# Resolving one npm version is seconds of work against a registry this
210-
# job already reaches, so it rides the runner that waits on the addon
211-
# builds instead of holding a second one for the whole wait.
212-
- name: Resolve the current npm Nightly as immutable evidence
213-
id: predecessor
214-
run: node scripts/release-cli-publication.mjs resolve-nightly-predecessor "$GITHUB_OUTPUT"
215206
- name: Install cargo-deny
216207
uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2
217208
with:
@@ -295,7 +286,13 @@ jobs:
295286
with:
296287
artifact-ids: ${{ needs.build.outputs.release_candidate_artifact_id }}
297288
path: packages/cli/release
289+
# The two supported Node versions share a runner, so a failure on the
290+
# first would otherwise mean the second never runs — which is the
291+
# independence the matrix these replaced bought with `fail-fast: false`.
292+
# The outcome is re-raised below, once both have had their turn.
298293
- name: Validate the installed tarball
294+
id: first-node-smoke
295+
continue-on-error: true
299296
run: node scripts/smoke-release-cli-package.mjs
300297
- uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
301298
if: matrix.second_node != ''
@@ -307,12 +304,24 @@ jobs:
307304
- name: Validate the installed tarball on the second Node
308305
if: matrix.second_node != ''
309306
run: node scripts/smoke-release-cli-package.mjs
307+
# `always()` because the second Node failing must not swallow the first.
308+
- name: Report the first Node result
309+
if: always() && steps.first-node-smoke.outcome != 'success'
310+
env:
311+
NODE_VERSION: ${{ matrix.node }}
312+
run: |
313+
echo "The installed tarball failed on Node $NODE_VERSION" >&2
314+
exit 1
310315
311316
state-root-qualification:
312317
name: Qualify released State Roots
313318
needs: build
314319
runs-on: ubuntu-24.04
315320
timeout-minutes: 45
321+
outputs:
322+
release_predecessor_version: ${{ steps.predecessor.outputs.version }}
323+
release_predecessor_tarball_url: ${{ steps.predecessor.outputs.tarball_url }}
324+
release_predecessor_integrity: ${{ steps.predecessor.outputs.integrity }}
316325
steps:
317326
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
318327
with:
@@ -333,15 +342,26 @@ jobs:
333342
with:
334343
artifact-ids: ${{ needs.build.outputs.release_candidate_artifact_id }}
335344
path: packages/cli/release
345+
# Resolved here rather than on `build`, which is the most expensive job in
346+
# the workflow: a registry blip used to forfeit the tarball build and
347+
# every job downstream of it, and this is the only job that reads the
348+
# answer. It still costs no runner of its own, and the exported identity
349+
# is now this job's output.
350+
- name: Resolve the current npm Nightly as immutable evidence
351+
id: predecessor
352+
run: node scripts/release-cli-publication.mjs resolve-nightly-predecessor "$GITHUB_OUTPUT"
336353
# Three runs of one script against one sandbox, not three runners. Two of
337354
# these transitions are between tarballs that were published and frozen,
338355
# so nothing in a pull request can change their outcome except the
339-
# qualifier itself, and the third reads the candidate this run built.
356+
# qualifier itself, and the third reads the candidate this run built —
357+
# which is why it goes first. Sharing one `set -e` with the frozen pair
358+
# meant a flaky `curl` on either of them left the only transition a pull
359+
# request can influence unexecuted.
340360
- name: Qualify the released State Root transitions
341361
env:
342362
MAKA_QUALIFICATION_BWRAP_USE_SUDO: '1'
343-
PREDECESSOR_TARBALL_URL: ${{ needs.build.outputs.release_predecessor_tarball_url }}
344-
PREDECESSOR_INTEGRITY: ${{ needs.build.outputs.release_predecessor_integrity }}
363+
PREDECESSOR_TARBALL_URL: ${{ steps.predecessor.outputs.tarball_url }}
364+
PREDECESSOR_INTEGRITY: ${{ steps.predecessor.outputs.integrity }}
345365
run: |
346366
set -euo pipefail
347367
evidence_root="$RUNNER_TEMP/released-state-root"
@@ -386,6 +406,10 @@ jobs:
386406
echo "::endgroup::"
387407
}
388408
409+
qualify current-nightly-predecessor-to-candidate \
410+
"$PREDECESSOR_TARBALL_URL" '' "$PREDECESSOR_INTEGRITY" \
411+
candidate '' '' any
412+
389413
qualify cross-epoch-74-to-76 \
390414
https://registry.npmjs.org/maka-agent/-/maka-agent-0.2.0-dev.3.20260830.tgz \
391415
66b1ce9307c9d5c06eaa7a6cbf533d4747d02caf71c1776c69c7dbfa12c3f414 '' \
@@ -397,29 +421,26 @@ jobs:
397421
b7d48adb466e16be7ffefbda3a0fcd833cc4108ea502b27778d0f4da680e1fc0 '' \
398422
published https://registry.npmjs.org/maka-agent/-/maka-agent-0.2.0-dev.5.20260830.tgz \
399423
e7a682157c6899fc7f1be86a2d7b0bd0696195a5771d8cc97bd1389a5b74989f same
400-
401-
qualify current-nightly-predecessor-to-candidate \
402-
"$PREDECESSOR_TARBALL_URL" '' "$PREDECESSOR_INTEGRITY" \
403-
candidate '' '' any
404424
# The three transitions used to be three matrix jobs, so one failing left
405425
# the others to upload their own reports. Folded into one step they share
406-
# a `set -e`, and the reports are wanted most on the run that failed —
407-
# `tee` has already written the failing transition's own output by then.
408-
# `if-no-files-found` stays `error` so a broken path is still caught on a
409-
# green run.
426+
# a `set -e`, and `tee` has already written the failing transition's own
427+
# output by then. `if-no-files-found` stays `error` on a green run, where
428+
# an empty directory means a broken path; on a red one it drops to `warn`,
429+
# because a `curl` that failed before any `tee` would otherwise add a
430+
# second, unrelated red to a job that already reported the real one.
410431
- name: Preserve the qualification reports
411432
if: always()
412433
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
413434
with:
414435
name: released-state-root
415436
path: ${{ runner.temp }}/released-state-root/*-report.json
416-
if-no-files-found: error
437+
if-no-files-found: ${{ job.status == 'success' && 'error' || 'warn' }}
417438
retention-days: 7
418439
- name: Require the qualified Nightly predecessor to remain current
419440
env:
420-
PREDECESSOR_VERSION: ${{ needs.build.outputs.release_predecessor_version }}
421-
PREDECESSOR_TARBALL_URL: ${{ needs.build.outputs.release_predecessor_tarball_url }}
422-
PREDECESSOR_INTEGRITY: ${{ needs.build.outputs.release_predecessor_integrity }}
441+
PREDECESSOR_VERSION: ${{ steps.predecessor.outputs.version }}
442+
PREDECESSOR_TARBALL_URL: ${{ steps.predecessor.outputs.tarball_url }}
443+
PREDECESSOR_INTEGRITY: ${{ steps.predecessor.outputs.integrity }}
423444
run: |
424445
node scripts/release-cli-publication.mjs assert-nightly-predecessor \
425446
"$PREDECESSOR_VERSION" \

scripts/release-cli-workflow-policy.test.mjs

Lines changed: 59 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,25 @@ test('validation consumers download the artifact produced by the build job', ()
4848

4949
test('CLI validation qualifies exact published State Roots without weakening artifact identity', () => {
5050
const workflow = readWorkflow('cli-package-validation.yml');
51-
// The predecessor is resolved on the job that already waits on the addon
52-
// builds, so the exported identity comes from `build` rather than a job of
53-
// its own. The exported names are the contract callers hold.
51+
// The predecessor is resolved on the only job that reads it, rather than on
52+
// `build`, where a registry blip forfeited the most expensive job in the
53+
// workflow and everything downstream of it. The exported names are the
54+
// contract callers hold; the job behind them is not, because a reusable
55+
// workflow publishes its outputs only once every job has finished.
5456
assert.match(
5557
workflow,
56-
/release_predecessor_version:[\s\S]*?value: \$\{\{ jobs\.build\.outputs\.release_predecessor_version \}\}/u,
58+
/release_predecessor_version:[\s\S]*?value: \$\{\{ jobs\.state-root-qualification\.outputs\.release_predecessor_version \}\}/u,
5759
);
5860
assert.match(
5961
workflow,
60-
/release_predecessor_integrity:[\s\S]*?jobs\.build\.outputs\.release_predecessor_integrity/u,
62+
/release_predecessor_integrity:[\s\S]*?jobs\.state-root-qualification\.outputs\.release_predecessor_integrity/u,
6163
);
6264
assert.match(
6365
workflow,
6466
/id: predecessor\n\s+run: node scripts\/release-cli-publication\.mjs resolve-nightly-predecessor "\$GITHUB_OUTPUT"/u,
6567
);
6668
assert.match(workflow, /state-root-qualification:\n[\s\S]*?needs: build\n/u);
69+
assert.doesNotMatch(workflow, /needs\.build\.outputs\.release_predecessor/u);
6770

6871
// Both frozen transitions keep their exact digests and the epoch relation
6972
// each one exists to prove. They are positional arguments now, so anchor on
@@ -82,13 +85,13 @@ test('CLI validation qualifies exact published State Roots without weakening art
8285
// `workflow_call` output, so callers hold it too.
8386
assert.match(
8487
workflow,
85-
/release_predecessor_tarball_url:[\s\S]*?value: \$\{\{ jobs\.build\.outputs\.release_predecessor_tarball_url \}\}/u,
88+
/release_predecessor_tarball_url:[\s\S]*?value: \$\{\{ jobs\.state-root-qualification\.outputs\.release_predecessor_tarball_url \}\}/u,
8689
);
8790
for (const name of ['tarball_url', 'integrity']) {
8891
assert.match(
8992
workflow,
9093
new RegExp(
91-
`PREDECESSOR_${name.toUpperCase()}: \\$\\{\\{ needs\\.build\\.outputs\\.release_predecessor_${name} \\}\\}`,
94+
`PREDECESSOR_${name.toUpperCase()}: \\$\\{\\{ steps\\.predecessor\\.outputs\\.${name} \\}\\}`,
9295
'u',
9396
),
9497
name,
@@ -111,14 +114,61 @@ test('CLI validation qualifies exact published State Roots without weakening art
111114
assert.match(qualify, /source_integrity/u);
112115
assert.match(qualify, /createHash\('sha512'\)/u);
113116
assert.match(qualify, /source_sha256="\$\(sha256sum/u);
117+
// The candidate is the only transition a pull request can influence, and the
118+
// three share one `set -e`, so it runs before either frozen tarball is
119+
// fetched. Read as positions in the script rather than restated, so a
120+
// reordering fails here instead of silently moving it back behind a `curl`.
121+
const script = [
122+
'current-nightly-predecessor-to-candidate',
123+
'cross-epoch-74-to-76',
124+
'same-epoch-76',
125+
].map((slug) => qualify.indexOf(`qualify ${slug}`));
126+
assert.ok(
127+
script.every((index) => index >= 0),
128+
'a declared State Root transition is no longer invoked',
129+
);
130+
assert.deepEqual(
131+
[...script].sort((left, right) => left - right),
132+
script,
133+
);
134+
114135
const preserve = namedStep(steps, 'Preserve the qualification reports');
115-
assert.match(preserve, /if-no-files-found: error/u);
136+
// `error` on a green run, where an empty directory means a broken path, and
137+
// `warn` on a red one: paired with `if: always()`, a plain `error` turned a
138+
// `curl` that failed before any `tee` into a second red on an already-red
139+
// job.
140+
assert.match(
141+
preserve,
142+
/if-no-files-found: \$\{\{ job\.status == 'success' && 'error' \|\| 'warn' \}\}/u,
143+
);
116144
const freshness = namedStep(steps, 'Require the qualified Nightly predecessor to remain current');
117145
assert.match(freshness, /assert-nightly-predecessor/u);
118-
assert.match(freshness, /needs\.build\.outputs\.release_predecessor_version/u);
146+
assert.match(freshness, /steps\.predecessor\.outputs\.version/u);
119147
assert.ok(steps.indexOf(freshness) > steps.indexOf(preserve));
120148
});
121149

150+
test('both supported Node versions validate the tarball even when the first fails', () => {
151+
// One runner and one tarball, so the two Node versions are two steps rather
152+
// than two jobs. Without this the first failing would end the job and the
153+
// second would never run at all — the matrix these replaced set
154+
// `fail-fast: false` for exactly that.
155+
const steps = workflowSteps(readWorkflow('cli-package-validation.yml'));
156+
const first = namedStep(steps, 'Validate the installed tarball');
157+
assert.match(first, /id: first-node-smoke/u);
158+
assert.match(first, /continue-on-error: true/u);
159+
160+
const second = namedStep(steps, 'Validate the installed tarball on the second Node');
161+
assert.match(second, /if: matrix\.second_node != ''/u);
162+
assert.ok(steps.indexOf(second) > steps.indexOf(first));
163+
164+
// `continue-on-error` alone would report a failing first Node as green, and
165+
// without `always()` a failing second Node would swallow it instead.
166+
const report = namedStep(steps, 'Report the first Node result');
167+
assert.match(report, /if: always\(\) && steps\.first-node-smoke\.outcome != 'success'/u);
168+
assert.match(report, /exit 1/u);
169+
assert.ok(steps.indexOf(report) > steps.indexOf(second));
170+
});
171+
122172
test('npm mutations revalidate the exact qualified Nightly predecessor', () => {
123173
const nightly = readWorkflow('npm-publication.yml');
124174
const nightlyFence = namedStep(

0 commit comments

Comments
 (0)