From 67259e41d67fa939f9cfdf3efe3c60a3f9fd3c8b Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 29 Apr 2025 10:47:33 +0200 Subject: [PATCH 1/2] chore: add type check for CI We have been running lint which did catch most TypeScript errors but there's more complex type issues which arise from the TypeScript compiler itself. This adds a type check step and fixes existing issues with it. --- .vscode/launch.json | 2 +- eslint.config.js | 2 +- package-lock.json | 6 +- package.json | 5 +- scripts/apply.ts | 1 + scripts/filter.ts | 3 + src/common/atlas/apiClient.ts | 4 +- src/server.ts | 2 +- src/telemetry/eventCache.ts | 2 +- src/telemetry/telemetry.ts | 7 ++- src/telemetry/types.ts | 56 +++++++++---------- src/tools/tool.ts | 1 - tests/integration/inMemoryTransport.ts | 6 +- tests/integration/tools/atlas/atlasHelpers.ts | 2 +- tests/unit/telemetry.test.ts | 21 +++++-- tsconfig.build.json | 19 +++++++ tsconfig.jest.json | 2 +- tsconfig.json | 20 ++----- tsconfig.lint.json | 8 --- 19 files changed, 93 insertions(+), 76 deletions(-) create mode 100644 tsconfig.build.json delete mode 100644 tsconfig.lint.json diff --git a/.vscode/launch.json b/.vscode/launch.json index a55e49aca..969a92f20 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,7 +10,7 @@ "name": "Launch Program", "skipFiles": ["/**"], "program": "${workspaceFolder}/dist/index.js", - "preLaunchTask": "tsc: build - tsconfig.json", + "preLaunchTask": "tsc: build - tsconfig.build.json", "outFiles": ["${workspaceFolder}/dist/**/*.js"] } ] diff --git a/eslint.config.js b/eslint.config.js index 072bef1d2..c46b8bd42 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -29,7 +29,7 @@ export default defineConfig([ files, languageOptions: { parserOptions: { - project: "./tsconfig.lint.json", + project: "./tsconfig.json", tsconfigRootDir: import.meta.dirname, }, }, diff --git a/package-lock.json b/package-lock.json index a19b9da25..96df7533d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -18,6 +18,7 @@ "mongodb-log-writer": "^2.4.1", "mongodb-redact": "^1.1.6", "mongodb-schema": "^12.6.2", + "native-machine-id": "^0.1.0", "openapi-fetch": "^0.13.5", "simple-oauth2": "^5.1.0", "yargs-parser": "^21.1.1", @@ -44,7 +45,6 @@ "jest-environment-node": "^29.7.0", "jest-extended": "^4.0.2", "mongodb-runner": "^5.8.2", - "native-machine-id": "^0.1.0", "openapi-types": "^12.1.3", "openapi-typescript": "^7.6.1", "prettier": "^3.5.3", @@ -6467,7 +6467,6 @@ "version": "1.5.0", "resolved": "https://registry.npmjs.org/bindings/-/bindings-1.5.0.tgz", "integrity": "sha512-p2q/t/mhvuOj/UeLlV6566GD/guowlr0hHxClI0W9m7MWYkL1F0hLo+0Aexs9HSPCtR1SXQ0TD3MMKrXZajbiQ==", - "devOptional": true, "license": "MIT", "dependencies": { "file-uri-to-path": "1.0.0" @@ -8787,7 +8786,6 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/file-uri-to-path/-/file-uri-to-path-1.0.0.tgz", "integrity": "sha512-0Zt+s3L7Vf1biwWZ29aARiVYLx7iMGnEUl9x33fbB/j3jR81u/O2LbqK+Bm1CDSNDKVtJ/YjwY7TUd5SkeLQLw==", - "devOptional": true, "license": "MIT" }, "node_modules/filelist": { @@ -11474,7 +11472,6 @@ "version": "0.1.0", "resolved": "https://registry.npmjs.org/native-machine-id/-/native-machine-id-0.1.0.tgz", "integrity": "sha512-Po7OPcXGsWZ/o+n93ZOhmF3G5RQsEUMTnVddX45u5GfoEnk803ba7lhztwMkDaPhUFHy5FpXLiytIFitVxMkTA==", - "dev": true, "hasInstallScript": true, "license": "Apache-2.0", "dependencies": { @@ -11489,7 +11486,6 @@ "version": "8.3.1", "resolved": "https://registry.npmjs.org/node-addon-api/-/node-addon-api-8.3.1.tgz", "integrity": "sha512-lytcDEdxKjGJPTLEfW4mYMigRezMlyJY8W4wxJK8zE533Jlb8L8dRuObJFWg2P+AuOIxoCgKF+2Oq4d4Zd0OUA==", - "dev": true, "license": "MIT", "engines": { "node": "^18 || ^20 || >= 21" diff --git a/package.json b/package.json index 5cd1c8557..a0090a403 100644 --- a/package.json +++ b/package.json @@ -18,14 +18,15 @@ "scripts": { "prepare": "npm run build", "build:clean": "rm -rf dist", - "build:compile": "tsc", + "build:compile": "tsc --project tsconfig.build.json", "build:chmod": "chmod +x dist/index.js", "build": "npm run build:clean && npm run build:compile && npm run build:chmod", "inspect": "npm run build && mcp-inspector -- dist/index.js", "prettier": "prettier", - "check": "npm run build && npm run check:lint && npm run check:format", + "check": "npm run build && npm run check:types && npm run check:lint && npm run check:format", "check:lint": "eslint .", "check:format": "prettier -c .", + "check:types": "tsc --noEmit --project tsconfig.json", "reformat": "prettier --write .", "generate": "./scripts/generate.sh", "test": "jest --coverage" diff --git a/scripts/apply.ts b/scripts/apply.ts index fa2a6917e..225fd304a 100755 --- a/scripts/apply.ts +++ b/scripts/apply.ts @@ -44,6 +44,7 @@ async function main() { const openapi = JSON.parse(specFile) as OpenAPIV3_1.Document; for (const path in openapi.paths) { for (const method in openapi.paths[path]) { + // @ts-expect-error This is a workaround for the OpenAPI types const operation = openapi.paths[path][method] as OpenAPIV3_1.OperationObject; if (!operation.operationId || !operation.tags?.length) { diff --git a/scripts/filter.ts b/scripts/filter.ts index 4dcdbdcc0..0146d072c 100755 --- a/scripts/filter.ts +++ b/scripts/filter.ts @@ -43,11 +43,14 @@ function filterOpenapi(openapi: OpenAPIV3_1.Document): OpenAPIV3_1.Document { for (const path in openapi.paths) { const filteredMethods = {} as OpenAPIV3_1.PathItemObject; for (const method in openapi.paths[path]) { + // @ts-expect-error This is a workaround for the OpenAPI types if (allowedOperations.includes((openapi.paths[path][method] as { operationId: string }).operationId)) { + // @ts-expect-error This is a workaround for the OpenAPI types filteredMethods[method] = openapi.paths[path][method] as OpenAPIV3_1.OperationObject; } } if (Object.keys(filteredMethods).length > 0) { + // @ts-expect-error This is a workaround for the OpenAPI types filteredPaths[path] = filteredMethods; } } diff --git a/src/common/atlas/apiClient.ts b/src/common/atlas/apiClient.ts index 34e1a0e73..3633e6326 100644 --- a/src/common/atlas/apiClient.ts +++ b/src/common/atlas/apiClient.ts @@ -3,7 +3,7 @@ import type { FetchOptions } from "openapi-fetch"; import { AccessToken, ClientCredentials } from "simple-oauth2"; import { ApiClientError } from "./apiClientError.js"; import { paths, operations } from "./openapi.js"; -import { BaseEvent } from "../../telemetry/types.js"; +import { CommonProperties, TelemetryEvent } from "../../telemetry/types.js"; import { packageInfo } from "../../packageInfo.js"; const ATLAS_API_VERSION = "2025-03-12"; @@ -123,7 +123,7 @@ export class ApiClient { }>; } - async sendEvents(events: BaseEvent[]): Promise { + async sendEvents(events: TelemetryEvent[]): Promise { let endpoint = "api/private/unauth/telemetry/events"; const headers: Record = { Accept: "application/json", diff --git a/src/server.ts b/src/server.ts index a105b33f4..f10e89318 100644 --- a/src/server.ts +++ b/src/server.ts @@ -119,7 +119,7 @@ export class Server { if (command === "start") { event.properties.startup_time_ms = commandDuration; event.properties.read_only_mode = this.userConfig.readOnly || false; - event.properties.disallowed_tools = this.userConfig.disabledTools || []; + event.properties.disabled_tools = this.userConfig.disabledTools || []; } if (command === "stop") { event.properties.runtime_duration_ms = Date.now() - this.startTime; diff --git a/src/telemetry/eventCache.ts b/src/telemetry/eventCache.ts index 49025227a..141e9b78a 100644 --- a/src/telemetry/eventCache.ts +++ b/src/telemetry/eventCache.ts @@ -13,7 +13,7 @@ export class EventCache { private cache: LRUCache; private nextId = 0; - private constructor() { + constructor() { this.cache = new LRUCache({ max: EventCache.MAX_EVENTS, // Using FIFO eviction strategy for events diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 9b5986af3..ba12dd085 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -113,7 +113,12 @@ export class Telemetry { */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise { try { - await client.sendEvents(events); + await client.sendEvents( + events.map((event) => ({ + ...event, + properties: { ...event.properties, ...this.getCommonProperties() }, + })) + ); return { success: true }; } catch (error) { return { diff --git a/src/telemetry/types.ts b/src/telemetry/types.ts index 5199590f2..76e1d4ae5 100644 --- a/src/telemetry/types.ts +++ b/src/telemetry/types.ts @@ -8,49 +8,46 @@ export type TelemetryBoolSet = "true" | "false"; /** * Base interface for all events */ -export interface Event { +export type TelemetryEvent = { timestamp: string; source: "mdbmcp"; - properties: Record; -} - -export interface BaseEvent extends Event { - properties: CommonProperties & { + properties: T & { component: string; duration_ms: number; result: TelemetryResult; category: string; - } & Event["properties"]; -} + }; +}; + +export type BaseEvent = TelemetryEvent; /** * Interface for tool events */ -export interface ToolEvent extends BaseEvent { - properties: { - command: string; - error_code?: string; - error_type?: string; - project_id?: string; - org_id?: string; - cluster_name?: string; - is_atlas?: boolean; - } & BaseEvent["properties"]; -} +export type ToolEventProperties = { + command: string; + error_code?: string; + error_type?: string; + project_id?: string; + org_id?: string; + cluster_name?: string; + is_atlas?: boolean; +}; +export type ToolEvent = TelemetryEvent; /** * Interface for server events */ -export interface ServerEvent extends BaseEvent { - properties: { - command: ServerCommand; - reason?: string; - startup_time_ms?: number; - runtime_duration_ms?: number; - read_only_mode?: boolean; - disabled_tools?: string[]; - } & BaseEvent["properties"]; -} +export type ServerEventProperties = { + command: ServerCommand; + reason?: string; + startup_time_ms?: number; + runtime_duration_ms?: number; + read_only_mode?: boolean; + disabled_tools?: string[]; +}; + +export type ServerEvent = TelemetryEvent; /** * Interface for static properties, they can be fetched once and reused. @@ -69,6 +66,7 @@ export type CommonStaticProperties = { * Common properties for all events that might change. */ export type CommonProperties = { + device_id?: string; mcp_client_version?: string; mcp_client_name?: string; config_atlas_auth?: TelemetryBoolSet; diff --git a/src/tools/tool.ts b/src/tools/tool.ts index f8091debe..d7ea909ec 100644 --- a/src/tools/tool.ts +++ b/src/tools/tool.ts @@ -43,7 +43,6 @@ export abstract class ToolBase { timestamp: new Date().toISOString(), source: "mdbmcp", properties: { - ...this.telemetry.getCommonProperties(), command: this.name, category: this.category, component: "tool", diff --git a/tests/integration/inMemoryTransport.ts b/tests/integration/inMemoryTransport.ts index c46f87a3a..daaf577ae 100644 --- a/tests/integration/inMemoryTransport.ts +++ b/tests/integration/inMemoryTransport.ts @@ -2,7 +2,7 @@ import { Transport } from "@modelcontextprotocol/sdk/shared/transport.js"; import { JSONRPCMessage } from "@modelcontextprotocol/sdk/types.js"; export class InMemoryTransport implements Transport { - private outputController: ReadableStreamDefaultController; + private outputController: ReadableStreamDefaultController | undefined; private startPromise: Promise; @@ -35,13 +35,13 @@ export class InMemoryTransport implements Transport { } send(message: JSONRPCMessage): Promise { - this.outputController.enqueue(message); + this.outputController?.enqueue(message); return Promise.resolve(); } // eslint-disable-next-line @typescript-eslint/require-await async close(): Promise { - this.outputController.close(); + this.outputController?.close(); this.onclose?.(); } onclose?: (() => void) | undefined; diff --git a/tests/integration/tools/atlas/atlasHelpers.ts b/tests/integration/tools/atlas/atlasHelpers.ts index 86cf43df9..d66a4041c 100644 --- a/tests/integration/tools/atlas/atlasHelpers.ts +++ b/tests/integration/tools/atlas/atlasHelpers.ts @@ -74,7 +74,7 @@ export function parseTable(text: string): Record[] { return data .filter((_, index) => index >= 2) .map((cells) => { - const row = {}; + const row: Record = {}; cells.forEach((cell, index) => { row[headers[index]] = cell; }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 525aa59fc..06d098e8b 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -21,16 +21,23 @@ describe("Telemetry", () => { // Helper function to create properly typed test events function createTestEvent(options?: { - source?: string; result?: TelemetryResult; component?: string; category?: string; command?: string; duration_ms?: number; - }): BaseEvent { + }): Omit & { + properties: { + component: string; + duration_ms: number; + result: TelemetryResult; + category: string; + command: string; + }; + } { return { timestamp: new Date().toISOString(), - source: options?.source || "mdbmcp", + source: "mdbmcp", properties: { component: options?.component || "test-component", duration_ms: options?.duration_ms || 100, @@ -48,6 +55,12 @@ describe("Telemetry", () => { appendEventsCalls = 0, sendEventsCalledWith = undefined, appendEventsCalledWith = undefined, + }: { + sendEventsCalls?: number; + clearEventsCalls?: number; + appendEventsCalls?: number; + sendEventsCalledWith?: BaseEvent[] | undefined; + appendEventsCalledWith?: BaseEvent[] | undefined; } = {}) { const { calls: sendEvents } = mockApiClient.sendEvents.mock; const { calls: clearEvents } = mockEventCache.clearEvents.mock; @@ -71,7 +84,7 @@ describe("Telemetry", () => { jest.clearAllMocks(); // Setup mocked API client - mockApiClient = new MockApiClient() as jest.Mocked; + mockApiClient = new MockApiClient({ baseUrl: "" }) as jest.Mocked; mockApiClient.sendEvents = jest.fn().mockResolvedValue(undefined); mockApiClient.hasCredentials = jest.fn().mockReturnValue(true); diff --git a/tsconfig.build.json b/tsconfig.build.json new file mode 100644 index 000000000..dd65f91d6 --- /dev/null +++ b/tsconfig.build.json @@ -0,0 +1,19 @@ +{ + "compilerOptions": { + "target": "es2020", + "module": "nodenext", + "moduleResolution": "nodenext", + "rootDir": "./src", + "outDir": "./dist", + "strict": true, + "strictNullChecks": true, + "esModuleInterop": true, + "types": ["node", "jest"], + "sourceMap": true, + "skipLibCheck": true, + "resolveJsonModule": true, + "allowSyntheticDefaultImports": true, + "typeRoots": ["./node_modules/@types", "./src/types"] + }, + "include": ["src/**/*.ts"] +} diff --git a/tsconfig.jest.json b/tsconfig.jest.json index a53ca4844..ad44307bb 100644 --- a/tsconfig.jest.json +++ b/tsconfig.jest.json @@ -1,5 +1,5 @@ { - "extends": "./tsconfig.json", + "extends": "./tsconfig.build.json", "compilerOptions": { "module": "esnext", "target": "esnext", diff --git a/tsconfig.json b/tsconfig.json index dd65f91d6..977d46fdd 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,19 +1,9 @@ { + "extends": "./tsconfig.build.json", "compilerOptions": { - "target": "es2020", - "module": "nodenext", - "moduleResolution": "nodenext", - "rootDir": "./src", - "outDir": "./dist", - "strict": true, - "strictNullChecks": true, - "esModuleInterop": true, - "types": ["node", "jest"], - "sourceMap": true, - "skipLibCheck": true, - "resolveJsonModule": true, - "allowSyntheticDefaultImports": true, - "typeRoots": ["./node_modules/@types", "./src/types"] + "rootDir": ".", + "types": ["jest"], + "skipLibCheck": true }, - "include": ["src/**/*.ts"] + "include": ["**/*"] } diff --git a/tsconfig.lint.json b/tsconfig.lint.json deleted file mode 100644 index 5b14e4709..000000000 --- a/tsconfig.lint.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "extends": "./tsconfig.json", - "compilerOptions": { - "rootDir": ".", - "types": ["jest"] - }, - "include": ["**/*"] -} From d5587a413a9d805de984b0a48782398d5577bcb1 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 29 Apr 2025 10:59:57 +0200 Subject: [PATCH 2/2] fix: adjust tests --- src/server.ts | 1 - src/telemetry/telemetry.ts | 2 +- tests/unit/telemetry.test.ts | 10 +++++++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/server.ts b/src/server.ts index f10e89318..b11ba31d0 100644 --- a/src/server.ts +++ b/src/server.ts @@ -107,7 +107,6 @@ export class Server { timestamp: new Date().toISOString(), source: "mdbmcp", properties: { - ...this.telemetry.getCommonProperties(), result: "success", duration_ms: commandDuration, component: "server", diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index ba12dd085..534312323 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -116,7 +116,7 @@ export class Telemetry { await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...event.properties, ...this.getCommonProperties() }, + properties: { ...this.getCommonProperties(), ...event.properties }, })) ); return { success: true }; diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 06d098e8b..5b37da8ed 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -71,7 +71,15 @@ describe("Telemetry", () => { expect(appendEvents.length).toBe(appendEventsCalls); if (sendEventsCalledWith) { - expect(sendEvents[0]?.[0]).toEqual(sendEventsCalledWith); + expect(sendEvents[0]?.[0]).toEqual( + sendEventsCalledWith.map((event) => ({ + ...event, + properties: { + ...telemetry.getCommonProperties(), + ...event.properties, + }, + })) + ); } if (appendEventsCalledWith) {