fix(web): page the whole patch catalog so >200 patches can be approved (#3157)

[ToddHebebrand]

Problem

The patches page issued exactly one GET /patches?limit=200 request. 200 is MAX_PAGE_LIMIT in apps/api/src/routes/patches/helpers.ts — the API's per-page ceiling — and PatchList filters, sorts and paginates entirely client-side over whatever that single response contained.

Everything past the first 200 rows therefore never reached the browser at all: it could not be searched, sorted, paged to, selected, or approved. A reporter with 333 Linux patches saw a single page of 200 and was left with 133 permanently unapproved.

Not Linux-specific. The same code path serves every OS; it surfaces on Linux first because apt/dnf catalogs produce far more discrete patch entries per fleet than Windows does.

Where the ceiling was not

Worth stating explicitly, because the reporter's framing ("should display several pages") pointed at the UI pager and the fix could easily have landed in the wrong layer:

• Not an API page cap. GET /patches and GET /update-rings/:id/patches both already return pagination: { page, limit, total } and honour ?page=.
• Not a bulk-approve cap. bulkApproveSchema puts no .max() on patchIds, and the handler loops the whole array.
• Not the client-side pager. It pages correctly — over a list that was already truncated before it got there.

The ceiling was purely the client's single-page fetch.

Fix

• PatchesPage walks every page until the reported total is exhausted, for both the all-rings and ring-scoped endpoints. Page 1 keeps its bare ?limit=200 URL, so the request is byte-identical for any catalog that fits in one page.
• The walk stops at 25 pages (5,000 patches) and renders a notice naming the shown and total counts. An unbounded walk would hammer the API and put an arbitrarily large array in memory — but an invisible cap is the bug being fixed here, so the new cap is never silent.
• A generation counter discards a superseded walk. Switching rings mid-walk could otherwise let the abandoned walk finish last and repaint the list with the previous ring's patches. This race pre-existed, but a walk is now up to 25 sequential requests instead of one, which widens the window enough to matter.
• "Select all N matching". The header checkbox only ever covered the visible page, so a multi-page result set needed one select-all click per page (14 of them at the default page size of 25). The bulk toolbar now offers a one-click select over the full filtered set. Added to all 7 locales.
• Stale comments in PatchList.tsx and routes/patches/list.ts that described the old fixed-200 fetch are updated — both are load-bearing warnings for whoever later pushes sorting server-side.

Verification

Suite	Result
apps/web patches components (9 files)	101 passed
apps/web i18n parity + gates	196 passed
apps/api routes/patches/ (5 files)	72 passed
astro check (1528 files)	0 errors
tsc --noEmit (api)	clean

New coverage:

• PatchesPage paging — all-rings walk, ring-scoped walk, cap + truncation notice, single request when the response carries no pagination metadata, a later-page failure surfacing an error rather than a partial list, and the superseded-walk discard.
• PatchList select-all-matching — appears only past one page, feeds the full id set to bulk approve, respects the active filter, absent when everything fits on one page.
• API bulk-approve at scale — 333 ids in one request asserting all 333 upserts actually fire, the audit entry records the full count, and per-id failures are reported rather than aborting the batch. This pins the server side so a well-meaning .max(200) on the schema can't silently re-create the ceiling after the client fix.

All seven behavioural tests were confirmed to fail against the pre-fix code (reverted the walk and the matching-id wiring, re-ran: 7 failed / 39 passed).

Follow-up (not in this PR)

The scalable end state is server-side filter/sort/select-all, which would let the page walk go away entirely. The API already accepts sortBy/sortDir; the web still doesn't send them. Note the severity-sort divergence flagged in PatchList.tsx before wiring that up — the client sorts severity semantically, the API alphabetically.

Closes #3157

🤖 Generated with Claude Code

[cloudflare-workers-and-pages]

Deploying breeze with    Cloudflare Pages

Latest commit:	04aee7a
Status:	✅  Deploy successful!
Preview URL:	https://843b1d4c.breeze-9te.pages.dev
Branch Preview URL:	https://fix-3157-patches-paginate-al.breeze-9te.pages.dev

View logs

[ToddHebebrand]

Review run: /pr-review-toolkit:review-pr — round 1 with code-reviewer, pr-test-analyzer, silent-failure-hunter, comment-analyzer (parallel); round 2 with code-reviewer scoped to the round-1 fix commit.

Findings: 12 consequential in round 1 + 3 in round 2 → all addressed in e8ee308c8 and 9ebac0e64; 0 outstanding.

The one that mattered — three agents independently flagged the same critical bug in my original fix. Both endpoints order by created_at alone with LIMIT/OFFSET. patches.created_at is defaultNow() and agent ingest runs inside db.transaction, so Postgres now() gives an entire scan report one identical timestamp — the reporter's 333 Linux patches are almost certainly a single such block. Paging over a key with mass ties has no defined order between queries, so the page walk could return some rows twice and never return others: #3157's exact symptom, hidden behind a correct-looking row count. Harmless before this PR (only page 1 was ever fetched), load-bearing the moment the client pages. Fixed by appending patches.id as a unique tiebreaker in both routes/patches/list.ts and routes/updateRings.ts, pinned by two API tests.

Also fixed from review:

• PatchList's error state only rendered when the list was empty, and a failed walk deliberately keeps the previous array — so a failed refresh showed stale rows, possibly from a different ring, with no indication at all. Added a stale-list banner with retry.
• Bulk-approve is now batched at 200 ids/request. The route upserts sequentially and writes its audit entry after the loop, so a proxy timeout on a 5,000-id request would have committed approvals with no audit record.
• A missing-pagination or empty page is only "that's everything" on page 1; from page 2 it's an anomaly and the list is reported incomplete.
• Rows are deduped by id, and a short list is never presented as complete.
• The incomplete-list notice now distinguishes its cause: cap (a reload can't help → use the API) vs shifted (the catalog changed mid-walk → reload button). The original copy told everyone to "narrow with filters", which can never surface the tail since every filter here is client-side over the already-loaded array.
• A 200 whose body can't be parsed no longer optimistically marks every id approved; per-id rejections from earlier batches survive a later batch aborting; the bare catch in the API loop now logs.
• Comment fixes: the "walks every page" claims name the 5,000 cap, and the severity sort-divergence warning notes that past the cap the client ranks a different subset.

Tests: apps/web patches + i18n parity + mutation gates — 304 passed (17 files). apps/api patches + updateRings — 89 passed (6 files). astro check 0 errors (1528 files); tsc --noEmit clean. New coverage includes an end-to-end test (333 patches walk in → select-all-matching → all 333 distinct ids reach the API in two batches) that the test analyst correctly noted was the missing regression net — the three suites each covered one leg and never met. All behavioural tests were confirmed to fail against the pre-fix code.

Deliberately not in scope (flagged by review, worth separate issues):

• handleBulkDecline still issues one POST /patches/:id/decline per id — "Select all N matching" can now put thousands of sequential browser requests behind one click. A bulk-decline endpoint mirroring bulk-approve would close the asymmetry.
• Every page of the walk recomputes count(*) plus a GROUP BY source over the whole catalog; page 1's total already sizes the walk.
• Server-side filter/sort/select-all remains the scalable end state that would remove the page walk entirely.

CI: the pull_request trigger dropped for this branch — gh pr checks showed only a Cloudflare Pages entry with zero Actions runs, which reads deceptively green. Dispatched manually: run 31127388127 against 9ebac0e64. Confirm that run is green before merging rather than trusting the checks list.

Status: review-clean, local verification green, awaiting maintainer merge.