Skip to content

feat!: bound pagination limit and page through list endpoints - #27807

Closed
jscottmiller wants to merge 5 commits into
scott/plat-386-workspace-paginationfrom
scott/plat-386-bound-pagination-limit
Closed

feat!: bound pagination limit and page through list endpoints#27807
jscottmiller wants to merge 5 commits into
scott/plat-386-workspace-paginationfrom
scott/plat-386-bound-pagination-limit

Conversation

@jscottmiller

@jscottmiller jscottmiller commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Stacked on #28028. Marked experimental: the open items below are not done.

ParsePagination left an omitted limit at 0, which every downstream query
treats as "no limit", so any client could ask a list endpoint for the entire
table in one request. It now resolves an omitted limit to
codersdk.MaxPaginationLimit (100) and rejects a limit outside 1..100 with a
validation error rather than clamping it.

  • MaxPaginationLimit lives in codersdk, next to the Pagination type that
    carries it, so clients can size their own page requests.
  • CLI commands that relied on an omitted limit returning every row now page to
    exhaustion: coder list, coder users list, template versions, template
    pull, and SSH completion.
  • Frontend callers that sent limit: 0 either request a single row when they
    only read the count, or page to exhaustion (org members).
  • Dead per-endpoint limit checks removed from enterprise/coderd/aibridge.go.

Breaking change

?limit=0 and ?limit=<n> for n > 100 now return 400 on every endpoint that
uses ParsePagination. Clients that relied on limit=0 meaning "all rows" must
page. The Go SDK never put limit=0 on the wire, so Go integrators see
truncation rather than an error.

Testing

  • go test ./coderd/ -run TestPagination
  • go test ./support/ -run TestRun
  • go test ./codersdk/ -run TestAllWorkspaces
  • pnpm vitest run src/api/api.test.ts

Outstanding review items

Blocking, each corroborated by more than one reviewer:

  • enterprise/cli/groupedit.go:64 calls client.Users unpaginated, so
    --add-users/--rm-users silently resolves against the first 100 users.
  • /templates/{template}/acl/available truncates at 100. GetGroups has no
    OFFSET and no ORDER BY, and codersdk.ACLAvailable has no count, so
    the truncation is neither pageable nor detectable.
  • The aibridge maximum dropped from 1000 to 100 and the error shape changed
    from Detail to Validations. codersdk/aibridge.go still documents
    1000. Decide whether to keep 100 and declare it, or give those endpoints
    their own cap.

The stated invariant does not hold yet:

  • ?limit=0 is still unbounded on coderd/provisionerjobs.go and
    coderd/provisionerdaemons.go.
  • coderd/exp_chats.go (chatCostUsers) accepts up to MaxInt32;
    enterprise/coderd/agentfirewall.go has a default but no maximum.
  • offset is uncapped and the sort runs before OFFSET, so this bounds
    response size, not database work. It should not be described as a DoS
    mitigation.

Contract and documentation:

  • No docs/reference/api/ page states the bound, and the documented
    ?limit=0 curl examples in audit.md and enterprise.md now return 400.
  • enterprise/coderd/templates.go, coderd/exp_chats.go (listChats), and
    the aibridge models/clients endpoints reject out-of-range limits without
    documenting a limit or offset parameter.
  • MaxPaginationLimit's comment claims it is the server-wide maximum;
    exp_chats enforces 1-200 and 0-2000 elsewhere. Scope the wording to
    endpoints that use ParsePagination.

Tests and lower priority:

  • coderd/pagination_test.go asserts only apiError.Message, which is the
    same constant for every failure. Assert Validations[0].Field, the detail
    text, and that exactly one validation error is returned.
  • No endpoint-level test that a list route with more than 100 rows returns
    at most 100.
  • ?limit= (present but empty) resolves to 100 instead of erroring, and
    ?limit=&limit=50 silently discards the 50.
  • cli/userlist.go and cli/templateversions.go page by offset where those
    endpoints support AfterID.
  • coder templates pull walks every version page to produce one warning.

This pull request was created by Coder Agents on behalf of @jscottmiller.

@jscottmiller jscottmiller added the experimental Changes that might not necessarily be merged, until its approved to proceed with. label Aug 3, 2026
@linear-code

linear-code Bot commented Aug 3, 2026

Copy link
Copy Markdown

PLAT-386

@jscottmiller
jscottmiller force-pushed the scott/plat-386-bound-pagination-limit branch 2 times, most recently from 75c8ea4 to 6b1951c Compare August 4, 2026 17:44
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

Docs preview

Check off each page once it's been reviewed. If a page changes in a later push, its checkbox clears automatically so it gets a fresh look. Pages not yet wired into the docs navigation aren't listed here.

@jscottmiller jscottmiller removed the experimental Changes that might not necessarily be merged, until its approved to proceed with. label Aug 10, 2026
@jscottmiller jscottmiller changed the title feat(coderd): bound pagination limit in ParsePagination feat!: bound pagination limit and page through list endpoints Aug 10, 2026
@jscottmiller jscottmiller added the experimental Changes that might not necessarily be merged, until its approved to proceed with. label Aug 10, 2026
Resolve a limit of 0 or an omitted limit to MaxPaginationLimit (1000)
and reject a larger explicit limit, so paginated endpoints can no longer
issue an unbounded query.

Refs PLAT-386.
ParsePagination now defaults an omitted limit to MaxPaginationLimit and
rejects anything outside 1..MaxPaginationLimit, so several doc strings no
longer describe the behavior:

- codersdk.Pagination.Limit no longer means "no limit" at <= 0.
- paginated-members no longer returns all members at limit=0.
- limit is no longer required on /audit and /connectionlog.

The ai-gateway session, model, and client list handlers each defaulted a
zero limit and rejected limits above 1000. ParsePagination guarantees both
conditions, so the blocks and their constants are removed. The effective
ceiling on those routes drops from 1000 to MaxPaginationLimit.

Refs PLAT-386.
The bound describes a value clients need in order to size their own page
requests, so it lives next to the Pagination type that carries it rather than
in coderd.
The server now caps a page at MaxPaginationLimit, so commands that relied on an
omitted limit returning every row request successive pages instead.
An explicit limit of 0 is now rejected, so the callers that used it either
request a single row when they only read the count, or page to exhaustion.
Workspace and support page sizes derive from MaxPaginationLimit.
@jscottmiller
jscottmiller force-pushed the scott/plat-386-bound-pagination-limit branch from 0bace98 to 546969f Compare August 11, 2026 15:24
@jscottmiller
jscottmiller changed the base branch from main to scott/plat-386-workspace-pagination August 11, 2026 15:25
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

experimental Changes that might not necessarily be merged, until its approved to proceed with.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant