(feat) Crashlytics tools improvements (#9138)

- Improving instructions and return values in crashlytics tools
- Making interval handling more robust
- Adding return messages to avoid undefined tool responses

Author: maxl0rd

src/crashlytics/events.spec.ts

@@ -2,7 +2,7 @@ import * as chai from "chai";
 import * as nock from "nock";
 import * as chaiAsPromised from "chai-as-promised";
 
-import { listEvents } from "./events";
+import { listEvents, batchGetEvents } from "./events";
 import { FirebaseError } from "../error";
 import { crashlyticsApiOrigin } from "../api";
 
@@ -82,4 +82,54 @@ describe("events", () => {
       ).to.be.rejectedWith(FirebaseError, "Unable to get the projectId from the AppId.");
     });
   });
+
+  describe("batchGetEvents", () => {
+    const eventNames = [
+      "projects/1234567890/apps/1:1234567890:android:abcdef1234567890/events/test_event_id_1",
+      "projects/1234567890/apps/1:1234567890:android:abcdef1234567890/events/test_event_id_2",
+    ];
+
+    it("should resolve with the response body on success", async () => {
+      const mockResponse = {
+        events: [{ id: "test_event_id_1" }, { id: "test_event_id_2" }],
+      };
+
+      nock(crashlyticsApiOrigin())
+        .get(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`)
+        .query({
+          "event.names": eventNames,
+        })
+        .reply(200, mockResponse);
+
+      const result = await batchGetEvents(appId, eventNames);
+
+      expect(result).to.deep.equal(mockResponse);
+      expect(nock.isDone()).to.be.true;
+    });
+
+    it("should throw a FirebaseError if the API call fails", async () => {
+      nock(crashlyticsApiOrigin())
+        .get(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`)
+        .query({
+          "event.names": eventNames,
+        })
+        .reply(500, { error: "Internal Server Error" });
+
+      await expect(batchGetEvents(appId, eventNames)).to.be.rejectedWith(
+        FirebaseError,
+        `Failed to batch get events for app_id ${appId}.`,
+      );
+    });
+
+    it("should throw a FirebaseError if there are too many events", async () => {
+      const tooManyEventNames = Array.from(Array(101).keys()).map(
+        (i) =>
+          `projects/1234567890/apps/1:1234567890:android:abcdef1234567890/events/test_event_id_${i}`,
+      );
+      await expect(batchGetEvents(appId, tooManyEventNames)).to.be.rejectedWith(
+        FirebaseError,
+        "Too many events in batchGet request",
+      );
+    });
+  });
 });

src/crashlytics/events.ts

@@ -1,7 +1,7 @@
 import { logger } from "../logger";
 import { FirebaseError, getError } from "../error";
 import { CRASHLYTICS_API_CLIENT, parseProjectNumber, TIMEOUT } from "./utils";
-import { ListEventsResponse } from "./types";
+import { BatchGetEventsResponse, ListEventsResponse } from "./types";
 import { EventFilter, filterToUrlSearchParams } from "./filters";
 
 /**
@@ -43,3 +43,44 @@ export async function listEvents(
     });
   }
 }
+
+/**
+ * Get multiple events by resource name.
+ * Can be used with the `sampleEvent` resource included in topIssues reports.
+ * @param appId Firebase app_id
+ * @param eventNames the resource names for the desired events.
+ * Format: "projects/{project}/apps/{app_id}/events/{event_id}"
+ * @return A BatchGetEventsResponse including an array of Event.
+ */
+export async function batchGetEvents(
+  appId: string,
+  eventNames: string[],
+): Promise<BatchGetEventsResponse> {
+  const requestProjectNumber = parseProjectNumber(appId);
+  if (eventNames.length > 100) throw new FirebaseError("Too many events in batchGet request");
+  logger.debug(
+    `[crashlytics] batchGetEvents called with appId: ${appId}, eventNames: ${eventNames.join(", ")}`,
+  );
+  const queryParams = new URLSearchParams();
+  eventNames.forEach((en) => {
+    queryParams.append("event.names", en);
+  });
+
+  try {
+    const response = await CRASHLYTICS_API_CLIENT.request<void, BatchGetEventsResponse>({
+      method: "GET",
+      headers: {
+        "Content-Type": "application/json",
+      },
+      path: `/projects/${requestProjectNumber}/apps/${appId}/events:batchGet`,
+      queryParams: queryParams,
+      timeout: TIMEOUT,
+    });
+
+    return response.body;
+  } catch (err: unknown) {
+    throw new FirebaseError(`Failed to batch get events for app_id ${appId}.`, {
+      original: getError(err),
+    });
+  }
+}

src/crashlytics/filters.ts

@@ -14,8 +14,14 @@ export const IssueIdSchema = z.string().describe("Crashlytics issue id, as hexid
 
 export const EventFilterSchema = z
   .object({
-    intervalStartTime: z.string().optional().describe(`A timestamp in ISO 8601 string format`),
-    intervalEndTime: z.string().optional().describe(`A timestamp in ISO 8601 string format.`),
+    intervalStartTime: z
+      .string()
+      .optional()
+      .describe(`A timestamp in ISO 8601 string format. Defaults to 7 days ago.`),
+    intervalEndTime: z
+      .string()
+      .optional()
+      .describe(`A timestamp in ISO 8601 string format. Defaults to now.`),
     versionDisplayNames: z
       .array(z.string())
       .optional()

src/crashlytics/issues.spec.ts

@@ -68,7 +68,7 @@ describe("issues", () => {
 
       nock(crashlyticsApiOrigin())
         .patch(`/v1alpha/projects/${requestProjectNumber}/apps/${appId}/issues/${issueId}`, {
-          issue: { state: state },
+          state,
         })
         .query({ updateMask: "state" })
         .reply(200, mockResponse);

src/crashlytics/issues.ts

@@ -51,7 +51,7 @@ export async function updateIssue(appId: string, issueId: string, state: State):
       },
       path: `/projects/${requestProjectNumber}/apps/${appId}/issues/${issueId}`,
       queryParams: { updateMask: "state" },
-      body: { issue: { state } },
+      body: { state },
       timeout: TIMEOUT,
     });
 

src/crashlytics/notes.ts

@@ -44,7 +44,7 @@ export async function createNote(appId: string, issueId: string, note: string):
  * @param issueId Crashlytics issue id
  * @param noteId Crashlytics note id
  */
-export async function deleteNote(appId: string, issueId: string, noteId: string): Promise<void> {
+export async function deleteNote(appId: string, issueId: string, noteId: string): Promise<string> {
   const requestProjectNumber = parseProjectNumber(appId);
 
   logger.debug(
@@ -56,6 +56,7 @@ export async function deleteNote(appId: string, issueId: string, noteId: string)
       path: `/projects/${requestProjectNumber}/apps/${appId}/issues/${issueId}/notes/${noteId}`,
       timeout: TIMEOUT,
     });
+    return `Deleted note ${noteId}`;
   } catch (err: unknown) {
     throw new FirebaseError(
       `Failed to delete note ${noteId} from issue ${issueId} for app ${appId}`,

src/crashlytics/types.ts

@@ -458,16 +458,12 @@ export enum ThreadState {
 
 /** Request message for the ListEvents method. */
 export interface ListEventsRequest {
-  /** The Firebase application. Formatted like "projects/{project}/apps/{app_id}" */
-  parent: string;
   /** The maximum number of events per page. If omitted, defaults to 10. */
   pageSize?: number;
   /** A page token, received from a previous calls. */
   pageToken?: string;
   /** Filter only the desired events. */
   filter?: EventFilters;
-  /** The list of Event fields to include in the response. If omitted, the full event is returned. */
-  readMask?: string;
 }
 
 /** Response message for the ListEvents method. */
@@ -478,6 +474,22 @@ export interface ListEventsResponse {
   nextPageToken?: string;
 }
 
+/** Request message for the BatchGetEvents method. */
+export interface BatchGetEventsRequest {
+  /**
+   * The resource names of the desired events.
+   * A maximum of 100 events can be retrieved in a batch.
+   * Format: "projects/{project}/apps/{app_id}/events/{event_id}"
+   */
+  names: string[];
+}
+
+/** Response message for the BatchGetEvents method. */
+export interface BatchGetEventsResponse {
+  /** Returns one or more events. */
+  events: Event[];
+}
+
 /**
  * Filters for ListEvents method.
  * Multiple conditions for the same field are combined in an ‘OR’ expr
@@ -590,8 +602,6 @@ export interface DeviceFilter {
 
 /** The request method for the GetReport method. */
 export interface GetReportRequest {
-  /** The report name. Formatted like "projects/{project}/apps/{app_id}/reports/{report}". */
-  name: string;
   /** Filters to customize the report. */
   filter?: ReportFilters;
   /** The maximum number of result groups to return. If omitted, defaults to 25. */
@@ -637,8 +647,6 @@ export interface ReportFilters {
 
 /** Request message for the UpdateIssue method. */
 export interface UpdateIssueRequest {
-  /** The issue to update. */
-  issue: Issue;
-  /** The list of Issue fields to update. Currently only "state" is mutable. */
-  updateMask?: string;
+  /** Only the "state" field is mutable. */
+  state: State;
 }

src/mcp/prompts/crashlytics/connect.ts

@@ -53,8 +53,11 @@ they would like to perform. Here are some possibilities and instructions follow
 
 Follow these steps to fetch issues and prioritize them.
 
-  1. Use the 'crashlytics_list_top_issues' tool to fetch up to 20 issues.
-  2. Use the 'crashlytics_list_top_versions' tool to fetch the top versions for this app.
+  1. Use the 'crashlytics_get_top_issues' tool to fetch up to 20 issues.
+    1a. Analyze the user's query and apply the appropriate filters.
+    1b. If the user asks for crashes, then set the issueErrorType filter to *FATAL*.
+    1c. If the user asks about a particular time range, then set both the intervalStartTime and intervalEndTime.
+  2. Use the 'crashlytics_get_top_versions' tool to fetch the top versions for this app.
   3. If the user instructions include statements about prioritization, use those instructions.
   4. If the user instructions do not include statements about prioritization, 
   then prioritize the returned issues using the following criteria:
@@ -73,8 +76,9 @@ Follow these steps to fetch issues and prioritize them.
 Follow these steps to diagnose and fix issues.
 
   1. Make sure you have a good understanding of the code structure and where different functionality exists
-  2. Use the 'crashlytics_get_issue_details' tool to get more context on the issue.
-  3. Use the 'crashlytics_get_sample_crash_for_issue' tool to get 3 example crashes for this issue.
+  2. Use the 'crashlytics_get_issue' tool to get more context on the issue.
+  3. Use the 'crashlytics_list_events' tool to get an example crash for this issue.
+    3a. Apply the same filtering criteria that you used to find the issue, so that you find an appropriate event.
   4. Read the files that exist in the stack trace of the issue to understand the crash deeply.
   5. Determine the root cause of the crash.
   6. Write out a plan using the following criteria:

src/mcp/tools/crashlytics/notes.ts

@@ -66,7 +66,8 @@ export const delete_note = tool(
     }),
     annotations: {
       title: "Delete Crashlytics Issue Note",
-      readOnlyHint: true,
+      readOnlyHint: false,
+      destructiveHint: true,
     },
     _meta: {
       requiresAuth: true,

src/mcp/tools/crashlytics/reports.ts

@@ -16,6 +16,10 @@ function getReportContent(
   return async ({ appId, filter, pageSize }) => {
     if (!appId) return mcpError(`Must specify 'appId' parameter.`);
     filter ??= {};
+    if (!!filter.intervalStartTime && !filter.intervalEndTime) {
+      // interval.end_time is required if interval.start_time is set but the agent likes to forget it
+      filter.intervalEndTime = new Date().toISOString();
+    }
     return toContent(await getReport(report, appId, filter, pageSize));
   };
 }