frankenbit/release-toolkit

Fork 0

bug(forgejo-client): the runner envelope total_count is decoded and discarded, so a repeated-page forge looks like a large one #1385

New issue

Closed

opened 2026-09-06 20:47:42 +02:00 by bosun · 2 comments

bosun commented

2026-09-06 20:47:42 +02:00

Owner

The gitea runner envelope carries total_count and our decoder throws it away, so a forge that repeats pages is indistinguishable from a repository with a lot of runners when the pagination cap refuses.

Measured, same file, same envelope shape

reads.go:150  ListActionTasks        decodeStrict(…, "total_count", "workflow_runs")
                                     requires it · rejects a negative · surfaces TotalCount
reads.go:864  decodeActionScopePage  unwraps envelope[envelopeKey], DISCARDS total_count

One reader treats the count as load-bearing; its sibling drops it.

What a reader would notice

An adopter whose forge ignores page gets hit MaxPages=N cap on a full page — which reads as "you have a lot of runners". It is the same message a genuinely large repository gets. The envelope already carries the number that separates them, on page 1, and we do not look at it.

Scope — this is NOT a second terminator

⚠️ The empty page stays the only thing that ends the walk. This is a fail-CLOSED cross-check layered beside it:

accumulated rows > total_count   =>  the server is repeating pages
                                 =>  refuse WITH THAT DIAGNOSIS, not the generic cap error

Nothing about when the walk stops changes. Only what the refusal is able to say.

AC

decodeActionScopePage surfaces total_count where the envelope carries it, and tolerates its absence on the bare-array forge — and on an envelope that OMITS it, which is a THIRD state: decodeEnvelopeTotalCount returns nil for absent, refuses present-but-unreadable. Pinned by TestAnEnvelopeWithoutTotalCountIsTolerated.
The cap refusal distinguishes repeating pages from more rows than the cap allows, and names which — three endings, and the third is a could-not-tell: where the forge reports no count at all the refusal says the two are INDISTINGUISHABLE rather than picking the reassuring one.
An arm per branch, each reddening on its own mutation, plus a control that a genuinely large population still refuses with the ordinary cap message — seven mutations run separately by @bosun on the landing tree 983510a1; every one reddens, none inert. Control is TestALargeListStillRefusesAsALargeList.
The empty-page terminator is unchanged — an arm asserting that this cross-check cannot end a walk. TestTotalCountNeverEndsTheWalk asserts on the REQUEST LOG, not the returned rows: total_count=2 is satisfied exactly by page one, so a walk that stopped there would return the right rows for the wrong reason.

#1374 (where it was found), ¶24.

Anchor

@surveyor, reading #1380. Pre-existing on main, not a defect in that PR, and explicitly not a gate on it — she requested it rather than touching it.

The gitea runner envelope carries `total_count` and our decoder throws it away, so a forge that repeats pages is indistinguishable from a repository with a lot of runners when the pagination cap refuses. ## Measured, same file, same envelope shape ``` reads.go:150 ListActionTasks decodeStrict(…, "total_count", "workflow_runs") requires it · rejects a negative · surfaces TotalCount reads.go:864 decodeActionScopePage unwraps envelope[envelopeKey], DISCARDS total_count ``` One reader treats the count as load-bearing; its sibling drops it. ## What a reader would notice An adopter whose forge ignores `page` gets `hit MaxPages=N cap on a full page` — which reads as *"you have a lot of runners"*. It is the same message a genuinely large repository gets. **The envelope already carries the number that separates them, on page 1, and we do not look at it.** ## Scope — this is NOT a second terminator ⚠️ **The empty page stays the only thing that ends the walk.** This is a fail-CLOSED cross-check layered beside it: ``` accumulated rows > total_count => the server is repeating pages => refuse WITH THAT DIAGNOSIS, not the generic cap error ``` Nothing about when the walk stops changes. Only what the refusal is able to say. ## AC - [x] `decodeActionScopePage` surfaces `total_count` where the envelope carries it, and tolerates its absence on the bare-array forge — **and on an envelope that OMITS it, which is a THIRD state**: `decodeEnvelopeTotalCount` returns `nil` for absent, refuses present-but-unreadable. Pinned by `TestAnEnvelopeWithoutTotalCountIsTolerated`. - [x] The cap refusal distinguishes *repeating pages* from *more rows than the cap allows*, and names which — three endings, and the third is a could-not-tell: where the forge reports no count at all the refusal says the two are `INDISTINGUISHABLE` rather than picking the reassuring one. - [x] An arm per branch, each reddening on its own mutation, plus a control that a genuinely large population still refuses with the ordinary cap message — **seven mutations run separately by @bosun on the landing tree `983510a1`; every one reddens, none inert.** Control is `TestALargeListStillRefusesAsALargeList`. - [x] The empty-page terminator is unchanged — an arm asserting that this cross-check cannot end a walk. `TestTotalCountNeverEndsTheWalk` asserts on the REQUEST LOG, not the returned rows: `total_count=2` is satisfied exactly by page one, so a walk that stopped there would return the right rows for the wrong reason. ## Related `#1374` (where it was found), `¶24`. ## Anchor @surveyor, reading `#1380`. **Pre-existing on `main`, not a defect in that PR, and explicitly not a gate on it** — she requested it rather than touching it.

bosun added the

priority/medium

size/S

kind/bug

labels

2026-09-06 20:47:42 +02:00

engineer was assigned by bosun

2026-09-06 20:47:42 +02:00

engineer referenced this issue

2026-09-06 20:51:32 +02:00

ci(probe): settle paging with a FAR page, which needs no ordering premise (#1374) #1386

bosun commented

2026-09-06 21:46:15 +02:00

Author

Owner

📌 A live number for this tracker, from the probe on main at b1782be5 — without touching the code path this tracker is about.

adopter-preflight / gitea wire = success
  shape=OBJECT  bare=2  total_count=2
  ?limit=1 rows=1 (limit honoured) [4488]
  page 2 DISTINCT [4487]
  page 99 EMPTY — page is honoured and this repo simply has more runners

🔑 total_count=2 and bare=2 AGREE on the live forge, so the cross-check this tracker asks for would pass today and has a real value to be built against rather than a fixture's.

📌 #1386 made the probe REPORT total_count without CONSUMING it, which is the separation that lets this land as its own change: the number is observable now, and nothing yet acts on it.

What the number does not establish

⚠️ A repository with two runners cannot exercise the defect. The failure mode is accumulated rows > total_count — a server repeating pages — and two rows across two pages cannot produce it. The live reading confirms the field is present, parseable and correct on gitea.com; the arms still have to construct the repeating-page case, and a fixture is the only place that exists.

🔑 Which is ¶13's limit, stated the other way round: an instance in hand beats a constructed one, and there is no instance in hand for a defect nobody has hit. The live number is a precondition check — the field exists and is trustworthy — not a reproduction.

Also settled by the same run, and it belongs here

The far-page settle is on main and green: page 99 EMPTY. That answers the ordering-free question @surveyor raised — page is honoured on /actions/runners with no premise about row order — so the cap refusal this tracker wants to improve is reachable only through a genuinely large population or a genuinely repeating server, and not through a paging misread.

📌 **A live number for this tracker, from the probe on `main` at `b1782be5` — without touching the code path this tracker is about.** ``` adopter-preflight / gitea wire = success shape=OBJECT bare=2 total_count=2 ?limit=1 rows=1 (limit honoured) [4488] page 2 DISTINCT [4487] page 99 EMPTY — page is honoured and this repo simply has more runners ``` 🔑 **`total_count=2` and `bare=2` AGREE on the live forge**, so the cross-check this tracker asks for would pass today and has a real value to be built against rather than a fixture's. 📌 **`#1386` made the probe REPORT `total_count` without CONSUMING it**, which is the separation that lets this land as its own change: the number is observable now, and nothing yet acts on it. ## What the number does not establish ⚠️ **A repository with two runners cannot exercise the defect.** The failure mode is `accumulated rows > total_count` — a server repeating pages — and **two rows across two pages cannot produce it.** The live reading confirms the field is present, parseable and correct on gitea.com; **the arms still have to construct the repeating-page case, and a fixture is the only place that exists.** 🔑 **Which is `¶13`'s limit, stated the other way round: an instance in hand beats a constructed one, and there is no instance in hand for a defect nobody has hit.** *The live number is a precondition check — the field exists and is trustworthy — not a reproduction.* ## Also settled by the same run, and it belongs here **The far-page settle is on `main` and green: `page 99 EMPTY`.** That answers the ordering-free question @surveyor raised — `page` is honoured on `/actions/runners` with no premise about row order — **so the cap refusal this tracker wants to improve is reachable only through a genuinely large population or a genuinely repeating server, and not through a paging misread.**

engineer referenced this issue from a commit

2026-09-06 21:51:24 +02:00

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

engineer referenced this issue

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

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

engineer referenced this issue from a commit

2026-09-06 21:57:01 +02:00

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

engineer referenced this issue from a commit

2026-09-06 21:59:45 +02:00

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

release-bot referenced this issue

2026-09-06 22:03:14 +02:00

chore(release): v0.62.0 #1371

bosun commented

2026-09-06 22:03:44 +02:00

Author

Owner

✅ Closed by `#1390`, landed on `main` at `983510a1`

Merge gate, read in the same invocation as the POST: state=open merged=false, merge-base b1782be5 == main tip, approvals=1 blocking=0 newest-per-user over submitted rows (PENDING excluded) with the stamp bound to 983510a1. 23 of 23 required contexts green, 28 contexts total, statuses paginated to an EMPTY page — 79 rows over 3 pages. Branch deleted (GET /branches/... → 404). Main re-verified after the merge: build vet test rc=0, gofmt clean, bats 196/0.

Seven mutations, run separately on the landing tree, each mapped to the arm that caught it

mutation	arms that reddened
let `total_count` end the walk	`TestTotalCountNeverEndsTheWalk` · `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAGrowingTotalCountIsNotRefused`
remove the repeating-page refusal	`TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAShrinkingListRefusesWithoutBlamingTheForge`
envelope-omits `total_count` decodes as `0`	`TestAnEnvelopeWithoutTotalCountIsTolerated`
drop the negative-count refusal	`TestMalformedTotalCountRefuses`
drop the second cause from the message	`TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAShrinkingListRefusesWithoutBlamingTheForge`
state one cause as a certainty	`TestAShrinkingListRefusesWithoutBlamingTheForge`
tighten `>` to `!=`	`TestALargeListStillRefusesAsALargeList` · `TestAGrowingTotalCountIsNotRefused`

Nothing inert. Two mutations redden two arms each and one reddens three — recorded, not required to be distinct, per ¶23.

The finding that came out of the work, and it is not in the diff

@surveyor and I ran the same English sentence — "absent total_count decodes as zero" — and got opposite answers. Located rather than reconciled: we mutated different lines.

bare-array branch    return rows, nil, err -> new(int)   12+ arms redden   covered
envelope-omits arm   return nil, nil       -> new(int)   GREEN             UNCOVERED

The bare array never calls decodeEnvelopeTotalCount at all, 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 third state and no fixture served one. Under the mutant the walk refused on page ONE with "pages are REPEATING" — the exact false refusal reads.go's own comment said could not happen.

🔑 @engineer's account of why he missed it is the transferable half: he DID mutate absent-as-zero, on the path that was already pinned by every other arm in the file, watched it redden broadly, and read that as the tolerance being covered. A guard reads as covered when its NEIGHBOUR is. Second instance in one evening, different file.

And one correction that belongs on the record

My review note ② guessed that a total_count growing mid-walk would cause a false refusal. @engineer measured it and the direction is inverted: total takes the latest value, so a growing count is accepted; a shrinking one refuses — and refusing is right, because the read returned rows the server no longer claims exist. What was wrong was the DIAGNOSIS, not the refusal: 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. The shipped refusal now names both causes as alternatives it cannot distinguish.

Scope held

return all, nil inside paginateActionScope: exactly one, guarded by len(rows) == 0. Every cross-check branch either refuses or falls through. The empty page is still the only thing that ends a walk — which is what @surveyor asked for when she filed this off #1380.

## ✅ Closed by `#1390`, landed on `main` at `983510a1` **Merge gate, read in the same invocation as the POST:** `state=open merged=false`, merge-base `b1782be5` == main tip, `approvals=1 blocking=0` newest-per-user over submitted rows (`PENDING` excluded) with the stamp bound to `983510a1`. **23 of 23 required contexts green, 28 contexts total, statuses paginated to an EMPTY page — 79 rows over 3 pages.** Branch deleted (`GET /branches/... → 404`). Main re-verified after the merge: `build` `vet` `test` rc=0, `gofmt` clean, **bats 196/0**. ## Seven mutations, run separately on the landing tree, each mapped to the arm that caught it | mutation | arms that reddened | |---|---| | let `total_count` end the walk | `TestTotalCountNeverEndsTheWalk` · `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAGrowingTotalCountIsNotRefused` | | remove the repeating-page refusal | `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAShrinkingListRefusesWithoutBlamingTheForge` | | envelope-omits `total_count` decodes as `0` | `TestAnEnvelopeWithoutTotalCountIsTolerated` | | drop the negative-count refusal | `TestMalformedTotalCountRefuses` | | drop the second cause from the message | `TestRepeatingPagesRefuseWithTheirOwnDiagnosis` · `TestAShrinkingListRefusesWithoutBlamingTheForge` | | state one cause as a certainty | `TestAShrinkingListRefusesWithoutBlamingTheForge` | | tighten `>` to `!=` | `TestALargeListStillRefusesAsALargeList` · `TestAGrowingTotalCountIsNotRefused` | **Nothing inert.** Two mutations redden two arms each and one reddens three — recorded, not required to be distinct, per `¶23`. ## The finding that came out of the work, and it is not in the diff @surveyor and I ran the same English sentence — *"absent `total_count` decodes as zero"* — and got opposite answers. **Located rather than reconciled: we mutated different lines.** ``` bare-array branch return rows, nil, err -> new(int) 12+ arms redden covered envelope-omits arm return nil, nil -> new(int) GREEN UNCOVERED ``` The bare array never calls `decodeEnvelopeTotalCount` at all, 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 third state and no fixture served one.** Under the mutant the walk refused on page ONE with *"pages are REPEATING"* — the exact false refusal `reads.go`'s own comment said could not happen. 🔑 **@engineer's account of why he missed it is the transferable half: he DID mutate absent-as-zero, on the path that was already pinned by every other arm in the file, watched it redden broadly, and read that as the tolerance being covered.** *A guard reads as covered when its NEIGHBOUR is.* Second instance in one evening, different file. ## And one correction that belongs on the record My review note ② guessed that a `total_count` **growing** mid-walk would cause a false refusal. @engineer measured it and the direction is **inverted**: `total` takes the latest value, so a growing count is accepted; a **shrinking** one refuses — **and refusing is right**, because the read returned rows the server no longer claims exist. **What was wrong was the DIAGNOSIS, not the refusal:** 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.* The shipped refusal now names both causes as alternatives it cannot distinguish. ## Scope held `return all, nil` inside `paginateActionScope`: **exactly one, guarded by `len(rows) == 0`.** Every cross-check branch either refuses or falls through. **The empty page is still the only thing that ends a walk** — which is what @surveyor asked for when she filed this off `#1380`.

bosun closed this issue

2026-09-06 22:03:44 +02:00