Skip to content

fix(gmail): reject repeated archive search page tokens - #1086

Closed
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/gmail-archive-page-token
Closed

fix(gmail): reject repeated archive search page tokens#1086
SebTardif wants to merge 1 commit into
openclaw:mainfrom
SebTardif:fix/gmail-archive-page-token

Conversation

@SebTardif

Copy link
Copy Markdown
Contributor

What Problem This Solves

gog gmail archive --query and gog gmail autoreply resolve matching IDs through searchMessageIDs. That helper assigned pageToken = resp.NextPageToken until the token was empty or --max was reached. If Google repeats nextPageToken while remaining > 0, the loop never finishes.

An empty list page is the hang. remaining only shrinks by len(resp.Messages). A stuck token with no messages leaves remaining unchanged, so gog gmail archive -q 'in:inbox' keeps calling users.messages.list until the process is killed.

Why

Gmail backup ID listing already rejects a repeated token with a local seenTokens map (#1004). searchMessageIDs cannot switch to collectAllPages because it still has to honor --max and the 500-result page cap. This change uses the same seen-token check before assigning the next token.

User Impact

A stuck Gmail nextPageToken now fails with pagination loop: repeated page token instead of hanging gog gmail archive --query or gog gmail autoreply. Distinct page tokens still walk normally.

Evidence

terminal output from the unpatched search loop versus this patch. A local Gmail users.messages.list peer always returns nextPageToken=stuck.

Unpatched (the HTTP peer answers immediately; the hang guard returns 400 on the third list call so the loop cannot run forever):

$ go test ./internal/cmd -count=1 -timeout 60s -run "TestSearchMessageIDsRejectsRepeatedPageToken|TestGmailArchiveCmd_QueryRejectsRepeatedPageToken"
--- FAIL: TestSearchMessageIDsRejectsRepeatedPageToken (0.00s)
    gmail_archive_test.go:346: err = googleapi: got HTTP response code 400 with body: too many list requests
         after 3 list calls, ids=[]
--- FAIL: TestGmailArchiveCmd_QueryRejectsRepeatedPageToken (0.00s)
    gmail_archive_test.go:400: err = googleapi: got HTTP response code 400 with body: too many list requests
         after 3 list calls
FAIL

Patched (same commands, same stuck token, returns immediately after two list calls):

$ go test ./internal/cmd -count=1 -timeout 30s -v -run "TestSearchMessageIDsRejectsRepeatedPageToken|TestSearchMessageIDsRejectsRepeatedEmptyPageToken|TestSearchMessageIDsFollowsDistinctPageTokens|TestGmailArchiveCmd_QueryRejectsRepeatedPageToken"
=== RUN   TestSearchMessageIDsRejectsRepeatedPageToken
    gmail_archive_test.go:351: err = pagination loop: repeated page token "stuck" after 2 list calls
--- PASS: TestSearchMessageIDsRejectsRepeatedPageToken (0.00s)
=== RUN   TestSearchMessageIDsRejectsRepeatedEmptyPageToken
    gmail_archive_test.go:379: err = pagination loop: repeated page token "stuck" after 2 list calls
--- PASS: TestSearchMessageIDsRejectsRepeatedEmptyPageToken (0.00s)
=== RUN   TestSearchMessageIDsFollowsDistinctPageTokens
--- PASS: TestSearchMessageIDsFollowsDistinctPageTokens (0.00s)
=== RUN   TestGmailArchiveCmd_QueryRejectsRepeatedPageToken
    gmail_archive_test.go:434: err = pagination loop: repeated page token "stuck" after 2 list calls
--- PASS: TestGmailArchiveCmd_QueryRejectsRepeatedPageToken (0.00s)
PASS
ok  github.com/openclaw/gogcli/internal/cmd  2.704s

Live CLI still exposes the public query path:

$ .\bin\gog.exe gmail archive --help
Usage: gog gmail (mail,email) archive [<messageId> ...] [flags]
  -q, --query=STRING            Archive all messages matching this Gmail search
                                query
      --max=100                 Max messages to archive (with --query)

gofmt is clean. golangci-lint run ./internal/cmd/ reported 0 issues. go build ./internal/cmd/ succeeded.

Real behavior proof

  • Behavior or issue addressed: Repeated Google nextPageToken values could hang gog gmail archive --query and gog gmail autoreply while searchMessageIDs paged users.messages.list and remaining stayed above zero.
  • Real environment tested: Windows NT 10.0.26200.0 amd64, Go go1.27.1, worktree C:\Users\sebta\.grok\tmp\pr-gate-batch\gogcli-f022 on fix/gmail-archive-page-token from origin/main at 25703c78.
  • Exact steps or command run after this patch: From that worktree, ran go test ./internal/cmd -count=1 -timeout 30s -v -run TestSearchMessageIDsRejectsRepeatedEmptyPageToken against a Gmail list peer that always returns nextPageToken=stuck and no messages (so remaining never shrinks), then ran .\bin\gog.exe gmail archive --help on the binary built from this branch.
  • Evidence after fix: terminal output above. The search now returns pagination loop: repeated page token "stuck" after 2 list calls (0.00s). The unpatched loop made a third list call and only stopped when the hang guard returned HTTP 400.
  • Observed result after fix: The stuck token no longer pages while remaining > 0. searchMessageIDs and gog gmail archive --query in:inbox --max 50 return the repeated-token error on the next page instead of hanging. Two distinct tokens still return m1,m2.
  • What was not tested: A live users.messages.list response that actually repeats a token. That requires a faulty Google page, which we cannot force from a healthy account.

Related

  • Same-repo guard already used by Gmail backup ListMessageIDs in #1004 and by collectAllPages in internal/cmd/paging.go.
  • gog gmail search --from-contact fallback uses the same error in #1045.
  • Unguarded archive query paging dates to 74d8089a (landed in #385, 2026-03-02, 187 days ago).

searchMessageIDs assigned pageToken = resp.NextPageToken until empty
or --max. A repeated Google token hangs while remaining > 0.

Record each next token before assigning, matching Gmail backup
ListMessageIDs. Archive --query and autoreply then fail closed.

Signed-off-by: Sebastien Tardif <SebTardif@ncf.ca>
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

🦞👀
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 clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed September 5, 2026, 4:07 AM ET / 08:07 UTC.

ClawSweeper review

What this changes

Adds repeated-page-token detection to Gmail query pagination used by bulk message commands and autoreply, with four regression tests.

Merge readiness

Needs changes before merge - 2 items remain

This fix remains necessary: current main and v0.39.0 still contain the unguarded query loop, while the related merged fixes cover separate pagination paths. The implementation and supplied fault-injection proof support keeping this PR as the landing candidate.

Priority: P2
Reviewed head: 45749612297ddcb8fc3c02ac0205a67700637ebe

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-supported reliability fix with no functional finding and one small repository-policy omission.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (terminal): The supplied Windows terminal transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The supplied Windows terminal transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.
Evidence reviewed 10 items Verified introduced change: The local checkout matches the pinned PR head, and its delta matches the host introduction evidence: two files, with eight production lines added, two removed, and 122 test lines added.
Current-main defect remains: The main implementation decrements remaining only by the number of returned messages and follows NextPageToken without tracking prior tokens. Empty responses carrying the same nonempty token therefore repeat indefinitely while requests keep succeeding.
Latest release also lacks the guard: v0.39.0 resolves to b058dbb; its gmail_archive.go blob is identical to current main and contains the same unguarded loop.
Findings 1 actionable finding [P3] Document the pagination fix in the Unreleased changelog
Security None None.

How this fits together

Gmail query pagination collects message IDs before bulk label changes or automatic replies. It consumes Google list responses and returns either matching IDs or an error to those commands.

flowchart TD
  A[Query and message limit] --> B[Gmail message listing]
  B --> C[Collect IDs and inspect next token]
  C -->|New token and remaining capacity| B
  C -->|Finished or limit reached| D[Return matching IDs]
  C -->|Repeated token| E[Return pagination error]
  D --> F[Bulk label changes or automatic replies]
Loading

Before merge

  • Document the pagination fix in the Unreleased changelog (P3) - This introduces a user-visible failure mode for stuck query pagination, but the PR changes only code and tests. The repository's AGENTS.md requires user-visible fixes in CHANGELOG.md as they land. Add an entry under 0.39.1 - Unreleased describing the affected Gmail commands, referencing this PR, and thanking the contributor.
  • Complete next step (P2) - Add an entry under CHANGELOG.md's 0.39.1 - Unreleased section with the fix description, PR reference, and contributor thanks.

Findings

  • [P3] Document the pagination fix in the Unreleased changelog — internal/cmd/gmail_archive.go:288-289
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test delta Production +6 net lines; tests +122 lines across 4 new cases The small production increase implements a bounded guard, with coverage for repeated tokens, empty pages, normal continuation, and command propagation.

Technical review

Best possible solution:

Preserve bounded query collection and normal pagination while returning a clear cycle error before message mutations or replies.

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

Yes: successful empty Gmail list responses with a repeated nonempty nextPageToken leave main's remaining count unchanged and repeat the request. Source inspection and the supplied HTTP fault transcript support this path; the reviewer did not execute it.

Is this the best way to solve the issue?

Yes: a local seen-token guard preserves the collector's existing result limit and page-size behavior, follows the backup precedent, and propagates failure through existing callers.

Full review comments:

  • [P3] Document the pagination fix in the Unreleased changelog — internal/cmd/gmail_archive.go:288-289
    This introduces a user-visible failure mode for stuck query pagination, but the PR changes only code and tests. The repository's AGENTS.md requires user-visible fixes in CHANGELOG.md as they land. Add an entry under 0.39.1 - Unreleased describing the affected Gmail commands, referencing this PR, and thanking the contributor.
    Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.96

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 focused reliability fix for Gmail commands hanging on malformed pagination responses.
  • add proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows terminal transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.
  • 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 transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.

Label justifications:

  • P2: This is a focused reliability fix for Gmail commands hanging on malformed pagination responses.
  • 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 transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.
  • proof: sufficient: Contributor real behavior proof is sufficient. The supplied Windows terminal transcript exercises searchMessageIDs and the archive command through the real Gmail HTTP client against an injected repeated-token server response, showing controlled failure after two requests and successful ordinary continuation. This satisfies the internal reliability proof requirement; CLI help output is supplemental.

Evidence

Acceptance criteria:

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

What I checked:

  • Verified introduced change: The local checkout matches the pinned PR head, and its delta matches the host introduction evidence: two files, with eight production lines added, two removed, and 122 test lines added. (internal/cmd/gmail_archive.go:259, 45749612297d)
  • Current-main defect remains: The main implementation decrements remaining only by the number of returned messages and follows NextPageToken without tracking prior tokens. Empty responses carrying the same nonempty token therefore repeat indefinitely while requests keep succeeding. (internal/cmd/gmail_archive.go:282, 25703c789da5)
  • Latest release also lacks the guard: v0.39.0 resolves to b058dbb; its gmail_archive.go blob is identical to current main and contains the same unguarded loop. (internal/cmd/gmail_archive.go:282, b058dbbaeb44)
  • Caller behavior and compatibility: Bulk commands propagate search errors before BatchModify; autoreply likewise returns before processing messages or sending replies. Existing message limits and the 500-result request cap remain intact. Autoreply can already create its deduplication label before searching, so this is not a claim of globally side-effect-free execution. (internal/cmd/gmail_archive.go:190, 45749612297d)
  • Supplied production-path fault evidence: The captured PR body reports Windows runs where the unpatched production search and archive command reach a third HTTP request, while the patched code returns the repeated-token error after two requests. It also records successful empty-page and ordinary two-page scenarios. Captured context identity: fe1b467b25f2107d59e93cc73b80d4fc2d3c2d27ad54ef2eb929427851057289. The separate CLI help output does not prove pagination behavior. (internal/cmd/gmail_archive_test.go:335, 45749612297d)
  • Real HTTP transport confirmed: The exercised helper constructs gmail.NewService against an httptest HTTP server using its real HTTP client. The injected fault is the server response; the production paginator and Gmail transport client are not replaced with mock implementations. (internal/cmd/google_service_testutil_test.go:71, 45749612297d)

Likely related people:

  • Peter Steinberger: 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 required Unreleased changelog entry 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.

@steipete

steipete commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

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.

@steipete steipete closed this Sep 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants