Refactor: Combine Rules Validation and Get Rules MCP Tools #9157
Conversation
![google-labs-jules[bot]](https://avatars.githubusercontent.com/in/842251?s=60&v=4){kind=small}
Combines the firestore_validate_rules, storage_validate_rules, and database_validate_rules MCP tools into a single validate_rules MCP tool in the core feature group. This new tool takes an additional required argument 'type', which can be either 'firestore', 'rtdb', or 'storage', and then triggers the appropriate behavior based on that.
|
👋 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. |
src/mcp/tools/core/validate_rules.ts
Outdated
| databaseUrl: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "For RTDB, connect to the database at url. If omitted, use default database instance. Can point to emulator URL.", | ||
| ), |
There was a problem hiding this comment.
Couldn't this apply to Firestore and Storage as well (multiple DBs / multiple buckets)? I think we probably either make this a general resource_id which is RTDB instance name, firestore database name, or bucket name, or we drop this altogether and do "defaults only" for now.
There was a problem hiding this comment.
Dropped this for now, as it is a bit unclear to me whether this even matters - ie can you even have a ruleset that is valid for one resource but invalid for another resource of the same type?
Also, ended up cleaning up a bunch of this logic in here to properly check the default RTDB instance, and make the behavior of rtdb more similar to firestore/storage.
src/mcp/tools/core/index.ts
Outdated
| export const coreTools: ServerTool[] = [ | ||
| login, | ||
| logout, | ||
| validate_rules, |
There was a problem hiding this comment.
interesting corner case for feature detection: we should only activate this tool if one of the three services that use it are active.
There was a problem hiding this comment.
Agreed - I think I'm gonna handle this in a follow up PR tho. I'd like to build out a slightly more robust system for feature detection rather than have logic scattered across the codebase, and I think it makes sense to cover a bunch of tools at once.
I'll add a todo to keep track of this tho.
* feat(mcp): combine get_rules tools into a single core_get_rules tool Combines the firestore_get_rules, storage_get_rules, and database_get_rules tools into a single get_rules tool in the core tool group. This new tool takes an additional required argument 'type', which can be either 'firestore', 'rtdb', or 'storage', and then triggers the appropriate behavior based on that. It also adds a validation check to ensure that the `databaseUrl` argument is only provided when the type is 'rtdb'. * one last file * PR fixes * GCA suggestions * Formats --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Joe Hanley <joehanley@google.com>
joehan
changed the title
There was a problem hiding this comment.
rtdb part lgtm (did not pass over the other implementations). Thanks for making these changes. fyi this is probably going to conflict with @charlotteliang 's pr to rename the rules to rtdb from database, so this or that will have to be resolved
github-project-automation
bot
moved this to Approved [PR]
in [Cloud] Extensions + Functions
github-project-automation
bot
moved this from Approved [PR]
to Done
in [Cloud] Extensions + Functions
This change combines the
firestore_validate_rules,storage_validate_rules, anddatabase_validate_rulesMCP tools into a singlevalidate_rulestool in thecorefeature group. This new tool takes atypeargument to specify which validation to run.PR created automatically by Jules for task 9425419525586992510