Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions .claude/skills/ci-gates/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -9897,3 +9897,55 @@ Grepping the symptom rather than the construct has the same failure in the other
direction: `src.len()` also appears in `String::with_capacity(src.len())`, which
has nothing to do with a path. `verilog_r_si_1.rs` would have been convicted by a
grep and is clean.

## 395. A guard's clean line is a claim about a population — print the remainder

`scripts/ci/check_pr_branch_filters.py` ended every run with

```
merge-critical workflows checked: 15
workflow files present: 49
explicitly not merge-critical: 4

CLEAN: no merge-critical workflow filters pull_request by branch.
```

The two list sizes and the file count are printed **three lines apart and never
subtracted**. `15 + 4 = 19` against `49`, so thirty files were read by nothing,
and the last line still said CLEAN.

**Two of the thirty carried the exact defect the check exists to detect** —
`corpus-ratchet.yml` and `withdrawn-live-gate.yml` both had
`pull_request: branches: [master]`, which means the gate does not run at all when
a PR targets any other base, so a stacked PR shows a green check list with that
gate simply absent from it.

The check was not wrong about the fifteen it read. It was wrong about what its
clean line **meant** — and the numbers that would have said so were already on
the screen.

So, for any guard that walks a list:

- **Make the parts sum to the directory, out loud.** One line,
`15 + 4 + 30 = 49 (must equal 49)`, is the whole finding.
- **Name the remainder as its own bucket.** "Not classified" is a third state
between pass and fail, and it must be printed even when nobody intends to act
on it.
- **Run the same read over the remainder and REPORT it.** Whether an unclassified
workflow ought to block a merge is a human call; whether anybody looked is not.
That is how the two offenders surfaced. Print the count as a zero when it is
zero, or the next reader cannot tell "none" from "not asked".
- **Ceiling, not refusal, on the remainder.** Twenty-seven files cannot be
classified in the commit that discovers them, and a gate red on the day it
lands teaches everyone to ignore red. `MAX_UNCLASSIFIED`, down only, buys the
thing that actually matters: the *next* file added cannot land unread.
- **Make the final line state the scope it earned.** "CLEAN … and 27 file(s)
remain unread at a ceiling of 27" is a sentence a reader can act on. "CLEAN" is
one they cannot.

This is the same shape as 384 (the list that went stale by MY addition) seen from
the other side: 384 is a list that stops covering something it used to; this is a
list that never covered the directory at all, and said so in numbers nobody
subtracted. The controls that hold it are three, each seen failing on purpose —
a 28th unclassified file, a name in both lists, and the restored filter on
`corpus-ratchet.yml`, which is also the historical control for the two findings.
6 changes: 5 additions & 1 deletion .github/workflows/corpus-ratchet.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ name: Corpus Ratchet

on:
pull_request:
branches: [master]
# No `branches:` filter. It was `branches: [master]`, so this gate did not
# run at all when a pull request targeted any other base -- on a stacked PR
# `gh pr checks` printed a green list with this gate simply absent from it.
# `paths:` selects by what changed and is fine; `branches:` selects by where
# the change is headed and is a gating hole.
push:
branches: [master]
# Without this the missing reading cannot be taken on purpose: a gate with
Expand Down
76 changes: 76 additions & 0 deletions .github/workflows/harness-scratch.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# Shared scratch directories -- test binaries whose tests delete each other's input.
#
# The shape, found in five test files at once (#2954):
#
# let dir = temp_dir().join(format!("t27-scaffold-{}-{}", process::id(), src.len()));
# std::fs::create_dir_all(&dir)?;
# ...
# let _ = std::fs::remove_dir_all(&dir); // deletes the WHOLE directory
#
# One process per test binary, so the pid is constant and the key is really
# `src.len()`. Two tests whose sources happen to be the same length compute the
# same path, and each deletes it on the way out. Under the default parallel
# runner one erases the spec another is mid-read of, `t27c` prints nothing, and
# the assertion reports an empty result.
#
# It is not a slow test, it is a WRONG one, and it does not announce itself:
# `scaffold_c` failed about one run in three, and the four siblings carrying the
# same shape had never failed once. Printing the paths of a single run is what
# settled it -- six tests produced THREE directories before the repair and six
# after.
#
# NO `paths:` FILTER, deliberately, for two reasons. The first is the same one
# the orphan gate gives: the file that introduces the collision is not
# necessarily the file that dies of it, so a filter on the failing path would be
# silent for exactly this defect. The second is newer and cost a measurement
# today -- `emit-bitexact-gate.yml` is `pull_request` with a `paths:` filter and
# no `push:`, so it has NEVER run on master. When a change made it fail there was
# no baseline to compare against and `gh run list --branch master` returned
# nothing, which reads like "no data" and is one careless sentence away from
# "no problem". A gate with no master history cannot answer the only question
# that matters when it goes red.
#
# A refusal, not a ratchet: the class is CLOSED as of #2949 and #2955 -- the
# command reports `none` on master today, and there is no ceiling to hold, no
# slack for the next one to hide in.

name: Harness scratch directories

on:
pull_request:
# No `branches:` filter, matching the rest of this repository: a branch
# filter makes a gate silent on any PR whose base is not listed, and a
# stacked PR then shows a green check list a master-based PR would not get.
push:
branches: [master]
workflow_dispatch:

concurrency:
group: harness-scratch-${{ github.ref }}
cancel-in-progress: true

jobs:
scratch:
name: No test binary shares one scratch directory
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- uses: actions/checkout@v4

- uses: dtolnay/rust-toolchain@stable

- name: Build the tool
run: cargo build --release -p tri

# A gate that cannot fail is not a gate, and CI reads the exit code, not
# the prose. Five legs, because this detector had two holes that were
# invisible by reading it and only appeared when it was run against files
# whose answer was already known by other means: a key interpolating only
# `process::id()` looked variable to its first rule, and judging the
# arguments alone then convicted an inline-capture key that is distinct
# per test. A clean run of a control that cannot fail claims nothing.
- name: Negative control
run: ./target/release/tri harness scratch --self-check

- name: No shared scratch directory
run: ./target/release/tri harness scratch --gate
6 changes: 5 additions & 1 deletion .github/workflows/withdrawn-live-gate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ name: Withdrawn Number Gate

on:
pull_request:
branches: [master]
# No `branches:` filter. It was `branches: [master]`, so this gate did not
# run at all when a pull request targeted any other base -- on a stacked PR
# `gh pr checks` printed a green list with this gate simply absent from it.
# `paths:` selects by what changed and is fine; `branches:` selects by where
# the change is headed and is a gating hole.
push:
branches: [master]
workflow_dispatch:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# NOW -- A gate that reported CLEAN over 19 of 49 files (2026-08-30)

## A gate that reported CLEAN over 19 of 49 files (Closes #2957)

- check_pr_branch_filters.py printed the two list sizes beside the file count and never subtracted them; 30 workflows were read by nothing and the last line still said CLEAN
- two of the thirty carried pull_request branches:[master] -- the defect it exists to detect -- and one is corpus-ratchet, absent from the check list of any stacked PR
- third bucket printed with its sum, a down-only ceiling of 27 so the next added workflow cannot land unread, and the clean line now states what remains unread
- tri harness scratch --gate is wired now that #2949 and #2955 have both landed; no paths filter and a push trigger on master, because emit-bitexact has no master baseline for exactly the opposite choice
87 changes: 85 additions & 2 deletions scripts/ci/check_pr_branch_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,31 @@
"secret-scan.yml",
"verilog-widths.yml",
"damage-negatives.yml",
# Added when the third bucket below was first printed. All three were in
# NEITHER list, so this check reported CLEAN without ever reading them --
# and two of them carried the exact defect it exists to detect.
"corpus-ratchet.yml",
"withdrawn-live-gate.yml",
"harness-scratch.yml",
)

# The two lists above are a partition ONLY of the files they name. Everything
# else in .github/workflows/ was read by nothing here, and the summary printed
# the two counts beside the file count without ever subtracting them: 15 + 4
# against 49 present, so 30 files were never examined and the last line still
# said CLEAN.
#
# Two of those 30 carried `pull_request: branches: [master]` -- the very defect
# this check exists to detect -- and one of them was `corpus-ratchet.yml`, which
# does not run at all on a stacked pull request and shows a green check list
# instead.
#
# A ceiling rather than a refusal, because 27 files cannot be classified in the
# commit that discovers them and a gate that is red on the day it lands teaches
# everyone to ignore red. It moves DOWN only: classify a file and lower this in
# the same commit, so the next unclassified workflow cannot hide in the slack.
MAX_UNCLASSIFIED = 27

# 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 = {
Expand Down Expand Up @@ -144,9 +167,55 @@ def main():
(name, f"{ev}.{k} = {cfg[k]!r} -- this gate does not run "
f"when a PR targets any other base"))

unclassified = sorted(present - set(MERGE_CRITICAL) - set(NOT_MERGE_CRITICAL))
both = sorted(set(MERGE_CRITICAL) & set(NOT_MERGE_CRITICAL))

# The same read, over the files no list names. Reported, not failed: whether
# one of these ought to block a merge is a human call. What is NOT a human
# call is whether anybody looked.
unclassified_filtered = []
for name in unclassified:
path = os.path.join(wf_dir, name)
try:
doc = load(path)
except Exception:
continue
on = on_block(doc)
if on is None:
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:
unclassified_filtered.append((name, f"{ev}.{k} = {cfg[k]!r}"))

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)}")
print(f"in NEITHER list, never read: {len(unclassified)}")
print(f"workflow files present: {len(present)}")
print(f" {len(MERGE_CRITICAL)} + {len(NOT_MERGE_CRITICAL)} + {len(unclassified)}"
f" = {len(MERGE_CRITICAL) + len(NOT_MERGE_CRITICAL) + len(unclassified)}"
f" (must equal {len(present)})")

if unclassified:
print(f"\nNOT CLASSIFIED ({len(unclassified)}, ceiling {MAX_UNCLASSIFIED}):")
for name in unclassified:
print(f" {name}")
print(" This check reads none of these. Put each in one of the two lists")
print(" above -- NOT_MERGE_CRITICAL carries its reason, so a later reader")
print(" can disagree with the reason rather than guess at the omission.")

if unclassified_filtered:
print(f"\nWORK LIST -- unclassified AND branch-filtered ({len(unclassified_filtered)}):")
for name, why in unclassified_filtered:
print(f" {name}\n {why}")
print(" Each does not run when a PR targets any base but master. That is")
print(" a gating hole if the workflow is merge-critical and a cost")
print(" decision if it is not, and nothing here can tell which.")
else:
print("\nUnclassified workflows carrying a pull_request branch filter: 0")

# 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
Expand Down Expand Up @@ -186,10 +255,24 @@ def main():
print("any `paths:` filter -- it selects by what changed, not by target.")
return 1

if both:
print(f"\nIN BOTH LISTS ({len(both)}): {', '.join(both)}")
print(" A file cannot be merge-critical and explicitly not. One list is")
print(" wrong and this check cannot say which.")
return 1

if len(unclassified) > MAX_UNCLASSIFIED:
print(f"\nUNCLASSIFIED ROSE {MAX_UNCLASSIFIED} -> {len(unclassified)}")
print(" A workflow was added and named in neither list, so this check")
print(" does not read it. Classify it and the ceiling holds; classify an")
print(" old one too and lower the ceiling in the same commit.")
return 1

if missing or hard:
return 1

print("\nCLEAN: no merge-critical workflow filters pull_request by branch.")
print(f"\nCLEAN: no merge-critical workflow filters pull_request by branch,"
f" and\n{len(unclassified)} file(s) remain unread at a ceiling of {MAX_UNCLASSIFIED}.")
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.")
Expand Down
Loading