Repository navigation
ci: run on push to main only, so each required check has one producer - #26
Merged
Merged
Conversation
check.yml triggered on a bare push as well as pull_request, so every push to a PR branch started two check suites. Each required context, `check` and the called workflow's `kind / conformance`, then had two producers on the PR head, and the merge gate resolved each name to whichever run it found. This is the defect kindspec/rowspec#67 fixed in rowspec; the trigger now matches it. kind.yml is not changed: it runs only through workflow_call, so its caller's triggers are its triggers. Signed-off-by: Cameron Brooks <cambrooks3393@gmail.com>
CameronBrooks11
added a commit
to kindspec/.github
that referenced
this pull request
Oct 8, 2026
`scripts/check_required_checks.py` refused every job that calls a reusable workflow. kindkit's and rowspec's `kind` jobs call kindkit's `kind.yml`, so the audit exited 2 on the real org with four INCOMPLETE lines, and the daily run in #12 would be red on every run. This teaches the script to expand those calls. ## What it does now A job with `uses: ./.github/workflows/F.yml` or `uses: OWNER/REPO/.github/workflows/F.yml@REF` produces one context per called job, `<caller job> / <called job>`, each named by its `name:` or else its id. That is the naming the existing required contexts already use (`kind / conformance`). - **Gating comes from the caller.** The caller workflow's `pull_request` trigger, branch filter, path filter and `types:` apply to every called job. The caller's `if:` and `needs:` chain make every called job CONDITIONAL; a skipped caller is reported under its own name only, so its `<caller> / <called>` contexts never appear and a required one holds the merge at *Expected* (the finding says so, rather than "reports success"). Each called job's own `if:` and `needs:` (within the called workflow) also make it CONDITIONAL, and a skipped called job does report as success. - **A job that `needs:` a caller job** is skippable unless every called context is required and none of the called jobs is itself skippable. - **Where the called file comes from.** A local call is read from the same default-branch snapshot as its caller. A call into another repository is fetched with the contents API at exactly `REF` (`?ref=`, URL-encoded) and recorded in the snapshot under a new per-repository `"called"` key, as text or as `{"unreadable": why}`. `--load` therefore reproduces the run offline. - **Still refused (exit 2):** a call into another repository that cannot be fetched at its ref, or that a snapshot does not hold (which includes every snapshot saved before this change; a local call still expands from an old snapshot, because it reads the workflow files the snapshot already holds); a local call whose file is not among the default branch's workflows; any other `uses:` form; a job with `uses:` together with `runs-on:` or `steps:`, which GitHub rejects; a called workflow with no `workflow_call` trigger or that the parser refuses; and a called job that itself calls a workflow (no recursion), or is matrix-, expression- or block-scalar-named. A caller job with a matrix or an expression name stays refused, as before. - `pull_request jobs` in the summary line now counts each called job, not the caller. ## Live, on the real org $ python3 -I scripts/check_required_checks.py --public-only # this branch 1 private repositories skipped (--public-only) 6 repositories, 6 workflows, 7 pull_request jobs, 7 required checks: 0 finding(s), 0 not evaluated exit 0 $ python3 -I scripts/check_required_checks.py --public-only # main, cb976f4 INCOMPLETE kindkit: .github/workflows/check.yml: job 'kind': not evaluated: calls a reusable workflow; contexts are '<caller> / <called job>' INCOMPLETE kindkit: required 'kind / conformance' not matched, and a job above was not read INCOMPLETE rowspec: .github/workflows/check.yml: job 'kind': not evaluated: calls a reusable workflow; contexts are '<caller> / <called job>' INCOMPLETE rowspec: required 'kind / conformance' not matched, and a job above was not read 1 private repositories skipped (--public-only) 6 repositories, 6 workflows, 7 pull_request jobs, 7 required checks: 0 finding(s), 4 not evaluated exit 2 The saved snapshot (`--save`) holds one called workflow, `kindspec/kindkit/.github/workflows/kind.yml@8e21b558c85d5612009f3a1d69a3a68bc0191637` under `rowspec`; kindkit's own call is local. `--load` of it gives the same line and exit 0. **The expanded contexts are evaluated, not waved through.** Edited copies of that snapshot: kindkit's 'kind / conformance' removed from required: UNGATED kindkit: .github/workflows/check.yml: job 'kind' (called job 'conformance') runs on pull requests; 'kind / conformance' is not a required check exit 1 rowspec's called kind.yml with the job renamed to `conform`: UNGATED rowspec: .github/workflows/check.yml: job 'kind' (called job 'conformance') runs on pull requests; 'kind / conform' is not a required check ORPHANED rowspec: required check 'kind / conformance' is produced by no job that reports on a pull request into 'main' exit 1 rowspec's "called" emptied: INCOMPLETE rowspec: .github/workflows/check.yml: job 'kind': not evaluated: calls kindspec/kindkit@8e21b55..., which the snapshot does not hold INCOMPLETE rowspec: required 'kind / conformance' not matched, and a job above was not read exit 2 ## Red first (first head, c5c303a) 26 self-test cases were added (23 snapshot, 3 collection through the fake API) before the implementation. `live()` now round-trips each collected snapshot through JSON, so a collection case also checks what `--save` writes is what `--load` reads. The fake API answers a key containing `?` only for that exact path, which is how the collection case checks that the ref was used. Against main's logic with the new cases: `selftest: 73 of 85 as expected`. The 12 failures are every case that expects a called workflow to be read: the local and remote expansions, the caller/called naming, UNGATED, ORPHANED and CONDITIONAL (caller `if:`, called `if:`, called `needs:`), the three `needs:` a caller cases, and the fetch at the pinned ref. The 14 new refusal cases already pass on main, because main refuses every call; what pins them is the sweep below. At this head: `selftest: 85 of 85 as expected`. ## Mutation sweep (first head) `python3 -I scripts/sweep_check_required_checks.py` (it hashes the target, prints BROKEN for a pattern that does not match exactly once or a no-op mutation, and asserts the target's sha256 unchanged afterwards). 18 mutants were added for the new code, and one (`reusable treated as plain`) was retargeted. Three existing mutants first came out **BROKEN** because their patterns moved (`conditional never reported`, `unrequired need ignored`, `jobs not a mapping accepted`, the last now matching twice); they were retargeted to the same checks. Output at this head: caught reusable treated as plain caught local call not read caught caller name not prefixed caught called job if ignored caught called job needs ignored caught caller conditions ignored for called jobs caught called job conditions ignored caught nested call accepted caught unread called job accepted caught no workflow_call accepted caught unreadable call accepted caught call missing from snapshot accepted caught unrecognised uses accepted caught unparseable call accepted caught call fetched without its ref caught calls not collected caught needed caller's called contexts ignored caught needed caller's called conditions ignored 66 mutants: 66 caught, 0 crashed the self-test, 0 survived, 0 broken (The other 48 lines are `caught`.) sha256 of `scripts/check_required_checks.py` before and after the sweep: `2c4e9a42c2a0395b5b5ff00387570f2ed7a52198f5017af75b45b2ba88334cbd` both times. The claim is exactly that each named mutation makes `--selftest` report a FAIL; the refusal for a missing local called file has a case but no mutant of its own. `ruff format --check` and `ruff check` (line length 100, the ruff kindkit locks) are clean on `scripts/`. ## After review (3bdbcfc, on top of a merge of main at 319a944) - **Refused again: `uses:` with `runs-on:` or `steps:`.** The first head let it through as a clean caller (main refused it). The two new cases, against the first head's logic: `selftest: 89 of 91 as expected`, both failing with `got [] exit 0`. - **Pinned what was right but unchecked.** New cases: two callers of one file at different refs, the second ref renaming the called job (UNGATED + ORPHANED); a path-filtered caller (PATH-FILTER); a caller on `types: [labeled]` (TRIGGER); and the summary's pull_request job count (3 for a plain job plus a caller of two jobs). The review's mutants for these all **survived** on the first head's self-test (its sha256 `2c4e9a42…` unchanged before and after): SURVIVED R1 called lookup ignores the ref SURVIVED R2 path filter not applied to called jobs SURVIVED R3 types not applied to called jobs SURVIVED R12 summary counts callers, not called jobs and are **caught** at this head. They are now in the committed sweep as `call looked up without its ref`, `path filter ignored for called jobs`, `types ignored for called jobs` and `called jobs not counted`, with `uses with runs-on or steps accepted` for the new refusal. `called job conditions ignored` was retargeted after its line changed (it first came out BROKEN). - **CONDITIONAL wording** for a skipped caller, as described above. The finding kind is unchanged, and no case pins the wording. At this head: `selftest: 91 of 91 as expected`. Sweep, with the target's sha256 `fe662a80eb1b6ebac5e9b753f5ecc571ba04f2f1c4c8d96e48039eba35c5df09` before and after: 71 mutants: 71 caught, 0 crashed the self-test, 0 survived, 0 broken Live, at this head: `6 repositories, 6 workflows, 7 pull_request jobs, 7 required checks: 0 finding(s), 0 not evaluated`, exit 0. Three of the review's mutants still survive and are not in the sweep: not URL-encoding the ref (no test ref contains a reserved character), accepting a fetched non-file (the fake API never returns a directory listing for a file path), and reading an unreadable call as empty text (an empty workflow is refused anyway, for its missing `on`). ## Not covered: two producers for one required check kindkit's `check.yml` runs on a bare `push` as well as `pull_request`, so on a PR head both `check` and `kind / conformance` come from two suites (kindspec/kindkit#26 fixes that, as kindspec/rowspec#67 did for rowspec). This audit does not flag it, before or after this change: only the `pull_request` trigger is read, and a `push` trigger is ignored. With this change the expanded contexts would be visible to such a rule, but no rule exists. It would be a new finding class (a workflow producing a required context on `pull_request` whose `push` trigger also admits non-default branches), and is not added here. Refs #3. Refs #12, which goes green on its live run once this merges. --------- Signed-off-by: Cameron Brooks <cambrooks3393@gmail.com>
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
1 participant
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.
What
check.ymlnow triggers oninstead of
on: [push, pull_request]. This is the change kindspec/rowspec#67 made to rowspec'scheck.yml. No job is renamed and no path filter is added, so the ruleset (check,kind / conformance) needs no change.kind.ymlis not changed: it runs only throughworkflow_call, so its caller's triggers are its triggers, and thekindcaller job lives incheck.yml.Why
With a bare
push, every push to a PR branch starts twochecksuites. Each required context then has two producers on the PR head. On #25's head (79d194a), fromgh api repos/kindspec/kindkit/commits/<sha>/check-runs --jq '.check_runs[] | "\(.name)\t\(.check_suite.id)"':The caller job has the same defect as
check, because both are jobs of the same workflow; fixing the trigger fixes both.Testing
{'push': {'branches': ['main']}, 'pull_request': None}under the required-checks audit's YAML reader, and itspr_triggergives a pull_request producer intomain.just check && just test:All checks passed!,220 passed.c3e3c7f), same command: each required context once, from thepull_requestsuite. The branch was pushed before the PR was opened, and that push started no run.mainshould get apushrun on the merge commit.Refs kindspec/rowspec#43