[Huyvux12]

Summary

nonEmptyList accepted every block indicator it did not recognise, so judged-jobs: >- passed the guard and a night with no judged jobs could look green. This inverts the guard: a value is one of two things, and nothing else — a block scalar (|, |-, |+, >, >-, >+) whose indented body holds a non-comment item, or a non-empty scalar on the same line. Anything the switch does not recognise is now refused: a block indicator the guard cannot parse (| # comment, >- # comment, an indent indicator like |2) is not a value it can vouch for, and neither is a comment-only rest. Refusing is the safe direction — a refused night is visible, while an empty list passed as non-empty restores the #830 race with green gates.

The inversion is by class, not per spelling: a literal default: return false would break the existing accepted inline scalars (incus, 'incus'), so the default refuses only rests starting with |, >, or # and still accepts any other non-empty same-line scalar.

Route taken: the inversion, as the issue asks. Not taken: switching to a YAML parser. That costs a dependency and the zero-dependency rule makes it a decision, not a detail — so this PR stays parser-free and the table test carries the coverage.

Evidence

Base: main @ 4a9c1b3a20e1d57cd38162076fca96d6b2355082.

Repro on that base (new table cases added first, fix not yet applied): go test ./tools/ci/ -run TestNonEmptyListRequiresAValue FAILED — 5 cases returned true, want false: >-, >, >+, | # nothing here, and >- followed by an indented comment only. The control case (folded block with a value) passed on the base.

After the fix: the targeted test passes; full go test ./tools/ci/ -count=1 ok (12.4s); go test -race ./tools/ci/ -count=1 ok (18.2s); gofmt -l tools/ci/ clean; go vet ./tools/ci/ clean; golangci-lint run ./tools/ci/ 0 issues. The guard test reads the real workflow files, so no real caller uses a refused spelling.

Falsification: reverting only the inversion (block case back to the three | spellings, default back to plain return true, new tests kept) makes the test FAIL — at least 4 of the new cases go red. Restoring the fix makes it pass again.

Two of the new cases go one step beyond the issue's literal list, in the same class: > # nothing here (folded header with comment only) and # nothing here (comment only, no value) — a comment is not a value. The still-accepted control is >- followed by an indented incus.

Out of scope, unchanged: the block-sequence spelling (key: followed by - item lines, empty rest) was already refused before this change and still is. That is a separate question about whether the guard should accept sequences at all, not part of #939.

Type of change

• A route added or changed
• Bug fix
• Refactor
• Documentation
• Chore / tooling
• Security / supply chain

Checklist

Always

• mise run check passes (gofmt, vet, golangci-lint, go test -race) — not fully green in my container, and I am not claiming it is. The full run could not complete here: shapes:check passed and gofmt was clean, but the test and lint steps died on disk — first /tmp (512MB tmpfs) filled during the parallel race build, then the home disk quota (EDQUOT) tripped on the build temp. What did run green, on the touched package: gofmt, go vet ./tools/ci/, golangci-lint run ./tools/ci/ (0 issues), go test ./tools/ci/ and go test -race ./tools/ci/. The full-suite result is for CI to give, after a maintainer approves the run.
• No new external Go dependency — test-only change to tools/ci/nightly_reporting_test.go; go.mod untouched.
• internal/core still knows no provider — untouched.
• Nothing in the diff could be written identically for another provider — the diff is a guard in tools/ci, no provider code.
• mise run testplan was run, and what it printed was run — run, but the named replay could not run here. mise run testplan --check printed: 1 path changed against origin/main — run mise run falsify -- tools/falsify/specs/a-silent-night-is-refused.json. I ran it twice; both times the harness exited 2 with "the copy is red before any mutation". The cause is this container, not the diff: the harness shells out through systemd-run --user, and there is no systemd user bus here (/run/user/ absent, "Failed to connect to bus: No medium found"), so the baseline go test never executes. The same failure was disclosed on fix(ci): the night report judges only its declared jobs (#830) #926. Note the spec's mutations target a different part of the same file (the excuse list), not nonEmptyList; the falsification evidence for this change is the table-test revert above.

When a model wrote a substantive part of this — otherwise N/A

• An Assisted-by: trailer names the tool and the model version — in the commit: Assisted-by: Muse (Muse Spark 1.3).
• I ran mise run conformance myself — N/A with a reason, not run: no route or provider is touched, so there is no response shape for conformance to check. The diff is a test-only guard in tools/ci.
• Every field name in the diff comes from the provider's SDK, their OpenAPI document, or a run of the real client — N/A: no field names in the diff.

When a route is added or changed — otherwise N/A

N/A — no route added or changed.

When behaviour a client can observe changes — otherwise N/A

N/A — no documented limit moves and no client-observable response shape changes; the night report still judges the same jobs, and the guard now refuses malformed input as #939 asks.

When .github/workflows/ is touched — otherwise N/A

N/A.

When a machine runtime is involved — otherwise N/A

N/A.

Related issues

Fixes #939

[stephrobert]

Reviewed, and this needs another pass before it can land. Two reviewers read it independently; what follows is what both measured.

On method, so you can judge the numbers: neither of them executed your branch. They copied nonEmptyList out of the patch into their own Go program, ran it against the same tables, and checked what YAML really means for each spelling with PyYAML. So every true/false below is the behaviour of your function, measured, but not a run of your tests.

What is right, and worth saying first

You reproduced the defect on main before taking the issue, and you named the commit. That is the order this repository asks for, and it is rarer than it should be.

The block half is also better than the issue asked for: testing HasPrefix(rest, "|") and HasPrefix(rest, ">") covers every block scalar header YAML can write, so that half is not a list of spellings anyone has to maintain. It is complete.

And the change is measured as neutral on the tree: of the six night-report.yml callers, only runtime-proof.yml passes lists, both in key: |, which both versions accept. Nothing in .github/workflows/ turns red.

What blocks it

1. The comment claims a property the code does not have. The new comment says "Anything the switch does not recognise is refused, not accepted". The default still ends in return true. This repository's most expensive rule is that a comment is not a control, so this line alone would block the merge even if the code were right: a future reader takes it as proved and stops looking.

The PR body carries the same claim and then contradicts it two paragraphs later ("the default refuses only rests starting with |, >, or # and still accepts any other non-empty same-line scalar"). The second sentence is the accurate one.

2. The defect of #939 is still open on the scalar branch. default: return true was kept, so seven spellings that YAML reads as empty still pass the guard. Measured, with the YAML value beside it:

written in a workflow	guard says	YAML value
judged-jobs: ~	non-empty	None
judged-jobs: null	non-empty	None
judged-jobs: Null	non-empty	None
judged-jobs: !!str	non-empty	''
judged-jobs: '' # none yet	non-empty	''
judged-jobs: "" # none yet	non-empty	''
judged-jobs: *names	non-empty	depends on the anchor

'' # none yet is the one that worries us most, because it is what somebody actually writes when they have no list yet, and because the input documents default: ''. A trailing comment is enough to leave the empty-scalar case and land in the permissive default. From there, NIGHT_JUDGED_JOBS is empty, night-report.sh takes the "judge everything" branch, and the #830 race is back with every gate green.

This is why the issue asked for the inversion rather than more exceptions: enumerate what is a value, and an unrecognised spelling refuses by construction instead of by a list someone has to extend.

3. Three real lists are now refused, and no case decided that. The prefix test refuses the header before the body is looked at, so these go from accepted to refused, while PyYAML gives each a real value:

• judged-jobs: | # pick these with an indented body → 'incus\n'
• judged-jobs: >- # pick these with a body → 'incus'
• judged-jobs: |2 with a body → 'incus\n'

The table only added the body-less variants (| # nothing here), so nothing in it says what a commented header with a body should be. The refusal is the safe direction, but the cost here is not a refused night: it is mise run check red on a correct workflow, and the reflex that teaches is to rewrite the YAML to please the guard. If you keep the prefix refusal deliberately, the error message at nightly_reporting_test.go:178 has to stop saying "passes no non-empty judged-jobs list" and say that the spelling is one the guard cannot read, which is the true statement.

4. Two of the six terms are measured by nothing. Removing ">" or ">+" from the case reddens no case in the table, because only a folded block with a body tells the case apart from the default, and the table has exactly one of those, in >-. Same pre-existing hole for "|-" and "|+". The rule here is one case per question the guard asks, and these four close it:

{"folded block indicator with a value",         "      judged-jobs: >\n        incus\n",  true},
{"folded block, keep indicator with a value",   "      judged-jobs: >+\n        incus\n", true},
{"literal block, strip indicator with a value", "      judged-jobs: |-\n        incus\n", true},
{"literal block, keep indicator with a value",  "      judged-jobs: |+\n        incus\n", true},

Plus judged-jobs: ~, judged-jobs: null and judged-jobs: '' # none yet as false, whichever route you take: without them, the property is asserted nowhere.

5. No mutation. tools/falsify/specs/a-silent-night-is-refused.json already covers ./tools/ci/, and no spec in the repository names nonEmptyList or TestNonEmptyListRequiresAValue — not since #926 created the guard. The closing criterion of #939 ("putting the inversion back must redden one of these cases") is therefore unmet. One entry per question, declared against the test, and mise run falsify -- a-silent-night-is-refused exiting 0 with every mutation biting and restoration green.

Not yours to fix

blockList in tools/ci/night_report_test.go:484 parses the same key with a different tolerance and t.Fatals on anything but key+": |". Two parsers for one key is the "written twice" defect, and a fix on one side does not survive on the other. It is out of scope for #939; we will file it.

Where this leaves the PR

NOT READY, on points 1 and 2. The direction the issue asked for closes 1, 2 and 3 at once, which is why we recommend it rather than extending the exception list. If you would rather keep the blocklist, that is a defensible choice, but then the comment must say what the code does, the nulls must be refused explicitly, and the remaining gap needs an issue of its own: a limit written in a comment is not a tracked debt here.

Take the time you need. The analysis above is yours to use, and nobody will touch the issue while it is assigned to you.