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 a55e49ac..969a92f2 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 072bef1d..c46b8bd4 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 a19b9da2..96df7533 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 5cd1c855..a0090a40 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 fa2a6917..225fd304 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 4dcdbdcc..0146d072 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 34e1a0e7..3633e632 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 a105b33f..f10e8931 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 49025227..141e9b78 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 9b5986af..ba12dd08 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 5199590f..76e1d4ae 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 f8091deb..d7ea909e 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 c46f87a3..daaf577a 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 86cf43df..d66a4041 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 525aa59f..06d098e8 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 00000000..dd65f91d --- /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 a53ca484..ad44307b 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 dd65f91d..977d46fd 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 5b14e470..00000000 --- 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 f10e8931..b11ba31d 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 ba12dd08..53431232 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 06d098e8..5b37da8e 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) {