From 6f08db4e1fd534713ce03eb2fae925dbeb86c697 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:20:30 +0800 Subject: [PATCH 1/8] ci: stop deciding whether to install a toolchain at all `requiresHeavyValidation` was a twelve-term disjunction over every other selection, and its only production effect was skipping `actions/setup-node` on a documentation-only pull request. That saves seconds on a runner the job is already holding, which is not what is scarce: the slot is. In exchange every new selection had to remember to join the disjunction, and a test was added to `ci-workflow-policy.test.mjs` for the sole purpose of noticing when one did not. Deleting the output deletes the reason that guard test existed, so it goes too. The six planner assertions phrased over it are replaced by reading the selections off the plan: `documentation-only changes select nothing at all` now covers `storageStress` and `full` as well, which the disjunction never did, and a selection added later joins it without anyone editing the test. Capability given up: a documentation-only run pays one `setup-node`. --- .github/workflows/ci.yml | 8 ++++-- scripts/ci-test-plan.mjs | 18 ------------ scripts/ci-test-plan.test.mjs | 44 ++++++++++++++++------------- scripts/ci-workflow-policy.test.mjs | 33 ---------------------- 4 files changed, 29 insertions(+), 74 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d30d08a5c..4952b14c49 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -132,10 +132,12 @@ jobs: - name: Check ASF source headers run: npm run check:asf-headers - # Everything below needs an installed toolchain. A plan that selected no - # surface stops at the install-free checks above. + # Everything below this line may need an installed toolchain, so each + # step names the selections it belongs to. `setup-node` itself is + # unconditional: it costs seconds on a runner the job is already holding, + # and gating it needed a twelve-term disjunction over every other + # selection that a new lane had to remember to join (#4475). - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 - if: steps.plan.outputs.heavy == 'true' with: node-version: '24' cache: npm diff --git a/scripts/ci-test-plan.mjs b/scripts/ci-test-plan.mjs index 0e75557442..64fd822ae9 100644 --- a/scripts/ci-test-plan.mjs +++ b/scripts/ci-test-plan.mjs @@ -531,23 +531,6 @@ export function planTests(changedFiles, options = {}) { }; } -export function requiresHeavyValidation(plan) { - return Boolean( - plan.appIcons || - plan.asfSource || - plan.astryxSurface || - plan.cliPackage || - plan.code || - plan.e2e || - plan.releaseContract || - plan.runtimeHost || - plan.runtimeSandbox || - plan.stateRootCompat || - plan.storybook || - plan.standardWorkspaces.length > 0, - ); -} - export function formatGitHubOutputs(plan) { return [ `app_icons=${plan.appIcons}`, @@ -556,7 +539,6 @@ export function formatGitHubOutputs(plan) { `cli_package=${plan.cliPackage}`, `code=${plan.code}`, `e2e=${plan.e2e}`, - `heavy=${requiresHeavyValidation(plan)}`, `runtime_host=${plan.runtimeHost}`, `runtime_sandbox=${plan.runtimeSandbox}`, `release_contract=${plan.releaseContract}`, diff --git a/scripts/ci-test-plan.test.mjs b/scripts/ci-test-plan.test.mjs index 9e02be9d2a..8bb342981c 100644 --- a/scripts/ci-test-plan.test.mjs +++ b/scripts/ci-test-plan.test.mjs @@ -31,12 +31,21 @@ import assert from 'node:assert/strict'; import { existsSync, readFileSync } from 'node:fs'; import test from 'node:test'; -import { - changedFilesBetween, - formatGitHubOutputs, - planTests, - requiresHeavyValidation, -} from './ci-test-plan.mjs'; +import { changedFilesBetween, formatGitHubOutputs, planTests } from './ci-test-plan.mjs'; + +/** + * Every surface a plan selected, read off the plan itself rather than from a + * list of the ones worth naming. `assert.deepEqual(selections(plan), [])` is + * therefore the whole "this change costs nothing" claim, and a selection added + * later joins it without anyone editing these tests. + */ +function selections(plan) { + return Object.entries(plan) + .filter(([key]) => key !== 'workspaces') + .filter(([, value]) => (Array.isArray(value) ? value.length > 0 : Boolean(value))) + .map(([key]) => key) + .sort(); +} const dirs = [ 'packages/core', @@ -62,34 +71,29 @@ const graph = { testDirs: new Set(dirs), }; -test('documentation-only changes do not select code validation', () => { +test('documentation-only changes select nothing at all', () => { const plan = planTests(['docs/ci.md'], { graph }); - assert.equal(plan.code, false); - assert.equal(plan.asfSource, false); - assert.equal(plan.astryxSurface, false); - assert.equal(requiresHeavyValidation(plan), false); + assert.deepEqual(selections(plan), []); assert.deepEqual(plan.workspaces, []); }); -test('documentation inside workspaces does not select heavy validation', () => { +test('documentation inside workspaces selects nothing at all', () => { for (const path of ['packages/runtime/README.md', 'apps/desktop/README.md']) { const plan = planTests([path], { graph }); - assert.equal(plan.code, false, path); - assert.equal(requiresHeavyValidation(plan), false, path); + assert.deepEqual(selections(plan), [], path); assert.deepEqual(plan.workspaces, [], path); } }); -test('mixed documentation and code changes still select heavy validation', () => { +test('mixed documentation and code changes still select code validation', () => { const plan = planTests(['README.md', 'packages/core/src/index.ts'], { graph }); assert.equal(plan.code, true); - assert.equal(requiresHeavyValidation(plan), true); }); -test('documentation with a dedicated contract still selects heavy validation', () => { +test('documentation with a dedicated contract still selects that contract', () => { for (const path of [ 'LICENSE', 'docs/astryx-surface-file-inventory.md', @@ -98,7 +102,7 @@ test('documentation with a dedicated contract still selects heavy validation', ( const plan = planTests([path], { graph }); assert.equal(plan.code, false, path); - assert.equal(requiresHeavyValidation(plan), true, path); + assert.notDeepEqual(selections(plan), [], path); } }); @@ -117,7 +121,7 @@ test('changed files are derived from the PR merge base', () => { const changedFiles = changedFilesBetween('main-now', 'pr-head', exec); assert.deepEqual(changedFiles, ['packages/runtime/README.md']); - assert.equal(requiresHeavyValidation(planTests(changedFiles, { graph })), false); + assert.deepEqual(selections(planTests(changedFiles, { graph })), []); assert.deepEqual(calls, [ ['merge-base', 'main-now', 'pr-head'], ['diff', '--no-renames', '--name-only', '--diff-filter=ACMRDT', 'fork-point', 'pr-head'], @@ -134,7 +138,7 @@ test('type changes remain in the PR-owned delta', () => { const changedFiles = changedFilesBetween('main-now', 'pr-head', exec); assert.deepEqual(changedFiles, ['packages/runtime/src/runtime.ts']); - assert.equal(requiresHeavyValidation(planTests(changedFiles, { graph })), true); + assert.equal(planTests(changedFiles, { graph }).code, true); }); test('the Astryx inventory can run without selecting the code suite', () => { diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 93978da036..02b249bfbe 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -125,39 +125,6 @@ test('contract checks run before dependency setup and can fail the job', () => { } }); -test('every selection that gates an installed step is one heavy validation covers', () => { - const workflow = readWorkflow('ci.yml'); - - // `heavy` decides whether the toolchain is installed at all, so any later - // step gated on a selection outside that disjunction would run against a - // runner with no dependencies — or, if the step is a `run:` that tolerates - // it, report green having done nothing. Derived from the workflow rather - // than restated, so a new plan output cannot gate a step without joining - // the disjunction first. - const [, installed] = workflow.split(/\n\s+- uses: actions\/setup-node[^\n]*\n/u); - assert.ok(installed, 'ci.yml no longer installs a toolchain'); - - const gating = [ - ...new Set([...installed.matchAll(/steps\.plan\.outputs\.(\w+) ==/gu)].map(([, name]) => name)), - ].sort(); - assert.ok(gating.length > 0, 'no installed step is gated on a selection'); - - // The planner names selections in camelCase and publishes them in snake_case. - const source = readFileSync(new URL('./ci-test-plan.mjs', import.meta.url), 'utf8'); - const disjunction = source.match(/export function requiresHeavyValidation[\s\S]*?\n\}/u)?.[0]; - assert.ok(disjunction, 'requiresHeavyValidation is no longer a single expression'); - - for (const output of gating) { - if (output === 'heavy') continue; - const camel = output.replace(/_(\w)/gu, (_, letter) => letter.toUpperCase()); - assert.match( - disjunction, - new RegExp(`plan\\.${camel}\\b`, 'u'), - `${output} gates an installed step but does not select the install`, - ); - } -}); - test('the app icon gate selects every file its own tests open', () => { // Derived from the step, not from a memory of it: whatever `App icon artwork // drift` runs is the authority on what has to select it. The list this From e7f2748ea295fd713c217f9c755d980bca653c4b Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:20:38 +0800 Subject: [PATCH 2/8] ci: derive the install contract from the gate that decides it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With `heavy` gone, `npm ci` is the only thing that decides whether a step has anything to run against, and its condition is a five-term disjunction that omits `state_root_compat`, `runtime_sandbox`, `runtime_host`, `standard_workspaces`, `e2e`, `storybook` and `app_icons`. Those hold today only through an implication nothing writes down — each of them also selects `code` — so moving one decoder path under `.github/`, which the planner's `code` loop skips, would gate a step on a selection that runs against a checkout with no `node_modules`, and every assertion phrased over the two lists would stay green. The assertion is now the implication itself, checked over the repository paths the planner's own source names, so a path added to one of its sets is exercised by the edit that adds it. The gate scrape also reads any selection named inside an `if:` rather than only `X == `, which is what dropped `standard_workspaces` — spelled `!= ''` and `contains(...)` — from the set. --- scripts/ci-workflow-policy.test.mjs | 78 +++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 02b249bfbe..f3cb744ba6 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -125,6 +125,48 @@ test('contract checks run before dependency setup and can fail the job', () => { } }); +test('every selection that gates a step needing dependencies selects the install', () => { + const workflow = readWorkflow('ci.yml'); + + // `npm ci` is the only thing that decides whether a later step has anything + // to run against, so a step below it gated on a selection that cannot reach + // the install would run on a bare checkout — or, if its `run:` tolerates + // that, report green having done nothing. Both sides are read out of the + // workflow, so a step or a term added to either is covered by the edit that + // adds it. + const install = workflow.indexOf('run: npm ci'); + assert.ok(install >= 0, 'ci.yml no longer installs dependencies'); + const installStep = workflow.slice( + workflow.lastIndexOf('\n - name:', install) + 1, + workflow.indexOf('\n - ', install), + ); + const installed = workflow.slice(workflow.indexOf('\n - ', install)); + + // Any selection named inside an `if:` gates that step, whichever way the + // condition spells it — `== 'true'`, `!= ''` and `contains(...)` are all + // already in this file, and recognising one form drops the rest silently. + assert.doesNotMatch(installed, /\n\s+if: [>|]/u, 'a block-scalar `if:` hides its own gates'); + const gating = conditionSelections(installed); + assert.ok(gating.length > 0, 'no step below the install is gated on a selection'); + const installGate = conditionSelections(installStep); + assert.ok(installGate.length > 0, 'the install step is unconditional'); + + // The implication, not the text: `state_root_compat` is not in the install + // condition and never needed to be, because every path that selects it also + // selects `code`. That is what has to hold, and it is unwritten — moving one + // of those paths under `.github/`, which the planner's `code` loop skips, + // breaks it while every assertion phrased over the two lists stays green. + for (const path of plannerPathCorpus()) { + const plan = planTests([path]); + const gated = gating.filter((output) => selects(plan, output)); + if (gated.length === 0) continue; + assert.ok( + installGate.some((output) => selects(plan, output)), + `${path} selects ${gated.join(', ')}, which gates a step run against no node_modules`, + ); + } +}); + test('the app icon gate selects every file its own tests open', () => { // Derived from the step, not from a memory of it: whatever `App icon artwork // drift` runs is the authority on what has to select it. The list this @@ -810,6 +852,42 @@ function pathFilter(name, trigger) { return paths; } +/** Plan selections named by any `if:` in `section`, whatever the condition spells. */ +function conditionSelections(section) { + return [ + ...new Set( + [...section.matchAll(/^\s+if: ([^\n]*)$/gmu)].flatMap(([, condition]) => + [...condition.matchAll(/steps\.plan\.outputs\.(\w+)/gu)].map(([, name]) => name), + ), + ), + ].sort(); +} + +/** The planner names selections in camelCase and publishes them in snake_case. */ +function selects(plan, output) { + const key = output.replace(/_(\w)/gu, (_, letter) => letter.toUpperCase()); + assert.ok(key in plan, `CI gates on ${output}, which the planner does not select`); + const value = plan[key]; + return Array.isArray(value) ? value.length > 0 : Boolean(value); +} + +/** + * Changed-file inputs to test an implication between selections with: every + * repository path the planner's own source names, plus a probe under each, + * because several selections are decided by `startsWith`. Derived from the + * planner, so a path added to one of its sets is exercised by the edit that + * adds it rather than by whoever remembers this list exists. + */ +function plannerPathCorpus() { + const source = readFileSync(new URL('./ci-test-plan.mjs', import.meta.url), 'utf8'); + const literals = [...source.matchAll(/'([\w.@][\w./@-]*)'/gu)] + .map(([, value]) => value) + .filter((value) => value.includes('/') || value.includes('.')); + assert.ok(literals.length > 0, 'the planner names no repository path'); + + return [...new Set(literals.flatMap((value) => [value, `${value}probe.ts`, `${value}/probe.ts`]))]; +} + /** * Workspace dirs `seeds` depend on, transitively, read off the same graph the * planner selects with rather than a second definition of the same edges. The From cfc28a82993fa59b9d536a167e11c72bc7762d1c Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:21:36 +0800 Subject: [PATCH 3/8] ci: cover every install-free entry point, not the ones one regex could match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The install-free import contract scraped `scripts/[\w.-]+\.test\.mjs`. That class cannot match a `/`, so `scripts/computer-use/lab-root.test.mjs` — run by a step above the install — was dropped, and the derivation yielded ten suites where the section names eleven. Four more entry points are reached through `npm run` rather than named as files: `windows-test-inventory.mjs`, `asf-npm-workflow-policy.test.mjs`, `check-app-shell-hooks.mjs` and `asf-license-headers.mjs`. Widening the class and expanding `npm run` one hop through `package.json` brings the set to eighteen, including the two planner-adjacent scripts the job itself runs. All of them are clean today, so this closes a latent gap rather than a live failure. --- scripts/ci-workflow-policy.test.mjs | 48 ++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index f3cb744ba6..9f90b7150f 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -768,28 +768,46 @@ test('core CI runs the live Eval proxy lifecycle when Eval is selected', () => { assert.doesNotMatch(evalPackage.scripts['test:dist'], /test_egress_filter_live\.py/u); }); -test('every suite that runs before dependency setup imports only node builtins', () => { - // The steps above `setup-node` run against a bare checkout, so a suite there - // that imports a devDependency throws `ERR_MODULE_NOT_FOUND` and turns the - // one required context red on every pull request — no merge, anywhere, until - // someone notices. Nothing about the file says so, which is how a closure - // assertion needing esbuild was very nearly added to one of them. +test('everything that runs before dependency setup imports only node builtins', () => { + // The steps above `setup-node` run against a bare checkout, so a script there + // that imports a devDependency throws `ERR_MODULE_NOT_FOUND`. Whether that + // turns the one required context red on every pull request or only on the + // introducing one depends on how the step is gated, and neither is a state + // anyone should reach by accident — a closure assertion needing esbuild was + // very nearly added to one of them. // - // Derived from the workflow: whichever suites those steps name are the ones - // that carry the constraint, so moving a step below the install lifts it and - // adding a step above imposes it, without anyone editing this test. + // Derived from the workflow: whichever entry points those steps name are the + // ones that carry the constraint, so moving a step below the install lifts it + // and adding a step above imposes it, without anyone editing this test. const workflow = readWorkflow('ci.yml'); const [installFree] = workflow.split(/\n\s+- uses: actions\/setup-node[^\n]*\n/u); - const suites = [ - ...new Set( - [...installFree.matchAll(/(scripts\/[\w.-]+\.test\.mjs)/gu)].map(([, path]) => path), - ), + + // `npm run` names a script, not a file, so expand one hop through the + // manifest. Four install-free entry points are reached only that way — + // `check:asf-headers` runs `scripts/asf-license-headers.mjs`, which carries + // the constraint and was outside the set that asserts it. + const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8')); + const commands = [ + installFree, + ...[...installFree.matchAll(/npm run ([\w:-]+)/gu)].map(([, name]) => { + const command = manifest.scripts?.[name]; + assert.ok(command, `ci.yml runs npm run ${name}, which package.json does not define`); + return command; + }), + ].join('\n'); + + // `[\w./-]`, not `[\w.-]`: a class without `/` cannot match a path with a + // directory in it, which is how `scripts/computer-use/lab-root.test.mjs` — + // named by a step above the install — was dropped from this derivation while + // the count still looked right. + const entryPoints = [ + ...new Set([...commands.matchAll(/(scripts\/[\w./-]+\.mjs)/gu)].map(([, path]) => path)), ].sort(); - assert.ok(suites.length > 0, 'no suite runs before dependency setup'); + assert.ok(entryPoints.length > 0, 'nothing runs before dependency setup'); // The constraint is transitive: a local module may be imported only if it too // stays inside `node:`. - const pending = [...suites]; + const pending = [...entryPoints]; const seen = new Set(pending); while (pending.length > 0) { const path = pending.shift(); From d0e346c6056a081ca40c905056a72b06533cf0d2 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:24:39 +0800 Subject: [PATCH 4/8] refactor(ci): read a workflow's pull_request paths in one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `windows-package-source-closure.mjs` parsed with the `yaml` package and `ci-workflow-policy.test.mjs` hand-scanned lines, because it runs before `npm ci` installs a parser. Compared across all eight workflows carrying a filter the two produce identical output, so one of them is redundant — and the install-free constraint decides which one has to stay. `scripts/workflow-pull-request-paths.mjs` is that reader, `node:` only. `readWindowsReleasePathPatterns` and `collectWindowsPackageSourceClosure` went with it: both were one-line forwards to the general version, called only from the suite next to them. Capability given up: tolerance for flow sequences (`paths: [a, b]`) and other legal YAML spellings of the same list. Several assertions over these filters already assume one entry per line. --- scripts/ci-workflow-policy.test.mjs | 73 +++----------- scripts/windows-package-source-closure.mjs | 19 ---- .../windows-package-source-closure.test.mjs | 14 +-- scripts/workflow-pull-request-paths.mjs | 98 +++++++++++++++++++ 4 files changed, 121 insertions(+), 83 deletions(-) create mode 100644 scripts/workflow-pull-request-paths.mjs diff --git a/scripts/ci-workflow-policy.test.mjs b/scripts/ci-workflow-policy.test.mjs index 9f90b7150f..f118a99863 100644 --- a/scripts/ci-workflow-policy.test.mjs +++ b/scripts/ci-workflow-policy.test.mjs @@ -32,6 +32,11 @@ import { readdirSync, readFileSync } from 'node:fs'; import test from 'node:test'; import { formatGitHubOutputs, loadWorkspaceGraph, planTests } from './ci-test-plan.mjs'; +import { + readPullRequestPathFilter, + readTriggerPathFilter, + workflowTriggerBlock, +} from './workflow-pull-request-paths.mjs'; test('GitHub output matches the selections consumed by CI', () => { const output = formatGitHubOutputs(planTests([], { forceFull: true })); @@ -373,9 +378,7 @@ test('the recovery lane pairs its path filter with a nightly run and a main push // Windows recovery run back on every pull request. The main push carries no // filter because `strict: false` lets a stale-base pull request go green, // and because a paths filter only sees the first 300 files of a diff. - // Stripped comment lines survive as blank ones, so the gap between the - // trigger and its list is any mix of blank and four-space lines. - assert.match(triggers, /\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u); + assert.ok(readPullRequestPathFilter('windows-recovery.yml').length > 0, 'no paths filter'); assert.match(triggers, /\n {2}push:\n {4}branches: \[main\]\n/u); assert.doesNotMatch( triggers.match(/\n {2}push:\n(?:(?: {4}[^\n]*)?\n)*/u)?.[0] ?? '', @@ -433,8 +436,8 @@ test('a lane that filters both triggers filters them on the same paths', () => { let checked = 0; for (const name of readdirSync(WORKFLOW_DIR).filter((file) => file.endsWith('.yml'))) { - const pullRequest = pathFilter(name, 'pull_request'); - const push = pathFilter(name, 'push'); + const pullRequest = readTriggerPathFilter(name, 'pull_request'); + const push = readTriggerPathFilter(name, 'push'); if (!pullRequest?.length || !push?.length) continue; assert.deepEqual(push, pullRequest, `${name}: pull_request and push filter different paths`); @@ -480,7 +483,7 @@ test('the recovery lane keeps every run kind out of one shared concurrency group test('the recovery lane leaves the suites it executes to the required test lane', () => { const workflow = readWorkflow('windows-recovery.yml'); - const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); + const filtered = new Set(readPullRequestPathFilter('windows-recovery.yml')); // Derived from the dist paths the steps run, then widened along the workspace // dependency graph the planner selects with. The separator class matches the @@ -513,7 +516,7 @@ test('the recovery lane leaves the suites it executes to the required test lane' }); test('the recovery lane filter follows the postinstall launcher chain', () => { - const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); + const filtered = new Set(readPullRequestPathFilter('windows-recovery.yml')); const manifest = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), 'utf8')); // Derived from postinstall itself, then one hop into whatever those entry // points launch, because a launcher the filter cannot see still decides what @@ -533,7 +536,7 @@ test('the recovery lane filter follows the postinstall launcher chain', () => { }); test('the recovery lane filters pull requests by what only Windows can prove', () => { - const filtered = new Set(pullRequestPathFilter('windows-recovery.yml')); + const filtered = new Set(readPullRequestPathFilter('windows-recovery.yml')); // What is left after the workspace sources came out: how `npm.cmd ci` resolves // and what it produces on Windows, what `npm.cmd run build:test` cleans up @@ -580,8 +583,8 @@ test('a filtered pull-request lane can still run when its filter misses', () => // `release-windows-check.yml`, whose filter was narrowed in this branch. const uncovered = []; for (const name of readdirSync(WORKFLOW_DIR).filter((file) => file.endsWith('.yml'))) { + if (readPullRequestPathFilter(name).length === 0) continue; const triggers = triggerBlock(name); - if (!/\n {2}pull_request:\n(?:(?: {4}[^\n]*)?\n)* {4}paths:/u.test(triggers)) continue; const push = triggers.match(/\n {2}push:\n(?:(?: {4,}[^\n]*)?\n)*/u)?.[0] ?? ''; const escapes = @@ -830,46 +833,6 @@ test('everything that runs before dependency setup imports only node builtins', const WORKFLOW_DIR = new URL('../.github/workflows/', import.meta.url); -/** - * Reads the `paths` list belonging to a workflow's `pull_request` trigger. - * Anchoring to the trigger, instead of matching entry text anywhere in the - * file, is what makes the filter assertions fail when entries move under - * `paths-ignore`, under another trigger, or out of `on:` altogether. - */ -function pullRequestPathFilter(name) { - const paths = pathFilter(name, 'pull_request'); - assert.ok(paths !== null, `${name}: no pull_request trigger`); - - return paths; -} -// Returns the trigger's `paths:` entries in order, or null when the workflow -// does not carry that trigger at all — which is what lets a caller tell "no -// such trigger" apart from "this trigger runs on everything". -// -// Reads the `on:` block with comments already stripped, so a comment between -// the trigger and its list cannot end the scan, and accepts the quoting and -// spacing YAML allows, so a legal rewrite reports the entries it really has -// instead of an empty list that reads as a missing filter. -function pathFilter(name, trigger) { - const lines = triggerBlock(name).split('\n'); - const start = lines.findIndex((line) => new RegExp(`^ {2}${trigger}:\\s*$`, 'u').test(line)); - if (start < 0) return null; - - const paths = []; - let inPaths = false; - for (const line of lines.slice(start + 1)) { - if (line.trim() === '') continue; - if (/^ {0,2}\S/u.test(line)) break; - if (/^ {4}\S/u.test(line)) { - inPaths = /^ {4}paths:\s*$/u.test(line); - continue; - } - const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null; - if (entry) paths.push(entry[1]); - } - return paths; -} - /** Plan selections named by any `if:` in `section`, whatever the condition spells. */ function conditionSelections(section) { return [ @@ -903,7 +866,9 @@ function plannerPathCorpus() { .filter((value) => value.includes('/') || value.includes('.')); assert.ok(literals.length > 0, 'the planner names no repository path'); - return [...new Set(literals.flatMap((value) => [value, `${value}probe.ts`, `${value}/probe.ts`]))]; + return [ + ...new Set(literals.flatMap((value) => [value, `${value}probe.ts`, `${value}/probe.ts`])), + ]; } /** @@ -930,14 +895,8 @@ function readWorkflow(name) { return readFileSync(new URL(name, WORKFLOW_DIR), 'utf8'); } -/** - * Reads the `on:` block only, so a workflow cannot escape a trigger contract by - * writing `on: [pull_request]`, and prose elsewhere in the file cannot fake one. - */ function triggerBlock(name) { - const withoutComments = readWorkflow(name).replaceAll(/^[ \t]*#.*$/gmu, ''); - - return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? ''; + return workflowTriggerBlock(readWorkflow(name)); } function hasPullRequestTrigger(name) { diff --git a/scripts/windows-package-source-closure.mjs b/scripts/windows-package-source-closure.mjs index f006863e85..63125608e9 100644 --- a/scripts/windows-package-source-closure.mjs +++ b/scripts/windows-package-source-closure.mjs @@ -20,7 +20,6 @@ import { existsSync, readFileSync } from 'node:fs'; import { extname, join, relative, resolve, sep } from 'node:path'; import { build } from 'esbuild'; -import { parse as parseYaml } from 'yaml'; const defaultRepoRoot = resolve(import.meta.dirname, '..'); const sourceExtensions = ['.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs', '.json']; @@ -31,10 +30,6 @@ export const windowsPackageSourceEntrypoints = [ 'packages/runtime/src/sandbox/index.ts', ]; -export async function collectWindowsPackageSourceClosure(repoRoot = defaultRepoRoot) { - return collectWorkspaceSourceClosure(windowsPackageSourceEntrypoints, repoRoot); -} - /** * Workspace sources `entryPoints` reach, transitively, resolved through the * workspace `exports` map back to `src` rather than to built `dist`. Two lanes @@ -64,20 +59,6 @@ export async function collectWorkspaceSourceClosure(entryPoints, repoRoot = defa .sort(); } -export function readWindowsReleasePathPatterns(repoRoot = defaultRepoRoot) { - return readPullRequestPathPatterns('release-windows-check.yml', repoRoot); -} - -export function readPullRequestPathPatterns(workflowName, repoRoot = defaultRepoRoot) { - const workflowPath = join(repoRoot, '.github', 'workflows', workflowName); - const workflow = parseYaml(readFileSync(workflowPath, 'utf8')); - const paths = workflow?.on?.pull_request?.paths; - if (!Array.isArray(paths) || !paths.every((path) => typeof path === 'string')) { - throw new Error(`${workflowName} must declare pull_request.paths as strings.`); - } - return paths; -} - export function windowsReleasePatternCoversSource(path, pattern) { if (path === pattern) return true; return pattern.endsWith('/**') && path.startsWith(pattern.slice(0, -2)); diff --git a/scripts/windows-package-source-closure.test.mjs b/scripts/windows-package-source-closure.test.mjs index 262717c7dc..92e4611386 100644 --- a/scripts/windows-package-source-closure.test.mjs +++ b/scripts/windows-package-source-closure.test.mjs @@ -21,12 +21,11 @@ import assert from 'node:assert/strict'; import { existsSync, readFileSync } from 'node:fs'; import test from 'node:test'; import { - collectWindowsPackageSourceClosure, collectWorkspaceSourceClosure, - readPullRequestPathPatterns, - readWindowsReleasePathPatterns, + windowsPackageSourceEntrypoints, windowsReleasePatternCoversSource, } from './windows-package-source-closure.mjs'; +import { readPullRequestPathFilter } from './workflow-pull-request-paths.mjs'; /** * The `packages/` half of this lane's filter that the import closure does not @@ -49,8 +48,9 @@ const UNDERIVED_PACKAGE_PATTERNS = new Map([ ]); test('the Windows package trigger is exactly its closure plus declared exceptions', async () => { - const closure = await collectWindowsPackageSourceClosure(); - const patterns = readWindowsReleasePathPatterns(); + const closure = await collectWorkspaceSourceClosure(windowsPackageSourceEntrypoints); + const patterns = readPullRequestPathFilter('release-windows-check.yml'); + assert.ok(patterns.length > 0, 'release-windows-check.yml declares no pull_request.paths'); const missing = closure.filter( (sourcePath) => !patterns.some((pattern) => windowsReleasePatternCoversSource(sourcePath, pattern)), @@ -82,7 +82,7 @@ test('the Windows package trigger is exactly its closure plus declared exception }); test('the Windows package workflow path list has no duplicate entries', () => { - const patterns = readWindowsReleasePathPatterns(); + const patterns = readPullRequestPathFilter('release-windows-check.yml'); assert.equal(new Set(patterns).size, patterns.length); }); @@ -98,7 +98,7 @@ test('the recovery filter is exactly the Windows-branching closure of its tests' new URL('../.github/workflows/windows-recovery.yml', import.meta.url), 'utf8', ); - const filtered = readPullRequestPathPatterns('windows-recovery.yml') + const filtered = readPullRequestPathFilter('windows-recovery.yml') .filter((path) => path.startsWith('packages/')) .sort(); diff --git a/scripts/workflow-pull-request-paths.mjs b/scripts/workflow-pull-request-paths.mjs new file mode 100644 index 0000000000..cc9b81f952 --- /dev/null +++ b/scripts/workflow-pull-request-paths.mjs @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * One reader for what a workflow's triggers filter on. Two lanes + * derive a path filter from an import closure and compare it against this, and + * `ci-workflow-policy.test.mjs` runs before `npm ci` installs a YAML parser, so + * the install-free spelling is the only one that can be shared — which makes it + * the one both use, rather than a second parser that agrees until it does not. + * + * Node builtins only, for that same reason. + * + * Given up by hand-scanning rather than parsing: flow sequences + * (`paths: [a, b]`), block scalars, anchors and the rest of legal YAML. The + * workflows are written one entry per line, and several assertions over these + * lists already depend on that. + */ + +import { readFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const defaultRepoRoot = dirname(dirname(fileURLToPath(import.meta.url))); + +export function readWorkflow(workflowName, repoRoot = defaultRepoRoot) { + return readFileSync(join(repoRoot, '.github', 'workflows', workflowName), 'utf8'); +} + +/** + * The `on:` block only, with comment lines stripped, so a workflow cannot + * escape a trigger contract by writing `on: [pull_request]`, prose elsewhere in + * the file cannot fake one, and a comment between a trigger and its list cannot + * end a scan. Stripped comments survive as blank lines. + */ +export function workflowTriggerBlock(source) { + const withoutComments = source.replaceAll(/^[ \t]*#.*$/gmu, ''); + + return withoutComments.match(/^on:(.*(?:\n(?![^\s#]).*)*)/mu)?.[1] ?? ''; +} + +/** + * The `paths` list belonging to one of a workflow's triggers, or `null` when it + * carries no such trigger at all — which is what lets a caller tell "no such + * trigger" apart from "this trigger runs on everything". Anchoring to the + * trigger, instead of matching entry text anywhere in the file, is what makes + * the filter assertions fail when entries move under `paths-ignore`, under + * another trigger, or out of `on:` altogether. + */ +export function triggerPathFilter(source, trigger) { + const lines = workflowTriggerBlock(source).split('\n'); + const start = lines.findIndex((line) => new RegExp(`^ {2}${trigger}:\\s*$`, 'u').test(line)); + if (start < 0) return null; + + // Accepts the quoting and spacing YAML allows, so a legal rewrite reports the + // entries it really has instead of an empty list that reads as no filter. + const paths = []; + let inPaths = false; + for (const line of lines.slice(start + 1)) { + if (line.trim() === '') continue; + if (/^ {0,2}\S/u.test(line)) break; + if (/^ {4}\S/u.test(line)) { + inPaths = /^ {4}paths:\s*$/u.test(line); + continue; + } + const entry = inPaths ? /^\s+-\s+['"]?(.+?)['"]?\s*$/u.exec(line) : null; + if (entry) paths.push(entry[1]); + } + return paths; +} + +export function readTriggerPathFilter(workflowName, trigger, repoRoot = defaultRepoRoot) { + return triggerPathFilter(readWorkflow(workflowName, repoRoot), trigger); +} + +/** + * The `pull_request` filter, with "no trigger" and "no filter" both reported as + * the empty list: every caller of this one asks what a pull request is filtered + * on, and neither answer is a filter. + */ +export function readPullRequestPathFilter(workflowName, repoRoot = defaultRepoRoot) { + return readTriggerPathFilter(workflowName, 'pull_request', repoRoot) ?? []; +} From 237f5e7879af1022d8a53e1699a08a322f444988 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:25:03 +0800 Subject: [PATCH 5/8] refactor(ci): keep the underived Windows patterns as the set they are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `UNDERIVED_PACKAGE_PATTERNS` was a `Map` whose values nothing read — the assertion takes `.keys()`. The reasons those five entries earn a Windows runner are worth keeping, so they stay as comments beside each entry, where a stale one is visible; the collection is a `Set`. --- scripts/windows-package-source-closure.test.mjs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/windows-package-source-closure.test.mjs b/scripts/windows-package-source-closure.test.mjs index 92e4611386..c1cfea95b5 100644 --- a/scripts/windows-package-source-closure.test.mjs +++ b/scripts/windows-package-source-closure.test.mjs @@ -33,18 +33,18 @@ import { readPullRequestPathFilter } from './workflow-pull-request-paths.mjs'; * it has to earn that here rather than sit indistinguishable among the twenty * derived entries — which is the only way a rotted one is ever noticed. */ -const UNDERIVED_PACKAGE_PATTERNS = new Map([ +const UNDERIVED_PACKAGE_PATTERNS = new Set([ // Regenerated and diffed by `check:release`, and consumed by the packaging // step rather than imported by the worker. - ['packages/cli/RUNTIME_HOST_PEER_DEPENDENCIES.rust.tsv', 'peer dependency manifest'], - ['packages/cli/RUNTIME_HOST_PEER_THIRD_PARTY_NOTICES.txt', 'peer dependency manifest'], + 'packages/cli/RUNTIME_HOST_PEER_DEPENDENCIES.rust.tsv', + 'packages/cli/RUNTIME_HOST_PEER_THIRD_PARTY_NOTICES.txt', // Candidate election runs in the packaged app, not in the worker closure, and // has broken this path before. - ['packages/runtime-host/src/client/connect-or-spawn.ts', 'Runtime Host candidate election'], - ['packages/runtime-host/src/client/launcher.ts', 'Runtime Host candidate election'], + 'packages/runtime-host/src/client/connect-or-spawn.ts', + 'packages/runtime-host/src/client/launcher.ts', // Builds the worker the closure starts from, so it precedes rather than joins // it. - ['packages/runtime/scripts/build-filesystem-worker.mjs', 'builds the worker itself'], + 'packages/runtime/scripts/build-filesystem-worker.mjs', ]); test('the Windows package trigger is exactly its closure plus declared exceptions', async () => { @@ -78,7 +78,7 @@ test('the Windows package trigger is exactly its closure plus declared exception .filter((pattern) => pattern.startsWith('packages/')) .filter((pattern) => !closure.some((path) => windowsReleasePatternCoversSource(path, pattern))) .sort(); - assert.deepEqual(underived, [...UNDERIVED_PACKAGE_PATTERNS.keys()].sort()); + assert.deepEqual(underived, [...UNDERIVED_PACKAGE_PATTERNS].sort()); }); test('the Windows package workflow path list has no duplicate entries', () => { From 0b52afdfe0a35d1235a28b3dc017efcea850a64f Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:25:50 +0800 Subject: [PATCH 6/8] docs(ci): say what the Windows filter derivations do not guarantee MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two limits that reading the code does not reveal, recorded rather than fixed. `collectWorkspaceSourceClosure` walks static imports, so a process boundary is invisible to it — `root-authority.test.ts` forks its race fixture and `worker-entry.ts` is bundled, and neither joins the closure. Both are `win32`-free today, so the `windows-recovery` filter is complete; "generated, not curated" overstated why. And `includes('win32')` is the criterion a machine can check, not the set of Windows-only paths: `git-worktree-child-executor.ts` carries one with no such literal. One fix alongside them: the executed-suite scrape accepted only forward slashes while the equivalent scrape in `ci-workflow-policy.test.mjs` handles `[/\\]`, and these steps run under pwsh where both are legal. A suite spelled with backslashes was dropped from the set that decides the filter, which is the fail-open direction. It derives the same fourteen suites today. --- scripts/windows-package-source-closure.mjs | 8 ++++++++ .../windows-package-source-closure.test.mjs | 18 +++++++++++++++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/scripts/windows-package-source-closure.mjs b/scripts/windows-package-source-closure.mjs index 63125608e9..4e546e79b7 100644 --- a/scripts/windows-package-source-closure.mjs +++ b/scripts/windows-package-source-closure.mjs @@ -36,6 +36,14 @@ export const windowsPackageSourceEntrypoints = [ * derive a path filter from this instead of maintaining one by hand, so a * dependency added anywhere under an entry point cannot escape the filter that * is supposed to schedule the runner able to observe it. + * + * Static imports only, which is the limit of what "generated, not curated" + * buys here: a process boundary is invisible to it. `root-authority.test.ts` + * forks `fixtures/root-initialization-race.js` and + * `filesystem-worker/worker-entry.ts` is bundled rather than imported, so + * neither appears in a closure that starts from them. Both are free of `win32` + * today, which is why the filters derived from this are complete — not because + * the derivation guarantees it. */ export async function collectWorkspaceSourceClosure(entryPoints, repoRoot = defaultRepoRoot) { const workspaces = loadWorkspacePackages(repoRoot); diff --git a/scripts/windows-package-source-closure.test.mjs b/scripts/windows-package-source-closure.test.mjs index c1cfea95b5..8af252ca14 100644 --- a/scripts/windows-package-source-closure.test.mjs +++ b/scripts/windows-package-source-closure.test.mjs @@ -105,11 +105,17 @@ test('the recovery filter is exactly the Windows-branching closure of its tests' // Derived from the dist tests the steps actually execute, mapped back to // source. A suite added to a step joins this set without anyone remembering // to widen the filter. + // The separator class matches the backslash form too, because these steps run + // under pwsh where both are legal — a suite spelled with backslashes would + // otherwise be dropped from the set that decides the filter, which is the + // fail-open direction. const entrypoints = [ ...new Set( - [...workflow.matchAll(/packages\/([\w-]+)\/dist\/__tests__\/([\w.-]+)\.test\.js/gu)].map( - ([, workspace, name]) => `packages/${workspace}/src/__tests__/${name}.test.ts`, - ), + [ + ...workflow.matchAll( + /packages[/\\]([\w-]+)[/\\]dist[/\\]__tests__[/\\]([\w.-]+)\.test\.js/gu, + ), + ].map(([, workspace, name]) => `packages/${workspace}/src/__tests__/${name}.test.ts`), ), ].sort(); assert.ok(entrypoints.length > 0, 'no executed suite was recognised'); @@ -122,6 +128,12 @@ test('the recovery filter is exactly the Windows-branching closure of its tests' // through the two listed lock authorities — sat outside a filter that was // supposed to cover exactly it. A superset check cannot see a file that // stopped branching and now schedules a Windows runner for nothing. + // + // `win32` is the criterion because it is the one a machine can check, not + // because it names every Windows-only path: `git-worktree-child-executor.ts` + // carries a genuine Windows workaround with no such literal. A branch written + // without it stays outside this filter, and the lane's schedule is what + // observes it. const closure = await collectWorkspaceSourceClosure(entrypoints); const windowsBranching = closure .filter((path) => From 1b20deb931c5a60bc97dd6cef79bc3a2b58798a8 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 22:27:22 +0800 Subject: [PATCH 7/8] ci: select the release gate when the shared path reader changes `check:release` is the only thing that runs `windows-package-source-closure.test.mjs`, which is now the only reader of `release-windows-check.yml`'s filter, and it reaches it through `workflow-pull-request-paths.mjs`. Without this the module could change and the suite that depends on it would not run. --- scripts/ci-test-plan.mjs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/ci-test-plan.mjs b/scripts/ci-test-plan.mjs index 64fd822ae9..a1acf8d6fe 100644 --- a/scripts/ci-test-plan.mjs +++ b/scripts/ci-test-plan.mjs @@ -88,6 +88,9 @@ const RELEASE_CONTRACT_FILES = new Set([ 'scripts/windows-upgrade-baseline.json', 'scripts/windows-package-source-closure.mjs', 'scripts/windows-package-source-closure.test.mjs', + // Reads the filter that closure test compares against, and `check:release` + // is the only gate that runs it against `release-windows-check.yml`. + 'scripts/workflow-pull-request-paths.mjs', ]); // What decides whether a build can read durable state an earlier release wrote. From fb998d5a044e4c43673cb20f1395a275341810c8 Mon Sep 17 00:00:00 2001 From: AstroHan Date: Tue, 1 Sep 2026 23:08:26 +0800 Subject: [PATCH 8/8] ci: give back the independent execution the folded matrices took MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .github/workflows/cli-package-validation.yml | 75 +++++++++++++------- scripts/release-cli-workflow-policy.test.mjs | 68 +++++++++++++++--- 2 files changed, 107 insertions(+), 36 deletions(-) diff --git a/.github/workflows/cli-package-validation.yml b/.github/workflows/cli-package-validation.yml index efd62c0b0b..a11d653674 100644 --- a/.github/workflows/cli-package-validation.yml +++ b/.github/workflows/cli-package-validation.yml @@ -66,13 +66,13 @@ on: value: ${{ jobs.build.outputs.release_candidate_run_attempt }} release_predecessor_version: description: Exact npm Nightly version qualified against this candidate - value: ${{ jobs.build.outputs.release_predecessor_version }} + value: ${{ jobs.state-root-qualification.outputs.release_predecessor_version }} release_predecessor_tarball_url: description: Exact npm Nightly tarball qualified against this candidate - value: ${{ jobs.build.outputs.release_predecessor_tarball_url }} + value: ${{ jobs.state-root-qualification.outputs.release_predecessor_tarball_url }} release_predecessor_integrity: description: npm SHA-512 integrity of the Nightly tarball qualified against this candidate - value: ${{ jobs.build.outputs.release_predecessor_integrity }} + value: ${{ jobs.state-root-qualification.outputs.release_predecessor_integrity }} workflow_dispatch: permissions: @@ -199,9 +199,6 @@ jobs: outputs: release_candidate_artifact_id: ${{ steps.release-candidate.outputs.artifact-id }} release_candidate_run_attempt: ${{ github.run_attempt }} - release_predecessor_version: ${{ steps.predecessor.outputs.version }} - release_predecessor_tarball_url: ${{ steps.predecessor.outputs.tarball_url }} - release_predecessor_integrity: ${{ steps.predecessor.outputs.integrity }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -213,12 +210,6 @@ jobs: cache: npm - name: Select the release npm toolchain run: npm install --global --no-audit --no-fund "$(node -p 'require("./package.json").packageManager')" - # Resolving one npm version is seconds of work against a registry this - # job already reaches, so it rides the runner that waits on the addon - # builds instead of holding a second one for the whole wait. - - name: Resolve the current npm Nightly as immutable evidence - id: predecessor - run: node scripts/release-cli-publication.mjs resolve-nightly-predecessor "$GITHUB_OUTPUT" - name: Install cargo-deny uses: taiki-e/install-action@1ed6d7be6168f6c9046541087ff549b6bc581fdf # v2 with: @@ -302,7 +293,13 @@ jobs: with: artifact-ids: ${{ needs.build.outputs.release_candidate_artifact_id }} path: packages/cli/release + # The two supported Node versions share a runner, so a failure on the + # first would otherwise mean the second never runs — which is the + # independence the matrix these replaced bought with `fail-fast: false`. + # The outcome is re-raised below, once both have had their turn. - name: Validate the installed tarball + id: first-node-smoke + continue-on-error: true run: node scripts/smoke-release-cli-package.mjs - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 if: matrix.second_node != '' @@ -314,12 +311,24 @@ jobs: - name: Validate the installed tarball on the second Node if: matrix.second_node != '' run: node scripts/smoke-release-cli-package.mjs + # `always()` because the second Node failing must not swallow the first. + - name: Report the first Node result + if: always() && steps.first-node-smoke.outcome != 'success' + env: + NODE_VERSION: ${{ matrix.node }} + run: | + echo "The installed tarball failed on Node $NODE_VERSION" >&2 + exit 1 state-root-qualification: name: Qualify released State Roots needs: build runs-on: ubuntu-24.04 timeout-minutes: 45 + outputs: + release_predecessor_version: ${{ steps.predecessor.outputs.version }} + release_predecessor_tarball_url: ${{ steps.predecessor.outputs.tarball_url }} + release_predecessor_integrity: ${{ steps.predecessor.outputs.integrity }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: @@ -340,15 +349,26 @@ jobs: with: artifact-ids: ${{ needs.build.outputs.release_candidate_artifact_id }} path: packages/cli/release + # Resolved here rather than on `build`, which is the most expensive job in + # the workflow: a registry blip used to forfeit the tarball build and + # every job downstream of it, and this is the only job that reads the + # answer. It still costs no runner of its own, and the exported identity + # is now this job's output. + - name: Resolve the current npm Nightly as immutable evidence + id: predecessor + run: node scripts/release-cli-publication.mjs resolve-nightly-predecessor "$GITHUB_OUTPUT" # Three runs of one script against one sandbox, not three runners. Two of # these transitions are between tarballs that were published and frozen, # so nothing in a pull request can change their outcome except the - # qualifier itself, and the third reads the candidate this run built. + # qualifier itself, and the third reads the candidate this run built — + # which is why it goes first. Sharing one `set -e` with the frozen pair + # meant a flaky `curl` on either of them left the only transition a pull + # request can influence unexecuted. - name: Qualify the released State Root transitions env: MAKA_QUALIFICATION_BWRAP_USE_SUDO: '1' - PREDECESSOR_TARBALL_URL: ${{ needs.build.outputs.release_predecessor_tarball_url }} - PREDECESSOR_INTEGRITY: ${{ needs.build.outputs.release_predecessor_integrity }} + PREDECESSOR_TARBALL_URL: ${{ steps.predecessor.outputs.tarball_url }} + PREDECESSOR_INTEGRITY: ${{ steps.predecessor.outputs.integrity }} run: | set -euo pipefail evidence_root="$RUNNER_TEMP/released-state-root" @@ -393,6 +413,10 @@ jobs: echo "::endgroup::" } + qualify current-nightly-predecessor-to-candidate \ + "$PREDECESSOR_TARBALL_URL" '' "$PREDECESSOR_INTEGRITY" \ + candidate '' '' any + qualify cross-epoch-74-to-76 \ https://registry.npmjs.org/maka-agent/-/maka-agent-0.2.0-dev.3.20260830.tgz \ 66b1ce9307c9d5c06eaa7a6cbf533d4747d02caf71c1776c69c7dbfa12c3f414 '' \ @@ -404,29 +428,26 @@ jobs: b7d48adb466e16be7ffefbda3a0fcd833cc4108ea502b27778d0f4da680e1fc0 '' \ published https://registry.npmjs.org/maka-agent/-/maka-agent-0.2.0-dev.5.20260830.tgz \ e7a682157c6899fc7f1be86a2d7b0bd0696195a5771d8cc97bd1389a5b74989f same - - qualify current-nightly-predecessor-to-candidate \ - "$PREDECESSOR_TARBALL_URL" '' "$PREDECESSOR_INTEGRITY" \ - candidate '' '' any # The three transitions used to be three matrix jobs, so one failing left # the others to upload their own reports. Folded into one step they share - # a `set -e`, and the reports are wanted most on the run that failed — - # `tee` has already written the failing transition's own output by then. - # `if-no-files-found` stays `error` so a broken path is still caught on a - # green run. + # a `set -e`, and `tee` has already written the failing transition's own + # output by then. `if-no-files-found` stays `error` on a green run, where + # an empty directory means a broken path; on a red one it drops to `warn`, + # because a `curl` that failed before any `tee` would otherwise add a + # second, unrelated red to a job that already reported the real one. - name: Preserve the qualification reports if: always() uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: released-state-root path: ${{ runner.temp }}/released-state-root/*-report.json - if-no-files-found: error + if-no-files-found: ${{ job.status == 'success' && 'error' || 'warn' }} retention-days: 7 - name: Require the qualified Nightly predecessor to remain current env: - PREDECESSOR_VERSION: ${{ needs.build.outputs.release_predecessor_version }} - PREDECESSOR_TARBALL_URL: ${{ needs.build.outputs.release_predecessor_tarball_url }} - PREDECESSOR_INTEGRITY: ${{ needs.build.outputs.release_predecessor_integrity }} + PREDECESSOR_VERSION: ${{ steps.predecessor.outputs.version }} + PREDECESSOR_TARBALL_URL: ${{ steps.predecessor.outputs.tarball_url }} + PREDECESSOR_INTEGRITY: ${{ steps.predecessor.outputs.integrity }} run: | node scripts/release-cli-publication.mjs assert-nightly-predecessor \ "$PREDECESSOR_VERSION" \ diff --git a/scripts/release-cli-workflow-policy.test.mjs b/scripts/release-cli-workflow-policy.test.mjs index 430c837f1f..743cd984c3 100644 --- a/scripts/release-cli-workflow-policy.test.mjs +++ b/scripts/release-cli-workflow-policy.test.mjs @@ -48,22 +48,25 @@ test('validation consumers download the artifact produced by the build job', () test('CLI validation qualifies exact published State Roots without weakening artifact identity', () => { const workflow = readWorkflow('cli-package-validation.yml'); - // The predecessor is resolved on the job that already waits on the addon - // builds, so the exported identity comes from `build` rather than a job of - // its own. The exported names are the contract callers hold. + // The predecessor is resolved on the only job that reads it, rather than on + // `build`, where a registry blip forfeited the most expensive job in the + // workflow and everything downstream of it. The exported names are the + // contract callers hold; the job behind them is not, because a reusable + // workflow publishes its outputs only once every job has finished. assert.match( workflow, - /release_predecessor_version:[\s\S]*?value: \$\{\{ jobs\.build\.outputs\.release_predecessor_version \}\}/u, + /release_predecessor_version:[\s\S]*?value: \$\{\{ jobs\.state-root-qualification\.outputs\.release_predecessor_version \}\}/u, ); assert.match( workflow, - /release_predecessor_integrity:[\s\S]*?jobs\.build\.outputs\.release_predecessor_integrity/u, + /release_predecessor_integrity:[\s\S]*?jobs\.state-root-qualification\.outputs\.release_predecessor_integrity/u, ); assert.match( workflow, /id: predecessor\n\s+run: node scripts\/release-cli-publication\.mjs resolve-nightly-predecessor "\$GITHUB_OUTPUT"/u, ); assert.match(workflow, /state-root-qualification:\n[\s\S]*?needs: build\n/u); + assert.doesNotMatch(workflow, /needs\.build\.outputs\.release_predecessor/u); // Both frozen transitions keep their exact digests and the epoch relation // 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 // `workflow_call` output, so callers hold it too. assert.match( workflow, - /release_predecessor_tarball_url:[\s\S]*?value: \$\{\{ jobs\.build\.outputs\.release_predecessor_tarball_url \}\}/u, + /release_predecessor_tarball_url:[\s\S]*?value: \$\{\{ jobs\.state-root-qualification\.outputs\.release_predecessor_tarball_url \}\}/u, ); for (const name of ['tarball_url', 'integrity']) { assert.match( workflow, new RegExp( - `PREDECESSOR_${name.toUpperCase()}: \\$\\{\\{ needs\\.build\\.outputs\\.release_predecessor_${name} \\}\\}`, + `PREDECESSOR_${name.toUpperCase()}: \\$\\{\\{ steps\\.predecessor\\.outputs\\.${name} \\}\\}`, 'u', ), name, @@ -111,14 +114,61 @@ test('CLI validation qualifies exact published State Roots without weakening art assert.match(qualify, /source_integrity/u); assert.match(qualify, /createHash\('sha512'\)/u); assert.match(qualify, /source_sha256="\$\(sha256sum/u); + // The candidate is the only transition a pull request can influence, and the + // three share one `set -e`, so it runs before either frozen tarball is + // fetched. Read as positions in the script rather than restated, so a + // reordering fails here instead of silently moving it back behind a `curl`. + const script = [ + 'current-nightly-predecessor-to-candidate', + 'cross-epoch-74-to-76', + 'same-epoch-76', + ].map((slug) => qualify.indexOf(`qualify ${slug}`)); + assert.ok( + script.every((index) => index >= 0), + 'a declared State Root transition is no longer invoked', + ); + assert.deepEqual( + [...script].sort((left, right) => left - right), + script, + ); + const preserve = namedStep(steps, 'Preserve the qualification reports'); - assert.match(preserve, /if-no-files-found: error/u); + // `error` on a green run, where an empty directory means a broken path, and + // `warn` on a red one: paired with `if: always()`, a plain `error` turned a + // `curl` that failed before any `tee` into a second red on an already-red + // job. + assert.match( + preserve, + /if-no-files-found: \$\{\{ job\.status == 'success' && 'error' \|\| 'warn' \}\}/u, + ); const freshness = namedStep(steps, 'Require the qualified Nightly predecessor to remain current'); assert.match(freshness, /assert-nightly-predecessor/u); - assert.match(freshness, /needs\.build\.outputs\.release_predecessor_version/u); + assert.match(freshness, /steps\.predecessor\.outputs\.version/u); assert.ok(steps.indexOf(freshness) > steps.indexOf(preserve)); }); +test('both supported Node versions validate the tarball even when the first fails', () => { + // One runner and one tarball, so the two Node versions are two steps rather + // than two jobs. Without this the first failing would end the job and the + // second would never run at all — the matrix these replaced set + // `fail-fast: false` for exactly that. + const steps = workflowSteps(readWorkflow('cli-package-validation.yml')); + const first = namedStep(steps, 'Validate the installed tarball'); + assert.match(first, /id: first-node-smoke/u); + assert.match(first, /continue-on-error: true/u); + + const second = namedStep(steps, 'Validate the installed tarball on the second Node'); + assert.match(second, /if: matrix\.second_node != ''/u); + assert.ok(steps.indexOf(second) > steps.indexOf(first)); + + // `continue-on-error` alone would report a failing first Node as green, and + // without `always()` a failing second Node would swallow it instead. + const report = namedStep(steps, 'Report the first Node result'); + assert.match(report, /if: always\(\) && steps\.first-node-smoke\.outcome != 'success'/u); + assert.match(report, /exit 1/u); + assert.ok(steps.indexOf(report) > steps.indexOf(second)); +}); + test('npm mutations revalidate the exact qualified Nightly predecessor', () => { const nightly = readWorkflow('npm-publication.yml'); const nightlyFence = namedStep(