-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Combine auth_get_user and auth_list_users MCP tools #9165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Combines the `auth_get_user` and `auth_list_users` MCP tools into a single `auth_get_users` tool. This new tool has two optional arguments, `uids` and `emails`. - When no arguments are provided, it behaves like `auth_list_users`. - When either `emails` or `uids` is provided, it looks up users by the provided identifiers. Removes unused variables and ensures that `passwordHash` and `salt` fields are consistently removed from all user objects returned by the tool.
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll acknowledge your comments with a 👀 emoji and then get to work. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! I will automatically address your feedback. For any comments you don't want me to act on, just include (aside). For security, I will only act on instructions from the user who triggered this task for this pull request. |
/gemini review |
I see you've requested a review from Gemini. I'm not able to trigger that review myself. Please let me know if there's anything else I can do to help. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully combines the auth_get_user
and auth_list_users
tools into a single auth_get_users
tool, which simplifies the toolset. The implementation is clean and the tests cover the basic functionality.
I've added a couple of suggestions:
- In
src/mcp/tools/auth/get_users.ts
, I've pointed out a performance issue where user lookups are done sequentially. I've suggested a refactoring to perform these lookups in parallel usingPromise.all
, which also helps reduce some code duplication. - In
src/mcp/tools/auth/get_users.spec.ts
, I've suggested adding a test case for when bothuids
andemails
are provided to ensure the prioritization logic is explicitly tested.
Overall, this is a good change that improves the API. Addressing these points will make the new tool more robust and performant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still looking
Passing back to Joe. Changed a few things so it should be better |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* feat(mcp): Combine auth_get_user and auth_list_users tools Combines the `auth_get_user` and `auth_list_users` MCP tools into a single `auth_get_users` tool. This new tool has two optional arguments, `uids` and `emails`. - When no arguments are provided, it behaves like `auth_list_users`. - When either `emails` or `uids` is provided, it looks up users by the provided identifiers. Removes unused variables and ensures that `passwordHash` and `salt` fields are consistently removed from all user objects returned by the tool. * Formats * Parallelize calls * Small fixes * npm format --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Joe Hanley <joehanley@google.com> Co-authored-by: Alexander Nohe <nohe@google.com>
* feat(mcp): Combine auth_get_user and auth_list_users tools Combines the `auth_get_user` and `auth_list_users` MCP tools into a single `auth_get_users` tool. This new tool has two optional arguments, `uids` and `emails`. - When no arguments are provided, it behaves like `auth_list_users`. - When either `emails` or `uids` is provided, it looks up users by the provided identifiers. Removes unused variables and ensures that `passwordHash` and `salt` fields are consistently removed from all user objects returned by the tool. * Formats * Parallelize calls * Small fixes * npm format --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Joe Hanley <joehanley@google.com> Co-authored-by: Alexander Nohe <nohe@google.com>
This change combines the
auth_get_user
andauth_list_users
MCP tools into a singleauth_get_users
tool with optionaluids
andemails
arguments. It also removes unused variables and ensures that sensitive fields are pruned from the output.PR created automatically by Jules for task 14520879572885862082