From dbe117fff83aa5b372e0628de396b6ff9e2e1e7d Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 15:10:32 -0600 Subject: [PATCH 1/7] ci: a red nightly opens an issue, and a green one closes it (#973) The deep gate ran red two nights and nothing surfaced it; before that, 25 consecutive nights. Three properties compound, each reasonable alone: a scheduled run has no pull request to be red on, the job that failed is named like a reporting step rather than a gate, and GitHub notifies the ACTOR of a schedule event -- whoever last touched the workflow file, not whoever broke it. So the workflow carries a `red-nightly` job that opens ONE issue on the first red, updates it on each red after, and CLOSES IT on the next green. `if: always()`, not `if: failure()`. The green path is half the mechanism: an alert that must be closed by hand becomes one nobody closes, and then a stale one nobody believes. ONE ISSUE, NOT ONE PER RUN, so the title carries nothing variable. A notifier that files nightly is one people filter, and a filtered notifier is the state this replaces. THE VERDICT COMES FROM needs.*.result, which cannot be late. The failing job NAMES come from the run's own API, because `needs` keys say `coverage` where a reader needs `coverage report (PG 18)`. If that call fails the verdict still stands and the body says no name was reported, rather than reporting green. AND THE JOB IS NAMED red-nightly BECAUSE `report` WOULD HAVE BEEN VACUOUS. selftest 240 asserts every nightly job key appears in docs/testing.md, and that paragraph already says "coverage report" -- so a job keyed `report` satisfies an existing guard by accident. Verified both ways: the guard reddens on the new key before the doc names it. A FIRE DRILL SKIPS THE HEAVY GATES. The drill proves the reporter fires, and a proof costing a full deep gate is one nobody re-runs -- which is how a notifier goes back to never having been seen to fire. It takes the same code path and differs only in where the verdict came from. Removal proofs, six: a job added and not wired into needs the set-equality arm reddens if: always() -> if: failure() 1 -> 0 issues: write removed yes -> empty the title carries the run id 0 digits -> 1 an unknown verdict defaults to green rc 2 -> 0 a nameless failure prints nothing 1 -> 0 Verified: harness_selftest 817/817 on PG16 with the new part's 13 checks, shellcheck clean, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .github/scripts/nightly-report.sh | 102 ++++++++++++++++++ .github/workflows/nightly.yml | 91 +++++++++++++++- docs/testing.md | 4 + .../450-a-red-nightly-must-be-findable.sh | 91 ++++++++++++++++ test/selftest/parts.manifest | 1 + 5 files changed, 285 insertions(+), 4 deletions(-) create mode 100755 .github/scripts/nightly-report.sh create mode 100644 test/selftest/450-a-red-nightly-must-be-findable.sh diff --git a/.github/scripts/nightly-report.sh b/.github/scripts/nightly-report.sh new file mode 100755 index 00000000..4a5b90b8 --- /dev/null +++ b/.github/scripts/nightly-report.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# Make a red nightly FINDABLE WITHOUT KNOWING TO LOOK (#973). +# +# The deep gate ran red two nights and nothing surfaced it; before that, 25 +# consecutive nights. Three properties compound and each is reasonable alone: a +# scheduled run has no pull request to be red on, the failing job is named like a +# reporting step rather than a gate, and GitHub notifies the *actor* of a +# schedule event -- whoever last touched the workflow file, not whoever broke it. +# +# So this puts the verdict where somebody already looks: one issue, opened on the +# first red, updated on every red after it, and CLOSED BY THE NEXT GREEN. +# +# ONE ISSUE, NOT ONE PER RUN. A notifier that files a new issue nightly is a +# notifier people filter, and a filtered notifier is the state this replaces. +# The issue is found by its exact title, which is why the title carries no run +# number, no date and no job name -- everything variable goes in the body. +# +# IT CLOSES ITSELF. An alert that must be closed by hand becomes an alert nobody +# closes, and then a stale one nobody believes. The green path is as load-bearing +# as the red path and has its own arm. +# +# DRY RUN IS THE TESTED PATH, not a debugging aid: PGC_NIGHTLY_REPORT_DRYRUN=1 +# prints the verdict, the title and the body to stdout and calls no `gh`. The +# selftest drives exactly this, so what the arms read is what CI composes. +set -euo pipefail + +TITLE="${PGC_NIGHTLY_REPORT_TITLE:-nightly deep gate is red}" +verdict="${1:?usage: nightly-report.sh [failed-job ...]}" +shift || true +failed=("$@") + +run_url="${PGC_NIGHTLY_RUN_URL:-${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-}/actions/runs/${GITHUB_RUN_ID:-}}" + +case "$verdict" in + failure|success) ;; + *) + # NOT a silent default. A verdict this script does not understand means the + # caller changed and this did not; treating it as success would restore the + # exact silence the issue is about. + echo "nightly-report: verdict [$verdict] is neither failure nor success" >&2 + exit 2 + ;; +esac + +body_failure() { + printf '%s\n' "The nightly deep gate failed." + printf '\n' + printf 'Run: %s\n' "$run_url" + printf '\n' + if [ "${#failed[@]}" -gt 0 ]; then + printf 'Jobs that did not succeed:\n\n' + printf ' - %s\n' "${failed[@]}" + else + # The caller reported a failure and named no job. Say so rather than + # printing an empty list, which reads as "nothing failed". + printf 'No job name was reported with this failure, which is itself worth looking at.\n' + fi + printf '\n' + printf 'This issue is opened by the nightly on its first red, updated on each red\n' + printf 'after it, and closed automatically by the next green run.\n' +} + +body_success() { + printf '%s\n' "The nightly deep gate is green again." + printf '\n' + printf 'Run: %s\n' "$run_url" + printf '\n' + printf 'Closing automatically. An alert that must be closed by hand becomes one\n' + printf 'nobody closes, and then a stale one nobody believes.\n' +} + +if [ "${PGC_NIGHTLY_REPORT_DRYRUN:-0}" = 1 ]; then + printf 'verdict: %s\n' "$verdict" + printf 'title: %s\n' "$TITLE" + printf -- '--- body ---\n' + if [ "$verdict" = failure ]; then body_failure; else body_success; fi + exit 0 +fi + +# EXACT TITLE MATCH, not a search-relevance match. `gh issue list --search` is a +# full-text query and would find any issue mentioning these words -- including +# the ones the two of us have filed ABOUT this mechanism. +existing="$(gh issue list --state open --limit 100 --json number,title \ + --jq "map(select(.title == \"$TITLE\")) | .[0].number // empty")" + +if [ "$verdict" = failure ]; then + if [ -n "$existing" ]; then + gh issue comment "$existing" --body "$(body_failure)" + echo "nightly-report: commented on #$existing" + else + gh issue create --title "$TITLE" --body "$(body_failure)" + echo "nightly-report: opened a new issue" + fi +else + if [ -n "$existing" ]; then + gh issue comment "$existing" --body "$(body_success)" + gh issue close "$existing" --reason completed + echo "nightly-report: closed #$existing" + else + echo "nightly-report: green, and nothing open to close" + fi +fi diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 508b5324..8f6c0e49 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -32,6 +32,11 @@ on: schedule: - cron: "0 6 * * *" # nightly, 06:00 UTC workflow_dispatch: + inputs: + fire_drill: + description: "Exercise the red-nightly reporter without breaking anything" + type: boolean + default: false permissions: contents: read @@ -47,7 +52,12 @@ jobs: name: suites (PG ${{ matrix.pg }}, ${{ matrix.runner == 'ubuntu-24.04-arm' && 'aarch64' || 'x86_64' }}) runs-on: ${{ matrix.runner }} timeout-minutes: 60 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' strategy: fail-fast: false # The full packaged matrix on x86_64, and the current major on aarch64. @@ -202,7 +212,12 @@ jobs: name: sanitizer gate (ASAN+UBSAN) runs-on: ubuntu-latest timeout-minutes: 120 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' env: PG_VERSION: "18.4" SAN_PREFIX: /home/runner/pg_san @@ -329,7 +344,12 @@ jobs: name: coverage report (PG 18) runs-on: ubuntu-latest timeout-minutes: 90 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' steps: - uses: actions/checkout@v4 @@ -488,7 +508,12 @@ jobs: name: extension upgrade guard (PG 18) runs-on: ubuntu-latest timeout-minutes: 45 - if: github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar' + # A FIRE DRILL SKIPS THE HEAVY GATES. The drill exists to prove the reporter + # fires, and a proof that costs a full deep gate is one nobody re-runs -- which + # is how a notifier goes back to never having been seen to fire. `skipped` is + # not a failure to the reporter, so the drill's verdict comes from the input. + if: (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + && github.event.inputs.fire_drill != 'true' # Named ONCE. Two steps consume it, and a tag spelled separately in each is # the kind of duplication that goes stale in one place and not the other. env: @@ -575,3 +600,61 @@ jobs: echo "::error::the guard skipped for want of an old source; in this job that is a failure" fi exit "$rc" + + # ---- make a red nightly findable without knowing to look (#973) ----------- + # + # The deep gate ran red two nights and nothing surfaced it; before that, 25 + # consecutive nights. A scheduled run has no pull request to be red on, the job + # that failed is named like a reporting step rather than a gate, and GitHub + # notifies the ACTOR of a schedule event -- whoever last touched this file, not + # whoever broke it. + # + # `if: always()` and not `if: failure()`, because the GREEN path is half the + # mechanism: an alert that must be closed by hand becomes one nobody closes. + # + # THE VERDICT COMES FROM needs.*.result, WHICH CANNOT BE LATE. The failing job + # NAMES come from the run's own API, because `needs` keys say `coverage` where a + # reader needs `coverage report (PG 18)` -- and if that call fails the verdict + # still stands and the body says no name was reported, rather than reporting a + # green. + red-nightly: + name: report a red nightly + runs-on: ubuntu-latest + needs: [suites, sanitizer, coverage, upgrade-guard] + if: always() && (github.event_name == 'workflow_dispatch' || github.repository == 'commandprompt/pgcolumnar') + permissions: + contents: read + issues: write + steps: + - uses: actions/checkout@v4 + - name: open, update or close the nightly's issue + env: + GH_TOKEN: ${{ github.token }} + SUITES: ${{ needs.suites.result }} + SANITIZER: ${{ needs.sanitizer.result }} + COVERAGE: ${{ needs.coverage.result }} + UPGRADE: ${{ needs.upgrade-guard.result }} + DRILL: ${{ github.event.inputs.fire_drill }} + run: | + set -euo pipefail + verdict=success + for r in "$SUITES" "$SANITIZER" "$COVERAGE" "$UPGRADE"; do + # `skipped` is not a failure: a fork run skips every gated job. + case "$r" in failure|cancelled|timed_out) verdict=failure ;; esac + done + # A FIRE DRILL GOES THROUGH THIS SAME PATH, differing only in where the + # verdict came from. A drill that posts by a separate route proves the + # route, not the mechanism -- and a notifier never seen to fire is + # indistinguishable from none, which is what #973 is about. + if [ "${DRILL:-false}" = "true" ]; then verdict=failure; fi + echo "suites=$SUITES sanitizer=$SANITIZER coverage=$COVERAGE upgrade-guard=$UPGRADE -> $verdict" + + names=() + if [ "$verdict" = failure ]; then + while IFS= read -r n; do [ -n "$n" ] && names+=("$n"); done < <( + gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion != "success" and .conclusion != "skipped" and .conclusion != null) | .name' 2>/dev/null || true) + if [ "${DRILL:-false}" = "true" ] && [ "${#names[@]}" -eq 0 ]; then + names+=("fire drill: no job actually failed") + fi + fi + .github/scripts/nightly-report.sh "$verdict" ${names+"${names[@]}"} diff --git a/docs/testing.md b/docs/testing.md index eb1d9bf5..bafa3754 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -348,6 +348,10 @@ instrumented PostgreSQL, and the coverage report. It also runs the extension-upgrade guard. That guard builds the previous release on PostgreSQL 18, loads data into it, and upgrades it in place. The sanitizer build stays in a cache, because it takes longer to build than to run the suites against it. +A final job, red-nightly, opens an issue when any of those fail. The next green +run closes it. A scheduled run has no pull request to be red on, so nothing else +surfaces a failure here. This gate once ran red for 25 consecutive nights behind +a green per-PR gate. The aarch64 run executes the suites rather than only building them. Misaligned reads, the class most often expected to differ by architecture, are already diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh new file mode 100644 index 00000000..ed03819d --- /dev/null +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -0,0 +1,91 @@ +# ---- a red nightly must be findable without knowing to look ---------------- +# +# #973. The deep gate ran red two nights and nothing surfaced it; before that, +# 25 consecutive nights. Three properties compound and each is reasonable alone: +# a scheduled run has no pull request to be red on, the job that failed is named +# like a reporting step rather than a gate, and GitHub notifies the ACTOR of a +# schedule event -- whoever last touched the workflow file, not whoever broke it. +# +# So the workflow carries a `red-nightly` job that opens one issue on the first +# red, updates it on each red after, and CLOSES IT on the next green. +# +# WHAT THIS PART CAN AND CANNOT SEE, said first because the gap is the point of +# #973. It drives the reporter's composition through its dry run, and it asserts +# the WIRING structurally. It cannot see the issue being created, because that +# needs a live run -- and "a notifier never seen to fire is indistinguishable +# from none" is this issue's own sentence. The firing is demonstrated once, by a +# fire drill on a branch, and recorded in the PR rather than claimed here. + +_rn_wf="$TESTDIR/../.github/workflows/nightly.yml" +_rn_sh="$TESTDIR/../.github/scripts/nightly-report.sh" + +check "premise: the nightly workflow and the reporter are both present" \ + "$([ -r "$_rn_wf" ] && [ -x "$_rn_sh" ] && echo yes || echo no)" "yes" + +# ---- the wiring, derived from the workflow rather than listed here ---------- +# +# A job added later and NOT added to `needs` would be invisible to the reporter, +# and the reporter would report green while it burned. So the claim is +# set equality against every other job key, computed from the file. +_rn_jobs="$(awk '/^jobs:/{inj=1;next} inj && /^ [a-zA-Z_-]+:[[:space:]]*$/{ + k=$1; sub(/:$/,"",k); print k}' "$_rn_wf" | grep -v '^red-nightly$' | sort)" +_rn_n=$(printf '%s\n' "$_rn_jobs" | grep -c .) +check "premise: the other nightly jobs were parsed, so the comparison is real" \ + "$([ "${_rn_n:-0}" -ge 3 ] && echo yes || echo no)" "yes" + +# `needs: [a, b, c]` on one line, which is how the job is written. +_rn_needs="$(sed -n 's/^ needs: \[\(.*\)\]$/\1/p' "$_rn_wf" | + tr ',' '\n' | tr -d ' ' | grep -v '^$' | sort)" +check "premise: the reporter's needs list was parsed" \ + "$([ -n "$_rn_needs" ] && echo yes || echo empty)" "yes" + +check "the reporter waits on EVERY other nightly job (#973)" \ + "$(printf '%s' "$_rn_jobs" | md5sum | cut -c1-12)" \ + "$(printf '%s' "$_rn_needs" | md5sum | cut -c1-12)" + +# It must run on failure AND on success, or the close path never happens. +check "the reporter runs on every outcome, not only on failure" \ + "$(grep -c '^ if: always() &&' "$_rn_wf")" "1" + +# Opening an issue needs the permission, and the workflow's default is read-only. +check "and it is granted issues: write, which the workflow default withholds" \ + "$(awk '/^ red-nightly:/{p=1} p&&/issues: write/{print "yes"; exit}' "$_rn_wf")" "yes" + +# ---- the reporter's own composition, driven rather than read --------------- + +_rn_fail="$(PGC_NIGHTLY_REPORT_DRYRUN=1 PGC_NIGHTLY_RUN_URL="https://example/run/7" \ + bash "$_rn_sh" failure "coverage report (PG 18)" 2>&1)" + +check "a failure names the run so a reader can reach it" \ + "$(printf '%s' "$_rn_fail" | grep -c 'https://example/run/7')" "1" +check "and names the job that failed, not just that something did" \ + "$(printf '%s' "$_rn_fail" | grep -c 'coverage report (PG 18)')" "1" + +# ONE ISSUE, NOT ONE PER RUN. A notifier that files a new issue nightly is one +# people filter, and a filtered notifier is the state this replaces. The title is +# what makes updates land on the same issue, so it must carry nothing variable. +check "the title carries no run number, so every red lands on one issue" \ + "$(printf '%s' "$_rn_fail" | sed -n 's/^title: //p' | grep -c '[0-9]')" "0" + +_rn_ok="$(PGC_NIGHTLY_REPORT_DRYRUN=1 PGC_NIGHTLY_RUN_URL="https://example/run/8" \ + bash "$_rn_sh" success 2>&1)" +check "a green run composes the closing message, so the alert clears itself" \ + "$(printf '%s' "$_rn_ok" | grep -c 'green again')" "1" +check "and the green path reuses the same title, or it would close nothing" \ + "$(printf '%s' "$_rn_ok" | sed -n 's/^title: //p')" \ + "$(printf '%s' "$_rn_fail" | sed -n 's/^title: //p')" + +# A VERDICT IT DOES NOT UNDERSTAND MUST NOT READ AS GREEN. If the caller changes +# and this does not, treating the unknown value as success restores exactly the +# silence #973 is about. +PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" probably >/dev/null 2>&1 && _rn_rc=0 || _rn_rc=$? +check "an unrecognised verdict is refused rather than treated as green" \ + "$_rn_rc" "2" + +# A failure with no job name must say so rather than print an empty list, which +# a reader parses as "nothing failed". +_rn_bare="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" failure 2>&1)" +check "a failure naming no job says so rather than printing an empty list" \ + "$(printf '%s' "$_rn_bare" | grep -c 'No job name was reported')" "1" + +unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare diff --git a/test/selftest/parts.manifest b/test/selftest/parts.manifest index 05fa1d47..3d87c0a5 100644 --- a/test/selftest/parts.manifest +++ b/test/selftest/parts.manifest @@ -41,3 +41,4 @@ 420-a-deleted-part-must-be-visible.sh 430-the-self-test-must-not-report.sh 440-a-count-grep-never-produced.sh +450-a-red-nightly-must-be-findable.sh From 9d779df6f12fb29614de3b0b3b0312cb473bad86 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 15:12:26 -0600 Subject: [PATCH 2/7] ci: the reporter must not list itself among the failures (#973) The first fire drill caught this rather than passing cleanly, which is what the drill is for. The job-name filter was NEGATIVE -- `conclusion != "success" and != "skipped" and != null` -- and this job is still RUNNING when it asks the API, so it matched its own in-progress row and the issue read: Jobs that did not succeed: - report a red nightly On a genuine red that names the reporter beside the real failures, which is the noise that teaches a reader to stop believing an alert. Selecting POSITIVELY (failure, cancelled, timed_out) cannot admit a state nobody thought of. Same lesson as every other instrument fault this week, and the first one caught by the mechanism under test exercising itself. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .github/workflows/nightly.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 8f6c0e49..0f67e4fd 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -652,7 +652,12 @@ jobs: names=() if [ "$verdict" = failure ]; then while IFS= read -r n; do [ -n "$n" ] && names+=("$n"); done < <( - gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion != "success" and .conclusion != "skipped" and .conclusion != null) | .name' 2>/dev/null || true) + # SELECT WHAT FAILED, NOT WHAT DID NOT SUCCEED. The first version filtered + # negatively and the fire drill caught it at once: this job is still RUNNING + # when it asks, so it does not match "success" and listed ITSELF as a failing + # job. On a real red the issue would have named the reporter beside the genuine + # failures. A positive list cannot admit a state nobody thought of. + gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") | .name' 2>/dev/null || true) if [ "${DRILL:-false}" = "true" ] && [ "${#names[@]}" -eq 0 ]; then names+=("fire drill: no job actually failed") fi From 1e49190146043adafc26c43e854c1f970cf9bf38 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 15:26:37 -0600 Subject: [PATCH 3/7] ci: one list, and the drill exercises the decision too (#973) @OffgridwithJD found #973 reintroduced inside the fix for #973. There were TWO lists: needs: [suites, sanitizer, coverage, upgrade-guard] guarded by the set-equality arm SUITES/SANITIZER/COVERAGE/UPGRADE env + the for-loop guarded by nothing So the arm I added COMPELLED a contributor to update the guarded list while saying nothing about the one the verdict was computed from. They proved it: a fifth gate job, registered in needs as arm 450 demands and named in testing.md as 240 demands, gives a fully green tree and a nightly whose failure the reporter cannot see. They proposed passing the four results as arguments. That still leaves a hand-maintained list. `toJSON(needs)` carries every entry, so the verdict is derived from ONE list that is already guarded -- nothing to keep in agreement beats a guard that two lists agree. AND IT MAKES THE DECISION TESTABLE, which answers their other point: the drill proved delivery completely and the decision not at all. It no longer overrides the verdict; it feeds a synthetic failing context through the same derivation every real night uses. selftest 450 drives that derivation with tuples, including a gate the part has never heard of, which the old shape could not express. THE REAL CONTEXT IS PRINTED ON EVERY RUN, including drills. The fix rests on toJSON(needs) carrying every entry, and that was an assumption about the platform rather than something seen. A drill now shows all four gates as `skipped`, so a gate missing from that line is visible without waiting for a real red. Also theirs: `gh issue list --limit 100` goes blind past a hundred open issues, the lookup returns empty, and every red opens a NEW issue -- which is the "notifier people filter" this design rests on not being. It now narrows server-side with `in:title` and still matches exactly here, because --search is full-text and would find the issues we have filed ABOUT this mechanism. AND A ZERO-COUNT ARM NOW CARRIES ITS OWN POSITIVE CASE. Probing one of these arms with an UNESCAPED `${{` read 0 against a line that plainly contains it, because `$` before `{` is not literal to GNU grep. That one reddened. The same slip in an arm wanting zero passes vacuously, so the zero-count arm reintroduces the old shape into a copy and requires the pattern to see it. Verified: harness_selftest 828/828 on PG16 with 24 checks in the new part, shellcheck clean, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .github/scripts/nightly-report.sh | 68 ++++++++++++++++- .github/workflows/nightly.yml | 68 ++++++++++------- .../450-a-red-nightly-must-be-findable.sh | 73 ++++++++++++++++++- 3 files changed, 177 insertions(+), 32 deletions(-) diff --git a/.github/scripts/nightly-report.sh b/.github/scripts/nightly-report.sh index 4a5b90b8..7f248e97 100755 --- a/.github/scripts/nightly-report.sh +++ b/.github/scripts/nightly-report.sh @@ -25,8 +25,62 @@ set -euo pipefail TITLE="${PGC_NIGHTLY_REPORT_TITLE:-nightly deep gate is red}" -verdict="${1:?usage: nightly-report.sh [failed-job ...]}" -shift || true + +# THE VERDICT IS DERIVED HERE, FROM THE `needs` CONTEXT, AND THAT IS THE POINT. +# +# The first version computed it in the workflow from four named environment +# variables, which made TWO lists: `needs: [...]`, guarded by a set-equality arm +# in selftest 450, and the env block, guarded by nothing. A contributor adding a +# gate was COMPELLED by the arm to update the guarded list and told nothing about +# the one the verdict actually came from -- so the new gate could burn while this +# reported green. #973 reintroduced inside the fix for #973, found by +# @OffgridwithJD, who added a fifth job to a copy of the branch and got a fully +# green tree with a nightly whose failure the reporter could not see. +# +# `toJSON(needs)` carries every entry, so there is one list and nothing to keep in +# agreement -- which beats a guard asserting that two lists agree. +# +# AND IT MAKES THE DECISION TESTABLE. The workflow can only be read; this can be +# DRIVEN, so selftest 450 feeds it tuples and the fire drill exercises the real +# decision path instead of overriding the answer. +verdict_from_needs() { # verdict_from_needs -> failure|success + local json="$1" results r + # `skipped` is not a failure: a fork run, and a fire drill, skip every gate. + results="$(printf '%s' "$json" | python3 -c ' +import json, sys +d = json.load(sys.stdin) +if not isinstance(d, dict) or not d: + sys.exit(3) +for v in d.values(): + print((v or {}).get("result", "missing")) +')" || return 3 + [ -n "$results" ] || return 3 + for r in $results; do + case "$r" in + failure|cancelled|timed_out) echo failure; return 0 ;; + success|skipped) ;; + # A result this does not know is NOT a pass. It means the platform + # grew a state while this did not, and treating it as green restores + # the silence #973 is about. + *) echo failure; return 0 ;; + esac + done + echo success +} + +if [ "${1:-}" = "--from-needs" ]; then + # AN EMPTY OR UNPARSEABLE CONTEXT IS REFUSED, not defaulted. `needs` empty + # means the job list changed shape; reporting green on that is the failure + # mode this whole file exists to remove. + if ! verdict="$(verdict_from_needs "${2:?usage: --from-needs }")"; then + echo "nightly-report: the needs context was empty or unparseable" >&2 + exit 3 + fi + shift 2 +else + verdict="${1:?usage: nightly-report.sh [job ...]}" + shift || true +fi failed=("$@") run_url="${PGC_NIGHTLY_RUN_URL:-${GITHUB_SERVER_URL:-https://github.com}/${GITHUB_REPOSITORY:-}/actions/runs/${GITHUB_RUN_ID:-}}" @@ -80,7 +134,15 @@ fi # EXACT TITLE MATCH, not a search-relevance match. `gh issue list --search` is a # full-text query and would find any issue mentioning these words -- including # the ones the two of us have filed ABOUT this mechanism. -existing="$(gh issue list --state open --limit 100 --json number,title \ +# NARROWED SERVER-SIDE, THEN MATCHED EXACTLY HERE. A bare `--limit 100` silently +# misses the target once the tracker holds more than a hundred open issues, the +# lookup returns empty, and every red opens a NEW issue -- which is precisely "a +# notifier people filter", the thing this design rests on not being +# (@OffgridwithJD). The search narrows the window; the exact match is still done +# here, because `--search` is full-text and would also find the issues the two of +# us have filed ABOUT this mechanism. +existing="$(gh issue list --state open --limit 100 --search "\"$TITLE\" in:title" \ + --json number,title \ --jq "map(select(.title == \"$TITLE\")) | .[0].number // empty")" if [ "$verdict" = failure ]; then diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 0f67e4fd..c65dbbcb 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -630,36 +630,48 @@ jobs: - name: open, update or close the nightly's issue env: GH_TOKEN: ${{ github.token }} - SUITES: ${{ needs.suites.result }} - SANITIZER: ${{ needs.sanitizer.result }} - COVERAGE: ${{ needs.coverage.result }} - UPGRADE: ${{ needs.upgrade-guard.result }} + # ONE LIST, NOT TWO. An earlier version named the four results in four + # environment variables, which meant `needs:` was guarded by selftest 450 + # and the verdict was computed from a second list guarded by nothing -- + # so a contributor adding a gate was compelled to update the guarded list + # and told nothing about the one that decided the verdict + # (@OffgridwithJD, who added a fifth job and got a fully green tree with + # a nightly whose failure this could not see). + # + # toJSON(needs) carries every entry, so there is nothing to keep in + # agreement. + NEEDS_JSON: ${{ toJSON(needs) }} DRILL: ${{ github.event.inputs.fire_drill }} run: | set -euo pipefail - verdict=success - for r in "$SUITES" "$SANITIZER" "$COVERAGE" "$UPGRADE"; do - # `skipped` is not a failure: a fork run skips every gated job. - case "$r" in failure|cancelled|timed_out) verdict=failure ;; esac - done - # A FIRE DRILL GOES THROUGH THIS SAME PATH, differing only in where the - # verdict came from. A drill that posts by a separate route proves the - # route, not the mechanism -- and a notifier never seen to fire is - # indistinguishable from none, which is what #973 is about. - if [ "${DRILL:-false}" = "true" ]; then verdict=failure; fi - echo "suites=$SUITES sanitizer=$SANITIZER coverage=$COVERAGE upgrade-guard=$UPGRADE -> $verdict" + # THE DRILL SUBSTITUTES THE INPUT, NOT THE ANSWER. An earlier version set + # verdict=failure directly, which meant the drill proved delivery and + # never touched the decision -- and the decision is the half that can + # fail the #973 way. It now feeds a synthetic context through the same + # derivation every real night uses. + # THE REAL CONTEXT IS PRINTED FIRST, ALWAYS, INCLUDING ON A DRILL. + # The whole fix rests on toJSON(needs) carrying every entry, and that was + # an assumption about the platform rather than something anyone had seen. + # Printing it makes every run -- including the cheap drill -- an + # observation of it: a drill shows all four gates as `skipped`, so a gate + # missing from this line is visible without waiting for a real red. + echo "needs context as given: $NEEDS_JSON" + needs_json="$NEEDS_JSON" + if [ "${DRILL:-false}" = "true" ]; then + needs_json='{"fire-drill":{"result":"failure"}}' + echo "fire drill: substituting a failing context through the same derivation" + fi + echo "verdict computed from: $needs_json" names=() - if [ "$verdict" = failure ]; then - while IFS= read -r n; do [ -n "$n" ] && names+=("$n"); done < <( - # SELECT WHAT FAILED, NOT WHAT DID NOT SUCCEED. The first version filtered - # negatively and the fire drill caught it at once: this job is still RUNNING - # when it asks, so it does not match "success" and listed ITSELF as a failing - # job. On a real red the issue would have named the reporter beside the genuine - # failures. A positive list cannot admit a state nobody thought of. - gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") | .name' 2>/dev/null || true) - if [ "${DRILL:-false}" = "true" ] && [ "${#names[@]}" -eq 0 ]; then - names+=("fire drill: no job actually failed") - fi - fi - .github/scripts/nightly-report.sh "$verdict" ${names+"${names[@]}"} + while IFS= read -r n; do [ -n "$n" ] && names+=("$n"); done < <( + # SELECT WHAT FAILED, NOT WHAT DID NOT SUCCEED. The first version + # filtered negatively and the first fire drill caught it: this job is + # still RUNNING when it asks, so it did not match "success" and listed + # ITSELF as a failing job. A positive list cannot admit a state nobody + # enumerated -- and where it OMITS one (neutral, action_required, + # stale), the reporter says no job name was reported rather than + # printing an empty list. + gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") | .name' 2>/dev/null || true) + + .github/scripts/nightly-report.sh --from-needs "$needs_json" ${names+"${names[@]}"} diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh index ed03819d..52edb6ed 100644 --- a/test/selftest/450-a-red-nightly-must-be-findable.sh +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -43,6 +43,76 @@ check "the reporter waits on EVERY other nightly job (#973)" \ "$(printf '%s' "$_rn_jobs" | md5sum | cut -c1-12)" \ "$(printf '%s' "$_rn_needs" | md5sum | cut -c1-12)" +# ---- ONE LIST, and the verdict driven rather than read --------------------- +# +# The first version computed the verdict in the workflow from four named +# environment variables. That made two lists: `needs:`, guarded by the arm above, +# and the env block, guarded by nothing -- so a contributor adding a gate was +# COMPELLED to update the guarded one and told nothing about the one that decided +# the verdict. @OffgridwithJD added a fifth job to a copy of this branch, +# registered it in `needs` as the arm above demands and named it in testing.md as +# 240 demands, and got a fully green tree with a nightly gate whose failure the +# reporter could not see. #973 reintroduced inside the fix for #973. +# +# `toJSON(needs)` carries every entry, so there is nothing to keep in agreement. +# The arms below drive the derivation, which the old shape could only be read. +check "the verdict comes from the whole needs context, not a second list" \ + "$(grep -c 'NEEDS_JSON: \${{ toJSON(needs) }}' "$_rn_wf")" "1" +# A ZERO-COUNT ARM CARRIES ITS OWN POSITIVE CASE. A pattern that matches nothing +# and a tree that contains nothing both report 0, and only one of those is the +# claim. This reintroduces the old shape into a copy and requires the pattern to +# see it -- which is not hypothetical care: probing this very file with an +# UNESCAPED `${{` read 0 against a line that plainly contains it, because `$` +# before `{` is not literal to GNU grep. That one reddened. In a zero-count arm +# the same slip passes. +_rn_probe="$(mktemp)"; cp "$_rn_wf" "$_rn_probe" +printf ' SUITES: ${{ needs.suites.result }}\n' >> "$_rn_probe" +check "premise: the per-job-variable pattern sees that shape when it is present" \ + "$(grep -cE '^ +[A-Z]+: \$\{\{ needs\.[a-z-]+\.result \}\}' "$_rn_probe")" "1" +check "and no per-job result variable survives to drift from the context" \ + "$(grep -cE '^ +[A-Z]+: \$\{\{ needs\.[a-z-]+\.result \}\}' "$_rn_wf")" "0" +rm -f "$_rn_probe" + +_rn_v() { # _rn_v JSON -> the derived verdict + PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs "$1" 2>&1 | + sed -n 's/^verdict: //p' +} +check "every gate green is a green verdict" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"success"}}')" "success" +check "one gate failing is a red verdict" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"failure"}}')" "failure" +check "a cancelled gate is a red verdict, not a shrug" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"cancelled"}}')" "failure" +check "all skipped is green, which is a fork run and a fire drill" \ + "$(_rn_v '{"a":{"result":"skipped"},"b":{"result":"skipped"}}')" "success" + +# THE ARM THAT MAKES THE ONE-LIST CLAIM REAL. A job this file has never heard of +# is covered the day it is added, because the derivation reads the context rather +# than a list anyone maintains. Under the old shape this could not be expressed: +# the verdict came from four names and a fifth was invisible to it. +check "a gate nobody listed here still decides the verdict (#973)" \ + "$(_rn_v '{"a":{"result":"success"},"b":{"result":"success"},"brand-new-gate":{"result":"failure"}}')" \ + "failure" + +# A RESULT THE SCRIPT DOES NOT KNOW IS NOT A PASS. The platform growing a state +# while this did not is exactly how a reporter goes quiet. +check "an unrecognised result is refused rather than treated as green" \ + "$(_rn_v '{"a":{"result":"brand_new_state"}}')" "failure" + +# An empty context means the job list changed shape. Reporting green on that is +# the failure this whole file exists to remove. +PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs '{}' >/dev/null 2>&1 && _rn_erc=0 || _rn_erc=$? +check "an empty needs context is refused, not defaulted to green" \ + "$_rn_erc" "3" + +# ---- the issue lookup must not go blind on a busy tracker ------------------ +# +# A bare `--limit 100` silently misses the target once more than a hundred issues +# are open: the lookup returns empty and every red opens a NEW issue, which is the +# "notifier people filter" this design rests on not being (@OffgridwithJD). +check "the issue lookup narrows server-side rather than paging blindly" \ + "$(grep -c 'in:title' "$_rn_sh")" "1" + # It must run on failure AND on success, or the close path never happens. check "the reporter runs on every outcome, not only on failure" \ "$(grep -c '^ if: always() &&' "$_rn_wf")" "1" @@ -88,4 +158,5 @@ _rn_bare="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" failure 2>&1)" check "a failure naming no job says so rather than printing an empty list" \ "$(printf '%s' "$_rn_bare" | grep -c 'No job name was reported')" "1" -unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare +unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe +unset -f _rn_v From c4a95f4078be529dca138be214affd40499557b4 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 15:31:20 -0600 Subject: [PATCH 4/7] ci: a drill must say it is a drill, and a refusal must not read as a crash (#973) Two findings, one from the third fire drill and one from @OffgridwithJD's review. Both are about a reader's first glance, which is the only thing this mechanism has. A DRILL MUST IDENTIFY ITSELF. Rewriting the step to derive the verdict from toJSON(needs) dropped the drill marker. Drill 3's issue read "No job name was reported with this failure" and nothing said it was a drill -- a reader finding it would conclude the nightly was genuinely red. An alert that cries wolf teaches a reader to discount it, which is the state #973 exists to leave. Two arms now pin the marker and the fact that it reaches the printed job list. A REFUSAL MUST NOT READ AS A CRASH. An unparseable context dumped a Python traceback before the script's own message. Behaviour was right -- rc 3, refused, never green -- but a traceback in a CI log sends the reader after a bug in the reporter instead of after the job list changing shape. with the traceback 3 matching lines the arm reddens without it 0 and the message survives THE REMOVAL PROOF FOR THAT ALMOST WENT IN WRONG. My first attempt used a sed whose delimiter clashed with the / in the pattern; it errored, the file was never mutated, and the "0 traceback lines" came from the unmutated script -- which reads exactly like a passing proof. Redone with the mutation asserted and the file compared. That is this repository's own recorded trap and it nearly reached a PR body as evidence. AND toJSON(needs) IS NOW OBSERVED RATHER THAN ASSUMED. Drill 3 printed all four gates as `skipped`, so the platform behaviour the whole fix rests on is visible on every run, drills included. Verified: harness_selftest 832/832 on PG16 with 28 checks in the new part, shellcheck clean, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .github/scripts/nightly-report.sh | 2 +- .github/workflows/nightly.yml | 6 +++++ .../450-a-red-nightly-must-be-findable.sh | 22 ++++++++++++++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/.github/scripts/nightly-report.sh b/.github/scripts/nightly-report.sh index 7f248e97..722ba8fa 100755 --- a/.github/scripts/nightly-report.sh +++ b/.github/scripts/nightly-report.sh @@ -53,7 +53,7 @@ if not isinstance(d, dict) or not d: sys.exit(3) for v in d.values(): print((v or {}).get("result", "missing")) -')" || return 3 +' 2>/dev/null)" || return 3 [ -n "$results" ] || return 3 for r in $results; do case "$r" in diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index c65dbbcb..39c623a7 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -660,6 +660,11 @@ jobs: if [ "${DRILL:-false}" = "true" ]; then needs_json='{"fire-drill":{"result":"failure"}}' echo "fire drill: substituting a failing context through the same derivation" + # AND THE ISSUE MUST SAY IT IS A DRILL. Drill 3 did not: rewriting this + # step dropped the marker, the job list came back empty, and the body + # read exactly like a genuine red. An alert that cries wolf teaches a + # reader to discount it, which is the state #973 exists to leave. + drill_name="fire drill for #973: no gate actually failed" fi echo "verdict computed from: $needs_json" @@ -674,4 +679,5 @@ jobs: # printing an empty list. gh run view "$GITHUB_RUN_ID" --json jobs --jq '.jobs[] | select(.conclusion == "failure" or .conclusion == "cancelled" or .conclusion == "timed_out") | .name' 2>/dev/null || true) + [ -n "${drill_name:-}" ] && names+=("$drill_name") .github/scripts/nightly-report.sh --from-needs "$needs_json" ${names+"${names[@]}"} diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh index 52edb6ed..64ab0b38 100644 --- a/test/selftest/450-a-red-nightly-must-be-findable.sh +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -105,6 +105,26 @@ PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs '{}' >/dev/null 2>&1 && check "an empty needs context is refused, not defaulted to green" \ "$_rn_erc" "3" +# A REFUSAL MUST READ AS A REFUSAL, NOT AS A CRASH. The parse dumped a Python +# traceback before the script's own message (@OffgridwithJD). Behaviour was right +# -- rc 3, never green -- but in a CI log a traceback sends the reader after a bug +# in the reporter instead of after the job list changing shape, and this +# mechanism's whole value is that a reader can tell at a glance what happened. +_rn_noise="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" --from-needs 'not json' 2>&1 || true)" +check "an unparseable context says so without a traceback" \ + "$(printf '%s' "$_rn_noise" | grep -c 'Traceback\|JSONDecodeError')" "0" +check "and it still says what went wrong, rather than saying nothing" \ + "$(printf '%s' "$_rn_noise" | grep -c 'empty or unparseable')" "1" + +# A DRILL MUST SAY IT IS A DRILL. Drill 3 did not: rewriting the step to derive +# the verdict from the needs context dropped the marker, the job list came back +# empty, and the issue body read exactly like a genuine red. An alert that cries +# wolf teaches a reader to discount it, which is the state #973 exists to leave. +check "a fire drill identifies itself in the issue it opens" \ + "$(grep -c 'fire drill for #973: no gate actually failed' "$_rn_wf")" "1" +check "and that marker reaches the job list the reporter prints" \ + "$(grep -c 'names+=("\$drill_name")' "$_rn_wf")" "1" + # ---- the issue lookup must not go blind on a busy tracker ------------------ # # A bare `--limit 100` silently misses the target once more than a hundred issues @@ -158,5 +178,5 @@ _rn_bare="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" failure 2>&1)" check "a failure naming no job says so rather than printing an empty list" \ "$(printf '%s' "$_rn_bare" | grep -c 'No job name was reported')" "1" -unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe +unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe _rn_noise unset -f _rn_v From 224453198c0a9cda2da5699494519a51db1eedc9 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Fri, 11 Sep 2026 15:41:30 -0600 Subject: [PATCH 5/7] ci: the issue lookup must be strongly consistent, not merely wide (#973) The fifth fire drill found this, in a fix @OffgridwithJD suggested and I implemented. `--limit 100` alone silently misses the target once more than a hundred issues are open: the lookup returns empty and every red opens a NEW issue, which is the "notifier people filter" this design rests on not being. `--search "" in:title` fixes that and introduces a worse one. MEASURED: immediately after closing #980 the search still reported it open, and caught up seconds later. GitHub's search index is eventually consistent. A lag in the other direction misses a freshly-opened issue and opens a DUPLICATE -- the same failure the single-issue design exists to prevent, by a different route -- and `concurrency: nightly` queues rather than cancels, so two runs can land close together. `gh issue list` without `--search` reads the REST collection, which is strongly consistent, and gh paginates past 100 on its own. A high limit therefore has neither problem. The exact match stays client-side because `--search` is full-text and would also match the issues we have filed ABOUT this mechanism. Two arms: the lookup reads past the first hundred, and it does not use the search index. Fifth defect found by the drills, none of which the 29 selftest arms could see, and four of the five would have reached a real red night. Verified: harness_selftest 833/833 on PG16 with 29 checks in the new part, shellcheck clean, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .github/scripts/nightly-report.sh | 29 +++++++++++++------ .../450-a-red-nightly-must-be-findable.sh | 18 ++++++++---- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/.github/scripts/nightly-report.sh b/.github/scripts/nightly-report.sh index 722ba8fa..209a191b 100755 --- a/.github/scripts/nightly-report.sh +++ b/.github/scripts/nightly-report.sh @@ -134,15 +134,26 @@ fi # EXACT TITLE MATCH, not a search-relevance match. `gh issue list --search` is a # full-text query and would find any issue mentioning these words -- including # the ones the two of us have filed ABOUT this mechanism. -# NARROWED SERVER-SIDE, THEN MATCHED EXACTLY HERE. A bare `--limit 100` silently -# misses the target once the tracker holds more than a hundred open issues, the -# lookup returns empty, and every red opens a NEW issue -- which is precisely "a -# notifier people filter", the thing this design rests on not being -# (@OffgridwithJD). The search narrows the window; the exact match is still done -# here, because `--search` is full-text and would also find the issues the two of -# us have filed ABOUT this mechanism. -existing="$(gh issue list --state open --limit 100 --search "\"$TITLE\" in:title" \ - --json number,title \ +# A HIGH LIMIT ON THE PLAIN LIST, NOT A SEARCH, AND BOTH HALVES OF THAT WERE +# MEASURED RATHER THAN CHOSEN. +# +# `--limit 100` alone silently misses the target once more than a hundred issues +# are open: the lookup returns empty and every red opens a NEW issue, which is +# precisely "a notifier people filter" -- the thing this design rests on not being +# (@OffgridwithJD). +# +# `--search` fixes that and introduces a worse one. GitHub's search index is +# EVENTUALLY CONSISTENT: measured here, immediately after closing an issue the +# search still reported it open, and caught up seconds later. A lag in the other +# direction misses a freshly-opened issue and opens a duplicate -- the same +# failure, arriving by a different route, and this mechanism can run twice in +# quick succession because `concurrency: nightly` queues rather than cancels. +# +# `gh issue list` without `--search` reads the REST collection, which is strongly +# consistent, and gh paginates past 100 on its own. So a high limit has neither +# problem. The exact match stays client-side: `--search` would also have matched +# the issues the two of us have filed ABOUT this mechanism. +existing="$(gh issue list --state open --limit 1000 --json number,title \ --jq "map(select(.title == \"$TITLE\")) | .[0].number // empty")" if [ "$verdict" = failure ]; then diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh index 64ab0b38..e2253d72 100644 --- a/test/selftest/450-a-red-nightly-must-be-findable.sh +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -127,11 +127,19 @@ check "and that marker reaches the job list the reporter prints" \ # ---- the issue lookup must not go blind on a busy tracker ------------------ # -# A bare `--limit 100` silently misses the target once more than a hundred issues -# are open: the lookup returns empty and every red opens a NEW issue, which is the -# "notifier people filter" this design rests on not being (@OffgridwithJD). -check "the issue lookup narrows server-side rather than paging blindly" \ - "$(grep -c 'in:title' "$_rn_sh")" "1" +# `--limit 100` silently misses the target once more than a hundred issues are +# open: the lookup returns empty and every red opens a NEW issue, the "notifier +# people filter" this design rests on not being (@OffgridwithJD). +# +# AND `--search` IS THE WRONG FIX, measured rather than reasoned: GitHub's search +# index is eventually consistent, and immediately after a close it still reported +# the issue open. A lag the other way misses a freshly-opened issue and opens a +# duplicate -- the same failure by another route. The plain list reads the REST +# collection, which is strongly consistent, and gh paginates past 100 itself. +check "the issue lookup reads past the first hundred open issues" \ + "$(grep -c 'gh issue list --state open --limit 1000' "$_rn_sh")" "1" +check "and does not use the eventually-consistent search index" \ + "$(grep -c 'in:title' "$_rn_sh")" "0" # It must run on failure AND on success, or the close path never happens. check "the reporter runs on every outcome, not only on failure" \ From 2ea9d3ebfd6c49355e68e6b373b80d6983d08c90 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" <jd@commandprompt.com> Date: Fri, 11 Sep 2026 15:47:36 -0600 Subject: [PATCH 6/7] test: count the lookup over code, not over the file (#973) @OffgridwithJD noticed that `--search` appears FOUR times in nightly-report.sh and ZERO times as a call: the comments record why the obvious fix is wrong, which is the most valuable kind of comment and also the kind that satisfies a grep wanting the call. That is the trap they recorded two hours earlier, demonstrated in the same file that taught it. The arm asserting no search-index query survives counted `in:title` over the WHOLE file and got 0. Verified it reddens on a reintroduced call -- both halves do -- but the 0 was LUCK: the prose says `--search` and never says `in:title`. A later comment mentioning the token would have reddened it for a prose reason. Now counted over code, with the strip's own premise, because a grep over an emptied file reports the same 0 as a grep that matched nothing: over the whole file --search = 4 the trap over code only --search|in:title = 0 premise the lookup survived the strip = 1 Verified: harness_selftest 834/834 on PG16 with 30 checks in part 450, shellcheck clean, docs_style 9/9. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- .../450-a-red-nightly-must-be-findable.sh | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/test/selftest/450-a-red-nightly-must-be-findable.sh b/test/selftest/450-a-red-nightly-must-be-findable.sh index e2253d72..7f24888d 100644 --- a/test/selftest/450-a-red-nightly-must-be-findable.sh +++ b/test/selftest/450-a-red-nightly-must-be-findable.sh @@ -136,10 +136,23 @@ check "and that marker reaches the job list the reporter prints" \ # the issue open. A lag the other way misses a freshly-opened issue and opens a # duplicate -- the same failure by another route. The plain list reads the REST # collection, which is strongly consistent, and gh paginates past 100 itself. +# COUNTED OVER CODE, NOT OVER THE FILE. `--search` appears FOUR times in this +# script and zero times as a call -- the comments record why the obvious fix is +# wrong, which is the most valuable kind of comment and also the kind that +# satisfies a grep wanting the call (@OffgridwithJD, in the same file that taught +# us both that). Counting `in:title` over the whole file happens to give 0 only +# because the prose says `--search` and never says `in:title`. That is luck. +_rn_code="$(mktemp)" +grep -v '^[[:space:]]*#' "$_rn_sh" > "$_rn_code" +# AND THE STRIP MUST NOT HAVE STRIPPED THE CODE. A grep over an emptied file +# reports the same 0 as a grep that matched nothing. +check "premise: removing comments left the lookup itself in place" \ + "$(grep -c 'gh issue list --state open' "$_rn_code")" "1" check "the issue lookup reads past the first hundred open issues" \ - "$(grep -c 'gh issue list --state open --limit 1000' "$_rn_sh")" "1" -check "and does not use the eventually-consistent search index" \ - "$(grep -c 'in:title' "$_rn_sh")" "0" + "$(grep -c 'gh issue list --state open --limit 1000' "$_rn_code")" "1" +check "and no search-index query survives in the code" \ + "$(grep -cE -- '--search|in:title' "$_rn_code")" "0" +rm -f "$_rn_code" # It must run on failure AND on success, or the close path never happens. check "the reporter runs on every outcome, not only on failure" \ @@ -186,5 +199,5 @@ _rn_bare="$(PGC_NIGHTLY_REPORT_DRYRUN=1 bash "$_rn_sh" failure 2>&1)" check "a failure naming no job says so rather than printing an empty list" \ "$(printf '%s' "$_rn_bare" | grep -c 'No job name was reported')" "1" -unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe _rn_noise +unset _rn_wf _rn_sh _rn_jobs _rn_n _rn_needs _rn_fail _rn_ok _rn_rc _rn_bare _rn_erc _rn_probe _rn_noise _rn_code unset -f _rn_v From 04147d066d1ce971e7c8ecbc259ca14d454da358 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" <jd@commandprompt.com> Date: Fri, 11 Sep 2026 16:01:15 -0600 Subject: [PATCH 7/7] test: regenerate the ledger for the thirty red-nightly checks (#973) #975 landed first, so this PR pays the tax second. Done in the order @OffgridwithJD established, because the earlier census was derived against 847 and was stale before anyone reviewed it: rebase onto f69ed1e3 (6 commits replayed, no conflict) FRESH log on the REBASED tree, not the pre-rebase one GUARD that log before it goes near the ledger merge it DERIVE the census from the resulting file The guard, because a log is an input and an input is a claim: verdict=PASSED fails=0 checks=846 unrunnable=0 records=846 records == checks run, and the total is above a floor DERIVED, NOT ADDED. The merge reports rows=889 and the census is counted from the file: 889 rows, 889 never, 0 ever red, and the partition closes. It happens to equal 859 + 30. Writing that by addition would have been right today and wrong the first time a merge order surprises me, which has now happened to the other PR in this queue twice in one afternoon. Thirty rows added, zero removed, all in 450-a-red-nightly-must-be-findable. Ceiling unchanged at 250: these checks are in a suite already covered, so nothing about coverage moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw --- test/check_ledger.tsv | 30 ++++++++++++++++++++++++++++++ test/check_ledger_budget.txt | 2 +- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/test/check_ledger.tsv b/test/check_ledger.tsv index ef8e0c5c..b885d8b6 100644 --- a/test/check_ledger.tsv +++ b/test/check_ledger.tsv @@ -813,6 +813,36 @@ harness_selftest 440-a-count-grep-never-produced premise: nor is one inside a ge harness_selftest 440-a-count-grep-never-produced premise: the sweep has a corpus to read never - harness_selftest 440-a-count-grep-never-produced premise: while a numeric comparison is not an offence never - harness_selftest 440-a-count-grep-never-produced premise: while a valid pattern prints a number never - +harness_selftest 450-a-red-nightly-must-be-findable a cancelled gate is a red verdict, not a shrug never - +harness_selftest 450-a-red-nightly-must-be-findable a failure names the run so a reader can reach it never - +harness_selftest 450-a-red-nightly-must-be-findable a failure naming no job says so rather than printing an empty list never - +harness_selftest 450-a-red-nightly-must-be-findable a fire drill identifies itself in the issue it opens never - +harness_selftest 450-a-red-nightly-must-be-findable a gate nobody listed here still decides the verdict (#973) never - +harness_selftest 450-a-red-nightly-must-be-findable a green run composes the closing message, so the alert clears itself never - +harness_selftest 450-a-red-nightly-must-be-findable all skipped is green, which is a fork run and a fire drill never - +harness_selftest 450-a-red-nightly-must-be-findable an empty needs context is refused, not defaulted to green never - +harness_selftest 450-a-red-nightly-must-be-findable an unparseable context says so without a traceback never - +harness_selftest 450-a-red-nightly-must-be-findable an unrecognised result is refused rather than treated as green never - +harness_selftest 450-a-red-nightly-must-be-findable an unrecognised verdict is refused rather than treated as green never - +harness_selftest 450-a-red-nightly-must-be-findable and it is granted issues: write, which the workflow default withholds never - +harness_selftest 450-a-red-nightly-must-be-findable and it still says what went wrong, rather than saying nothing never - +harness_selftest 450-a-red-nightly-must-be-findable and names the job that failed, not just that something did never - +harness_selftest 450-a-red-nightly-must-be-findable and no per-job result variable survives to drift from the context never - +harness_selftest 450-a-red-nightly-must-be-findable and no search-index query survives in the code never - +harness_selftest 450-a-red-nightly-must-be-findable and that marker reaches the job list the reporter prints never - +harness_selftest 450-a-red-nightly-must-be-findable and the green path reuses the same title, or it would close nothing never - +harness_selftest 450-a-red-nightly-must-be-findable every gate green is a green verdict never - +harness_selftest 450-a-red-nightly-must-be-findable one gate failing is a red verdict never - +harness_selftest 450-a-red-nightly-must-be-findable premise: removing comments left the lookup itself in place never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the nightly workflow and the reporter are both present never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the other nightly jobs were parsed, so the comparison is real never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the per-job-variable pattern sees that shape when it is present never - +harness_selftest 450-a-red-nightly-must-be-findable premise: the reporter's needs list was parsed never - +harness_selftest 450-a-red-nightly-must-be-findable the issue lookup reads past the first hundred open issues never - +harness_selftest 450-a-red-nightly-must-be-findable the reporter runs on every outcome, not only on failure never - +harness_selftest 450-a-red-nightly-must-be-findable the reporter waits on EVERY other nightly job (#973) never - +harness_selftest 450-a-red-nightly-must-be-findable the title carries no run number, so every red lands on one issue never - +harness_selftest 450-a-red-nightly-must-be-findable the verdict comes from the whole needs context, not a second list never - native_join_runtime_filter native_join_runtime_filter 3-table answer equals filter-off never - native_join_runtime_filter native_join_runtime_filter 3-table join order matches filter-off never - native_join_runtime_filter native_join_runtime_filter 3-table plan has coordinator never - diff --git a/test/check_ledger_budget.txt b/test/check_ledger_budget.txt index 81741bf1..63961bdf 100644 --- a/test/check_ledger_budget.txt +++ b/test/check_ledger_budget.txt @@ -34,4 +34,4 @@ suites_not_covered 250 # Without that it is a hand-maintained count that drifts, which is the failure # this repository has spent a day proving. It is not a ceiling; it is a # measurement that must be true. -checks_never_observed_red 859 +checks_never_observed_red 889