Skip to content

feat(tools): Add provider-scoped userLookup - #1359

Merged
dcramer merged 9 commits into
mainfrom
feat/user-lookup-github-username
Aug 10, 2026
Merged

feat(tools): Add provider-scoped userLookup#1359
dcramer merged 9 commits into
mainfrom
feat/user-lookup-github-username

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Replace Slack-only slackUserLookup with userLookup. 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 typecheck
  • pnpm --filter @sentry/junior exec oxlint src/chat/tools/user-lookup.ts tests/integration/user-lookup.test.ts
  • pnpm --filter @sentry/junior exec vitest run tests/integration/user-lookup.test.ts (25/25)

Refs #1163

Requested by David Cramer.

--

View Junior Session [Sentry]

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>
@vercel

vercel Bot commented Aug 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
junior-docs Ready Ready Preview Aug 10, 2026 1:05am

Request Review

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>
@sentry-junior sentry-junior Bot changed the title feat(tools): Rename slackUserLookup to userLookup with GitHub mode feat(tools): Add provider-scoped userLookup Aug 9, 2026
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
dcramer marked this pull request as ready for review August 9, 2026 22:44
@github-actions github-actions Bot added the risk: high PR risk score: high label Aug 9, 2026
Comment thread packages/junior/src/chat/slack/users.ts Outdated
Comment thread packages/junior/src/chat/slack/users.ts Outdated
Co-Authored-By: David Cramer <david@sentry.io>
Comment thread packages/junior/src/handlers/oauth-callback.ts Outdated
Co-Authored-By: David Cramer <david@sentry.io>

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found three issues to fix before merge:

  • upsertIdentityRecord checks the current user before the upsert. The conflict update then keeps any existing userId. 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 different userId, or verify the returned userId before success. Add a conflict test for this case.
  • createTools adds every enabled plugin to the userLookup provider 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 coalesce during 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>

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found three policy issues to fix before merge:

  • resolveStoredProvider uses ilike(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.
  • userLookup declares readOnlyHint: true, but Slack lookup writes identity rows through storeProfile. The annotation must match the real behavior. Either make profile storage an explicit best-effort side effect outside the read-only tool contract, or set readOnlyHint: false.

Co-Authored-By: David Cramer <david@sentry.io>

@sentry-junior sentry-junior Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found two issues to fix before merge:

  • Provider identities do not follow the connection lifecycle. unlinkProvider deletes 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 userLookup has 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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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.

Comment thread packages/junior/src/chat/credentials/unlink-provider.ts
Co-Authored-By: David Cramer <david@sentry.io>
@dcramer
dcramer merged commit 7b6f5bb into main Aug 10, 2026
41 checks passed
@dcramer
dcramer deleted the feat/user-lookup-github-username branch August 10, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant