← Back to PRs

#5312: fix(slack): skip users.list API call when all user entries are IDs

by gunpyo-park-musinsa open 2026-01-31 09:50 View on GitHub →
channel: slack
## Problem Same issue as #5305, but for user allowlists instead of channels. When Slack users are configured using user IDs (e.g., `U07SQFJCAU8`) instead of names (e.g., `@john`), the `resolveSlackUserAllowlist` function still calls `users.list` API to fetch all users. This causes: - **Unnecessary API calls** - User IDs don't need resolution - **Rate limit issues** - Repeated calls trigger Slack's rate limits (30s retry loops) - **Slow startup** - Fetching user list delays provider initialization ## Solution Pre-parse all entries before fetching the user list. Only call `users.list` when at least one entry requires name or email-based resolution. ```typescript const needsUserList = parsedEntries.some( ({ parsed }) => !parsed.id && (parsed.name || parsed.email) ); const users = needsUserList ? await listSlackUsers(client) : []; ``` ## Testing Added 6 test cases: - ✅ Skips API call when all entries are user IDs - ✅ Calls API when entries contain a mix of IDs and names - ✅ Handles Slack mention format (`<@U123>`) without API call - ✅ Calls API when entry is an email - ✅ Resolves by name and prefers non-deleted users - ✅ Keeps unresolved entries All existing tests pass. ## Impact - **Zero breaking changes** - **Significant performance improvement** for ID-based configurations - **Eliminates rate limit issues** when using user IDs ## Related - Follows same pattern as #5305 (channel optimization) <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR applies the same optimization recently added for Slack channel allowlists to Slack user allowlists: it pre-parses configured entries and only calls `users.list` / `conversations.list` when at least one entry requires name/email-based resolution. Tests were expanded to cover ID-only entries, mixed ID+name cases, Slack mention formats, and email resolution. The change fits the existing Slack allowlist resolution pattern in `src/slack/resolve-channels.ts` and `src/slack/resolve-users.ts`, reducing startup latency and avoiding Slack API rate limiting when configurations are already in canonical ID form. <h3>Confidence Score: 4/5</h3> - This PR is safe to merge with low risk and clear behavior change coverage via tests. - The core logic change is small and well-contained (conditional Slack list calls based on input parsing), and new tests cover the key scenarios (IDs, mixed inputs, mentions, emails). The main concern is a small redundancy in `resolveSlackUserAllowlist` where entries are parsed twice, which is easy to fix but unlikely to cause functional regressions. - src/slack/resolve-users.ts <!-- greptile_other_comments_section --> <sub>(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!</sub> **Context used:** - Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=fd949e91-5c3a-4ab5-90a1-cbe184fd6ce8)) - Context from `dashboard` - AGENTS.md ([source](https://app.greptile.com/review/custom-context?memory=0d0c8278-ef8e-4d6c-ab21-f5527e322f13)) <!-- /greptile_comment -->

Most Similar PRs