diff --git a/.github/workflows/test-eql.yml b/.github/workflows/test-eql.yml index 8fc829565..c26843b06 100644 --- a/.github/workflows/test-eql.yml +++ b/.github/workflows/test-eql.yml @@ -81,8 +81,9 @@ on: # surface removed in 3.0.0. All four were missing from all three copies of # this list, so a push to main touching only docs started no run of this # workflow at all — `pull_request` was unaffected, because it applies no - # `paths:` filter and `docs-static` is deliberately not relevance-gated. - # Derived rather than remembered now: the second half of + # `paths:` filter and `docs-static` was never gated on this list. (It is + # gated on the `changes` job's wider `eql:` filter, which selects the whole + # subtree.) Derived rather than remembered now: the second half of # `scripts/__tests__/eql-workflow-filters.test.mjs` walks each `mise run` # out to the paths the task names and fails on any this list does not select. paths: @@ -149,13 +150,18 @@ concurrency: jobs: # Runs on EVERY event and MUST always succeed (never skipped, never failed) — - # downstream heavy jobs `needs: [changes]`, and a skipped/failed `changes` + # every gated job `needs: [changes]`, and a skipped/failed `changes` # would either skip the merge-queue matrix or deadlock `ci-required`. + # + # Two outputs. `relevant` gates the heavy jobs. `eql` gates the three cheap + # ones (`docs-static`, `doc-anchors`, `known-failures`), which read more of + # the subtree than `relevant` selects. See the filter step below. changes: name: "Detect relevant changes" runs-on: blacksmith-16vcpu-ubuntu-2204 outputs: relevant: ${{ steps.r.outputs.relevant }} + eql: ${{ steps.r.outputs.eql }} steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 with: @@ -186,11 +192,37 @@ jobs: # reading as coverage). # # The four documentation entries are inputs to `docs-static` and - # `doc-anchors`, neither of which is relevance-gated, so this copy - # does not strictly need them. It carries them because the three - # copies are held EQUAL — see the note above the `push` list, and the - # reasoning on `bench-eql.yml`'s own filter for why equal beats - # minimal. + # `doc-anchors`, which are gated on `eql:` rather than on this list, + # so this copy does not strictly need them. It lists them because the + # three copies are kept EQUAL — see the note above the `push` list, + # and the reasoning on `bench-eql.yml`'s own filter for why equal + # beats minimal. + # + # `eql:` is a SECOND question, not a fourth copy of the list, and the + # parity check reads `relevant:` alone. It gates `docs-static`, + # `doc-anchors` and `known-failures`, which read more of the subtree + # than `relevant:` selects: `test:public_identifiers` runs `git grep` + # over every tracked file under `packages/eql/`, and + # `test:doc-anchors` reads every tracked `*.md` file there + # (`AGENTS.md`, `CHANGELOG.md`, `DEVELOPMENT.md`, …). So it takes the + # whole subtree, plus what the three jobs read OUTSIDE it: + # + # * `mise.toml` and the task files its `[task_config].includes` + # names. mise reads config from every parent directory, so each + # job installs the root's tools (go, golangci-lint, wasm-pack) + # and loads the root's tasks. Four of those task files are under + # the crate globs below; `stack-guest-abi`'s is not. + # * `.cargo/config.toml`, `Cargo.toml` and the five stack-* crates. + # `docs:validate:source` depends on `build`, which runs + # `cargo run -p eql-codegen`. cargo reads config from every parent + # directory, and to resolve the EQL workspace it loads the crates + # `eql-bindings` and `tests/encryption` reach by path, plus the + # root manifest those crates inherit from. + # + # None of the three jobs runs `require-cs-secrets`, so it is absent. + # `eql-workflow-filters.test.mjs` fails if this list stops selecting + # any input named above; `eql-suite-ci.test.mjs` fails if it selects + # any other path outside the subtree. filters: | relevant: - ".github/workflows/test-eql.yml" @@ -213,12 +245,24 @@ jobs: - "packages/stack-auth/**" - "packages/stack-profile/**" - "Cargo.toml" + eql: + - ".github/workflows/test-eql.yml" + - "packages/eql/**" + - "mise.toml" + - "packages/stack-guest-abi/tasks.toml" + - ".cargo/config.toml" + - "Cargo.toml" + - "packages/stack-encrypt/**" + - "packages/stack-encrypt-derive/**" + - "packages/stack-kms/**" + - "packages/stack-auth/**" + - "packages/stack-profile/**" # Explicit default (not `|| 'true'`, which trips GitHub's inconsistent # treatment of the string 'false'). push/schedule/merge_group/dispatch # never run the filter above — it needs a base ref that only a pull - # request has — so they take the default, and `push` is already narrowed - # by its own `on:`-level `paths:`. + # request has — so both outputs take the default, and `push` is already + # narrowed by its own `on:`-level `paths:`. # # The event arrives through `env:`, not through a `${{ }}` interpolated # into the body. That is what lets @@ -229,12 +273,15 @@ jobs: env: EVENT_NAME: ${{ github.event_name }} FILTER_RELEVANT: ${{ steps.f.outputs.relevant }} + FILTER_EQL: ${{ steps.f.outputs.eql }} run: | set -euo pipefail if [ "$EVENT_NAME" = "pull_request" ]; then echo "relevant=$FILTER_RELEVANT" >> "$GITHUB_OUTPUT" + echo "eql=$FILTER_EQL" >> "$GITHUB_OUTPUT" else echo "relevant=true" >> "$GITHUB_OUTPUT" + echo "eql=true" >> "$GITHUB_OUTPUT" fi # Pure bash; no checkout/toolchain. Derives the PG-version + shard fan-out @@ -833,12 +880,19 @@ jobs: mise run --output prefix test:splinter --postgres "${POSTGRES_VERSION}" # Source-only SQL documentation validation (coverage + required Doxygen tags). - # Deliberately NOT relevance-gated: it runs on EVERY pull_request — including - # docs-only PRs that skip the heavy jobs — so documentation is always - # validated. DB-free and creds-free (the psql-backed syntax check stays in the - # per-version `validate` job). + # Gated on `eql`, not `relevant`: it runs on every pull_request that touches + # the EQL subtree or a file it reads outside it, and skips the rest. The + # `relevant` list leaves out files this job reads — `test:public_identifiers` + # greps every tracked file under `packages/eql/`. DB-free and creds-free (the + # psql-backed syntax check stays in the per-version `validate` job). docs-static: name: "SQL doc validation" + needs: [changes] + # Exclusion, not an allowlist — see the note on `build-archive`. No fork + # clause: this job has no credentials. + if: >- + github.event_name != 'pull_request' + || needs.changes.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -875,9 +929,21 @@ jobs: # this is the half that stops a suppression outliving a closed issue. # # Credential-free and DB-free — it only reads the registry and asks GitHub for - # issue state, so it runs on every PR rather than hiding behind the e2e job. + # issue state, so it runs on its own rather than behind the e2e job. + # + # Gated on `eql`. The second step asks GitHub whether each issue is open NOW, + # so ungated, closing an issue would fail the next run of every pull request, + # including one that touches nothing in EQL. It still runs on every PR that + # touches EQL, and on every push and schedule run, where a closed issue + # belongs. known-failures: name: "known-failure markers" + needs: [changes] + # Exclusion, not an allowlist — see the note on `build-archive`. No fork + # clause: this job has no credentials. + if: >- + github.event_name != 'pull_request' + || needs.changes.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 permissions: contents: read @@ -910,12 +976,19 @@ jobs: run: | mise run test:known-failures - # Markdown anchor links. DB-free, credential-free and fast, and deliberately - # NOT relevance-gated: its inputs are the docs themselves, so gating it on the - # `relevant` filter (src/**, crates/**) would skip it on exactly the docs-only - # PRs it exists to check. + # Markdown anchor links. DB-free, credential-free and fast. Gated on `eql`, + # not `relevant`: it reads every tracked `*.md` file in the subtree, and the + # `relevant` list leaves out several of them (`AGENTS.md`, `CHANGELOG.md`, + # `DEVELOPMENT.md`, …), so gating it there would skip it on exactly the + # docs-only PRs it exists to check. doc-anchors: name: "doc anchor links" + needs: [changes] + # Exclusion, not an allowlist — see the note on `build-archive`. No fork + # clause: this job has no credentials. + if: >- + github.event_name != 'pull_request' + || needs.changes.outputs.eql == 'true' runs-on: blacksmith-16vcpu-ubuntu-2204 steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 @@ -992,8 +1065,9 @@ jobs: # protection never references an event-dependent leaf name (which would # deadlock). Passes iff every needed job is success or skipped. Treating # skipped as pass is intentional: heavy jobs are legitimately skipped on - # docs-only PRs, and a genuine failure is still caught because the FAILING - # source job is itself in `needs` and reports failure. + # docs-only PRs, the three `eql`-gated jobs on PRs that touch no EQL input, + # and a genuine failure is still caught because the FAILING source job is + # itself in `needs` and reports failure. ci-required: name: "ci-required" needs: [changes, setup, build-archive, test, validate, schema, rust-crates, diff --git a/AGENTS.md b/AGENTS.md index ce21666b0..9343ecf68 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -440,7 +440,13 @@ monorepo, which is where the silent failures are. `docs-static`'s `mise run test:docs_v3_grep` scanned every one of them: a push to main touching only documentation started no EQL workflow at all. **`pull_request` was never affected** — it applies no `paths:` filter, and - `docs-static` and `doc-anchors` are deliberately not relevance-gated. The + `docs-static` and `doc-anchors` never read `relevant:`. They and + `known-failures` are gated on the same step's second key, `eql:`, which is + **not a fourth copy** and is deliberately wider: the whole subtree (the jobs + `git grep` every tracked file and read every tracked `*.md` there), plus the + root `mise.toml` and its task includes, `.cargo/config.toml`, and the crates + cargo loads to resolve the EQL workspace. The parity check reads `relevant:` + alone; `eql:` has its own checks in the same file. The derivation reads paths that are WRITTEN DOWN; it cannot see `postgres:up` picking up `tests/docker-compose.yml` from its working directory, or the glob pathspec in `tasks/test/doc-anchors.sh` (`git ls-files '*.md'`, i.e. every diff --git a/packages/eql/tasks/docs/validate/source.sh b/packages/eql/tasks/docs/validate/source.sh index 5829a7f60..81c4cebf5 100755 --- a/packages/eql/tasks/docs/validate/source.sh +++ b/packages/eql/tasks/docs/validate/source.sh @@ -5,8 +5,8 @@ # # This is the DB-free subset of `docs:validate`: coverage + required-tags read # the `--!` doxygen comments out of src/**/*.sql and need no Postgres. It exists -# so CI can validate documentation on EVERY PR (including docs-only PRs that skip -# the heavy, relevance-gated jobs) without standing up a database. The +# so CI can validate documentation on every PR that touches EQL (including PRs +# that skip the heavy jobs) without standing up a database. The # `documented-sql` syntax check (which needs psql) stays in the per-Postgres # `validate` job. diff --git a/scripts/__tests__/eql-matrix-triggers.test.mjs b/scripts/__tests__/eql-matrix-triggers.test.mjs index 1f0fe9f60..7f1acf7a8 100644 --- a/scripts/__tests__/eql-matrix-triggers.test.mjs +++ b/scripts/__tests__/eql-matrix-triggers.test.mjs @@ -137,17 +137,27 @@ const MATRIX_SOURCE = (() => { return null })() -/** The relevance flag every gated job's `if:` reads. */ -const RELEVANCE_SOURCE = (() => { +/** + * Every flag a job `if:` reads, de-duplicated. More than one: the heavy jobs + * read `relevant` and the cheap ones read `eql`, and a wrong default on either + * silently skips its jobs on push and schedule. + */ +const GATE_SOURCES = (() => { + const found = new Map() for (const job of Object.values(wf?.jobs ?? {})) { - const ref = needsOutputRef(job?.if) - if (ref) return ref + for (const match of String(job?.if ?? '').matchAll( + /needs\.([A-Za-z0-9_-]+)\.outputs\.([A-Za-z0-9_-]+)/g, + )) { + found.set(`${match[1]}.${match[2]}`, { job: match[1], output: match[2] }) + } } - return null + return [...found.values()] })() +/** A minimum, not an equality: a third flag is checked the day it lands. */ +const EXPECTED_GATE_FLAGS = ['eql', 'relevant'] + const MATRIX_STEP = MATRIX_SOURCE ? stepProducing(MATRIX_SOURCE) : null -const RELEVANCE_STEP = RELEVANCE_SOURCE ? stepProducing(RELEVANCE_SOURCE) : null /** * The step `env:` key wired to `${{ github.event_name }}`. @@ -281,32 +291,41 @@ describe('the EQL matrix is reachable by a trigger that fires', () => { }) }) -describe('the relevance flag opens on every non-PR event', () => { - it('follows the workflow to the step that computes it', () => { +describe('every gate flag opens on every non-PR event', () => { + it('finds the flags the job conditions read', () => { expect( - RELEVANCE_SOURCE, - `No job \`if:\` in ${WORKFLOW} reads a \`needs..outputs.\` relevance flag, so this suite has nothing to evaluate.`, - ).toBeTruthy() - expect(RELEVANCE_STEP).toBeTruthy() - expect( - eventEnvKey(RELEVANCE_STEP?.step), - `The "${RELEVANCE_SOURCE?.job}" job's \`${RELEVANCE_STEP?.stepId}\` step must read the event from its \`env:\` rather than inlining \`\${{ github.event_name }}\`, for the reason given on the matrix step.`, - ).toBeTruthy() + GATE_SOURCES.map((source) => source.output), + `The job \`if:\`s in ${WORKFLOW} no longer read every flag this file expects, so a default below goes unchecked. Found: ${GATE_SOURCES.map((s) => `needs.${s.job}.outputs.${s.output}`).join(', ') || '(none)'}`, + ).toEqual(expect.arrayContaining(EXPECTED_GATE_FLAGS)) }) - for (const event of CANDIDATE_EVENTS.filter((e) => e !== 'pull_request')) { - it(`defaults to relevant on ${event}`, () => { - // The path filter only runs on `pull_request` — it needs a base ref — so - // on every other event the flag is a hardcoded default. Get that default - // wrong and every gated job skips on the new trigger while the run still - // reports success, which is the same silent-skip class as the trigger bug - // this file exists for. - const outputs = runStep(RELEVANCE_STEP, event) + for (const source of GATE_SOURCES) { + const step = stepProducing(source) + + it(`follows ${source.output} to the step that computes it`, () => { expect( - outputs[RELEVANCE_SOURCE.output], - `The relevance step wrote ${JSON.stringify(outputs)} on a ${event} event. Every heavy job is gated on this being 'true', and the filter that would compute it does not run outside \`pull_request\`.`, - ).toBe('true') + step, + `\`jobs.${source.job}.outputs.${source.output}\` does not resolve to a step \`id:\` in that job.`, + ).toBeTruthy() + expect( + eventEnvKey(step?.step), + `The "${source.job}" job's \`${step?.stepId}\` step must read the event from its \`env:\` rather than inlining \`\${{ github.event_name }}\`, for the reason given on the matrix step.`, + ).toBeTruthy() }) + + for (const event of CANDIDATE_EVENTS.filter((e) => e !== 'pull_request')) { + it(`defaults ${source.output} to true on ${event}`, () => { + // The path filter only runs on `pull_request` — it needs a base ref — + // so on every other event the flag is a hardcoded default. Get it wrong + // and every job gated on it skips on that trigger while the run still + // reports success: the silent-skip class this file exists for. + const outputs = runStep(step, event) + expect( + outputs[source.output], + `The step wrote ${JSON.stringify(outputs)} on a ${event} event. The jobs gated on \`${source.output}\` run only when it is 'true', and the filter that would compute it does not run outside \`pull_request\`.`, + ).toBe('true') + }) + } } }) diff --git a/scripts/__tests__/eql-suite-ci.test.mjs b/scripts/__tests__/eql-suite-ci.test.mjs index fcf8a7ad9..72fdc59ab 100644 --- a/scripts/__tests__/eql-suite-ci.test.mjs +++ b/scripts/__tests__/eql-suite-ci.test.mjs @@ -3,6 +3,7 @@ import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' import { dirname, join, relative, sep } from 'node:path' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' +import { inheritedToolConfig } from './lib/inherited-tool-config.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow, WORKFLOW_DIR, workflowFiles } from './lib/workflows.mjs' @@ -204,18 +205,18 @@ describe('the relevance filter still selects the imported tree', () => { const wf = readWorkflow(EQL_WORKFLOW) /** - * Every path the `dorny/paths-filter` step lists, across every filter it - * declares. `filters:` is a YAML document embedded in a YAML string, so it is + * Every filter the `dorny/paths-filter` step declares, as `[key, paths]`. + * `filters:` is a YAML document embedded in a YAML string, so it is * parsed rather than pattern-matched — a regex over the raw block would also * match the surrounding commentary. */ - const filterPaths = Object.values(wf?.jobs ?? {}) + const filters = Object.values(wf?.jobs ?? {}) .flatMap((job) => job?.steps ?? []) .filter((step) => (step?.uses ?? '').startsWith('dorny/paths-filter@')) - .flatMap((step) => { - const filters = yaml.load(step?.with?.filters ?? '') - return Object.values(filters ?? {}).flat() - }) + .flatMap((step) => + Object.entries(yaml.load(step?.with?.filters ?? '') ?? {}), + ) + const filterPaths = filters.flatMap(([, paths]) => paths) it('finds the paths-filter step', () => { // Without this the check below iterates an empty list and passes having @@ -371,10 +372,18 @@ describe('the relevance filter still selects the imported tree', () => { ).toContain('packages/stack-auth') }) + /** + * The fourth legitimate class: config that mise and cargo read from the + * parent directories of the subtree, which the cheap jobs' `eql:` filter + * selects because they run both tools there. + */ + const inheritedConfig = inheritedToolConfig(EQL_PREFIX.replace(/\/$/, '')) + it('prefixes every source path with the subtree root', () => { - // Three legitimate exceptions, all outside the subtree by nature: the - // workflow's own path, a composite action it `uses:`, and a crate the - // workspace compiles by path (plus the root manifest it inherits from). + // Four legitimate exceptions, all outside the subtree by nature: the + // workflow's own path, a composite action it `uses:`, a crate the + // workspace compiles by path (plus the root manifest it inherits from), + // and parent-directory config that mise and cargo read. const external = externalPathDependencies.dirs.map((dir) => `${dir}/**`) const offenders = filterPaths.filter( (path) => @@ -382,30 +391,36 @@ describe('the relevance filter still selects the imported tree', () => { !path.startsWith(EQL_PREFIX) && !localActionPrefixes.some((prefix) => path.startsWith(prefix)) && !external.includes(path) && - !(externalPathDependencies.inherits && path === 'Cargo.toml'), + !(externalPathDependencies.inherits && path === 'Cargo.toml') && + !inheritedConfig.includes(path), ) expect( offenders, - `These \`paths:\` entries in ${EQL_WORKFLOW} are not under \`${EQL_PREFIX}\`, are not the workflow itself, are not a composite action it \`uses:\`, and are not a crate the EQL workspace compiles by path (${external.join(', ')}). dorny/paths-filter matches repo-root-relative paths, so after the subtree import an unprefixed glob matches the WRONG tree — \`src/**\` selects \`languages/typescript/packages/stack/src/**\` and never \`packages/eql/src/**\`. The heavy jobs then skip on real EQL changes, report \`skipped\`, and \`ci-required\` treats skipped as pass.\n${offenders.map((p) => ` ${p}`).join('\n')}`, + `These \`paths:\` entries in ${EQL_WORKFLOW} are not under \`${EQL_PREFIX}\`, are not the workflow itself, are not a composite action it \`uses:\`, are not a crate the EQL workspace compiles by path (${external.join(', ')}), and are not config mise or cargo read from a parent directory (${inheritedConfig.join(', ')}). dorny/paths-filter matches repo-root-relative paths, so after the subtree import an unprefixed glob matches the WRONG tree — \`src/**\` selects \`languages/typescript/packages/stack/src/**\` and never \`packages/eql/src/**\`. The heavy jobs then skip on real EQL changes, report \`skipped\`, and \`ci-required\` treats skipped as pass.\n${offenders.map((p) => ` ${p}`).join('\n')}`, ).toEqual([]) }) - it('selects every crate the EQL workspace compiles by path', () => { - // The other direction: a path dependency outside the subtree that the - // filter does NOT select is a job that compiles a crate and never runs on + it('selects every crate the EQL workspace compiles by path, in every filter', () => { + // The other direction: a path dependency outside the subtree that a + // filter does NOT select is a job that reaches a crate and never runs on // a change to it — the gap commit 7219ea1ad repaired by hand, now derived. - const missing = externalPathDependencies.dirs - .map((dir) => `${dir}/**`) - .filter((glob) => !filterPaths.includes(glob)) - if ( - externalPathDependencies.inherits && - !filterPaths.includes('Cargo.toml') - ) - missing.push('Cargo.toml') + // + // Per filter, because each one gates a job that runs cargo here: + // `relevant:` gates `rust-crates`, which compiles them, and `eql:` gates + // `docs-static`, whose `build` runs `cargo run -p eql-codegen` and so + // loads their manifests to resolve the workspace. + const missing = filters.flatMap(([key, paths]) => { + const globs = externalPathDependencies.dirs + .map((dir) => `${dir}/**`) + .filter((glob) => !paths.includes(glob)) + if (externalPathDependencies.inherits && !paths.includes('Cargo.toml')) + globs.push('Cargo.toml') + return globs.map((glob) => `${key}: ${glob}`) + }) expect( missing, - `The EQL workspace compiles these by path and the relevance filter in ${EQL_WORKFLOW} does not select them, so the \`rust-crates\` job skips on a change that can break it. Add each to the filter — and, because the three copies are held equal, to the \`push: paths:\` list and to bench-eql.yml in the same edit.\n${missing.map((p) => ` ${p}`).join('\n')}`, + `The EQL workspace reaches these by path and a filter in ${EQL_WORKFLOW} does not select them, so the jobs it gates skip on a change that can break them. Add each to that filter — and, for \`relevant:\`, because the three copies are held equal, to the \`push: paths:\` list and to bench-eql.yml in the same edit.\n${missing.map((p) => ` ${p}`).join('\n')}`, ).toEqual([]) }) }) @@ -1127,7 +1142,7 @@ describe('the scan sees every task mise sees', () => { // TOML form. expect(TASKS.get('test:sqlx')?.depends).toEqual(['test:sqlx:prep']) // `#MISE depends=[...]` header-comment form, which only file tasks use. Without - // it `docs:validate:source` — which test-eql.yml runs on every PR — stops + // it `docs:validate:source` — which test-eql.yml runs in `docs-static` — stops // being a caller of `build`. expect(TASKS.get('docs:validate:source')?.depends).toEqual(['build']) }) diff --git a/scripts/__tests__/eql-workflow-filters.test.mjs b/scripts/__tests__/eql-workflow-filters.test.mjs index 5f230772d..3bc67b7c7 100644 --- a/scripts/__tests__/eql-workflow-filters.test.mjs +++ b/scripts/__tests__/eql-workflow-filters.test.mjs @@ -1,8 +1,11 @@ import { execFileSync } from 'node:child_process' -import { readFileSync } from 'node:fs' +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' import { join } from 'node:path' import yaml from 'js-yaml' import { describe, expect, it } from 'vitest' +import { runsWhen } from './lib/expressions.mjs' +import { inheritedToolConfig } from './lib/inherited-tool-config.mjs' import { REPO_ROOT } from './lib/repo-root.mjs' import { readWorkflow } from './lib/workflows.mjs' @@ -72,22 +75,21 @@ function pushPaths(relPath) { } /** - * Every path listed by a `dorny/paths-filter` step, across every filter it - * declares. `filters:` is a YAML document embedded in a YAML string, so it is - * parsed rather than pattern-matched — a regex over the raw block would also - * match the commentary around it. + * One named filter of a `dorny/paths-filter` step. `filters:` is YAML inside a + * YAML string, so it is parsed: a regex would also match the commentary. + * + * One key, not all of them: `relevant:` is the copy of the shared list, while + * `eql:` is a different, deliberately wider question (the whole subtree plus + * parent-directory config, for the cheap jobs) with its own checks below. */ -function dornyFilterPaths(relPath) { +function dornyFilter(relPath, key) { const wf = readWorkflow(relPath) return Object.values(wf?.jobs ?? {}) .flatMap((job) => job?.steps ?? []) .filter((step) => String(step?.uses ?? '').startsWith('dorny/paths-filter@'), ) - .flatMap((step) => - Object.values(yaml.load(step?.with?.filters ?? '') ?? {}), - ) - .flat() + .flatMap((step) => yaml.load(step?.with?.filters ?? '')?.[key] ?? []) } /** The local composite actions a workflow reaches through `uses: ./…`. */ @@ -152,7 +154,10 @@ const TRACKED = trackedUnder('.') /** The list each mechanism carries, keyed for the messages below. */ const LISTS = [ { id: `${TEST_EQL} (on.push.paths)`, entries: pushPaths(TEST_EQL) }, - { id: `${TEST_EQL} (dorny relevant:)`, entries: dornyFilterPaths(TEST_EQL) }, + { + id: `${TEST_EQL} (dorny relevant:)`, + entries: dornyFilter(TEST_EQL, 'relevant'), + }, { id: `${BENCH_EQL} (on.push.paths)`, entries: pushPaths(BENCH_EQL) }, ] @@ -265,8 +270,8 @@ describe('every composite action a workflow runs can trigger it', () => { // and then every one of those paths must be selected by the workflow's // `on: push: paths:` filter. That filter is the one this can be strict about: // it decides whether the workflow STARTS, so it has to cover the inputs of -// every job in the file, relevance-gated or not. The `dorny` copy gates only a -// subset of the jobs, so it is not asserted here — it inherits the entries +// every job in the file, whichever flag gates the job. The `relevant:` copy +// gates only the heavy jobs, so it is not asserted here — it inherits the entries // through the parity check above, which is the looser but correct claim (the // bench inherits them the same way; see the comment on its own filter for why // equal beats minimal). @@ -560,3 +565,158 @@ describe('the EQL push filters cover what the workflows actually read', () => { }) } }) + +// --------------------------------------------------------------------------- +// The cheap jobs' gate. +// +// `test:public_identifiers` greps every tracked file under the subtree and +// `test:doc-anchors` reads every tracked `*.md` there, which is wider than the +// shared list. A gate narrower than that skips the job on a pull request that +// breaks it, and `ci-required` counts the skip as a pass. + +const CHEAP_JOBS = ['docs-static', 'doc-anchors', 'known-failures'] + +const TEST_EQL_JOBS = readWorkflow(TEST_EQL)?.jobs ?? {} + +const cheapJobs = CHEAP_JOBS.map((name) => { + const condition = String(TEST_EQL_JOBS[name]?.if ?? '') + return { + name, + condition, + needs: [TEST_EQL_JOBS[name]?.needs ?? []].flat(), + flag: + /needs\.changes\.outputs\.([A-Za-z0-9_-]+)/.exec(condition)?.[1] ?? null, + } +}) + +/** + * The filter key behind a `changes` output, found by RUNNING the step that + * writes the output as a pull request, with every other env var set to a + * sentinel that names it. Reading the shell instead would agree with a + * rewritten script only by luck. + */ +function filterBehind(flag) { + const steps = TEST_EQL_JOBS.changes?.steps ?? [] + const writerId = /steps\.([A-Za-z0-9_-]+)\.outputs\./.exec( + String(TEST_EQL_JOBS.changes?.outputs?.[flag] ?? ''), + )?.[1] + const writer = steps.find((step) => step?.id === writerId) + if (!writer) return null + + const scratch = mkdtempSync(join(tmpdir(), 'eql-workflow-filters-')) + try { + const outputFile = join(scratch, 'output') + writeFileSync(outputFile, '') + const env = { PATH: process.env.PATH, GITHUB_OUTPUT: outputFile } + for (const [key, value] of Object.entries(writer.env ?? {})) { + env[key] = /^\$\{\{\s*github\.event_name\s*\}\}$/.test(String(value)) + ? 'pull_request' + : `sentinel:${key}` + } + execFileSync('bash', ['-c', String(writer.run)], { cwd: scratch, env }) + + const line = readFileSync(outputFile, 'utf8') + .split('\n') + .find((entry) => entry.startsWith(`${flag}=`)) + const envKey = /^sentinel:(.+)$/.exec(line?.slice(flag.length + 1))?.[1] + const source = /steps\.([A-Za-z0-9_-]+)\.outputs\.([A-Za-z0-9_-]+)/.exec( + String(writer.env?.[envKey] ?? ''), + ) + if (!source) return null + const filterStep = steps.find((step) => step?.id === source[1]) + const entries = yaml.load(filterStep?.with?.filters ?? '')?.[source[2]] + return Array.isArray(entries) ? { key: source[2], entries } : null + } finally { + rmSync(scratch, { recursive: true, force: true }) + } +} + +describe('the cheap EQL jobs are gated on a filter covering what they read', () => { + const flags = [...new Set(cheapJobs.map((job) => job.flag))] + const gate = flags.length === 1 && flags[0] ? filterBehind(flags[0]) : null + const selects = (file) => + (gate?.entries ?? []).some((entry) => matches(entry, file)) + + it('gates every cheap job on one `changes` output', () => { + for (const { name, needs, flag } of cheapJobs) { + expect( + needs, + `${TEST_EQL} job "${name}" must \`needs: [changes]\` to read the flag that gates it.`, + ).toContain('changes') + expect( + flag, + `${TEST_EQL} job "${name}" reads no \`needs.changes.outputs.\` in its \`if:\`, so it runs on every pull request — including one that touches nothing in EQL.`, + ).toBeTruthy() + } + expect( + flags, + 'The cheap jobs read different flags. They have the same inputs, so one gate is enough, and the checks below follow only one.', + ).toHaveLength(1) + }) + + it('skips a cheap job only on a pull request whose flag is false', () => { + // Evaluated rather than pattern-matched. Off a pull request the flag is a + // default, so the condition alone must open there. + const context = (eventName, value) => ({ + github: { event_name: eventName }, + needs: { changes: { outputs: { [flags[0]]: value } } }, + vars: {}, + }) + for (const { name, condition } of cheapJobs) { + const verdicts = { + 'pull_request, flag true': runsWhen( + condition, + context('pull_request', 'true'), + ), + 'pull_request, flag false': runsWhen( + condition, + context('pull_request', 'false'), + ), + ...Object.fromEntries( + ['push', 'schedule', 'merge_group', 'workflow_dispatch'].map( + (event) => [ + `${event}, flag unset`, + runsWhen(condition, context(event, '')), + ], + ), + ), + } + expect(verdicts, `${name}: if: ${condition}`).toEqual({ + 'pull_request, flag true': true, + 'pull_request, flag false': false, + 'push, flag unset': true, + 'schedule, flag unset': true, + 'merge_group, flag unset': true, + 'workflow_dispatch, flag unset': true, + }) + } + }) + + it('follows that output to the filter that computes it', () => { + expect( + gate, + `\`jobs.changes.outputs.${flags[0]}\` in ${TEST_EQL} does not lead, through the step that writes it, to a key of the \`dorny/paths-filter\` step. Either the wiring changed shape or the output is no longer written from a filter.`, + ).toBeTruthy() + }) + + it('selects every tracked file under the subtree', () => { + const missed = TRACKED_EQL.filter((file) => !selects(file)) + expect( + missed, + `The \`${gate?.key}:\` filter in ${TEST_EQL} gates the cheap jobs, and these tracked files are outside it. \`test:public_identifiers\` greps every tracked file under \`${EQL}/\`, so a pull request touching only one of them skips the job that checks it. \`${EQL}/**\` covers the lot.`, + ).toEqual([]) + }) + + it('selects the workflow and the config mise and cargo read from parent directories', () => { + const inherited = inheritedToolConfig(EQL) + expect( + inherited, + `inheritedToolConfig("${EQL}") no longer finds the root mise.toml, so the check below is vacuous for it.`, + ).toContain('mise.toml') + const missed = [TEST_EQL, ...inherited].filter((file) => !selects(file)) + expect( + missed, + `The \`${gate?.key}:\` filter in ${TEST_EQL} does not select these. mise and cargo read config from every parent directory of \`${EQL}\`, so the cheap jobs install the root's tools, load the root's tasks and build with the root's cargo config — a change here can break them. Add each path to the filter.`, + ).toEqual([]) + }) +}) diff --git a/scripts/__tests__/lib/inherited-tool-config.mjs b/scripts/__tests__/lib/inherited-tool-config.mjs new file mode 100644 index 000000000..21ee79b27 --- /dev/null +++ b/scripts/__tests__/lib/inherited-tool-config.mjs @@ -0,0 +1,65 @@ +import { execFileSync } from 'node:child_process' +import { readFileSync } from 'node:fs' +import { join } from 'node:path' +import { REPO_ROOT } from './repo-root.mjs' + +/** + * Tracked config files that mise and cargo read from the PARENT directories of + * `dir`. Both tools search upward, so a job running them in `packages/eql` + * reads the root `mise.toml` (its tools and its tasks, including every file + * its `[task_config].includes` names) and the root `.cargo/config.toml`. + * + * Only `mise.toml` and `.cargo/config{,.toml}` are modelled: they are the + * names this repository tracks. `mise.test.toml` loads only under + * `MISE_ENV=test`, which no EQL job sets. A parent config of another name is + * missed here, and the subtree-prefix check in `eql-suite-ci.test.mjs` + * reports it as an offender the day a filter names it. + */ +export function inheritedToolConfig(dir) { + const found = new Set() + for (const parent of parentDirs(dir)) { + const mise = join(parent, 'mise.toml') + if (isTracked(mise)) { + found.add(mise) + for (const include of miseTaskIncludes(mise)) { + for (const file of trackedAt(join(parent, include))) found.add(file) + } + } + for (const cargo of ['.cargo/config.toml', '.cargo/config']) { + const path = join(parent, cargo) + if (isTracked(path)) found.add(path) + } + } + return [...found].sort() +} + +/** `packages/eql` -> `['packages', '.']`, nearest first. */ +function parentDirs(dir) { + const segments = dir.split('/').filter(Boolean) + return segments.map((_, i) => + i === segments.length - 1 ? '.' : segments.slice(0, -1 - i).join('/'), + ) +} + +function trackedAt(path) { + return execFileSync('git', ['ls-files', '-z', '--', path], { + cwd: REPO_ROOT, + encoding: 'utf8', + }) + .split('\0') + .filter(Boolean) +} + +const isTracked = (path) => trackedAt(path).includes(path) + +/** Read by hand: the repository has no TOML reader. */ +function miseTaskIncludes(relPath) { + const text = readFileSync(join(REPO_ROOT, relPath), 'utf8') + const table = /^\[task_config\]\s*\n([\s\S]*?)(?=^\[|(?![\s\S]))/m.exec( + text, + )?.[1] + const list = /^\s*includes\s*=\s*\[([\s\S]*?)\]/m.exec(table ?? '')?.[1] + return [...(list ?? '').matchAll(/"([^"]+)"|'([^']+)'/g)].map( + (match) => match[1] ?? match[2], + ) +} diff --git a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs index 59172aa0a..81754f549 100644 --- a/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs +++ b/scripts/__tests__/workflow-dispatch-job-conditions.test.mjs @@ -178,7 +178,7 @@ function extractForkClause(condition) { * run. */ const PERMISSIVE_NEEDS = { - changes: { outputs: { relevant: 'true' } }, + changes: { outputs: { relevant: 'true', eql: 'true' } }, // The release workflows' gates, none of which is about how the run was // triggered. Held open, or "skips because there is nothing to publish" would // be indistinguishable from "skips on a dispatch". diff --git a/scripts/__tests__/workflow-paths-filter-parity.test.mjs b/scripts/__tests__/workflow-paths-filter-parity.test.mjs index 5e719237c..640b28c97 100644 --- a/scripts/__tests__/workflow-paths-filter-parity.test.mjs +++ b/scripts/__tests__/workflow-paths-filter-parity.test.mjs @@ -161,7 +161,7 @@ const EXPECTED_ASYMMETRIES = new Map([ // not unchecked: `eql-workflow-filters.test.mjs` compares it against the // `relevant:` filter in the `changes` job, which is the second copy of the // same list and the one this file cannot see. - 'pull_request is deliberately unfiltered (per-job relevance instead), so PRs run a superset of what push runs; the push list is compared against the `changes` job filter by eql-workflow-filters.test.mjs', + 'pull_request is deliberately unfiltered (per-job path gates instead), so every PR starts the workflow; the push list is compared against the `changes` job `relevant:` filter by eql-workflow-filters.test.mjs', ], [ '.github/workflows/bench-eql.yml',