From 7a475b6a528b9d7d25e3f33db111cae7c19a9532 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 28 Jul 2026 14:10:03 -0500 Subject: [PATCH 1/2] =?UTF-8?q?feat(ci):=20add=20a=20gate-liveness=20check?= =?UTF-8?q?=20=E2=80=94=20prove=20each=20quality=20gate=20actually=20measu?= =?UTF-8?q?red=20something?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects across two of this workflow's gates spent months green. Two were gates measuring nothing: diff-coverage (a shallow fetch destroyed its merge base, and the empty report looked clean) and mutation (mutmut crashed before producing a mutant, and `|| true` made that green in 37s). The third was the close cousin — a gate that measured correctly and published a wrong number (`killed=0` from a grep for a line mutmut never prints). The rubric's anti-metric rule guards against trusting a NUMBER too much. Nothing guarded against trusting a GREEN CHECK THAT NEVER RAN. This is that control. Each measurement job now emits a JSON receipt as a job output; a new `liveness` job reads toJSON(needs) and rules on all four. It is the ONLY job here allowed to go red — no continue-on-error, no `|| true` — and it still blocks nothing, since these contexts are not required and must never become required. THE DISTINCTION IT RESTS ON: liveness is not "the gate found something". A clean repo legitimately has zero clones, and a check that fires on good news gets muted, which would leave us worse off than before. Receipts count units EXAMINED — files scanned, mutants processed, changed lines analysed — non-zero whenever the tool ran, whatever it concluded. A gate with nothing to measure passes by saying so explicitly, with a reason. VERIFIED AGAINST REAL DATA, not fixtures: complexity 256 files scanned / 122 findings, clone 234 files / 39 clones, mutation 461 mutants (87 killed, 19 survived, 355 not covered) parsed from the actual artifact of run 30308667584. All four failure modes replayed through the real CLI and caught. AN ADVERSARIAL REVIEW BEFORE MERGE FOUND THIS CONTROL CARRYING THE SAME WEAKNESS IT WAS BUILT TO CATCH, IN THREE PLACES. All fixed here: * A dead coverage gate could pass by claiming `not-applicable`. Reproduced: a valid-but-EMPTY coverage.xml makes diff-cover print the identical "No lines with coverage information in this diff" as the legitimate case, because its console template branches on whether any source has measured lines and never consults the diff. Same string, opposite meaning — the exact ambiguity this control exists to resolve, one layer up. Now the receipt proves coverage.xml measured files first, a missing file is `failed` rather than inapplicable, and verify() refuses a `not-applicable` receipt from a job whose result is `failure`. * An empty mutmut results file reported a FLAWLESS score. LISTED=0 gives killed=TOTAL, survived=0 — 461/461 killed, reconciling perfectly. A 100% kill rate with nothing listed is a tool that produced no output, not a triumph. Now `failed`. * The reconciliation was algebraically BLIND to the count it claimed to protect. Since killed = total - listed, the sum killed+survived+no_tests+other == total reduces to survived+no_tests+other == listed: total cancels, and any derived killed satisfies it. So the incident-3 test was replaying numbers the production path can no longer produce. Fixed by cross-checking killed against mutmut's OWN progress counter — two independent derivations that must agree (verified: derived 87 == reported 87). The sum's blindness is now asserted by its own test rather than quietly deleted, so nobody mistakes it for protection it does not give. Also here: `other` is counted independently rather than as a remainder (a remainder would make the sum true by construction), across mutmut's full status vocabulary so the check cannot redden a healthy run; a non-numeric killed count emits `failed` instead of malformed JSON; and the coverage receipt step gets BASE_REF, which it was missing. Slug rot carried along, since this commit is already in the file: the coverage job's "move it to the mirror if that cost bites" is gone — there is no mirror post-cutover. Adjacent to the sweep in #21, which does not touch this workflow. --- .github/workflows/quality-advisory.yml | 201 ++++++++++- scripts/quality/liveness.py | 384 ++++++++++++++++++++ tests/test_gate_liveness.py | 413 ++++++++++++++++++++++ tests/test_quality_advisory_invariants.py | 42 +++ 4 files changed, 1034 insertions(+), 6 deletions(-) create mode 100644 scripts/quality/liveness.py create mode 100644 tests/test_gate_liveness.py diff --git a/.github/workflows/quality-advisory.yml b/.github/workflows/quality-advisory.yml index 9c910783..3cc828f8 100644 --- a/.github/workflows/quality-advisory.yml +++ b/.github/workflows/quality-advisory.yml @@ -1,12 +1,17 @@ name: quality-advisory # Advisory-only quality-measurement gates from the Code Quality and Anti-Slop Standards rubric -# (docs/Code_Quality_Standards.md, signals 6-11). EVERY job here is advisory: it runs with `--exit-zero` -# (or continue-on-error) so it can NEVER fail a build or block a merge -- it surfaces findings for triage -# only. This is the rubric's "advisory-first, ratchet to blocking once the baseline is trusted" rule: -# raw complexity, coverage %, and clone counts are weak / gameable signals (rubric section 4.1) and must +# (docs/Code_Quality_Standards.md, signals 6-11). Every MEASUREMENT job here runs with `--exit-zero` +# (or continue-on-error) so it can never fail on what it finds -- it surfaces findings for triage only. +# This is the rubric's "advisory-first, ratchet to blocking once the baseline is trusted" rule: raw +# complexity, coverage %, and clone counts are weak / gameable signals (rubric section 4.1) and must # never be a hard gate. This workflow is NOT in branch protection; adding a job here cannot freeze the repo. # +# ONE DELIBERATE EXCEPTION: the `liveness` job at the bottom CAN go red, and is built to. It never rules +# on findings -- it rules on whether each gate proved it MEASURED anything at all. A red mark there still +# blocks nothing (this workflow holds no required contexts); it is simply the loudest honest way to say +# a gate has stopped working. See that job's own comment for the three incidents that motivated it. +# # Signals from the rubric (renumbered by tier -- docs/Code_Quality_Standards.md v0.7), each with how # it reaches a reviewer: # * signal 11 - complexity triage (ruff C901) -> BUILT (complexity job); PR-caused DELTA as @@ -68,6 +73,8 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + outputs: + receipt: ${{ steps.receipt.outputs.receipt }} steps: - name: Check out the source uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -131,6 +138,22 @@ jobs: --head c901-head.json \ --repo-root . \ --summary-file "$GITHUB_STEP_SUMMARY" + - name: Record gate liveness + id: receipt + if: always() + continue-on-error: true + run: | + # Units count what was EXAMINED, never what was found: a repo with zero functions over the + # threshold is good news, and a liveness check that fires on good news gets muted. + # `--show-files` lists exactly the files ruff would check, so it is non-zero iff ruff ran. + FILES="$(ruff check --select C901 --show-files messagefoundry 2>/dev/null | wc -l)" + FINDINGS="$(ruff check --select C901 --output-format=json --exit-zero messagefoundry \ + | python3 -c 'import json,sys; print(len(json.load(sys.stdin)))')" + python3 scripts/quality/liveness.py record \ + --signal complexity --status measured \ + --units "$FILES" --unit-name "files scanned" \ + --evidence "ruff C901 scanned $FILES files; $FINDINGS over the threshold" \ + --extra "{\"findings\":$FINDINGS}" clone: # Signal 9 - duplication / clone detection. Flags copy-pasted blocks (the "copy-instead-of-abstract" @@ -141,6 +164,8 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + outputs: + receipt: ${{ steps.receipt.outputs.receipt }} steps: - name: Check out the source uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -173,19 +198,37 @@ jobs: echo "workflow for why these are not annotated on the diff." } >> "$GITHUB_STEP_SUMMARY" fi + - name: Record gate liveness + id: receipt + if: always() + continue-on-error: true + run: | + # Files ANALYSED, not clones found -- zero clones is a healthy repo, not a dead gate. + REPORT=jscpd-report/jscpd-report.md + FILES="$(grep -oE 'duplicated lines in [0-9]+' "$REPORT" 2>/dev/null | grep -oE '[0-9]+$' || true)" + CLONES="$(grep -oE 'Found [0-9]+ exact clones' "$REPORT" 2>/dev/null | grep -oE '[0-9]+' || true)" + python3 scripts/quality/liveness.py record \ + --signal clone --status measured \ + --units "${FILES:-0}" --unit-name "files analysed" \ + --evidence "jscpd analysed ${FILES:-0} files; found ${CLONES:-0} clones" \ + --extra "{\"clones\":${CLONES:-0}}" coverage: # Signal 8 - diff-coverage visibility. Runs the suite under coverage, then reports coverage of the # lines CHANGED by this PR (diff-cover) -- never a whole-repo % gate, which the rubric (section 4.1) # calls a gameable signal. ADVISORY: --fail-under=0 + `|| true` so it never fails a build, and this # job is not a required status check. PR-only (needs a base ref to diff against). It re-runs the - # ubuntu suite (~a few min) per PR; gate it tighter or move it to the mirror if that cost bites. + # ubuntu suite (~a few min) per PR; add a paths filter if that cost bites. (This used to say + # "move it to the mirror" -- there is no mirror since the cutover; development happens directly + # on the public repo.) name: diff-coverage (advisory) if: github.event_name == 'pull_request' runs-on: ubuntu-latest timeout-minutes: 20 permissions: contents: read + outputs: + receipt: ${{ steps.receipt.outputs.receipt }} steps: - name: Check out the source uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -242,7 +285,7 @@ jobs: # line flagged is a line this PR changed, so there is no pre-existing-debt noise by construction. # The console report still prints, so the job log is unchanged. diff-cover coverage.xml --compare-branch="origin/$BASE_REF" --fail-under=0 \ - --format "github-annotations:notice,markdown:diff-cover.md" || true + --format "github-annotations:notice,markdown:diff-cover.md" 2>&1 | tee diff-cover-console.txt || true # `-s` not `-f`: diff-cover creates and truncates this file BEFORE it can fail, so a crashed # run leaves a 0-byte report. Testing for existence alone would append an empty section that # looks like a clean result -- the exact silent-success shape this workflow keeps producing. @@ -258,6 +301,48 @@ jobs: else echo "::notice title=Diff coverage unavailable::diff-cover produced no report (see the job log)" fi + - name: Record gate liveness + id: receipt + if: always() + # BASE_REF is needed here too -- without it the evidence string read "against origin/". + env: + BASE_REF: ${{ github.base_ref }} + continue-on-error: true + run: | + # THE DISTINCTION THIS WHOLE CHECK RESTS ON, and the place it is easiest to get wrong. + # + # "No lines with coverage information in this diff" is a real, correct outcome -- it is what + # PRs #18 and #19 produced, because neither touched measured source. But diff-cover prints + # that SAME line when coverage.xml itself measured nothing (its console template branches on + # whether any source has measured lines, and never consults the diff size). So the message + # alone cannot tell "nothing to measure" from "the measurement is dead" -- exactly the + # ambiguity this control exists to resolve, one layer up. Reproduced with a valid-but-empty + # coverage.xml against a diff that really did change six lines. + # + # So: prove coverage.xml measured something before accepting the not-applicable branch, and + # treat a MISSING coverage.xml as a dead gate (failed), never as inapplicable -- the same way + # the mutation job treats a missing mutmut-counts.env. + if [ ! -s coverage.xml ]; then + python3 scripts/quality/liveness.py record --signal coverage --status failed \ + --reason "coverage.xml was not produced; the measurement never happened" + exit 0 + fi + MEASURED_FILES="$(grep -c '/dev/null; then + python3 scripts/quality/liveness.py record --signal coverage --status failed \ + --reason "coverage.xml contains no measured files; diff-cover had nothing to compare against" + exit 0 + fi + if grep -q "No lines with coverage information" diff-cover-console.txt 2>/dev/null; then + python3 scripts/quality/liveness.py record --signal coverage --status not-applicable \ + --reason "no lines with coverage information in this diff; coverage.xml measured ${MEASURED_FILES} files, so the tooling is live and this PR simply changed no measured source" + else + LINES="$(grep -oE 'Total:[[:space:]]+[0-9]+' diff-cover-console.txt 2>/dev/null \ + | grep -oE '[0-9]+' | head -1 || true)" + python3 scripts/quality/liveness.py record --signal coverage --status measured \ + --units "${LINES:-0}" --unit-name "changed lines analysed" \ + --evidence "diff-cover reported on ${LINES:-0} changed lines against origin/${BASE_REF}; coverage.xml measured ${MEASURED_FILES} files" + fi mutation: # Signal 7 - mutation testing (advisory; the HIGHEST-leverage gate -- it adversarially checks the @@ -273,6 +358,8 @@ jobs: timeout-minutes: 30 permissions: contents: read + outputs: + receipt: ${{ steps.receipt.outputs.receipt }} steps: - name: Check out the source uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -331,6 +418,21 @@ jobs: LISTED="$(grep -cE '^[[:space:]]+\S+: ' mutmut-results.txt || true)" SURVIVED="$(grep -c ': survived' mutmut-results.txt || true)" NOTESTS="$(grep -c ': no tests' mutmut-results.txt || true)" + # Counted INDEPENDENTLY, never as `LISTED - SURVIVED - NOTESTS`. Deriving it as a + # remainder would make the liveness reconciliation tautological -- the parts would add up + # by construction and the check could never fire. + # + # The alternation must cover EVERY non-killed status mutmut can print, or the sum breaks + # on a healthy run and the liveness job reddens for no reason. A check whose only + # reachable failure is a false positive gets muted within a month, which would leave us + # worse off than having no check. + OTHER="$(grep -cE ': (timeout|suspicious|skipped|segfault|not checked|caught by type check|check was interrupted by user)$' mutmut-results.txt || true)" + # mutmut's OWN killed counter, off its final progress line. KILLED above is derived a + # completely different way (total minus listed); the liveness check reconciles the two. + # Two independent derivations agreeing is what actually protects the number -- the sum + # check alone cannot, because TOTAL cancels out of it algebraically. + KILLED_REPORTED="$(tr '\r' '\n' < mutmut-run.txt | grep -oE '🎉 [0-9]+' | tail -1 \ + | grep -oE '[0-9]+' || true)" if [ -n "$TOTAL" ] && [ "$TOTAL" -ge "$LISTED" ] 2>/dev/null; then KILLED=$((TOTAL - LISTED)) else @@ -357,6 +459,16 @@ jobs: echo "Advisory only. A survivor is a hint to strengthen an assertion, not a defect —" echo "and mutation score is a poor single number (rubric §2), so this never gates." } >> "$GITHUB_STEP_SUMMARY" + # Stash the breakdown for the liveness receipt in the next step. + { + echo "MUT_TOTAL=$TOTAL" + echo "MUT_KILLED=$KILLED" + echo "MUT_SURVIVED=$SURVIVED" + echo "MUT_NOTESTS=$NOTESTS" + echo "MUT_OTHER=$OTHER" + echo "MUT_LISTED=$LISTED" + echo "MUT_KILLED_REPORTED=${KILLED_REPORTED:-}" + } > mutmut-counts.env else echo "::warning title=Mutation run failed::mutmut exited non-zero — see the job log" { @@ -369,6 +481,47 @@ jobs: echo '```' } >> "$GITHUB_STEP_SUMMARY" fi + - name: Record gate liveness + id: receipt + if: always() + continue-on-error: true + run: | + if [ ! -s mutmut-counts.env ]; then + # mutmut crashed. This is the 2.5.1 case verbatim -- and it must NOT be silent. + python3 scripts/quality/liveness.py record --signal mutation --status failed \ + --reason "mutmut produced no counts; the run did not complete (see the job log)" + exit 0 + fi + . ./mutmut-counts.env + # `?` means the mutant total was unreadable, so KILLED is underivable. Interpolating it into + # the --extra JSON would emit `"killed":?` -- malformed, and it would surface as "receipt is + # not valid JSON" rather than the real diagnosis. + case "$MUT_KILLED" in + ''|*[!0-9]*) + python3 scripts/quality/liveness.py record --signal mutation --status failed \ + --reason "could not read mutmut's mutant total from the progress line; the killed count is underivable" + exit 0 + ;; + esac + # An EMPTY results file yields LISTED=0, hence killed=TOTAL, survived=0 -- a flawless score + # that reconciles perfectly. A 100% kill rate with nothing listed is not a triumph, it is a + # tool that produced no output. Refuse to report it as success. + if [ "${MUT_LISTED:-0}" -eq 0 ] && [ "${MUT_TOTAL:-0}" -gt 0 ] 2>/dev/null; then + python3 scripts/quality/liveness.py record --signal mutation --status failed \ + --reason "mutmut listed no mutants at all while reporting $MUT_TOTAL processed; a perfect score here means the results file was empty, not that every mutant died" + exit 0 + fi + EXTRA="{\"killed\":$MUT_KILLED,\"survived\":$MUT_SURVIVED,\"no_tests\":$MUT_NOTESTS,\"other\":$MUT_OTHER" + case "$MUT_KILLED_REPORTED" in + ''|*[!0-9]*) ;; + *) EXTRA="$EXTRA,\"killed_reported\":$MUT_KILLED_REPORTED" ;; + esac + EXTRA="$EXTRA}" + python3 scripts/quality/liveness.py record \ + --signal mutation --status measured \ + --units "$MUT_TOTAL" --unit-name "mutants processed" \ + --evidence "mutmut processed $MUT_TOTAL mutants; $MUT_LISTED listed as non-killed" \ + --extra "$EXTRA" - name: Upload the mutmut results (advisory) if: always() continue-on-error: true @@ -386,3 +539,39 @@ jobs: # Deliberately `warn`, not `ignore`: if mutmut ever renames its cache, that must be visible in # the log rather than silently swallowed. if-no-files-found: warn + + liveness: + # THE META-GATE. Every other job here is built so it can never fail; this one is built so it CAN. + # + # Three separate signals in this workflow spent months reporting success while measuring nothing: + # diff-coverage (a shallow fetch destroyed its merge base, and the empty report looked clean), + # mutation (mutmut 2.5.1 crashed before producing a mutant, and `|| true` made that green in 37s), + # and the killed count (a grep for a line mutmut never prints, so a healthy run said "Killed 0"). + # The rubric's anti-metric rule guards against trusting a NUMBER too much. Nothing guarded against + # trusting a GREEN CHECK THAT NEVER RAN. This job is that control. + # + # It reads each job's liveness receipt and demands proof of EXECUTION -- a count of things + # examined -- or an explicit, reasoned declaration that there was nothing to measure. It does not + # demand findings: a clean repo legitimately has zero clones, and a gate that fires on good news + # gets muted, which would leave us worse off than before. + # + # DELIBERATELY has no `continue-on-error` and no `|| true`. A dead gate should be loud, and a red + # mark here blocks nothing -- this job is not, and must never become, a required status check. + name: gate liveness (advisory) + if: always() + needs: [complexity, clone, coverage, mutation] + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - name: Check out the source + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - name: Verify every gate proved it measured something + # `needs` carries each job's result AND its receipt output. Routed through `env:` rather than + # interpolated into the shell body -- the receipts are job-authored strings, and expanding + # them into a command line is the template-injection shape zizmor exists to catch. + env: + NEEDS_JSON: ${{ toJSON(needs) }} + run: python3 scripts/quality/liveness.py verify --needs-json "$NEEDS_JSON" diff --git a/scripts/quality/liveness.py b/scripts/quality/liveness.py new file mode 100644 index 00000000..9527f9ae --- /dev/null +++ b/scripts/quality/liveness.py @@ -0,0 +1,384 @@ +"""Gate liveness: prove each advisory quality job actually MEASURED something. + +Three defects across two of quality-advisory.yml's gates, all green the whole time. Two are the same +failure -- a gate measuring nothing -- and the third is its close cousin, a gate that measured +correctly and published a wrong number: + + * diff-coverage MEASURED NOTHING -- a `git fetch --depth=1` re-shallowed the clone and destroyed + diff-cover's merge base. diff-cover truncates its report file before it can fail, so `|| true` + swallowed the error and the summary rendered an empty section reading as "nothing uncovered". + * mutation MEASURED NOTHING -- `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 before + generating a single mutant. `|| true` made the job report success in 37 seconds, for months. + * mutation PUBLISHED A WRONG NUMBER -- `grep -c ': killed'` searched for a line mutmut never + prints, so a healthy 461-mutant run reported "Killed 0". Execution proof alone would pass this + one; it needs the reconciliation rules below. + +The rubric's anti-metric rule (Code_Quality_Standards.md section 4.1) guards against trusting a +NUMBER too much. Nothing guarded against trusting a GREEN CHECK THAT NEVER RAN. This is that control. + +THE DISTINCTION THAT MAKES THIS WORK: liveness is not "the gate found something". A clean repository +legitimately has zero clones and zero new complexity, and demanding a non-zero finding count would +make the check fire on good news. Liveness is *proof the measurement was performed* -- a count of +units EXAMINED (mutants processed, files scanned, changed lines analysed), which is non-zero whenever +the tool actually ran, regardless of what it concluded. + +A job may also honestly declare it had nothing to measure (`not-applicable` + a reason). That is the +diff-coverage case on a PR touching no covered code: an explicit "no lines with coverage information +in this diff" PASSES, while a silent empty report FAILS. Saying why is the whole difference. + +Two subcommands: + record -- a job emits its receipt (one compact JSON line, written to $GITHUB_OUTPUT) + verify -- the liveness job reads every receipt out of `toJSON(needs)` and rules on them +""" + +import argparse +import json +import os +import sys +from typing import Any + +# Signals that must account for themselves. A job absent from this map is not checked; a job here +# that RAN and produced no receipt is a liveness violation -- that is the "green check that never +# ran" case, and it is the whole reason this file exists. +EXPECTED_SIGNALS = { + "complexity": "cyclomatic-complexity triage (ruff C901)", + "clone": "duplication / clone detection (jscpd)", + "coverage": "diff-coverage visibility (diff-cover)", + "mutation": "test-signal proof (mutmut)", +} + +MEASURED = "measured" +NOT_APPLICABLE = "not-applicable" +FAILED = "failed" +_STATUSES = (MEASURED, NOT_APPLICABLE, FAILED) + +# Job results that mean "this job did not run", so no receipt is owed. Anything else -- including +# `failure` -- owes one: a job that crashed still has to be visible as a dead gate rather than +# quietly absent. +_DID_NOT_RUN = ("skipped", "cancelled") + + +class Violation(Exception): + """A liveness rule was broken. Carries the human-facing message.""" + + +def _fail(signal: str, message: str) -> dict[str, str]: + return {"signal": signal, "message": message} + + +# -------------------------------------------------------------------------------------------- +# record +# -------------------------------------------------------------------------------------------- + + +def build_receipt( + signal: str, + status: str, + units: int | None, + unit_name: str, + evidence: str, + reason: str, + extra: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Assemble and validate one receipt. Raises rather than emitting a malformed one.""" + if signal not in EXPECTED_SIGNALS: + raise Violation(f"unknown signal {signal!r}; expected one of {sorted(EXPECTED_SIGNALS)}") + if status not in _STATUSES: + raise Violation(f"unknown status {status!r}; expected one of {_STATUSES}") + + receipt: dict[str, Any] = {"signal": signal, "status": status} + if status == MEASURED: + if units is None or units <= 0: + raise Violation( + f"{signal}: status={MEASURED} requires units > 0 (got {units!r}). Units count what " + "was EXAMINED, not what was found -- if the tool ran, this is non-zero." + ) + if not evidence.strip(): + raise Violation( + f"{signal}: status={MEASURED} requires evidence naming what proves it ran" + ) + receipt["units"] = units + receipt["unit_name"] = unit_name or "units" + receipt["evidence"] = evidence.strip() + else: + if not reason.strip(): + raise Violation( + f"{signal}: status={status} requires a reason. An unexplained silent gate is exactly " + "what this check exists to catch." + ) + receipt["reason"] = reason.strip() + if extra: + receipt.update(extra) + return receipt + + +# -------------------------------------------------------------------------------------------- +# verify +# -------------------------------------------------------------------------------------------- + + +def _consistency_violations(receipt: dict[str, Any]) -> list[str]: + """Cross-field checks: a gate can run correctly and still report a wrong derived number. + + TWO checks, because the obvious one is weaker than it looks. The workflow derives + `killed = total - listed`, so asserting `killed + survived + no_tests + other == total` reduces + algebraically to `survived + no_tests + other == listed` -- total cancels, and the killed count + is never validated at all. That sum only proves every LISTED mutant carries a recognised status. + Useful (it catches mutmut adding a status we do not classify) but blind to the exact bug it was + written for. + + So the killed count is reconciled against mutmut's OWN counter, parsed from its progress line -- + a genuinely independent measurement that a derived number cannot satisfy by construction. + + The breakdown is MANDATORY for mutation. Returning early on a missing key would make the whole + check opt-in from the receipt side: thin the payload and the reconciliation silently stops + running, which is this control's own failure mode one level up. + """ + problems: list[str] = [] + if receipt.get("signal") != "mutation" or receipt.get("status") != MEASURED: + return problems + + total = receipt.get("units") + parts = {k: receipt.get(k) for k in ("killed", "survived", "no_tests", "other")} + missing = [k for k, v in parts.items() if not isinstance(v, int)] + if missing: + problems.append( + f"mutation receipt carries no usable breakdown (missing/non-numeric: {', '.join(missing)}), " + "so the parts-add-up check could not run. A signal with a known breakdown must report it." + ) + return problems + + summed = sum(int(v) for v in parts.values()) + if summed != total: + detail = ", ".join(f"{k}={v}" for k, v in parts.items()) + problems.append( + f"mutation breakdown does not reconcile: {detail} sums to {summed}, but {total} mutants " + "were processed -- a mutant carries a status nothing here classifies." + ) + + # The independent cross-check. `killed_reported` comes from mutmut's own progress counter; the + # workflow derives `killed` a completely different way (total minus listed). Agreement between + # two independent derivations is what actually protects the number. + reported = receipt.get("killed_reported") + if ( + isinstance(reported, int) + and isinstance(parts["killed"], int) + and reported != parts["killed"] + ): + problems.append( + f"killed count disagrees with mutmut's own counter: derived {parts['killed']}, mutmut " + f"reported {reported}. One of the two is wrong (this is the shape of the `killed=0` bug)." + ) + return problems + + +def verify(needs: dict[str, Any]) -> tuple[list[dict[str, str]], list[dict[str, Any]]]: + """Rule on every expected signal. Returns (violations, accepted receipts).""" + violations: list[dict[str, str]] = [] + accepted: list[dict[str, Any]] = [] + + for signal, description in sorted(EXPECTED_SIGNALS.items()): + job = needs.get(signal) + if job is None: + violations.append( + _fail( + signal, + f"no job named {signal!r} among this workflow's needs -- the signal was renamed " + f"or removed without updating EXPECTED_SIGNALS ({description}).", + ) + ) + continue + + result = str(job.get("result", "")).lower() + if result in _DID_NOT_RUN: + continue # legitimately did not run (e.g. coverage is PR-only) + + raw = (job.get("outputs") or {}).get("receipt", "") + if not str(raw).strip(): + violations.append( + _fail( + signal, + f"job finished with result={result!r} but emitted NO liveness receipt. This is " + f"the silent-gate case: {description} reported a conclusion without recording " + "that it measured anything.", + ) + ) + continue + + try: + receipt = json.loads(raw) + except json.JSONDecodeError as exc: + violations.append( + _fail(signal, f"liveness receipt is not valid JSON ({exc}): {raw[:200]!r}") + ) + continue + + status = receipt.get("status") + if status == MEASURED: + units = receipt.get("units") + if not isinstance(units, int) or units <= 0: + violations.append( + _fail( + signal, + f"claims status={MEASURED} but units={units!r} -- nothing was examined", + ) + ) + continue + for problem in _consistency_violations(receipt): + violations.append(_fail(signal, problem)) + accepted.append(receipt) + elif status == NOT_APPLICABLE: + if not str(receipt.get("reason", "")).strip(): + violations.append(_fail(signal, f"status={NOT_APPLICABLE} with no reason given")) + continue + if result == "failure": + # A job that DIED has no standing to declare itself inapplicable. Without this, the + # softest possible receipt launders a hard failure into a pass -- and every step in + # these jobs is continue-on-error, so nothing else would be red either. + violations.append( + _fail( + signal, + f"job result={result!r} but the receipt claims {NOT_APPLICABLE} " + f"({receipt.get('reason')!r}). A failed gate is dead, not inapplicable.", + ) + ) + continue + accepted.append(receipt) + elif status == FAILED: + violations.append( + _fail( + signal, f"the gate reported ITSELF dead: {receipt.get('reason', '(no reason)')}" + ) + ) + else: + violations.append(_fail(signal, f"unknown status {status!r} in receipt")) + + return violations, accepted + + +def render_summary(violations: list[dict[str, str]], accepted: list[dict[str, Any]]) -> str: + lines = ["## Gate liveness", ""] + if violations: + lines += [ + f"**{len(violations)} gate(s) reported a conclusion without proving they measured " + "anything.** A green advisory check that never ran is worse than a red one — it looks " + "like good news.", + "", + "| Signal | Problem |", + "| --- | --- |", + ] + for v in violations: + lines.append(f"| `{v['signal']}` | {v['message']} |") + lines.append("") + else: + lines += ["Every gate that ran proved it measured something. ✅", ""] + + if accepted: + lines += ["| Signal | Status | Examined | Evidence |", "| --- | --- | --- | --- |"] + for r in sorted(accepted, key=lambda x: str(x.get("signal"))): + if r.get("status") == MEASURED: + examined = f"{r.get('units')} {r.get('unit_name', 'units')}" + note = str(r.get("evidence", "")) + else: + examined = "—" + note = f"*not applicable:* {r.get('reason', '')}" + lines.append(f"| `{r.get('signal')}` | {r.get('status')} | {examined} | {note} |") + lines.append("") + + lines += [ + "Liveness is proof a measurement happened, not proof it found anything — a clean repo " + "legitimately reports zero findings. A gate with nothing to measure passes by saying so.", + "", + ] + return "\n".join(lines) + + +def _annotation(violation: dict[str, str]) -> str: + message = violation["message"].replace("%", "%25").replace("\r", "%0D").replace("\n", "%0A") + return f"::error title=Dead quality gate ({violation['signal']})::{message}" + + +# -------------------------------------------------------------------------------------------- +# CLI +# -------------------------------------------------------------------------------------------- + + +def _write_output(name: str, value: str) -> None: + path = os.environ.get("GITHUB_OUTPUT") + if not path: + print(f"{name}={value}") + return + with open(path, "a", encoding="utf-8") as handle: + handle.write(f"{name}={value}\n") + + +def _append_summary(text: str) -> None: + path = os.environ.get("GITHUB_STEP_SUMMARY") + if not path: + sys.stderr.write(text) + return + with open(path, "a", encoding="utf-8") as handle: + handle.write(text) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + sub = parser.add_subparsers(dest="command", required=True) + + rec = sub.add_parser("record", help="emit one liveness receipt") + rec.add_argument("--signal", required=True, choices=sorted(EXPECTED_SIGNALS)) + rec.add_argument("--status", required=True, choices=_STATUSES) + rec.add_argument("--units", type=int, default=None, help="how many things were EXAMINED") + rec.add_argument("--unit-name", default="units") + rec.add_argument("--evidence", default="", help="what proves the tool actually ran") + rec.add_argument("--reason", default="", help="why there was nothing to measure") + rec.add_argument("--extra", default="", help="optional JSON object merged into the receipt") + + ver = sub.add_parser("verify", help="rule on every gate's receipt") + ver.add_argument("--needs-json", required=True, help="the workflow's toJSON(needs) blob") + + args = parser.parse_args(argv) + + if args.command == "record": + extra = json.loads(args.extra) if args.extra.strip() else None + receipt = build_receipt( + signal=args.signal, + status=args.status, + units=args.units, + unit_name=args.unit_name, + evidence=args.evidence, + reason=args.reason, + extra=extra, + ) + # Compact + single-line: this travels through a GitHub Actions job output. + _write_output("receipt", json.dumps(receipt, separators=(",", ":"))) + return 0 + + try: + needs = json.loads(args.needs_json) + except json.JSONDecodeError as exc: + # Fail loud: an unreadable needs blob means this check verified NOTHING, which is precisely + # the condition it exists to detect. Never pass silently here. + print(f"::error title=Gate liveness could not run::needs JSON did not parse ({exc})") + _append_summary( + "## Gate liveness\n\n**Could not run** — the `needs` payload did not parse, so no gate " + "was checked. Treated as a failure, because a liveness check that silently verifies " + "nothing is the exact bug it guards against.\n" + ) + return 2 + + violations, accepted = verify(needs) + for violation in violations: + print(_annotation(violation)) + _append_summary(render_summary(violations, accepted)) + + for receipt in sorted(accepted, key=lambda x: str(x.get("signal"))): + print(f"ok: {receipt.get('signal')} -> {json.dumps(receipt, separators=(',', ':'))}") + + # Non-zero on violation. This job is advisory and NOT a required check, so a red mark here + # blocks no merge -- it is simply the loudest honest way to say a gate stopped measuring. + return 1 if violations else 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/tests/test_gate_liveness.py b/tests/test_gate_liveness.py new file mode 100644 index 00000000..b31ded87 --- /dev/null +++ b/tests/test_gate_liveness.py @@ -0,0 +1,413 @@ +"""Tests for scripts/quality/liveness.py. + +The core of this file is a replay of the three real incidents that motivated the check. A liveness +gate that cannot catch the failures it was built for is itself a dead gate, so each historical bug is +reconstructed from what CI actually reported at the time and asserted to be caught. + +Equally important, and easier to get wrong: the check must NOT fire on good news. A clean repository +legitimately reports zero clones and zero new complexity, and a PR touching no covered code +legitimately has no diff coverage. Those cases are asserted to pass. +""" + +import importlib.util +import json +import sys +from pathlib import Path +from types import ModuleType + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] + + +def _load() -> ModuleType: + path = _ROOT / "scripts" / "quality" / "liveness.py" + assert path.is_file(), f"liveness script not found at {path}" + spec = importlib.util.spec_from_file_location("gate_liveness", path) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = mod + spec.loader.exec_module(mod) + return mod + + +liveness = _load() + + +def _job(result: str = "success", receipt: dict | None = None) -> dict: + outputs = {"receipt": json.dumps(receipt)} if receipt is not None else {} + return {"result": result, "outputs": outputs} + + +def _healthy(signal: str, units: int = 10, **extra) -> dict: + return { + "signal": signal, + "status": "measured", + "units": units, + "unit_name": "things", + "evidence": f"{signal} ran", + **extra, + } + + +def _healthy_mutation() -> dict: + """Mutation is the one signal with a MANDATORY breakdown, so its healthy receipt carries one. + Numbers are the real ones from run 30315798347: 87 + 19 + 355 + 0 = 461.""" + return _healthy( + "mutation", units=461, killed=87, survived=19, no_tests=355, other=0, killed_reported=87 + ) + + +def _all_healthy() -> dict: + needs = {s: _job(receipt=_healthy(s)) for s in liveness.EXPECTED_SIGNALS} + needs["mutation"] = _job(receipt=_healthy_mutation()) + return needs + + +# -------------------------------------------------------------------------------------------- +# Replay of the three real incidents. These are the tests that justify the check existing. +# -------------------------------------------------------------------------------------------- + + +def test_incident_mutation_crashed_but_job_was_green() -> None: + """mutmut 2.5.1 crashed on Python 3.14 before generating a single mutant. `|| true` made the job + report SUCCESS in 37 seconds, and it stayed green for months (run 30248096425).""" + needs = _all_healthy() + needs["mutation"] = _job(result="success") # green, and no receipt at all + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["mutation"] + assert "NO liveness receipt" in violations[0]["message"] + + +def test_incident_diff_coverage_produced_an_empty_report_silently() -> None: + """The shallow-fetch bug: diff-cover truncated its report on open, then died. `|| true` ate the + error, the `-f` guard passed on the 0-byte file, and the summary looked clean. If the job + naively records that as 'measured', units=0 must expose it.""" + needs = _all_healthy() + needs["coverage"] = _job(receipt={**_healthy("coverage"), "units": 0}) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["coverage"] + assert "nothing was examined" in violations[0]["message"] + + +def test_incident_killed_count_did_not_reconcile() -> None: + """The `grep -c ': killed'` bug. The gate genuinely RAN -- 461 mutants really were processed -- + so execution proof alone passes it. CI reported killed=0 survived=19 no_tests=355, summing to + 374 rather than 461.""" + needs = _all_healthy() + needs["mutation"] = _job( + receipt=_healthy("mutation", units=461, killed=0, survived=19, no_tests=355, other=0) + ) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["mutation"] + assert "does not reconcile" in violations[0]["message"] + assert "374" in violations[0]["message"] and "461" in violations[0]["message"] + + +def test_the_sum_check_alone_cannot_validate_the_killed_count() -> None: + """HONESTY TEST -- documents a real limit rather than papering over it. + + The workflow derives `killed = total - listed`, so `killed + survived + no_tests + other == total` + reduces to `survived + no_tests + other == listed`: total cancels, and any derived killed value + satisfies it. This asserts that blindness explicitly, so nobody mistakes the sum for protection + of the killed count. The independent cross-check below is what actually protects it. + """ + # A wildly wrong killed value that is nonetheless self-consistent with listed = 19+355+0 = 374. + needs = _all_healthy() + needs["mutation"] = _job( + receipt=_healthy( + "mutation", units=374 + 999, killed=999, survived=19, no_tests=355, other=0 + ) + ) + + violations, _ = liveness.verify(needs) + + assert violations == [], "the sum check is blind here -- that is the point of this test" + + +def test_a_killed_count_disagreeing_with_mutmuts_own_counter_is_caught() -> None: + """The check that DOES protect the killed count: two independent derivations must agree. + `killed` is total-minus-listed; `killed_reported` is mutmut's own progress counter.""" + needs = _all_healthy() + needs["mutation"] = _job( + receipt=_healthy( + "mutation", units=461, killed=0, survived=19, no_tests=355, other=0, killed_reported=87 + ) + ) + + violations, _ = liveness.verify(needs) + + messages = " ".join(v["message"] for v in violations) + assert "disagrees with mutmut's own counter" in messages + assert "derived 0" in messages and "reported 87" in messages + + +def test_a_missing_breakdown_is_a_violation_not_a_silent_skip() -> None: + """Otherwise the reconciliation is opt-in from the receipt side: thin the payload and the check + quietly stops running -- this control's own failure mode, one level up.""" + needs = _all_healthy() + needs["mutation"] = _job(receipt=_healthy("mutation", units=461)) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["mutation"] + assert "no usable breakdown" in violations[0]["message"] + + +def test_the_corrected_killed_count_reconciles() -> None: + """The real numbers from run 30315798347 after the fix: 87 + 19 + 355 = 461, and mutmut's own + counter agrees with the derived 87.""" + needs = _all_healthy() + needs["mutation"] = _job( + receipt=_healthy( + "mutation", units=461, killed=87, survived=19, no_tests=355, other=0, killed_reported=87 + ) + ) + + violations, accepted = liveness.verify(needs) + + assert violations == [] + assert any(r["signal"] == "mutation" for r in accepted) + + +# -------------------------------------------------------------------------------------------- +# It must not fire on good news. +# -------------------------------------------------------------------------------------------- + + +def test_a_clean_repo_with_zero_findings_still_passes() -> None: + """Liveness counts what was EXAMINED, not what was found. A repo with no clones at all must not + trip the check -- otherwise the gate fires on good news and gets muted.""" + needs = _all_healthy() + needs["clone"] = _job( + receipt={ + "signal": "clone", + "status": "measured", + "units": 234, + "unit_name": "files analysed", + "evidence": "jscpd analysed 234 files, found 0 clones", + "clones": 0, + } + ) + + violations, _ = liveness.verify(needs) + assert violations == [] + + +def test_an_honest_not_applicable_passes() -> None: + """The real diff-coverage case on PRs #18 and #19: no lines with coverage information, because + neither PR touched `messagefoundry/`. Saying so is the difference between this and the silent + empty report above.""" + needs = _all_healthy() + needs["coverage"] = _job( + receipt={ + "signal": "coverage", + "status": "not-applicable", + "reason": "no lines with coverage information in this diff", + } + ) + + violations, accepted = liveness.verify(needs) + + assert violations == [] + assert any(r["status"] == "not-applicable" for r in accepted) + + +def test_not_applicable_without_a_reason_is_rejected() -> None: + """Otherwise 'not-applicable' becomes a free pass and the check is worthless.""" + needs = _all_healthy() + needs["coverage"] = _job( + receipt={"signal": "coverage", "status": "not-applicable", "reason": " "} + ) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["coverage"] + assert "no reason given" in violations[0]["message"] + + +def test_a_skipped_job_owes_no_receipt() -> None: + """coverage is PR-only, so it is legitimately skipped on the nightly cron.""" + needs = _all_healthy() + needs["coverage"] = {"result": "skipped", "outputs": {}} + + violations, _ = liveness.verify(needs) + assert violations == [] + + +def test_a_failed_job_still_owes_a_receipt() -> None: + """A crashed job must be visible as a dead gate, not quietly excused.""" + needs = _all_healthy() + needs["clone"] = _job(result="failure") + + violations, _ = liveness.verify(needs) + assert [v["signal"] for v in violations] == ["clone"] + + +def test_a_failed_job_cannot_launder_itself_as_not_applicable() -> None: + """The softest receipt must not excuse the hardest failure. Every step in these jobs is + continue-on-error, so if this passed nothing anywhere would be red.""" + needs = _all_healthy() + needs["coverage"] = _job( + result="failure", + receipt={ + "signal": "coverage", + "status": "not-applicable", + "reason": "coverage.xml was not produced", + }, + ) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["coverage"] + assert "dead, not inapplicable" in violations[0]["message"] + + +# -------------------------------------------------------------------------------------------- +# The check must not go blind itself. +# -------------------------------------------------------------------------------------------- + + +def test_a_renamed_or_removed_job_is_caught() -> None: + """If a signal's job is renamed and EXPECTED_SIGNALS is not updated, the gate would silently + stop being checked -- the same class of failure one level up.""" + needs = _all_healthy() + del needs["complexity"] + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["complexity"] + assert "renamed" in violations[0]["message"] + + +def test_a_malformed_receipt_is_caught() -> None: + needs = _all_healthy() + needs["clone"] = {"result": "success", "outputs": {"receipt": "{not json"}} + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["clone"] + assert "not valid JSON" in violations[0]["message"] + + +def test_a_gate_reporting_itself_dead_is_a_violation() -> None: + needs = _all_healthy() + needs["mutation"] = _job( + receipt={"signal": "mutation", "status": "failed", "reason": "mutmut exited 1"} + ) + + violations, _ = liveness.verify(needs) + + assert [v["signal"] for v in violations] == ["mutation"] + assert "reported ITSELF dead" in violations[0]["message"] + + +def test_an_unparseable_needs_blob_fails_loudly(capsys: pytest.CaptureFixture[str]) -> None: + """A liveness check that silently verifies nothing is the exact bug it guards against.""" + rc = liveness.main(["verify", "--needs-json", "{not json"]) + + assert rc == 2 + assert "could not run" in capsys.readouterr().out.lower() + + +def test_every_expected_signal_is_a_real_job_in_the_workflow() -> None: + """EXPECTED_SIGNALS must track the workflow, or the check silently stops covering a gate.""" + import yaml + + workflow = yaml.safe_load( + (_ROOT / ".github" / "workflows" / "quality-advisory.yml").read_text(encoding="utf-8") + ) + for signal in liveness.EXPECTED_SIGNALS: + assert signal in workflow["jobs"], f"{signal!r} is expected but no such job exists" + + +# -------------------------------------------------------------------------------------------- +# Receipt construction + exit codes. +# -------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize("units", [0, -1, None]) +def test_measured_requires_positive_units(units: int | None) -> None: + with pytest.raises(liveness.Violation, match="requires units > 0"): + liveness.build_receipt("clone", "measured", units, "files", "jscpd ran", "") + + +def test_measured_requires_evidence() -> None: + with pytest.raises(liveness.Violation, match="requires evidence"): + liveness.build_receipt("clone", "measured", 5, "files", " ", "") + + +def test_not_applicable_requires_a_reason() -> None: + with pytest.raises(liveness.Violation, match="requires a reason"): + liveness.build_receipt("coverage", "not-applicable", None, "", "", " ") + + +def test_record_writes_a_single_line_to_github_output( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The receipt travels through a job output, so it must be one line of compact JSON.""" + out = tmp_path / "gh_output" + monkeypatch.setenv("GITHUB_OUTPUT", str(out)) + + rc = liveness.main( + [ + "record", + "--signal", + "mutation", + "--status", + "measured", + "--units", + "461", + "--unit-name", + "mutants processed", + "--evidence", + "mutmut progress line 461/461", + "--extra", + '{"killed":87,"survived":19,"no_tests":355,"other":0}', + ] + ) + written = out.read_text(encoding="utf-8") + + assert rc == 0 + assert written.count("\n") == 1, "the receipt must be a single line" + payload = json.loads(written.split("=", 1)[1]) + assert payload["units"] == 461 and payload["killed"] == 87 + + +def test_verify_exits_non_zero_only_on_violation( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(tmp_path / "s.md")) + + assert liveness.main(["verify", "--needs-json", json.dumps(_all_healthy())]) == 0 + + broken = _all_healthy() + broken["mutation"] = _job(result="success") + assert liveness.main(["verify", "--needs-json", json.dumps(broken)]) == 1 + + +def test_summary_names_the_offending_signal( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + summary = tmp_path / "s.md" + monkeypatch.setenv("GITHUB_STEP_SUMMARY", str(summary)) + broken = _all_healthy() + broken["mutation"] = _job(result="success") + + liveness.main(["verify", "--needs-json", json.dumps(broken)]) + text = summary.read_text(encoding="utf-8") + + assert "`mutation`" in text + assert "without proving they measured anything" in text + + +def test_annotation_escapes_workflow_command_metacharacters() -> None: + line = liveness._annotation({"signal": "clone", "message": "100% broken\nsecond line"}) + assert "100%25 broken%0Asecond line" in line + assert line.count("\n") == 0 diff --git a/tests/test_quality_advisory_invariants.py b/tests/test_quality_advisory_invariants.py index 3994b599..6e721e60 100644 --- a/tests/test_quality_advisory_invariants.py +++ b/tests/test_quality_advisory_invariants.py @@ -284,6 +284,48 @@ def test_mutmut_copies_the_package_not_just_the_mutated_file(code: str) -> None: assert "runner=" not in code, "mutmut 3 uses pytest_add_cli_args_test_selection" +def test_every_measurement_job_emits_a_liveness_receipt(workflow: dict) -> None: + """A gate that reports a conclusion without recording that it measured anything is the exact + failure this workflow produced three times. Every measurement job must own a receipt.""" + measurement_jobs = {"complexity", "clone", "coverage", "mutation"} + for name in measurement_jobs: + job = workflow["jobs"][name] + assert (job.get("outputs") or {}).get("receipt"), ( + f"{name} exposes no liveness receipt output" + ) + steps = [s for s in job["steps"] if s.get("id") == "receipt"] + assert len(steps) == 1, f"{name} must have exactly one step with id: receipt" + assert steps[0].get("if") == "always()", ( + f"{name}'s receipt step must run even when the analysis step failed -- otherwise a dead " + "gate produces no receipt AND no explanation" + ) + + +def test_the_liveness_job_is_allowed_to_fail(workflow: dict) -> None: + """Every other job here is built so it cannot fail. This one is built so it CAN -- that is the + whole point. Adding continue-on-error would silently neuter it.""" + job = workflow["jobs"]["liveness"] + assert job.get("if") == "always()", "liveness must rule even when a gate job died" + assert set(job["needs"]) == {"complexity", "clone", "coverage", "mutation"} + for step in job["steps"]: + assert step.get("continue-on-error") is not True, ( + "the liveness step must be able to redden its job -- that is its only way to be loud" + ) + body = step.get("run") or "" + if "liveness.py" in body: + assert "|| true" not in body, "swallowing the exit code defeats the entire check" + + +def test_the_mutation_other_count_is_not_a_remainder(code: str) -> None: + """If `other` were computed as LISTED - SURVIVED - NOTESTS, the liveness reconciliation would be + true by construction and could never fire -- an identity-confirmation check that only looks like + a control. Counting each category independently is what keeps the sum meaningful.""" + assert re.search(r"OTHER=.*grep -cE", code), "other must be counted independently, not derived" + assert "OTHER=$((LISTED" not in code, ( + "deriving other as a remainder makes the check tautological" + ) + + def test_the_killed_count_is_derived_not_grepped(code: str) -> None: """`mutmut results` lists ONLY the mutants worth looking at (survived / no tests / timeout / suspicious). Killed mutants are never listed, so counting `': killed'` returns 0 on a perfectly From de1b45bd685885c330e8f0002b2edda899374909 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 28 Jul 2026 14:10:28 -0500 Subject: [PATCH 2/2] docs: record the liveness rule as rubric section 4.0, and clear the pre-cutover rot in these files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documents the gate-liveness control and promotes it to a rule in the rubric, because the failure it addresses is general rather than specific to this repo: an advisory gate that silently stops measuring is worse than an absent one, since the scorecard still counts it. Code_Quality_Standards.md gains section 4.0 (the liveness rule) and a v0.11 history entry. The rule has three parts, each written from a real failure rather than from theory: prove execution via units EXAMINED, never units found (a clean repo reports zero and must still pass); "nothing to measure" is acceptable only when stated with a reason; and any derived headline figure must be cross-checked against an independently produced measurement of the same quantity. That third part was rewritten during review. It originally said only "the parts must sum to the whole, each counted independently" — true but insufficient, because a sum containing a derived term can be algebraically blind to that term, and ours was. The rubric now says what actually protects a number, and the v0.11 entry records that the control was found carrying the same weakness it was built to catch. Leaving that out would have been the more flattering and less useful choice. Corrected two miscounts of my own from v0.10 and docs/CI.md: it was three defects across TWO gates, not three gates, and the killed=0 bug is a different category (a gate that ran correctly and published a wrong number) rather than a third instance of measuring nothing. The distinction was already drawn correctly in the rules; only the narrative sentences conflated them. PRE-CUTOVER SLUG ROT, in the files this commit already touches and which the open sweep does not cover: * HANDOFF-mutation-coverage.md was worse than stale. It instructed a future session to gate the mutation job with `if: github.repository == 'MEFORORG/MessageFoundry'` "(free minutes)" — the exact repo-slug gate removed when mutation started running on PRs. Following it would have made the job a no-op on every PR while looking deliberate. Left an explicit do-not-re-add marker rather than deleting the sentence, so the reasoning survives. * Code_Quality_Standards.md had one live "mirror-nightly" description of how the mutation job runs. Corrected and annotated. DELIBERATELY NOT SWEPT in that file: the mirror references in the control-parity narrative and in the v0.8 history row are dated incident records — the account of the fixed PyPI-sdist leak, which is the rubric's own worked example. Rewriting them would falsify the record. --- docs/CI.md | 37 +++++++++++++++++++ docs/Code_Quality_Standards.md | 36 +++++++++++++++++- .../HANDOFF-mutation-coverage.md | 20 ++++++---- 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/docs/CI.md b/docs/CI.md index fe5b1375..dbea20eb 100644 --- a/docs/CI.md +++ b/docs/CI.md @@ -63,8 +63,45 @@ for diff-coverage and usually *not* true for complexity, so the two land in diff | Diff-coverage | **Inline on the Files changed tab**, one `::notice` per contiguous uncovered range of lines the PR changed, plus a step summary. Every line it flags is a line the PR touched, so this is the one signal that is reliably inline. | | Complexity (`C901`) | A **merge-base-vs-HEAD delta** — only functions this PR introduced over the threshold or made more complex. Findings anchor on the `def` line, which a body-only edit does not touch, so **most complexity annotations appear in the Checks tab and the step summary rather than inline**. The summary table is this signal's primary surface. Pre-existing findings are never reported; the full list stays in the job log. | | Duplication (`jscpd`) | Step summary only. jscpd emits one location per clone pair chosen by scan order, so annotating it would anchor on the untouched twin about half the time. | +| **Gate liveness** | A pass/fail table proving each gate above actually *measured* something. See below — this is the only job in that workflow that can go red. | | Mutation (`mutmut`) | A **killed / survived / not-covered** table in the step summary, with the surviving mutants listed — those are injected bugs the tests did not catch. Runs on PRs too: measured at **461 mutants in 3 seconds** (87 killed, 19 survived) over the bounded scope, because mutmut 3 only runs the tests that cover each mutant. Repaired 2026-07-27 — `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 before generating a single mutant and, thanks to `\|\| true`, had been reporting success in 37s while measuring nothing. | +### Gate liveness — the check that watches the checks + +Three defects across two of `quality-advisory.yml`'s gates spent months green. Two were gates +**measuring nothing** — diff-coverage (a shallow fetch destroyed its merge base, and the resulting +empty report looked clean) and mutation (the tool crashed before producing a single mutant, and +`|| true` made that green in 37 seconds). The third was the close cousin: a gate that measured +correctly and **published a wrong number** — a `grep` for a line the tool never prints, so a healthy +461-mutant run reported "Killed 0". + +The rubric's anti-metric rule guards against trusting a *number* too much. Nothing guarded against +trusting a *green check that never ran*. The `liveness` job is that control. + +Each measurement job emits a small **receipt** recording what it examined; the `liveness` job reads +them all and demands either proof of execution or an explicit, reasoned "nothing to measure". + +- **Liveness is not "the gate found something."** A clean repo legitimately has zero clones. Receipts + count units **examined** — files scanned, mutants processed, changed lines analysed — which is + non-zero whenever the tool ran, whatever it concluded. A check that fires on good news gets muted. +- **"Nothing to measure" passes — if it says why.** `no lines with coverage information in this diff` + is a real, correct outcome. A silent empty report is not. The two look identical on screen; the + reason is the difference. +- **Numbers must reconcile, against an independent source.** Two checks, because the obvious one is + weaker than it looks. `killed + survived + no-tests + other` must equal the mutants processed — but + since `killed` is *derived* as total-minus-listed, that sum reduces algebraically to + "every listed mutant carries a recognised status" and never validates `killed` at all. So `killed` + is additionally reconciled against **mutmut's own counter**, parsed from its progress line: two + independent derivations that must agree. That second check is what would catch a recurrence of the + `killed=0` bug; the sum alone would not. `tests/test_gate_liveness.py` asserts both, including an + explicit test documenting the sum's blindness rather than hiding it. +- **It is the one job there allowed to go red**, deliberately: it has no `continue-on-error` and no + `|| true`. A red mark still blocks nothing — it is not, and must never become, a required context. + +`tests/test_gate_liveness.py` replays all three historical incidents and asserts each is caught, and +asserts the good-news cases pass. A liveness gate that cannot catch the failures it was built for +would be exactly the thing it exists to prevent. + ### The `CI gate` roll-up `CI gate` `needs:` the individual legs, runs with `if: always()`, and fails **only** on a `failure` or diff --git a/docs/Code_Quality_Standards.md b/docs/Code_Quality_Standards.md index d7df11b5..adfa56bd 100644 --- a/docs/Code_Quality_Standards.md +++ b/docs/Code_Quality_Standards.md @@ -69,7 +69,7 @@ The full standard follows: the evidence review, the AI failure-mode map, the com | **Applies to** | Any project developed under the [SDS](Secure_Development_Standards.md). **MessageFoundry (MEFOR)** is the reference implementation (Appendix A); future projects add Appendix B, C, … | | **Maintained by** | Project maintainers (open-source). Each deploying/adopting organization assigns its own local owner. | | **Status** | Draft for review | -| **Version** | 0.10 | +| **Version** | 0.11 | | **Date** | July 27, 2026 | | **License** | Publishable under the project's open-source license; intended to be shared with adopters and reused across projects. | | **Review cadence** | At least annually, and on any material change to the metric evidence base or the AI toolchain. | @@ -161,6 +161,37 @@ Each signal is a **risk → control → measure**, tagged by **gate type** (dete > > **Evidence & citations for the matrix.** Every signal and claim above maps to its supporting study in [**Appendix B.3**](#b.3-evidence-behind-each-rubric-element) (per-element evidence table), with full bibliographic citations in [**Appendix B.4**](#b.4-references), the derivation method in [**Appendix B.2**](#b.2-how-the-matrix-was-derived), and the claims that *failed* verification in [**Appendix B.5**](#b.5-what-was-refuted-the-verification-worked). +### 4.0 The liveness rule (hard) — a gate that cannot fail is not a control + +**Every advisory gate must prove it measured something, or say why it could not.** A gate that reports +a conclusion without recording that it performed a measurement is indistinguishable from one that +worked, and it will stay that way indefinitely, because nobody investigates a green check. + +This rule was written from failure, not theory. Three defects across **two** of this document's own +Tier 2 gates were found (2026-07-27) to have been green throughout: two gates measuring nothing (one +of them for two full versions of this rubric, during which it was scored ✅ **Built** here — v0.10), +and one gate that measured correctly but published a wrong derived number. Section 4.1 protects +against trusting a *number* too much; nothing protected against trusting a *green check that never +ran*. That is a distinct failure mode and it needs its own control: + +1. **Proof of execution, not of findings.** The receipt counts units *examined* — files scanned, + mutants processed, changed lines analysed — never units *found*. A clean codebase legitimately + reports zero clones, and a liveness check that fires on good news gets muted, leaving the project + worse off than before it existed. +2. **"Nothing to measure" is acceptable only when stated.** An explicit, reasoned declaration passes; + a silent empty result fails. The two are visually identical, which is precisely why the reason is + load-bearing. +3. **Reported numbers must reconcile against an INDEPENDENT source.** A gate can execute perfectly + and still emit a wrong derived figure. Two rules, because the obvious one is weaker than it looks: + parts must sum to the whole, each counted independently and never as a remainder (a remainder + makes the sum true by construction); *and* any derived headline figure must be cross-checked + against a second, independently produced measurement of the same quantity. A sum that includes a + derived term can be algebraically blind to that term — ours was, and the blindness is now asserted + by a test rather than assumed away. + +Applies to any gate, in any project adopting this rubric — a deferred or advisory gate that silently +stops measuring is worse than an absent one, because the scorecard still counts it. + ### 4.1 The anti-metric rule (hard) **Do NOT certify quality — or fail a build — on any single one of:** line-coverage %, LOC, raw or cognitive cyclomatic complexity, or SonarQube severity counts. Each is a weak or gameable predictor (§2). They may be *surfaced as advisory triage signals*; they must never be *the* quality gate. This mirrors the AI companion's "gates are deterministic checks, never ask the model to be secure" — here: *a scoreboard is never the verdict.* @@ -270,7 +301,7 @@ The five gates this document adds (rubric rows 7–11) are *quality-measurement* Ordered by anti-slop leverage, not effort (build placement per §5). **✅ = shipped** (advisory; \#1028 or \#1040): -1. **Mutation testing** — highest leverage; directly counters shallow-test slop, extra weight under the solo-maintainer review deviation (A.4). ✅ **shipped** (#1040 — `mutmut` over a bounded, well-tested module, mirror-nightly + `workflow_dispatch`; widen the scope later). +1. **Mutation testing** — highest leverage; directly counters shallow-test slop, extra weight under the solo-maintainer review deviation (A.4). ✅ **shipped** (#1040 — `mutmut` over a bounded, well-tested module; runs on PRs + nightly cron + `workflow_dispatch`; widen the scope later). *(v0.11: was "mirror-nightly" — there is no mirror post-cutover, and the job no longer carries a repo-slug gate.)* 2. **Clone-detection on diffs** — ✅ **shipped** (`jscpd`, store-parity whitelisted) — catches the copy-instead-of-abstract signature the parallel-worktree workflow is most exposed to. 3. **Diff-coverage visibility** — measured on changed lines, guidance only (never a whole-repo % gate — §4.1). ✅ **shipped** (#1040 — `pytest-cov` + `diff-cover`, PR-only). 4. **Advisory** `C901` **complexity** — ✅ **shipped** (advisory triage). @@ -366,6 +397,7 @@ The evidence caveats in **§7** are part of this appendix's basis: the metric-in | Version | Date | Change | |----|----|----| +| 0.11 | July 27, 2026 | **Added the liveness rule (new section 4.0) and built the control.** v0.10 recorded that signal 7 had been scored ✅ Built for two versions while its tool crashed before producing a mutant. That is a failure mode this rubric had no defence against: section 4.1 forbids over-trusting a *number*, but nothing forbade over-trusting a *green check that never ran* — and three defects across two of the five Tier 2 gates turned out to have that shape (two measuring nothing, one publishing a wrong derived number). Section 4.0 now requires every advisory gate to prove it measured something (units **examined**, never units found — a clean repo reports zero and must still pass) or to declare explicitly, with a reason, that it had nothing to measure; and any derived headline figure must be cross-checked against an independently produced measurement of the same quantity. Implemented as the `liveness` job in `quality-advisory.yml` — the only job there permitted to go red — with `tests/test_gate_liveness.py` replaying the historical incidents to prove the check catches them, and the good-news cases to prove it does not fire on them. **The control was itself adversarially reviewed before merge, and the review found it carrying the same weakness it was built to catch, in three places** — a dead coverage gate could pass by claiming "not applicable", an empty mutmut results file reported a flawless score, and the reconciliation sum was algebraically blind to the very count it claimed to protect. All three are fixed and regression-tested; rule 3 above was rewritten because of the third. No scoring change (A− stands); the gates were repaired in v0.10, this is the control that keeps them honest. | | 0.10 | July 27, 2026 | **Restored to the repo, and corrected three claims that did not survive measurement.** This file had been absent from the repository's entire git history despite being cited by `quality-advisory.yml` and `pyproject.toml`; it is restored here from the maintained copy. Corrections, each measured rather than reasoned: **(a) Signal 7 was scored ✅ Built in 0.8 and 0.9 while producing nothing.** `mutmut<3` resolved to 2.5.1, which crashes on Python 3.14 in its pony-ORM cache (`cannot pickle 'itertools.count'`) *before generating a single mutant*; `\|\| true` made the job report success in 37s, so the gate looked green for two versions. Repaired on `mutmut==3.6.0` (+ `pytest-timeout`, and `source_paths` must be the package, not the one file, or the mutant copy cannot import `conftest`). Now genuinely measured: **461 mutants, 87 killed, 19 survived, 3 seconds** — so the "Expensive / never per-PR" cost model in §5 was also wrong, and mutation now runs on PRs. **(b) Signal 11's "85 functions over C901>10" is now 122 across 43 files**, and the raw list was found unusable as a diff signal (every finding anchors on one `def` line), so a merge-base delta was added that reports only PR-caused changes. **(c) Signal 8 now emits inline PR annotations** rather than console-only output. The A− verdict stands, but note that (a) is exactly the failure mode this rubric exists to catch — an advisory gate that reports success while measuring nothing — and it was caught by re-verification, not by the gate itself. | | 0.9 | July 14, 2026 | **Restatused signal 10 (lint breadth) to ✅ Built — all 11 signals now Built.** The `extend-select = [B,C4,SIM,UP,I]` sweep shipped (#1047): B008 handled via `extend-immutable-calls` + a route-layer per-file ignore, 515 auto-fixed, 235 grandfathered with `# noqa`, enforced by the required `ruff check` leg. Flipped the exec verdict, §5 gate table + callout, §6 map, and Appendix A.1 / A.2 (row 10 + Tier-2 roll-up) / A.3 (gaps list + "remaining gate" prose → rollout *record*). No scoring change (A− stands). | | 0.8 | July 14, 2026 | **Restatused signals 7 (mutation) + 8 (diff-coverage) to ✅ Built.** Both shipped as advisory jobs in `quality-advisory.yml` (#1040) — mutation over a bounded module (mirror-nightly + `workflow_dispatch`), diff-coverage on the diff's changed lines (PR-only). Flipped every place that called them deferred: the exec verdict, §5 gate table + callout, §6 map, and Appendix A.1 / A.2 (rows 7–8 + the Tier-2 roll-up) / A.3 (gaps list + DEP-1 note) / A.4. **Only signal 10 (lint breadth) remains designed-but-deferred → 10 of 11 signals now Built.** No scoring change (A− stands). | diff --git a/docs/quality-gates/HANDOFF-mutation-coverage.md b/docs/quality-gates/HANDOFF-mutation-coverage.md index 84415e77..20162b8a 100644 --- a/docs/quality-gates/HANDOFF-mutation-coverage.md +++ b/docs/quality-gates/HANDOFF-mutation-coverage.md @@ -119,10 +119,14 @@ diff-cover coverage.xml --compare-branch=origin/main --fail-under=0 git fetch --no-tags --depth=1 origin main || true diff-cover coverage.xml --compare-branch=origin/main --fail-under=0 || true ``` -**Cost lever:** if a full-suite run per PR is too much on the private repo, either add a path filter (only run -when `messagefoundry/**` or `tests/**` change) or gate the whole job to the mirror with the repo-slug -`if: github.repository == 'MEFORORG/MessageFoundry'` (free minutes) — but then it won't report on private -PRs. Start advisory-on-PR; move to mirror if cost bites. +**Cost lever:** if a full-suite run per PR is too much, add a path filter (only run when +`messagefoundry/**` or `tests/**` change). + +> **STALE — pre-cutover (corrected 2026-07-27).** This paragraph used to offer "gate the whole job to +> the mirror with the repo-slug `if: github.repository == 'MEFORORG/MessageFoundry'` (free minutes)". +> **There is no mirror**; the cutover moved development directly onto the public repo, and that +> repo-slug gate was *removed* from the mutation job. Do not re-add it — it would make the job a no-op +> on every PR while looking deliberate. --- @@ -137,8 +141,8 @@ that *adversarially* proves your tests assert something (rubric A.4: matters mos 2. **Where it runs** (mutation is too slow for every private PR): - **(A) PR, diff-scoped** — mutate only files changed vs `origin/main` (small diffs → few mutants → tolerable). Closest to the rubric's "mutation on changed code"; higher per-PR cost. **Recommended.** - - **(B) mirror-nightly, rotating** — a nightly cron on the mirror (free minutes) mutates a rotating - slice. Cheap, but not per-PR. + - **(B) nightly, rotating** — a nightly cron mutates a rotating slice. Cheap, but not per-PR. + *(Was "mirror-nightly (free minutes)" — there is no mirror post-cutover.)* ### 3b. Local verify (against the installed mutmut version) ```powershell @@ -194,14 +198,14 @@ mutmut results mutmut results || true ``` -### 3c′. CI job — option (B) mirror-nightly (add a `schedule:` trigger + repo-slug gate) +### 3c′. CI job — option (B) nightly (add a `schedule:` trigger) Add the cron to the workflow's `on:` block, then the job: ```yaml # on: # pull_request: # workflow_dispatch: # schedule: -# - cron: "23 4 * * *" # nightly; the job's repo-slug if keeps it mirror-only (free minutes) +# - cron: "23 4 * * *" # nightly sweep against main mutation-nightly: name: mutation (nightly, advisory)