feat(guard): require a bounded timeout-minutes on every required-context job (#398) - #411
Merged
Merged
Conversation
…ext job (#398) A required status check that hangs is an indefinite merge block with no signal: the context sits at "Expected — waiting for status" (identical to "still running"), and a job with no timeout-minutes inherits GitHub's 360-minute default before anything cancels it — holding a self-hosted runner and, with strict branch protection, queuing every other PR. 15 of the 18 required contexts declared no timeout-minutes; Detect changed paths, which gates ten other jobs, was the sharpest case. Add tools/check_job_timeouts.py, a sibling of check_required_contexts.py that reuses its restricted, PyYAML-cross-checked workflow reader: every GATEABLE job (exactly the set that gate proves must be required) must declare a static timeout-minutes in (0, 360). Missing, templated, <=0, or >=360 all fail; the scan fails closed on zero gateable jobs or a glob that misses .yaml. A 9-row self-test flips a verdict each way (missing->1, valid->0, advisory/undeliverable no-timeout->0) and runs before the gate, in the rivet-validate job. Give the 15 unbounded jobs a timeout sized to a few multiples of their observed p95 (cold-cache worst case), cited per job: classifiers/fmt/deny/vet at 5, audit 15, test/proptest/bench/coverage/miri/fuzz 30, clippy/rivet-validate 45, codegen-oracle 60. Extend check_required_contexts.py's reader to capture timeout-minutes (additive; cross-check unaffected). Track as REQ-GUARD-CONTEXT-TIMEOUT-001 / TEST-GUARD-CONTEXT-TIMEOUT. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Rivet verification gate✅ 20/20 passed
Filter: Failed artifacts(none) Updated automatically by |
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.
Closes #398.
The problem
A required status check that hangs is not a wrong-green — it is an indefinite merge block with no signal. A wedged job leaves its context at
Expected — waiting for status to be reported, which is byte-identical to "still running", and a job with notimeout-minutesinherits GitHub's 360-minute default before anything cancels it. For up to six hours the PR is unmergeable for a reason nobody can see, the job holds a self-hosted runner, and — branch protection isstrict = true— every other open PR queues behind it.Detect changed pathsis the sharpest case: it gates ten of the other jobs, so wedging it stalls the entire suite.Measured against the live required set: 15 of the 18 required contexts declared no
timeout-minutes. The 3 that did: Mutation Testing (240), Verification Gate (60), Lean typecheck (90).What this does
1. A guard —
tools/check_job_timeouts.py. A sibling ofcheck_required_contexts.pythat reuses its restricted, PyYAML---cross-checked workflow reader rather than copying it. Every GATEABLE job — exactly the set that gate proves must be required — must declare atimeout-minutesthat is a static integer in the open interval(0, 360):${{ … }}template → not a static bound a reader can evaluate;<= 0→ not a number of minutes;>= 360→ no tighter than the default it replaces, so it buys nothing.Advisory (
continue-on-error) and undeliverable (path-scoped) jobs are out of scope — they can't block a merge. The scan fails closed: zero gateable jobs found, or a glob that misses.yaml, is a blind read, not a PASS.The check_required_contexts.py reader is extended (additively) to capture
timeout-minutes;--self-testand--cross-checkon it still pass.2. The 15 missing values, sized to a few multiples of each job's observed p95 (cold-cache worst case), cited per-job in the workflow comments:
Durations were read from recent CI run history (2 main runs + 1 PR run). This does not try to machine-check that the values are well-sized — that's a human judgement; the gate only draws the line that isn't one: an unbounded (or uselessly-bounded) required job must not ship.
Oracle (executed, non-vacuous)
tools/check_job_timeouts.py --self-test— a 9-row decision table plus two production-corpus assertions — runs before the gate, as a step of therivet-validatejob (same steps-of-this-job rationale as the sibling: a new job would itself be gateable and need admin-token branch-protection sync).>=360→ 1,359→ 0, advisory/undeliverable with no timeout → 0..yaml).An independent clean-room pass re-derived the 15/18 numbers, proved non-vacuity by sabotaging
validate_timeout(self-test then fails), and checked the PROVEN-vs-ASSUMED honesty — no over-claim found.Honest scoping
PROVEN: every job backing a required context declares a static bound in
(0, 360), and the gate fails closed on a missing/templated/degenerate bound and on a blind scan.ASSUMED (not proven here): that the chosen minute values are correctly sized; that GitHub actually cancels at the declared minute (runtime, not exercised); that the GATEABLE set equals the branch-protection-enforced set (the admin-token gap
check_required_contexts.pystates and this inherits).NOT claimed: that advisory jobs are bounded — a hung
continue-on-errorjob wastes a runner but can't wedge the merge gate; left to a successor.Tracked as
REQ-GUARD-CONTEXT-TIMEOUT-001/TEST-GUARD-CONTEXT-TIMEOUT(real steps +satisfies/verifieslinks).rivet validate: 0 broken cross-refs.🤖 Generated with Claude Code
Generated by Claude Code