Reorganizing Crashlytics MCP Tools #9594

maxl0rd · 2025-12-05T21:26:58Z

Description

Consolidating all of the crashlytics_get_* tools into a single tool crashlytics_get_report. This will enable the set of supported reports to expand without adding additional tools.
Moving most of the content from the crashlytics:connect prompt to guide resources. This provides better support for the many MCP clients without support for prompts.
Numerous other improvements in prompt engineering and error handling

gemini-code-assist · 2025-12-05T21:27:23Z

Summary of Changes

Hello @maxl0rd, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the Crashlytics MCP tools by streamlining report generation into a single, unified function and externalizing detailed instructions into comprehensive resource guides. This change aims to enhance the system's scalability, improve the user experience for various MCP clients, and provide more robust error handling and guidance for developers interacting with Crashlytics data.

Highlights

Crashlytics Tool Consolidation: The multiple crashlytics_get_* tools have been consolidated into a single, more flexible crashlytics_get_report tool, allowing for easier expansion of supported reports without adding new tools.
Prompt Content Migration to Guides: Extensive instructional content from the crashlytics:connect prompt has been moved into dedicated resource guides, improving support for MCP clients that do not support interactive prompts.
Improved Prompt Engineering and Error Handling: The system now provides clearer instructions and better error handling, especially when required parameters like appId are missing, by directing users to relevant guides.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request is a great refactoring of the Crashlytics MCP tools. Consolidating the report-fetching tools into a single crashlytics_get_report tool is a significant improvement, making it more extensible. Moving the large prompt content into separate guide resources is also an excellent change for modularity and maintainability. The error handling has been improved across the tools, providing more context to the agent. I've found a few minor issues, including some typos in user-facing text, a small bug in output formatting, and opportunities for code simplification. Overall, this is a high-quality change.

gemini-code-assist · 2025-12-05T21:29:04Z

src/crashlytics/filters.ts


 /**
- * Perform some simplistic validation on filters.
+ * Perform some simplistic validation on filters and fill .


The JSDoc for this function appears to be incomplete. The sentence ends abruptly with a period after "fill". It should probably clarify what is being filled.

Suggested change

* Perform some simplistic validation on filters and fill .

* Perform some simplistic validation on filters and fill in missing values.

gemini-code-assist · 2025-12-05T21:29:05Z

src/crashlytics/reports.ts

+export function toReportEnum(reportName: string): CrashlyticsReport | undefined {
+  const enumValues = Object.values(CrashlyticsReport);
+  if (enumValues.includes(reportName as CrashlyticsReport)) {
+    return reportName as CrashlyticsReport;
+  } else {
+    return undefined;
+  }
+}


This function can be simplified and made more type-safe. The cast reportName as CrashlyticsReport within the includes method is a bit of a code smell. A more explicit approach would be to cast the enum values to a string array for the check.

export function toReportEnum(reportName: string): CrashlyticsReport | undefined { if ((Object.values(CrashlyticsReport) as string[]).includes(reportName)) { return reportName as CrashlyticsReport; } return undefined; }

gemini-code-assist · 2025-12-05T21:29:05Z

src/mcp/prompts/crashlytics/connect.ts

+    `.trim();
+
+    const notLoggedInInstruction = `
+**Instruct the User to Log In***


There seems to be a typo here. There's an extra asterisk at the end of the markdown bold tag.

Suggested change

**Instruct the User to Log In***

**Instruct the User to Log In**

gemini-code-assist · 2025-12-05T21:29:05Z

src/mcp/resources/guides/crashlytics_connect.ts

+  1. [Firebase App Id Guide](firebase://guides/app_id)
+     This guide provides crucial instructions for obtaining the application's App Id which is required for all API calls.
+
+  1. [Firebase Crashlytics Reports Guide](firebase://guides/crashlytics/reports)


The numbered list is incorrect. This item should be 2. instead of 1. to maintain the correct sequence.

Suggested change

1. [Firebase Crashlytics Reports Guide](firebase://guides/crashlytics/reports)

2. [Firebase Crashlytics Reports Guide](firebase://guides/crashlytics/reports)

gemini-code-assist · 2025-12-05T21:29:05Z

src/mcp/tools/crashlytics/reports.ts

+function toText(response: Report, filters: EventFilter): Record<string, string> {
+  const result: Record<string, string> = {
+    name: response.name || "",
+    filters: dump(filters, DUMP_OPTIONS),
+  };
+  for (const [key, value] of Object.entries(response)) {
+    result[key] = dump(value, DUMP_OPTIONS);
+  }
+  return result;
+}


The name property is initialized in the result object and then potentially overwritten in the loop. This is redundant and can lead to inconsistent formatting. For example, if response.name is undefined, it's initialized to "", but the loop would overwrite it with the result of dump(undefined), which is "null\n". It's better to handle name once, outside the loop, or skip it inside the loop if already handled.

Suggested change

function toText(response: Report, filters: EventFilter): Record<string, string> {

const result: Record<string, string> = {

name: response.name || "",

filters: dump(filters, DUMP_OPTIONS),

};

for (const [key, value] of Object.entries(response)) {

result[key] = dump(value, DUMP_OPTIONS);

}

return result;

}

function toText(response: Report, filters: EventFilter): Record<string, string> {

const result: Record<string, string> = {

name: response.name || "",

filters: dump(filters, DUMP_OPTIONS),

};

for (const [key, value] of Object.entries(response)) {

if (key === "name") {

continue; // Already handled

}

result[key] = dump(value, DUMP_OPTIONS);

}

return result;

}

…ntent

schnecle · 2025-12-09T02:46:23Z

src/crashlytics/filters.ts

-      "from GoogleService-Info.plist. If neither is available, ask the user to " +
-      "provide the app id.",
-  );
+  .describe("Firebase App Id. Leave blank if unknown to receive instructions for finding it.");


Should we instruct it to use read_resource here rather than telling it to leave it blank?

schnecle · 2025-12-09T13:00:23Z

src/mcp/tools/crashlytics/reports.ts

-      Only relevant for iOS, iPadOS and MacOS applications.`,
+    name: "get_report",
+    description:
+      `Use this to request numerical reports from Crashlytics. Reports contain aggregated metrics describing the number of crash events and number of impacted end users. The reports are grouped by different dimensions such as issue, version or device.


Should we make mention of the guide in the tool description?

schnecle · 2025-12-09T13:02:48Z

src/crashlytics/reports.spec.ts

    nock.cleanAll();
  });

  it("should resolve with the response body on success", async () => {


Can we add some tests around the different validation cases we expect with the error responses?

schnecle · 2025-12-09T13:06:14Z

src/mcp/resources/guides/crashlytics_connect.ts

+export const RESOURCE_CONTENT = `
+### Instructions for Working with Firebase Crashlytics Tools
+
+Only ask the user one question at a time. Do not proceed without user instructions. Upon receiving user instructions, refer to the relevant resources for guidance.


Is this a strong enough statement? I've seen it ignore this pretty readily without emphasis - asking multiple questions at a time being the main thing it tries to do.

schnecle · 2025-12-09T13:06:57Z

src/mcp/resources/guides/crashlytics_connect.ts

+
+### Check That You Are Connected
+
+Verify that you can read the app's Crashlytics data by getting the topVersions report. This report will tell you which app versions have the most events.


We didn't have this step before - how has this played out in your testing? My one concern here is that it increases token consumption by default without interaction from the user. That happens not infrequently anyway, but curious if it happens more with this instruction.

schnecle · 2025-12-09T13:12:36Z

src/mcp/tools/crashlytics/issues.ts

+    if (!appId) {
+      result.isError = true;
+      result.content.push({ type: "text", text: "Must specify 'appId' parameter" });
+      result.content.push({ type: "text", text: forceAppIdGuide });


We don't return the issue guide here. At some iteration, I feel like we were doing that. If we were, why did we pull it back?

schnecle · 2025-12-09T13:14:07Z

src/mcp/tools/crashlytics/reports.ts

-);
+    if (!report) {
+      result.isError = true;
+      result.content.push({ type: "text", text: `Error: ${REPORT_ERROR_CONTENT}` });


Same question here - why not reference reading the guide in the error response?

gemini-code-assist bot reviewed Dec 5, 2025

View reviewed changes

maxl0rd added 10 commits December 8, 2025 15:37

WIP moving crashlytics prompt into guides

5250bf0

Improving crashlytics guides

750361a

Improving crashlytics guides

d9312f6

Restore new app_id instructions

0c0d321

Fix bug where additional prompt said report result was empty

41eca55

Include request filters in report tool response

adab0c0

Improving error handling in crashlytics tools and using more guide co…

3e4c6f7

…ntent

Fix format

9978883

Ignore gemini environment dotfile

35f83e4

Remove .env

89c8cdd

schnecle force-pushed the ml_crashlytics_resources branch from 97e8d00 to 89c8cdd Compare December 8, 2025 20:38

Fix imports

690e4d8

schnecle reviewed Dec 9, 2025

View reviewed changes

visumickey requested review from raymondlam and visumickey December 9, 2025 17:22

	* Perform some simplistic validation on filters and fill .
	* Perform some simplistic validation on filters and fill in missing values.

	Instruct the User to Log In*
	Instruct the User to Log In

	1. [Firebase Crashlytics Reports Guide](firebase://guides/crashlytics/reports)
	2. [Firebase Crashlytics Reports Guide](firebase://guides/crashlytics/reports)


		### Check That You Are Connected

		Verify that you can read the app's Crashlytics data by getting the topVersions report. This report will tell you which app versions have the most events.

Reorganizing Crashlytics MCP Tools #9594

Are you sure you want to change the base?

Reorganizing Crashlytics MCP Tools #9594

Uh oh!

Conversation

maxl0rd commented Dec 5, 2025

Description

Uh oh!

gemini-code-assist bot commented Dec 5, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

schnecle Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants