feat(workflows): add the checks reusable and dogfood it - #551
Merged
Conversation
One job, one runner spin-up: `checks.yml` runs `change-detection` once and then every content-agnostic hygiene composite as a step, each behind a boolean toggle and the change-detection filter group named after that toggle, each under `continue-on-error: true`, joined by one outcome step that names the first failure. Outputs `results` (the detection JSON, verbatim) and `outcome`. Composites stay the unit of reuse: the reusable never calls a per-tool reusable workflow. They are referenced by full path at a pinned SHA rather than `./`, because inside a CALLED workflow a relative action path resolves against the CALLER's checkout (actions/runner#1348) and would fail in every consumer. `zizmor` is absent: it has no composite, only the reusable that needs `security-events: write`. `check-jsonschema` is the one toggle that defaults off; its `files` input is required and one call carries one schema family. ci.yml dogfoods it: `checks` replaces the `typos`, `gitleaks`, `markdown`, `links` and `hygiene` jobs and the dependabot.yml schema step, and `ci-status` aggregates it in their place. `actionlint` and `shellcheck` keep their own jobs with those toggles off, since both carry extra dogfood steps that need the composite's pinned binary on PATH. Refs melodic-software/github-iac#378 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 4m 13s —— View job Code review: PR #551No correctness bugs found. This is a careful, well-tested consolidation — traced the fan-out replacement end to end and everything checks out. What I verified:
One behavioral note (not a bug): the comment-hygiene prefilter superset test moved from the always-run Not independently verified: this review environment's Bash tool required interactive approval for |
Security review in progress
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c664132c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 2m 10s —— View job Security review complete
I reviewed this PR under the No security issues found. What I checked specifically:
Branch · |
|
Claude has reviewed this PR 2 times. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
|
Last security-reviewed head: |
An enabled `check-jsonschema` with an empty or misspelled `check-jsonschema-files` skipped the validation step and the join reported success, so a schema gate the caller believed was on could be silently off — the failure mode this lane exists to prevent. A `continue-on-error` guard step now fires on that combination and the join names it `configuration`, so the job goes red and says why. Raised by the Codex reviewer on this pull request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 56s —— View job Security review complete
Skip gate applied: the last successful security review on this PR covered head No security issues found in that commit. Specifically checked:
This finding (the Codex P2 report) was already resolved by this commit before this review ran, so there is nothing further to flag. |
The input description and the step comment still described the pre-guard behaviour, which is the one shape a reader must not be told twice about. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Claude finished @kyle-sexton's task in 54s —— View job Security review complete
Skip gate applied: the last security review on this PR covered head No security issues found. That commit is comment/description-text only:
I verified the |
checks.yml references its composites by full path at a pinned SHA, because a relative action path inside a called workflow resolves against the caller's checkout. The dogfood call in ci.yml therefore ran nine composite bodies at 449157a rather than at the commit under test, so a pull request that broke one of their run.sh bodies could pass this repository's own CI. That is deviation 9 of the Phase 6a report. The new composites-head job runs typos, gitleaks, editorconfig, markdown, exec-bit, machine-specific-paths, eol-renormalize, comment-hygiene and lychee-offline through ./.github/actions/<x>, with the same exclusions the checks caller passes. actionlint, shellcheck and check-jsonschema are absent because each already runs at HEAD in its own job. The job copies its sibling lanes: ubuntu-24.04, needs changes, a fifteen minute budget, the contract-only gate, and the pinned checkout with persist-credentials false. It gates on the actionlint change-detection group, the narrowest group naming composite actions; every group in that job includes .github/** so any composite edit selects the lane. ci-status gains the lane in both its needs list and its aggregated results. GitHub's $/ self-repository syntax would remove the pin lag and this job together, but actionlint 1.7.12, which this repository's own actionlint composite pins, rejects it as an invalid format with a missing ref. Phase 6b promotes it once actionlint ships the support of rhysd/actionlint#732 in a version this repository pins, actions/runner#4669 merges, and one cross-repository $/ run is measured. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019APkoJh8NVN773d82fBBSG
|
Claude finished @kyle-sexton's task in 1m 37s —— View job Security review complete
Skip gate applied: the last security review on this PR covered head No security issues found. Specifically checked:
This is a docs/CI-shape commit with no functional security delta from the already-reviewed baseline. Branch · |
kyle-sexton
added a commit
that referenced
this pull request
Sep 8, 2026
No related issue: runner-host download stall consumed the full checks budget on 2026-09-08 (dotfiles run 34190855785) ## Summary On 2026-09-08 the self-hosted runner host's network path collapsed to roughly 12 KB/s for about fifteen minutes. `dotfiles`' `checks / checks` job — which calls this repository's `checks.yml` reusable at `v0.22.2` with `timeout-minutes: 15` — stalled inside the `check-jsonschema` composite, on `astral-sh/setup-uv`'s download. No step had its own budget, so the stall consumed the whole job budget and the run ended **`cancelled`**. `cancelled` is the worst possible verdict for this lane. The `outcome` join is gated on `!cancelled()`, so it never ran and named nothing; a reusable-workflow job that does not complete publishes no outputs; and the caller's `ci-status` aggregate read the lane as a failure with no cause attached. Three reruns were needed to get green. A healthy run of that job takes 1 to 4.5 minutes. ## Fix Every step in the `checks` job that downloads something now carries `timeout-minutes: 8` — comfortably above the 4.5-minute healthy ceiling for the whole job, well under the 15-minute default job budget: | Step | What it downloads | | --- | --- | | `typos` | `crate-ci/typos` release tarball behind `actions/cache` | | `gitleaks` | `gitleaks/gitleaks` release tarball behind `actions/cache` | | `editorconfig` | `editorconfig-checker` release tarball behind `actions/cache` | | `markdown` | `actions/setup-node`, then `markdownlint-cli2` | | `shellcheck` | `koalaman/shellcheck` release tarball behind `actions/cache` | | `actionlint` | `rhysd/actionlint` **and** `koalaman/shellcheck` tarballs | | `lychee_offline` | `lycheeverse/lychee` release tarball behind `actions/cache` | | `check_jsonschema` | `astral-sh/setup-uv`, then `uvx check-jsonschema` — the step that stalled | Untouched, because they download nothing: `configuration`, `detect` (`actions/github-script`, API only), `exec_bit`, `machine_specific_paths`, `eol_renormalize`, `comment_hygiene`, and the `outcome` join. No composite action file is modified. ### Why the key goes on the workflow step and not inside the composite A composite action's `runs.steps[*]` accepts `run`, `shell`, `if`, `name`, `id`, `env`, `working-directory`, `uses`, `with` and `continue-on-error` — and no `timeout-minutes` ([Metadata syntax for GitHub Actions](https://docs.github.com/en/actions/reference/workflows-and-actions/metadata-syntax)). The feature request to add it is [actions/runner#1979](actions/runner#1979), still open. The workflow step's own `timeout-minutes` *is* honoured for the whole composite invocation: [actions/runner#599](actions/runner#599) (merged 2020-07-22) wired the step's `timeout-minutes` into the cancellation token of the composite action step's execution context. ### Why this restores the signal rather than just shortening the stall A step killed by its own `timeout-minutes` is marked **`Failed`**, not `Canceled` — [`StepsRunner.cs`](https://github.com/actions/runner/blob/main/src/Runner.Worker/StepsRunner.cs) distinguishes the two by whether the *job* token also fired, and sets `TaskResult.Failed` for the step-timeout branch. So `continue-on-error: true` still holds, `steps.<id>.outcome` is `failure`, the `outcome` join runs and emits `::error::check-jsonschema failed (outcome=failure).`, and the caller's aggregate gets a named cause instead of a bare `cancelled`. ### Known ceiling This bounds **one** stalled step. In the 2026-09-08 incident the host's whole network path was degraded, so several downloads would have stalled in turn: the first fails at 8:00, the next stalls, and the job still hits its 15-minute budget and ends `cancelled`. Narrowing the per-step budget, or deriving it from `inputs.timeout-minutes`, is a separate change and is deliberately not made here. ## Verification Run from the worktree `D:/worktrees/ciw-checks-step-timeout` on `ci/checks-step-timeouts`: - `actionlint --version` → `1.7.12` — the exact version `.github/actions/actionlint/action.yml` pins as its default. - `actionlint .github/workflows/checks.yml` → no output, **exit 0**. actionlint parses the file as YAML before linting it, so this covers the parse too. - `git diff --stat` → `.github/workflows/checks.yml | 18 ++++++++++++++++++`, 1 file changed, 18 insertions, 0 deletions. - `git diff` → exactly eight `+ timeout-minutes: 8` lines and a ten-line `# Stalls:` paragraph appended to the file header. No other key, value, pin, or SHA changed. No file outside `.github/workflows/checks.yml` is touched. Change-contract check: this repository has no `CHANGELOG.md` (`grep -rli changelog --include='*.md' .` returns nothing) and no version file — `README.md` §Versioning states the version lives only in the git tag, cut by `release.yml`'s manual `workflow_dispatch`. No bump or changelog entry is owed for this PR. This PR's own CI executes the modified file: `ci.yml` dogfoods it through `uses: ./.github/workflows/checks.yml` at the commit under test, so the runner accepts the new keys on a real run. Not exercised: the timeout firing. Reproducing a 12 KB/s network path on a runner is out of reach here, so that half rests on the two runner citations above. ## Related - Follows the `checks` reusable added in #551 and the pin convergence in #558. - Consumer affected: `melodic-software/dotfiles` `.github/workflows/ci.yml`, which pins `checks.yml@5776760 # v0.22.2`. It picks this up only after a release is cut and the pin is advanced, and neither hop is automatic: `release.yml` is `workflow_dispatch`-only, and dotfiles' `dependabot.yml` `ignore`s `melodic-software/ci-workflows/*` so that every ci-workflows bump moves through a deliberate reviewed PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01L5ZJnwTRCgc31bmfrdugyU --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.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.
No related issue: melodic-software/github-iac#378 tracks the ci-perf program (Phase 6a)
Summary
checks.ymlis the consolidated hygiene lane the ci-perf program's Phase 6aspecifies, and the one the private repositories adopt in Phase 4: one job, one
runner spin-up. It runs
change-detectiononce against caller-suppliedfilters, then each content-agnostic composite as a step, and publishesresults(the detection JSON, verbatim) andoutcome.typos,gitleaks,editorconfig,markdown,shellcheck,actionlint,exec-bit,machine-specific-paths,eol-renormalize,comment-hygiene,lychee-offline,check-jsonschema),all default
trueexceptcheck-jsonschema.toggle and that group evaluated
false. An undeclared group leaves thecomposite ungated: fail-open is the detection contract.
continue-on-error: true; one join step reportseach outcome, names the first failure, and fails the job. Run-everything,
fail-at-end — one failing tool never hides the rest.
permissionsarecontents: readpluspull-requests: read(the detectionpass reads the pull request's file listing). A called workflow cannot elevate,
so the caller's job block grants both.
runneris required with no default: a hosted default would silently bill aprivate caller's pool, which is the placement accident this consolidation
exists to end.
timeout-minutesdefaults to15, the per-tool reusable value.Composites stay the unit of reuse — the reusable never calls a per-tool reusable
workflow, which is another job and another spin-up.
Two composites of the plan's thirteen are not toggles:
zizmorhas no composite, only.github/workflows/zizmor.yml, which needssecurity-events: writeand an optional SARIF upload. Callers keep a separatejob (this repository does).
check-jsonschemadefaults off: itsfilesinput is required with nouniversal default, and one call validates one schema family.
A
composites-headjob keeps the moved composites covered at HEAD. Becausechecks.ymlcan only reach its composites at a pinned SHA (below), thisrepository's dogfood ran nine composite bodies at
449157aaa8e30f7b1457305d8048ebe6168e174a, not at the commit under test, so apull request breaking one of their
run.shbodies could have gone green here.The new job (display name "Composites at HEAD") runs
typos,gitleaks,editorconfig,markdown,exec-bit,machine-specific-paths,eol-renormalize,comment-hygieneandlychee-offlinethrough./.github/actions/<x>with the same exclusions thecheckscaller passes.actionlint,shellcheckandcheck-jsonschemaare absent because eachalready runs at HEAD in its own job.
ci-statusaggregates the lane.GitHub's
$/self-repository syntax is deferred to Phase 6b. It wouldresolve against the workflow's own repository and remove both the pin lag and
the extra job, but actionlint 1.7.12, the version this repository's own
actionlintcomposite pins, rejects it:specifying action "$/..." in invalid format because ref is missing. Promotion needs three things: actionlintshipping the
$/support of rhysd/actionlint#732 in a version this repositorypins, actions/runner#4669 (composite manifests) merging, and one measured
cross-repository
$/run.Fix
The composites are referenced by full path at a pinned SHA, never
./.github/actions/.... Inside a called workflow a relative action pathresolves against the caller's checkout (actions/runner#1348),
so a relative reference would fail in every consumer. The pins therefore lag one
release behind the workflow carrying them; Dependabot's existing
github-actionsgroup bumps them like any otheruses:. Thecomposites-headjob is the counterweight to that lag inside this repository, and the
checks.ymlheader records both the reason it exists and the Phase 6b promotion trigger.
Dogfood in this repository's own
ci.yml: thechecksjob replaces thetypos,gitleaks,markdown,linksandhygienejobs and theValidate dependabot.ymlstep, andci-statusaggregatesneeds.checks.resultin theirplace.
actionlintandshellcheckkeep their own jobs with those toggles off —each carries extra dogfood steps that run against the pinned binary the composite
installs, which no
uses:in a shared reusable can hand back. Thecomment-hygieneprefilter superset test moves toselector-contract, since ashared reusable cannot run a repo-local script.
changesstays for the languageand toolchain lanes this repository fans out and Phase 6b converges; the
checkslane runs its own detection pass over the groups its composites are gated on.
composites-headcopies its sibling lanes (ubuntu-24.04,needs: changes,timeout-minutes: 15, the contract-only gate, the pinned checkout withpersist-credentials: false) and gates on theactionlintchange-detectiongroup, the narrowest group naming composite actions; every group in
changesincludes
.github/**, so any composite edit selects the lane..github/scripts/checks-outcome.test.cjsextracts the join'srun:block withworkflow-yaml.cjsand executes it under fixture outcomes: all green, allskipped, one failure, two failures (names the first, counts the rest), and a step
that never ran. It also proves structurally that every
continue-on-errorstepreaches the join and references its composite by pinned full path, so a composite
added without a join entry fails the build instead of riding to silent green.
ci-fanout-consolidation.test.cjsis updated where the job set legitimatelychanged: it now locks the
checkscaller's shape, its two permission grants, andthe absence of the five replaced jobs from
needsand from the aggregation. Anew case locks
composites-head: its display name, itsneeds: changes, allnine
uses: ./.github/actions/<x>references, the absence of any pinnedmelodic-software/ci-workflows/...reference inside it, and its presence in boththe
ci-statusneedslist and the aggregated results. The contract-onlypredicate gate is unchanged and still asserted on every job including the new
one.
.github/dependabot.ymlalready carries agithub-actionsgroup; no change wasneeded.
Verification
ls .github/workflows/checks.yml— succeeds.yq '.on.workflow_call.outputs | keys' .github/workflows/checks.yml— printsresultsandoutcome.yq '.jobs[] | select(.uses != null) | .uses' .github/workflows/ci.yml—includes
./.github/workflows/checks.yml.actionlint .github/workflows/checks.yml .github/workflows/ci.yml— clean(actionlint 1.7.12).
zizmor --persona regular .github/workflows/checks.yml— no findings. Onci.ymlthe pre-existing lowself-repositoryfindings went 59 to 46 with theconsolidation and back to 55 with
composites-head, since that audit is theone recommending
$/, the syntax actionlint 1.7.12 rejects. The lane isadvisory (
fail-on-severitydefaults tonever).git diff 449157aaa8e30f7b1457305d8048ebe6168e174a..HEAD --stat -- .github/actions/— empty. No composite body changed since the pin, sochecks.ymlat the next tag runs bodies byte-identical to HEAD and nofollow-up pin bump is owed after the tag.
node --test .github/scripts/*.test.cjs— 782 tests, 777 pass, 5 fail, allfive in
probe-billing-usage.test.cjsand failing identically on an untouchedtree (environment, not this change).
npx @biomejs/biome@2.5.11 ci --config-path=fixtures/typescript/good/biome.json --error-on-warnings fixtures/typescript/good .github/scripts— clean.typos,editorconfig-checkerandmarkdownlint-cli2— clean.checksjobfor the reusable's contract, and
Composites at HEADfor the nine compositebodies at the commit under test.
Related
Refs melodic-software/github-iac#378