From 580ce32a02d57dd55b4f593c53029e62cce8a784 Mon Sep 17 00:00:00 2001 From: Max Lord Date: Tue, 16 Sep 2025 11:31:26 -0400 Subject: [PATCH 1/2] feat: Improving instructions and return values in crashlytics tools - Making interval handling more robust - Adding return messages to avoid undefined tool responses --- src/crashlytics/filters.ts | 10 ++++++++-- src/crashlytics/issues.ts | 6 +++--- src/crashlytics/notes.ts | 3 ++- src/mcp/prompts/crashlytics/connect.ts | 12 ++++++++---- src/mcp/tools/crashlytics/notes.ts | 3 ++- src/mcp/tools/crashlytics/reports.ts | 4 ++++ 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/crashlytics/filters.ts b/src/crashlytics/filters.ts index 2d64ea08cc1..f3ceb26120c 100644 --- a/src/crashlytics/filters.ts +++ b/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() diff --git a/src/crashlytics/issues.ts b/src/crashlytics/issues.ts index e6f500a43d8..e6db43975bc 100644 --- a/src/crashlytics/issues.ts +++ b/src/crashlytics/issues.ts @@ -38,13 +38,13 @@ export async function getIssue(appId: string, issueId: string): Promise { * @param state State.OPEN or State.CLOSED * @return An updated Issue */ -export async function updateIssue(appId: string, issueId: string, state: State): Promise { +export async function updateIssue(appId: string, issueId: string, state: State): Promise { const requestProjectNumber = parseProjectNumber(appId); try { logger.debug( `[crashlytics] updateIssue called with appId: ${appId}, issueId: ${issueId}, state: ${state}`, ); - const response = await CRASHLYTICS_API_CLIENT.request({ + await CRASHLYTICS_API_CLIENT.request({ method: "PATCH", headers: { "Content-Type": "application/json", @@ -55,7 +55,7 @@ export async function updateIssue(appId: string, issueId: string, state: State): timeout: TIMEOUT, }); - return response.body; + return `Issue ${issueId} state changed to ${state}`; } catch (err: unknown) { throw new FirebaseError(`Failed to update issue ${issueId} for app ${appId}`, { original: getError(err), diff --git a/src/crashlytics/notes.ts b/src/crashlytics/notes.ts index 66684173a73..79c178507c8 100644 --- a/src/crashlytics/notes.ts +++ b/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 { +export async function deleteNote(appId: string, issueId: string, noteId: string): Promise { 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}`, diff --git a/src/mcp/prompts/crashlytics/connect.ts b/src/mcp/prompts/crashlytics/connect.ts index 24549469833..43b3ed59d3b 100644 --- a/src/mcp/prompts/crashlytics/connect.ts +++ b/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: diff --git a/src/mcp/tools/crashlytics/notes.ts b/src/mcp/tools/crashlytics/notes.ts index f28c18b4db6..f312d415b63 100644 --- a/src/mcp/tools/crashlytics/notes.ts +++ b/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, diff --git a/src/mcp/tools/crashlytics/reports.ts b/src/mcp/tools/crashlytics/reports.ts index 87d52930187..b241b926051 100644 --- a/src/mcp/tools/crashlytics/reports.ts +++ b/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)); }; } From 6837f9f18da3c2de29ffc9468a4039633667a9ea Mon Sep 17 00:00:00 2001 From: Max Lord Date: Wed, 17 Sep 2025 16:08:44 -0400 Subject: [PATCH 2/2] (fix) Fixing method signatures in crashlytics tools --- src/crashlytics/events.spec.ts | 52 +++++++++++++++++++++++++++++++++- src/crashlytics/events.ts | 43 +++++++++++++++++++++++++++- src/crashlytics/issues.spec.ts | 2 +- src/crashlytics/issues.ts | 8 +++--- src/crashlytics/types.ts | 28 +++++++++++------- 5 files changed, 116 insertions(+), 17 deletions(-) diff --git a/src/crashlytics/events.spec.ts b/src/crashlytics/events.spec.ts index a6b74e11854..0c7c45afb0a 100644 --- a/src/crashlytics/events.spec.ts +++ b/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", + ); + }); + }); }); diff --git a/src/crashlytics/events.ts b/src/crashlytics/events.ts index d1352e3d0fa..0727ef26d73 100644 --- a/src/crashlytics/events.ts +++ b/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 { + 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({ + 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), + }); + } +} diff --git a/src/crashlytics/issues.spec.ts b/src/crashlytics/issues.spec.ts index 277c25fc9ba..aca3f8a831e 100644 --- a/src/crashlytics/issues.spec.ts +++ b/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); diff --git a/src/crashlytics/issues.ts b/src/crashlytics/issues.ts index e6db43975bc..b3881b030ad 100644 --- a/src/crashlytics/issues.ts +++ b/src/crashlytics/issues.ts @@ -38,24 +38,24 @@ export async function getIssue(appId: string, issueId: string): Promise { * @param state State.OPEN or State.CLOSED * @return An updated Issue */ -export async function updateIssue(appId: string, issueId: string, state: State): Promise { +export async function updateIssue(appId: string, issueId: string, state: State): Promise { const requestProjectNumber = parseProjectNumber(appId); try { logger.debug( `[crashlytics] updateIssue called with appId: ${appId}, issueId: ${issueId}, state: ${state}`, ); - await CRASHLYTICS_API_CLIENT.request({ + const response = await CRASHLYTICS_API_CLIENT.request({ method: "PATCH", headers: { "Content-Type": "application/json", }, path: `/projects/${requestProjectNumber}/apps/${appId}/issues/${issueId}`, queryParams: { updateMask: "state" }, - body: { issue: { state } }, + body: { state }, timeout: TIMEOUT, }); - return `Issue ${issueId} state changed to ${state}`; + return response.body; } catch (err: unknown) { throw new FirebaseError(`Failed to update issue ${issueId} for app ${appId}`, { original: getError(err), diff --git a/src/crashlytics/types.ts b/src/crashlytics/types.ts index 97f266aa809..20d7993c5ed 100644 --- a/src/crashlytics/types.ts +++ b/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; }