fix(backup): reject repeated Drive backup page tokens

[SebTardif]

What Problem This Solves

gog backup push with Drive lists shared drives (Drives.List) and files (Files.List) by walking Google page tokens. Both helpers in internal/cmd/backup_drive.go copied nextPageToken into the next request with no seen-set.

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

The same hang class is already closed for Chat and Classroom backup (#1063), Drive sync push listing (#1065), Drive audit permission listing (#1066), calendar and Gmail listing (#1004), and People/Gmail-from-contact/contacts-export listing (#1044, #1045, #1046). Drive backup listing was still on the unguarded loop.

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-paging.exe -test.run TestFetchBackupSharedDrivesRejectsRepeatedPageToken -test.v
    backup_drive_test.go:53: err = pagination loop: repeated page token "stuck" after 2 list calls

$ gogcli-backup-paging.exe -test.run TestFetchBackupDriveFilesRejectsRepeatedPageToken -test.v
    backup_drive_test.go:101: 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: Drive backup shared-drive and file listing hung when Google repeated a page token. gog backup push with Drive never finished those listings.
• Real environment tested: Windows, Go 1.27.1, branch fix/backup-paging-seen-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-paging.exe with -test.v on the shared-drive and file hang-guard cases.
• Evidence after fix: terminal output from that binary shows pagination loop: repeated page token "stuck" after 2 list calls for both fetchBackupSharedDrives and fetchBackupDriveFiles.
• Observed result after fix: A repeated Drive backup page token now fails closed with the shared paginator error after two requests. Distinct pages still concatenate. Shared drives stay sorted by id. File listing still uses driveFilesListCallWithDriveSupport (corpora=allDrives).
• What was not tested: A live Google Drive account with a real repeated token. Snapshot write / --best-effort handling after the listing error.

Summary

Route fetchBackupSharedDrives and fetchBackupDriveFiles through existing collectAllPages. Keep the original field lists, page sizes, query, order, shared-drive flags, and shared-drive sort.

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

Introduced in 068ff0e5 (2026-04-27, feat(backup): expand workspace backup coverage).

[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, 12:38 AM ET / 04:38 UTC.

ClawSweeper review

What this changes

Route Drive backup’s shared-drive and file listings through the existing pagination guard, with regression tests for repeated tokens and successful multi-page results.

Merge readiness

⛔ Blocked before merge - 3 items remain

This remains a distinct, source-proven gap on main and v0.39.0; the linked merged fixes cover other listing paths. The supplied fault-injection evidence supports the repair.

Priority: P2
Reviewed head: e255ffd7057ba329183701685452b0a8a811286a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with credible production-boundary evidence and bounded release-note and compatibility follow-up.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.
Evidence reviewed	11 items	Applicable repository policy: Read the full root AGENTS.md. It requires user-visible fixes in Unreleased with a PR reference and contributor credit. No nested AGENTS.md or maintainer-notes directory was present. Current main still has both unguarded loops: Both collectors copy NextPageToken into another request without detecting repeated tokens. A peer that repeatedly returns the same nonempty token keeps both loops running. Latest release also lacks this fix: The v0.39.0 version contains the same unguarded Drive backup loops.
Findings	1 actionable finding	[P3] [P3] Record the backup behavior change in Unreleased
Security	None	None.

How this fits together

Drive backup collects shared-drive and file metadata before building encrypted backup shards. Listing errors stop snapshot collection before the final backup snapshot is published.

flowchart TD
 A[Backup push command] --> B[Drive backup collector]
 B --> C[Google Drive HTTP listings]
 C --> D[Shared pagination guard]
 D -->|Complete listing| E[Build encrypted backup shards]
 D -->|Repeated token or page limit| F[Return collection error]
 E --> G[Publish completed snapshot]

Loading

Before merge

• [P3] Record the backup behavior change in Unreleased (P3) - This changes observable gog backup push behavior, but the branch has no changelog entry. Root AGENTS.md requires user-visible fixes in Unreleased with a PR reference and contributor thanks. Add the entry under 0.39.1 - Unreleased, including the newly inherited page ceiling rather than placing it under the published release.
• Resolve merge risk (P1) - The inherited 10,000-page ceiling can reject an otherwise valid large Drive backup, including under --best-effort. This upgrade boundary is undocumented for Drive, and the supplied adapter runs do not exercise backup-level error propagation.
• Complete next step (P2) - Add the Unreleased entry, document the Drive page ceiling and strict error behavior, and add focused backup-boundary compatibility coverage.

Findings

• [P3] [P3] Record the backup behavior change in Unreleased — internal/cmd/backup_drive.go:102

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production -4 lines; tests +192 lines	The repair reuses existing production logic and adds four focused regression cases.

Merge-risk options

Maintainer options:

1. Document and verify the inherited limit (recommended)
   Explain the Drive page ceiling and add backup-boundary coverage showing that guard failures abort collection under both best-effort settings.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Preserve the shared paginator and existing Drive error policy. Document the 10,000-page ceiling and fatal Drive collection errors under either best-effort setting in docs/backup.md, add focused backup-boundary regression coverage, and add the required Unreleased changelog entry with the PR reference and contributor credit.

Technical review

Best possible solution:

Keep the shared guard, document its Drive listing ceiling, and preserve the strict snapshot-error contract with focused compatibility coverage.

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

Yes: current main repeatedly requests the same page when either Drive endpoint returns a repeated nonempty token. The contributor supplies a bounded before/after HTTP reproduction; this read-only review did not execute it.

Is this the best way to solve the issue?

Yes: reusing the established collector is a narrow repair that preserves normal request and result behavior. Its additional page ceiling needs explicit Drive documentation and compatibility coverage.

Full review comments:

• [P3] [P3] Record the backup behavior change in Unreleased — internal/cmd/backup_drive.go:102
  This changes observable gog backup push behavior, but the branch has no changelog entry. Root AGENTS.md requires user-visible fixes in Unreleased with a PR reference and contributor thanks. Add the entry under 0.39.1 - Unreleased, including the newly inherited page ceiling rather than placing it under the published release.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

• add P2: This is a bounded backup reliability repair for repeated provider continuation tokens, with no evidence of a widespread current outage.
• add merge-risk: 🚨 compatibility: Using the shared collector newly limits Drive backup to 10,000 pages, after which even valid pagination aborts the backup.
• add proof: sufficient: Contributor real behavior proof is sufficient. The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.

Label justifications:

• P2: This is a bounded backup reliability repair for repeated provider continuation tokens, with no evidence of a widespread current outage.
• merge-risk: 🚨 compatibility: Using the shared collector newly limits Drive backup to 10,000 pages, after which even valid pagination aborts the backup.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured Windows terminal traces exercise both changed production collectors through Google's generated Drive client and an actual HTTP fault-injection server, showing rejection after two requests with no partial results. This sufficiently proves the narrow loop repair; live Google faults and final snapshot publication were not claimed or demonstrated.

Evidence

Acceptance criteria:

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

What I checked:

• Applicable repository policy: Read the full root AGENTS.md. It requires user-visible fixes in Unreleased with a PR reference and contributor credit. No nested AGENTS.md or maintainer-notes directory was present. (AGENTS.md:38, e255ffd7057b)
• Current main still has both unguarded loops: Both collectors copy NextPageToken into another request without detecting repeated tokens. A peer that repeatedly returns the same nonempty token keeps both loops running. (internal/cmd/backup_drive.go:101, 25703c789da5)
• Latest release also lacks this fix: The v0.39.0 version contains the same unguarded Drive backup loops. (internal/cmd/backup_drive.go:101, b058dbbaeb44)
• Verified introduced scope: The pinned delta changes only the two Drive backup collectors and adds four tests. Production changes preserve fields, filters, page sizes, request context, shared-drive support, sorting, and nil-on-error behavior. (internal/cmd/backup_drive.go:102, e255ffd7057b)
• After-fix production-boundary evidence: The captured PR body reports Windows runs of the compiled test binary calling both production collectors. Each receives repeated HTTP continuation tokens and returns the pagination error after exactly two requests, without partial results; the old implementation reached the third-request safety cap. (internal/cmd/backup_drive_test.go:13, e255ffd7057b)
• Proof uses the real generated HTTP client: newDriveTestService delegates to drive.NewService; the shared fixture supplies an actual HTTP client connected to an httptest server. The transport and production collectors are not mocked. (internal/cmd/google_service_testutil_test.go:71, e255ffd7057b)

Likely related people:

• 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)
• Sebastien Tardif: Raw commit eaea842 adds internal/cmd/paging_test.go:113 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)

Rank-up moves

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

• Add the required Unreleased entry and document the new Drive page ceiling.
• Cover Drive collection failure through the backup boundary with best-effort enabled and disabled, preserving normal multi-page success.

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.

[steipete]

Superseded by #1087, which incorporates the Drive backup pagination repair while preserving unlimited distinct-page scans. Stock-CLI proof verifies termination and unchanged existing backup state on failure. Thanks @SebTardif. Superseded by #1087, which incorporates Groups, Admin, and Keep cycle protection and preserves fetched membership rows on ordinary later-page API failures. Thanks @SebTardif. The separate Cloud Identity query issue #1059 remains open. Superseded by #1087, which incorporates Drive collaboration cycle protection without adding a new page ceiling. Encrypted per-file error records and continued collection were verified. Thanks @SebTardif.