test: arm orphan-scan in the matrix runner (#983, #1015)

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).
 

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	-

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

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")

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