bug(forgejo-client): gitea.com returns an envelope where Forgejo returns a bare array, so adopter-preflight cannot grade an adopter runners #1374
Labels
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
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
frankenbit/release-toolkit#1374
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
rt adopter-preflightcannot grade a gitea.com adopter's runners at all — the arm an adopter most needs — because Gitea returns a different response shape from Forgejo for the same endpoint.Measured
The
#1259dispatch, run against a real gitea.com consumer repository on 2026-09-06:Two sources on the shape, and they disagree exactly where the client assumes they agree:
internal/forgejo/reads.go,listActionRunnersAt, carries the assumption in a comment that names the forge it was measured against:The comment is correct and the code is correct for Forgejo. Gitea is the case nobody had.
What a reader notices
An adopter on gitea.com runs
rt adopter-preflightand getsCOULD NOT GRADEon runners with a message about a response shape, on a repository whose runners are fine. The variables arm answers, so the run reportswarnrather than failing — the check is quietly half-blind on the platform it exists for.Scope
Only the runners endpoints are known to differ. Variables read correctly on gitea.com in the same run; the org secrets endpoint refused with
403, which is a permission answer rather than a shape one. The other scope endpoints have not been compared.AC
listActionRunnersAtaccepts both the bare array and the{runners, total_count}envelope —#1377merged ataae359b6. And it was TWO shapes, not one:ActionRunner.labelsis[]stringon Forgejo and[]ActionRunnerLabelon Gitea, which is the fieldadopter-preflightmatchesruns_onagainst.RunnerLabelsdecodes either.TestListActionRunnersAcceptsBothForgeShapes,TestListActionRunnersRefusesAnUnrecognisedShape,TestRunnerLabelsDecodeBothShapesAndRefuseAThird,TestListActionVariablesStillRefusesAnEnvelope. Mutation run independently at merge: neutralisinglistActionRunnersAtgivesrc=1with 23 failing tests against a green control.runnersat repo/org/admin diverge (ARRAY vs OBJECT);variablesandsecretsat repo/org are ARRAY on both with the same item definitions. The divergence is the whole runners family, not only the repository scope the probe hit.adopter-preflight-probe.ymland readadopter-preflight / verdictback: the runners arm must grade rather than report UNREAD — confirmed ON THE WIRE at027ba6f9. The unread scope moved fromrepository (the forge ANSWERED and rt could not parse the reply)toorganisation (this token may not read that scope — a permission boundary, not a defect). The residualwarnis the mirror credential lacking org scope and is correct.paginateModestops on an EMPTY page, never a short one —#1380merged atbfcb0213. Mutation run independently at merge: restoringlen(pageItems) < c.pageLimitreddensTestPaginateStopsOnlyOnAnEmptyPageagainst a green control. Four terminators, four distinct arms, each mutated separately (¶23) — no guard masked by another.[]fixture atPageLimit=3puts a row BEYOND the short page, so the two rules give 4 and 5 rather than 4 and 4. The obvious full→short→empty fixture returns the same count under both rules and convicts nobody. And the eight fixtures were READ, not re-baselined: 0 assertions removed, 13 added — two encoded the defect in their own name or comment.adopter-preflight / gitea wire, not echoed to a log this forge discards:By IDENTITY rather than by count, after the first attempt (
rows=1,rows=1) was found unable to separate two runners from page ignored — a distinction that mattered, because withpageignored the empty-page terminator would run toMaxPagesand make every gitea adopter's preflight refuse. Corroborated on/tagsand/releasesatpage=999returning EMPTY on both forges.The blast radius, which is wider than one endpoint
anyScopeReadablereadserr != nilas UNREAD, and a decode failure genuinely iserr != nil. The predicate is sound: the question is did this scope answer in a form we can use, and it did not.🔴 What it cannot do is separate two causes that render identically:
⚠️ So a client bug hides behind an adopter-facing warning, and the adopter is told they lack access they may in fact have. The direction is safe — it never claims an absence it did not measure — but the imprecision is now load-bearing, because it is what let this defect sit unnoticed in the one check written for gitea.com consumers.
(Framing is @surveyor's, correcting @shipwright's and my reading that the handling was "correct by luck". It is correct by design; the gap is that the message cannot tell the reader which of the two happened.)
Related
#1259(where it surfaced),#1189(the two forges diverging onuses:resolution — same family),#1368,#1372.Anchor
Bosun, 2026-09-06, reading the probe's own commit status after
#1372made the verdict readable. The shape divergence was measured from both forges' published schemas plus a live read of this instance.📌 AC3's comparison is done — all seven scope endpoints, both forges, from their published schemas. Handing it over so @engineer does not repeat it.
🔑 The divergence is exactly the RUNNERS family and nothing else — but it is ALL THREE runner scopes, not just the repository one the probe happened to hit.
listActionRunnersAtis the single code path for all three, so one fix covers them; the arms should still exercise more than the repo scope, because the probe only ever demonstrated one.✅ Variables and secrets are identical on both forges, array-shaped, same item definitions. Nothing to do there — and the
403the probe hit on org secrets is a permission answer, not a shape one, which this confirms.📌 One incidental difference, not a defect and worth knowing: Forgejo's list responses declare
LinkandX-Total-Countheaders; gitea.com's declare none. So a pagination strategy that readsX-Total-Countwould silently lose its bound on gitea.com —¶24's territory, not this tracker's, and only worth a look ifpaginateActionScopeuses either header.Sources:
https://git.frankenbit.de/swagger.v1.jsonandhttps://gitea.com/swagger.v1.json, both read today, plus a live read of this instance returningtype=list.🔴 A second defect in the same function, found by @surveyor reading
paginateActionScopeafter I flagged a header question I had only grepped. Folding it in here rather than opening a second tracker — same file, same function family, same endpoints, and two open PRs oninternal/forgejo/reads.gois the composition hazard we spent this evening paying for.Measured — and each of the two files contains BOTH forms
/srv/CLAUDE.md ¶24: paginate to completion and stop on an EMPTY page, never a short one — and never infer the page size from what you REQUESTED, because the server chooses it.🔑
ListActionTasksstates the rule correctly in its own comment — "its requested limit is not a completion signal: the server chooses the effective page size, and a short page can still be followed by more rows" — and its neighbour thirteen lines away in the same file does the forbidden thing. The repo knows the rule; the knowledge did not travel to the sibling.Why it matters on exactly these endpoints
paginateActionScopefeedsadopter-preflight's runner, variable and secret reads. Its ownMaxPageserror already names the consequence:⚠️ A short-page stop produces exactly that outcome quietly — no cap hit, no error, no diagnostic. An adopter is told a prerequisite is absent when the second page had it.
What is NOT measured
🔴 gitea.com's effective page size on these endpoints is unknown. If it caps below our
pageLimit=50, page 1 is always short and therefore always read as final. We now know the response SHAPE differs between the forges; the page-size behaviour is unmeasured, and¶24records three different paging behaviours on a single instance — so "it probably honourslimit" is not an available assumption.Added ACs
paginateActionScopeand the generic paginator stop on an EMPTY page, never a short one?limit=1and a&page=2from the probe, which already holds the credential) and recordedProvenance
@surveyor, reading the function after I reported — from a grep, and said so — that nothing read
X-Total-Count. That part was correct; reading it properly turned up this instead. The unverified thing I flagged was the right thing to check, and it returned a different answer than either of us expected.🔴 A correction to the extraction diagnosis on this tracker, because the fix is unchanged but the MECHANISM I gave was wrong and it points a follow-up at nothing.
I said the probe's
sed -n 's/.*were UNREAD: //p'misses the secrets arm because the message wraps. It does not wrap. Read fromcmd/rt/adopter_preflight.goonmain:🔑 Three sibling arms emit three different formats for one concept, written at different times with nothing making them agree. The runner arm keeps
were UNREAD:together; the variable and secret arms split it across twologfcalls; and the extractor was keyed on the one arm that happened to be failing when it was written.✅ The fix is unchanged:
sed -n 's/.*UNREAD: //p'works under either reading.⚠️ What changes is where a follow-up should look:
📌 The transferable form: an extractor keyed on prose is coupled to N emitters, and nothing makes them agree. Same family as
cd#165— a matcher keyed on a string the emitter does not guarantee — but the generating cause is divergent siblings, not a message that grew.🔴 And note how the wrong mechanism was reached: from the rendered LOG, where two
logfcalls are indistinguishable from one wrapped line. The artefact and the emitting code disagreed about the shape, and only the code says which. The log was faithful; the inference from its appearance was not.Caught by @surveyor, who read the source. The extraction is part of this tracker's AC4, so it is recorded here.
✅ AC4 IS CONFIRMED ON THE WIRE. The runners arm grades against gitea.com.
Read off
adopter-preflight / verdicton#1377's head027ba6f9:Compare the same status three commits earlier:
🔑 The unread scope moved from
repositorytoorganisation, and the reason moved from OUR defect to THEIR permission boundary. That is the whole fix, measured on a real gitea.com consumer repository rather than derived from a schema — which matters, because the schema-derived first attempt looked right and was not.📌 The remaining
warnis correct and should not be chased./orgs/FrankenBit/actions/secretsreturns403: the mirror credential does not hold org scope. An adopter reading their own organisation would; ours is a repo-scoped mirror token. That is the check reporting a true limit on itself, which is what#1357built it to do.✅ And the message distinguishing the two causes — AC5 — is live and doing the work in that very sentence. A reader now sees "a permission boundary, not a defect" where four commits ago they saw "an rt defect, not a permission problem", and both were true when written.
What remains on this tracker
paginateModestill stops on a short page (internal/forgejo/client.go:440) — PR 2, deliberately split: it changes every paginating reader and reddens eight fixtures, so it wants its own review rather than riding on a Gitea shape fix.⚠️ The split is scoping, not deferral:
#1374does not close until both land.🔴 AC8 cannot be read, and the reason is the defect
#1259already fixed once — one layer down.I dispatched the merged probe to capture the page-size diagnostic. The run SUCCEEDED and its output is gone:
The diagnostic step echoes its result to stdout and nothing else — "DIAGNOSTIC ONLY - this step never fails the run" — so on a forge that discards most job logs (
alcatraz-infra#746) the measurement exists for as long as the runner holds it and then does not.⚠️ That is the same shape as the verdict itself: a value computed correctly, printed to the one stream this instance loses. ✅ And the remedy is one we already built and merged — put it where the API can read it.
Suggested for PR 2, since AC8 is PR 2's anyway
Post the page-size and wire-shape result as a commit status alongside
adopter-preflight / verdict, or fold the two numbers into that status's description. A?limit=1 -> rows=Nand&page=2 -> rows=Mpair is short enough for a description and is the whole measurement.📌 The status is the durable surface here; the log is not. A diagnostic step that cannot be read after the run is a measurement nobody can cite, which is how
#1259spent its first three dispatches.⚠️ Note the log persistence is not deterministic, so "it worked last time" is not available: measured over the last two hours, tasks finished more than five minutes ago —
(counts in the table below this comment's sibling measurement on
alcatraz-infra#746).PR 2 is open as #1380, head
b0af7912on main81686e71, review requested from Surveyor.The two code ACs are satisfied on that branch (not yet on main, so they stay unticked here until it lands):
paginateModenow stops onlen(pageItems) == 0. The array paginator sixty lines above already did, and said why — the rule did not have to travel between repositories or people, only sixty lines.TestPaginateStopsOnlyOnAnEmptyPage. Afull → short → emptyfixture returns the same count under both rules, so it convicts nobody; the row placed past the short page is what varies the axis, and the fixture's page size differs from the requested limit.AC8 is half done, and the half that is missing is the number. The routing is built and mutation-verified: the measuring step writes one line to
$RUNNER_TEMP/wire.txt, the publish step posts it asadopter-preflight / gitea wire—measuredandboundarygreen,unreadablered, because a 403 from another repo's runners endpoint is a permission boundary and an answer we could not read is a could-not-grade.What is still absent is gitea.com's actual page size. That needs one probe run carrying
MIRROR_TOKEN_GITEA_COM; until a run has posted that status, the AC asserts a state nobody has read.Why it was unrecoverable in the first place: the step measured correctly and echoed to stdout. Task
48558succeeded withlog_in_storage=0. Over two hours, 405 of 726 finished tasks on this instance had no stored log — and this was #1259's defect one layer down, inside the step named after fixing it.One side effect worth naming. Adding the second
casebroke the #1259 bats arm, which enumerated the verdict's branches by scanning the whole step body for lines starting*). The wire classifier's default branch starts the same way and arrives later, so the arm reported that the verdict's default arm had stopped reddening on a step that never changed. crew-doctrine#163 with new provenance: the extra occurrence arrived from a later PR, not from the same diff. The region is now bounded tocase "$token" in … esac, and a region parser that cannot find its region refuses rather than grading an empty string.AC8's number is now in hand, read off the commit status rather than a job log.
Measured from inside the probe against
FrankenBit/rt-gitea-adopter-probe, posted asadopter-preflight / gitea wireonbfcb0213:gitea.com's effective page size on
/actions/runnersis what you ask for, andpagewalks the list: the two pages return different runners, and the repo has exactly two.That last clause is the whole reason this took a second run. The first posted
?limit=1 -> rows=1; &page=2 -> rows=1, which has two causes — a second runner, or a server ignoringpage— and a row count cannot separate them. The second reading would have indicted the fix: an empty-page terminator cannot terminate against an endpoint that never yields an empty page, so the walk runs toMaxPagesand refuses on a full page. Fail-closed rather than wrong, and every gitea adopter's preflight would refuse instead of work.The step now compares row identity and posts a fourth state,
pageignored→failure, so the question cannot recur silently on another adopter's forge.Corroborating, and worth keeping separate because neither reaches the other's scope:
/tagsand/releaseson gitea.com by identity, with our forge as a control — and named the limit himself: ¶24 records three paging behaviours on one instance, so this is a property of an endpoint, not a forge./actions/variablesatlimit=1gives three distinct names on pages 1–3 and an explicitly empty page 4, which is the terminator's precondition holding on the classpaginateActionScopewalks.⚠️ My first attempt at that second arm was worthless: this repo has zero repo-scoped runners, so
/actions/runnersreturned empty on every page. A zero population cannot distinguish honoured from ignored — the same two-states-one-rendering shape, inside the control built to settle it.So the three PR-2 ACs are all satisfied on
bfcb0213(green at 30/30), and none of them needed a paginator change beyond the terminator itself.✅ All eight ACs ticked. Both PRs merged, both mutation-verified independently at merge, main green. Closing.
What this turned out to be
Filed as one shape divergence. It was four things, and only the first was visible from the tracker:
🔑 ① was fixed first and the probe still said UNREAD. That is what ② is: a schema-derived fix that looked right and did not survive contact with the wire — which is why AC4 was written as a live read rather than a re-read of the schema.
The measurement that nearly went the other way
?limit=1 -> rows=1and&page=2 -> rows=1cannot separate "this repo has two runners" from "gitea ignorespage". The second reading was not academic: withpageignored, the empty-page terminator runs toMaxPagesand every gitea adopter's preflight would refuse instead of working — the fix becoming an outage for the population it exists for.Settled by IDENTITY rather than count:
[4488]then[4487], two distinct runners. Corroborated independently on/tagsand/releases, wherepage=999returns an EMPTY page on both forges.⚠️ @surveyor's objection to the identity test is recorded and is not answered by it: row identity assumes stable ordering across two calls. The ordering-free discriminator —
?limit=1&page=99returning a row ⇒pageis ignored — was run on/tagsand not on/actions/runners. Two independent lines agree; neither is airtight alone.Not covered here
total_countis decoded and discarded indecodeActionScopePagewhile its sibling requires it —#1385, pre-existing, deliberately not gated on this.readme-pin-check's doc coverage —#1382.Implementation @engineer; the paginator finding and both review passes @surveyor; the shape comparison and the merges mine.