fix(api): stop Slack pagination on a repeated cursor

[SebTardif]

What Problem This Solves

Fixes an issue where users running slacrawl sync (or any path that pages Slack conversations, channel history, thread replies, or DMs) would hang until the process was killed when Slack kept returning the same next_cursor. Channel listing, history backfill, thread replies, and DM listing all followed that token with no stop condition.

Why This Change Was Made

A stuck Slack cursor is a hang, not a slow page. MCP tools/list in this repo already rejects a repeated cursor. The Slack HTTP loops did not. This copies that fail-closed check onto conversations.list, conversations.history, conversations.replies, and DM listing. Same class as openclaw/notcrawl#102 and openclaw/discrawl#181.

The unguarded loops date to bootstrap PR #1 (df45322) on 2026-03-07. DM listing added the same pattern in PR #12 (ffd4ad6) on 2026-04-23.

User Impact

A wedged Slack pager now stops with a clear repeated cursor error instead of spinning until someone kills the sync. Successful pages (empty cursor, or a new cursor) are unchanged.

Evidence

terminal output from the patched Slack client. A conversations.list pager that always returns next_cursor=stuck now fails closed after two pages.

Before this patch the same pager never returned. A 300ms deadline expired with:

$ GOWORK=off go test ./internal/slackapi/ -count=1 -timeout 15s -run TestFetchChannelsRejectsRepeatedCursor
Error: Error "Post \"http://127.0.0.1:65193/conversations.list\": context deadline exceeded" does not contain "conversations.list repeated cursor \"stuck\""
FAIL	github.com/openclaw/slacrawl/internal/slackapi	4.677s

After this patch (6fc35a7346538fec822532a0e381e43ec3d933d8) the same pager returns immediately:

$ GOWORK=off go test ./internal/slackapi/ -count=1 -timeout 15s -run TestPrintRepeatedCursorError -v
fetchChannels stuck next_cursor: conversations.list repeated cursor "stuck"
ok  	github.com/openclaw/slacrawl/internal/slackapi	0.261s

Channel history, thread replies, and DM listing use the same stop. Those four paths return in about 0.00s to 0.01s instead of waiting out the deadline:

$ GOWORK=off go test ./internal/slackapi/ -count=1 -v -timeout 15s -run 'TestFetchChannelsRejectsRepeatedCursor|TestSyncChannelHistoryRejectsRepeatedCursor|TestSyncThreadRejectsRepeatedCursor|TestFetchDMsRejectsRepeatedCursor'
=== RUN   TestFetchChannelsRejectsRepeatedCursor
--- PASS: TestFetchChannelsRejectsRepeatedCursor (0.00s)
=== RUN   TestSyncChannelHistoryRejectsRepeatedCursor
--- PASS: TestSyncChannelHistoryRejectsRepeatedCursor (0.01s)
=== RUN   TestSyncThreadRejectsRepeatedCursor
--- PASS: TestSyncThreadRejectsRepeatedCursor (0.00s)
=== RUN   TestFetchDMsRejectsRepeatedCursor
--- PASS: TestFetchDMsRejectsRepeatedCursor (0.00s)
ok  	github.com/openclaw/slacrawl/internal/slackapi	0.299s

Real behavior proof

• Behavior or issue addressed: Slack conversation, history, thread, and DM pagers followed a repeated next_cursor forever, so slacrawl sync never returned.

• Real environment tested: macOS Darwin arm64, Go 1.26.6, full clone of openclaw/slacrawl at /tmp/pr-slacrawl on fix/slack-repeated-cursor (6fc35a7346538fec822532a0e381e43ec3d933d8).

• Exact steps or command run after this patch:

  cd /tmp/pr-slacrawl
  GOWORK=off go test ./internal/slackapi/ -count=1 -timeout 15s -run TestPrintRepeatedCursorError -v
  GOWORK=off go test ./internal/slackapi/ ./internal/mcpclient/ -count=1

• Evidence after fix: terminal output from the patched Slack client. The stuck conversations.list pager now prints conversations.list repeated cursor "stuck" and returns in 0.00s instead of hanging until the 300ms deadline:

  fetchChannels stuck next_cursor: conversations.list repeated cursor "stuck"
  ok  	github.com/openclaw/slacrawl/internal/slackapi	0.261s

• Observed result after fix: After two pages with the same next_cursor=stuck, fetchChannels returns conversations.list repeated cursor "stuck" before the 300ms deadline. History, replies, and DM listing do the same for their Slack methods.

• What was not tested: A live Slack workspace that actually repeats next_cursor. slack-go GetUsersContext pagination inside the library was left unchanged.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

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

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 29, 2026, 3:57 AM ET / 07:57 UTC.

ClawSweeper review

What this changes

The PR stops Slack channel, history, thread-reply, and DM pagination when a cursor repeats, with four regression tests.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

Keep this PR open: current main still lacks repeated-cursor guards, while the introduced implementation is a focused, correct fix with production-client HTTP regression coverage.

Priority: P2
Reviewed head: 6fc35a7346538fec822532a0e381e43ec3d933d8

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused implementation and four direct production-client regression tests provide normal merge-ready evidence after a base refresh.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.
Evidence reviewed	5 items	Introduced cursor guards: The PR records returned cursors in each affected Slack pager and returns a method-specific error before issuing a third request for the same cursor. Current main still needs the change: The current default-branch revision has none of the 123 introduced additions; its Slack pagination loops advance the cursor without a repeated-cursor stop condition. Behavior proof: The supplied terminal trace runs all four checked-in repeated-cursor tests. Those tests use the production Slack client configured with a real local HTTP transport, inject a repeated API cursor, require the error, and require exactly two requests.
Findings	None	None.
Security	None	None.

How this fits together

Slacrawl’s Slack sync client pages conversations and messages before storing them in the local archive. Cursor handling determines whether a sync proceeds through pages or terminates with an error.

flowchart LR
  A[Slack API pages] --> B[Slack sync client]
  B --> C[Pagination cursor]
  C --> D{New or repeated?}
  D -->|New| E[Next Slack request]
  D -->|Repeated| F[Return sync error]
  E --> G[Local archive]

Loading

Before merge

• Resolve merge risk (P1) - The branch is behind current main; refresh the merge review after rebasing onto dcbc12d. GitHub reports it mergeable, so this is not a demonstrated semantic conflict.
• Complete next step (P2) - No mechanical repair is needed; a normal rebase and maintainer review of the refreshed head are the remaining steps.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and regression coverage	production +21, tests +102, 4 files affected	The small production change is paired with direct coverage of all four affected pagination paths.

Merge-risk options

Maintainer options:

1. Decide the mitigation before merge
   Rebase onto current main and land the four guards with their focused regression coverage, retaining the established MCP-style repeated-cursor error behavior.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Technical review

Best possible solution:

Rebase onto current main and land the four guards with their focused regression coverage, retaining the established MCP-style repeated-cursor error behavior.

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

Yes—source establishes a deterministic reproduction: return the same non-empty Slack cursor twice through the production client, as each of the four checked-in HTTP tests does.

Is this the best way to solve the issue?

Yes—the patch mirrors the repository’s existing MCP cursor-cycle guard and preserves normal empty and newly advancing cursor behavior.

AGENTS.md: not found in the target repository.

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

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: needs maintainer proof decision: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

• P2: This prevents a Slack sync from hanging on a malformed repeated cursor, with a bounded and localized user impact.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.
• proof: sufficient: Contributor real behavior proof is sufficient. The changed production owner is the Slack API client’s four pagination loops; the supplied exact-head terminal run exercises each through a real local HTTP transport with an injected repeated cursor and observes the expected error after two requests.

Evidence

What I checked:

• Introduced cursor guards: The PR records returned cursors in each affected Slack pager and returns a method-specific error before issuing a third request for the same cursor. (internal/slackapi/api.go:443, 6fc35a734653)
• Current main still needs the change: The current default-branch revision has none of the 123 introduced additions; its Slack pagination loops advance the cursor without a repeated-cursor stop condition. (internal/slackapi/api.go:422, dcbc12d44837)
• Behavior proof: The supplied terminal trace runs all four checked-in repeated-cursor tests. Those tests use the production Slack client configured with a real local HTTP transport, inject a repeated API cursor, require the error, and require exactly two requests. (internal/slackapi/api_test.go:1853, 6fc35a734653)
• Existing repository pattern: The MCP client already uses the same seen-cursor pattern for paginated tools/list calls, so this follows an established local failure mode rather than adding a new policy. (internal/mcpclient/client.go:133, 6fc35a734653)
• Feature history: History identifies Vincent Koc’s bootstrap as the origin of the API pagination surface, Matt Van Horn’s merged DM sync as the origin of the DM pager, and Peter Steinberger as the most recent main-branch contributor to the affected API sync path. (internal/slackapi/api.go:422, b407a906004b)

Likely related people:

• Peter Steinberger: Most recently changed the API sync path’s per-page processing on main. (role: recent area contributor; confidence: high; commits: b407a906004b; files: internal/slackapi/api.go)
• Vincent Koc: Introduced the bootstrap Slack API pagination surface that contains the channel/history/thread loops. (role: original API sync author; confidence: high; commits: df45322db90e; files: internal/slackapi/api.go)
• Matt Van Horn: Introduced the separately paginated DM and MPIM sync path. (role: DM sync feature author; confidence: high; commits: ffd4ad687b6c; files: internal/slackapi/dms.go)

Rank-up moves

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

• Rebase onto current main and refresh the targeted test results for the resulting head.

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 (19 earlier review cycles; latest 8 shown)

• reviewed 2026-08-23T13:10:57.965Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-23T16:58:51.101Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-23T20:57:50.150Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-24T12:00:45.364Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-24T22:58:20.766Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-25T11:04:56.372Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-25T19:12:58.342Z sha 6fc35a7 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-28T12:43:16.437Z sha 6fc35a7 :: needs real behavior proof before merge. :: none