This repository was archived by the owner on Sep 10, 2026. It is now read-only.
Repository navigation
Parse query parameters instead of coercing them; opaque pagination cursors - #60
Merged
Merged
Conversation
…rsors
Every paged list could silently return an empty page. The origin is that
`Number(null)` and `Number("")` are both `0`, so `z.coerce.number()` is not a
parser. Four layers then behaved correctly and made it invisible:
1. `z.coerce.number().int().min(0)` genuinely accepts null and yields 0, so
the emitted schema honestly published `["integer", "null"]`. With
`.min(1)` it does not, which is why only some parameters were affected.
2. openapi-generator read that nullability and correctly dropped its
`if (x != null)` guard: the API had declared null welcome.
3. dio rendered the null as a bare `?cursor` / `?to=`.
4. The server coerced `""` straight back to `0`.
A cursor of 0 means "older than anything that has ever happened", so the lobby,
both games/mine buckets, the friends list and every history screen answered 200
with an empty array. A `to` of 0 clamped replays to a single frame. Nothing was
logged, because nothing had gone wrong as far as any layer could tell.
Integer query parameters now parse from a strict pattern. They reject null and
empty alike, so the generated client omits an absent parameter on its own and a
malformed one is a loud 400 rather than a plausible, wrong 200. No library was
misbehaving and nothing downstream compensates for this.
Cursors are now opaque tokens carrying (sortValue, id), and paged responses
return `nextCursor`, null exactly when the list is exhausted. That makes the
bug class unrepresentable rather than merely caught, fixes the timestamp-tie
row loss that d1/reads.ts documented but did not fix, and stops clients having
to restate the server's ORDER BY to ask for the next page. The keyset
comparison is a SQLite row value, planned against the same index.
A contract test asserts no query parameter in the published document is
nullable: that was the tell, and `.min(0)` versus `.min(1)` was invisible in
review.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Uint8Array.toBase64`/`fromBase64` take the alphabet and padding as options, so the hand-rolled `+/` to `-_` swap and the padding arithmetic around `atob` both go away. That arithmetic was the only part of this worth getting wrong: it fails for particular input lengths, which is exactly the shape of bug a test suite of round-trips can miss. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Every paged list in the product could silently return an empty page. Reported as "finished games don't appear in history" and "an active game doesn't appear on home"; both were the same bug, and it was in neither history nor the home screen.
Root cause
Number(null)andNumber("")are both0, soz.coerce.number()is not a parser. Four layers then behaved correctly and made it invisible:z.coerce.number().int().min(0)genuinely acceptsnulland yields0, so the emitted schema honestly published["integer", "null"]. With.min(1)it does not — which is why only some parameters were affected, and why the difference was invisible in review.openapi-generatorread that nullability and correctly dropped itsif (x != null)guard. The API had declared null welcome.?cursor/?to=.""straight back to0.A cursor of
0means "older than anything that has ever happened", so the lobby, bothgames/minebuckets, the friends list and every history screen answered200with an empty array. Atoof0clamped replays to a single frame. Every one of those requests was logged as a success.No library was misbehaving. There is no upstream issue to file.
Changes
Query parameters are parsed, never coerced. Integer params parse from a strict pattern, so they reject null and empty alike. The generated client now omits an absent parameter on its own, and a malformed one is a loud
400. Nothing downstream compensates — no request-rewriting middleware, no client interceptor.Cursors are opaque tokens carrying
(sortValue, id), and paged responses returnnextCursor, null exactly when the list is exhausted. This:d1/reads.tsdocumented but did not fixORDER BYto request the next pageThe keyset comparison is a SQLite row value,
(sortKey, id) < (?, ?), planned against the same index as the hand-expanded form.A contract test asserts no query parameter is nullable. Verified load-bearing: reverting
totoz.coercefails it and namesgetFrames.to.Breaking
cursoris now a string, the four paged responses (Lobby,MyGames,PlayerGames,FriendsGames) gained a requirednextCursor, and there is a newinvalidCursorerror code. The generated Dart client is regenerated here.eigen-flutter must ship with this. Its PR is blocked until this releases a new
eigen_api— it pins^0.3.0and only builds locally via the gitignored override.Verification
openapi.jsonandclients/dartregenerated🤖 Generated with Claude Code