fix(drive): reject repeated tree inventory page tokens

[SebTardif]

What Problem This Solves

gog drive tree, gog drive inventory, and gog drive du walk folder children through listDriveChildren in internal/cmd/drive_reporting.go. That helper copied nextPageToken into the next Files.List request with no seen-set.

When Drive repeats a continuation token, the loop never terminates. Tree, inventory, and disk-usage scans keep requesting the same page and never return.

The same hang class is already closed for 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). Folder children used by the read-only reporting commands were 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. The same error reaches the public commands:

$ gogcli-drive-reporting.exe -test.run TestListDriveChildrenRejectsRepeatedPageToken -test.v
    drive_reporting_test.go:381: err = pagination loop: repeated page token "stuck" after 2 list calls

$ gogcli-drive-reporting.exe -test.run TestExecuteDriveReportingRejectsRepeatedPageToken -test.v
    drive_reporting_test.go:496: list Drive folder root: pagination loop: repeated page token "stuck"; no success output
--- PASS: TestExecuteDriveReportingRejectsRepeatedPageToken/tree
--- PASS: TestExecuteDriveReportingRejectsRepeatedPageToken/inventory
--- PASS: TestExecuteDriveReportingRejectsRepeatedPageToken/du

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 tree, inventory, and du hung when Google repeated a folder-list page token. Those commands never finished a folder walk.
• Real environment tested: Windows, Go 1.27.1, branch fix/drive-children-page-token from origin/main at 25703c78, compiled internal/cmd listing binary.
• Exact steps or command run after this patch: Compiled the listing binary with go, then ran gogcli-drive-reporting.exe with -test.v on the helper hang-guard and the public drive tree / drive inventory / drive du command path.
• Evidence after fix: terminal output from that binary shows pagination loop: repeated page token "stuck" after 2 list calls for listDriveChildren, and list Drive folder root: pagination loop: repeated page token "stuck" with empty stdout for tree, inventory, and du.
• Observed result after fix: A repeated Drive children page token now fails closed with the shared paginator error after two requests. Distinct pages still concatenate in order. A later provider error still discards partial children. Query, page size 1000, orderBy=folder,name, and driveFilesListCallWithDriveSupport (corpora=allDrives) are unchanged.
• What was not tested: A live Google Drive account with a real repeated token.

Summary

Route listDriveChildren through existing collectAllPages. Keep the original query, fields, page size, order, shared-drive flags, and empty-parent default to root.

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

Introduced in e9c496ef (2026-05-05, feat(drive): add read-only reporting commands, #554).

[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: needs changes before merge. Reviewed September 5, 2026, 4:00 AM ET / 08:00 UTC.

ClawSweeper review

What this changes

The PR uses the shared pagination guard for Drive tree, inventory, and disk-usage reports, with regression coverage for repeated tokens, ordered results, and failed listings.

Merge readiness

⛔ Needs changes before merge - 2 items remain

This PR remains necessary: current main and v0.39.0 retain the unguarded reporting loop, and the related merged fixes cover different callers. The implementation and supplied fault-injection proof are sound.

Priority: P2
Reviewed head: 090dcec2776edb43c9717b176063cef1ded1e81f

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused implementation with relevant transport and command-path evidence; the required release note remains.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.
Evidence reviewed	9 items	Current main still needs the fix: The fetched main implementation appends each page and copies NextPageToken into the next request without tracking previously requested tokens. Latest release retains the same loop: The v0.39.0 source also contains the unguarded folder-child paginator; the requested fix is not present in that release. Verified introduced implementation: The pinned base-to-head diff preserves the query, selected fields, request context, ordering, page size, and shared-drive flags while replacing the loop with collectAllPages.
Findings	1 actionable finding	[P3] [P3] Record the reporting fix in Unreleased
Security	None	None.

How this fits together

Drive reporting commands fetch folder contents through Google's Drive client, then traverse those results to produce tree, inventory, or storage reports. Pagination failures must reach the command before it emits a successful report.

flowchart TD
  A[Drive reporting command] --> B[Folder traversal]
  B --> C[Drive file listing]
  C --> D[Shared pagination guard]
  D -->|Another distinct token| C
  D -->|Complete listing| E[Build report]
  D -->|Repeated token or error| F[Error without success output]

Loading

Before merge

• [P3] Record the reporting fix in Unreleased (P3) - This changes the observable failure behavior of drive tree, drive inventory, and drive du, but the branch has no changelog entry. Root AGENTS.md requires user-visible fixes in Unreleased with a PR reference and contributor thanks. Add that entry under 0.39.1 - Unreleased so the reporting repair is included in release notes.
• Complete next step (P2) - Add the repository-required Unreleased changelog entry before landing.

Findings

• [P3] [P3] Record the reporting fix in Unreleased — internal/cmd/drive_reporting.go:384

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus test delta	Production -8 lines (+4/-12); tests +173 lines	The repair reduces production code by reusing the existing collector and adds adapter and command-path coverage.

Technical review

Best possible solution:

Keep one pagination policy across Drive scanners, preserving ordered results and rejecting incomplete reports without partial success output.

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

Yes: two Drive responses returning the same nonempty nextPageToken cause current main to request that page again. Source inspection establishes the defect; the contributor supplies before/after fault-injection results, and this read-only review did not execute tests.

Is this the best way to solve the issue?

Yes: delegating to the existing collector is the narrowest maintainable repair, and the inspected callers preserve output and error behavior.

Full review comments:

• [P3] [P3] Record the reporting fix in Unreleased — internal/cmd/drive_reporting.go:384
  This changes the observable failure behavior of drive tree, drive inventory, and drive du, but the branch has no changelog entry. Root AGENTS.md requires user-visible fixes in Unreleased with a PR reference and contributor thanks. Add that entry under 0.39.1 - Unreleased so the reporting repair is included in release notes.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.95

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 reliability fix for three reporting commands when a provider repeats a continuation token.
• add proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.
• 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 supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.

Label justifications:

• P2: This is a bounded reliability fix for three reporting commands when a provider repeats a continuation token.
• 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 supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.
• proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows terminal trace exercises listDriveChildren and all three production command paths through Google's generated Drive HTTP client against an injected repeated-token response; it shows termination after two requests and no success output. This satisfies the internal reliability proof boundary.

Evidence

Acceptance criteria:

• [P1] git diff --check.
• [P1] git diff -- CHANGELOG.md.

What I checked:

• Current main still needs the fix: The fetched main implementation appends each page and copies NextPageToken into the next request without tracking previously requested tokens. (internal/cmd/drive_reporting.go:367, 25703c789da5)
• Latest release retains the same loop: The v0.39.0 source also contains the unguarded folder-child paginator; the requested fix is not present in that release. (internal/cmd/drive_reporting.go:367, b058dbbaeb44)
• Verified introduced implementation: The pinned base-to-head diff preserves the query, selected fields, request context, ordering, page size, and shared-drive flags while replacing the loop with collectAllPages. (internal/cmd/drive_reporting.go:384, 090dcec2776e)
• Existing pagination contract: The shared collector rejects repeated tokens before another request, retains page order, returns nil on errors, and enforces its existing 10,000-page bound. Its tests cover repeated tokens, longer cycles, successful paging, partial-result rejection, and the bound. (internal/cmd/paging.go:19, 090dcec2776e)
• After-fix production-path evidence: The supplied PR body records Windows/Go 1.27.1 terminal output from the compiled test binary: the production listing helper rejects the repeated token after two requests, and all three command paths return the folder-specific error without success output. The inspected fixtures assert token handoff, request counts, empty stdout, and exit code 1. This is synthetic provider-fault evidence, not a claim of a faulty live Google response. (internal/cmd/drive_reporting_test.go:446, 090dcec2776e)
• Real transport behind the fixture: newDriveTestService constructs Google's generated Drive service. The shared fixture supplies an actual httptest HTTP server and HTTP client, while executeWithTestRuntime invokes the production command executor. (internal/cmd/google_service_testutil_test.go:71, 090dcec2776e)

Likely related people:

• steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

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

• Add the Unreleased changelog entry with the PR reference and contributor credit.

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 cycle protection for Drive tree, inventory, and disk-usage scans. All three stock CLI commands were verified to fail without partial success output. Thanks @SebTardif. Superseded by #1087, which incorporates Gmail query-cycle protection, including repeated empty pages, while retaining existing result limits. Failure occurs before downstream message changes. Thanks @SebTardif.