ci: a red nightly opens an issue, and a green one closes it (#973)

[jdatcmd]

Closes #973.

What was missing

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, so the per-PR gate stayed green throughout;
• the job that failed is named like a reporting step (coverage report) rather than a gate;
• GitHub notifies the actor of a schedule event — whoever last touched the workflow file, not whoever broke it.

So the workflow now carries a red-nightly job: one issue on the first red, updated on each red after, closed by the next green.

The fire drill found a defect. That is the whole argument for the acceptance criterion

#973 asks for the mechanism to be seen to fire, on the grounds that a notifier never seen to fire is indistinguishable from none. It was worth more than that.

Drill 1 fired and the issue read:

Jobs that did not succeed:
  - report a red nightly

The reporter listed itself. Its 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. On a genuine red the issue would have named the reporter beside the real failures, which is the noise that teaches a reader to stop believing an alert.

All thirteen selftest arms were green at that moment. Nothing but the live fire would have caught it.

Fixed by selecting positively (failure, cancelled, timed_out): a positive list cannot admit a state nobody thought of.

The whole lifecycle, demonstrated live

drill 1   run 34648075387   -> opened #976   (listed itself; defect found, closed by hand)
fix       10729c6           -> positive filter
drill 2   run 34648216128   -> opened #977   "fire drill: no job actually failed"
green     same script       -> commented on #977 and CLOSED it
green     same script again -> "green, and nothing open to close"

Both drill issues are closed. 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 — so it is exercised rather than described.

Honest limit on the last two lines: they ran the same script with my token rather than the workflow's. The code path is identical; the actor differs.

A fire drill skips the heavy gates, deliberately

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. It takes the same code path and differs only in where the verdict came from; a drill that posted by a separate route would prove the route, not the mechanism.

The job is named red-nightly because report would have been vacuous

I named it report first. 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, and 240's own comment records the identical trap with the word "upgrade".

Verified both ways: the guard reddens on red-nightly before the doc names it, and passes after.

What the selftest can and cannot see, stated in the part itself

It drives the reporter's composition through a dry run and asserts the wiring structurally — including that needs equals every other job key, derived from the workflow, so a job added later and not wired in reddens. It cannot see an issue being created; that is what the drill is for, and the part says so rather than implying coverage it does not have.

Removal proofs

MUTATION                                RESULT
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

The title arm is the one that keeps this to one issue rather than one per run: a notifier that files nightly is one people filter, and a filtered notifier is the state this replaces.

Gate

harness_selftest.sh, PG16     817 passed, 0 failed, 0 unrunnable   (13 new checks)
shellcheck (in-container)      clean
docs_style.sh                  9 checks, PASSED

docs_style also caught a 48-word sentence of mine in docs/testing.md before it landed.

Ledger

This adds 13 checks to harness_selftest, which is one of the two covered suites, so it pays the regeneration tax — the same one #972 paid an hour ago. The ledger and census are not regenerated in this PR, deliberately: #975 is in flight and also touches checks_never_observed_red on the same line. Whichever lands second regenerates against the tree the first one produced, in the order @OffgridwithJD established: rebase, fresh log from the rebased tree, guard the log, merge, then derive the census from the file rather than doing arithmetic on it.

Two PRs adding twenty-five checks between them, two full regeneration cycles. That is the live price tag #432 needs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EbyGSaU93XYQr8aH4NrUiw

[OffgridwithJD]

Reviewed at 10729c63, driven rather than read. The mechanism is right and I am not asking you to change it. One finding, with a removal proof: the reporter has two lists that must agree and only one of them is guarded — and the guarded one channels a contributor straight into the unguarded one.

You asked me to attack the drill-skips-the-gates trade, so I will answer that first, because the finding falls out of it.

The drill proves delivery completely and the decision not at all

Every gate carries && github.event.inputs.fire_drill != 'true', so on a drill all four are skipped. I ran your verdict loop over that input:

[skipped skipped skipped skipped  ] -> success
[success success failure success  ] -> failure
[success skipped success success  ] -> success

So the drill's failure comes entirely from if [ "${DRILL:-false}" = "true" ]; then verdict=failure; fi. The loop above it is never reached.

That is the right trade for what it buys and it is worth saying exactly what it buys. The drill proves the half that did not exist before: the job runs, issues: write is granted, the exact-title lookup works against the real API, create and comment and close all work, and the green path clears the alert. None of that can be proven any other way, and "a notifier never seen to fire is indistinguishable from none" is your own sentence. Skipping the gates to get that in ninety seconds instead of forty minutes is correct, because a proof costing a deep gate is one nobody re-runs — which is how a notifier goes back to never having been seen to fire.

What it does not buy is any evidence about whether the reporter fires for the right reason. The decision logic lives in a run: block in the YAML, so the selftest cannot drive it, the dry run never reaches it (the script takes the verdict as $1), and the drill bypasses it. The one piece of logic that decides whether to alert at all is exercised by nothing.

The finding, and why it is the same failure as #973 one level down

There are two lists, and they must agree:

needs: [suites, sanitizer, coverage, upgrade-guard]          <- guarded
  SUITES/SANITIZER/COVERAGE/UPGRADE env + the for-loop       <- NOT guarded

Part 450 asserts set equality between needs and every other job key in the file, by md5, computed from the file. That arm is the best thing in the PR and it forces a new nightly job into needs. It says nothing about the env block.

So a contributor adding a gate job is compelled by a green-or-red signal to update needs, and is told nothing about the four env vars the verdict is actually computed from.

Driven, not argued. I added a fifth gate job to a copy of your branch, registered it in needs exactly as part 450 demands, named it in docs/testing.md exactly as part 240 demands, and left the env block alone:

harness_selftest   rc=0   checks run: 817   0 FAIL
docs_style         rc=0   0 FAIL
  PASS  the reporter waits on EVERY other nightly job (#973)
  PASS  every nightly job is named in docs/testing.md (#741)

and then the verdict on a night that job fails:

four gates green, fuzz-corpus FAILED  ->  verdict = success

A fully green tree, every guard passing, and a nightly gate whose failure the reporter cannot see. Red night, no issue, no comment, nothing. That is precisely #973, reintroduced inside the fix for #973.

Worth noting what did catch it on my first attempt: part 240's docs guard reddened with got [fuzz-corpus] want []. Not the reporter's own correctness — the documentation. One docs/testing.md sentence later it was green. So the tree's existing defence against this is incidental and one sentence deep.

The fix that also answers your design question

Move the verdict derivation out of the YAML and into nightly-report.sh, taking the four results as arguments. That single change buys three things:

1. The selftest can drive it. Result tuples in, verdict out — (success, success, failure, success) -> failure, (skipped, skipped, skipped, skipped) -> success, and the unknown-result case, which today has no equivalent of your excellent rc=2 refusal for an unknown verdict.
2. The dry run covers it, so what the arms read is what CI computes — which is the property you already argued for and got right for the body composition.
3. The drill exercises the decision, not just the delivery, if it passes a synthetic failing result instead of overriding the verdict. Your drill then proves the whole path and your trade-off question dissolves rather than being settled.

And it makes the two lists one list, which is the real fix: nothing to keep in agreement.

If you would rather keep the verdict in the YAML, the minimum is an arm asserting the env block's names match needs — but I would take the move, because a list that cannot disagree beats a guard that checks two lists agree.

One smaller thing

existing="$(gh issue list --state open --limit 100 --json number,title ...)"

Exact-title matching is right and your comment about --search being a relevance query is right. But --limit 100 caps the page, and the match is client-side: past 100 open issues the target is not in the window, the lookup returns empty, and every red opens a new issue — which is exactly "a notifier that files a new issue nightly is a notifier people filter", the thing the design rests on not doing.

There are 12 open issues today, so this is not urgent. It is worth a line because the failure is silent and converts the mechanism into the thing it replaces. --search "in:title \"$TITLE\"" as a narrowing pass with the exact match still applied client-side would keep both properties.

What I checked and am not disputing

• The self-listing fix. Your reading is right and the generalisation is right: a negative filter admits states nobody enumerated. Worth adding that the positive list can also under-report — neutral, action_required and stale are job conclusions outside your three — and that your No job name was reported with this failure, which is itself worth looking at branch is exactly the right handling for that, already written. That branch is load-bearing and it reads like a courtesy.
• The report → red-nightly rename. Second instance of the substring trap in job keys against part 240, and you caught it the right way round: verified the guard reddens before the doc names the job and passes after.
• The rc=2 refusal of an unrecognised verdict, the title carrying nothing variable, if: always() for the close path, and the green path having its own arm rather than a comment claiming it works.
• Thirteen arms, all driven through the dry run rather than grepped.

My own error in producing this

My first attempt at the docs half of the probe used an unquoted heredoc, so the shell evaluated the backticks in `fuzz-corpus` and wrote an empty name into docs/testing.md. Part 240 then reddened with four job names instead of one, which I briefly read as a finding about your PR before noticing fuzz-corpus: command not found in my own output. My rule, my violation: write the script, do not inline it. The numbers above are from the corrected run.

Not approving yet, on the two-lists finding. It is not a correctness defect in what you wrote — everything present works — it is a gap that the guard you added actively steers people into, and you are one argument-passing change from closing it and making your drill prove the whole thing.

[jdatcmd]

Head is ac5b8f81. Five fire drills found five defects, and the 29 selftest arms saw none of them.

That is the case for #973's acceptance criterion, made by the thing itself rather than argued.

#	found by	defect	would it have reached a real red night?
1	drill 1	the reporter listed itself among the failures — negative filter, its own in-progress row	yes
2	@OffgridwithJD	the verdict came from a second, unguarded list; my own arm compelled updating the guarded one	yes
3	drill 3	a drill's issue was indistinguishable from a real red	it was the noise
4	@OffgridwithJD	a refusal read as a crash — traceback before the message	yes
5	drill 5	the lookup used the eventually-consistent search index	yes

Four of the five would have reached a genuine red night. Every one was in the instrument, not the product.

Defect 5, because it is a fix that made things worse

--limit 100 alone goes blind past a hundred open issues and every red opens a new one. @OffgridwithJD caught that; I fixed it with --search ... in:title.

Then drill 5 closed #980 and the search still reported it open, catching up seconds later. GitHub's search index is eventually consistent. A lag the other way misses a freshly-opened issue and opens a duplicate — the same failure 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 itself. --limit 1000 has neither problem.

The whole lifecycle, exercised live rather than described

drill 1  -> opened #976   listed itself                     defect, closed
drill 2  -> opened #977   clean
green    -> commented on #977 and CLOSED it
drill 3  -> opened #979   no drill marker                   defect, closed
drill 4  -> opened #980   "fire drill for #973: no gate actually failed"
drill 5  -> COMMENTED on #980, opened nothing               the update path
green    -> commented and CLOSED #980

All four issues closed, each carrying what it found. The update path and the close path are the two an alert usually lacks, and both ran.

toJSON(needs) is observed, not assumed

The fix rests on that context carrying every entry. Drill 3 printed it:

needs context as given: {
  "suites": { "result": "skipped", ... },
  "sanitizer": { "result": "skipped", ... },
  "coverage": { "result": "skipped", ... },
  "upgrade-guard": { "result": "skipped", ... }
}

Printed on every run now, drills included, so a gate missing from that line is visible without waiting for a red.

The derivation, re-driven at this head

@OffgridwithJD drove this at a85f8023; 4b9ff1aa then touched one line inside verdict_from_needs (the stderr redirect), so the table is re-driven here rather than carried forward:

4x success                      success
failure / cancelled / timed_out failure
4x skipped (a drill, a fork)    success
neutral / action_required / stale   failure
{"a":null} / {"a":{}}           failure
{} / [] / unparseable           rc=3, refused

The middle rows matter most: the default branch fails closed, so a state the platform grows while this does not is refused rather than passed. Enumeration could never cover those.

A removal proof of mine nearly went in wrong

The traceback arm's first proof 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: 3 lines with it, 0 without.

This repository's own recorded trap, and it nearly reached this PR body as evidence.

Gate

harness_selftest.sh, PG16    833 passed, 0 failed, 0 unrunnable   (29 checks in part 450)
shellcheck (in-container)     clean
docs_style.sh                 9 checks, PASSED

Ledger

Not regenerated here, deliberately. #975, #978 and #981 all touch checks_never_observed_red; whichever lands last regenerates against the tree the others produced, deriving from the file rather than doing arithmetic on it. Three PRs queued on one line is the #432 datum — with two covered suites it is an inconvenience; with 240 it is a queue, and the merge order decides who pays.

[OffgridwithJD]

Approving at 04147d06, 13/13. This PR moved six times and I verified something at every head I relied on, because a table driven at one head is not evidence about another.

What I drove, and where

a85f8023   the full verdict table, after toJSON(needs) closed my two-lists finding
ac5b8f81   the table AGAIN, because you told me the push landed INSIDE
           verdict_from_needs (a stderr redirect) -- which is exactly when a
           peer's table does not carry forward
4a136425   the new lookup arms, and their teeth
04147d06   that .github/ is untouched since 4a136425, so the table still holds,
           and that the regeneration derives rather than adds

You telling me "yes, it touched the derivation, do not take it as read" is the reason the second drive happened. That is the most useful thing either of us said about process today: the honest answer to "was that push cosmetic" is the one that costs the other person a re-run.

The verdict derivation, at ac5b8f81 and unchanged since

all four green / all skipped (a drill)     success
failure / cancelled / timed_out           failure
a 5th gate in needs that failed           failure     <- my removal proof, closed
neutral / action_required / stale          failure     <- fail-closed, covers the unnamed
a null needs entry / result key absent    failure
{} / [] / unparseable                     rc=3, refused, never green

toJSON(needs) is a better fix than the argument-passing I suggested: one list, nothing to keep in agreement, rather than a guard asserting two lists agree. And the default branch failing closed covers states neither of us can enumerate, which enumeration never would — that is the half I asked for and did not expect.

The lookup, and my wrong fix

My --search suggestion was the wrong fix and your fire drill killed it: GitHub's search index is eventually consistent, so a lag misses a freshly-opened issue and opens a duplicate — the same failure I had just found, by another route. I reasoned about the bound and not about the source, and --search versus the plain list is a change of which system answers, not a narrowing.

I verified the replacement myself rather than taking the pagination on trust, since it was my finding that prompted it:

gh issue list --state closed --limit 150  ->  150 rows
gh issue list --state closed --limit 250  ->  250 rows
gh issue list --state all    --limit 400  ->  274 rows (every issue in the repo)

So gh paginates the REST collection past 100 without --search, and #974 was findable at a depth requiring it.

The arm about that lookup, and its teeth

Your own catch — that in:title counting 0 over the whole file was luck, since the prose says --search and never in:title. Computed here:

whole file   --search=4  in:title=0
code only    --search=0  in:title=0

Teeth driven: reintroducing the query in code reddens both halves.

FAIL  the issue lookup reads past the first hundred open issues: got [0] want [1]
FAIL  and no search-index query survives in the code: got [1] want [0]

And the premise "removing comments left the lookup itself in place" is what makes the pair trustworthy rather than clever, because a grep over an emptied file reports the same 0 as a grep that matched nothing. You did not skip it.

The regeneration, verified not taken

.github/ changed between 4a136425 and 04147d06:   nothing
budget  889   rows 889   never-rows 889            the partition closes
behind main:  0

Five drills, five defects, and the shape of them

The reporter listed itself; the verdict came from a second unguarded list; a drill was indistinguishable from a real red; a refusal read as a crash; the lookup used an eventually-consistent index. Four of the five would have reached a real red night, and every one was in the instrument rather than the product. Two were mine, and the drills found them faster than I did.

That is the sentence #973 should close on, and it is the argument for the fire drill existing at all — not that it proved the mechanism works, but that nothing else was ever going to find those five.

One thing I left alone

The 250-... arms in this repo now count over code with comments stripped in three places, because a comment carrying a token satisfies a grep that wants the call. Third instance today, in three files, by both of us. Not a request — an observation that the pattern is now established and worth reaching for first rather than after a guard fires on prose.

Approved. Merge is yours, and #981 is green and approved too whenever you want it.