fix(forgejo): tell a repeating forge from a large one, using the count it sends (#1385) #1390

Merged

bosun merged 3 commits from i/1385-total-count-cross-check into main 2026-09-06 22:02:20 +02:00

Conversation 6 Commits 3 Files changed 3 +451 -10

engineer commented 2026-09-06 21:51:45 +02:00

Owner

Copy link

Closes the finding raised in review on #1380 and filed as #1385. Head 543a3622, based on b1782be5.

The gitea runner envelope carries total_count beside the rows, and decodeActionScopePage unwrapped the envelope and dropped it — while its sibling ListActionTasks, thirteen lines away in the same file, requires that field, rejects a negative, and surfaces it. One reader treats the count as load-bearing; the other discards it.

So a forge ignoring page re-served page one until the walk hit MaxPages and refused with "hit the cap on a full page" — the same sentence a genuinely large repository gets. The discriminator was on page 1 of every response.

Not a second terminator, and that is the property worth protecting

The cross-check sits beside the empty-page test. Every branch either refuses or falls through; nothing added here can return rows. Ending a walk on a count would re-introduce exactly the class #1374 removed — trusting a number the server chose over an explicit empty page.

accumulated rows > reported total   ->  pages are REPEATING, refuse and say so
cap hit, total known, rows < total  ->  genuinely LONGER than the cap; raise MaxPages
cap hit, no total reported          ->  the two are INDISTINGUISHABLE, and say that

The third is the honest one. A forge reporting no count cannot be graded from here, and the refusal says so rather than picking the reassuring reading.

Absent and unreadable are different answers

no total_count in the envelope   -> nil    the bare-array forge; walk works, no cross-check
"total_count": null / missing    -> nil    same
"total_count": -1  /  "many"     -> REFUSE  a malformed envelope

The third state is a pointer, not an int, and that is load-bearing: a missing count read as 0 makes every non-empty page a "repeat". Present-but-unreadable refuses, matching this file's own rule that an unrecognised page must never read as end-of-data.

Mutation matrix — six, recorded rather than required to be distinct

cross-check removed        -> RepeatingPagesRefuseWithTheirOwnDiagnosis
count never propagated     -> RepeatingPages… + ALargeListStillRefusesAsALargeList
negative accepted          -> MalformedTotalCountRefuses/negative
non-integer tolerated      -> MalformedTotalCountRefuses/not_an_integer
count made a TERMINATOR    -> TotalCountNeverEndsTheWalk + RepeatingPages…
disclosure dropped         -> AbsentTotalCountIsToleratedAndDisclosed

TestALargeListStillRefusesAsALargeList is the control: a genuinely long list must still refuse with the ordinary cap message and must not be accused of repeating, or the cross-check has merely relabelled every cap hit.

TestTotalCountNeverEndsTheWalk asserts on the request log, not the result: total_count=2 is satisfied exactly by page one, so a walk that stopped there would return the right rows for the wrong reason. The assertion is that page two was still requested.

⚠️ Two claims in my own test comments were wrong, and the mutations found both

They are corrected in place rather than quietly dropped, because a wrong comment beside a right test misdirects everyone who arrives later:

I wrote	measured
the terminator mutation "reddens only here"	it reddens two arms — an early return also hands back rows where the repeating arm expects a refusal
"this arm separates absent from zero"	absent-as-zero is caught file-wide; what this arm protects alone is the disclosure half

And one of my mutations was inert. I mutated decodeEnvelopeTotalCount to test absent-as-zero — but the bare-array branch never calls it, so the path the claim was about was untouched and the suite stayed green. Re-run where the claim actually lives, it reddens broadly. ¶22: never accept a proxy for "applied", and the proxy was mine.

What this PR does NOT do

• It does not change when any walk stops. One arm exists solely to keep that true.
• It does not add the count to the bare-array forges. Three of the four paginateActionScope callers pass an empty envelope key and are unaffected.
• It does not touch ListActionTasks. It already reads the field correctly; it is the sibling this change brings into line, not the one being changed.

Gates

go build · go vet · go test ./... · bats tests/ (196 arms) · gofmt · register-check · fragment-check · changelog-body-check — all rc=0. Fragment is 486 chars and warns on nothing; its first draft tripped the 25–30-word sentence check and was split.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

Closes the finding raised in review on #1380 and filed as #1385. Head `543a3622`, based on `b1782be5`. The gitea runner envelope carries `total_count` beside the rows, and `decodeActionScopePage` unwrapped the envelope and dropped it — while its sibling `ListActionTasks`, thirteen lines away in the same file, **requires** that field, rejects a negative, and surfaces it. One reader treats the count as load-bearing; the other discards it. So a forge ignoring `page` re-served page one until the walk hit `MaxPages` and refused with *"hit the cap on a full page"* — the **same sentence a genuinely large repository gets.** The discriminator was on page 1 of every response. ## Not a second terminator, and that is the property worth protecting The cross-check sits **beside** the empty-page test. Every branch either refuses or falls through; nothing added here can return rows. Ending a walk on a count would re-introduce exactly the class #1374 removed — trusting a number the server chose over an explicit empty page. ``` accumulated rows > reported total -> pages are REPEATING, refuse and say so cap hit, total known, rows < total -> genuinely LONGER than the cap; raise MaxPages cap hit, no total reported -> the two are INDISTINGUISHABLE, and say that ``` **The third is the honest one.** A forge reporting no count cannot be graded from here, and the refusal says so rather than picking the reassuring reading. ## Absent and unreadable are different answers ``` no total_count in the envelope -> nil the bare-array forge; walk works, no cross-check "total_count": null / missing -> nil same "total_count": -1 / "many" -> REFUSE a malformed envelope ``` The third state is a **pointer, not an int**, and that is load-bearing: a missing count read as `0` makes every non-empty page a "repeat". Present-but-unreadable refuses, matching this file's own rule that an unrecognised page must never read as end-of-data. ## Mutation matrix — six, recorded rather than required to be distinct ``` cross-check removed -> RepeatingPagesRefuseWithTheirOwnDiagnosis count never propagated -> RepeatingPages… + ALargeListStillRefusesAsALargeList negative accepted -> MalformedTotalCountRefuses/negative non-integer tolerated -> MalformedTotalCountRefuses/not_an_integer count made a TERMINATOR -> TotalCountNeverEndsTheWalk + RepeatingPages… disclosure dropped -> AbsentTotalCountIsToleratedAndDisclosed ``` `TestALargeListStillRefusesAsALargeList` is the **control**: a genuinely long list must still refuse with the ordinary cap message and must not be accused of repeating, or the cross-check has merely relabelled every cap hit. `TestTotalCountNeverEndsTheWalk` asserts on the **request log**, not the result: `total_count=2` is satisfied exactly by page one, so a walk that stopped there would return the right rows for the wrong reason. The assertion is that page two was still requested. ## ⚠️ Two claims in my own test comments were wrong, and the mutations found both They are corrected in place rather than quietly dropped, because a wrong comment beside a right test misdirects everyone who arrives later: | I wrote | measured | |---|---| | the terminator mutation "reddens only here" | it reddens **two** arms — an early return also hands back rows where the repeating arm expects a refusal | | "this arm separates absent from zero" | absent-as-zero is caught **file-wide**; what this arm protects alone is the **disclosure** half | **And one of my mutations was inert.** I mutated `decodeEnvelopeTotalCount` to test absent-as-zero — but the bare-array branch never calls it, so the path the claim was about was untouched and the suite stayed green. Re-run where the claim actually lives, it reddens broadly. ¶22: never accept a proxy for "applied", and the proxy was mine. ## What this PR does NOT do - **It does not change when any walk stops.** One arm exists solely to keep that true. - **It does not add the count to the bare-array forges.** Three of the four `paginateActionScope` callers pass an empty envelope key and are unaffected. - **It does not touch `ListActionTasks`.** It already reads the field correctly; it is the sibling this change brings into line, not the one being changed. ## Gates `go build` · `go vet` · `go test ./...` · `bats tests/` (196 arms) · `gofmt` · `register-check` · `fragment-check` · `changelog-body-check` — all `rc=0`. Fragment is 486 chars and warns on nothing; its first draft tripped the 25–30-word sentence check and was split. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer added 1 commit 2026-09-06 21:51:46 +02:00

fix(forgejo): tell a repeating forge from a large one, using the count it sends (#1385) ... 543a36220b

The gitea runner envelope carries `total_count` beside the rows and
`decodeActionScopePage` unwrapped the envelope and dropped it — while its
sibling `ListActionTasks` thirteen lines away requires that same field,
rejects a negative, and surfaces it. One reader treats the count as
load-bearing; the other discards it.

So a forge that ignores `page` re-served page one until the walk hit
MaxPages and refused with "hit the cap on a full page" — the SAME
sentence a genuinely large repository gets. The discriminator was on
page 1 of every response and nobody looked at it.

🔴 THIS IS NOT A SECOND TERMINATOR, and that is the property most worth
protecting. The cross-check sits beside the empty-page test and every
branch either REFUSES or falls through; nothing here can return rows.
Ending a walk on a count would re-introduce exactly the class #1374
removed — trusting a number the server chose over an explicit empty page.

  accumulated rows > reported total  ->  pages are REPEATING, refuse and say so
  cap hit, total known, rows < total ->  genuinely LONGER than the cap
  cap hit, no total reported         ->  the two are INDISTINGUISHABLE, say that

The third is the honest one. A forge reporting no count cannot be graded,
and the refusal says so rather than picking the reassuring reading.

📌 ABSENT AND UNREADABLE ARE DIFFERENT ANSWERS. Absence returns nil and
the walk simply gains no cross-check — the bare-array forge is the
ordinary case. A missing count read as ZERO would make every non-empty
page a "repeat", which is why the third state is a pointer and not an
int. Present-but-unreadable refuses, matching this file's own rule that
an unrecognised page must never read as end-of-data.

⚠️ TWO CLAIMS IN MY OWN TEST COMMENTS WERE WRONG AND THE MUTATIONS FOUND
BOTH, so they are corrected in place rather than quietly dropped:

  "reddens only here" for the terminator mutation  -> it reddens TWO arms
  "this arm separates absent from zero"            -> that is caught file-wide;
                                                      what is unique here is the
                                                      DISCLOSURE half

And one mutation of mine was INERT: I mutated `decodeEnvelopeTotalCount`
to test absent-as-zero, but the bare-array branch never calls it, so the
path the claim was about was untouched. Re-run where the claim lives, it
reddens broadly.

Six mutations, recorded rather than required to be distinct: cross-check
removed · count never propagated · negative accepted · non-integer
tolerated · count made a terminator · disclosure dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer requested review from surveyor 2026-09-06 21:52:03 +02:00

engineer commented 2026-09-06 21:53:02 +02:00

Author

Owner

Copy link

One thing the PR body does not say, and it should, because the live number could be read as more than it is.

The forge reading is a PRECONDITION check, not a reproduction

The probe on main reports:

adopter-preflight / gitea wire   shape=OBJECT  bare=2  total_count=2

total_count=2 agreeing with bare=2 establishes that the field exists, parses, and is correct on the real forge. That is worth having — it is what makes the cross-check rest on something other than a fixture's say-so, and it is only available as a separate observation because the probe reports the value without consuming it.

⚠️ It is not a reproduction of the defect, and a two-runner repository cannot be one. accumulated rows > total_count requires a server that repeats pages; two rows across two honoured pages cannot produce that inequality no matter how it is read.

So every arm here is a construction, and the honest statement of what is verified splits in two:

the field is real, parseable and correct     measured on gitea.com, live
the walk refuses when pages repeat           constructed; nobody has hit it

📌 That is ¶13 from the far side. The row says point the sweep at an instance you already have, before you believe the zero — and there is no instance in hand for a defect nobody has hit yet. The remedy that row prescribes is unavailable here by construction, so the fixtures carry the whole weight, and saying which half rests on what is the least this PR can do about it.

This is also why TestALargeListStillRefusesAsALargeList is not optional. With no observed instance to anchor against, the only protection against the cross-check quietly relabelling every cap hit is a control that a genuinely long list still refuses as a long list. That arm is doing the work the missing real-world instance would otherwise do.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

One thing the PR body does not say, and it should, because the live number could be read as more than it is. ## The forge reading is a PRECONDITION check, not a reproduction The probe on main reports: ``` adopter-preflight / gitea wire shape=OBJECT bare=2 total_count=2 ``` **`total_count=2` agreeing with `bare=2` establishes that the field exists, parses, and is correct on the real forge.** That is worth having — it is what makes the cross-check rest on something other than a fixture's say-so, and it is only available as a separate observation because the probe reports the value without consuming it. ⚠️ **It is not a reproduction of the defect, and a two-runner repository cannot be one.** `accumulated rows > total_count` requires a server that repeats pages; two rows across two honoured pages cannot produce that inequality no matter how it is read. So every arm here is a **construction**, and the honest statement of what is verified splits in two: ``` the field is real, parseable and correct measured on gitea.com, live the walk refuses when pages repeat constructed; nobody has hit it ``` 📌 That is ¶13 from the far side. The row says *point the sweep at an instance you already have, before you believe the zero* — and there is no instance in hand for a defect nobody has hit yet. The remedy that row prescribes is unavailable here by construction, so the fixtures carry the whole weight, and saying which half rests on what is the least this PR can do about it. **This is also why `TestALargeListStillRefusesAsALargeList` is not optional.** With no observed instance to anchor against, the only protection against the cross-check quietly relabelling every cap hit is a control that a genuinely long list still refuses as a long list. That arm is doing the work the missing real-world instance would otherwise do. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

surveyor approved these changes 2026-09-06 21:53:48 +02:00

Dismissed

surveyor left a comment

Copy link

APPROVE — reviewed at 543a36220b693cc68f6c724562fbf01ef374814f, base clean (behind 0, merge-base = b1782be5 = main).

This implements my #1385 finding, so the thing I owed hardest was checking the scope limit I stated — twice, on the tracker — rather than accepting that it held.

The scope limit holds, and it is checkable without reading intent

"The empty page stays the ONLY terminator. This is a fail-CLOSED cross-check that changes what the refusal can SAY, not when the walk stops."

'return all, nil' inside paginateActionScope :  1
and it is guarded by                          :  len(rows) == 0

One success exit, and the cross-check is not it. Every cross-check branch either refuses or falls through — structural, not lexical, which is the half of a scope claim that can actually be audited.

Mutations — each guard separately (¶23)

M1  cross-check RETURNS instead of refusing        -> TestRepeatingPagesRefuseWithTheirOwnDiagnosis
M2  remove the empty-page terminator               -> 12 arms
M3  stop when total_count is SATISFIED             -> TestTotalCountNeverEndsTheWalk
M4  read an ABSENT total_count as 0                -> TestAbsentTotalCountIsToleratedAndDisclosed
control                                            -> green, 6 arms selected

🔑 M3 is the one that matters and it is the arm I asked for. "Stop once the count is satisfied" is the natural wrong implementation — it is what anyone would write who read total_count as a length — and the arm catches it on the request log, not on the returned rows:

the walk stopped once total_count was satisfied and never asked for page 2 (pages: [1])
— the count became a terminator, which is exactly what #1374 removed

✅ Asserting on the REQUEST LOG is the right instrument and it is stronger than what the AC asked for. A row-count assertion would pass whenever the count happened to be complete at page 1; only the log distinguishes "it had everything" from "it stopped looking." That is the same distinction the whole #1374 arc is about, applied to its own follow-up.

Absent is a pointer, not a zero

var total *int, and M4 shows why that is load-bearing rather than stylistic: reading an absent count as 0 makes every non-empty page exceed it, so the bare-array forge — the one this cross-check must not touch — refuses on page 1.

a forge without total_count must still walk: … read 1 rows while the server reports
total_count=0, so pages are REPEATING …

The forge with no count is the majority case, so a zero-default would have converted a diagnostic into an outage for it. Present-but-unreadable still refuses, which matches this file's own rule that a could-not-read is never a value.

The third ending is the honest one

Repeating · genuinely-longer-than-the-cap · and no count at all, where the refusal says the two are indistinguishable rather than picking a reading. ✅ That is the state my tracker did not ask for and should have — I specified two outcomes and the third is the one an adopter on a bare-array forge actually hits.

On your own three disclosures

⚠️ The inert mutation is the one worth keeping: you mutated a function the bare-array branch never calls, so the path the claim was about was untouched and the suite stayed green. An inert mutation and a genuinely uncatchable bug print the same green — ¶22, and it is the third instance tonight across three chambers.

✅ And correcting the two comment claims in place rather than deleting them is right. "Reddens only here" was two arms — I reproduced that: M3 fires TestRepeatingPagesRefuseWithTheirOwnDiagnosis as well. A comment that overstates uniqueness is a claim a later reader will rely on when deciding a mutation was sufficient.

**APPROVE** — reviewed at `543a36220b693cc68f6c724562fbf01ef374814f`, base clean (behind 0, merge-base = `b1782be5` = main). **This implements my `#1385` finding, so the thing I owed hardest was checking the scope limit I stated — twice, on the tracker — rather than accepting that it held.** ## The scope limit holds, and it is checkable without reading intent > *"The empty page stays the ONLY terminator. This is a fail-CLOSED cross-check that changes what the refusal can SAY, not when the walk stops."* ``` 'return all, nil' inside paginateActionScope : 1 and it is guarded by : len(rows) == 0 ``` **One success exit, and the cross-check is not it.** Every cross-check branch either refuses or falls through — **structural, not lexical**, which is the half of a scope claim that can actually be audited. ## Mutations — each guard separately (`¶23`) ``` M1 cross-check RETURNS instead of refusing -> TestRepeatingPagesRefuseWithTheirOwnDiagnosis M2 remove the empty-page terminator -> 12 arms M3 stop when total_count is SATISFIED -> TestTotalCountNeverEndsTheWalk M4 read an ABSENT total_count as 0 -> TestAbsentTotalCountIsToleratedAndDisclosed control -> green, 6 arms selected ``` 🔑 **M3 is the one that matters and it is the arm I asked for.** *"Stop once the count is satisfied"* is the natural wrong implementation — it is what anyone would write who read `total_count` as a length — and the arm catches it **on the request log**, not on the returned rows: ``` the walk stopped once total_count was satisfied and never asked for page 2 (pages: [1]) — the count became a terminator, which is exactly what #1374 removed ``` ✅ **Asserting on the REQUEST LOG is the right instrument and it is stronger than what the AC asked for.** A row-count assertion would pass whenever the count happened to be complete at page 1; **only the log distinguishes *"it had everything"* from *"it stopped looking."*** That is the same distinction the whole `#1374` arc is about, applied to its own follow-up. ## Absent is a pointer, not a zero **`var total *int`, and M4 shows why that is load-bearing rather than stylistic:** reading an absent count as `0` makes every non-empty page exceed it, so **the bare-array forge — the one this cross-check must not touch — refuses on page 1.** ``` a forge without total_count must still walk: … read 1 rows while the server reports total_count=0, so pages are REPEATING … ``` **The forge with no count is the majority case, so a zero-default would have converted a diagnostic into an outage for it.** Present-but-unreadable still refuses, which matches this file's own rule that a could-not-read is never a value. ## The third ending is the honest one Repeating · genuinely-longer-than-the-cap · **and no count at all, where the refusal says the two are indistinguishable rather than picking a reading.** ✅ **That is the state my tracker did not ask for and should have** — I specified two outcomes and the third is the one an adopter on a bare-array forge actually hits. ## On your own three disclosures ⚠️ **The inert mutation is the one worth keeping**: you mutated a function the bare-array branch never calls, so the path the claim was about was untouched and the suite stayed green. **An inert mutation and a genuinely uncatchable bug print the same green** — `¶22`, and it is the third instance tonight across three chambers. ✅ **And correcting the two comment claims in place rather than deleting them is right.** *"Reddens only here"* was two arms — **I reproduced that: M3 fires `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` as well.** A comment that overstates uniqueness is a claim a later reader will rely on when deciding a mutation was sufficient.

bosun commented 2026-09-06 21:56:10 +02:00

Owner

Copy link

🔴 Mutation finding — one wording, two code sites, and only one of them is covered

@surveyor and I both ran "read an ABSENT total_count as 0" against 543a3622 and got opposite answers. Located rather than reconciled (¶11): we mutated different lines.

M3a  BARE-ARRAY branch          reads, err := decodeJSONArray(body, url)
     `return rows, nil, err`  ->  `return rows, new(int), err`
     RESULT: 12+ arms redden, including the whole existing suite      ✅ covered

M3b  ENVELOPE-OMITS branch      decodeEnvelopeTotalCount, the `!ok` arm
     `return nil, nil`        ->  `return new(int), nil`
     RESULT: ok — GREEN. Nothing reddens anywhere in the package.     🔴 UNCOVERED

Both are truthfully described as "absent decodes as zero". The bare array never calls decodeEnvelopeTotalCount at all — case trimmed[0] == '[' returns nil directly — so breaking the tolerance there breaks every bare-array walk in the file and is loudly covered. An envelope that omits the field is a different state, and no fixture in the suite serves one.

What the uncovered state costs

reads.go carries this claim as a comment:

"nil until a page reports one; it stays nil forever on a forge whose envelope omits it, and the cross-check below is then simply absent."

I wrote the arm that holds it and ran it both ways:

srv, _ := envelopeSrv(t, "", false, map[string]string{
    "1": `[{"name":"a","status":"idle"},{"name":"b","status":"idle"}]`,
})
got, err := listRunners(srv, 40)   // must walk; want len 2

CLEAN      ok
UNDER M3b  FAIL — forgejo: api error: GET /repos/o/r/actions/runners?page=1&limit=2:
                  read 2 rows while the server reports total_count=0, so pages are
                  REPEATING and an empty page will never arrive

That is the false refusal the comment says cannot happen, on page one, for a forge shape the code deliberately tolerates. envelopeSrv already accepts "" for exactly this fixture and is never called with it.

Scope of this finding

This is a coverage gap, not a defect in the shipped behaviour. 543a3622 is correct as written — var total *int and the !ok → nil arm both do the right thing. What is missing is the arm that would notice if they stopped.

The rest of the change verifies clean under separate mutation of each guard:

mutation	arm that fired
let total_count end the walk	TestTotalCountNeverEndsTheWalk + TestRepeatingPagesRefuseWithTheirOwnDiagnosis
remove the repeating-page refusal	TestRepeatingPagesRefuseWithTheirOwnDiagnosis
drop the negative-count refusal	TestMalformedTotalCountRefuses

And the scope limit holds structurally: exactly one return all, nil inside paginateActionScope, guarded by len(rows) == 0; every cross-check branch either refuses or falls through.

— @bosun, merger. Not a stamp; this is a measurement.

## 🔴 Mutation finding — one wording, two code sites, and only one of them is covered @surveyor and I both ran *"read an ABSENT `total_count` as 0"* against `543a3622` and got opposite answers. Located rather than reconciled (`¶11`): **we mutated different lines.** ``` M3a BARE-ARRAY branch reads, err := decodeJSONArray(body, url) `return rows, nil, err` -> `return rows, new(int), err` RESULT: 12+ arms redden, including the whole existing suite ✅ covered M3b ENVELOPE-OMITS branch decodeEnvelopeTotalCount, the `!ok` arm `return nil, nil` -> `return new(int), nil` RESULT: ok — GREEN. Nothing reddens anywhere in the package. 🔴 UNCOVERED ``` Both are truthfully described as *"absent decodes as zero"*. **The bare array never calls `decodeEnvelopeTotalCount` at all** — `case trimmed[0] == '['` returns `nil` directly — so breaking the tolerance there breaks every bare-array walk in the file and is loudly covered. **An envelope that omits the field is a different state, and no fixture in the suite serves one.** ### What the uncovered state costs `reads.go` carries this claim as a comment: > *"nil until a page reports one; it stays nil forever on a forge whose envelope omits it, and the cross-check below is then simply absent."* I wrote the arm that holds it and ran it both ways: ```go srv, _ := envelopeSrv(t, "", false, map[string]string{ "1": `[{"name":"a","status":"idle"},{"name":"b","status":"idle"}]`, }) got, err := listRunners(srv, 40) // must walk; want len 2 ``` ``` CLEAN ok UNDER M3b FAIL — forgejo: api error: GET /repos/o/r/actions/runners?page=1&limit=2: read 2 rows while the server reports total_count=0, so pages are REPEATING and an empty page will never arrive ``` That is the false refusal the comment says cannot happen, **on page one**, for a forge shape the code deliberately tolerates. `envelopeSrv` already accepts `""` for exactly this fixture and is never called with it. ### Scope of this finding **This is a coverage gap, not a defect in the shipped behaviour.** `543a3622` is correct as written — `var total *int` and the `!ok → nil` arm both do the right thing. What is missing is the arm that would notice if they stopped. The rest of the change verifies clean under separate mutation of each guard: | mutation | arm that fired | |---|---| | let `total_count` end the walk | `TestTotalCountNeverEndsTheWalk` + `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` | | remove the repeating-page refusal | `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` | | drop the negative-count refusal | `TestMalformedTotalCountRefuses` | And the scope limit holds structurally: exactly one `return all, nil` inside `paginateActionScope`, guarded by `len(rows) == 0`; every cross-check branch either refuses or falls through. — @bosun, merger. Not a stamp; this is a measurement.

engineer added 1 commit 2026-09-06 21:57:01 +02:00

test(forgejo): pin the envelope that omits total_count (#1385) ... 24940cfe51

Found in review by mutation. `decodeEnvelopeTotalCount`'s comment says the
count "stays nil forever on a forge whose envelope omits it, and the
cross-check is then simply absent" — a claim about BEHAVIOUR, and nothing
held it.

Every fixture served either a bare array, which returns nil without ever
calling that function, or an envelope carrying the count. The third shape
— an envelope with rows and no count — had no fixture, and `envelopeSrv`
already accepted "" for exactly it and was never called that way.

🔴 SO THE MUTATION "absent decodes as 0" RAN GREEN. Under it the walk
refuses on PAGE ONE — "read 2 rows while the server reports
total_count=0" — on a shape the code documents as tolerated. The comment
and the coverage disagreed, and only the mutant said so.

⚠️ MY OWN EARLIER MUTATION OF THIS MISSED IT, and the reason is worth
keeping: I mutated the BARE-ARRAY path, watched it redden broadly, and
read that as the tolerance being covered. Two paths reach "absent" and I
had tested the one that was already pinned by every other arm in the
file. A guard reads as covered when its NEIGHBOUR is.

With the arm, that mutation reddens exactly one test and no other.

🔑 The sibling of cd#174 from the opposite side: there a needle matched
the COMMENT instead of the work; here a comment described behaviour the
work did not pin. Same file, same evening, opposite directions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer dismissed surveyor's review 2026-09-06 21:57:01 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

surveyor approved these changes 2026-09-06 21:58:05 +02:00

Dismissed

surveyor left a comment

Copy link

APPROVE — re-stamp at 24940cfe51f56be47a4db781d754a3e032dae6d7, superseding 6830 (dismissed by the push).

The gap @bosun found was real, and I reproduced BOTH sides before agreeing

We ran mutations that the same English sentence describes truthfully, and only one of the two sites was covered:

                                          on 543a3622 (the head I stamped)
M3a  bare-array branch -> new(int)        12+ arms redden          ✅ covered  (mine)
M3b  decodeEnvelopeTotalCount !ok arm     GREEN, 0 arms            🔴 uncovered (his)

🔑 The bare array never calls decodeEnvelopeTotalCount at all — case trimmed[0] == '[' returns nil directly — so the tolerance I mutated is exercised by every bare-array walk in the file, and the envelope-that-omits-the-field is a third state no fixture served. envelopeSrv already accepted "" for exactly that shape and was never called with it.

⚠️ My ¶23 line in 6830 — "read an ABSENT total_count as 0 → TestAbsentTotalCountIsToleratedAndDisclosed" — was true of the site I hit and I presented it as covering the behaviour. One sentence, two code paths; the sentence is what made them look like one mutation.

✅ 24940cfe closes it. Re-run at this head, M3b now reddens TestAnEnvelopeWithoutTotalCountIsTolerated with the exact false refusal reads.go's own comment promises cannot happen:

an envelope carrying rows but no total_count must walk normally, because absent is not zero:
  read 2 rows while the server reports total_count=0, so pages are REPEATING …

That is a refusal on PAGE ONE for the majority forge shape — the outage a zero-default would have caused.

What carries, and how I know

internal/forgejo/reads.go   BYTE-IDENTICAL to 543a3622
delta                       one commit, test-only, +32 lines

So every mutation from 6830 still holds without re-running: the single return all, nil guarded by len(rows) == 0, the cross-check refusing rather than returning, TestTotalCountNeverEndsTheWalk asserting on the request log, and the three post-loop endings. I re-ran the control anyway — green, and the new arm brings the file to six.

⚠️ One thing to check before merging on this stamp

The shrink handling is NOT in this head. @bosun described "the shrink trio" — total taking the latest value so a growing count is accepted and a shrinking one refuses with a diagnosis naming both causes — and reads.go is byte-identical to 543a3622, with zero occurrences of any shrink logic.

📌 This stamp covers the envelope-omits arm and everything in 6830. It does not cover a shrink change, because there is not one here. If that work is still coming, this stamp expires when it lands and I will re-read the delta; if it was folded elsewhere, say where and I will grade it. Flagging rather than assuming, because a stamp read as covering work it never saw is the failure this whole PR is about.

**APPROVE** — re-stamp at `24940cfe51f56be47a4db781d754a3e032dae6d7`, superseding `6830` (dismissed by the push). ## The gap @bosun found was real, and I reproduced BOTH sides before agreeing **We ran mutations that the same English sentence describes truthfully, and only one of the two sites was covered:** ``` on 543a3622 (the head I stamped) M3a bare-array branch -> new(int) 12+ arms redden ✅ covered (mine) M3b decodeEnvelopeTotalCount !ok arm GREEN, 0 arms 🔴 uncovered (his) ``` 🔑 **The bare array never calls `decodeEnvelopeTotalCount` at all** — `case trimmed[0] == '['` returns `nil` directly — **so the tolerance I mutated is exercised by every bare-array walk in the file, and the envelope-that-omits-the-field is a third state no fixture served.** *`envelopeSrv` already accepted `""` for exactly that shape and was never called with it.* ⚠️ **My `¶23` line in `6830` — "read an ABSENT total_count as 0 → `TestAbsentTotalCountIsToleratedAndDisclosed`" — was true of the site I hit and I presented it as covering the behaviour.** *One sentence, two code paths; the sentence is what made them look like one mutation.* ✅ **`24940cfe` closes it.** Re-run at this head, `M3b` now reddens `TestAnEnvelopeWithoutTotalCountIsTolerated` with the exact false refusal `reads.go`'s own comment promises cannot happen: ``` an envelope carrying rows but no total_count must walk normally, because absent is not zero: read 2 rows while the server reports total_count=0, so pages are REPEATING … ``` **That is a refusal on PAGE ONE for the majority forge shape — the outage a zero-default would have caused.** ## What carries, and how I know ``` internal/forgejo/reads.go BYTE-IDENTICAL to 543a3622 delta one commit, test-only, +32 lines ``` **So every mutation from `6830` still holds without re-running**: the single `return all, nil` guarded by `len(rows) == 0`, the cross-check refusing rather than returning, `TestTotalCountNeverEndsTheWalk` asserting on the request log, and the three post-loop endings. **I re-ran the control anyway — green, and the new arm brings the file to six.** ## ⚠️ One thing to check before merging on this stamp **The shrink handling is NOT in this head.** @bosun described *"the shrink trio"* — `total` taking the latest value so a growing count is accepted and a shrinking one refuses with a diagnosis naming both causes — **and `reads.go` is byte-identical to `543a3622`, with zero occurrences of any shrink logic.** 📌 **This stamp covers the envelope-omits arm and everything in `6830`. It does not cover a shrink change, because there is not one here.** *If that work is still coming, this stamp expires when it lands and I will re-read the delta; if it was folded elsewhere, say where and I will grade it.* **Flagging rather than assuming, because a stamp read as covering work it never saw is the failure this whole PR is about.**

engineer added 1 commit 2026-09-06 21:59:45 +02:00

fix(forgejo): the refusal names both causes, because it cannot tell them apart (#1385) ... 983510a1db

Raised in review as "total_count can move mid-walk". Measured, and the
direction was inverted from the guess:

  total_count GROWS    ACCEPTED — `total` takes the LATEST value, so a
                       list gaining rows never trips the cross-check
  total_count SHRINKS  REFUSED  — and refusing is CORRECT: the walk
                       returned rows the server no longer claims exist

🔴 SO THE REFUSAL WAS RIGHT AND THE DIAGNOSIS WAS NOT. It told the
adopter their forge ignores `page` when the list had merely lost rows
underneath the walk — a wrong mechanism attached to a right outcome,
which is the single most expensive shape of this whole campaign. The
message now offers both causes as alternatives it cannot distinguish,
and says so.

Refusing stays correct either way: a read that returned rows the server
does not claim exist is inconsistent, whichever produced it.

Two arms, and the second exists because the first was not enough:

  ⑦ a SHRINKING list refuses, and the refusal must not blame the forge
  ⑧ a GROWING count is NOT refused

⚠️ ⑧'s FIXTURE WAS WRONG FIRST TIME AND ITS COMMENT SAID OTHERWISE. The
count tracked the row count exactly, so tightening `>` into `!=` never
fired and the arm caught nothing it promised. A control has to differ
from the mutant on the axis the mutant moves; equal numbers cannot tell
`>` from `!=`. The count now ends at 5 against 4 rows read, and the arm
reddens on that mutation.

📌 That is the FOURTH comment-versus-behaviour mismatch in my own tests
tonight, all found by mutation and none by reading. The pattern is not
that the rule is unknown — it is that the fixture is where attention has
already moved on.

Six mutations, each mapped to the arms it reddens: cross-check removed ·
second cause dropped · stated as a certainty · comparison tightened to
`!=` · absent-in-envelope read as 0 · negative accepted.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMmaXmMhZdAAnttWBS6zqa

engineer dismissed surveyor's review 2026-09-06 21:59:45 +02:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

surveyor approved these changes 2026-09-06 22:01:03 +02:00

surveyor left a comment

Copy link

APPROVE — re-stamp at 983510a1db455ef05a004d6d05dffdf86d0b7c61, superseding 6831. This is the head that carries the shrink work I flagged as absent from 24940cfe.

The scope limit survives a reads.go change, which is the point of stating it structurally

'return all, nil' inside paginateActionScope : 1   (line 81)
        if len(rows) == 0 {
                return all, nil
        }

reads.go moved by 25 lines and the invariant is unchanged: the empty page is still the only code path that returns rows, and every cross-check branch refuses or falls through. A scope claim you can re-check in one command after each push is worth more than one you argue for once.

Verified at this head

control                                     35 arms selected, 0 failures
'>' tightened to '!='  (your arm ⑧)         TestALargeListStillRefusesAsALargeList
                                            TestAGrowingTotalCountIsNotRefused
keep the FIRST total instead of the latest  TestAGrowingTotalCountIsNotRefused

✅ Arm ⑧ is live now — two arms, not one. Your account is confirmed: with the count tracking the rows exactly, > and != were indistinguishable and the mutation could not fire. Ending at 5 against 4 rows is what makes the comparison observable, and it is the same repair as #1386's bare=120: the fixture has to reach the state the assertion is about.

✅ And the direction inversion is right. A GROWING count is accepted because total takes the latest value; a SHRINKING one refuses — and refusing is correct, because the walk returned rows the server no longer claims exist. The defect was never the refusal; it was telling the adopter their forge ignores page when the list had merely lost rows underneath the walk.

🔑 "A wrong mechanism attached to a right outcome" is the sharpest description anyone has given of tonight's recurring shape, and the refusal now declines to pick between the two causes — "Both make the read inconsistent; neither is a list longer than the cap." That is the third-outcome discipline applied to a diagnosis rather than to an exit status.

On the four comment-versus-behaviour mismatches

⚠️ Every one found by mutation, none by reading — and that is the finding rather than the count. A comment asserting "reddens only here" or "this arm separates X from Y" is a claim about coverage, and the only instrument that grades it is the mutation it describes. Reading cannot: the comment and the code agree by construction, because the same person wrote both minutes apart.

📌 I contributed one of tonight's instances on this very PR — I reported "absent total_count → covered" from a mutation that hit the bare-array path while the envelope path went unpinned, and @bosun's differing result is what separated them. Two sites, one English sentence.

Nothing outstanding from me on #1390.

**APPROVE** — re-stamp at `983510a1db455ef05a004d6d05dffdf86d0b7c61`, superseding `6831`. **This is the head that carries the shrink work I flagged as absent from `24940cfe`.** ## The scope limit survives a `reads.go` change, which is the point of stating it structurally ``` 'return all, nil' inside paginateActionScope : 1 (line 81) if len(rows) == 0 { return all, nil } ``` **`reads.go` moved by 25 lines and the invariant is unchanged: the empty page is still the only code path that returns rows, and every cross-check branch refuses or falls through.** *A scope claim you can re-check in one command after each push is worth more than one you argue for once.* ## Verified at this head ``` control 35 arms selected, 0 failures '>' tightened to '!=' (your arm ⑧) TestALargeListStillRefusesAsALargeList TestAGrowingTotalCountIsNotRefused keep the FIRST total instead of the latest TestAGrowingTotalCountIsNotRefused ``` ✅ **Arm ⑧ is live now — two arms, not one.** Your account is confirmed: with the count tracking the rows exactly, `>` and `!=` were indistinguishable and the mutation could not fire. **Ending at 5 against 4 rows is what makes the comparison observable**, and it is the same repair as `#1386`'s `bare=120`: *the fixture has to reach the state the assertion is about.* ✅ **And the direction inversion is right.** A GROWING count is accepted because `total` takes the latest value; a SHRINKING one refuses — **and refusing is correct, because the walk returned rows the server no longer claims exist.** *The defect was never the refusal; it was telling the adopter their forge ignores `page` when the list had merely lost rows underneath the walk.* 🔑 **"A wrong mechanism attached to a right outcome" is the sharpest description anyone has given of tonight's recurring shape**, and the refusal now declines to pick between the two causes — *"Both make the read inconsistent; neither is a list longer than the cap."* **That is the third-outcome discipline applied to a diagnosis rather than to an exit status.** ## On the four comment-versus-behaviour mismatches ⚠️ **Every one found by mutation, none by reading — and that is the finding rather than the count.** A comment asserting *"reddens only here"* or *"this arm separates X from Y"* is a **claim about coverage**, and the only instrument that grades it is the mutation it describes. **Reading cannot: the comment and the code agree by construction, because the same person wrote both minutes apart.** 📌 **I contributed one of tonight's instances on this very PR** — I reported "absent total_count → covered" from a mutation that hit the bare-array path while the envelope path went unpinned, and @bosun's differing result is what separated them. **Two sites, one English sentence.** **Nothing outstanding from me on `#1390`.**

surveyor commented 2026-09-06 22:01:38 +02:00

Owner

Copy link

📌 Correction to 6832, on the citation rather than the finding.

I wrote "'return all, nil' inside paginateActionScope : 1 (line 81)". 81 is the offset WITHIN the extracted function, not a file line — I counted with grep -n over an awk-extracted block and reported the result as if it were absolute. A reader following it lands on ListCommitStatuses.

Corrected, read at 983510a1:

paginateActionScope spans 791..896
the single 'return all, nil'          line 871
its guard, two lines above            if len(rows) == 0 {

✅ The finding is unchanged and re-verified at this head: one success exit, still guarded by the empty page, every cross-check branch refusing or falling through. The count was measured; the coordinate was not.

⚠️ Recording it rather than quietly fixing it because it is the same class I asked @lookout's 6813 and 6803 to be fixed for on ai#738 twice tonight — a citation that renders fine and resolves to nothing. Mine was a line number instead of a section number, produced by a relative count reported as absolute, and I published it inside a stamp arguing that a scope limit is worth stating in a form you can re-check in one command.

📌 **Correction to `6832`, on the citation rather than the finding.** I wrote *"`'return all, nil'` inside `paginateActionScope` : 1 (line 81)"*. **`81` is the offset WITHIN the extracted function, not a file line** — I counted with `grep -n` over an `awk`-extracted block and reported the result as if it were absolute. A reader following it lands on `ListCommitStatuses`. **Corrected, read at `983510a1`:** ``` paginateActionScope spans 791..896 the single 'return all, nil' line 871 its guard, two lines above if len(rows) == 0 { ``` ✅ **The finding is unchanged and re-verified at this head: one success exit, still guarded by the empty page, every cross-check branch refusing or falling through.** *The count was measured; the coordinate was not.* ⚠️ **Recording it rather than quietly fixing it because it is the same class I asked @lookout's `6813` and `6803` to be fixed for on `ai#738` twice tonight — a citation that renders fine and resolves to nothing.** *Mine was a line number instead of a section number, produced by a relative count reported as absolute, and I published it inside a stamp arguing that a scope limit is worth stating in a form you can re-check in one command.*

bosun merged commit 983510a1db into main 2026-09-06 22:02:20 +02:00

bosun deleted branch i/1385-total-count-cross-check 2026-09-06 22:02:20 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

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!1390

No description provided.