From 8c4abd288c49769d0181a80ad48261cbcb1e85c7 Mon Sep 17 00:00:00 2001 From: Savio Dias Date: Thu, 17 Sep 2026 15:28:59 +0530 Subject: [PATCH] fix(tools): record telemetry for setupBrowserStackAppAutomateTests and createTestCasesFromFile Two tools were invisible in MCPInstrumentation: - setupBrowserStackAppAutomateTests never called trackMCP on either path. Add the standard entry (success) and catch (failure) calls. - createTestCasesFromFile called trackMCP without config, so the event went out with no Authorization header and Rails dropped it. Pass config on both paths. Tests assert both paths call trackMCP with the tool name, client info, error slot, and config, so a regression to the config-in-error-slot shape (PR #424) is caught. Co-Authored-By: Claude Fable 5.1 --- src/tools/appautomate.ts | 12 ++++ src/tools/testmanagement.ts | 8 ++- tests/tools/appautomate-sdk-tool.test.ts | 77 ++++++++++++++++++++++++ tests/tools/testmanagement.test.ts | 7 +++ 4 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 tests/tools/appautomate-sdk-tool.test.ts diff --git a/src/tools/appautomate.ts b/src/tools/appautomate.ts index 0fbd0e29..2cc7e4ea 100644 --- a/src/tools/appautomate.ts +++ b/src/tools/appautomate.ts @@ -435,8 +435,20 @@ export default function addAppAutomationTools( }, async (args) => { try { + trackMCP( + "setupBrowserStackAppAutomateTests", + server.server.getClientVersion()!, + undefined, + config, + ); return await setupAppAutomateHandler(args, config); } catch (error) { + trackMCP( + "setupBrowserStackAppAutomateTests", + server.server.getClientVersion()!, + error, + config, + ); const error_message = error instanceof Error ? error.message : "Unknown error"; return { diff --git a/src/tools/testmanagement.ts b/src/tools/testmanagement.ts index f13061a6..43ec2aac 100644 --- a/src/tools/testmanagement.ts +++ b/src/tools/testmanagement.ts @@ -488,10 +488,16 @@ export async function createTestCasesFromFileTool( "createTestCasesFromFile", server.server.getClientVersion()!, undefined, + config, ); return await createTestCasesFromFile(args, context, config); } catch (err) { - trackMCP("createTestCasesFromFile", server.server.getClientVersion()!, err); + trackMCP( + "createTestCasesFromFile", + server.server.getClientVersion()!, + err, + config, + ); return { content: [ { diff --git a/tests/tools/appautomate-sdk-tool.test.ts b/tests/tools/appautomate-sdk-tool.test.ts new file mode 100644 index 00000000..7ce82c3b --- /dev/null +++ b/tests/tools/appautomate-sdk-tool.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import addAppAutomationTools from "../../src/tools/appautomate"; +import { setupAppAutomateHandler } from "../../src/tools/appautomate-utils/appium-sdk/handler"; +import { trackMCP } from "../../src/lib/instrumentation"; + +vi.mock("../../src/tools/appautomate-utils/appium-sdk/handler", () => ({ + setupAppAutomateHandler: vi.fn(), +})); +vi.mock("webdriverio", () => ({ remote: vi.fn() })); +vi.mock("../../src/logger", () => ({ + default: { error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); +vi.mock("../../src/lib/instrumentation", () => ({ trackMCP: vi.fn() })); + +const mockConfig = { + "browserstack-username": "fake-user", + "browserstack-access-key": "fake-key", +}; +const clientVersion = { name: "test-client", version: "1.0" }; + +describe("setupBrowserStackAppAutomateTests telemetry", () => { + let serverMock: any; + + beforeEach(() => { + vi.clearAllMocks(); + serverMock = { + tool: vi.fn((...toolArgs: any[]) => { + const name = toolArgs[0]; + const handler = toolArgs[toolArgs.length - 1]; + serverMock.handlers = serverMock.handlers || {}; + serverMock.handlers[name] = handler; + }), + server: { getClientVersion: vi.fn().mockReturnValue(clientVersion) }, + }; + addAppAutomationTools(serverMock, mockConfig as any); + }); + + const args = { + language: "java", + test_framework: "testng", + app_platform: "android", + app_path: "/path/app.apk", + }; + + it("records a success event with config on the success path", async () => { + (setupAppAutomateHandler as any).mockResolvedValue({ + content: [{ type: "text", text: "ok" }], + }); + + await serverMock.handlers["setupBrowserStackAppAutomateTests"](args); + + expect(trackMCP).toHaveBeenCalledTimes(1); + expect(trackMCP).toHaveBeenCalledWith( + "setupBrowserStackAppAutomateTests", + clientVersion, + undefined, + mockConfig, + ); + }); + + it("records a failure event with the error and config on throw", async () => { + const boom = new Error("handler exploded"); + (setupAppAutomateHandler as any).mockRejectedValue(boom); + + const result = + await serverMock.handlers["setupBrowserStackAppAutomateTests"](args); + + expect(result.isError).toBe(true); + expect(trackMCP).toHaveBeenCalledTimes(2); + expect(trackMCP).toHaveBeenLastCalledWith( + "setupBrowserStackAppAutomateTests", + clientVersion, + boom, + mockConfig, + ); + }); +}); diff --git a/tests/tools/testmanagement.test.ts b/tests/tools/testmanagement.test.ts index 73a32706..29c636bf 100644 --- a/tests/tools/testmanagement.test.ts +++ b/tests/tools/testmanagement.test.ts @@ -31,6 +31,7 @@ import { createLCASteps } from '../../src/tools/testmanagement-utils/create-lca- import axios from 'axios'; import { beforeAll, beforeEach, it, expect, describe, Mocked} from 'vitest'; import { vi, Mock } from 'vitest'; +import { trackMCP } from '../../src/lib/instrumentation'; import { signedUrlMap } from '../../src/lib/inmemory-store'; import { uploadFile } from '../../src/tools/testmanagement-utils/upload-file'; @@ -598,6 +599,10 @@ describe("createTestCasesFromFileTool", () => { const res = await createTestCasesFromFileTool(args as any, mockContext, mockConfig, mockServer); expect(res.isError).toBe(true); expect(res.content?.[0]?.text).toContain("Re-Upload the file"); + // Both telemetry calls must carry config, otherwise the event is sent unauthenticated and dropped. + expect(trackMCP).toHaveBeenCalledTimes(2); + expect(trackMCP).toHaveBeenNthCalledWith(1, "createTestCasesFromFile", "test-version", undefined, mockConfig); + expect(trackMCP).toHaveBeenNthCalledWith(2, "createTestCasesFromFile", "test-version", expect.any(Error), mockConfig); }); it("creates test cases from a file successfully", async () => { signedUrlMap.set(testDocumentId, { fileId: mockFileId, downloadUrl: mockDownloadUrl }); @@ -631,6 +636,8 @@ describe("createTestCasesFromFileTool", () => { const res = await createTestCasesFromFileTool(args as any, mockContext, mockConfig, mockServer); expect(res.isError ?? false).toBe(false); expect(res.content?.[0]?.text).toContain("test cases created"); + expect(trackMCP).toHaveBeenCalledTimes(1); + expect(trackMCP).toHaveBeenCalledWith("createTestCasesFromFile", "test-version", undefined, mockConfig); }); });