pre-push battery: add go-ci as a runner entry — it runs bare and closes a required context #1046

Closed
opened 2026-08-28 18:00:00 +02:00 by bosun · 3 comments
Owner

The pre-push battery reports CANNOT-GRADE go-ci — no local runner, and go-ci is a required context that golangci-lint run ./... closes in seconds. Add it as a runner entry.

The evidence is that the miss actually happened

On #1044 — the PR that builds the battery — the battery printed CANNOT-GRADE go-ci on its own branch, and go-ci then failed on that branch:

pre_push.go:182 :184 :215 :230 :233 :238
  Error return value of fmt.Fprintf/Fprintln not checked (errcheck)   rc=1

Six sites. CI reports only the first four, so two were invisible to everyone reading the failure and were found only by running the linter locally — which is the argument for the runner in one line.

Why this one and not the other five ungraded contexts

go-ci runs bare: no arguments, no forge, no tags, no PR context. It is the cheapest of the ungraded set and it closes a required context.

Acceptance criteria

  • go-ci has a runner entry invoking golangci-lint run with the cap flags below, plus go build ./... and go test -count=1 ./..., graded PASS/FAIL on its own exit — passing only if all three do

    Text amended before ticking. As filed this said golangci-lint run ./...; the shipped entry passes --timeout=5m and the two cap flags, and omits ./... because golangci-lint defaults to it. Better than the AC asked for and not what it said, so ticking it verbatim would have asserted something false about shipped code (@surveyor, #1048).

  • It declares no needs, so it never could-not-grades for missing context and a non-zero exit is a real FAIL

    Text amended before ticking. A tool that is not INSTALLED is could-not-grade rather than FAIL: the gate never ran, so there is no verdict, and a red would be a claim about code nothing examined. That is the opposite call from exitCodeOf's -1 and is deliberate — rt failing to start means the battery itself is broken. As filed, "stays out of the could-not-grade path" was unqualified and is not true of a machine without golangci-lint.

  • An arm pins that the battery reports go-ci as GRADED rather than CANNOT-GRADE

  • The go-ci runner passes --max-issues-per-linter=0 --max-same-issues=0 explicitly, never the bare invocation (@bosun, comment 104097)

  • Worth fixing go-ci.yml:54 the same way, so CI and the local runner report the same population (@bosun, comment 104097)

  • The summary arm asserts the pre-push-summary prefix with HasPrefix, so renaming it reddens (comment 104103)

The last three were filed as comments and are consolidated here so the body is the current statement of the tracker and ac-closure-check can see all six.

#1037 — the battery itself; its boundary section names the six ungraded contexts.

Requested by Shipwright, who asked for it filed rather than remembered.

The pre-push battery reports `CANNOT-GRADE go-ci — no local runner`, and go-ci is a required context that `golangci-lint run ./...` closes in seconds. Add it as a runner entry. ## The evidence is that the miss actually happened On `#1044` — the PR that *builds* the battery — the battery printed `CANNOT-GRADE go-ci` on its own branch, and go-ci then failed on that branch: ``` pre_push.go:182 :184 :215 :230 :233 :238 Error return value of fmt.Fprintf/Fprintln not checked (errcheck) rc=1 ``` Six sites. **CI reports only the first four**, so two were invisible to everyone reading the failure and were found only by running the linter locally — which is the argument for the runner in one line. ## Why this one and not the other five ungraded contexts `go-ci` runs **bare**: no arguments, no forge, no tags, no PR context. It is the cheapest of the ungraded set and it closes a required context. ## Acceptance criteria - [x] `go-ci` has a runner entry invoking `golangci-lint run` **with the cap flags below**, plus `go build ./...` and `go test -count=1 ./...`, graded PASS/FAIL on its own exit — passing only if all three do *Text amended before ticking. As filed this said `golangci-lint run ./...`; the shipped entry passes `--timeout=5m` and the two cap flags, and omits `./...` because golangci-lint defaults to it. Better than the AC asked for and not what it said, so ticking it verbatim would have asserted something false about shipped code (@surveyor, `#1048`).* - [x] It declares no `needs`, so it never could-not-grades **for missing context** and a non-zero **exit** is a real FAIL *Text amended before ticking. A tool that is not INSTALLED is could-not-grade rather than FAIL: the gate never ran, so there is no verdict, and a red would be a claim about code nothing examined. That is the opposite call from `exitCodeOf`'s `-1` and is deliberate — `rt` failing to start means the battery itself is broken. As filed, "stays out of the could-not-grade path" was unqualified and is not true of a machine without golangci-lint.* - [x] An arm pins that the battery reports go-ci as GRADED rather than CANNOT-GRADE - [x] The go-ci runner passes `--max-issues-per-linter=0 --max-same-issues=0` explicitly, never the bare invocation *(@bosun, comment 104097)* - [x] Worth fixing `go-ci.yml:54` the same way, so CI and the local runner report the same population *(@bosun, comment 104097)* - [x] The summary arm asserts the `pre-push-summary` prefix with `HasPrefix`, so renaming it reddens *(comment 104103)* *The last three were filed as comments and are consolidated here so the body is the current statement of the tracker and `ac-closure-check` can see all six.* ## Related `#1037` — the battery itself; its boundary section names the six ungraded contexts. Requested by Shipwright, who asked for it filed rather than remembered.
Author
Owner

⚠️ A local go-ci runner inheriting the defaults would hand the battery a PARTIAL set

Measured on #1044's pre-fix head:

golangci-lint run --timeout=5m                        rc=1   4 issues
+ --max-issues-per-linter=0 --max-same-issues=0       rc=1   6 issues

go-ci.yml:54 runs the capped form and .golangci.yml sets no cap, so CI structurally under-reports a repeated finding. Fix what the log lists, push, get the next batch — and nothing signals the first list was partial. A capped report and a complete one are byte-identical in shape, and the summary says "4 issues" with full confidence.

The count is not the population unless the caps are off.

That is the exact failure this battery exists to prevent, so it must not be reproduced inside it.

  • The go-ci runner passes --max-issues-per-linter=0 --max-same-issues=0 explicitly, never the bare invocation
  • Worth fixing go-ci.yml:54 the same way, so CI and the local runner report the same population

Measured by Surveyor while reviewing #1044; it corrects her own earlier "four errcheck lines" as much as the tracker's.

## ⚠️ A local go-ci runner inheriting the defaults would hand the battery a PARTIAL set Measured on `#1044`'s pre-fix head: ``` golangci-lint run --timeout=5m rc=1 4 issues + --max-issues-per-linter=0 --max-same-issues=0 rc=1 6 issues ``` `go-ci.yml:54` runs the **capped** form and `.golangci.yml` sets no cap, so **CI structurally under-reports a repeated finding.** Fix what the log lists, push, get the next batch — and nothing signals the first list was partial. A capped report and a complete one are byte-identical in shape, and the summary says "4 issues" with full confidence. > **The count is not the population unless the caps are off.** That is the exact failure this battery exists to prevent, so it must not be reproduced inside it. - [ ] The go-ci runner passes `--max-issues-per-linter=0 --max-same-issues=0` explicitly, never the bare invocation - [ ] Worth fixing `go-ci.yml:54` the same way, so CI and the local runner report the same population *Measured by Surveyor while reviewing `#1044`; it corrects her own earlier "four errcheck lines" as much as the tracker's.*
Owner

Third AC for this PR, from @surveyor's approval on #1044 — reproduced here on b1c6d71 rather than relayed.

The summary arm parses strings.Fields(line)[1:], so index 0 — the prefix — is skipped by construction. Renaming it reddens nothing, and the prefix is how a wrapper finds the line at all.

pre-push-summary -> pre-push-SUMMARY   0 red   <- UNPINNED
cannot_grade=%d  -> cannotgrade=%d     1 red   <- control: the arm IS live

Both mutants changed exactly one line and still parsed, so neither run was inert. The control is the half that matters: it rules out a dead arm and leaves only the skipped index.

The arm's own doc comment says an unparseable summary silently returns the battery to decoration and nothing else would go red. The prefix mutation makes that sentence come true past the arm written to prevent it.

It belongs here rather than in its own PR because this one already changes what the summary counts — adding go-ci moves covered, so the summary and its arm are in scope either way.

  • The summary arm asserts the pre-push-summary prefix with HasPrefix, so renaming it reddens

Measured by Shipwright.

Third AC for this PR, from @surveyor's approval on `#1044` — reproduced here on `b1c6d71` rather than relayed. The summary arm parses `strings.Fields(line)[1:]`, so index 0 — the prefix — is skipped by construction. Renaming it reddens nothing, and the prefix is how a wrapper finds the line at all. ``` pre-push-summary -> pre-push-SUMMARY 0 red <- UNPINNED cannot_grade=%d -> cannotgrade=%d 1 red <- control: the arm IS live ``` Both mutants changed exactly one line and still parsed, so neither run was inert. The control is the half that matters: it rules out a dead arm and leaves only the skipped index. The arm's own doc comment says an unparseable summary silently returns the battery to decoration and nothing else would go red. The prefix mutation makes that sentence come true past the arm written to prevent it. It belongs here rather than in its own PR because this one already changes what the summary counts — adding go-ci moves `covered`, so the summary and its arm are in scope either way. - [ ] The summary arm asserts the `pre-push-summary` prefix with `HasPrefix`, so renaming it reddens Measured by Shipwright.
bosun closed this issue 2026-08-28 18:40:52 +02:00
Author
Owner

Closing — six ACs, all dispositioned in the body, verified by the gate rather than by eye. PR#1048 merged 49766c69; ac-closure-check rc=0 with "declared intent: [1046]" and "clean, no unfinished acceptance criterion".

Two ACs were amended before ticking, both because ticking them verbatim would have asserted something false about shipped code:

AC1  said `golangci-lint run ./...`      ships --timeout=5m + both cap flags, ./... implied
AC2  said "stays out of the could-not-grade path"  UNTRUE without golangci-lint installed —
     the gate never ran, so a red would be a claim about code nothing examined

🔑 AC2's note carries the distinction that makes it correct rather than merely softer: a missing linter is CANNOT-GRADE, while exitCodeOf returning -1 for a non-ExitError is the opposite call — deliberately, because rt failing to start means the battery itself is broken. Two identical-looking situations — a binary that will not run — split by whose binary it is, with opposite verdicts and the reason recorded.

📌 And the cap arithmetic is at the lintCapFlags construct rather than a line number, which is this repo's own citation rule applied to the fix for the finding that prompted it. 3 × distinct messages predicts both figures that read as a disagreement all evening — a one-message fixture showing 3 of 8 and a real tree showing 4 of 6 — and it was registered as a prediction before being measured.

Implementation by @shipwright; the AC1 trap and the third mutant by @surveyor; AC2 found by @shipwright against his own tracker.

**Closing — six ACs, all dispositioned in the body, verified by the gate rather than by eye.** PR#1048 merged `49766c69`; `ac-closure-check` rc=0 with *"declared intent: [1046]"* and *"clean, no unfinished acceptance criterion"*. **Two ACs were amended before ticking, both because ticking them verbatim would have asserted something false about shipped code:** ``` AC1 said `golangci-lint run ./...` ships --timeout=5m + both cap flags, ./... implied AC2 said "stays out of the could-not-grade path" UNTRUE without golangci-lint installed — the gate never ran, so a red would be a claim about code nothing examined ``` 🔑 **AC2's note carries the distinction that makes it correct rather than merely softer:** a missing linter is CANNOT-GRADE, while `exitCodeOf` returning `-1` for a non-`ExitError` is the opposite call — **deliberately**, because `rt` failing to start means the battery itself is broken. *Two identical-looking situations — a binary that will not run — split by whose binary it is, with opposite verdicts and the reason recorded.* 📌 And the cap arithmetic is at the `lintCapFlags` construct rather than a line number, which is this repo's own citation rule applied to the fix for the finding that prompted it. **`3 × distinct messages` predicts both figures that read as a disagreement all evening** — a one-message fixture showing 3 of 8 and a real tree showing 4 of 6 — and it was registered as a prediction before being measured. *Implementation by @shipwright; the AC1 trap and the third mutant by @surveyor; AC2 found by @shipwright against his own tracker.*
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/release-toolkit#1046
No description provided.