Skip to content

feat(slack): expose user email in message.author#239

Open
Seventy7dot2 wants to merge 4 commits intovercel:mainfrom
Seventy7dot2:main
Open

feat(slack): expose user email in message.author#239
Seventy7dot2 wants to merge 4 commits intovercel:mainfrom
Seventy7dot2:main

Conversation

@Seventy7dot2
Copy link

Summary

Resolves #237

Adds support for exposing Slack user email in message.author.email
when the users:read.email scope is granted.

Test plan

  • Created a Slack app with the following OAuth scopes:
    • users:read
    • users:read.email
  • Installed the app in a test Slack workspace.
  • Triggered message events (app_mention and message) to ensure user resolution is performed by the SDK.
  • Verified that the user's email is correctly populated in message.author.email.
  • Confirmed that when the users:read.email scope is not granted, the email field remains undefined and existing behavior is unchanged.
  • Ensured no breaking changes to the existing user object structure.

@vercel
Copy link
Contributor

vercel bot commented Mar 14, 2026

@Seventy7dot2 is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@haydenbleasel haydenbleasel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, thanks for the PR! Nice clean feature addition. A few things I noticed:

Issues

email: "unknown" vs undefined

The Author.email field is typed as email?: string (optional), but the implementation defaults to "unknown" instead of undefined in two places:

  • packages/adapter-slack/src/index.tsuserEmail = "unknown" initialization
  • packages/adapter-slack/src/index.tsemail: "unknown" hardcoded in the fetchMessages path

This makes it harder for consumers to check if an email is available — they'd need to check both === undefined and === "unknown". The PR description says "the email field remains undefined" when the scope isn't granted, but the code doesn't actually do that. Should be undefined to match the type.

fetchMessages doesn't populate email

The fetchMessages code path hardcodes email: "unknown" instead of using the email from lookupUser (which is already being called for username resolution in that path). The new test asserts email is populated from fetchMessages, so worth double-checking it actually passes against this diff.

Missing changeset

This is a minor feature addition (new field on Author in core types + new scope). Looks like the .changeset/ file is missing — the repo requires one per PR.

Nits

README formatting churn

The README diff is ~60 lines of table/code reformatting unrelated to the email feature. Makes the diff harder to review — would be cleaner as a separate commit.

Scope as default vs optional

The docs/manifests add users:read.email to the default scopes. Since this is optional behavior, might be better to document it as an optional additional scope. Existing users upgrading would need to re-install their Slack app to pick up the new scope.

@Seventy7dot2
Copy link
Author

Seventy7dot2 commented Mar 16, 2026

@haydenbleasel
Thanks for the detailed review!

I've addressed the points you raised:

• Updated the implementation so Author.email resolves to undefined when the email scope isn't available, removing the "unknown" sentinel.

• Ensured the fetchMessages path correctly populates email via the existing lookupUser resolution used for user info.

• Added tests verifying:

  • email is returned when Slack provides it
  • email is undefined when the users:read.email scope is not present.

• Added the required .changeset for the feature.

• Cleaned up the README changes and limited them to documenting the optional users:read.email scope.

I also reorganized the commits to separate feature, tests, docs, and changeset for easier review.

Would appreciate another look when you get a chance. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slack Adapter: Support fetching Slack user email (users:read.email)

2 participants