forge: paginate list and resolver commands #205

Merged

vnprc merged 2 commits from agent/forge-paginate into master

2026-09-11 21:29:30 +01:00

allod-agent commented

2026-09-11 21:05:38 +01:00

Member

forge list and lookup commands now page through the whole result set instead of trusting one request. The server caps a page at 50 rows whatever limit is asked for, which is why forge issue list -L 500 returned exactly 50 issues and silently dropped the rest, and why a label, milestone, or branch past the first page could not be resolved. -L/--limit on issue list and label list means what it says now, with the same defaults, and commands without a limit fetch everything. Output shape is unchanged. The proof is fixture tests with two-page servers; rollback is a revert of two commits, and nothing is written anywhere.

What changes

One helper, fetchPages in cmd/forge/paging.go, requests limit=<size>&page=<n> from page 1 with size 50, or the caller's limit when smaller, and stops on a page shorter than the size, when the limit is reached, or on an empty page. A server that answers every page with the same rows is refused with server repeated page N; refusing to loop rather than looping without bound.
Callers: pr list, pr find-by-head, branch-name targets for pr close, issue list, label list, label and milestone resolution, milestone list. Resolvers keep their exact-match and refuse-duplicates behavior and simply see every page.
A leading-zero limit such as -L 010 is accepted exactly as before and now means the decimal 10 the server always read it as; only the acceptance rule was ever octal.
Request URLs change by the added limit/page parameters; issue list's limit now follows its filter parameters. Exact-path tests were updated for that and for the resolvers moving from limit=100 to paged requests of 50.

Risk

R2. Behavior change on every list and resolver path, but read-only: the worst credible failure is a request sequence that stops early on a server whose page cap is below 50, which would reproduce today's behavior rather than worsen it. Rollback is a revert.

Validation

gofmt -l . empty; go vet and go test ./... green untagged and with site, secret, and site,secret, run by the overseer in the branch worktree after the final commit.
New fixture tests: a label and a milestone resolved from page 2; a pr find-by-head and a pr close branch target resolved from page 2; a missing label, milestone, and head still missing after both pages, with the request sequence asserted; issue list -L 3 sends one request and prints 3 rows, -L 60 prints 60 rows over two pages, the default sends one limit=30 request, -L 010 prints 10 rows; pr list over two pages prints every row and a short first page sends one request; a server that repeats a full page is refused.
Cross-family review after the first commit: both reviewers found the octal-limit regression and one found the unbounded loop; both fixed in the second commit.

Closes allod/tools#89

`forge` list and lookup commands now page through the whole result set instead of trusting one request. The server caps a page at 50 rows whatever limit is asked for, which is why `forge issue list -L 500` returned exactly 50 issues and silently dropped the rest, and why a label, milestone, or branch past the first page could not be resolved. `-L/--limit` on `issue list` and `label list` means what it says now, with the same defaults, and commands without a limit fetch everything. Output shape is unchanged. The proof is fixture tests with two-page servers; rollback is a revert of two commits, and nothing is written anywhere. ## What changes - One helper, `fetchPages` in `cmd/forge/paging.go`, requests `limit=<size>&page=<n>` from page 1 with size 50, or the caller's limit when smaller, and stops on a page shorter than the size, when the limit is reached, or on an empty page. A server that answers every page with the same rows is refused with `server repeated page N; refusing to loop` rather than looping without bound. - Callers: `pr list`, `pr find-by-head`, branch-name targets for `pr close`, `issue list`, `label list`, label and milestone resolution, `milestone list`. Resolvers keep their exact-match and refuse-duplicates behavior and simply see every page. - A leading-zero limit such as `-L 010` is accepted exactly as before and now means the decimal 10 the server always read it as; only the acceptance rule was ever octal. - Request URLs change by the added `limit`/`page` parameters; `issue list`'s limit now follows its filter parameters. Exact-path tests were updated for that and for the resolvers moving from `limit=100` to paged requests of 50. ## Risk R2. Behavior change on every list and resolver path, but read-only: the worst credible failure is a request sequence that stops early on a server whose page cap is below 50, which would reproduce today's behavior rather than worsen it. Rollback is a revert. ## Validation - `gofmt -l .` empty; `go vet` and `go test ./...` green untagged and with `site`, `secret`, and `site,secret`, run by the overseer in the branch worktree after the final commit. - New fixture tests: a label and a milestone resolved from page 2; a `pr find-by-head` and a `pr close` branch target resolved from page 2; a missing label, milestone, and head still missing after both pages, with the request sequence asserted; `issue list -L 3` sends one request and prints 3 rows, `-L 60` prints 60 rows over two pages, the default sends one `limit=30` request, `-L 010` prints 10 rows; `pr list` over two pages prints every row and a short first page sends one request; a server that repeats a full page is refused. - Cross-family review after the first commit: both reviewers found the octal-limit regression and one found the unbounded loop; both fixed in the second commit. Closes allod/tools#89

allod-agent added 2 commits

2026-09-11 21:05:38 +01:00

Paginate forge list and resolver commands so results past page one are no longer silently dropped 35aa199485

pr list, pr find-by-head, and pr close's branch-name lookup used a fixed
limit=50 single request; label and milestone name/title resolution used a
fixed limit=100 single request; issue list and label list exposed -L/--limit
but only ever made one request. The server caps a page at 50 items
regardless of the limit asked for, so any of these against a repo with more
than one page of results silently returned an incomplete set -- the
forge-groom run on 2026-09-10 hit this exactly: `issue list -s all -L 500`
returned 50 rows and dropped the rest.

Add fetchPages (cmd/forge/paging.go), one helper used by every list and
resolver call site above: it walks pages of the server's 50-item cap (or the
caller's limit, when smaller) until a short page ends the result set or the
caller's limit is satisfied, then returns the concatenated items. Callers
with no --limit fetch every page; issueList and labelList honor -L/--limit
exactly as before but fetch as many pages as it needs and truncate to it.
Resolvers (findLabelIDByName, resolveMilestoneID) and the PR head lookups
(prNumberForHead's two callers) always fetch every page, so a match or a
disambiguation past the first page still works. digitsValue is split out of
bashArithPositive (helpers.go) so limitCount can give fetchPages the -L/
--limit magnitude while parsePositiveInt keeps returning the caller's
verbatim digit string everywhere else.

Every base query path fetchPages is given now omits its own limit, since the
helper appends `limit=<size>&page=<n>` itself; that moves issue list's limit
parameter after its labels/milestones/search filters, so the "filters issues
with gh-style list flags" exact-path assertion in read_test.go changed
accordingly, and every other request-path assertion touching these six
call sites picked up `&page=1` (or, for the two resolvers and milestone
list, a smaller `limit=50` in place of the old `limit=100`, since a full
page is 50 regardless of the size the caller no longer sends).

docs/forge.md gets one sentence per affected command family noting it pages
through the whole result set and how --limit still bounds it where one
applies.

Refs allod/tools#89

Fix two review findings on forge pagination: decimal limits and a repeated-page guard 76022f3f5d

limitCount reused parsePositiveInt's bash-arithmetic value, which reads a
leading zero as octal for validation purposes only; that made "-L 010" fetch
and print 8 rows instead of 10. Split the concerns: parsePositiveInt's own
octal-on-leading-zero validation is unchanged (digitsValue/bashArithPositive
still decide what's accepted), but limitCount now decodes the accepted digit
string with strconv.ParseInt base 10, matching what the server and the old
single-request code both did with it. An overflow (more digits than fit an
int64) is treated as "everything" -- limit 0 -- rather than truncating to a
bogus count.

fetchPages had no termination guard beyond a short page, so a server that
answers every page number with the same full page would loop forever with
unbounded memory. It now compares each page's first element's identity
(pageIdentityKey: id, else number, else the element's own JSON encoding)
against the previous page's and dies naming the path and the page number on
a repeat.

Refs allod/tools#89