fix: attach rate limit details to gRPC errors for proper SDK retry backoff

[btipling]

Summary

• Attaches RateLimitDescription to gRPC status details when Slack rate limits the connector, so the SDK's retry logic uses correct backoff timing instead of linear 1s/2s/3s defaults
• Handles both rate limit paths: HTTP 429 (uses exact Retry-After from header) and HTTP 200 ok:false with "ratelimited" (uses 30s default)
• No change to non-rate-limit error handling (permission, auth, not-found errors are unaffected)

Context

Resolves CXH-1301

The connector was hitting cascading 429s during usergroup grant listing because WrapError mapped rate limit errors to codes.Unavailable (triggering SDK retry) but didn't embed the Retry-After timing in the gRPC status details. The SDK's retry loop extracts RateLimitDescription from status details to determine wait time — without it, it guesses with linear backoff starting at 1s, which is too aggressive.

Test plan

• Run a sync against a workspace with usergroups that have many members to confirm rate limit retries respect proper backoff
• Verify non-rate-limit errors (permission denied, auth failures) are unaffected

🤖 Generated with Claude Code

[github-actions]

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

---

PR Review

• Read the changed file and PR diff
• Review code for correctness, bugs, and best practices
• Post review feedback

---

Overall: Good change that addresses a real problem — the SDK's retry loop at vendor/.../retry/retry.go:87-111 does extract RateLimitDescription from gRPC status details, and without it the linear backoff (1s, 2s, 3s...) is too aggressive for Slack rate limits. The implementation follows the same errors.Join(st.Err(), originalErr) pattern used by the SDK's own WrapErrorsWithRateLimitInfo.

Bug: Overly broad codes.Unavailable check

The check at pkg/connector/client/helpers.go:80:

if grpcCode == codes.Unavailable {
    return wrapErrorWithRateLimitDetails(grpcCode, contextMsg, defaultRateLimitRetryAfter, err)
}

This catches all SlackErrorResponse errors that map to codes.Unavailable, not just rate limits. Looking at MapSlackErrorToGRPCCode, these also map to Unavailable:

• "no_such_subteam" (line 145) — transient subteam error, not a rate limit
• "internal_error", "service_unavailable", "request_timeout" (line 163) — server errors, not rate limits

For these non-rate-limit errors, the SDK would wait 30s before retrying (because RateLimitDescription with Remaining: 0 and ResetAt: now+30s forces a full wait), instead of the normal linear backoff (1s, 2s, 3s...). This makes retries for transient server errors unnecessarily slow.

Suggested fix — check the Slack error string directly instead of the mapped gRPC code:

if containsAny(strings.ToLower(slackErrResp.Err), "ratelimited") {
    return wrapErrorWithRateLimitDetails(grpcCode, contextMsg, defaultRateLimitRetryAfter, err)
}

Fix this →

Minor: Silent error discard on WithDetails

st, _ = st.WithDetails(rlDesc)

This matches the SDK's own pattern in WrapErrorsWithRateLimitInfo (wrapper.go:412), so it's consistent. Just noting that if WithDetails ever fails, the error silently loses its rate limit data and falls back to linear backoff. A debug log could help diagnose issues, but this is low priority.

Looks good

• The HTTP 429 path (slack.RateLimitedError) correctly uses the actual RetryAfter from the Slack header
• The 30s default for HTTP 200 rate limits is reasonable (Slack's Tier 1 rate limit window)
• Non-rate-limit error paths are untouched (aside from the bug above)
• The function signature and structure are clean

[linear]

CXH-1301 Slack connector never works for Verkada tenant

[github-actions]

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

---

PR Review

• Read PR diff and changed files
• Analyze code changes for correctness and issues
• Post review feedback

---

Overall: Good change that solves a real problem — without RateLimitDescription in SyncOpResults, the SDK's retry loop falls back to aggressive linear backoff (1s, 2s, 3s...) which causes cascading 429s. The approach of threading *annotations.Annotations through WrapError is clean and the two rate limit paths (HTTP 429 vs HTTP 200 ok:false) are handled correctly.

Bug: Overly broad codes.Unavailable check

The previous review flagged this and it's still present. At pkg/connector/client/helpers.go:85:

if grpcCode == codes.Unavailable && annos != nil {
    annos.WithRateLimiting(rateLimitDescription(defaultRateLimitRetryAfter))
}

This applies rate limit annotations to all SlackErrorResponse errors that map to codes.Unavailable, not just "ratelimited". Looking at MapSlackErrorToGRPCCode, these also map to Unavailable:

• "no_such_subteam" (line 150) — transient error, not a rate limit
• "internal_error", "service_unavailable", "request_timeout" (line 168) — server errors, not rate limits

For these errors, the SDK would wait 30s before retrying (because RateLimitDescription with Remaining: 0 and ResetAt: now+30s forces a full wait), instead of the normal linear backoff. This makes retries for transient server errors unnecessarily slow.

Suggested fix — check the Slack error string directly:

if containsAny(strings.ToLower(slackErrResp.Err), "ratelimited") && annos != nil {
    annos.WithRateLimiting(rateLimitDescription(defaultRateLimitRetryAfter))
}

Fix this →

Minor: Inconsistent annos passing in workspace.go Grants

In pkg/connector/workspace.go, the business+ path (lines 162-167) already has its own ratelimitData from the SDK pagination client, so nil is passed to WrapError — that's fine. But the standard API path at line 182 now passes &outputAnnotations where outputAnnotations was initialized via annotations.New() on line 155 (inside the business+ branch). If the code takes the standard API path, outputAnnotations would be uninitialized (zero-value nil).

Looking more carefully, the if/else structure means:

• Business+ path: outputAnnotations = annotations.New() on line 155, WrapError(..., nil) — correct
• Standard path: outputAnnotations is zero-value, WrapError(..., &outputAnnotations) — this works because &outputAnnotations is a valid pointer to nil annotations, and WithRateLimiting will initialize it. So this is fine functionally, just slightly asymmetric.

Looks good

• HTTP 429 path (slack.RateLimitedError) correctly uses the actual RetryAfter from the Slack header
• 30s default for HTTP 200 rate limits is reasonable (Slack Tier 1 window)
• rateLimitDescription helper is clean and correctly sets STATUS_OVERLIMIT with Remaining: 0
• All callers that return SyncOpResults on error paths now include annotations — this is the key fix
• Non-rate-limit callers (e.g., Validate) correctly pass nil for annos
• Success paths also return annotations (e.g., user.go:222, user_group.go:159) so non-error rate limit info is preserved