Add web support for Crashlytics MCP tools and prompts - #10822
Conversation
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request extends Firebase Crashlytics support to web applications, introducing web-specific reports (topBrowsers and webMetrics), metrics (sessionsCount), and filters (browserFilterDisplayNames). It also updates MCP prompts, guides, tools, and availability checks to distinguish between mobile and web platforms. The reviewer feedback highlights a critical issue in the web Crashlytics detection logic, which incorrectly expects "firebase/crashlytics" in package.json instead of the standard "firebase" or "@firebase/crashlytics" dependencies, and points out a few minor typos in comments and documentation.
| .array(z.enum(["SIGNAL_EARLY", "SIGNAL_FRESH", "SIGNAL_REGRESSED", "SIGNAL_REPETITIVE"])) | ||
| .optional() | ||
| .describe(`Counts events matching the given signals`), | ||
| .describe(`Counts events matching the given signals. Only supported for mobile apps.`), |
There was a problem hiding this comment.
This looks like the kind of thing that an agent is really going to mess up. Have you tested this one out locally? It might perform better if you say mobile apps and then qualify it like "mobile apps (e.g. iOS and Android)"
There was a problem hiding this comment.
I think this instruction will mostly work. If the api can just discard invalid filters, this is probably fine.
| .array(z.enum(["SIGNAL_EARLY", "SIGNAL_FRESH", "SIGNAL_REGRESSED", "SIGNAL_REPETITIVE"])) | ||
| .optional() | ||
| .describe(`Counts events matching the given signals`), | ||
| .describe(`Counts events matching the given signals. Only supported for mobile apps.`), |
There was a problem hiding this comment.
I think this instruction will mostly work. If the api can just discard invalid filters, this is probably fine.
| return false; | ||
| } | ||
|
|
||
| const sourceFiles = await detectFiles(appPath, "*.{js,jsx,ts,tsx,mjs,cjs,html,vue,svelte}"); |
There was a problem hiding this comment.
This could be very costly to the MCP start up time on a big app. For the other options, we try to do something that works for most folks and then give the workaround that they can force the crashlytics tools to be there even if not detected. Do we have any conventions around where someone would import the crashlytics dep that we could use to cover that 90% case?
There was a problem hiding this comment.
Agreed-- I'm worried about the performance of this as well. @tonybaroneee is there a particular file where we expect most developers to import crashlytics?
I just remembered for the app testing agent that we just do a check to see if the app distribution API is enabled on the project to determine if we expose the app testing agent MCP tools. I wonder if is there an API or onboarding step that we can check to determine if they're onboarded for crash for web. That may be easier than trying to pinpoint a specific file that has a crashlytics import.
There was a problem hiding this comment.
Yeah, maybe let's just call a GET to the telemetry admin API config endpoint to see if they've onboarded. That's a pretty strong signal (even though they still might not have provisioned the SDK in their app yet).
There was a problem hiding this comment.
Updated this to just check if the telemetry admin API config exists instead of checking to see if the depedency is declared in the package.json file.
5 participants
Description
Add support for web apps with Crashlytics MCP tools and prompts.
Scenarios Tested
Sample Commands