From befdf99950932b1b8f0be7699af3e370fd6da2bd Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Tue, 28 Jul 2026 18:46:23 -0500 Subject: [PATCH] =?UTF-8?q?fix(ci):=20let=20the=20complexity=20DELTA=20acc?= =?UTF-8?q?ount=20for=20itself=20=E2=80=94=20the=20receipt=20could=20not?= =?UTF-8?q?=20see=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The liveness control shipped in #25 had this workflow's own failure mode inside it. The complexity receipt proved liveness with `ruff --show-files`, which counts the files ruff would enumerate — true whether or not the DELTA step ran. So a delta that bailed on an unresolvable merge base emitted a `::notice`, exited 0, and left a green `measured` receipt behind. On a PR the delta IS the point of that job, and it could go quiet while the gate reported success. The delta now records its own outcome to c901-delta.env, written LAST on the success path so its absence proves the step died, and written explicitly on the merge-base bail path so a skip is distinguishable from a crash. The receipt rules on it: PR + delta ran -> measured, evidence names the merge base and the base/head finding counts PR + marker missing -> failed, "did not complete" PR + delta bailed -> failed, carrying the recorded reason cron / dispatch -> measured, "the PR delta does not apply on " That last row is deliberate. On a non-PR event the delta legitimately does not run and the whole-repo triage IS the measurement; failing there would fire on good news, and a check that fires on good news gets muted -- which would leave us worse off than before. TWO BUGS FOUND BY EXECUTING THE RECEIPT SHELL, not by reading it: * The reason was silently discarded. The receipt SOURCES the marker, and the reason was written unquoted, so bash parsed a value containing spaces as a command and left the variable unset. The failure reported "no reason recorded" -- throwing away the only useful diagnostic, on the exact path that exists to provide one. Now single-quoted, and asserted. * My own new test was vacuous. It asserted the marker appeared at least twice in the step, which the bail path satisfies on its own (it writes two lines), so deleting the SUCCESS-path marker passed. Caught by mutating the workflow and watching the assertion fail to go red. It now partitions the step at the c901_delta.py call and requires a marker on both sides. All four receipt paths were executed against the extracted shell; all four negative probes on the new assertions now go red. --- .github/workflows/quality-advisory.yml | 54 ++++++++++++++++++++++- tests/test_quality_advisory_invariants.py | 50 +++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/.github/workflows/quality-advisory.yml b/.github/workflows/quality-advisory.yml index 3cc828f8..dedbf02c 100644 --- a/.github/workflows/quality-advisory.yml +++ b/.github/workflows/quality-advisory.yml @@ -122,6 +122,16 @@ jobs: # leave a reason in the log, since nobody watches a green advisory job for absence. if ! MERGE_BASE="$(git merge-base HEAD "origin/$BASE_REF" 2>/dev/null)"; then echo "::notice title=Complexity delta skipped::could not resolve a merge base against origin/$BASE_REF" + # Record the bail so the liveness receipt can SEE it. Without this the step exits 0, the + # receipt still reports "measured" off the whole-repo triage, and the PR-caused delta -- + # the entire point of this step on a PR -- goes quiet behind a green check. That is the + # failure mode this workflow's liveness job exists to catch, sitting inside it. + # QUOTE the reason: the receipt SOURCES this file, and an unquoted value containing + # spaces parses as a command, silently leaving the variable unset -- which reported + # "no reason recorded" and threw away the one useful diagnostic. Caught by executing + # the receipt shell, not by reading it. + echo "C901_DELTA=skipped" > c901-delta.env + echo "C901_DELTA_REASON='could not resolve a merge base against origin/$BASE_REF'" >> c901-delta.env exit 0 fi echo "merge base: $MERGE_BASE" @@ -138,9 +148,23 @@ jobs: --head c901-head.json \ --repo-root . \ --summary-file "$GITHUB_STEP_SUMMARY" + # Written LAST, so it exists only if every step above actually completed. Its absence is + # therefore proof the delta died somewhere, which the receipt reports as a dead gate. + BASE_N="$(python3 -c 'import json,sys; print(len(json.load(open(sys.argv[1]))))' c901-base.json)" + HEAD_N="$(python3 -c 'import json,sys; print(len(json.load(open(sys.argv[1]))))' c901-head.json)" + { + echo "C901_DELTA=ok" + echo "C901_DELTA_MERGE_BASE=$MERGE_BASE" + echo "C901_DELTA_BASE_N=$BASE_N" + echo "C901_DELTA_HEAD_N=$HEAD_N" + } > c901-delta.env - name: Record gate liveness id: receipt if: always() + # Routed through env, not interpolated into the shell body (zizmor: template injection). + env: + IS_PR: ${{ github.event_name == 'pull_request' }} + EVENT_NAME: ${{ github.event_name }} continue-on-error: true run: | # Units count what was EXAMINED, never what was found: a repo with zero functions over the @@ -149,11 +173,37 @@ jobs: 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)))')" + # THE DELTA IS THE POINT ON A PR, and until now this receipt could not see it. `--show-files` + # proves ruff enumerated files, which is true whether or not the delta step ran -- so a delta + # that bailed on an unresolvable merge base left a green "measured" receipt and vanished. + # That is this workflow's own failure mode reproduced inside its liveness control. + # + # On a non-PR event the delta legitimately does not run and the whole-repo triage IS the + # measurement. On a PR its silence is a dead gate, not an absence of news. + if [ "$IS_PR" != "true" ]; then + 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 (whole-repo triage; the PR delta does not apply on $EVENT_NAME)" \ + --extra "{\"findings\":$FINDINGS}" + exit 0 + fi + if [ ! -s c901-delta.env ]; then + python3 scripts/quality/liveness.py record --signal complexity --status failed \ + --reason "the PR complexity delta produced no outcome marker; it did not complete (see the job log)" + exit 0 + fi + . ./c901-delta.env + if [ "$C901_DELTA" != "ok" ]; then + python3 scripts/quality/liveness.py record --signal complexity --status failed \ + --reason "the PR complexity delta did not run: ${C901_DELTA_REASON:-no reason recorded}" + exit 0 + fi 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}" + --evidence "ruff C901 scanned $FILES files ($FINDINGS over the threshold); delta compared ${C901_DELTA_BASE_N} base vs ${C901_DELTA_HEAD_N} head findings against ${C901_DELTA_MERGE_BASE}" \ + --extra "{\"findings\":$FINDINGS,\"delta_base_n\":${C901_DELTA_BASE_N},\"delta_head_n\":${C901_DELTA_HEAD_N}}" clone: # Signal 9 - duplication / clone detection. Flags copy-pasted blocks (the "copy-instead-of-abstract" diff --git a/tests/test_quality_advisory_invariants.py b/tests/test_quality_advisory_invariants.py index 6e721e60..7573fba3 100644 --- a/tests/test_quality_advisory_invariants.py +++ b/tests/test_quality_advisory_invariants.py @@ -326,6 +326,56 @@ def test_the_mutation_other_count_is_not_a_remainder(code: str) -> None: ) +def test_the_complexity_delta_reports_its_own_outcome(workflow: dict) -> None: + """The delta is the POINT of the complexity job on a PR, and the receipt could not see it. + + `--show-files` proves ruff enumerated files, which is true whether or not the delta ran — so a + delta that bailed on an unresolvable merge base left a green `measured` receipt and vanished. + That is this workflow's own failure mode reproduced inside its liveness control, so the delta + now writes an outcome marker and the receipt rules on it. + """ + steps = {s.get("id"): s for s in workflow["jobs"]["complexity"]["steps"]} + delta = next( + s + for s in workflow["jobs"]["complexity"]["steps"] + if "c901_delta.py" in (s.get("run") or "") + ) + + # Split at the delta invocation: everything before it is the bail path, everything after is the + # success path. BOTH must write the marker, or its absence is ambiguous. Counting occurrences is + # NOT enough — the bail path writes twice on its own, so a count check passes even with the + # success-path marker deleted. (Found by negative-probing this very assertion.) + bail, _, success = delta["run"].partition("c901_delta.py") + assert "c901-delta.env" in bail, "the merge-base bail path must record that it bailed" + assert "c901-delta.env" in success, ( + "the success path must record completion — written last, so its absence proves the delta died" + ) + + receipt = steps["receipt"]["run"] + assert "c901-delta.env" in receipt, "the receipt must consult the delta's outcome" + assert "--status failed" in receipt, "a vanished delta must be reported as a dead gate on a PR" + + +def test_the_delta_marker_reason_is_quoted(code: str) -> None: + """The receipt SOURCES the marker file. An unquoted value containing spaces parses as a command + and leaves the variable unset — which silently reported "no reason recorded" and threw away the + only useful diagnostic. Found by executing the receipt shell, not by reading it.""" + assert re.search(r"C901_DELTA_REASON='[^']+'", code), ( + "the reason must be single-quoted so `source` yields the whole string" + ) + + +def test_the_non_pr_path_is_not_treated_as_a_dead_delta(workflow: dict) -> None: + """On cron/dispatch the delta legitimately does not run and the whole-repo triage IS the + measurement. Failing there would fire on good news, which gets a check muted.""" + steps = {s.get("id"): s for s in workflow["jobs"]["complexity"]["steps"]} + env = steps["receipt"].get("env") or {} + assert any("event_name" in str(v) for v in env.values()), ( + "the receipt must know whether this is a PR before ruling the delta missing" + ) + assert "does not apply" in steps["receipt"]["run"] + + 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