fix(checks): bound each download-heavy step with timeout-minutes - #577
Conversation
A stalled dependency download in the shared `checks` lane consumed the calling job's entire `timeout-minutes` budget, so the run ended `cancelled` rather than `failure`. The `outcome` join never ran, the reusable workflow published no outputs, and the caller's `ci-status` aggregate read the lane as a failure with no named cause. Every step in the job that downloads something now carries `timeout-minutes: 8`: the four `actions/cache`-backed tarball installers (typos, gitleaks, editorconfig, lychee-offline), the two that also fetch ShellCheck (shellcheck, actionlint), the `actions/setup-node` one (markdown), and the `astral-sh/setup-uv` one (check-jsonschema). The pure-git composites (exec-bit, machine-specific-paths, eol-renormalize, comment-hygiene) and the API-only `change-detection` are unchanged. The key goes on the workflow step, not inside the composite: a composite action's `runs.steps[*]` accept no `timeout-minutes`. The runner sets a timed-out step's result to `Failed`, not `Canceled`, so `continue-on-error: true` still holds and the join names the stalled tool instead of the run dying silently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L5ZJnwTRCgc31bmfrdugyU
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 3m 43s —— View job Code review
Reviewed the diff ( Finding: the
|
|
Claude finished @kyle-sexton's task in 42s —— View job Security review — no security-relevant surface
Skip gate hit: no security-relevant surface. The diff (
No triggers, No inline findings to report. |
|
Last security-reviewed head: |
![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: e287209f68
ℹ️ 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".
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
The eight `timeout-minutes: 8` values were unasserted, so a future edit that dropped one, or a new download-heavy composite added without one, would not surface until the next stall incident. `checks-outcome.test.cjs` already parses `checks.yml` structurally and enumerates the twelve `continue-on-error` composites, so the assertion reuses that enumeration: every composite except the four shell-only ones must carry `timeout-minutes: 8`. A new composite is required to declare a budget or be named as shell-only, rather than defaulting to unbounded. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L5ZJnwTRCgc31bmfrdugyU
…umers (#578) No related issue: drift found while cutting the v0.24.0 release for PR #577; the correction is small enough to carry on its own PR rather than open an issue first. ## Summary The `## Versioning` section justified the tag-per-change policy with a claim that is not true of this repository's consumers: > cutting one after every meaningful change keeps `main` at the latest tag so > Dependabot's `github-actions` group bumps consumers to a tagged SHA instead > of tracking `main` HEAD by drift. Nothing bumps consumers automatically. Consumers pin `ci-workflows` by full 40-character commit SHA with a `# vX.Y.Z` trailing comment, and that pin moves only through a reviewed pull request in the consumer's own repository. At least one consumer's Dependabot configuration explicitly ignores `melodic-software/ci-workflows/*`, so the `github-actions` group never opens a bump PR for this repository at all. The claim mattered in practice: PR #577 added `timeout-minutes` to eight steps in `.github/workflows/checks.yml` and merged at `2c1de45`, but because consumers pin by SHA and no release existed at that commit, the change reached no consumer. Believing the README meant believing the change had already propagated. `SECURITY.md` stated the same two now-corrected claims, that consumers pick up fixes via an advancing pinned SHA Dependabot bumps and that they should keep the `github-actions` Dependabot ecosystem enabled, and is rewritten here to the same reviewed-repin model. ## Fix Replaced the Dependabot clause with what actually happens, and left the rest of the paragraph's reasoning intact: - The stated reason to cut a release is now that it makes a tagged SHA available to pin to, which is the true benefit. - Added one sentence stating the real propagation path: SHA pin plus version comment, moved by a reviewed PR in the consumer's repository. - The following `release-gap-check` sentence was already correct and is preserved verbatim; it is only rewrapped because the replacement changed the line boundaries. - `SECURITY.md`: the `## Supported versions` paragraph now says a security fix reaches a consumer only when that consumer repins by reviewed pull request, names the `release-gap-check` advisory issue, and tells consumers to watch this repository's Releases; the **Pin by full commit SHA** bullet now says to keep this repository out of Dependabot's `github-actions` updates with an `ignore` entry and move the pin by reviewed pull request. The diff is one README paragraph plus two SECURITY.md sentences. ## Verification - `npx markdownlint-cli2@0.23.2 README.md` against the repo's own `.markdownlint-cli2.jsonc`: `Summary: 0 issues in 0 files`, exit 0. - `npx markdownlint-cli2 --config .markdownlint-cli2.jsonc SECURITY.md`: `Summary: 0 issues in 0 files`, exit 0. - `git diff --stat` for the README commits: `README.md | 15 +++++++-------`, `1 file changed, 8 insertions(+), 7 deletions(-)`. - `git diff --stat` for the SECURITY.md commit: `SECURITY.md | 14 +++++++-----`, `1 file changed, 9 insertions(+), 5 deletions(-)`. - `git diff | grep '^+.*[ \t]$'` returns no matches, so no trailing whitespace was introduced. - Read back the rendered paragraph to confirm the `release-gap-check` sentence survives word-for-word and only its line wrapping changed. - Confirmed the underlying facts before writing: `release.yml` computes the next version from published Releases and creates a tag plus Release at `GITHUB_SHA`; nothing in it or in this repository opens a bump PR against a consumer. ## Related - Follows from #577, whose `timeout-minutes` change sat unreleased and therefore unreachable by SHA-pinned consumers. - Release `v0.24.0` (https://github.com/melodic-software/ci-workflows/releases/tag/v0.24.0) was cut at `2c1de45aa0e1b1489afb8edfebc12cb3a4fa6ac3` immediately before this PR, which is what actually delivers #577 to consumers once they repin. - The same incorrect Dependabot claim also appears in the header comment of `.github/workflows/release.yml`. It is deliberately left alone here to keep this PR to documentation files; it is reported separately as follow-up. 🤖 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>
1 participant
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 / checksjob — whichcalls this repository's
checks.ymlreusable atv0.22.2withtimeout-minutes: 15— stalled inside thecheck-jsonschemacomposite, onastral-sh/setup-uv's download. No step had its own budget, so the stallconsumed the whole job budget and the run ended
cancelled.cancelledis the worst possible verdict for this lane. Theoutcomejoin isgated on
!cancelled(), so it never ran and named nothing; a reusable-workflowjob that does not complete publishes no outputs; and the caller's
ci-statusaggregate 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
checksjob that downloads something now carriestimeout-minutes: 8— comfortably above the 4.5-minute healthy ceiling for thewhole job, well under the 15-minute default job budget:
typoscrate-ci/typosrelease tarball behindactions/cachegitleaksgitleaks/gitleaksrelease tarball behindactions/cacheeditorconfigeditorconfig-checkerrelease tarball behindactions/cachemarkdownactions/setup-node, thenmarkdownlint-cli2shellcheckkoalaman/shellcheckrelease tarball behindactions/cacheactionlintrhysd/actionlintandkoalaman/shellchecktarballslychee_offlinelycheeverse/lycheerelease tarball behindactions/cachecheck_jsonschemaastral-sh/setup-uv, thenuvx check-jsonschema— the step that stalledUntouched, because they download nothing:
configuration,detect(
actions/github-script, API only),exec_bit,machine_specific_paths,eol_renormalize,comment_hygiene, and theoutcomejoin. No compositeaction file is modified.
Why the key goes on the workflow step and not inside the composite
A composite action's
runs.steps[*]acceptsrun,shell,if,name,id,env,working-directory,uses,withandcontinue-on-error— andno
timeout-minutes(Metadata syntax for GitHub Actions).
The feature request to add it is
actions/runner#1979, still
open. The workflow step's own
timeout-minutesis honoured for the wholecomposite invocation:
actions/runner#599 (merged
2020-07-22) wired the step's
timeout-minutesinto the cancellation token ofthe composite action step's execution context.
Why this restores the signal rather than just shortening the stall
A step killed by its own
timeout-minutesis markedFailed, notCanceled—StepsRunner.csdistinguishes the two by whether the job token also fired, and sets
TaskResult.Failedfor the step-timeout branch. Socontinue-on-error: truestill holds,
steps.<id>.outcomeisfailure, theoutcomejoin runs andemits
::error::check-jsonschema failed (outcome=failure)., and the caller'saggregate 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 frominputs.timeout-minutes, is a separate change and is deliberately not madehere.
Verification
Run from the worktree
D:/worktrees/ciw-checks-step-timeoutonci/checks-step-timeouts:actionlint --version→1.7.12— the exact version.github/actions/actionlint/action.ymlpins as its default.actionlint .github/workflows/checks.yml→ no output, exit 0. actionlintparses 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: 8lines and aten-line
# Stalls:paragraph appended to the file header. No other key,value, pin, or SHA changed. No file outside
.github/workflows/checks.ymlis 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'smanual
workflow_dispatch. No bump or changelog entry is owed for this PR.This PR's own CI executes the modified file:
ci.ymldogfoods it throughuses: ./.github/workflows/checks.ymlat the commit under test, so the runneraccepts 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
checksreusable added infeat(workflows): add the checks reusable and dogfood it #551 and the pin convergence in ci: converge on ci-workflows v0.22.0 and adopt the contract-only concurrency group #558.
melodic-software/dotfiles.github/workflows/ci.yml,which pins
checks.yml@5776760254f8b63cba44e896f51604cb755350d9 # v0.22.2.It picks this up only after a release is cut and the pin is advanced, and
neither hop is automatic:
release.ymlisworkflow_dispatch-only, anddotfiles'
dependabot.ymlignoresmelodic-software/ci-workflows/*sothat every ci-workflows bump moves through a deliberate reviewed PR.
🤖 Generated with Claude Code
https://claude.ai/code/session_01L5ZJnwTRCgc31bmfrdugyU