Skip to content

fix(google): cap contacts list pagination - #7

Merged
steipete merged 4 commits into
openclaw:mainfrom
SebTardif:fix/google-import-page-cap
Aug 16, 2026
Merged

fix(google): cap contacts list pagination#7
steipete merged 4 commits into
openclaw:mainfrom
SebTardif:fix/google-import-page-cap

Conversation

@SebTardif

@SebTardif SebTardif commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

clawdex import google walks gog contacts list by following nextPageToken with no page cap and no cycle check. If gog or the People API keeps returning the same token, or a fresh token on every page, the import loops until the process is killed. The public command never returns.

This is the same hang class as notcrawl#98 and babelfish#17: a pager that never empties nextPageToken.

Evidence

Public command against a local gog stand-in on PATH (same argv as production: gog --no-input contacts list --json --max 1000). Built binary: /tmp/clawdex-pagecap from this branch.

A sticky token still fails closed on the second page:

$ clawdex import google --account ada@example.com --dry-run
gog contacts list: repeated nextPageToken "same"
exit=1 elapsed=0.037s calls=2

A finite two-page listing still imports Ada then Grace.

$ clawdex import google --account ada@example.com --dry-run --json
create Ada people/c1
create Grace people/c2

A valid listing that ends on page 51 now succeeds (Claw P1: do not reject a terminating walk past 50). An incrementing hang still stops at 500 pages (--max 1000 is 500_000 contacts, above a normal account).

$ go test -count=1 -timeout 30s ./internal/google -run 'TestGogAdapterListContacts(UsesNoInput|RejectsRepeatedPageToken|CapsIncrementingPageTokens|CompletesAfterFiftyPages)' -v
--- PASS: TestGogAdapterListContactsUsesNoInput
--- PASS: TestGogAdapterListContactsRejectsRepeatedPageToken
--- PASS: TestGogAdapterListContactsCapsIncrementingPageTokens
--- PASS: TestGogAdapterListContactsCompletesAfterFiftyPages
ok  github.com/openclaw/clawdex/internal/google

CHANGELOG.md is left to the release process.

Real behavior proof

  • Behavior or issue addressed: clawdex import google no longer follows nextPageToken forever. A repeated token fails on the second page. Unique incrementing tokens fail at page 500. A listing that ends on page 51 still imports.

  • Real environment tested: macOS (Darwin arm64), Go from the worktree at /tmp/oc-impl-clawdex-pages, built ./cmd/clawdex as /tmp/clawdex-pagecap, local contacts repo from clawdex init.

  • Exact steps or command run after this patch: Built go build -o /tmp/clawdex-pagecap ./cmd/clawdex. Put gog stand-ins on PATH (repeat, increment, two-page, 51-page). Ran clawdex --config ... --repo ... import google --account ada@example.com --dry-run for the hang and two-page cases. Ran go test -count=1 -race ./internal/google for the 51-page success and 500-page hang.

  • Evidence after fix: terminal output from the patched binary and package run:

    $ clawdex import google --account ada@example.com --dry-run
    gog contacts list: repeated nextPageToken "same"
    exit=1 elapsed=0.037s calls=2
    
    $ go test -count=1 ./internal/google -run CompletesAfterFiftyPages
    ok  github.com/openclaw/clawdex/internal/google
  • Observed result after fix: A sticky nextPageToken of same stops after two gog contacts list calls. Incrementing page-N tokens stop at 500 pages. A 51-page terminating listing returns 51 contacts instead of exceeded 50 pages.

  • What was not tested: A live Google People API account with a real sticky nextPageToken. Avatar fetch during a capped walk.

Summary

  • Cap gog contacts list pagination at 500 pages.
  • Reject a repeated nextPageToken.
  • Keep a terminating listing that needs more than 50 pages.
  • Check ctx.Err() at the start of each page so a cancelled import stops between pages.

Origin

The unbounded for { nextPageToken } walk landed in fc837601e11b (2026-05-08, Peter Steinberger) in the initial CLI bootstrap. Present for 99 days. Each page already used CommandContext; the loop itself had no stop.

Related

  • Sibling: notcrawl#98 rejects a repeated MCP tools/list cursor.
  • Sibling: babelfish#17 caps MCP tools/list at 50 pages and rejects a repeated cursor.

Verification

  • gofumpt / gofmt clean on internal/google/gog.go and gog_test.go.
  • go vet ./internal/google
  • go test -count=1 ./...
  • go test -count=1 -race ./internal/google
  • go build -o /tmp/clawdex-pagecap ./cmd/clawdex
  • Live clawdex import google against the hang and two-page gog stand-ins above.

gog contacts list followed nextPageToken with no page cap or
repeated-token stop. A sticky or incrementing token hangs
clawdex import google forever.

Cap the walk at 50 pages and reject a repeated nextPageToken.

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

clawsweeper Bot commented Aug 15, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 15, 2026
@clawsweeper

clawsweeper Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 15, 2026, 10:28 PM ET / August 16, 2026, 02:28 UTC.

ClawSweeper review

What this changes

This PR bounds Google contact pagination, rejects repeated continuation tokens, and adds regression coverage for token cycles and long page walks.

Regression provenance

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

Merge readiness

⚠️ Ready for maintainer review - 3 items remain

Keep open for maintainer review: the patch fixes a source-proven infinite Google-import loop, but accepting a fixed 500-page ceiling is an intentional availability boundary for exceptionally large existing imports.

Priority: P1
Reviewed head: 00550b632f6b59e4da70c0fbadec27b87150868e
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The patch is focused and proof-positive, with the remaining merge question being the intentional operational ceiling rather than a correctness defect.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (terminal): The PR body records after-fix execution of a built CLI binary against production-shaped gog stand-ins, including observed failure of a sticky token and successful finite imports.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (terminal): The PR body records after-fix execution of a built CLI binary against production-shaped gog stand-ins, including observed failure of a sticky token and successful finite imports.
Evidence reviewed 4 items Current-main defect: Current main has an unbounded continuation-token loop, so any non-terminating gog paginator repeatedly launches another command.
No partial local import on failure: The CLI waits for ListContactsWithOptions to succeed before calling the local contact-store import, so the proposed limit fails before writes.
Proposed safeguard: The PR replaces the unbounded loop with a 500-page bound, records all non-empty tokens, and errors on a repeated token.
Findings None None.
Security None None.

How this fits together

The Google importer invokes the external gog contacts list command repeatedly and converts returned contact pages before the CLI writes to the local contacts repository. Continuation tokens decide whether another command runs, the import completes, or an error is returned.

flowchart LR
  A[Google import command] --> B[Google contacts importer]
  B --> C[gog contacts list]
  C --> D[Contact page and token]
  D --> E{Token empty, new, or repeated?}
  E -->|new token| B
  E -->|empty token| F[Local contact import]
  E -->|repeated or limit reached| G[Import error]
Loading

Decision needed

Question Recommendation
Should clawdex import google treat 500 pages (up to 500,000 requested contacts) as the supported upper limit for an otherwise valid import? Accept the 500-page ceiling: Merge with the fixed bound as the availability safeguard against non-terminating paginators.

Why: The code correctly prevents an unbounded external pagination walk, but the fixed ceiling deliberately changes behavior for unusually large existing accounts and is not derived from a documented Google maximum.

Before merge

  • Resolve merge risk (P1) - A legitimate Google contact listing requiring more than 500 pages will now fail before writing any local contacts; current main would continue fetching it.
  • Complete next step (P2) - The remaining blocker is maintainer acceptance of the intentional 500-page compatibility and availability boundary, not a mechanical repair.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Pagination coverage 15 production lines changed, 101 test lines added The focused patch adds both the guard implementation and cases for repeated, alternating, incrementing, and 51-page token sequences.

Merge-risk options

Maintainer options:

  1. Accept the bounded-import policy (recommended)
    Merge after explicitly accepting that imports beyond 500 pages fail instead of continuing indefinitely.
  2. Set a different supported limit
    Pause for a maintainer-selected boundary if a 500-page limit is not acceptable for supported Google accounts.

Technical review

Best possible solution:

Retain cycle detection and a bounded paginator, with explicit maintainer acceptance that 500 pages is the intended operational limit for Google imports.

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

Yes—the current-main loop has no termination guard beyond an empty token, and the PR supplies a concrete gog stand-in path that produces repeating or perpetually fresh tokens.

Is this the best way to solve the issue?

Yes for the loop defect: token-cycle detection plus a bounded walk is a narrow solution; maintainer approval is still needed for the selected 500-page policy boundary.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 7b533852b1fd.

Labels

Label justifications:

  • P1: A non-terminating Google contacts paginator can make a public import command hang indefinitely.
  • merge-risk: 🚨 availability: The new fixed ceiling deliberately turns exceptionally long otherwise-valid imports into an error.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster 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 PR body records after-fix execution of a built CLI binary against production-shaped gog stand-ins, including observed failure of a sticky token and successful finite imports.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records after-fix execution of a built CLI binary against production-shaped gog stand-ins, including observed failure of a sticky token and successful finite imports.

Evidence

What I checked:

  • Current-main defect: Current main has an unbounded continuation-token loop, so any non-terminating gog paginator repeatedly launches another command. (internal/google/gog.go:48, 7b533852b1fd)
  • No partial local import on failure: The CLI waits for ListContactsWithOptions to succeed before calling the local contact-store import, so the proposed limit fails before writes. (internal/cli/cli.go:586, 7b533852b1fd)
  • Proposed safeguard: The PR replaces the unbounded loop with a 500-page bound, records all non-empty tokens, and errors on a repeated token. (internal/google/gog.go:50, 00550b632f6b)
  • Feature provenance: Blame attributes the original unbounded loop to the initial bootstrap implementation, which is also present in the v0.1.0 release. (internal/google/gog.go:48, ce439f2d7fb6)

Likely related people:

  • Peter Steinberger: Git blame attributes the current pagination loop to the initial Google importer implementation. (role: original feature contributor; confidence: high; commits: fc837601e11b; files: internal/google/gog.go)

Rank-up moves

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

  • Obtain maintainer acceptance of the 500-page availability boundary.

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 (4 earlier review cycles)
  • reviewed 2026-08-15T23:48:10.832Z sha 4a43b3b :: found issues before merge. :: [P1] Preserve valid imports beyond the hard page cap | [P3] Remove the release-owned changelog entry
  • reviewed 2026-08-16T00:17:36.420Z sha ab6cc51 :: needs changes before merge. :: [P2] Reject multi-token pagination cycles
  • reviewed 2026-08-16T00:21:48.273Z sha ab6cc51 :: needs changes before merge. :: [P2] Reject every previously seen page token
  • reviewed 2026-08-16T01:28:32.349Z sha 511abc5 :: needs maintainer review before merge. :: none

Keep hang protection for a repeating or incrementing pager, but
raise the safety ceiling to 500 pages so a valid 51-page listing
still completes. Drop the release-owned changelog entry.

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

Copy link
Copy Markdown
Contributor Author

@clawsweeper

[P1] Preserve valid imports beyond the hard page cap
A non-cycling listing that terminates on page 51 now returns an error here, while current main continues.

Pushed ab6cc51. maxContactsListPages is 500. TestGogAdapterListContactsCompletesAfterFiftyPages imports a listing that ends on page 51. An incrementing hang still errors at 500. Repeated-token reject is unchanged.

Also dropped the Unreleased changelog line (P3).

@clawsweeper re-review

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. and removed merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Aug 16, 2026
Adjacent-duplicate checks miss A to B to A cycles. Track every
non-empty continuation token and fail closed on the first repeat.

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

Copy link
Copy Markdown
Contributor Author

@clawsweeper

[P2] Reject every previously seen page token
This check catches only adjacent duplicates. A gog responder that returns A → B → A never has nextPage == page.

Pushed 511abc5. Every non-empty continuation token is recorded. TestGogAdapterListContactsRejectsAlternatingPageTokens fails closed on the third call.

@clawsweeper re-review

@clawsweeper clawsweeper Bot added 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. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Aug 16, 2026
@steipete
steipete merged commit f166ada into openclaw:main Aug 16, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P1 Urgent regression or broken agent/channel workflow affecting real users now. 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