fix(contacts): reject repeated dedupe page tokens

[SebTardif]

What Problem This Solves

gog contacts dedupe lists personal connections with pageToken = resp.NextPageToken and no seen-set. The public default is --max 0, which means scan every page. If Google repeats nextPageToken, dedupe never finishes and the CLI hangs until the process is killed.

Why

The repo already has this guard. collectAllPages errors with pagination loop: repeated page token. Calendar listing uses it after #1004. People raw email resolve uses it in #1044. The Gmail --from-contact fallback uses it in #1045. Contacts export group listing uses it in #1046. Dedupe listing lives in the same cmd package, so it now calls that helper instead of growing a second loop. --max still stops the scan early by returning an empty next token once the requested count is reached.

User Impact

A stuck Connections page token now fails with repeated page token instead of hanging gog contacts dedupe on the default all-pages scan.

Evidence

terminal output from the unpatched connections listing loop versus this patch. A People httptest server always returns nextPageToken=stuck from people.me.connections.list. The public command is gog contacts dedupe with default --max 0.

Unpatched (the HTTP peer answers immediately; the handler returns HTTP 400 after a third list call so the loop cannot run forever):

$ go test ./internal/cmd -run TestContactsDedupeExecuteRejectsRepeatedPageToken -count=1 -timeout 60s -v
=== RUN   TestContactsDedupeExecuteRejectsRepeatedPageToken
    contacts_dedupe_test.go:195: err = googleapi: got HTTP response code 400 with body: unexpected extra connections page request
         after 3 list calls
--- FAIL: TestContactsDedupeExecuteRejectsRepeatedPageToken (0.06s)
FAIL

Patched (same command, same stuck token, returns immediately):

$ go test ./internal/cmd -run TestContactsDedupeExecuteRejectsRepeatedPageToken -count=1 -timeout 60s -v
=== RUN   TestContactsDedupeExecuteRejectsRepeatedPageToken
    contacts_dedupe_test.go:203: err = pagination loop: repeated page token "stuck" after 2 list calls
--- PASS: TestContactsDedupeExecuteRejectsRepeatedPageToken (0.07s)
PASS
ok  	github.com/openclaw/gogcli/internal/cmd	2.522s

Live CLI still exposes the default all-pages scan:

$ bin/gog contacts dedupe --help
Usage: gog contacts (contact) dedupe [flags]
      --match="email,phone"      Match fields: email,phone,name
      --max=0                    Max contacts to scan (0 = all)

go test ./internal/cmd -count=1 -run TestContactsDedupe completed on this tree. gofmt is clean. golangci-lint run ./internal/cmd reported 0 issues.

Real behavior proof

• Behavior or issue addressed: Repeated Google nextPageToken values could hang gog contacts dedupe while listing people.me.connections on the default --max 0 scan.
• Real environment tested: Windows amd64, Go go1.27.1, worktree at C:\Users\sebta\.grok\tmp\pr-gate-batch\gogcli-f020 on fix/contacts-dedupe-page-token from origin/main at 25703c78.
• Exact steps or command run after this patch: From that worktree, ran go test ./internal/cmd -run TestContactsDedupeExecuteRejectsRepeatedPageToken -count=1 -timeout 60s -v against a People httptest that always returns nextPageToken=stuck from people.me.connections.list, then ran bin/gog contacts dedupe --help on the binary built from this branch.
• Evidence after fix: terminal output above. Connections listing now returns pagination loop: repeated page token "stuck" after 2 list calls (0.07s). The unpatched loop made 3 list calls and only stopped when the handler returned HTTP 400.
• Observed result after fix: The stuck token no longer pages forever. contactsDedupeList returns the repeated-token error on the next page instead of hanging. gog contacts dedupe --help still shows --max=0 as scan all pages.
• What was not tested: A live people.connections.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 collectAllPages in internal/cmd/paging.go, by calendar listing in #1004, by People raw email resolve in #1044, by the Gmail --from-contact fallback in #1045, and by contacts export group listing in #1046.
• Unguarded Connections paging dates to 62a7257ab (feat(contacts): add dedupe preview, #555, 2026-05-05, 123 days ago).

[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, 3:45 AM ET / 07:45 UTC.

ClawSweeper review

What this changes

The PR routes contact dedupe pagination through the existing repeated-token guard and adds a command-level regression test.

Merge readiness

⛔ Needs changes before merge - 2 items remain

This remains necessary: current main and v0.39.0 retain the unguarded dedupe loop, while the cited merged fixes cover separate callers. The implementation and recorded fault-injection proof support keeping this PR as the landing candidate.

Priority: P2
Reviewed head: 954989980bd619b54af43db7b1dfe9d04e214c85

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused, correct repair with convincing fault-injection proof and one small release-note omission.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The captured Windows amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.
Patch quality	🐚 platinum hermit (4/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured Windows amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.
Evidence reviewed	9 items	Verified introduced change: The pinned base-to-head diff changes only the dedupe paginator and its regression test: production +27/-19 lines and tests +38/-0 lines. Current main still needs the fix: The default scan is unlimited, and contactsDedupeList repeatedly assigns NextPageToken without remembering previously requested tokens. Successful responses carrying a repeated token can therefore continue indefinitely. Latest release retains the same loop: The v0.39.0 source still contains the unguarded dedupe pagination loop; the tag resolves to the supplied release commit.
Findings	1 actionable finding	[P3] [P3] Record the dedupe hang fix in Unreleased
Security	None	None.

How this fits together

Contact dedupe reads personal contacts from the Google People API and groups likely duplicates for preview or confirmed merging. Pagination must finish successfully before the command emits results or starts contact updates.

flowchart TD
  A[Contact dedupe command] --> B[Fetch People API page]
  B --> C[Collect non-null contacts]
  C --> D{Scan limit or final page?}
  D -->|Yes| E[Build duplicate groups]
  D -->|No| F{Page token already seen?}
  F -->|No| B
  F -->|Yes| G[Return error before output or updates]
  E --> H[Preview or confirmed merge]

Loading

Before merge

• [P3] Record the dedupe hang fix in Unreleased (P3) - This introduces a user-visible failure-mode fix, but the branch has no changelog entry. The repository's AGENTS.md requires fixes to be recorded in Unreleased with a PR reference and contributor thanks. Add the entry to the existing 0.39.1 - Unreleased section so this correction is included in the next release notes.
• Complete next step (P2) - Add the dedupe pagination fix to CHANGELOG.md under 0.39.1 - Unreleased, with the PR reference and thanks to @SebTardif.

Findings

• [P3] [P3] Record the dedupe hang fix in Unreleased — internal/cmd/contacts_dedupe.go:189

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +8 net lines; tests +38 lines	The small production increase adapts the existing paginator while retaining scan limits, and the added test exercises the command through HTTP.

Technical review

Best possible solution:

Dedupe should terminate malformed pagination before producing a plan or applying changes while preserving bounded scans and normal multi-page results.

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

Yes: consecutive Connections responses carrying nextPageToken=stuck exercise the unguarded default scan on current main. Source inspection and the supplied before/after command transcript establish the mechanism; this review did not execute it.

Is this the best way to solve the issue?

Yes: reusing collectAllPages is the narrowest established repair, and the adapter preserves non-null contact counting and early completion at --max.

Full review comments:

• [P3] [P3] Record the dedupe hang fix in Unreleased — internal/cmd/contacts_dedupe.go:189
  This introduces a user-visible failure-mode fix, but the branch has no changelog entry. The repository's AGENTS.md requires fixes to be recorded in Unreleased with a PR reference and contributor thanks. Add the entry to the existing 0.39.1 - Unreleased section so this correction is included in the next release notes.
  Confidence: 0.99

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 fixes a source-proven command hang under malformed upstream pagination with a limited affected surface.
• add proof: sufficient: Contributor real behavior proof is sufficient. The captured Windows amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster 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 amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.

Label justifications:

• P2: This fixes a source-proven command hang under malformed upstream pagination with a limited affected surface.
• 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 captured Windows amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured Windows amd64 transcript exercises the dedupe command and changed paginator through the generated People client over real local HTTP. Injected repeated tokens produce the intended error after two requests with empty stdout; the unpatched path reaches a third-request sentinel. This satisfies production-path reliability proof, with help output only supplemental.

Evidence

Acceptance criteria:

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

What I checked:

• Verified introduced change: The pinned base-to-head diff changes only the dedupe paginator and its regression test: production +27/-19 lines and tests +38/-0 lines. (internal/cmd/contacts_dedupe.go:150, 954989980bd6)
• Current main still needs the fix: The default scan is unlimited, and contactsDedupeList repeatedly assigns NextPageToken without remembering previously requested tokens. Successful responses carrying a repeated token can therefore continue indefinitely. (internal/cmd/contacts_dedupe.go:150, 25703c789da5)
• Latest release retains the same loop: The v0.39.0 source still contains the unguarded dedupe pagination loop; the tag resolves to the supplied release commit. (internal/cmd/contacts_dedupe.go:150, b058dbbaeb44)
• Existing shared pagination contract: collectAllPages detects repeated tokens before issuing another fetch and discards accumulated results on errors. Its existing tests cover distinct pages, repeated tokens, longer cycles, later-page errors, and the page ceiling. (internal/cmd/paging.go:19, 954989980bd6)
• Recorded production-path fault proof: The captured PR body reports a Windows amd64 run through the dedupe command: the unpatched implementation reaches the third-request HTTP 400 sentinel, whereas the patch returns the repeated-token error after exactly two requests. The passing regression also asserts empty stdout. These are contributor-recorded results; this read-only review did not rerun tests. (internal/cmd/contacts_dedupe_test.go:169, 954989980bd6)
• Proof uses a real transport client: newPeopleService constructs the generated People client with an actual httptest HTTP server and its HTTP client. The command harness injects that service into the normal runtime and invokes executeWithRuntime, exercising the changed production paginator. (internal/cmd/contacts_crud_error_test.go:19, 954989980bd6)

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 required 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 contact-dedupe cycle protection while preserving early completion at --max. Repeated tokens stop before a partial plan or contact changes. Thanks @SebTardif. Superseded by #1087, which incorporates recurring-instance cycle protection while preserving immediate success when the requested occurrence is found. Failed lookup performs no event deletion. Thanks @SebTardif. 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.