fix(backup): reject repeated Calendar Contacts Tasks page tokens

[SebTardif]

What Problem This Solves

gog backup push with calendar, contacts, or tasks walks Google page tokens in nine list helpers in internal/cmd/backup_services.go (calendar ACL, settings, calendars, events, People connections, other contacts, contact groups, task lists, and tasks). Each helper copied nextPageToken into the next request with no seen-set.

When Calendar, People, or Tasks repeats a continuation token, those loops never terminate. Backup collection keeps requesting the same page and never finishes the snapshot.

The same hang class is already closed for Chat and Classroom backup (#1063), Drive backup listing (#1078), Groups/Admin/Keep backup listing (#1079), Drive collaboration listing (#1080), Drive sync and audit listing (#1065, #1066), interactive calendar listing (#1004), and People/Gmail-from-contact/contacts-export listing (#1044, #1045, #1046). Calendar, Contacts, and Tasks backup listing was still on the unguarded loop. Interactive gog calendar listing already uses collectAllPages; backup did not.

Evidence

terminal output from the compiled internal/cmd listing binary after the patch. A stuck continuation token is rejected after two list calls, with no third request:

$ gogcli-backup-services.exe -test.run TestFetchBackupCalendarsRejectsRepeatedPageToken -test.v
    backup_services_test.go:56: err = pagination loop: repeated page token "stuck" after 2 list calls

$ gogcli-backup-services.exe -test.run TestFetchBackupConnectionsRejectsRepeatedPageToken -test.v
    backup_services_test.go:98: err = pagination loop: repeated page token "stuck" after 2 list calls

$ gogcli-backup-services.exe -test.run TestFetchBackupTaskListsRejectsRepeatedPageToken -test.v
    backup_services_test.go:140: err = pagination loop: repeated page token "stuck" after 2 list calls

$ gogcli-backup-services.exe -test.run TestFetchBackupCalendarEventsRejectsRepeatedPageToken -test.v
    backup_services_test.go:226: err = pagination loop: repeated page token "stuck" after 2 list calls

Before the patch, the same stuck token made a third list request and returned HTTP 400 from the safety cap (unexpected extra ... page request after 3 list calls) instead of stopping on the repeated token.

Real behavior proof

• Behavior or issue addressed: Calendar, Contacts, and Tasks backup listing hung when Google repeated a page token. gog backup push with those services never finished those listings.
• Real environment tested: Windows, Go 1.27.1, branch fix/backup-services-page-token at current HEAD, compiled internal/cmd listing binary.
• Exact steps or command run after this patch: Compiled the listing binary with go, then ran gogcli-backup-services.exe with -test.v on the calendar, connections, task-list, and event hang-guard cases.
• Evidence after fix: terminal output from that binary shows pagination loop: repeated page token "stuck" after 2 list calls for fetchBackupCalendars, fetchBackupConnections, fetchBackupTaskLists, and fetchBackupCalendarEvents.
• Observed result after fix: A repeated Calendar, Contacts, or Tasks backup page token now fails closed with the shared paginator error after two requests. Distinct calendar pages still concatenate and stay sorted by id. ACL listing still records per-calendar API errors as rows instead of aborting the snapshot.
• What was not tested: A live Google account with a real repeated token. Snapshot write / --best-effort handling after the listing error. Settings, other-contacts, contact-groups, ACL, and per-list task paging use the same collectAllPages rewrite.

Summary

Route the nine Calendar, Contacts, and Tasks backup list helpers through existing collectAllPages. Keep the original page sizes, field masks, show-deleted flags, sorts, and ACL best-effort error rows.

Related: #1063, #1078, #1079, #1080, #1065, #1066, #1004, #1044, #1045, #1046.

Introduced in efc3df2e (2026-04-27, feat(backup): expand google backup coverage). Calendar list helper later moved in 6f917ee7.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: blocked before merge. Reviewed September 5, 2026, 3:28 AM ET / 07:28 UTC.

ClawSweeper review

What this changes

Route nine Calendar, Contacts, and Tasks backup listings through the existing pagination guard and add repeated-token and successful-pagination tests.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⛔ Blocked before merge - 4 items remain

The fix remains necessary on main and v0.39.0, and the supplied fault-injection proof supports the pagination guard. The previously reported ACL data-loss regression remains unresolved.

Priority: P2
Reviewed head: 15d84fd1cbdf818bff08e17f8a05e7a0519e8636

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The hang protection has useful production-path proof, but the unresolved ACL regression blocks merge.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The captured Windows traces exercise four changed backup adapters through generated Google clients and a real HTTP peer, showing repeated-token rejection after two requests with no partial result. This sufficiently proves the guard; ACL retention and final snapshot outcomes remain repair-validation gaps.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured Windows traces exercise four changed backup adapters through generated Google clients and a real HTTP peer, showing repeated-token rejection after two requests with no partial result. This sufficiently proves the guard; ACL retention and final snapshot outcomes remain repair-validation gaps.
Evidence reviewed	12 items	Applicable repository policy: Read the full root AGENTS.md; no nested instructions or matching maintainer notes were found. The repository requires user-visible fixes to be recorded in Unreleased with references and contributor thanks. Verified introduced change: The local diff matches the supplied introduction evidence. The test merge has the pinned main parent followed by the exact PR head and changes only the two reviewed files. Main and latest release still need the guard: Main contains the nine unguarded continuation loops. v0.39.0 and pinned main reference the identical backup_services.go blob, 0038cb5cbfeea624ddee5609eacb7713ae83ba4a, so the latest release does not contain this repair.
Findings	1 actionable finding	[P1] Preserve earlier ACL pages when a later API request fails
Security	None	None.

How this fits together

Google service adapters collect account data for encrypted backup snapshots. Their pagination and error handling determine which records reach the backup repository and whether collection can finish.

flowchart TD
  A[Backup push] --> B[Calendar Contacts Tasks adapters]
  C[Google API pages] --> B
  B --> D[Pagination guard]
  D --> E[Collected records]
  D --> F[Listing error]
  E --> G[Encrypted snapshot]
  F --> H[Abort collection or record ACL error]

Loading

Before merge

• Preserve earlier ACL pages when a later API request fails (P1) - If the first ACL page contains rules and the next request returns an ordinary error such as HTTP 403, collectAllPages returns nil and this branch appends only the error row. Main retained the earlier rules, and buildCalendarBackupSnapshot still treats this result as successful, so the resulting backup loses fetched ACL records. This previously reported blocker remains unchanged. Preserve the earlier pages alongside the error row and add a populated-first-page/403 regression without weakening the shared paginator.
• Resolve merge risk (P1) - Until repaired, a later ACL API failure can produce a successful Calendar backup missing rules already fetched.
• Resolve merge risk (P1) - The inherited 10,000-page limit can stop exceptionally large listings that previously completed; the supplied proof does not exercise final snapshot behavior on new or existing backup repositories.
• Complete next step (P2) - Preserve earlier ACL pages on later API errors, add focused regression and snapshot-boundary coverage, document the page limit, and include the required Unreleased entry.

Findings

• [P1] Preserve earlier ACL pages when a later API request fails — internal/cmd/backup_services.go:223-228

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +72/-89 (net -17); tests +217	The implementation reuses an existing helper and reduces production code.
Adapter coverage	9 adapters changed; 4 repeated-token cases and 1 success case added	The changed ACL error-preservation path has no regression coverage.

Merge-risk options

Maintainer options:

1. Preserve the ACL backup contract (recommended)
   Handle ordinary ACL API errors locally, retain successful pages, and verify the pagination and snapshot boundaries before landing.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve previously fetched ACL rules alongside the per-calendar error row when a later ordinary API request fails, keeping collectAllPages strict and avoiding error-string classification. Add generated-client regressions for populated-first-page/403 and repeated-token ACL cases. Verify that Calendar, Contacts, and Tasks collection failures stop the final snapshot with either best-effort setting, document the 10,000-page limit, and add the required Unreleased changelog entry with the PR reference and contributor thanks.

Technical review

Best possible solution:

Keep bounded pagination while preserving earlier ACL pages and per-calendar error rows, with documented and verified final-snapshot failure behavior.

Do we have a high-confidence way to reproduce the issue?

Yes, source establishes deterministic triggers: repeated tokens keep main’s loops running, and a populated ACL page followed by HTTP 403 exposes the PR’s partial-result regression. No tests were executed in this read-only review.

Is this the best way to solve the issue?

The shared guard is the right approach, but the ACL adapter must preserve its existing best-effort contract using local error handling, as the merged Classroom adapter already does.

Full review comments:

• [P1] Preserve earlier ACL pages when a later API request fails — internal/cmd/backup_services.go:223-228
  If the first ACL page contains rules and the next request returns an ordinary error such as HTTP 403, collectAllPages returns nil and this branch appends only the error row. Main retained the earlier rules, and buildCalendarBackupSnapshot still treats this result as successful, so the resulting backup loses fetched ACL records. This previously reported blocker remains unchanged. Preserve the earlier pages alongside the error row and add a populated-first-page/403 regression without weakening the shared paginator.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 25703c789da5.

Labels

Label justifications:

• P2: This is a bounded backup reliability repair for faulty provider pagination, without evidence of an active widespread outage.
• merge-risk: 🚨 compatibility: The patch changes ACL partial-result behavior and applies a new page bound to previously unbounded backup listings.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (terminal): The captured Windows traces exercise four changed backup adapters through generated Google clients and a real HTTP peer, showing repeated-token rejection after two requests with no partial result. This sufficiently proves the guard; ACL retention and final snapshot outcomes remain repair-validation gaps.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured Windows traces exercise four changed backup adapters through generated Google clients and a real HTTP peer, showing repeated-token rejection after two requests with no partial result. This sufficiently proves the guard; ACL retention and final snapshot outcomes remain repair-validation gaps.

Evidence

Acceptance criteria:

• [P2] go test ./internal/cmd -run '^(TestFetchBackup|TestCollectAllPages|TestBackupPush|TestBuildBackupSnapshots)' -count=1 -timeout=120s.
• [P2] go test -race ./internal/cmd -run '^(TestFetchBackup|TestCollectAllPages|TestBackupPush|TestBuildBackupSnapshots)' -parallel=1 -count=1 -timeout=180s.
• [P1] make ci.
• [P1] git diff --check.

What I checked:

• Applicable repository policy: Read the full root AGENTS.md; no nested instructions or matching maintainer notes were found. The repository requires user-visible fixes to be recorded in Unreleased with references and contributor thanks. (AGENTS.md:38, 15d84fd1cbdf)
• Verified introduced change: The local diff matches the supplied introduction evidence. The test merge has the pinned main parent followed by the exact PR head and changes only the two reviewed files. (internal/cmd/backup_services.go:212, 80625afbe3ca)
• Main and latest release still need the guard: Main contains the nine unguarded continuation loops. v0.39.0 and pinned main reference the identical backup_services.go blob, 0038cb5cbfeea624ddee5609eacb7713ae83ba4a, so the latest release does not contain this repair. (internal/cmd/backup_services.go:212, b058dbbaeb44)
• ACL partial-result regression: Main appends each successful ACL page before requesting another. The introduced collector discards accumulated rules on a later API error, after which the adapter emits only an error row and the Calendar snapshot can still succeed. (internal/cmd/backup_services.go:223, 15d84fd1cbdf)
• Strict shared-paginator contract: The shared helper returns nil on fetch errors, repeated tokens, and the 10,000-page bound. Existing tests explicitly require discarding partial results on fetch errors; the ACL exception belongs in its adapter. (internal/cmd/paging.go:24, 25703c789da5)
• Actual backup failure boundary: Calendar, Contacts, and Tasks are registered directly, without the optional-service wrapper. Their returned collection errors abort before the final PushSnapshot call regardless of --best-effort; ACL errors remain embedded rows. (internal/cmd/backup_dispatch.go:25, 25703c789da5)

Likely related people:

• Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Sebastien Tardif: Raw commit eaea842 adds internal/cmd/paging_test.go:92 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: eaea84227d4f; files: internal/cmd/paging_test.go)
• salmonumbrella: Raw commit 3371e3f adds internal/cmd/paging.go:24 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 3371e3f3ad6d; files: internal/cmd/paging.go)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Preserve earlier ACL pages on ordinary API errors and add generated-client coverage for that case and repeated tokens.
• Document the inherited page limit and verify final snapshot failure behavior with both best-effort settings on new and existing backup repositories.
• Record the fix under 0.39.1 - Unreleased with the PR reference and contributor thanks.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-09-05T07:20:45.157Z sha 6dcde08 :: blocked before merge. :: [P1] Preserve earlier ACL pages when a later API request fails

[steipete]

Superseded by #1087, which incorporates Calendar, Contacts, and Tasks cycle protection while retaining fetched ACL rows on ordinary API errors. Backup failure and encrypted-row behavior were verified. Thanks @SebTardif. Superseded by #1087, which incorporates Workspace and Forms pagination protection without introducing new page limits. Stock-CLI runs verified fatal inventory failures and embedded Forms error records. Thanks @SebTardif. Superseded by #1087, which incorporates contact-dedupe cycle protection while preserving early completion at --max. Repeated tokens stop before a partial plan or contact changes. Thanks @SebTardif.