fix: stop archived-thread and guild paging when the cursor does not advance

[SebTardif]

Guild and archived-thread sync can loop indefinitely when Discord repeats a pagination cursor. This stops repeated full guild pages, rejects missing guild cursors, and stops repeated archived-thread pages while preserving the existing best-effort catalog warning policy.

The maintainer update incorporates current main and the validated repair from triage/181-cursor-guards. The archived-thread regression uses the current cutoff API; sync documentation and the Unreleased changelog describe the behavior. Sebastien Tardif's original commits remain in this branch, with co-author credit on the maintainer repair.

Validation uses the built CLI, production Discord HTTP client, and disposable SQLite stores against synthetic loopback REST responses. Repeated guild and archive pages stop after two requests, a missing guild cursor fails after one, and a healthy guild sync succeeds. This is controlled HTTP integration proof; no live Discord incident is claimed. The final-head CI and proof results are recorded in the maintainer landing comment.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 29, 2026, 5:08 AM ET / 09:08 UTC.

ClawSweeper review

What this changes

The PR makes Discord guild and archived-thread pagination return an error when a full page repeats its cursor instead of continuing indefinitely.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 5 items remain

Keep open: the cursor guards address a current sync-hang risk, but the archived-thread regression test targets an obsolete client API and the supplied evidence is limited to a stubbed test run.

Priority: P2
Reviewed head: 62c46dd0c4a2d5b57423b9d73dbd167925340119

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The guard is a useful bounded repair, but the stale archived-thread test and mock-only proof prevent merge readiness.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: The changed production owner is the Discord client's guild and archived-thread pagers, but the supplied after-fix evidence is only a passing Go test with stubbed HTTP responses. Add a redacted trace from the production client and real transport boundary showing a repeated response, the second request, and the returned cursor error; redact tokens, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Patch quality	🦐 gold shrimp (3/6)	1 actionable review finding remain.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: The changed production owner is the Discord client's guild and archived-thread pagers, but the supplied after-fix evidence is only a passing Go test with stubbed HTTP responses. Add a redacted trace from the production client and real transport boundary showing a repeated response, the second request, and the returned cursor error; redact tokens, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed	5 items	Current archived-thread API: Current default-branch source requires an after time.Time argument for archived-thread paging; it retains the cutoff behavior introduced by the merged archived-thread discovery work. Introduced incompatible test call: The introduced archived-thread test calls the previous three-argument API, so it must be updated when rebased onto current main. Current behavior remains unguarded: Current main's guild pager still advances directly from the last ID, and its archived-thread pager lacks a non-advancing-cursor exit; the proposed guards therefore remain useful after adapting to the newer archived-thread API.
Findings	1 actionable finding	[P1] Pass the current archived-thread after argument
Security	None	None.

How this fits together

Discrawl's Discord client fetches guild and archived-thread catalogs page by page for sync. Those catalogs feed channel discovery and ultimately determine whether discrawl sync completes or stalls.

flowchart LR
A[Discord REST pages] --> B[Discrawl Discord client]
B --> C{Cursor advances?}
C -->|yes| D[Guild and thread catalog]
C -->|no| E[Sync returns cursor error]
D --> F[discrawl sync CLI]
E --> F

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The changed production owner is the Discord client's guild and archived-thread pagers, but the supplied after-fix evidence is only a passing Go test with stubbed HTTP responses. Add a redacted trace from the production client and real transport boundary showing a repeated response, the second request, and the returned cursor error; redact tokens, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Pass the current archived-thread `after` argument (P1) - Current main changed archived-thread paging to take after time.Time in fix(sync): discover newly archived threads #183, but this introduced test still invokes the old three-argument form. Update the call with time.Time{} when rebasing so the regression test compiles while preserving the cutoff behavior.
• Resolve merge risk (P1) - Rebasing without updating the introduced archived-thread test leaves it calling the obsolete three-argument client method, which will fail compilation against current main.
• Resolve merge risk (P1) - The supplied proof only establishes a stubbed test path; it does not show the production Discord client handling a repeated-page response and returning the cursor error.
• Complete next step (P2) - The fix is still valuable, but this branch needs a contributor rebase and real behavior evidence; automation cannot supply proof from the contributor's environment.

Findings

• [P1] Pass the current archived-thread after argument — internal/discord/client_test.go:435

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	production +11/-1; tests +124 across 2 files	The narrowly scoped guard is paired with focused pagination regressions, but one introduced test needs the current API signature.

Merge-risk options

Maintainer options:

1. Rebase and update the archive test (recommended)
   Retain the cursor guards on current main, pass time.Time{} to the archived-thread regression call, and show that the rebased package test compiles and passes.
2. Pause for a refreshed implementation
   Pause this branch if the guard cannot be reconciled with the current archived-thread cursor and cutoff contract without changing its intended behavior.

Technical review

Best possible solution:

Rebase the guards onto the current archived-thread cursor API, preserve its after cutoff, update the regression call, and add a redacted production-client transport trace that shows termination after a repeated cursor.

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

Yes, at source level: a repeated full guild page or has_more archived-thread page preserves the cursor in current main's pagers, giving a deterministic stubbed reproduction shape. No live current-main Discord trace was supplied.

Is this the best way to solve the issue?

No, not as currently rebased: the cursor guard is the narrow solution, but the archived-thread test must retain current main's after argument and cutoff contract.

Full review comments:

• [P1] Pass the current archived-thread after argument — internal/discord/client_test.go:435
  Current main changed archived-thread paging to take after time.Time in fix(sync): discover newly archived threads #183, but this introduced test still invokes the old three-argument form. Update the call with time.Time{} when rebasing so the regression test compiles while preserving the cutoff behavior.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 4c29bb75ef7f.

Labels

Label changes:

• add merge-risk: 🚨 automation: The introduced test calls an obsolete method signature and can fail the rebased branch's compile/test automation.
• remove merge-risk: 🚨 availability: Current PR review merge-risk labels are merge-risk: 🚨 automation.

Label justifications:

• P2: A repeated Discord pagination cursor can stall a sync, but the evidence supports a bounded client repair rather than an emergency incident.
• merge-risk: 🚨 automation: The introduced test calls an obsolete method signature and can fail the rebased branch's compile/test automation.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The changed production owner is the Discord client's guild and archived-thread pagers, but the supplied after-fix evidence is only a passing Go test with stubbed HTTP responses. Add a redacted trace from the production client and real transport boundary showing a repeated response, the second request, and the returned cursor error; redact tokens, endpoints, and other private details. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence

What I checked:

• Current archived-thread API: Current default-branch source requires an after time.Time argument for archived-thread paging; it retains the cutoff behavior introduced by the merged archived-thread discovery work. (internal/discord/client.go:343, 4c29bb75ef7f)
• Introduced incompatible test call: The introduced archived-thread test calls the previous three-argument API, so it must be updated when rebased onto current main. (internal/discord/client_test.go:435, 62c46dd0c4a2)
• Current behavior remains unguarded: Current main's guild pager still advances directly from the last ID, and its archived-thread pager lacks a non-advancing-cursor exit; the proposed guards therefore remain useful after adapting to the newer archived-thread API. (internal/discord/client.go:284, 4c29bb75ef7f)
• Archived-thread API provenance: Merged archived-thread discovery work introduced the current API shape after this branch was created. (internal/discord/client.go:343, 938b6b279dfc)
• Behavior proof supplied: The recorded PR-body evidence is passing go test output using stubbed HTTP pages; it does not include a production-client request/response trace showing the repeated cursor and resulting recovery.

Likely related people:

• obviyus: Authored the merged archived-thread discovery work that established the current after cursor API this PR must preserve. (role: merged archived-thread API contributor; confidence: high; commits: 938b6b279dfc; files: internal/discord/client.go, internal/syncer/channel_catalog.go)
• steipete: Git shortlog shows the largest contribution history on the Discord client, including recent surrounding client and sync work. (role: long-running Discord client contributor; confidence: high; commits: 3d066a520c55, 5371db1fa5d1; files: internal/discord/client.go)
• SebTardif: Previously contributed the merged guild-member cursor safeguards adjacent to this pagination behavior. (role: prior pagination-hardening contributor; confidence: medium; commits: b560fd127b34; files: internal/discord/client.go, internal/discord/client_test.go)

Rank-up moves

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

• Rebase onto current main and update the archived-thread regression call for the after time.Time API.
• Add redacted production-client transport evidence for the repeated-cursor error, then update the PR body for re-review or ask a maintainer to comment @clawsweeper re-review.

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 (7 earlier review cycles)

• reviewed 2026-08-18T22:01:51.821Z sha 62c46dd :: needs real behavior proof before merge. :: none
• reviewed 2026-08-21T19:07:47.086Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Update the archived-thread test for the current client API
• reviewed 2026-08-22T11:56:02.315Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Pass the current archived-thread cursor argument
• reviewed 2026-08-22T12:37:03.395Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Pass the archived-thread after argument
• reviewed 2026-08-23T07:05:10.068Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Update the archived-thread test for the current API
• reviewed 2026-08-24T03:15:30.178Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Pass the current archived-thread cursor argument
• reviewed 2026-08-25T01:15:52.095Z sha 62c46dd :: needs real behavior proof before merge. :: [P1] Pass the current archived-thread cursor argument

[steipete]

Maintainer triage: LAND recommended using the refreshed, credited implementation on triage/181-cursor-guards, commit 4f8c9c2. No merge performed.

The guard fixes a reproducible repeated-page loop. I ported the archived-thread regression to the current after time.Time API, preserving its cutoff behavior, and added sync documentation plus a changelog thanks to @SebTardif. The commit retains contributor credit.

Built-CLI proof used the real cmd/discrawl entrypoint and Discord HTTP client against a loopback REST fixture. Only the Discord base URL was changed at link time; all data was synthetic. This is controlled HTTP integration proof, not a claim that live Discord returned malformed pages.

GOWORK=off go build -ldflags '-X github.com/bwmarrin/discordgo.EndpointDiscord=http://127.0.0.1:38081/' -o /tmp/discrawl ./cmd/discrawl
python3 pagination-smoke.py /tmp/discrawl

Current main: repeated guild, missing guild ID, and repeated archive pages each reached the fixture's six-request safety cap.
Repaired build:
  repeated guild: exit 1, 2 requests, guild page cursor did not advance
  missing guild ID: exit 1, 1 request, guild page missing id
  repeated archive: 2 archive requests, cursor warning, overall exit 0
  healthy guild: exit 0, guilds=1

Archived-thread failures remain best-effort warnings under the existing catalog policy. GOWORK=off go test ./internal/discord -count=1 passed. Codex autoreview was clean, and an independent source-blind validator repeated all four CLI scenarios twice with no findings. The full-suite result will be included in the final handoff; existing PR CI is green but predates this refreshed maintainer branch.

[steipete]

Maintainer landing verification for a7c3fecc74f1a21928840ee5c4a41342af32b8c7:

The contributor branch now contains the repaired implementation as a new commit, preserving the original commits and adding co-author credit. Its final tree exactly matches the validated triage/181-cursor-guards tree. Codex autoreview returned no actionable findings, and GOWORK=off go test ./internal/discord -count=1 passed.

The built CLI was linked only to a synthetic loopback Discord REST fixture and exercised the production HTTP client and disposable SQLite stores:

guild-repeat: exit=1, 2 guild requests, cursor did not advance
guild-empty: exit=1, 1 guild request, missing id
thread-repeat: exit=0, 2 archive requests, cursor warning
healthy: exit=0, guilds=1

Archived-thread errors remain best-effort warnings. These are synthetic HTTP integration results, not evidence of a live Discord incident.

[clawsweeper]

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix: stop archived-thread and guild paging when the cursor does not advance This is item 1/1 in the current shard. Shard 0/1.

This placeholder means the worker is alive and reading the current context. I will edit this same comment with the actual review when the claws are done clicking.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.