diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ad0b461..29c0eafd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,37 @@ true until the next version shipped. ### Added +- `orphan-scan` is armed in the matrix runner, so a ledger row naming a deleted + check is refused rather than reported (#983, #1015). + + The gate answers "has this run a check the ledger has never seen". Nothing answered + the other half, "does the ledger name a check that no longer exists" -- and two rows + naming deleted checks sat in the committed ledger from #917 until #983 found them + while doing something else. The census counted both. `orphan-scan` was written for + exactly that and had NO CALLER IN THE TREE: tested, and unable to fire on anybody's + change. + + **One log per call, not all of them.** `_by_run` returns one entry per LOG, so + `len(runs) > 1` is true whenever more than one file is passed even when they came + from one matrix run. A call shaped like the gate's `$_led_logs` is refused by the + COUNT, before the before-log/after-log union the message names. So the runner loops. + + **`--orphans-only`, because a skip is not a deletion.** By default `rc=1` also covers + a part that skipped, and that is right: `not checked` is silence by construction and + out of scope, while `unprunable` is a gap in a part the run claimed and in it. But a + skip is box-dependent -- part 340 skips only where there is no non-root user to read + as -- and failing a matrix for that is a gate somebody turns off. The flag changes the + QUESTION rather than the severity of an answer, so `rc=1` never acquires a second + meaning, and the skipped part is still printed. + + Measured before wiring: only part 340 of the 46 has a real `check_skip` call; 320, + 400, 410 and 480 match on comments, a grep-based sweep and `printf` fixtures. Five + majors on one box gave `orphans=0, unprunable=0` -- but with `108 PASS and 0 SKIP`, + so the skip path never fired and that run tested nothing about skips. The skipped-part + case was forced synthetically instead, and the tool classifies the siblings + `unprunable` rather than orphans. + +- The piped-loop sweep reported a clean tree without reading one (#1033). - Userinfo in an object-store ENDPOINT was accepted, and the diagnostic told the operator to allow-list it (#995). diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index f7515224..39c2fd6b 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -908,6 +908,7 @@ harness_selftest 410-a-check-must-have-been-red a run that emits every row in it harness_selftest 410-a-check-must-have-been-red a run whose checks are all ledgered passes the gate 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a second major is ADDED to the set, sorted, not written over the first 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a second mutation ACCUMULATES rather than replacing the first 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red a skipped part is NOT an orphan, so --orphans-only returns 0 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a stale prior is named WITH its distance from HEAD 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red a verdict pgc_record cannot emit is an integrity failure 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red an absolute budget path resolves against git rather than shrugging 15;16;17;18;19 never - @@ -942,6 +943,7 @@ harness_selftest 410-a-check-must-have-been-red and it says so rather than decli harness_selftest 410-a-check-must-have-been-red and it says the change introduces the file rather than raising anything 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and it says the ref does not resolve, rather than claiming the file is new 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and it says why, rather than falling back to something weaker 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red and it still REPORTS the skipped part, so the gate cannot silence it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and never says a change introduces a file at a ref that is not there 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and none of them ends in a tab 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red and not against one that stayed green 15;16;17;18;19 never - @@ -971,16 +973,22 @@ harness_selftest 410-a-check-must-have-been-red and the summary keeps the two ap harness_selftest 410-a-check-must-have-been-red auto refuses when GITHUB_BASE_REF names a ref that is not here 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red auto uses the base ref when it resolves, and names it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red auto with no base ref and no upstream is an integrity failure 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red both conditions survive the reduction whatever order they arrive in (1 2) 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red both conditions survive the reduction whatever order they arrive in (2 1) 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red both failure arms fail the major 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but that suite is counted as not covered, which is the debt 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red but the gate says so, so the skip is visible rather than silent 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: a REAL orphan still returns 1 under --orphans-only 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a check the ledger claims for 15 that a PG15 run did not emit IS an orphan 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a prune that leaves nothing outstanding returns 0 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: a well-formed log still merges 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: all-clean logs leave both flags down 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: an all-PASS log still merges with no flag at all 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: and a clean run still returns 0 under it 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and that one did prune, so 0 is not a refusal in disguise 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: and the ledger really is shorter afterwards 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: on a covered major a new check is named and refused 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red control: one orphan among clean logs raises only the orphan flag 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the row in the part the run never mentioned SURVIVES the prune 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the same log merges with a different reason, so the refusal above is --mutation-across-two-checks and not the log 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red control: the same row with a real major is read without complaint 15;16;17;18;19 never - @@ -1007,6 +1015,7 @@ harness_selftest 410-a-check-must-have-been-red premise: that part has rows the harness_selftest 410-a-check-must-have-been-red premise: the budget is a tracked file too 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the budget was restored byte-exact 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the check has history before the rename 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the fixture is back to a state with rows to speak about 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the fixture ledger holds two rows 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the fixture log names checks the real ledger already knows 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the gate's status block was found in the runner 15;16;17;18;19 never - @@ -1020,10 +1029,12 @@ harness_selftest 410-a-check-must-have-been-red premise: the orphan about to be harness_selftest 410-a-check-must-have-been-red premise: the orphan now carries a date and a mutation 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the raised copy really does carry a higher ceiling 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the real budget is inside a git repository 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the runner's orphan reduction was extracted, not an empty range 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch prior really is three commits behind 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a committed ceiling and no upstream 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the scratch repo has a prior ceiling committed 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the skip-loop sweep is present, so its counts can be read 15;16;17;18;19 never - +harness_selftest 410-a-check-must-have-been-red premise: the skipped-part fixture still yields a finding by default 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red premise: the workflow file is where this part thinks it is 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red raising the ceiling above its committed value is refused 15;16;17;18;19 never - harness_selftest 410-a-check-must-have-been-red raising the ceiling in the tracked file is refused 15;16;17;18;19 never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 1a00d366..bdfcff19 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -54,4 +54,4 @@ suites_not_covered 249 # the number of attacked checks -- and it overcounts from the first moment this # ledger does the job it exists for. The gate prints both quantities side by side # (`rows=N | never observed red=M`) because they are different questions. -checks_never_observed_red 1209 +checks_never_observed_red 1220 diff --git a/test/pgc_ledger.py b/test/pgc_ledger.py index a1d66925..ab50f1ea 100755 --- a/test/pgc_ledger.py +++ b/test/pgc_ledger.py @@ -721,6 +721,22 @@ def cmd_orphan_scan(args): f"ledger rows -- every row must land in exactly one of the four") if not args.prune: + # THE FLAG CHANGES THE QUESTION, NOT THE SEVERITY OF AN ANSWER (#983, #1015). + # The default question is "is anything outstanding in the parts this run + # claimed", and a skipped part IS outstanding -- the run held those rows and + # failed to speak for them, which is why it differs from `not checked`: that + # one is silence by construction and out of scope, this one is a gap in a part + # the run claimed. So rc=1 covering both is right by default. + # + # A GATE NEEDS THE NARROWER QUESTION. `run_all_versions.sh` must refuse a row + # whose check no longer exists and must NOT refuse a box where a part skipped, + # since a skip is box-dependent and is not a deletion. Rather than give rc=1 a + # second meaning under a flag -- the same defect one level up -- this asks a + # different question, so rc=1 always means "the thing this invocation asked + # about was found". `unprunable` is still PRINTED above either way, so + # narrowing the gate's question cannot silence the report. + if getattr(args, "orphans_only", False): + return 1 if orphans else 0 return 1 if (orphans or unprunable) else 0 if unprunable: @@ -1136,6 +1152,10 @@ def main(argv=None): o = sub.add_parser("orphan-scan", help="refuse a ledger row no record in its own part matches") o.add_argument("--ledger", required=True) + o.add_argument("--orphans-only", action="store_true", + help="ask only whether a ledger row's check has been DELETED, so a " + "part that merely skipped does not make the exit status non-zero; " + "the skipped part is still reported") o.add_argument("--prune", action="store_true", help="remove orphan rows that carry no history; refuse the whole " "prune if any of them does") diff --git a/test/run_all_versions.sh b/test/run_all_versions.sh index 3f460709..3f297e3c 100755 --- a/test/run_all_versions.sh +++ b/test/run_all_versions.sh @@ -1451,6 +1451,69 @@ pgc_tally_suite() { # pgc_tally_suite NAME VERDICT LOGFILE esac fi + # THE OTHER HALF OF THE COMPARISON (#983, #1015). The gate answers "has this run a + # check the ledger has never seen". It cannot answer "does the ledger name a check + # that no longer exists", and nothing did: two rows naming deleted checks sat in the + # committed ledger from #917 until #983 found them by accident, and the census + # counted both. `orphan-scan` was written for exactly that and had no caller in the + # tree at all -- tested, and unable to fire on anybody's change. + # + # ONE LOG PER CALL, NOT ALL OF THEM. `_by_run` returns one entry per LOG, so + # `len(runs) > 1` is true whenever more than one file is passed even when they came + # from the same matrix run, and `cmd_orphan_scan` refuses outright. Shaping this call + # like the gate's `$_led_logs` is refused by the COUNT, before the union argument the + # message names. So it loops. + # + # --orphans-only, BECAUSE A SKIP IS NOT A DELETION. Without it rc=1 also covers a + # part that skipped, which is box-dependent -- part 340 skips only where there is no + # non-root user to read as -- and failing a matrix for that would be a gate somebody + # turns off. The skipped part is still PRINTED by the tool, so narrowing what the + # gate refuses on does not hide it. + if [ -n "${_led_logs:-}" ] && [ -f "$builddir/test/check_ledger.tsv" ]; then + # TWO FLAGS, NOT A COMBINED STATUS. The loop runs once per log, so the + # statuses have to be reduced, and every single-variable reduction loses + # one of the two answers: + # + # `|| _orph_fail=$?` OVERWRITES, so across 246 logs the operator is told + # about whichever failed LAST. Measured with a stub: + # orphan-then-toolfail reports "could not run" and + # hides a real orphan; the reverse hides the broken + # tool. The verdict is right either way and the + # DIAGNOSIS is wrong half the time, which is the + # defect the gate's own comment above says it fixed. + # keeping the MAX still hides a real orphan behind a tool failure. + # `|| { [ "$?" -gt ... ; }` is worse again: `$?` inside the braces is the + # `[` test, so it yields 0 for every input and reports + # CLEAN. Measured, both orders. + # + # Reported by @OffgridwithJD, who also measured that third one before + # suggesting it. Two independent conditions need two independent flags. + _orph_orphan=0 + _orph_broken=0 + # shellcheck disable=SC2086 + for _orph_log in $_led_logs; do + python3 "$builddir/test/pgc_ledger.py" orphan-scan --orphans-only \ + --ledger "$builddir/test/check_ledger.tsv" "$_orph_log" + _orph_rc=$? + case "$_orph_rc" in + 0) ;; + 1) _orph_orphan=1 ;; + *) _orph_broken=1 ;; + esac + done + if [ "$_orph_orphan" = 1 ]; then + echo " PG$major: the ledger names a check that no longer exists, which is" + echo " not a pass. Regenerate the ledger, or rename the row if the check" + echo " moved rather than went." + verfail=1 + fi + if [ "$_orph_broken" = 1 ]; then + echo " PG$major could not run the orphan scan on at least one log, which is" + echo " not a pass. That is separate from the line above, and both can be true." + verfail=1 + fi + fi + # How many of the suites counted as having RUN actually accounted for their # checks (#916). Counting a suite that never accounted among the suites that ran # is the overcount #447 added this line to stop, one level further down, and diff --git a/test/selftest/410-a-check-must-have-been-red.sh b/test/selftest/410-a-check-must-have-been-red.sh index 46bf621d..1c1e1ad5 100644 --- a/test/selftest/410-a-check-must-have-been-red.sh +++ b/test/selftest/410-a-check-must-have-been-red.sh @@ -527,6 +527,73 @@ check "control: a prune that leaves nothing outstanding returns 0" \ check "control: and that one did prune, so 0 is not a refusal in disguise" \ "$(wc -l < "$_lw/rc.tsv" | tr -d ' ')" "1" +# ---- --orphans-only: a QUESTION, not a change of severity (#983, #1015) ------ +# +# The default question is "is anything outstanding in the parts this run claimed", +# and a skipped part IS outstanding: the run held those rows and failed to speak for +# them. That is why rc=1 covers it and why the three arms above are right on the +# merits rather than only by precedent -- `not checked` is silence by construction +# and OUT of scope, `unprunable` is a gap in a part the run claimed and IN it. +# +# A GATE NEEDS THE NARROWER QUESTION. `run_all_versions.sh` must refuse a ledger row +# whose check no longer exists (#983) and must NOT refuse a box where a part skipped, +# because a skip is box-dependent and not a deletion. Those share rc=1 by design, so +# the flag changes WHAT IS ASKED rather than what a code means: with it, rc=1 is +# orphans and nothing else. rc=1 never acquires a second meaning. +_oo_pre="$(wc -l < "$_lw/rc.tsv" | tr -d ' ')" +check "premise: the fixture is back to a state with rows to speak about" \ + "$([ "$_oo_pre" -ge 1 ] && echo yes || echo no)" "yes" +: > "$_lw/oo.tsv" +_led_run merge --ledger "$_lw/oo.tsv" --date 2026-09-01 "$_lw/sk_full.log" >/dev/null +check "premise: the skipped-part fixture still yields a finding by default" \ + "$(_led_rc orphan-scan --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log")" "1" +check "a skipped part is NOT an orphan, so --orphans-only returns 0" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log")" "0" +check "and it still REPORTS the skipped part, so the gate cannot silence it" \ + "$(_led_run orphan-scan --orphans-only --ledger "$_lw/oo.tsv" "$_lw/sk_skipped.log" \ + | grep -c 'unprunable:')" "1" +check "control: a REAL orphan still returns 1 under --orphans-only" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/orph.tsv" "$_lw/o_after.log")" "1" +check "control: and a clean run still returns 0 under it" \ + "$(_led_rc orphan-scan --orphans-only --ledger "$_lw/orph.tsv" "$_lw/o_before.log")" "0" + +# ---- the WIRING, not the tool: the runner's reduction over N logs ------------ +# +# The arms above prove the TOOL. They say nothing about run_all_versions.sh, and +# the first version of that wiring was wrong in a way none of them could see: it +# reduced N per-log statuses with `|| _orph_fail=$?`, which OVERWRITES, so the +# operator was told about whichever log failed LAST. Measured with a stub, both +# orders: orphan-then-toolfail reported "could not run the orphan scan" and hid a +# real orphan; the reverse hid the broken tool. The verdict was right both times +# and the DIAGNOSIS was wrong half the time -- the same defect the gate's own +# comment says it fixed, which is how I know a comment does not transfer. +# +# Caught by @OffgridwithJD, who also measured that the obvious repair is worse: +# `|| { [ "$?" -gt "$_orph_fail" ] && _orph_fail=$?; }` yields 0 for EVERY input, +# because `$?` inside the braces is the `[` test, so it reports CLEAN. +# +# EVALS THE RUNNER'S OWN TEXT, the way link 5 of part 330 does, because an arm +# that re-derives the rule tests the world instead of the code. +_orw="$PGC_TESTDIR/run_all_versions.sh" +_orw_txt="$(awk '/^\t\t\tcase "\$_orph_rc" in$/{f=1} f{print} f&&/^\t\t\tesac$/{exit}' "$_orw")" +check "premise: the runner's orphan reduction was extracted, not an empty range" \ + "$(printf '%s\n' "$_orw_txt" | grep -c '_orph_orphan=1')" "1" + +for _ord in "1 2" "2 1"; do + _orph_orphan=0; _orph_broken=0 + for _orph_rc in $_ord; do eval "$_orw_txt"; done + check "both conditions survive the reduction whatever order they arrive in ($_ord)" \ + "$_orph_orphan$_orph_broken" "11" +done +# CONTROL: it does not simply set both flags for everything. +_orph_orphan=0; _orph_broken=0 +for _orph_rc in 0 0 0; do eval "$_orw_txt"; done +check "control: all-clean logs leave both flags down" "$_orph_orphan$_orph_broken" "00" +_orph_orphan=0; _orph_broken=0 +for _orph_rc in 0 1 0; do eval "$_orw_txt"; done +check "control: one orphan among clean logs raises only the orphan flag" \ + "$_orph_orphan$_orph_broken" "10" + # WHY THIS REPORTS AND DOES NOT GATE -- pinned to the PRECONDITION, not to one # instance of it. #