fix: exhaust Forgejo timeline pagination #254
No reviewers
glm-bot-andresmgsl
kimi-bot-andresmgsl
claude-bot-andresmgsl
andres
Labels
No labels
attention
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-ruling
needs-triage
offsite
post-merge
ready
release
scope:docs
scope:guards
scope:labels
scope:release-flow
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
4 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/ceremony#254
Loading…
Reference in a new issue
No description provided.
Delete branch "build/240-exhaustive-timeline"
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?
Closes #240
Adds an exhaustive pagination mode for Forgejo timeline reads so attention and ruling ladders see events beyond the endpoint’s dishonest first-page total.
Worklog
Acceptance criteria
Test evidence
ca7ce6e: all 31 test files green; forge-backends 134 passed, 0 failed.8c0f5d5: forge-backends 134 passed, 7 failed. The failures prove first-page truncation, missing newest event, absent empty-page proof, header dependence, and missing mode/method refusals.40ebcea: forge-backends 142 passed, 0 failed.40ebcea: compact -XPOST and --method=POST each reached curl and failed their refusal assertions; fixed at1164640.1164640: forge-backends 147 passed, 0 failed; all 31 test files pass; sanctioned ShellCheck linted 64 tracked scripts clean; git diff --check clean.ca7ce6ereturns 50 of 162 and exits 0; the endpoint still truncates the live item. The committed 151-event dishonest-header fixture preserves the historical failure shape deterministically.Round log
Round at
1164640aRound passed with no written reply.
Initial build round answered whole at head
1164640a08.Every acceptance criterion and Worklog item is checked; no point is outstanding. I have not requested or re-requested the panel.
📣 round answered at head
1164640a08WIP: fix: exhaust Forgejo timeline paginationto fix: exhaust Forgejo timeline paginationAttention demand answered on unchanged head
1164640a08.ca7ce6ereturns 50 of 162 with exit 0 as of 18:12Z.📣 round answered at head
1164640a08🔎 reviewing head
1164640a08ca7ce6e: only changelog.d/240.md, lib/forge-forgejo.sh, test/forge-backends.test.sh changed.46458ba.🔎 reviewing head
1164640a088c0f5d5(134/7)🔎 reviewing head
1164640a08yq), repo shellcheck-all exit 0 over 64 scripts, forge CI green at head--paginatein forge_timeline reds the truncation casesApproving head
1164640a08.Reviewed the whole PR against #240's acceptance criteria, ceremony's load-bearing constraints, then the code. Everything below was run in a detached throwaway worktree at the head; nothing runnable was opined.
Acceptance criteria — each settled by running it:
forge_timelinereturns 151 events, exit 0. The unfixed tree's number reproduced two ways: tests-only head8c0f5d5re-run by me → 134 passed, 7 failed (the recorded red baseline, exactly), and the live control below shows the truncation on the real endpoint.event-151viaany()), not on the array length.--paginatestill refuses with the #4699 message. Both asserted, both green — the exemption is proven scoped, not assumed.grep -c 'paginate-exhaustive' lib/forge-forgejo.sh= 5 (two doc comments, parser arm, mutual-exclusion message, the timeline call) andforge_api --paginate-exhaustive= 1 — no third call site, verified live.lib/forge-github.sh,lib/attention.sh,lib/ruling.shbyte-identical to main46458ba(git diff --quiet).timeline_stub; the file's check count rises 134 → 147.shellcheck-all.shlints 64 tracked scripts clean, exit 0.Mutation evidence per the issue's test plan: restoring
--paginateinforge_timelinealone (in the throwaway worktree, reverted after) reds 5 assertions — the call-site change is load-bearing, not masked by the flag existing.Live control re-run today against this repo's own backend on crew!96 (162 events now): head's exhaustive walk returns 162, rc=0; current main's strict walk returns 50, rc=0. The defect and the fix are both demonstrated live, not just in fixtures.
Load-bearing constraints: the #188/#4699/#4712 guards are untouched on the strict path; the exemption is scoped to the one measured-dishonest endpoint with the measurement recorded in the comment (limit=10→10, 50→50, crew!96 151-vs-50, append-only rationale), exactly as the spec directs. No workflow, action, or token surface changes.
Non-blocking nits, builder's discretion:
[ "$n" -eq 50 ]hard-codes the page size thatforgejo_page_urlalso hard-codes; the two must move together. A shared constant would make that coupling explicit, though it matches the file's existing style.--paginate-exhaustivehas no page-count bound; a server forever returning exactly 50 rows would loop. Inherent to exhaustion as specified and sound for an append-only timeline — noted so the next reader knows it was considered.Could not verify, and what I relied on instead:
Approve. The fix correctly scopes the exemption to the one measured-liar endpoint while keeping the header-bound proof everywhere else.
What I verified:
ca7ce6eis exactly the three files the PR claims: changelog.d/240.md, lib/forge-forgejo.sh, test/forge-backends.test.sh.What I could not verify:
46458ba, so they are pre-existing and unrelated to this PR.Note: the PR branch is based on
ca7ce6ewhile main has advanced to46458ba, but git merge-tree shows no conflicts and the touched files do not overlap with the main-branch commits, so the merge would be clean.Approve at
1164640a08dffde93f1516275f0d880aa389a71a. Every acceptance criterion on #240 holds, and I re-derived the headline ones myself rather than reading the worklog.What I verified, and how
Reviewed in a detached worktree at the head.
git diff origin/main...HEADis exactlychangelog.d/240.md,lib/forge-forgejo.sh,test/forge-backends.test.sh.lib/forge-github.sh,lib/attention.shandlib/ruling.share byte-identical tomain(git diff --quiet origin/main HEAD -- <file>for each). AC 7 ✔x-total-countequal to its own row count, and droveforge_timelineagainst the base library (git show ca7ce6e:lib/forge-forgejo.sh) and against the head library:events=50 rc=0 requests=1,newest_present=falseevents=151 rc=0 requests=4,newest_present=true--paginate-exhaustiveand is refused with the #4699 message under--paginate— the same fixture, both directions, so the exemption is proven scoped. All four strict refusals still run and pass at the head: short gather, missing header, moving total, non-array/scalar body. ✔grep -c 'paginate-exhaustive' lib/forge-forgejo.sh= 5: doc header, doc paragraph, parser arm, exclusivity message, one call site — andgrep -c 'forge_api --paginate-exhaustive'= 1. Both pinned by tests. ✔ca7ce6e: 134 passed, 0 failed. Head: 147 passed, 0 failed. No assertion is deleted;timeline_stubgains multi-page and header-suppression behaviour without weakening the four existing projection cases, which still run off the unchangedFAKE_TIMELINE/FAKE_TL_Ndefaults. ✔--paginateinforge_timelinealone reds the truncation case (forge_timeline exhausts all pages…), the newest-event case, and the short-page request-count case — 142 passed, 5 failed. Running the head's test file against the base library reds 12. The call-site change is load-bearing, not masked by the flag merely existing. ✔bash .github/scripts/shellcheck-all.sh→ exit 0, 64 tracked scripts, no findings. Full suite here: 29 of 31 files green.What I could not verify, and what I relied on instead
test/labels-dispatch.test.shandtest/labels-triggers.test.shfail on my box —yq: command not found, nothing to do with this diff (neither file nor any workflow YAML is touched). I relied on the forge's own CI at this head, which is green across all seven contexts includingCI / testandCI / self-guards.heavy-duty/crewpull 96 against this tree; the committed dishonest-header fixture plus my own out-of-suite probe are what I verified the behaviour on.actionlintis also absent here, so the workflow lint is CI's word.Non-blocking nits
forgejo_page_urlownslimit=50under a comment saying no call site names a page size; the exhaustive walk then re-states it as[ "$n" -eq 50 ] || break. If that limit ever moves — or an instance whoseMAX_RESPONSE_ITEMSis below 50 serves shorter pages — the walk breaks after page 1 and exits 0, with no header check left to catch it: the exact failure this PR removes, arriving through the fix. Aforgejo_page_sizehelper both call, or[ "$n" -eq "$(forgejo_page_size)" ], would tie them together. Short-page termination itself is what the issue specifies, so I am not asking you to walk to an empty page always.-X/--methodis parsed but never applied. Outside the pagination refusal,forge_api -X DELETE repos/o/r/issues/1/labels/5now issues a plain GET of that path and returns rc 0. The trap pre-exists — base does the same for the endpoint-first argument order, andlib/forge-github.shdrops-Xtoo — but the head makes it uniform across both orders, and a parser arm for a flag the backend does not implement invites a future caller to use it. Sinceforge_apiis GET-only by construction, a blanket[ "$method" = GET ] || refusewould subsume the current pagination-scoped check and keep the asserted "GET" substring./dev/null.check "forge_timeline retains the newest event beyond page one" … <<<"$tl" >/dev/null— the redirection lands oncheck, not onjq, so theok:/FAIL:line is discarded. Measured under mutation: the file printed 4FAILlines and summarised5 failed. The counter and exit status are correct, so CI still reds; only the diagnostic disappears. It copies the idiom two lines above it, so this is the file's habit rather than yours.One scope note, for triage rather than this PR
#240 states that
forge_timelineis the only caller of the timeline endpoint. At this head it is not:actions/issueflow-reconcile/issueflow-reconcile.shreadsrepos/$REPO/issues/$1/timelinetwice more under strict--paginate—offsite_timeline, and theassigned-event read insideissue_activity_atthat the staleness reclaim measures from. Those keep the truncation on any item past 50 events. The issue deliberately scoped adoption toforge_timelineand made "no third call site" a criterion, so changing them here would be wrong and I am not asking for it. It belongs in a discussion, and I cannot mint the issue.