-health/online linter's commit-statuses read may silently truncate at Gitea's default page size, causing false "context missing" verdicts
#19
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: oleks/pipetree#19
Reference in New Issue
Block a user
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?
Why: Gitea's
GET /repos/{owner}/{repo}/statuses/{sha}paginates at 30 by default and returns one entry per status write, not per context. On a busy commit this means the default page can silently drop a context that WAS posted, and any check comparing "required contexts" against "contexts seen" will false-positive that the context is missing.Verified reproduction (live against git.oleks.space, commit
e25e180e5dacaded41cbf4eae7369fd2e17e93d2inoleks/builder-arbitrage, tag v0.1.69):GET /repos/oleks/builder-arbitrage/statuses/<sha>(default) -> 30 entries, 7 distinct contextsGET /repos/oleks/builder-arbitrage/statuses/<sha>?limit=50-> 48 entries, 8 distinct contextsEvery context on that commit had 6 status writes (pipeline retries): 8 contexts x 6 = 48, so the default page (30) truncates before exhausting them. The context that disappears on the default page is
ci/woodpecker/push/version-guard.Why it matters for pipetree specifically: the class-2 rules (WP010 / WP011 / WP014) decide "this required context was never posted on this commit" by comparing required contexts against posted ones read via the online linter. If that read does not exhaust pages, a context that WAS posted reads as absent, and the linter reports an unsatisfiable required context on a correctly-configured repo -- hard-failing a push (exit 1) on a config that is fine. The failure is silent and load-dependent: it appears only once a repo accumulates enough status writes to overflow page 1 (busiest repos first), and intermittently -- the worst debugging profile. This is also the exact defect class the companion cicd-insights skill teaches people to avoid: concluding a context is missing and "fixing" a config that was correct.
Not confirmed against the implementation -- I have not read
lint_online.go(or wherever the online reader lives) and am not asserting it is currently broken. The ask is to verify whether it exhausts pages (or already uses the combined endpoint), and add a regression test if it does not.Suggested robust approach: prefer
GET /repos/{owner}/{repo}/commits/{sha}/status, the combined endpoint, which reports latest-per-context and is unaffected by retry volume -- on the same commit it correctly returns all 8 contexts. If the per-write list is genuinely needed, page to exhaustion rather than trusting one response.Evidence this bites real readers, not just theoretically: two independent sessions (the one that found this, and a separate pipetree implementation session) each concluded from a default-page read that
ci/woodpecker/push/version-guarddid not exist on that commit, before pagination was suspected. Documented in cicd-insights v1.4.5 (commit 3074ac8) at diagnostic step 2 and inreferences/context-naming.md.Acceptance:
Correction from the reporter: the mechanism described above is wrong, and no shipped rule was ever affected.
I filed this on the inference that the class-2 rules decide "was this context posted?" by reading posted statuses. They do not. WP010/WP011/WP014 compute producible contexts from the config (triggers + the verified slug table) and compare byte-for-byte against branch protection's
status_check_contexts. Posted statuses are never consulted, so the false-positive path I described is not reachable in any shipped rule, and no release is affected. That was checked by grep against the implementation, not reasoned about.I should have verified the call graph before asserting the consequence. The reproduction in the issue body is sound — the pagination behaviour is real and measured — but the impact claim attached to it was not.
There was a real defect one step over, and it is fixed.
CommitStatusesdid read the paginated/statuses/{sha}endpoint with no limit, and a code comment named that function as the way to extend the verified slug table. So the next person adding an event→slug row would have hit exactly the false negative described here — "no such context was ever posted", from page one — and a wrong entry in that table is what would go on to make a rule condemn a correct config. Fixed ine02e65bby switching to the combined/commits/{sha}/statusendpoint (latest-per-context, unaffected by retry volume).The same fix also corrected
BranchProtections, which paginated in the opposite direction: a protection rule falling off page one makes its required contexts invisible, so a gated branch reads as clean and is reported as examined when it was not.Suggest closing as fixed by
e02e65b— leaving that to whoever owns the change, since they have the implementation context. Keeping the measurements here as the durable record:/statuses/{sha}default page → 30 entries, 7 distinct contexts?limit=50→ 48 entries, 8 distinct contexts/commits/{sha}/status→ 8 entries, 8 distinct, completeCommit
e25e180e5dacaded41cbf4eae7369fd2e17e93d2,oleks/builder-arbitragev0.1.69. Three separate sessions produced a confident wrong answer from a default-page read of that commit before pagination was suspected;30being exactly the default is the only tell that distinguishes it from a complete response.Documented for humans in cicd-insights v1.4.5 (statuses) and v1.4.6 (branch-protection listing).
Verified fixed in
e02e65bby reading the implementation, not the commit message.internal/gitea/client.go:109—CommitStatusesnow calls/api/v1/repos/{ownerRepo}/commits/{sha}/status, the combined latest-per-context endpoint, instead of the paginated per-write list.internal/gitea/client.go:62—BranchProtectionsnow pages to exhaustion (page/limitloop, terminating on a short page) rather than reading page one.Both directions of the class are closed, and the reasoning is recorded in the code comment so the endpoint choice does not read as arbitrary to the next person.
Restating the correction for anyone arriving from a search: the impact claim in the original report was wrong. WP010/WP011/WP014 compute producible contexts from config + slug table and compare against branch protection; they never read posted statuses. No shipped rule could have false-positived, and no release was affected. The real exposure was that
CommitStatusesis the documented path for extending the verified event→slug table, so a truncated read there would have written a wrong slug into the table — and a wrong slug is what makes a rule condemn a correct config.Closing as fixed.