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/.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/ci-test-plan.mjs b/scripts/ci-test-plan.mjs index 0e75557442..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. @@ -531,23 +534,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 +542,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..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 })); @@ -125,35 +130,44 @@ 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', () => { +test('every selection that gates a step needing dependencies selects the install', () => { 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`, + // `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`, ); } }); @@ -364,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] ?? '', @@ -424,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`); @@ -471,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 @@ -504,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 @@ -524,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 @@ -571,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 = @@ -759,28 +771,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(); @@ -803,44 +833,42 @@ test('every suite 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`); +/** 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(); +} - return paths; +/** 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); } -// 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; + +/** + * 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`])), + ]; } /** @@ -867,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/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( diff --git a/scripts/windows-package-source-closure.mjs b/scripts/windows-package-source-closure.mjs index f006863e85..4e546e79b7 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,16 +30,20 @@ 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 * 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); @@ -64,20 +67,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..8af252ca14 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 @@ -34,23 +33,24 @@ import { * 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 () => { - 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)), @@ -78,11 +78,11 @@ 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', () => { - const patterns = readWindowsReleasePathPatterns(); + const patterns = readPullRequestPathFilter('release-windows-check.yml'); assert.equal(new Set(patterns).size, patterns.length); }); @@ -98,18 +98,24 @@ 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(); // 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) => 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) ?? []; +}