fix(paging): stop calendar and Gmail listing on a repeated page token - #1004
fix(paging): stop calendar and Gmail listing on a repeated page token#1004SebTardif wants to merge 1 commit into
Conversation
listCalendarList now uses collectAllPages. Gmail backup ID listing rejects a repeated NextPageToken the same way Photos Picker already does, so a stuck Google token cannot hang --all or backup. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 18, 2026, 7:22 PM ET / 23:22 UTC. ClawSweeper reviewWhat this changesThe PR detects repeated Google page tokens in calendar-list and Gmail-backup ID pagination and adds regression coverage for both paths. Regression provenancePossible regression — probable (reviewed change; failure trace). No predecessor PR is attributed. Merge readiness✅ Ready for maintainer review Keep open: current main lacks both repeated-token guards, and this focused patch follows the repository’s existing pagination contract with credible terminal proof. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCalendar commands enumerate calendars before selecting or fetching events, while Gmail backup enumerates message IDs before fetching content. Both consume Google API page tokens; this change turns a token cycle into an explicit error rather than an unbounded request loop. flowchart LR
A[Calendar or Gmail command] --> B[Google list request]
B --> C[Items and next token]
C --> D{Token seen before?}
D -->|No| B
D -->|Yes| E[Return paging error]
C --> F[Event selection or backup fetch]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Land the focused guards after normal checks complete so both affected list flows fail explicitly instead of consuming requests indefinitely. Do we have a high-confidence way to reproduce the issue? Yes. Deterministic Calendar HTTP and Gmail source harnesses can return the same non-empty token repeatedly, and the source directly establishes the current-main loop condition. Is this the best way to solve the issue? Yes. Reusing the existing shared calendar collector and adding an equivalent local guard in the isolated Gmail package is the narrowest maintainable solution. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against eb85a99366eb. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
1 participant
What Problem This Solves
gog calendar events --all(and any other path that lists every calendar) walksCalendarList.Listwith no memory of page tokens it has already seen. Gmail backup ID listing does the same onusers.messages.list. If Google repeatsnextPageToken, both loops never finish.Why
The repo already has this guard.
collectAllPageserrors withpagination loop: repeated page token. Photos Picker and Drive changes do the same with a local seen-token map.listCalendarListis in packagecmd, so it now callscollectAllPagesinstead of growing a second helper. Gmail backup lives underinternal/backup/gmailand cannot importcmd, so it adds the same localseenTokensmap Photos Picker already uses.User Impact
A stuck Google page token now fails with
repeated page tokeninstead of hanginggog calendar events --allor a Gmail backup list until the process is killed.Evidence
terminal output from the unpatched loops versus this patch, using a Calendar
httptestserver and a Gmail source that always returnnextPageToken=stuck.Unpatched (300ms context; the HTTP peer answers immediately, so the deadline is the only stop):
Patched (same commands, same stuck token, returns immediately):
go test ./internal/cmd/ ./internal/backup/gmail/ -count=1also completed on this tree.Real behavior proof
nextPageTokenvalues could hang calendar listing (--all) and Gmail backup ID listing./tmp/pr-gogcli-pagesonfix/repeated-page-tokensfromupstream/main./tmp/pr-gogcli-pages, ran the samego testcommands shown above against a Calendarhttptestthat always returnsnextPageToken=stuck, then rango run proof_gmail_pages.gowhich calls exportedListMessageIDswith a source that always returnsNextPageToken "stuck".pagination loop: repeated page token "stuck"in 0.00s. GmailListMessageIDsreturnslist Gmail backup messages: repeated page token "stuck"withelapsed=0s.Related
collectAllPagesininternal/cmd/paging.go, Photos Picker (errPhotosPickerRepeatedPage), and Drive changes.9977c0be, 2026-02-15, 184 days ago).12461f5d(2026-06-13, 66 days ago).