fix: paginate usergroup grants to avoid rate limit cascades

[btipling]

Summary

• Paginates usergroup grant processing in batches of 20 members per page
• Previously fetched all members then called users.info per member in a single Grants call — large groups exhaust the Slack rate limit, and SDK retries restart from member 0
• Now returns a NextPageToken so completed batches are preserved across retries

Context

Resolves CXH-1301

Datadog logs show continuous 429s on /api/users.info with Retry-After: 4. The previous fix (v0.1.21) added rate limit annotations to SyncOpResults, but the fundamental issue was the N+1 unbatched loop — every retry re-fetched all members from scratch.

Test plan

• Run sync against a workspace with large usergroups (50+ members) and verify grants complete without cascading 429s
• Verify small usergroups (< 20 members) complete in a single page with no regression

🤖 Generated with Claude Code

[linear]

CXH-1301 Slack connector never works for Verkada tenant

[github-actions]

Claude finished @btipling's task in 1m 0s —— View job

---

PR Review: Paginate usergroup grants

• Gather context and read changed files
• Analyze changes and provide review feedback

---

Overall: Good direction — batching the N+1 users.info calls is the right fix for the rate-limit cascade. A few issues to address:

---

Bug: Potential panic if membership shrinks between pages

GetUserGroupMembersContext is called on every page (line 142). If members leave the group between page N and page N+1, the returned slice could be shorter than offset, causing a slice bounds out of range panic at line 161:

page := groupMembers[offset:end]  // panics if offset > len(groupMembers)

Add a bounds check after computing offset:

if offset >= len(groupMembers) {
    return nil, &resource.SyncOpResults{Annotations: outputAnnotations}, nil
}

Fix this →

Efficiency: Re-fetching the full member list on every page

GetUserGroupMembersContext (line 142) is called on every page, even though only a slice is used. For a 200-member group that's 10 identical API calls to list all members. This is a minor concern since it's a single lightweight call (vs. the N users.info calls being batched), but worth noting — if the Slack API supports cursor-based pagination for usergroup members, that would be cleaner. If not, this is an acceptable trade-off.

Nit: Error prefix

Per the repo's CLAUDE.md convention, errors should include a connector prefix (e.g. baton-slack:). The new error at line 153 uses "parsing page token: %w". That said, the existing code in this file also omits the prefix, so this is consistent with the file — just flagging for awareness.

---

Summary: The core pagination logic is sound and addresses the root cause well. The one must-fix is the bounds check to guard against membership changes between pages — without it, this can panic in production.

[github-actions]

Claude finished @btipling's task in 43s —— View job

---

PR Review: Paginate usergroup grants

• Gather context and read changed files
• Analyze changes and provide review feedback

---

Overall: This is a well-targeted fix. Batching the N+1 users.info calls into pages of 20 directly addresses the rate-limit cascade, and the implementation is clean. The previous review's bounds-check suggestion has been incorporated (line 157). A couple of remaining observations:

---

Re-fetching the full member list on every page

GetUserGroupMembersContext (line 142) is called on every page invocation. For a 200-member group, that's 10 identical calls to list all members just to slice a different window each time. This is a lightweight single API call compared to the per-member users.info calls being batched, so it's an acceptable trade-off for now — but worth noting as a future optimization opportunity if Slack adds cursor-based pagination for usergroup members.

Membership instability between pages

The bounds check at line 157 correctly prevents a panic when the group shrinks. However, if members are added or removed between pages, the offset-based windowing can silently skip members or emit duplicate grants (since the underlying list order may shift). This is inherent to offset-based pagination over a mutable list — it's not easily fixable without Slack providing a stable cursor. Just worth documenting as a known limitation. In practice, membership churn during a sync window is rare enough that this is fine.

Nit: Error prefix consistency

The new error at line 152 correctly uses the baton-slack: prefix per CLAUDE.md conventions — nice. The existing errors at lines 91 and 173 omit the prefix, but that's pre-existing and out of scope for this PR.

---

Verdict: The core logic is sound, the bounds check is in place, and this should resolve the cascading 429s for large usergroups. Looks good to merge. 👍

[github-actions]

Claude finished @btipling's task in 51s —— View job

---

PR Review: Paginate usergroup grants

• Gather context and read changed files
• Analyze changes and provide review feedback

---

Overall: Looks good to merge 👍

This is a clean, well-targeted fix. Batching the N+1 users.info calls into pages of 20 directly addresses the rate-limit cascade described in CXH-1301. Two commits, both solid.

---

Commit 1: helpers.go — Preserve gRPC error code in WrapError fallthrough

The new block at lines 92-96 correctly preserves an existing gRPC status code on errors that don't match the earlier errors.As branches (e.g. errors originating from uhttp/businessPlusClient). Previously these would all get codes.Unknown, losing retry-relevant codes like Unavailable. Good catch.

One subtlety: status.Code(err) returns codes.OK for non-gRPC errors (including nil), so the != codes.OK guard is correct — non-gRPC errors still fall through to codes.Unknown.

Commit 2: user_group.go — Paginate grants

The pagination logic is straightforward and correct:

• Page size of 20 (line 18) — reasonable for Slack's Tier 4 rate limits on users.info.
• Offset parsing (lines 147-154) — clean, with proper error handling and the baton-slack: prefix.
• Bounds check (lines 156-159) — correctly guards against membership shrinking between pages. This was flagged in the first CI review and addressed.
• Slicing (lines 161-163) — uses min() to handle the final partial page.
• Next page token (lines 180-185) — only set when there are more members, so pagination terminates correctly.

Known limitations (acceptable)

1. Re-fetching the full member list on every page — GetUserGroupMembersContext (line 142) is called on every Grants invocation. For a 200-member group, that's 10 identical calls. This is a single lightweight API call vs. the per-member users.info calls being batched, so the trade-off is acceptable. Slack's usergroup members API doesn't support cursor-based pagination, so there's no easy alternative.

2. Offset-based pagination over a mutable list — if members are added or removed between pages, the offset window can skip or duplicate members. This is inherent to the approach and not fixable without a stable cursor from Slack. In practice, membership churn during a sync window is rare enough that this is fine.

No blocking issues found. The code is clean, well-commented, and addresses the root cause.

---