Repository navigation
Conversation
The selector job decides which checks run on a pull request. A job that does not read its answer runs on every change. That costs money, and nothing shows it. The failsafe reads every workflow that runs on pull_request or merge_group. It fails when the selector job does not decide whether a job runs, unless the job has a written reason to run on every change. An exemption that matches nothing also fails, so the lists stay true. Only test-eql.yml has a selector job so far. Every other workflow starts as an exemption, and each conversion removes its entry. The two review bot workflows are exempt for good: they are not checks, and no branch rule requires them.
|
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
impact.yml posts an advisory report on every pull request. It is not a check, and no branch rule requires it, so it will not get a selector job. It moves from the temporary "not yet converted" list to the permanent one, next to the two review bots.
The per-job list was called RUNS_ON_EVERY_CHANGE. Some jobs the selector job does not decide never run on a pull request at all, such as a fuzz campaign that runs only on a schedule. EXEMPT_JOBS fits both kinds. Each entry still needs its reason.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This adds the failsafe. It is a test that fails when a workflow's selector job does not decide whether a job runs. The selector job is the first job of a workflow. It compares the files a pull request changes with each check's trigger paths, and each job's
if:reads its answer. A job that does not read the answer runs on every change. That costs CI money, and nothing showed it until now.ADR 0003 in #1416 describes the design.
.github/CONTEXT.mdon that branch defines "failsafe", "selector job" and "summary job".Changes
scripts/__tests__/failsafe.test.mjs: the new test.pnpm run test:scriptsruns it, and thelintjob intests.ymlruns that on every pull request.pull_requestormerge_group. In each one, a job passes when:selector) or the summary job (the last job, withif: always()and every other job in itsneeds:);if:reads an output the selector job declares;if:calls noalways(),failure()orcancelled(), so GitHub skips it whenever that job skips; orEXEMPT_JOBS.NOT_CHECKS:claude-review.yml,code-review.ymlandimpact.yml, for good. The first two are review bots and the third is an advisory report. None is a check, and no branch rule requires them.NOT_YET_CONVERTED: the 24 workflows with no selector job yet. Each conversion removes its entry, so this list shrinks to empty.EXEMPT_JOBS: jobs the selector job does not decide, on purpose. Today these are four jobs intest-eql.ymlthat run on every change:setup,docs-static,known-failuresanddoc-anchors. A job that never runs on a pull request, such as a scheduled fuzz campaign, goes here too.Verification
pnpm run test:scripts: 82 files passed, 1434 tests passed, 3 skipped.pnpm exec biome checkon the new file: no findings.EXEMPT_JOBS;test-eql.yml#schema);test-eql.ymladded toNOT_YET_CONVERTED;NOT_YET_CONVERTEDentry for a workflow that does not exist;NOT_YET_CONVERTED;if:removed fromtest-eql.yml#schema;if:readingneeds.selector.outputs.eqll, an output the selector does not declare;needs:orif:;schemabut hasif: ${{ !cancelled() }}.schemaand has noif:, with the job added to the summary job'sneeds:.NOT_YET_CONVERTED. After removing those entries and adding the two job exemptions those pull requests describe (tests-auth-refresh.yml#shortandfuzz.yml#fuzz-campaign), it passed for all six.Related
test-eql.yml. Merge that first.NOT_YET_CONVERTEDentry.Review notes
test-eql.yml, a gated job runs on every event other thanpull_request, includingmerge_group. ADR 0003 says the selector should decide in the merge queue too. When it does, this test can checkmerge_groupas well.scripts/__tests__/lib/selector.mjs. Once both land, the failsafe could read selector outputs throughselectorOutputsReadfrom there. I kept its helper names out of this file, so the guard inshared-test-helpers.test.mjswill not fail when they meet.needs:, makes the failsafe report the summary job as undecided. The failure message says how to fix that.