[sc-30416]: Cache Slack user details by GitHub user login to mitigate API rate limiting #167

jakeschurch · 2025-08-21T15:36:21Z

Description

This PR introduces robust caching and mapping of Slack user details by GitHub login, significantly reducing reliance on Slack and GitHub API calls and mitigating rate limiting issues. It incorporates extensive requested changes and feedback from review:

Mapping Support & Caching
- Add support for providing a GitHub-to-Slack user mapping via environment variable (SLACK_DEPLOY_GITHUB_USERS, formerly SLACK_GITHUB_MAPPING_RAW) or file.
- Cache Slack user details by GitHub login, skipping API calls when mapping is available.
- Skip adding users to cache if errors are thrown or if users are deactivated/deleted.
Action & Workflow Refactor
- Split notify and generate-mapping actions into distinct entrypoints.
- Rename default action workflow to notify in dist output.
- Introduce .github/actions/generate-mapping/action.yml for generating a JSON mapping of Slack users by GitHub login.
- Refactor main entry to pass mapping input for author resolution.
Code Quality & Feedback
- Unify mapping shape validation and add type guard tests.
- Extract Slack user lookup logic to reusable helper function.
- Reference existing MessageAuthor type for mapping output.
- Improve code readability, error handling, and caching logic.
- Use getEnv for environment variable access.
- Rename variables and inputs for clarity and consistency.
- Remove redundant error logging and unnecessary print statements.
- Address parallel API call concerns and clarify rate limiting handling.
Documentation
- Update README to document new environment variable, mapping usage, and benefits.
- Add examples and clarify usage for custom mapping.

Testing

Fieldguide/infrastructure/actions/runs/17133941858/job/48605070955?pr=1045
#181: Cache Slack users.list API response as artifact for deploy pipeline notifications
#1045: HOLD: test branch
Slack channel for testing

Reviewer Notes

All requested changes from @namoscato have been addressed, including variable naming, error handling, type validation, and code extraction.
No merge conflicts; all checks have passed.

NOTES: - Introduce `.github/actions/generate-mapping/action.yml` for generating a JSON mapping of Slack users by GitHub login. - Add `src/githubToSlackMapping.ts` to fetch GitHub org users, match them to Slack users, and output mapping. - Refactor `src/main.ts` to support `notify` and `generate-mapping` modes via `__mode` input. - Update `action.yml` to include `__mode` input for internal mode selection. Signed-off-by: Jake Schurch <[email protected]>

…er by GitHub login - Introduce `user_mapping_filepath` input in `action.yml` to optionally provide a mapping file for Slack user details by GitHub login. - Update `getMessageAuthorFactory` and related logic to use the mapping file if provided, avoiding Slack and GitHub API calls when possible. - Add fallback and error handling for mapping file usage. - Refactor main entry to pass `user_mapping_filepath` to author resolution. Signed-off-by: Jake Schurch <[email protected]>

Signed-off-by: Jake Schurch <[email protected]>

- Separate notify and generateMapping logic into distinct entrypoints (`src/notify.ts` and new `src/generateMapping.ts`) - Remove internal `__mode` input from action.yml files - Update action.yml and generate-mapping/action.yml to reference new dist entrypoints - Refactor package.json exports and scripts for new structure - Improve maintainability and clarity by decoupling action responsibilities refs: [sc-30416] Signed-off-by: Jake Schurch <[email protected]>

…g to file - Change output from file path to raw JSON string in GitHub Action output (`raw_mapping_json`) - Update `generateGithubToSlackMapping` to return JSON string instead of writing to disk - Remove file system operations from mapping generation logic - Update action input/output and usage to reflect new output format Signed-off-by: Jake Schurch <[email protected]>

…Slack user mapping - Add details to README about the optional SLACK_GITHUB_MAPPING_RAW environment variable, allowing users to provide raw JSON or YAML mappings of GitHub logins to Slack user details. - Explains usage, example formats, and benefits of avoiding API calls for user mapping. refs: sc-30416 Signed-off-by: Jake Schurch <[email protected]>

Signed-off-by: Jake Schurch <[email protected]>

namoscato

Thanks for your work on this, @jakeschurch – this is looking great!

I might take a pass through the README once the dust settles.

src/notify.ts

src/getMessageAuthorFactory.ts

src/notify.ts

src/getMessageAuthorFactory.ts

src/githubToSlackMapping.ts

… guard tests - Refactor mapping extraction to use `githubUserMapping` instead of `rawMapping` - Validate mapping shape using new `isMessageAuthor` type guard - Extract and use `isMemberWithProfile` and `isMessageAuthor` type guards across codebase - Add tests for type guards in `src/slack/__tests__/types.test.ts` - Update environment variable to `SLACK_DEPLOY_GITHUB_USERS` - Remove redundant debug logging and simplify Slack user matching logic - Update README refs: [sc-30416] Signed-off-by: Jake Schurch <[email protected]>

Signed-off-by: Jake Schurch <[email protected]>

…gic to helper - Move Slack user lookup by GitHub name to new `getSlackUserFromGithubName` function for reuse and clarity. - Replace inline Slack user matching logic in `getMessageAuthorFactory` and `generateGithubToSlackMapping` with the new helper. - Update `isMemberWithProfile` type guard to remove `real_name` null check, relying only on `display_name` and `image_48` presence. - Minor option default value change in `getMessageAuthorFactory` (`githubUserMapping: undefined`). Signed-off-by: Jake Schurch <[email protected]>

Signed-off-by: Jake Schurch <[email protected]>

- move to separate module - rename from getSlackUserFromGithubName - filter MemberWithProfile in SlackClient

- remove unnecessary reduce - surface empty set short circuits; move to warning - preserve not found `null` values - fix GitHubUser type

namoscato

This is close! I pushed up a few commits and have a few more comments before I do some manual testing on my end. I'm happy to help with some of the loose ends – just let me know.

src/getMessageAuthorFactory.ts

namoscato · 2025-09-27T14:30:42Z

src/generateMapping.ts

+  }
+}
+
+async function generateMapping(


@jakeschurch, would you be open to making this name more specific? I'd be happy to make the change, if so.

namoscato · 2025-09-27T18:19:39Z

src/generateUserMapping.ts

+    channel,
+    errorReaction


@jakeschurch, any objections to making these channel / errorReaction dependencies optional?

src/slack/getSlackUserFromName.ts

namoscato · 2025-09-27T19:28:57Z

src/utils/githubToSlackMapping.ts

+/**
+ * Return all GitHub usernames from the specified `github_org`, mapped to their Slack user.
+ *
+ * The users are matched by full name. If a match is not found, the value will be `null`.
+ */
+export async function githubToSlackMapping(
+  octokit: OctokitClient,
+  slack: SlackClient,
+  github_org: string
+): Promise<Record<string, MessageAuthor | null>> {
+  const githubUsers = await fetchGitHubUsers(octokit, github_org)
+  if (githubUsers.length === 0) {
+    warning('No GitHub members found.')
+    return {}
+  }
+
+  info('Fetching Slack users')
+  const slackUsers = await slack.getRealUsers()
+  if (slackUsers.length === 0) {
+    warning('No Slack users found.')
+    return {}
+  }
+
+  const result: Record<string, MessageAuthor | null> = {}
+
+  for (const githubUser of githubUsers) {
+    try {
+      const slackUser = getSlackUserFromName(slackUsers, githubUser.name)
+
+      result[githubUser.login] = {
+        slack_user_id: slackUser.id,
+        username: slackUser.profile.display_name,
+        icon_url: slackUser.profile.image_48
+      }
+    } catch (err) {
+      warning(err instanceof Error ? err.message : String(err))
+
+      result[githubUser.login] = null
+    }
+  }
+
+  return result
+}


4edb065 simplified this implementation a bit.

Also notably, I widened the return interface to include null values, signifying a Slack user could not be matched. I was thinking that could enable a nice getMessageAuthor optimization – essentially skipping the expensive API calls when we already know a user will not match (via null) while still falling back to them for a presumably new / unknown user (via undefined).

Thoughts on that proposed behavior? I didn't update getMessageAuthor yet but happy to do so if you're onboard.

Updated getMessageAuthor in 4732a6a.

- collapse getMessageAuthorFromGithubUserMapping into maybeGetMessageAuthorFromGithubUserMapping - tighten type predicate - improve error handling - introduce GitHubUserMapping type

- notify -> notifySlack - generateMapping -> generateUserMapping - move non-entrypoints to utils/ namespace

channel and errorReaction are unnecessary in the generateUserMapping context

namoscato

Alright I think this is pretty close to code-complete. @jakeschurch, would you mind glancing at where this is landing?

I just need to do some manual testing and would like to take a pass at documentation.

namoscato · 2025-10-01T14:59:52Z

src/generateMapping.ts

+  }
+}
+
+async function generateMapping(


Renamed to generateUserMapping in b1f2cb5.

I also renamed notify → notifySlack.

namoscato · 2025-10-01T15:01:31Z

src/utils/githubToSlackMapping.ts

+/**
+ * Return all GitHub usernames from the specified `github_org`, mapped to their Slack user.
+ *
+ * The users are matched by full name. If a match is not found, the value will be `null`.
+ */
+export async function githubToSlackMapping(
+  octokit: OctokitClient,
+  slack: SlackClient,
+  github_org: string
+): Promise<Record<string, MessageAuthor | null>> {
+  const githubUsers = await fetchGitHubUsers(octokit, github_org)
+  if (githubUsers.length === 0) {
+    warning('No GitHub members found.')
+    return {}
+  }
+
+  info('Fetching Slack users')
+  const slackUsers = await slack.getRealUsers()
+  if (slackUsers.length === 0) {
+    warning('No Slack users found.')
+    return {}
+  }
+
+  const result: Record<string, MessageAuthor | null> = {}
+
+  for (const githubUser of githubUsers) {
+    try {
+      const slackUser = getSlackUserFromName(slackUsers, githubUser.name)
+
+      result[githubUser.login] = {
+        slack_user_id: slackUser.id,
+        username: slackUser.profile.display_name,
+        icon_url: slackUser.profile.image_48
+      }
+    } catch (err) {
+      warning(err instanceof Error ? err.message : String(err))
+
+      result[githubUser.login] = null
+    }
+  }
+
+  return result
+}


Updated getMessageAuthor in 4732a6a.

namoscato · 2025-10-01T15:13:14Z

src/generateUserMapping.ts

+    channel,
+    errorReaction


Updated in 942c96a.

jakeschurch added 2 commits August 21, 2025 11:31

jakeschurch self-assigned this Aug 21, 2025

jakeschurch added 2 commits August 21, 2025 11:57

[sc-30416] exec pnpm run package

346c9a6

Signed-off-by: Jake Schurch <[email protected]>

jakeschurch force-pushed the jakeschurch/sc-30416/cache-slack-users-to-mitigate-api-rate-limiting branch from fc64a95 to 346c9a6 Compare August 21, 2025 15:58

jakeschurch added 2 commits August 21, 2025 12:10

[sc-30416] fmt

0cc19aa

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] more testing

ae30272

Signed-off-by: Jake Schurch <[email protected]>

jakeschurch force-pushed the jakeschurch/sc-30416/cache-slack-users-to-mitigate-api-rate-limiting branch from 9430cde to ae30272 Compare August 21, 2025 16:58

jakeschurch added 10 commits August 21, 2025 13:12

[sc-30416] add check

bab69f8

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] package dist

854a171

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] docs: fmt

f5a6c80

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] add new dist

1430d1c

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] remove print statements

15aabd4

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] fix for gitattrs

431b635

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] fixup

251c8b8

Signed-off-by: Jake Schurch <[email protected]>

jakeschurch force-pushed the jakeschurch/sc-30416/cache-slack-users-to-mitigate-api-rate-limiting branch from fdef784 to 251c8b8 Compare September 9, 2025 18:43

jakeschurch marked this pull request as ready for review September 9, 2025 18:59

jakeschurch requested a review from namoscato September 12, 2025 19:52

namoscato requested changes Sep 18, 2025

View reviewed changes

jakeschurch added 7 commits September 22, 2025 14:45

[sc-30416] fixup readme

e48f0df

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] add requested feedback on action input / output names

44a8537

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] make messageAuthor caching more readable

efee6c5

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] improve code readability

d5e06b7

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] bundle

e2ab954

Signed-off-by: Jake Schurch <[email protected]>

jakeschurch added 4 commits September 22, 2025 15:55

[sc-30416] bug: skip deactivated users

9c5dea5

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] remove deleted

dc871c3

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] skip adding users to cache if error thrown

b5eb3a9

Signed-off-by: Jake Schurch <[email protected]>

[sc-30416] run bundle

fcede99

Signed-off-by: Jake Schurch <[email protected]>

jakeschurch requested a review from namoscato September 23, 2025 17:02

jakeschurch changed the title ~~[sc-30416]: cache slack user details by github user login to mitigate api rate limiting~~ [sc-30416]: Cache Slack user details by GitHub user login to mitigate API rate limiting Sep 23, 2025

namoscato added 5 commits September 27, 2025 11:50

Tighten type predicates

d6f8d7a

Deepen getSlackUserFromName

a43dead

- move to separate module - rename from getSlackUserFromGithubName - filter MemberWithProfile in SlackClient

Revert setFailed changes

6d32203

Simplify githubToSlackMapping

4edb065

- remove unnecessary reduce - surface empty set short circuits; move to warning - preserve not found `null` values - fix GitHubUser type

Fix setFailed handling

927703b

namoscato requested changes Sep 27, 2025

View reviewed changes

namoscato added 4 commits October 1, 2025 08:07

Handle null githubUserMapping values

4732a6a

- collapse getMessageAuthorFromGithubUserMapping into maybeGetMessageAuthorFromGithubUserMapping - tighten type predicate - improve error handling - introduce GitHubUserMapping type

Favor githubUserMapping over withSlackUserId

1663b68

Rename entrypoints

b1f2cb5

- notify -> notifySlack - generateMapping -> generateUserMapping - move non-entrypoints to utils/ namespace

Make SlackClient dependencies optional

942c96a

channel and errorReaction are unnecessary in the generateUserMapping context

namoscato reviewed Oct 1, 2025

View reviewed changes

[sc-30416]: Cache Slack user details by GitHub user login to mitigate API rate limiting #167

Are you sure you want to change the base?

[sc-30416]: Cache Slack user details by GitHub user login to mitigate API rate limiting #167

Uh oh!

Conversation

jakeschurch commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Reviewer Notes

Uh oh!

namoscato left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

namoscato left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

namoscato Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

namoscato Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

namoscato Sep 27, 2025

Choose a reason for hiding this comment

Uh oh!

namoscato Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

namoscato left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

namoscato Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

namoscato Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

namoscato Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakeschurch commented Aug 21, 2025 •

edited

Loading

namoscato left a comment •

edited

Loading

namoscato left a comment •

edited

Loading