diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4082d69..854dd55 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,10 @@ jobs: changes: name: Detect changed paths runs-on: [self-hosted, linux, x64, light] + # Trivial classifier (observed <=15s), but it gates 10 of the other jobs, so + # a wedge here stalls the whole suite. 5m is a generous ceiling vs the 360m + # default. Enforced by tools/check_job_timeouts.py (#398). + timeout-minutes: 5 outputs: code: ${{ steps.filter.outputs.code }} steps: @@ -84,6 +88,8 @@ jobs: fmt: name: Format runs-on: [self-hosted, linux, x64, light] + # fmt across every workspace (observed <=40s). Cap far under the 360m default. + timeout-minutes: 5 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -113,6 +119,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # Workspace clippy (observed ~21m on a cold cache). 45m is ~2x that. + timeout-minutes: 45 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -136,6 +144,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # nextest across the workspace (observed ~7.5m). 30m absorbs a cold cache. + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -169,6 +179,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # Generated-crate + wasmtime/kiln runtime oracles (observed ~27m cold). 60m ~2x. + timeout-minutes: 60 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -196,6 +208,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # Compile-only bench check (observed ~9m). 30m absorbs a cold cache. + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -217,6 +231,9 @@ jobs: # Follow-up: drop sccache from ralf's profile before bumping the # toolchains-role version, then move this back to [self-hosted, light]. runs-on: ubuntu-latest + # rustsec/audit-check hits the network advisory DB (observed ~3m). 15m for + # network variance. + timeout-minutes: 15 steps: - uses: actions/checkout@v4 - uses: rustsec/audit-check@v2 @@ -231,6 +248,8 @@ jobs: # newuidmap is setuid but NoNewPrivileges=true blocks the escalation # (newuidmap: write to uid_map failed: Operation not permitted). runs-on: [self-hosted, linux, x64, light] + # cargo-deny bans/licenses/sources (observed <=20s). + timeout-minutes: 5 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable @@ -249,6 +268,8 @@ jobs: needs: [changes, test] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # llvm-cov across the workspace (observed ~8.5m). 30m absorbs a cold cache. + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -285,6 +306,8 @@ jobs: # lean-mem class — Miri allocates aggressively and benefits from the 24G # MemoryHigh ceiling on smithy lean-mem runners over the 12G rust-cpu cap. runs-on: [self-hosted, linux, x64, lean-mem] + # Miri instance-model tests (observed ~2m; lean-mem contention can stretch it). + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -310,6 +333,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, rust-cpu] + # proptest at 10x cases (observed ~8m). 30m absorbs a cold cache. + timeout-minutes: 30 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@nightly @@ -410,6 +435,9 @@ jobs: fuzz-smoke: name: Fuzz smoke (60s/target) runs-on: [self-hosted, linux, x64, rust-cpu] + # 3 targets x 60s + cold ASan builds (observed ~5m on PRs; this job is + # PR-only). 30m ceiling. + timeout-minutes: 30 # Only run on PRs — pushes to main hit the nightly workflow instead. needs: [changes] if: github.event_name == 'pull_request' && needs.changes.outputs.code == 'true' @@ -439,6 +467,8 @@ jobs: needs: [changes] if: needs.changes.outputs.code == 'true' runs-on: [self-hosted, linux, x64, light] + # cargo-vet supply-chain check (observed <=20s). + timeout-minutes: 5 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable @@ -456,6 +486,9 @@ jobs: rivet-validate: name: Rivet validate (artifacts) runs-on: [self-hosted, linux, x64, rust-cpu] + # rivet validate + all artifact guardrail self-tests (observed ~21m cold, + # dominated by the verification-filter guardrail). 45m is ~2x. + timeout-minutes: 45 steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable @@ -580,6 +613,15 @@ jobs: run: tools/check_required_contexts.py --self-test - name: Required-context guardrail run: tools/check_required_contexts.py + # Required-context timeouts (#398). Sibling of the guardrail above, reusing + # its restricted workflow reader: every job that backs a required context + # must declare a static timeout-minutes in (0, 360), so a wedged merge gate + # cancels instead of holding a runner (and, with strict=true, the queue) for + # the 6-hour default. Same steps-of-this-job rationale as above. + - name: Job-timeout guardrail self-test + run: tools/check_job_timeouts.py --self-test + - name: Job-timeout guardrail + run: tools/check_job_timeouts.py # Closure MSRV drift (#369). Here rather than in a code-gated job for the # same reason as above, and it matters more here than anywhere: the PR # that raises the real floor is a `cargo update`, which touches only diff --git a/.github/workflows/proofs.yml b/.github/workflows/proofs.yml index 3880798..0af1288 100644 --- a/.github/workflows/proofs.yml +++ b/.github/workflows/proofs.yml @@ -36,6 +36,9 @@ jobs: changes: name: Detect changed paths (proofs) runs-on: ubuntu-latest + # Trivial classifier (observed <10s); gates the required Lean typecheck. + # Enforced by tools/check_job_timeouts.py (#398). + timeout-minutes: 5 outputs: code: ${{ steps.filter.outputs.code }} steps: diff --git a/artifacts/requirements.yaml b/artifacts/requirements.yaml index e1ed222..bff6acb 100644 --- a/artifacts/requirements.yaml +++ b/artifacts/requirements.yaml @@ -5093,3 +5093,68 @@ artifacts: status: verified release: v0.36.0 tags: [process, guardrail, ci, tooling, fuzzing] + + - id: REQ-GUARD-CONTEXT-TIMEOUT-001 + type: requirement + title: Every job backing a required context shall declare a bounded timeout-minutes + description: > + A required status check that hangs is not a wrong-green; it is an + indefinite block with no signal (#398). 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 no `timeout-minutes` inherits 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 is `strict = true` — every other open PR queues + behind it. `Detect changed paths` is the sharpest case: it gates ten of the + other jobs, so wedging it stalls the entire suite. + + Measured against the live required set when this landed, 15 of the 18 + required contexts declared no `timeout-minutes`. That is not hypothetical — + it is the blast radius of the first hang, whenever it comes. This is the + same shape REQ-GUARD-GATE-EVIDENCE-002 names ("nothing happened" must render + differently from "it worked"), applied to the merge gate's own liveness. + + OBLIGATION. Every GATEABLE job — PR-triggered against main, no path filter, + no continue-on-error, static name, i.e. exactly the set + `check_required_contexts.py` proves must be required — shall declare a + `timeout-minutes` that is a static integer strictly inside (0, 360). Missing + is the defect; a `${{ }}` template is not a static bound a reader can + evaluate; <= 0 is not a number of minutes; >= 360 is no tighter than the + default it replaces and buys nothing. The check does NOT assert the value is + well-sized ("a few multiples of p95") — that is a human judgement a gate + would only be guessing at; it draws the one line that is not a judgement + call: an unbounded (or uselessly-bounded) required job must not ship. + + ORACLE (executed, non-vacuous). `tools/check_job_timeouts.py --self-test` + carries a 9-row decision table plus two production-corpus assertions. The + rows flip a verdict each way on the same tool: a gateable job with a valid + timeout exits 0, the same job with no timeout exits 1, and templated / + zero / non-integer / >=360 each exit 1 while `359` exits 0 (the upper bound + is satisfiable, not merely restrictive). Two rows prove SCOPE — an advisory + (continue-on-error) job and a path-scoped (undeliverable) job with no + timeout both exit 0, so the gate flags only jobs that can actually block a + merge. The corpus assertions fail closed: a scan that finds zero gateable + jobs, or a glob that misses `.yaml`, is a blind read, not a PASS. + Demonstrated live: against the tree before this change the check exited 1 + naming all 15 unbounded contexts; against the tree after, it exits 0 with + "18 gateable job(s)". The reader is the SAME restricted parser as + `check_required_contexts.py` (extended only to capture `timeout-minutes`) + and stays `--cross-check`ed against PyYAML on the real corpus. + + PROVEN vs ASSUMED, stated precisely. PROVEN: every job that backs a required + context now declares a static bound in (0, 360), and the gate fails closed + on a missing, templated, or degenerate bound and on a blind scan. ASSUMED, + not proven here: (1) that the numbers chosen are correctly sized — they are + a human judgement from the run history, cited per-job in the workflow + comments, not a machine-checked property; (2) that GitHub actually cancels a + job at its declared `timeout-minutes` — runtime behaviour this gate reads + the YAML for but does not exercise; (3) that the GATEABLE set equals the + enforced branch-protection set — `check_required_contexts.py` reconciles the + committed list to the workflows but cannot read branch protection without an + admin token, a gap it states and this REQ inherits. NOT CLAIMED: that + advisory jobs are bounded — a hung `continue-on-error` job still wastes a + runner, but it cannot wedge the merge gate, so it is out of scope here and + left to a successor. + status: implemented + release: v0.37.0 + tags: [process, guardrail, ci, tooling, v0370] diff --git a/artifacts/verification.yaml b/artifacts/verification.yaml index 2d9b214..403d3b3 100644 --- a/artifacts/verification.yaml +++ b/artifacts/verification.yaml @@ -4725,3 +4725,65 @@ artifacts: - type: verifies target: REQ-NC-BUS-PAYLOAD-001 + + - id: TEST-GUARD-CONTEXT-TIMEOUT + type: feature + title: The job-timeout gate fails on an unbounded required job and is blind to advisory ones + description: > + Verifies REQ-GUARD-CONTEXT-TIMEOUT-001 through the decision table in + tools/check_job_timeouts.py --self-test, which runs as its own CI step + BEFORE the gate it protects, in the same rivet-validate job as the other + artifact guardrails. + + THE LOAD-BEARING CASE is "gateable job with no timeout-minutes" -> exit 1. + That was the live state of 15 of the 18 required contexts before this + landed; on that same tree GitHub's PR UI shows every one of them as a green, + well-behaved check. The case is a fixture because the defect is invisible + until a job hangs, and only executing the gate against a job that lacks the + key demonstrates it. + + NON-VACUITY IS TESTED BOTH WAYS AND ACROSS THE SCOPE BOUNDARY. A valid + timeout exits 0 and a missing one exits 1 on the same tool (distinct inputs, + distinct verdicts). Templated (`${{ }}`), zero, non-integer and `>= 360` + each exit 1 — a bound a reader cannot evaluate, or one no tighter than the + default it replaces, is not a bound — while `359` exits 0, so the upper + guard is satisfiable rather than merely restrictive. Two cases pin the + SCOPE: a continue-on-error (advisory) job and a path-scoped (undeliverable) + job, each with NO timeout, both exit 0, because neither can block a merge; + treating either as gateable would report a violation this gate does not own. + + FAIL-CLOSED ON A BLIND SCAN, the recurring hazard in this family. Two + production-corpus assertions run against the real WORKFLOW_GLOB: a scan that + finds zero gateable jobs fails (on this repo there are 18, so zero means the + reader went blind, which must not render as PASS), and the glob must match + `.yaml` as well as `.yml` (Actions loads both; a glob that sees one reports + PASS on a directory it never fully read). + + MEASURED AGAINST THE REAL TREE, not only fixtures. Before this change the + production run exited 1 naming all 15 unbounded required contexts; after + adding a `timeout-minutes` to each of the 15 it exits 0 with "18 gateable + job(s)". The reader is check_required_contexts.py's restricted parser + (extended only to capture `timeout-minutes`), still --cross-checked against + PyYAML on the real corpus, so the extension did not silently diverge. + + HONEST CEILING. The table verifies the gate's own logic and its read of the + committed workflows; it does NOT verify that GitHub cancels a job at the + declared minute (runtime behaviour, not asserted), nor that the chosen + values are well-sized (a human judgement cited in the workflow comments), + nor that branch protection enforces exactly this set (the LIST-to-branch + -protection gap check_required_contexts.py states and this inherits). Like + its siblings it assumes python3 on the runner class; the failure mode is + fail-closed (a missing interpreter 127s the required step red), so the PR + that lands this is the measurement. + fields: + method: automated-test + steps: + - run: tools/check_job_timeouts.py --self-test + - run: tools/check_job_timeouts.py + status: implemented + tags: [process, guardrail, ci, tooling, v0370] + links: + - type: satisfies + target: REQ-GUARD-CONTEXT-TIMEOUT-001 + - type: verifies + target: REQ-GUARD-CONTEXT-TIMEOUT-001 diff --git a/tools/check_job_timeouts.py b/tools/check_job_timeouts.py new file mode 100755 index 0000000..ca06895 --- /dev/null +++ b/tools/check_job_timeouts.py @@ -0,0 +1,449 @@ +#!/usr/bin/env python3 +"""Every job backing a required context must declare a real timeout-minutes (issue #398). + +THE PROBLEM. A required status check that hangs is not a wrong-green — it is an +indefinite 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 no `timeout-minutes` inherits GitHub's 360-minute default before +anything cancels it. For six hours the PR is unmergeable for a reason nobody can +see, the job holds a self-hosted runner the whole time, and with branch +protection's `strict = true` every other open PR queues behind it. `Detect +changed paths` is the sharpest case: it gates ten of the other jobs, so wedging +it stalls the entire suite. + +Measured against the live required set at the time this landed, 15 of the 18 +required contexts declared no `timeout-minutes`. That is not a hypothetical — it +is the blast radius of the *first* hang, whenever it comes. + +THE CHECK. This is a sibling of `check_required_contexts.py`, and reuses its +restricted workflow reader rather than copying it — one parser, one set of +fail-closed refusals, `--cross-check`ed against PyYAML in the same place. That +gate answers "is the committed required set exactly the set of jobs that must be +required?"; this one answers "does every one of those jobs carry a bound that +actually reduces the blast radius of a hang?" + +A job's `timeout-minutes` is *valid* here iff it is a static integer in the open +interval (0, 360): + + * missing — the defect this gate exists for; inherits the 360 default. + * a `${{ … }}` template — not a static bound this reader (or a human auditing + the YAML) can evaluate; a bound you cannot read is not a bound you can trust. + * <= 0 — not a number of minutes. + * >= 360 — no tighter than the default it replaces, so it buys + nothing. A declared 360 is a 360-minute blind wait with extra characters. + +It does NOT check that the value is well-sized ("a few multiples of p95"). That +is a judgement call the issue leaves to a human with the run history, and a gate +that guessed it would be guessing. This gate draws the one line that is not a +judgement call: an unbounded (or uselessly-bounded) required job must not ship. + +SCOPE — GATEABLE ONLY, and why that is exactly "backing a required context". +`check_required_contexts.py` proves the committed required set equals the +GATEABLE set (PR-triggered against main, no path filter, no continue-on-error, +static name). So checking every GATEABLE job here *is* checking every job that +backs a required context — and it does so straight from the workflows, with no +dependency on the committed list, which means a newly-added gateable job is +covered the moment it exists rather than the moment someone remembers to list +it. Advisory (`continue-on-error`) and undeliverable (path-scoped / templated) +jobs are deliberately out of scope: they cannot block a merge, so a hang in one +is not the six-hour merge-gate wedge this gate is about. (An advisory hang still +wastes a runner; that is real but separate, and out of scope here so this gate +stays about the merge gate.) + +WHY NOT FOLD THIS INTO check_required_contexts.py. That gate's whole subject is +set membership; a timeout is a per-job property with different remedies and a +different failure. Kept apart, each self-test stays legible and neither gate's +fixtures have to carry the other's concern. + +Usage: + tools/check_job_timeouts.py # check the real workflows + tools/check_job_timeouts.py --self-test # fixture cases; run this FIRST in CI + tools/check_job_timeouts.py --list # print each gateable job and its timeout + +Exit: 0 clean, 1 violation, 2 usage/environment error (fail-closed). +""" + +from __future__ import annotations + +import argparse +import glob +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from check_required_contexts import ( # noqa: E402 (path set above) + GATEABLE, + WORKFLOW_GLOB, + WorkflowSyntaxError, + classify_jobs, + parse_workflow, +) + +#: GitHub's default job timeout. A job with no `timeout-minutes` runs up to this +#: long before it is cancelled; a declared value >= this is no improvement. This +#: is the number the issue names as the blind-wait ceiling. +DEFAULT_TIMEOUT_MINUTES = 360 + + +def validate_timeout(raw) -> str | None: + """None if `raw` is a valid bound; otherwise the reason it is not. + + `raw` is the parsed scalar of a job's `timeout-minutes`, or None if the key + is absent. A valid bound is a static integer strictly inside (0, 360): large + enough to be a real number of minutes, small enough to beat the default it + replaces. + """ + if raw is None: + return "declares no `timeout-minutes`" + s = str(raw).strip() + if "${{" in s: + return ( + f"sets `timeout-minutes: {s}`, a template expression rather than a " + f"static bound — a timeout you cannot read at rest is not one you can rely on" + ) + try: + minutes = int(s) + except ValueError: + return f"sets `timeout-minutes: {s}`, which is not an integer" + if minutes <= 0: + return f"sets `timeout-minutes: {minutes}`, which is not a positive number of minutes" + if minutes >= DEFAULT_TIMEOUT_MINUTES: + return ( + f"sets `timeout-minutes: {minutes}`, which is >= GitHub's " + f"{DEFAULT_TIMEOUT_MINUTES}-minute default and so does not reduce the " + f"blast radius of a wedged job" + ) + return None + + +def scan_gateable(workflow_glob: str = WORKFLOW_GLOB) -> list[tuple[str, str, str, object]]: + """(context name, workflow path, job key, raw timeout) for every GATEABLE job. + + Reuses check_required_contexts' reader and classifier verbatim, so the set + scanned here is exactly the set that gate proves must be required. The reader + raises WorkflowSyntaxError on anything it cannot represent at the depths it + reads; that propagates and main() renders it as exit 2 (refuse, do not guess). + """ + out: list[tuple[str, str, str, object]] = [] + for path in sorted(glob.glob(workflow_glob)): + with open(path) as fh: + doc = parse_workflow(fh.read(), path) + for jid, (bucket, name, _reasons) in classify_jobs(doc).items(): + if bucket != GATEABLE: + continue + raw = (doc["jobs"].get(jid) or {}).get("timeout-minutes") + out.append((name, path, jid, raw)) + return out + + +def run_check(workflow_glob: str = WORKFLOW_GLOB, out=None) -> int: + out = out or sys.stdout + gateable = scan_gateable(workflow_glob) + + errors: list[str] = [] + for name, path, jid, raw in sorted(gateable): + why = validate_timeout(raw) + if why: + errors.append( + f"{path}: job `{jid}` produces required context {name!r} but {why}. " + f"A wedged required job with no real bound sits at \"Expected — waiting " + f"for status\" for up to {DEFAULT_TIMEOUT_MINUTES} minutes, holds a " + f"runner, and (branch protection is strict) blocks every other PR." + ) + + print(f"scanned {len(gateable)} gateable job(s) backing a required context", file=out) + + if errors: + for e in errors: + print(f"::error::{e}", file=out) + print(f"\nFAIL: {len(errors)} required-context job(s) without a valid timeout-minutes.", file=out) + return 1 + + print( + f"PASS: every gateable job declares a static timeout-minutes in " + f"(0, {DEFAULT_TIMEOUT_MINUTES}).", + file=out, + ) + return 0 + + +# ── self-test ───────────────────────────────────────────────────────────── +# +# Run on every CI invocation via --self-test. A guardrail exercised only against +# a repo that already satisfies it cannot be told apart from one that returns 0 +# unconditionally: the live repo is a single input. Each fixture below turns one +# verdict the other way, so deleting any single guard reds this list. + +_OK = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: 30 + runs-on: ubuntu-latest +""" + +_MISSING = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + runs-on: ubuntu-latest +""" + +_TEMPLATED = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: ${{ fromJSON(env.T) }} + runs-on: ubuntu-latest +""" + +_ZERO = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: 0 + runs-on: ubuntu-latest +""" + +_NONINT = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: soon + runs-on: ubuntu-latest +""" + +_OVER_DEFAULT = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: 400 + runs-on: ubuntu-latest +""" + +_JUST_UNDER = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha + timeout-minutes: 359 + runs-on: ubuntu-latest +""" + +#: A continue-on-error job with no timeout. It cannot block a merge, so it is out +#: of scope and must NOT be flagged — treating it as gateable would report a +#: violation this gate does not own. +_ADVISORY_NO_TIMEOUT = """ +name: W +on: + pull_request: + branches: [main] +jobs: + a: + name: Alpha (advisory) + continue-on-error: true + runs-on: ubuntu-latest +""" + +#: A path-scoped workflow: its job is UNDELIVERABLE, never required, so a missing +#: timeout is not this gate's concern. +_UNDELIVERABLE_NO_TIMEOUT = """ +name: W +on: + pull_request: + paths: + - "src/**" +jobs: + a: + name: Alpha + runs-on: ubuntu-latest +""" + +SELF_TESTS = [ + ( + "gateable job with a valid timeout", + _OK, 0, + "the checker can return 0 at all — without this the other cases are " + "consistent with a script that always fails", + ), + ( + "gateable job with no timeout-minutes", + _MISSING, 1, + "THE DEFECT: an unbounded required job inherits the 360-minute default " + "and wedges the merge gate for six hours with no signal (the 15-job shape)", + ), + ( + "gateable job with a templated timeout", + _TEMPLATED, 1, + "a `${{ }}` timeout is not a static bound a reader can evaluate; it could " + "resolve to nothing or to something huge, and the gate must not bless it", + ), + ( + "gateable job with timeout-minutes: 0", + _ZERO, 1, + "zero (and any non-positive) is not a real number of minutes", + ), + ( + "gateable job with a non-integer timeout", + _NONINT, 1, + "a non-integer would be a YAML error at runtime; catch it at rest, not in " + "the wedged job it was supposed to bound", + ), + ( + "gateable job with timeout >= the 360 default", + _OVER_DEFAULT, 1, + "a value at or above GitHub's 360-minute default reduces no blast radius — " + "a declared 360 is the same six-hour blind wait with extra characters", + ), + ( + "gateable job with timeout just under the default", + _JUST_UNDER, 0, + "the upper bound must be satisfiable, not merely restrictive: 359 beats " + "the default and is accepted", + ), + ( + "advisory job with no timeout is not flagged", + _ADVISORY_NO_TIMEOUT, 0, + "a continue-on-error job cannot block a merge, so it is out of scope; " + "flagging it would claim a violation this gate does not own", + ), + ( + "undeliverable (path-scoped) job with no timeout is not flagged", + _UNDELIVERABLE_NO_TIMEOUT, 0, + "a path-scoped job can never be required, so its missing timeout is not " + "the merge-gate wedge this gate guards", + ), +] + + +def self_test() -> int: + import io + import os + import tempfile + + failures = 0 + for name, wf_text, want, proves in SELF_TESTS: + with tempfile.TemporaryDirectory() as tmp: + # `.yaml`, the extension the original `*.yml` glob could not see — + # every fixture therefore also exercises discovery through the shared + # WORKFLOW_GLOB, which is `.y*ml`. + with open(os.path.join(tmp, "w.yaml"), "w") as fh: + fh.write(wf_text) + buf = io.StringIO() + try: + got = run_check(os.path.join(tmp, "*.y*ml"), out=buf) + except WorkflowSyntaxError as exc: + got = 2 + print(f"refused: {exc}", file=buf) + + mark = "ok " if got == want else "FAIL" + if got != want: + failures += 1 + print(f" {mark} {name}: exit {got} (want {want})") + print(f" proves: {proves}") + if got != want: + print(" ---- output ----") + for line in buf.getvalue().splitlines(): + print(f" {line}") + + # The fixtures run against a glob this function builds, so they never touch + # the real corpus. Two production-only ways to go silently green are checked + # directly, against the same WORKFLOW_GLOB the real run uses: + # + # 1. The scan finds zero gateable jobs. On this repo that is impossible + # (there are ~18) — so zero means the reader went blind, which must fail, + # not pass on a corpus it never read. This is the empty-scan failure mode + # the rest of tools/ guards against, asserted here rather than assumed. + try: + live = scan_gateable(WORKFLOW_GLOB) + except WorkflowSyntaxError as exc: + print(f" FAIL live scan refused to parse a workflow: {exc}") + return 1 + if not live: + print(" FAIL live scan found zero gateable jobs") + print(" proves: a scan that sees nothing must fail, not report PASS " + "on a corpus it never read") + failures += 1 + else: + print(f" ok live scan found {len(live)} gateable job(s)") + print(" proves: the production scan is not silently empty") + + # 2. The glob misses `.yaml`. Actions loads .yml AND .yaml, so a glob that + # sees only one reports PASS on a directory it never fully read. + import fnmatch + + if not fnmatch.fnmatch(".github/workflows/x.yaml", WORKFLOW_GLOB): + print(f" FAIL WORKFLOW_GLOB does not match `.yaml`: {WORKFLOW_GLOB!r}") + failures += 1 + else: + print(" ok WORKFLOW_GLOB matches both .yml and .yaml") + + total = len(SELF_TESTS) + 2 + if failures: + print(f"\n{failures} of {total} self-test(s) FAILED.") + return 1 + print(f"\n{total} self-test(s) passed.") + return 0 + + +def main() -> int: + p = argparse.ArgumentParser(description=__doc__) + p.add_argument("--workflows", default=WORKFLOW_GLOB) + p.add_argument("--self-test", action="store_true") + p.add_argument("--list", dest="do_list", action="store_true") + args = p.parse_args() + + if args.self_test: + return self_test() + + if args.do_list: + for name, path, jid, raw in sorted(scan_gateable(args.workflows)): + shown = "(none)" if raw is None else raw + print(f"{shown!s:>8} {name} [{path} :: {jid}]") + return 0 + + return run_check(args.workflows) + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except WorkflowSyntaxError as exc: + # Fail closed, exactly as the shared reader's own entry point does: a + # workflow shape the reader cannot classify is one whose timeout it cannot + # read either. + print(f"::error::{exc}", file=sys.stderr) + print( + "::error::The workflow reader is deliberately restricted (see " + "check_required_contexts.py) and stops rather than guessing.", + file=sys.stderr, + ) + raise SystemExit(2) diff --git a/tools/check_required_contexts.py b/tools/check_required_contexts.py index 7b4804e..29256e9 100755 --- a/tools/check_required_contexts.py +++ b/tools/check_required_contexts.py @@ -225,10 +225,13 @@ def parse_workflow(text: str, path: str = "") -> dict: checkable. What it reads: on.pull_request.{branches,paths,paths-ignore} (indent 0/2/4) - jobs..{name,continue-on-error} (indent 0/2/4) + jobs..{name,continue-on-error,timeout-minutes} (indent 0/2/4) Everything deeper (steps, strategy, with:) is deliberately invisible — a step's `name:` sits at indent 8 and must not be mistaken for a job's. + (`timeout-minutes` is read here purely so a sibling gate — + check_job_timeouts.py — can reuse this reader; nothing in *this* gate looks + at it, which is why cross_check does not compare it.) """ doc: dict = {"on": None, "jobs": {}} section = None # None | "on" | "jobs" @@ -305,7 +308,7 @@ def parse_workflow(text: str, path: str = "") -> dict: continue if indent == 4 and section == "jobs" and job is not None: - if key in ("name", "continue-on-error"): + if key in ("name", "continue-on-error", "timeout-minutes"): _reject_unsupported(val, path, lineno, f"`jobs.{job}.{key}:`") doc["jobs"][job][key] = val continue