feat(tools): Add provider-scoped userLookup - #1359
Merged
Merged
Conversation
Hard-cutover the Slack-only lookup tool to a shared userLookup that resolves people by Slack ID, email, name, or GitHub username against workspace profile fields and returns Slack mentions. Co-Authored-By: David Cramer <david@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Replace hardcoded mode/value with a provider enum and provider-specific query string so identity lookup can grow without new tool modes. Co-Authored-By: David Cramer <david@sentry.io>
Drop speculative provider-registry scaffolding, collapse defensive result shaping, and keep github search on already-normalized logins. Co-Authored-By: David Cramer <david@sentry.io>
dcramer
marked this pull request as ready for review
August 9, 2026 22:44
Co-Authored-By: David Cramer <david@sentry.io>
Co-Authored-By: David Cramer <david@sentry.io>
Contributor
Author
There was a problem hiding this comment.
I found three issues to fix before merge:
upsertIdentityRecordchecks the current user before the upsert. The conflict update then keeps any existinguserId. Two OAuth callbacks for the same provider account can both pass the first check. One callback can then keep the other user's link and still return success. Make the write reject a differentuserId, or verify the returneduserIdbefore success. Add a conflict test for this case.createToolsadds every enabled plugin to theuserLookupprovider enum. Only providers that write linked identities can return results. Other plugins now look supported but always return an empty result. Only register providers with identity support. If there is no clear capability yet, keep the supported list explicit.- Linked identity fields use
coalesceduring updates. A verified GitHub login change will not replace the old handle. Since lookup uses the handle, reconnecting after a rename still leaves lookup broken. Trusted provider writes should update mutable provider fields. Add a rename test.
Co-Authored-By: David Cramer <david@sentry.io>
Contributor
Author
There was a problem hiding this comment.
I found three policy issues to fix before merge:
resolveStoredProviderusesilike(handle, query). SQL treats%and_as wildcards. A query such as%can list unrelated users in the current workspace. Use case-insensitive equality, or escape pattern characters. Add a test for%and_.- Provider lookup returns the linked Slack email even though the tool only needs a Slack mention. This adds cross-provider email exposure without an access rule. Return the minimum safe Slack identity fields for non-Slack providers. Do not include email or other Slack profile data unless the contract and access rule require them.
userLookupdeclaresreadOnlyHint: true, but Slack lookup writes identity rows throughstoreProfile. The annotation must match the real behavior. Either make profile storage an explicit best-effort side effect outside the read-only tool contract, or setreadOnlyHint: false.
Co-Authored-By: David Cramer <david@sentry.io>
Contributor
Author
There was a problem hiding this comment.
I found two issues to fix before merge:
- Provider identities do not follow the connection lifecycle.
unlinkProviderdeletes credentials but leaves the linked identity row. Connecting a different account also adds the new row without removing the old one. As a result, an unlinked or replaced GitHub account can still resolve to the Slack user. This is stale personal-data linkage and makes unlink misleading. Keep the provider identity in sync with the one-account-per-provider token slot. Remove the exact old linked identity on unlink and account replacement, with tests for both flows. - Identity linking is best effort, but failure has no repair path and the UI still reports full success. The token can be stored while
userLookuphas no provider identity, or while an old identity remains active. A transient Slack or database failure then leaves lookup wrong until the user reconnects, with no signal that reconnect is needed. Preserve the successful OAuth connection, but add a durable retry/reconciliation path or surface a partial-success state that tells the user identity lookup is not ready. Add a callback test that proves the chosen recovery behavior.
Co-Authored-By: David Cramer <david@sentry.io>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0d9cf0a. Configure here.
Co-Authored-By: David Cramer <david@sentry.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Replace Slack-only
slackUserLookupwithuserLookup. The tool takes an identity provider and a query. It returns a Slack mention for the current workspace.Direct Slack lookup returns the Slack profile data needed to choose the right person. Other provider lookups return only the linked Slack ID, handle, and mention. Lookup stays read-only. Identity writes happen only in trusted OAuth flows.
Store provider identities after trusted OAuth account checks. GitHub’s authenticated user response is trusted. Content claims, such as Git commit attribution, are not trusted for identity writes.
Only plugins that can verify OAuth accounts appear in the provider enum. Trusted provider updates refresh mutable account fields. A provider account cannot move to another user during a conflicting write.
OAuth token storage stays successful when the optional identity link fails. Unlink removes only the stored account identity. Other identities for the same provider stay linked.
Checks
pnpm --filter @sentry/junior typecheckpnpm --filter @sentry/junior exec oxlint src/chat/tools/user-lookup.ts tests/integration/user-lookup.test.tspnpm --filter @sentry/junior exec vitest run tests/integration/user-lookup.test.ts(25/25)Refs #1163
Requested by David Cramer.
--
View Junior Session [Sentry]