diff --git a/.github/workflows/build-paper.yml b/.github/workflows/build-paper.yml index cac6ceda47..9af48fc9d6 100644 --- a/.github/workflows/build-paper.yml +++ b/.github/workflows/build-paper.yml @@ -6,7 +6,11 @@ on: paths: - 'docs/WHITEPAPER/**' pull_request: - branches: [ main, master ] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. paths: - 'docs/WHITEPAPER/**' diff --git a/.github/workflows/catalog-count-gate.yml b/.github/workflows/catalog-count-gate.yml index b1fdc21f6c..a0d1226228 100644 --- a/.github/workflows/catalog-count-gate.yml +++ b/.github/workflows/catalog-count-gate.yml @@ -14,7 +14,11 @@ name: Catalog Count Invariant (CI-01) on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. paths: - "specs/numeric/formats_catalog.t27" - "tools/gen_formats_catalog.py" diff --git a/.github/workflows/check-now-freshness.yml b/.github/workflows/check-now-freshness.yml index 6d2ddd8c30..dda23e3432 100644 --- a/.github/workflows/check-now-freshness.yml +++ b/.github/workflows/check-now-freshness.yml @@ -2,7 +2,11 @@ name: Check Now Freshness on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. jobs: check: diff --git a/.github/workflows/coq-kernel.yml b/.github/workflows/coq-kernel.yml index 4dcbadc531..2ae379b9f9 100644 --- a/.github/workflows/coq-kernel.yml +++ b/.github/workflows/coq-kernel.yml @@ -5,7 +5,11 @@ on: - 'coq/**' - '.github/workflows/coq-kernel.yml' pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. paths: - 'coq/**' - '.github/workflows/coq-kernel.yml' diff --git a/.github/workflows/emit-bitexact-gate.yml b/.github/workflows/emit-bitexact-gate.yml index cabc7d1fc9..e53d05936c 100644 --- a/.github/workflows/emit-bitexact-gate.yml +++ b/.github/workflows/emit-bitexact-gate.yml @@ -12,7 +12,11 @@ name: Emit Bit-Exact Gate (CI-02) on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. paths: # The four backends this gate proves equivalent all live in ONE file, # bootstrap/src/compiler.rs -- gen_zig:3437, gen_verilog:6741, gen_c:10522, diff --git a/.github/workflows/fpga-build.yml b/.github/workflows/fpga-build.yml index 132e9a4495..668d7a5f3e 100644 --- a/.github/workflows/fpga-build.yml +++ b/.github/workflows/fpga-build.yml @@ -2,7 +2,11 @@ name: FPGA E2E Build on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. paths: - 'specs/fpga/**' - 'bootstrap/**' diff --git a/.github/workflows/gate-topology.yml b/.github/workflows/gate-topology.yml new file mode 100644 index 0000000000..e2241187bf --- /dev/null +++ b/.github/workflows/gate-topology.yml @@ -0,0 +1,28 @@ +name: Gate Topology + +# Checks the gates themselves rather than the code they guard. +# +# A gate with `branches: [master]` under `pull_request` does not run when a PR +# targets any other base. On a stacked PR every such gate is absent and +# `gh pr checks` prints a green list -- the green of a gate that never fired. +# Observed in this repository on 2026-08-15 on three gates at once. +# +# This workflow deliberately carries NO `branches:` filter, since a branch filter +# here would reproduce exactly the defect it exists to detect. + +on: + pull_request: + push: + branches: [master] + +jobs: + gate-topology: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install pyyaml + run: python3 -m pip install --quiet pyyaml + + - name: Merge-critical workflows must not filter pull_request by branch + run: python3 scripts/ci/check_pr_branch_filters.py diff --git a/.github/workflows/issue-gate.yml b/.github/workflows/issue-gate.yml index 7c2cbb3620..cec0cf8a0f 100644 --- a/.github/workflows/issue-gate.yml +++ b/.github/workflows/issue-gate.yml @@ -1,9 +1,12 @@ # Enforces L1 TRACEABILITY: No code merged without Closes #N name: Issue Gate on: + # `branches: [master]` removed on purpose: this is a required check, and with + # the filter it never ran on a stacked PR, so traceability was unenforced for + # exactly the PRs most likely to need it. See the same note in + # now-sync-gate.yml. An absent check is not a passing check. pull_request_target: types: [opened, edited, reopened, synchronize] - branches: [master] permissions: issues: read diff --git a/.github/workflows/notebook-gate.yml b/.github/workflows/notebook-gate.yml index 23bcebd6b9..8a5d44646d 100644 --- a/.github/workflows/notebook-gate.yml +++ b/.github/workflows/notebook-gate.yml @@ -5,8 +5,12 @@ name: NotebookLM Gate on: pull_request: + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. types: [opened, synchronize, reopened] - branches: [master, main] push: branches: - '*' diff --git a/.github/workflows/now-sync-gate.yml b/.github/workflows/now-sync-gate.yml index aba25249f2..5988427b93 100644 --- a/.github/workflows/now-sync-gate.yml +++ b/.github/workflows/now-sync-gate.yml @@ -1,7 +1,16 @@ name: NOW Sync Gate on: + # No `branches:` filter on pull_request, deliberately. This gate is in the + # required-check set, and with the filter it did not run at all on a PR whose + # base was another branch -- `gh pr checks` then showed a green list on a + # stacked PR that no substantive gate had examined. A check that is absent is + # not a check that passed, and a reviewer reading the green list cannot tell + # the two apart. Measured on this repository: three gates (this one, + # issue-gate, seal-staleness-warn) were all silently skipped on stacked PRs. + # + # push keeps its filter: pushes to topic branches do not need the gate, only + # the protected branch does. pull_request: - branches: [master] push: branches: [master] diff --git a/.github/workflows/phi-loop-ci.yml b/.github/workflows/phi-loop-ci.yml index e9f28e9562..f06150d614 100644 --- a/.github/workflows/phi-loop-ci.yml +++ b/.github/workflows/phi-loop-ci.yml @@ -2,7 +2,11 @@ name: PHI Loop CI on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. jobs: phi-loop-check: diff --git a/.github/workflows/schema-validation.yml b/.github/workflows/schema-validation.yml index 8e2d5e8c49..96b17a4cd3 100644 --- a/.github/workflows/schema-validation.yml +++ b/.github/workflows/schema-validation.yml @@ -20,7 +20,11 @@ name: Schema Validation on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. push: branches: [master] workflow_dispatch: diff --git a/.github/workflows/seal-coverage.yml b/.github/workflows/seal-coverage.yml index f2ba39aca6..2756eab197 100644 --- a/.github/workflows/seal-coverage.yml +++ b/.github/workflows/seal-coverage.yml @@ -27,7 +27,11 @@ name: Seal Coverage on: pull_request: - branches: [master] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. push: branches: [master] workflow_dispatch: diff --git a/.github/workflows/seal-staleness-warn.yml b/.github/workflows/seal-staleness-warn.yml index 9a1b546903..c8fabbcb9e 100644 --- a/.github/workflows/seal-staleness-warn.yml +++ b/.github/workflows/seal-staleness-warn.yml @@ -12,8 +12,12 @@ name: Seal Staleness Warning # rule: we never auto-reseal here; refreezing is an explicit human/PR step. on: + # `branches: [master]` removed: this warning exists to tell a reviewer that the + # sealed numbers were certified against an older compiler, and a stacked PR + # that moves the seal is precisely the case where the reviewer needs to be + # told. The `paths:` filter stays -- it selects PRs that touch the seal, which + # is the intended scope; the base-branch filter selected nothing useful. pull_request: - branches: [master] paths: - 'bootstrap/src/compiler.rs' - 'bootstrap/stage0/FROZEN_HASH' diff --git a/.github/workflows/secret-scan.yml b/.github/workflows/secret-scan.yml index bed134feb7..67e4ab5107 100644 --- a/.github/workflows/secret-scan.yml +++ b/.github/workflows/secret-scan.yml @@ -10,7 +10,11 @@ on: push: branches: [master, main] pull_request: - branches: [master, main] + # No `branches:` filter on pull_request, deliberately. A branch filter makes + # this gate silent on any PR whose base is not listed -- a stacked PR then + # shows a green check list that a master-based PR would not get. Enforced by + # scripts/ci/check_pr_branch_filters.py. The `push` filter below, where one + # exists, is left alone: restricting post-merge runs is a cost decision. jobs: scan: diff --git a/docs/now/2026-08-21-merge-critical-gates-silent-on-stacked-prs.md b/docs/now/2026-08-21-merge-critical-gates-silent-on-stacked-prs.md new file mode 100644 index 0000000000..210b5bfdde --- /dev/null +++ b/docs/now/2026-08-21-merge-critical-gates-silent-on-stacked-prs.md @@ -0,0 +1,11 @@ +# NOW -- eleven gates were silent on stacked PRs (2026-08-21) + +## ci: merge-critical workflows must not filter pull_request by branch (Closes #2167) + +- **A gate written `on: pull_request: branches: [master]` does not run when the PR base is another branch.** On a stacked PR every such gate is absent and `gh pr checks` prints a green list -- the green of a gate that never fired. Observed here on three gates at once; those were patched, and this closes the rest +- **Measured, by parsing every workflow rather than reading names: eleven, not seven.** A previous work order said seven; that number came from a filtered `grep` and was wrong. Recorded rather than quietly replaced, because the seven was quoted downstream +- `paths:` filters are kept -- they select by what changed, not by target. `push:` branch filters are kept too: restricting post-merge runs is a cost decision, not a gating hole +- **The fix is a configuration test, not vigilance.** `scripts/ci/check_pr_branch_filters.py` lists merge-critical workflows explicitly, in code, reviewed as code. A test that inferred the list -- "everything named `*-gate`" -- would stop covering a gate the moment someone renamed it. Negative-tested: 11 violations before the patch, 0 after +- **Separately measured**: `auto-merge-ready-prs.yml` does not parse as YAML, so GitHub cannot load it and it does not run. Left untouched on purpose -- auto-merge is disabled by policy, and repairing the file would restore an automation that must not run. The test reports it as a warning, since a gate that lands red and stays red for a reason nobody may fix teaches everyone to ignore red +- **`schema-validation.yml` resolved against master, not taken verbatim.** Master had independently added `push:` and `workflow_dispatch:` triggers this branch never saw; applying the branch's version wholesale would have deleted both while removing the `branches:` filter. Only the `branches:` line was dropped, so all three triggers survive -- verified by parsing the merged YAML, and `check_pr_branch_filters.py` reports CLEAN on the result +- Entry migrated from `docs/NOW.md` to `docs/now/` (the layout #2298 introduced); the original entry was dated 2026-08-15 diff --git a/scripts/ci/check_pr_branch_filters.py b/scripts/ci/check_pr_branch_filters.py new file mode 100755 index 0000000000..34bfc78730 --- /dev/null +++ b/scripts/ci/check_pr_branch_filters.py @@ -0,0 +1,198 @@ +#!/usr/bin/env python3 +"""Configuration test: merge-critical workflows must not filter `pull_request` by branch. + +WHY THIS TEST EXISTS +-------------------- +A workflow declared as + + on: + pull_request: + branches: [master] + +does not run at all when the pull request's BASE is another branch. On a stacked +PR -- base = some other feature branch -- every such gate is simply absent, and +`gh pr checks` then prints a green list. That green list is not the green list a +master-based PR would get: it is the green of a gate that never ran. + +Observed 2026-08-15 in this repository: three gates (`now-sync-gate`, +`issue-gate`, `seal-staleness-warn`) were invisible on a stacked PR for exactly +this reason, and the PR read as fully checked. The failure mode is silent by +construction -- nothing reports a gate that did not fire -- so it needs a +configuration test rather than vigilance. + +WHAT IT CHECKS +-------------- +1. Every workflow listed in MERGE_CRITICAL must exist. +2. None of them may carry a `branches` (or `branches-ignore`) filter under + `pull_request` or `pull_request_target`. A `paths` filter is fine: it selects + by what changed, not by where the change is headed. A `push` branch filter is + also fine and is left alone -- restricting post-merge runs to master is a cost + decision, not a gating hole. +3. Every workflow file must parse as YAML. A workflow GitHub cannot load is a + gate that does not exist, which is the same hazard by a different route. + +WHAT IT DELIBERATELY DOES NOT CHECK +----------------------------------- +It does not decide which workflows *ought* to be merge-critical. That list is a +human judgement about what must block a merge, so it is written out explicitly +below and reviewed as code. A test that inferred the list -- say, "everything +whose name contains gate" -- would quietly stop covering a gate the moment +someone renamed it. + +Exit status: 0 clean, 1 violations found, 2 setup error. +""" +import glob +import os +import sys + +try: + import yaml +except ImportError: + print("pyyaml is required: pip install pyyaml", file=sys.stderr) + sys.exit(2) + +# Merge-critical: failure of this workflow should be able to block a merge. +# Reviewed as code on purpose -- see the docstring. +MERGE_CRITICAL = ( + "build-paper.yml", + "catalog-count-gate.yml", + "check-now-freshness.yml", + "coq-kernel.yml", + "emit-bitexact-gate.yml", + "fpga-build.yml", + "issue-gate.yml", + "notebook-gate.yml", + "now-sync-gate.yml", + "phi-loop-ci.yml", + "schema-validation.yml", + "seal-coverage.yml", + "secret-scan.yml", +) + +# Not merge-critical, and each exclusion is stated with its reason so that a +# future reader can disagree with the reason rather than guess at the omission. +NOT_MERGE_CRITICAL = { + "pr-dashboard.yml": "reporting only; a stale dashboard does not gate a merge", + "notebook-sync.yml": "automation targeted at feature branches by design", + "seal-staleness-warn.yml": "warn-only by name and by intent", + "auto-merge-ready-prs.yml": "auto-merge is disabled by policy in this repo", +} + +FILTER_KEYS = ("branches", "branches-ignore") +PR_EVENTS = ("pull_request", "pull_request_target") + + +def load(path): + with open(path) as fh: + return yaml.safe_load(fh) + + +def on_block(doc): + """Return the `on:` mapping. + + YAML 1.1 resolves the bare word `on` to the boolean True, so a document + written with `on:` may land under the key True depending on the loader. Both + are accepted; a test that missed the whole trigger block because of a loader + quirk would pass by vacuity. + """ + if not isinstance(doc, dict): + return None + for key in ("on", True, "On", "ON"): + if key in doc: + v = doc[key] + return v if isinstance(v, dict) else None + return None + + +def main(): + wf_dir = os.path.join(".github", "workflows") + if not os.path.isdir(wf_dir): + print(f"no {wf_dir} -- run from the repository root", file=sys.stderr) + return 2 + + violations = [] + unparseable = [] + missing = [] + + present = {os.path.basename(p) for p in glob.glob(os.path.join(wf_dir, "*.yml"))} + for name in MERGE_CRITICAL: + if name not in present: + missing.append(name) + + for path in sorted(glob.glob(os.path.join(wf_dir, "*.yml"))): + name = os.path.basename(path) + try: + doc = load(path) + except Exception as e: + unparseable.append((name, f"{type(e).__name__}: {e}".splitlines()[0])) + continue + if name not in MERGE_CRITICAL: + continue + on = on_block(doc) + if on is None: + violations.append((name, "no readable `on:` block")) + continue + for ev in PR_EVENTS: + cfg = on.get(ev) + if not isinstance(cfg, dict): + continue + for k in FILTER_KEYS: + if k in cfg: + violations.append( + (name, f"{ev}.{k} = {cfg[k]!r} -- this gate does not run " + f"when a PR targets any other base")) + + print(f"merge-critical workflows checked: {len(MERGE_CRITICAL)}") + print(f"workflow files present: {len(present)}") + print(f"explicitly not merge-critical: {len(NOT_MERGE_CRITICAL)}") + + # An unparseable file is split by whether it is merge-critical, and the split + # is a deliberate judgement rather than leniency. A merge-critical workflow + # that does not parse is a gate that does not exist, so it fails. A file that + # is not merge-critical is reported as a warning, because a gate that is red + # on the day it lands, and stays red for a reason nobody is allowed to fix, + # teaches everyone to ignore red -- which costs more than the file it flags. + hard = [(n, e) for n, e in unparseable if n in MERGE_CRITICAL] + soft = [(n, e) for n, e in unparseable if n not in MERGE_CRITICAL] + + if hard: + print(f"\nUNPARSEABLE MERGE-CRITICAL WORKFLOWS ({len(hard)}):") + for name, err in hard: + print(f" {name}: {err}") + print(" A workflow that does not parse is a gate that does not exist.") + + if soft: + print(f"\nWARNING -- unparseable, not merge-critical ({len(soft)}):") + for name, err in soft: + why = NOT_MERGE_CRITICAL.get(name, "not in the merge-critical list") + print(f" {name}: {err}") + print(f" treated as a warning because: {why}") + print(" GitHub cannot load these files either, so they do not run. That") + print(" may be harmless or may be a silently dead automation; deciding") + print(" which is a human call, so this does not fail the build.") + + if missing: + print(f"\nMISSING ({len(missing)}): {', '.join(missing)}") + print(" Either the file was renamed and this list was not updated, or a") + print(" gate was deleted. Both need a human decision, not a silent pass.") + + if violations: + print(f"\nBRANCH-FILTERED MERGE-CRITICAL WORKFLOWS ({len(violations)}):") + for name, why in violations: + print(f" {name}\n {why}") + print("\nRemove the `branches:` filter from the pull_request trigger. Keep") + print("any `paths:` filter -- it selects by what changed, not by target.") + return 1 + + if missing or hard: + return 1 + + print("\nCLEAN: no merge-critical workflow filters pull_request by branch.") + print("Scope: this checks trigger configuration only. It does not verify that") + print("the gates are registered as required checks in branch protection, which") + print("is repository settings and cannot be read from the tree.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())