From 8184f73c14db06ea74c9081a4512f1fdc7c114d3 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 13 Oct 2025 14:55:25 +0200 Subject: [PATCH 01/17] feat: introduces drop-search-index tool - Tool is currently only visible to tests - Tool has been added to the list of tools requiring elicitation - Tests have been added for basic tool functionality and elicitation - listSearchIndexes tool and its tests have been modified to re-use some of the existing code. --- src/common/config.ts | 1 + src/tools/mongodb/delete/dropSearchIndex.ts | 79 +++++++++ src/tools/mongodb/search/listSearchIndexes.ts | 49 +++--- src/tools/mongodb/tools.ts | 2 + tests/integration/helpers.ts | 78 +++++++++ .../mongodb/delete/dropSearchIndex.test.ts | 154 ++++++++++++++++++ .../tools/mongodb/mongodbClusterProcess.ts | 3 +- .../mongodb/search/listSearchIndexes.test.ts | 36 ++-- 8 files changed, 360 insertions(+), 42 deletions(-) create mode 100644 src/tools/mongodb/delete/dropSearchIndex.ts create mode 100644 tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts diff --git a/src/common/config.ts b/src/common/config.ts index b7bf527b..6011e5a5 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -204,6 +204,7 @@ export const defaultUserConfig: UserConfig = { "drop-collection", "delete-many", "drop-index", + "drop-search-index", ], transport: "stdio", httpPort: 3000, diff --git a/src/tools/mongodb/delete/dropSearchIndex.ts b/src/tools/mongodb/delete/dropSearchIndex.ts new file mode 100644 index 00000000..562a18eb --- /dev/null +++ b/src/tools/mongodb/delete/dropSearchIndex.ts @@ -0,0 +1,79 @@ +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { CommonArgs } from "../../args.js"; +import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; +import { type OperationType, type ToolArgs } from "../../tool.js"; +import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; + +export class DropSearchIndexTool extends MongoDBToolBase { + public name = "drop-search-index"; + protected description = "Drop a search index or vector search index for the provided database and collection."; + protected argsShape = { + ...DbOperationArgs, + indexName: CommonArgs.string() + .nonempty() + .describe("The name of the search or vector search index to be dropped."), + }; + public operationType: OperationType = "delete"; + + protected override verifyAllowed(): boolean { + // Only enable this on tests for now. + return process.env.VITEST === "true"; + } + + protected override async execute({ + database, + collection, + indexName, + }: ToolArgs): Promise { + const provider = await this.ensureConnected(); + const searchIndexes = await ListSearchIndexesTool.getSearchIndexes(provider, database, collection); + const indexDoesNotExist = !searchIndexes.find((index) => index.name === indexName); + if (indexDoesNotExist) { + return { + content: [ + { + type: "text", + text: `Index with name "${indexName}" does not exist in the provided namespace "${database}.${collection}".`, + }, + ], + isError: true, + }; + } + + await provider.dropSearchIndex(database, collection, indexName); + return { + content: [ + { + type: "text", + text: `Successfully dropped the index with name "${indexName}" from the provided namespace "${database}.${collection}".`, + }, + ], + }; + } + + protected getConfirmationMessage({ database, collection, indexName }: ToolArgs): string { + return ( + `You are about to drop the \`${indexName}\` index from the \`${database}.${collection}\` namespace:\n\n` + + "This operation will permanently remove the index and might affect the performance of queries relying on this index.\n\n" + + "**Do you confirm the execution of the action?**" + ); + } + + protected handleError( + error: unknown, + args: ToolArgs + ): Promise | CallToolResult { + if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { + return { + content: [ + { + text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", + type: "text", + isError: true, + }, + ], + }; + } + return super.handleError(error, args); + } +} diff --git a/src/tools/mongodb/search/listSearchIndexes.ts b/src/tools/mongodb/search/listSearchIndexes.ts index 1b520d52..cb55b882 100644 --- a/src/tools/mongodb/search/listSearchIndexes.ts +++ b/src/tools/mongodb/search/listSearchIndexes.ts @@ -1,10 +1,11 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import type { ToolArgs, OperationType } from "../../tool.js"; import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; import { formatUntrustedData } from "../../tool.js"; import { EJSON } from "bson"; -export type SearchIndexStatus = { +export type SearchIndexWithStatus = { name: string; type: string; status: string; @@ -20,14 +21,13 @@ export class ListSearchIndexesTool extends MongoDBToolBase { protected async execute({ database, collection }: ToolArgs): Promise { const provider = await this.ensureConnected(); - const indexes = await provider.getSearchIndexes(database, collection); - const trimmedIndexDefinitions = this.pickRelevantInformation(indexes); + const searchIndexes = await ListSearchIndexesTool.getSearchIndexes(provider, database, collection); - if (trimmedIndexDefinitions.length > 0) { + if (searchIndexes.length > 0) { return { content: formatUntrustedData( - `Found ${trimmedIndexDefinitions.length} search and vector search indexes in ${database}.${collection}`, - trimmedIndexDefinitions.map((index) => EJSON.stringify(index)).join("\n") + `Found ${searchIndexes.length} search and vector search indexes in ${database}.${collection}`, + searchIndexes.map((index) => EJSON.stringify(index)).join("\n") ), }; } else { @@ -45,22 +45,6 @@ export class ListSearchIndexesTool extends MongoDBToolBase { return process.env.VITEST === "true"; } - /** - * Atlas Search index status contains a lot of information that is not relevant for the agent at this stage. - * Like for example, the status on each of the dedicated nodes. We only care about the main status, if it's - * queryable and the index name. We are also picking the index definition as it can be used by the agent to - * understand which fields are available for searching. - **/ - protected pickRelevantInformation(indexes: Record[]): SearchIndexStatus[] { - return indexes.map((index) => ({ - name: (index["name"] ?? "default") as string, - type: (index["type"] ?? "UNKNOWN") as string, - status: (index["status"] ?? "UNKNOWN") as string, - queryable: (index["queryable"] ?? false) as boolean, - latestDefinition: index["latestDefinition"] as Document, - })); - } - protected handleError( error: unknown, args: ToolArgs @@ -78,4 +62,25 @@ export class ListSearchIndexesTool extends MongoDBToolBase { } return super.handleError(error, args); } + + static async getSearchIndexes( + provider: NodeDriverServiceProvider, + database: string, + collection: string + ): Promise { + const searchIndexes = await provider.getSearchIndexes(database, collection); + /** + * Atlas Search index status contains a lot of information that is not relevant for the agent at this stage. + * Like for example, the status on each of the dedicated nodes. We only care about the main status, if it's + * queryable and the index name. We are also picking the index definition as it can be used by the agent to + * understand which fields are available for searching. + **/ + return searchIndexes.map((index) => ({ + name: (index["name"] ?? "default") as string, + type: (index["type"] ?? "UNKNOWN") as string, + status: (index["status"] ?? "UNKNOWN") as string, + queryable: (index["queryable"] ?? false) as boolean, + latestDefinition: index["latestDefinition"] as Document, + })); + } } diff --git a/src/tools/mongodb/tools.ts b/src/tools/mongodb/tools.ts index 6e96b2ba..e8564d9b 100644 --- a/src/tools/mongodb/tools.ts +++ b/src/tools/mongodb/tools.ts @@ -21,6 +21,7 @@ import { LogsTool } from "./metadata/logs.js"; import { ExportTool } from "./read/export.js"; import { ListSearchIndexesTool } from "./search/listSearchIndexes.js"; import { DropIndexTool } from "./delete/dropIndex.js"; +import { DropSearchIndexTool } from "./delete/dropSearchIndex.js"; export const MongoDbTools = [ ConnectTool, @@ -46,4 +47,5 @@ export const MongoDbTools = [ LogsTool, ExportTool, ListSearchIndexesTool, + DropSearchIndexTool, ]; diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index bde3c622..2d133987 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -1,3 +1,4 @@ +import type { Collection, Document } from "mongodb"; import { CompositeLogger } from "../../src/common/logger.js"; import { ExportsManager } from "../../src/common/exportsManager.js"; import { Session } from "../../src/common/session.js"; @@ -22,6 +23,9 @@ import { Keychain } from "../../src/common/keychain.js"; import { Elicitation } from "../../src/elicitation.js"; import type { MockClientCapabilities, createMockElicitInput } from "../utils/elicitationMocks.js"; +const SEARCH_READY_CHECK_RETRIES = 200; +const SEARCH_INDEX_STATUS_CHECK_RETRIES = 100; + export const driverOptions = setupDriverConfig({ config, defaults: defaultDriverOptionsFromConfig, @@ -417,3 +421,77 @@ export function getDataFromUntrustedContent(content: string): string { export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } + +export async function waitUntilSearchManagementServiceIsReady( + collection: Collection, + abortSignal?: AbortSignal +): Promise { + for (let i = 0; i < SEARCH_READY_CHECK_RETRIES && !abortSignal?.aborted; i++) { + try { + await collection.listSearchIndexes({}).toArray(); + return; + } catch (error) { + if ( + error instanceof Error && + error.message.includes("Error connecting to Search Index Management service") + ) { + await sleep(100); + continue; + } else { + throw error; + } + } + } +} + +async function waitUntilSearchIndexIs( + collection: Collection, + searchIndex: string, + indexValidator: (index: { name: string; queryable: boolean }) => boolean, + abortSignal?: AbortSignal +): Promise { + for (let i = 0; i < SEARCH_INDEX_STATUS_CHECK_RETRIES && !abortSignal?.aborted; i++) { + try { + const searchIndexes = (await collection.listSearchIndexes(searchIndex).toArray()) as { + name: string; + queryable: boolean; + }[]; + + if (searchIndexes.some((index) => indexValidator(index))) { + return; + } + await sleep(100); + } catch (error) { + if ( + error instanceof Error && + error.message.includes("Error connecting to Search Index Management service") + ) { + await sleep(100); + continue; + } else { + throw error; + } + } + } +} + +export async function waitUntilSearchIndexIsListed( + collection: Collection, + searchIndex: string, + abortSignal?: AbortSignal +): Promise { + return waitUntilSearchIndexIs(collection, searchIndex, (index) => index.name === searchIndex, abortSignal); +} + +export async function waitUntilSearchIndexIsQueryable( + collection: Collection, + searchIndex: string, + abortSignal?: AbortSignal +): Promise { + return waitUntilSearchIndexIs( + collection, + searchIndex, + (index) => index.name === searchIndex && index.queryable, + abortSignal + ); +} diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts new file mode 100644 index 00000000..32b5328a --- /dev/null +++ b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts @@ -0,0 +1,154 @@ +import { beforeEach, afterEach, describe, expect, it } from "vitest"; +import { + databaseCollectionInvalidArgs, + databaseCollectionParameters, + defaultDriverOptions, + defaultTestConfig, + getResponseContent, + setupIntegrationTest, + validateThrowsForInvalidArguments, + validateToolMetadata, + waitUntilSearchManagementServiceIsReady, + waitUntilSearchIndexIsListed, +} from "../../../helpers.js"; +import { describeWithMongoDB } from "../mongodbHelpers.js"; +import type { Collection } from "mongodb"; +import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; +import { Elicitation } from "../../../../../src/elicitation.js"; + +const SEARCH_TIMEOUT = 20_000; + +describeWithMongoDB("drop-search-index tool - metadata and parameters", (integration) => { + validateToolMetadata( + integration, + "drop-search-index", + "Drop a search index or vector search index for the provided database and collection.", + [ + ...databaseCollectionParameters, + { + name: "indexName", + type: "string", + description: "The name of the search or vector search index to be dropped.", + required: true, + }, + ] + ); + + validateThrowsForInvalidArguments(integration, "drop-search-index", [ + ...databaseCollectionInvalidArgs, + { database: "test", collection: "testColl", indexName: null }, + { database: "test", collection: "testColl", indexName: undefined }, + { database: "test", collection: "testColl", indexName: [] }, + { database: "test", collection: "testColl", indexName: true }, + { database: "test", collection: "testColl", indexName: false }, + { database: "test", collection: "testColl", indexName: 0 }, + { database: "test", collection: "testColl", indexName: 12 }, + { database: "test", collection: "testColl", indexName: "" }, + ]); +}); + +describeWithMongoDB("drop-search-index tool - when connected to MongoDB without search support", (integration) => { + it("should fail with appropriate error when invoked", async () => { + await integration.connectMcpClient(); + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "any", collection: "foo", indexName: "default" }, + }); + const content = getResponseContent(response.content); + expect(content).toEqual( + "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." + ); + }); +}); + +describeWithMongoDB( + "drop-search-index tool - when connected to MongoDB with search support", + (integration) => { + beforeEach(async () => { + await integration.connectMcpClient(); + }); + + describe("when attempting to delete a non-existent index", () => { + it("should fail with appropriate error", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "any", collection: "foo", indexName: "non-existent" }, + }); + expect(response.isError).toBe(true); + const content = getResponseContent(response.content); + expect(content).toEqual( + 'Index with name "non-existent" does not exist in the provided namespace "any.foo".' + ); + }); + }); + + describe("when attempting to delete an existing index", () => { + let moviesCollection: Collection; + beforeEach(async ({ signal }) => { + await integration.connectMcpClient(); + const mongoClient = integration.mongoClient(); + moviesCollection = mongoClient.db("mflix").collection("movies"); + await moviesCollection.insertMany([ + { + name: "Movie1", + plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the original MangoDB.", + }, + ]); + await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); + await moviesCollection.createSearchIndex({ + name: "searchIdx", + definition: { mappings: { dynamic: true } }, + }); + await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); + }); + + afterEach(async () => { + try { + await moviesCollection.dropSearchIndex("searchIdx"); + } catch (error) { + if (error instanceof Error && !error.message.includes("not found in")) { + throw error; + } + } + await moviesCollection.drop(); + }); + + it("should succeed in deleting the index", { timeout: SEARCH_TIMEOUT }, async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, + }); + const content = getResponseContent(response.content); + expect(content).toEqual( + 'Successfully dropped the index with name "searchIdx" from the provided namespace "mflix.movies".' + ); + }); + }); + }, + undefined, + undefined, + { search: true } +); + +describe("drop-search-index tool - when invoked via an elicitation enabled client", () => { + const mockElicitInput = createMockElicitInput(); + const integration = setupIntegrationTest( + () => defaultTestConfig, + () => defaultDriverOptions, + { elicitInput: mockElicitInput } + ); + + it("should ask for confirmation before proceeding with tool call", async () => { + mockElicitInput.confirmYes(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "any", collection: "foo", indexName: "default" }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining("You are about to drop the `default` index from the `any.foo` namespace"), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + }); +}); diff --git a/tests/integration/tools/mongodb/mongodbClusterProcess.ts b/tests/integration/tools/mongodb/mongodbClusterProcess.ts index b0f7ee86..bd0da659 100644 --- a/tests/integration/tools/mongodb/mongodbClusterProcess.ts +++ b/tests/integration/tools/mongodb/mongodbClusterProcess.ts @@ -27,7 +27,8 @@ export class MongoDBClusterProcess { return new MongoDBClusterProcess( () => runningContainer.stop(), - () => `mongodb://${runningContainer.getHost()}:${runningContainer.getMappedPort(27017)}` + () => + `mongodb://${runningContainer.getHost()}:${runningContainer.getMappedPort(27017)}/?directConnection=true` ); } else if (MongoDBClusterProcess.isMongoRunnerOptions(config)) { const { downloadOptions, serverArgs } = config; diff --git a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts index 477f9fae..fcbd880e 100644 --- a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts +++ b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts @@ -1,9 +1,5 @@ -import { - describeWithMongoDB, - getSingleDocFromUntrustedContent, - waitUntilSearchIndexIsQueryable, - waitUntilSearchIsReady, -} from "../mongodbHelpers.js"; +import type { Collection } from "mongodb"; +import { describeWithMongoDB, getSingleDocFromUntrustedContent } from "../mongodbHelpers.js"; import { describe, it, expect, beforeEach } from "vitest"; import { getResponseContent, @@ -12,13 +8,14 @@ import { validateThrowsForInvalidArguments, databaseCollectionInvalidArgs, getDataFromUntrustedContent, + waitUntilSearchManagementServiceIsReady, + waitUntilSearchIndexIsQueryable, } from "../../../helpers.js"; -import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; -import type { SearchIndexStatus } from "../../../../../src/tools/mongodb/search/listSearchIndexes.js"; +import type { SearchIndexWithStatus } from "../../../../../src/tools/mongodb/search/listSearchIndexes.js"; const SEARCH_TIMEOUT = 20_000; -describeWithMongoDB("list search indexes tool in local MongoDB", (integration) => { +describeWithMongoDB("list-search-indexes tool in local MongoDB", (integration) => { validateToolMetadata( integration, "list-search-indexes", @@ -42,14 +39,14 @@ describeWithMongoDB("list search indexes tool in local MongoDB", (integration) = }); describeWithMongoDB( - "list search indexes tool in Atlas", + "list-search-indexes tool in Atlas", (integration) => { - let provider: NodeDriverServiceProvider; + let fooCollection: Collection; beforeEach(async ({ signal }) => { await integration.connectMcpClient(); - provider = integration.mcpServer().session.serviceProvider; - await waitUntilSearchIsReady(provider, signal); + fooCollection = integration.mongoClient().db("any").collection("foo"); + await waitUntilSearchManagementServiceIsReady(fooCollection, signal); }); describe("when the collection does not exist", () => { @@ -79,9 +76,10 @@ describeWithMongoDB( }); describe("when there are indexes", () => { - beforeEach(async () => { - await provider.insertOne("any", "foo", { field1: "yay" }); - await provider.createSearchIndexes("any", "foo", [{ definition: { mappings: { dynamic: true } } }]); + beforeEach(async ({ signal }) => { + await fooCollection.insertOne({ field1: "yay" }); + await waitUntilSearchManagementServiceIsReady(fooCollection, signal); + await fooCollection.createSearchIndexes([{ definition: { mappings: { dynamic: true } } }]); }); it("returns the list of existing indexes", { timeout: SEARCH_TIMEOUT }, async () => { @@ -90,7 +88,7 @@ describeWithMongoDB( arguments: { database: "any", collection: "foo" }, }); const content = getResponseContent(response.content); - const indexDefinition = getSingleDocFromUntrustedContent(content); + const indexDefinition = getSingleDocFromUntrustedContent(content); expect(indexDefinition?.name).toEqual("default"); expect(indexDefinition?.type).toEqual("search"); @@ -101,7 +99,7 @@ describeWithMongoDB( "returns the list of existing indexes and detects if they are queryable", { timeout: SEARCH_TIMEOUT }, async ({ signal }) => { - await waitUntilSearchIndexIsQueryable(provider, "any", "foo", "default", signal); + await waitUntilSearchIndexIsQueryable(fooCollection, "default", signal); const response = await integration.mcpClient().callTool({ name: "list-search-indexes", @@ -109,7 +107,7 @@ describeWithMongoDB( }); const content = getResponseContent(response.content); - const indexDefinition = getSingleDocFromUntrustedContent(content); + const indexDefinition = getSingleDocFromUntrustedContent(content); expect(indexDefinition?.name).toEqual("default"); expect(indexDefinition?.type).toEqual("search"); From 1065ee8fcaed2023577ba68ac59a84120c97ba6b Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 13 Oct 2025 15:00:56 +0200 Subject: [PATCH 02/17] chore: fix isError property placement with tests --- src/tools/mongodb/delete/dropSearchIndex.ts | 2 +- tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/mongodb/delete/dropSearchIndex.ts b/src/tools/mongodb/delete/dropSearchIndex.ts index 562a18eb..74caae07 100644 --- a/src/tools/mongodb/delete/dropSearchIndex.ts +++ b/src/tools/mongodb/delete/dropSearchIndex.ts @@ -69,9 +69,9 @@ export class DropSearchIndexTool extends MongoDBToolBase { { text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", type: "text", - isError: true, }, ], + isError: true, }; } return super.handleError(error, args); diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts index 32b5328a..4c35c6bf 100644 --- a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts @@ -55,6 +55,7 @@ describeWithMongoDB("drop-search-index tool - when connected to MongoDB without arguments: { database: "any", collection: "foo", indexName: "default" }, }); const content = getResponseContent(response.content); + expect(response.isError).toBe(true); expect(content).toEqual( "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." ); From f26ae1a39200a0ae06c43170603e8a5a39958a11 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 13 Oct 2025 15:02:00 +0200 Subject: [PATCH 03/17] chore: fix isError property on listSearchIndexes tool with tests --- src/tools/mongodb/search/listSearchIndexes.ts | 2 +- .../integration/tools/mongodb/search/listSearchIndexes.test.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/tools/mongodb/search/listSearchIndexes.ts b/src/tools/mongodb/search/listSearchIndexes.ts index cb55b882..498dc5b6 100644 --- a/src/tools/mongodb/search/listSearchIndexes.ts +++ b/src/tools/mongodb/search/listSearchIndexes.ts @@ -55,9 +55,9 @@ export class ListSearchIndexesTool extends MongoDBToolBase { { text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", type: "text", - isError: true, }, ], + isError: true, }; } return super.handleError(error, args); diff --git a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts index fcbd880e..2dad468b 100644 --- a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts +++ b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts @@ -32,6 +32,7 @@ describeWithMongoDB("list-search-indexes tool in local MongoDB", (integration) = arguments: { database: "any", collection: "foo" }, }); const content = getResponseContent(response.content); + expect(response.isError).toBe(true); expect(content).toEqual( "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." ); From 8960ffa7bf308febb5a682abf81494b2bdf473b5 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 13 Oct 2025 15:04:14 +0200 Subject: [PATCH 04/17] chore: remove redundant call --- tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts index 4c35c6bf..bd468a8d 100644 --- a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts @@ -86,7 +86,6 @@ describeWithMongoDB( describe("when attempting to delete an existing index", () => { let moviesCollection: Collection; beforeEach(async ({ signal }) => { - await integration.connectMcpClient(); const mongoClient = integration.mongoClient(); moviesCollection = mongoClient.db("mflix").collection("movies"); await moviesCollection.insertMany([ From eed44799d11a6e7b405ad1bdc352abefb88a95cb Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Mon, 13 Oct 2025 15:09:19 +0200 Subject: [PATCH 05/17] chore: remove unused import --- tests/integration/helpers.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 2d133987..748ba927 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -1,4 +1,4 @@ -import type { Collection, Document } from "mongodb"; +import type { Collection } from "mongodb"; import { CompositeLogger } from "../../src/common/logger.js"; import { ExportsManager } from "../../src/common/exportsManager.js"; import { Session } from "../../src/common/session.js"; From 52dc7c5388444c0a0267b334fbc877e05ce256c9 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 14 Oct 2025 11:35:48 +0200 Subject: [PATCH 06/17] chore: addresses a few common concerns 1. Wraps the toolcall result in untrusted data wrapper 2. Add additional test for confirming that we don't drop when confirmation is not provided --- src/tools/mongodb/delete/dropSearchIndex.ts | 25 +++-- tests/integration/helpers.ts | 93 +++++++++++++------ .../mongodb/delete/dropSearchIndex.test.ts | 92 ++++++++++++++---- 3 files changed, 149 insertions(+), 61 deletions(-) diff --git a/src/tools/mongodb/delete/dropSearchIndex.ts b/src/tools/mongodb/delete/dropSearchIndex.ts index 74caae07..dd462d84 100644 --- a/src/tools/mongodb/delete/dropSearchIndex.ts +++ b/src/tools/mongodb/delete/dropSearchIndex.ts @@ -1,7 +1,7 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { CommonArgs } from "../../args.js"; import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; -import { type OperationType, type ToolArgs } from "../../tool.js"; +import { formatUntrustedData, type OperationType, type ToolArgs } from "../../tool.js"; import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; export class DropSearchIndexTool extends MongoDBToolBase { @@ -30,24 +30,23 @@ export class DropSearchIndexTool extends MongoDBToolBase { const indexDoesNotExist = !searchIndexes.find((index) => index.name === indexName); if (indexDoesNotExist) { return { - content: [ - { - type: "text", - text: `Index with name "${indexName}" does not exist in the provided namespace "${database}.${collection}".`, - }, - ], + content: formatUntrustedData( + "Index does not exist in the provided namespace.", + JSON.stringify({ indexName, namespace: `${database}.${collection}` }) + ), isError: true, }; } await provider.dropSearchIndex(database, collection, indexName); return { - content: [ - { - type: "text", - text: `Successfully dropped the index with name "${indexName}" from the provided namespace "${database}.${collection}".`, - }, - ], + content: formatUntrustedData( + "Successfully dropped the index from the provided namespace.", + JSON.stringify({ + indexName, + namespace: `${database}.${collection}`, + }) + ), }; } diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 748ba927..6878be6c 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -422,57 +422,90 @@ export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -export async function waitUntilSearchManagementServiceIsReady( - collection: Collection, - abortSignal?: AbortSignal +export async function waitFor( + condition: () => boolean | Promise, + { + retries, + retryTimeout, + abortSignal, + shouldRetryOnError, + }: { + retries: number; + retryTimeout: number; + abortSignal?: AbortSignal; + shouldRetryOnError?: (error: unknown) => boolean | Promise; + } = { + retries: 100, + retryTimeout: 100, + } ): Promise { - for (let i = 0; i < SEARCH_READY_CHECK_RETRIES && !abortSignal?.aborted; i++) { + for (let i = 0; i < retries && !abortSignal?.aborted; i++) { try { - await collection.listSearchIndexes({}).toArray(); - return; + if (await condition()) { + return; + } + + await sleep(retryTimeout); } catch (error) { - if ( - error instanceof Error && - error.message.includes("Error connecting to Search Index Management service") - ) { - await sleep(100); + if (shouldRetryOnError && (await shouldRetryOnError(error))) { + await sleep(retryTimeout); continue; - } else { - throw error; } + throw error; } } } +export async function waitUntilSearchManagementServiceIsReady( + collection: Collection, + abortSignal?: AbortSignal +): Promise { + await waitFor( + async (): Promise => { + await collection.listSearchIndexes({}).toArray(); + return true; + }, + { + retries: SEARCH_READY_CHECK_RETRIES, + retryTimeout: 100, + abortSignal, + shouldRetryOnError: (error: unknown) => { + return ( + error instanceof Error && + error.message.includes("Error connecting to Search Index Management service") + ); + }, + } + ); +} + async function waitUntilSearchIndexIs( collection: Collection, searchIndex: string, indexValidator: (index: { name: string; queryable: boolean }) => boolean, abortSignal?: AbortSignal ): Promise { - for (let i = 0; i < SEARCH_INDEX_STATUS_CHECK_RETRIES && !abortSignal?.aborted; i++) { - try { + await waitFor( + async (): Promise => { const searchIndexes = (await collection.listSearchIndexes(searchIndex).toArray()) as { name: string; queryable: boolean; }[]; - if (searchIndexes.some((index) => indexValidator(index))) { - return; - } - await sleep(100); - } catch (error) { - if ( - error instanceof Error && - error.message.includes("Error connecting to Search Index Management service") - ) { - await sleep(100); - continue; - } else { - throw error; - } + return searchIndexes.some((index) => indexValidator(index)); + }, + { + retries: SEARCH_INDEX_STATUS_CHECK_RETRIES, + retryTimeout: 100, + abortSignal, + shouldRetryOnError: (error: unknown) => { + return ( + error instanceof Error && + error.message.includes("Error connecting to Search Index Management service") + ); + }, } - } + ); } export async function waitUntilSearchIndexIsListed( diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts index bd468a8d..16eec062 100644 --- a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, afterEach, describe, expect, it } from "vitest"; +import { beforeEach, afterEach, describe, expect, it, vi, type MockInstance } from "vitest"; import { databaseCollectionInvalidArgs, databaseCollectionParameters, @@ -10,8 +10,9 @@ import { validateToolMetadata, waitUntilSearchManagementServiceIsReady, waitUntilSearchIndexIsListed, + getDataFromUntrustedContent, } from "../../../helpers.js"; -import { describeWithMongoDB } from "../mongodbHelpers.js"; +import { describeWithMongoDB, setupMongoDBIntegrationTest } from "../mongodbHelpers.js"; import type { Collection } from "mongodb"; import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; import { Elicitation } from "../../../../../src/elicitation.js"; @@ -77,9 +78,10 @@ describeWithMongoDB( }); expect(response.isError).toBe(true); const content = getResponseContent(response.content); - expect(content).toEqual( - 'Index with name "non-existent" does not exist in the provided namespace "any.foo".' - ); + expect(content).toContain("Index does not exist in the provided namespace."); + + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ indexName: "non-existent", namespace: "any.foo" }); }); }); @@ -91,7 +93,7 @@ describeWithMongoDB( await moviesCollection.insertMany([ { name: "Movie1", - plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the original MangoDB.", + plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", }, ]); await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); @@ -103,13 +105,7 @@ describeWithMongoDB( }); afterEach(async () => { - try { - await moviesCollection.dropSearchIndex("searchIdx"); - } catch (error) { - if (error instanceof Error && !error.message.includes("not found in")) { - throw error; - } - } + // dropping collection also drops the associated search indexes await moviesCollection.drop(); }); @@ -119,9 +115,10 @@ describeWithMongoDB( arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, }); const content = getResponseContent(response.content); - expect(content).toEqual( - 'Successfully dropped the index with name "searchIdx" from the provided namespace "mflix.movies".' - ); + expect(content).toContain("Successfully dropped the index from the provided namespace."); + + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ indexName: "searchIdx", namespace: "mflix.movies" }); }); }); }, @@ -132,23 +129,82 @@ describeWithMongoDB( describe("drop-search-index tool - when invoked via an elicitation enabled client", () => { const mockElicitInput = createMockElicitInput(); + const mdbIntegration = setupMongoDBIntegrationTest({ search: true }); const integration = setupIntegrationTest( () => defaultTestConfig, () => defaultDriverOptions, { elicitInput: mockElicitInput } ); + let moviesCollection: Collection; + let dropSearchIndexSpy: MockInstance; + + beforeEach(async ({ signal }) => { + const mongoClient = mdbIntegration.mongoClient(); + moviesCollection = mongoClient.db("mflix").collection("movies"); + await moviesCollection.insertMany([ + { + name: "Movie1", + plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", + }, + ]); + await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); + await moviesCollection.createSearchIndex({ + name: "searchIdx", + definition: { mappings: { dynamic: true } }, + }); + await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); + + await integration.mcpClient().callTool({ + name: "connect", + arguments: { + connectionString: mdbIntegration.connectionString(), + }, + }); + + // Note: Unlike drop-index tool test, we don't test the final state of + // indexes because of possible longer wait periods for changes to + // reflect, at-times taking >30 seconds. + dropSearchIndexSpy = vi.spyOn(integration.mcpServer().session.serviceProvider, "dropSearchIndex"); + }); + + afterEach(async () => { + // dropping collection also drops the associated search indexes + await moviesCollection.drop(); + }); + it("should ask for confirmation before proceeding with tool call", async () => { mockElicitInput.confirmYes(); await integration.mcpClient().callTool({ name: "drop-search-index", - arguments: { database: "any", collection: "foo", indexName: "default" }, + arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + + expect(dropSearchIndexSpy).toHaveBeenCalledExactlyOnceWith("mflix", "movies", "searchIdx"); + }); + + it("should not drop the index if the confirmation was not provided", async () => { + mockElicitInput.confirmNo(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, }); expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); expect(mockElicitInput.mock).toHaveBeenCalledWith({ // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining("You are about to drop the `default` index from the `any.foo` namespace"), + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), requestedSchema: Elicitation.CONFIRMATION_SCHEMA, }); + expect(dropSearchIndexSpy).not.toHaveBeenCalled(); }); }); From 746643bb163757a2282a9a4b0a264858d53ed498 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Tue, 14 Oct 2025 11:37:12 +0200 Subject: [PATCH 07/17] chore: use Zod.string instead of CommonArgs.string --- src/tools/mongodb/delete/dropSearchIndex.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/tools/mongodb/delete/dropSearchIndex.ts b/src/tools/mongodb/delete/dropSearchIndex.ts index dd462d84..22d61f10 100644 --- a/src/tools/mongodb/delete/dropSearchIndex.ts +++ b/src/tools/mongodb/delete/dropSearchIndex.ts @@ -1,5 +1,5 @@ +import z from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { CommonArgs } from "../../args.js"; import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; import { formatUntrustedData, type OperationType, type ToolArgs } from "../../tool.js"; import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; @@ -9,9 +9,7 @@ export class DropSearchIndexTool extends MongoDBToolBase { protected description = "Drop a search index or vector search index for the provided database and collection."; protected argsShape = { ...DbOperationArgs, - indexName: CommonArgs.string() - .nonempty() - .describe("The name of the search or vector search index to be dropped."), + indexName: z.string().nonempty().describe("The name of the search or vector search index to be dropped."), }; public operationType: OperationType = "delete"; From 00f221287245d6278a6b8083232c4818a3390c2d Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 15 Oct 2025 11:37:21 +0200 Subject: [PATCH 08/17] chore: adapt test for new interface --- .../mongodb/delete/dropSearchIndex.test.ts | 154 +++++++++--------- 1 file changed, 76 insertions(+), 78 deletions(-) diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts index 16eec062..c5447534 100644 --- a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts @@ -2,17 +2,14 @@ import { beforeEach, afterEach, describe, expect, it, vi, type MockInstance } fr import { databaseCollectionInvalidArgs, databaseCollectionParameters, - defaultDriverOptions, - defaultTestConfig, getResponseContent, - setupIntegrationTest, validateThrowsForInvalidArguments, validateToolMetadata, waitUntilSearchManagementServiceIsReady, waitUntilSearchIndexIsListed, getDataFromUntrustedContent, } from "../../../helpers.js"; -import { describeWithMongoDB, setupMongoDBIntegrationTest } from "../mongodbHelpers.js"; +import { describeWithMongoDB } from "../mongodbHelpers.js"; import type { Collection } from "mongodb"; import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; import { Elicitation } from "../../../../../src/elicitation.js"; @@ -122,89 +119,90 @@ describeWithMongoDB( }); }); }, - undefined, - undefined, - { search: true } + { + downloadOptions: { search: true }, + } ); -describe("drop-search-index tool - when invoked via an elicitation enabled client", () => { - const mockElicitInput = createMockElicitInput(); - const mdbIntegration = setupMongoDBIntegrationTest({ search: true }); - const integration = setupIntegrationTest( - () => defaultTestConfig, - () => defaultDriverOptions, - { elicitInput: mockElicitInput } - ); +const mockElicitInput = createMockElicitInput(); +describeWithMongoDB( + "drop-search-index tool - when invoked via an elicitation enabled client", + (integration) => { + let moviesCollection: Collection; + let dropSearchIndexSpy: MockInstance; + + beforeEach(async ({ signal }) => { + const mongoClient = integration.mongoClient(); + moviesCollection = mongoClient.db("mflix").collection("movies"); + await moviesCollection.insertMany([ + { + name: "Movie1", + plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", + }, + ]); + await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); + await moviesCollection.createSearchIndex({ + name: "searchIdx", + definition: { mappings: { dynamic: true } }, + }); + await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); - let moviesCollection: Collection; - let dropSearchIndexSpy: MockInstance; + await integration.mcpClient().callTool({ + name: "connect", + arguments: { + connectionString: integration.connectionString(), + }, + }); - beforeEach(async ({ signal }) => { - const mongoClient = mdbIntegration.mongoClient(); - moviesCollection = mongoClient.db("mflix").collection("movies"); - await moviesCollection.insertMany([ - { - name: "Movie1", - plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", - }, - ]); - await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); - await moviesCollection.createSearchIndex({ - name: "searchIdx", - definition: { mappings: { dynamic: true } }, + // Note: Unlike drop-index tool test, we don't test the final state of + // indexes because of possible longer wait periods for changes to + // reflect, at-times taking >30 seconds. + dropSearchIndexSpy = vi.spyOn(integration.mcpServer().session.serviceProvider, "dropSearchIndex"); }); - await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); - await integration.mcpClient().callTool({ - name: "connect", - arguments: { - connectionString: mdbIntegration.connectionString(), - }, + afterEach(async () => { + mockElicitInput.clear(); + // dropping collection also drops the associated search indexes + await moviesCollection.drop(); }); - // Note: Unlike drop-index tool test, we don't test the final state of - // indexes because of possible longer wait periods for changes to - // reflect, at-times taking >30 seconds. - dropSearchIndexSpy = vi.spyOn(integration.mcpServer().session.serviceProvider, "dropSearchIndex"); - }); - - afterEach(async () => { - // dropping collection also drops the associated search indexes - await moviesCollection.drop(); - }); + it("should ask for confirmation before proceeding with tool call", async () => { + mockElicitInput.confirmYes(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); - it("should ask for confirmation before proceeding with tool call", async () => { - mockElicitInput.confirmYes(); - await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + expect(dropSearchIndexSpy).toHaveBeenCalledExactlyOnceWith("mflix", "movies", "searchIdx"); }); - expect(dropSearchIndexSpy).toHaveBeenCalledExactlyOnceWith("mflix", "movies", "searchIdx"); - }); - - it("should not drop the index if the confirmation was not provided", async () => { - mockElicitInput.confirmNo(); - await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + it("should not drop the index if the confirmation was not provided", async () => { + mockElicitInput.confirmNo(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + expect(dropSearchIndexSpy).not.toHaveBeenCalled(); }); - expect(dropSearchIndexSpy).not.toHaveBeenCalled(); - }); -}); + }, + { + downloadOptions: { search: true }, + getMockElicitationInput: () => mockElicitInput, + } +); From 716d9cbc1e9215a9c56e489df1ab81d5648caae9 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 15 Oct 2025 16:50:31 +0200 Subject: [PATCH 09/17] chore: merge drop-search-index into drop-index tool --- src/tools/mongodb/delete/dropIndex.ts | 78 ++- src/tools/mongodb/delete/dropSearchIndex.ts | 76 --- .../tools/mongodb/delete/dropIndex.test.ts | 541 +++++++++++++----- .../mongodb/delete/dropSearchIndex.test.ts | 208 ------- .../tools/mongodb/mongodbHelpers.ts | 2 +- 5 files changed, 482 insertions(+), 423 deletions(-) delete mode 100644 src/tools/mongodb/delete/dropSearchIndex.ts delete mode 100644 tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts diff --git a/src/tools/mongodb/delete/dropIndex.ts b/src/tools/mongodb/delete/dropIndex.ts index e87db417..ac924fc2 100644 --- a/src/tools/mongodb/delete/dropIndex.ts +++ b/src/tools/mongodb/delete/dropIndex.ts @@ -1,7 +1,9 @@ import z from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; -import { type ToolArgs, type OperationType, formatUntrustedData } from "../../tool.js"; +import { type ToolArgs, type OperationType, formatUntrustedData, FeatureFlags } from "../../tool.js"; +import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; export class DropIndexTool extends MongoDBToolBase { public name = "drop-index"; @@ -9,15 +11,33 @@ export class DropIndexTool extends MongoDBToolBase { protected argsShape = { ...DbOperationArgs, indexName: z.string().nonempty().describe("The name of the index to be dropped."), + type: this.isFeatureFlagEnabled(FeatureFlags.VectorSearch) + ? z + .enum(["classic", "search"]) + .describe( + "The type of index to be deleted. Use 'classic' for standard indexes and 'search' for atlas search and vector search indexes." + ) + : z + .literal("classic") + .default("classic") + .describe("The type of index to be deleted. Is always set to 'classic'."), }; public operationType: OperationType = "delete"; - protected async execute({ - database, - collection, - indexName, - }: ToolArgs): Promise { + protected async execute(toolArgs: ToolArgs): Promise { const provider = await this.ensureConnected(); + switch (toolArgs.type) { + case "classic": + return this.dropClassicIndex(provider, toolArgs); + case "search": + return this.dropSearchIndex(provider, toolArgs); + } + } + + private async dropClassicIndex( + provider: NodeDriverServiceProvider, + { database, collection, indexName }: ToolArgs + ): Promise { const result = await provider.runCommand(database, { dropIndexes: collection, index: indexName, @@ -35,6 +55,34 @@ export class DropIndexTool extends MongoDBToolBase { }; } + private async dropSearchIndex( + provider: NodeDriverServiceProvider, + { database, collection, indexName }: ToolArgs + ): Promise { + const searchIndexes = await ListSearchIndexesTool.getSearchIndexes(provider, database, collection); + const indexDoesNotExist = !searchIndexes.find((index) => index.name === indexName); + if (indexDoesNotExist) { + return { + content: formatUntrustedData( + "Index does not exist in the provided namespace.", + JSON.stringify({ indexName, namespace: `${database}.${collection}` }) + ), + isError: true, + }; + } + + await provider.dropSearchIndex(database, collection, indexName); + return { + content: formatUntrustedData( + "Successfully dropped the index from the provided namespace.", + JSON.stringify({ + indexName, + namespace: `${database}.${collection}`, + }) + ), + }; + } + protected getConfirmationMessage({ database, collection, indexName }: ToolArgs): string { return ( `You are about to drop the \`${indexName}\` index from the \`${database}.${collection}\` namespace:\n\n` + @@ -42,4 +90,22 @@ export class DropIndexTool extends MongoDBToolBase { "**Do you confirm the execution of the action?**" ); } + + protected handleError( + error: unknown, + args: ToolArgs + ): Promise | CallToolResult { + if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { + return { + content: [ + { + text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", + type: "text", + }, + ], + isError: true, + }; + } + return super.handleError(error, args); + } } diff --git a/src/tools/mongodb/delete/dropSearchIndex.ts b/src/tools/mongodb/delete/dropSearchIndex.ts deleted file mode 100644 index 22d61f10..00000000 --- a/src/tools/mongodb/delete/dropSearchIndex.ts +++ /dev/null @@ -1,76 +0,0 @@ -import z from "zod"; -import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; -import { formatUntrustedData, type OperationType, type ToolArgs } from "../../tool.js"; -import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; - -export class DropSearchIndexTool extends MongoDBToolBase { - public name = "drop-search-index"; - protected description = "Drop a search index or vector search index for the provided database and collection."; - protected argsShape = { - ...DbOperationArgs, - indexName: z.string().nonempty().describe("The name of the search or vector search index to be dropped."), - }; - public operationType: OperationType = "delete"; - - protected override verifyAllowed(): boolean { - // Only enable this on tests for now. - return process.env.VITEST === "true"; - } - - protected override async execute({ - database, - collection, - indexName, - }: ToolArgs): Promise { - const provider = await this.ensureConnected(); - const searchIndexes = await ListSearchIndexesTool.getSearchIndexes(provider, database, collection); - const indexDoesNotExist = !searchIndexes.find((index) => index.name === indexName); - if (indexDoesNotExist) { - return { - content: formatUntrustedData( - "Index does not exist in the provided namespace.", - JSON.stringify({ indexName, namespace: `${database}.${collection}` }) - ), - isError: true, - }; - } - - await provider.dropSearchIndex(database, collection, indexName); - return { - content: formatUntrustedData( - "Successfully dropped the index from the provided namespace.", - JSON.stringify({ - indexName, - namespace: `${database}.${collection}`, - }) - ), - }; - } - - protected getConfirmationMessage({ database, collection, indexName }: ToolArgs): string { - return ( - `You are about to drop the \`${indexName}\` index from the \`${database}.${collection}\` namespace:\n\n` + - "This operation will permanently remove the index and might affect the performance of queries relying on this index.\n\n" + - "**Do you confirm the execution of the action?**" - ); - } - - protected handleError( - error: unknown, - args: ToolArgs - ): Promise | CallToolResult { - if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { - return { - content: [ - { - text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", - type: "text", - }, - ], - isError: true, - }; - } - return super.handleError(error, args); - } -} diff --git a/tests/integration/tools/mongodb/delete/dropIndex.test.ts b/tests/integration/tools/mongodb/delete/dropIndex.test.ts index 46360b81..018c2d55 100644 --- a/tests/integration/tools/mongodb/delete/dropIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropIndex.test.ts @@ -1,18 +1,26 @@ -import { describe, beforeEach, it, afterEach, expect } from "vitest"; +import { describe, beforeEach, it, afterEach, expect, vi, type MockInstance } from "vitest"; import type { Collection } from "mongodb"; import { databaseCollectionInvalidArgs, databaseCollectionParameters, + defaultTestConfig, getDataFromUntrustedContent, getResponseContent, validateThrowsForInvalidArguments, validateToolMetadata, + waitUntilSearchIndexIsListed, + waitUntilSearchManagementServiceIsReady, } from "../../../helpers.js"; -import { describeWithMongoDB } from "../mongodbHelpers.js"; +import { describeWithMongoDB, type MongoDBIntegrationTestCase } from "../mongodbHelpers.js"; import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; import { Elicitation } from "../../../../../src/elicitation.js"; -describeWithMongoDB("drop-index tool", (integration) => { +const SEARCH_TIMEOUT = 20_000; + +function setupForClassicIndexes(integration: MongoDBIntegrationTestCase): { + getMoviesCollection: () => Collection; + getIndexName: () => string; +} { let moviesCollection: Collection; let indexName: string; beforeEach(async () => { @@ -36,144 +44,413 @@ describeWithMongoDB("drop-index tool", (integration) => { await moviesCollection.drop(); }); - validateToolMetadata(integration, "drop-index", "Drop an index for the provided database and collection.", [ - ...databaseCollectionParameters, - { - name: "indexName", - type: "string", - description: "The name of the index to be dropped.", - required: true, - }, - ]); - - validateThrowsForInvalidArguments(integration, "drop-index", [ - ...databaseCollectionInvalidArgs, - { database: "test", collection: "testColl", indexName: null }, - { database: "test", collection: "testColl", indexName: undefined }, - { database: "test", collection: "testColl", indexName: [] }, - { database: "test", collection: "testColl", indexName: true }, - { database: "test", collection: "testColl", indexName: false }, - { database: "test", collection: "testColl", indexName: 0 }, - { database: "test", collection: "testColl", indexName: 12 }, - { database: "test", collection: "testColl", indexName: "" }, - ]); - - describe.each([ - { - database: "mflix", - collection: "non-existent", - }, - { - database: "non-db", - collection: "non-coll", - }, - ])( - "when attempting to delete an index from non-existent namespace - $database $collection", - ({ database, collection }) => { - it("should fail with error", async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-index", - arguments: { database, collection, indexName: "non-existent" }, - }); - expect(response.isError).toBe(true); - const content = getResponseContent(response.content); - expect(content).toEqual(`Error running drop-index: ns not found ${database}.${collection}`); - }); - } - ); - - describe("when attempting to delete an index that does not exist", () => { - it("should fail with error", async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-index", - arguments: { database: "mflix", collection: "movies", indexName: "non-existent" }, - }); - expect(response.isError).toBe(true); - const content = getResponseContent(response.content); - expect(content).toEqual(`Error running drop-index: index not found with name [non-existent]`); + return { + getMoviesCollection: () => moviesCollection, + getIndexName: () => indexName, + }; +} + +function setupForVectorSearchIndexes(integration: MongoDBIntegrationTestCase): { + getMoviesCollection: () => Collection; + getIndexName: () => string; +} { + let moviesCollection: Collection; + const indexName = "searchIdx"; + beforeEach(async ({ signal }) => { + await integration.connectMcpClient(); + const mongoClient = integration.mongoClient(); + moviesCollection = mongoClient.db("mflix").collection("movies"); + await moviesCollection.insertMany([ + { + name: "Movie1", + plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", + }, + ]); + await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); + await moviesCollection.createSearchIndex({ + name: indexName, + definition: { mappings: { dynamic: true } }, }); + await waitUntilSearchIndexIsListed(moviesCollection, indexName, signal); }); - describe("when attempting to delete an index that exists", () => { - it("should succeed", async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-index", - // The index is created in beforeEach - arguments: { database: "mflix", collection: "movies", indexName: indexName }, - }); - expect(response.isError).toBe(undefined); - const content = getResponseContent(response.content); - expect(content).toContain(`Successfully dropped the index from the provided namespace.`); - const data = getDataFromUntrustedContent(content); - expect(JSON.parse(data)).toMatchObject({ indexName, namespace: "mflix.movies" }); - }); + afterEach(async () => { + // dropping collection also drops the associated search indexes + await moviesCollection.drop(); }); -}); - -const mockElicitInput = createMockElicitInput(); - -describeWithMongoDB( - "drop-index tool - when invoked via an elicitation enabled client", - (integration) => { - let moviesCollection: Collection; - let indexName: string; - - beforeEach(async () => { - moviesCollection = integration.mongoClient().db("mflix").collection("movies"); - await moviesCollection.insertMany([ - { name: "Movie1", year: 1994 }, - { name: "Movie2", year: 2001 }, - ]); - indexName = await moviesCollection.createIndex({ year: 1 }); - await integration.mcpClient().callTool({ - name: "connect", - arguments: { - connectionString: integration.connectionString(), + + return { + getMoviesCollection: () => moviesCollection, + getIndexName: () => indexName, + }; +} + +describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( + "drop-index tool", + ({ vectorSearchEnabled }) => { + describe(`when vector search is ${vectorSearchEnabled ? "enabled" : "disabled"}`, () => { + describeWithMongoDB( + "tool metadata and parameters", + (integration) => { + validateToolMetadata( + integration, + "drop-index", + "Drop an index for the provided database and collection.", + [ + ...databaseCollectionParameters, + { + name: "indexName", + type: "string", + description: "The name of the index to be dropped.", + required: true, + }, + vectorSearchEnabled + ? { + name: "type", + type: "string", + description: + "The type of index to be deleted. Use 'classic' for standard indexes and 'search' for atlas search and vector search indexes.", + required: true, + } + : { + name: "type", + type: "string", + description: "The type of index to be deleted. Is always set to 'classic'.", + required: false, + }, + ] + ); + + const invalidArgsTestCases = vectorSearchEnabled + ? [ + ...databaseCollectionInvalidArgs, + { database: "test", collection: "testColl", indexName: null, type: "classic" }, + { database: "test", collection: "testColl", indexName: undefined, type: "classic" }, + { database: "test", collection: "testColl", indexName: [], type: "classic" }, + { database: "test", collection: "testColl", indexName: true, type: "classic" }, + { database: "test", collection: "testColl", indexName: false, type: "search" }, + { database: "test", collection: "testColl", indexName: 0, type: "search" }, + { database: "test", collection: "testColl", indexName: 12, type: "search" }, + { database: "test", collection: "testColl", indexName: "", type: "search" }, + // When feature flag is enabled anything other than search and + // classic are invalid + { database: "test", collection: "testColl", indexName: "goodIndex", type: "anything" }, + ] + : [ + ...databaseCollectionInvalidArgs, + { database: "test", collection: "testColl", indexName: null }, + { database: "test", collection: "testColl", indexName: undefined }, + { database: "test", collection: "testColl", indexName: [] }, + { database: "test", collection: "testColl", indexName: true }, + { database: "test", collection: "testColl", indexName: false }, + { database: "test", collection: "testColl", indexName: 0 }, + { database: "test", collection: "testColl", indexName: 12 }, + { database: "test", collection: "testColl", indexName: "" }, + // When feature flag is disabled even "search" is an invalid + // argument + { database: "test", collection: "testColl", indexName: "", type: "search" }, + ]; + + validateThrowsForInvalidArguments(integration, "drop-index", invalidArgsTestCases); }, - }); - }); + { + getUserConfig: () => ({ + ...defaultTestConfig, + voyageApiKey: vectorSearchEnabled ? "test-api-key" : "", + }), + } + ); - afterEach(async () => { - await moviesCollection.drop(); - }); + describeWithMongoDB( + "dropping classic indexes", + (integration) => { + const { getIndexName } = setupForClassicIndexes(integration); + describe.each([ + { + database: "mflix", + collection: "non-existent", + }, + { + database: "non-db", + collection: "non-coll", + }, + ])( + "when attempting to delete an index from non-existent namespace - $database $collection", + ({ database, collection }) => { + it("should fail with error", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-index", + arguments: vectorSearchEnabled + ? { database, collection, indexName: "non-existent", type: "classic" } + : { database, collection, indexName: "non-existent" }, + }); + expect(response.isError).toBe(true); + const content = getResponseContent(response.content); + expect(content).toEqual( + `Error running drop-index: ns not found ${database}.${collection}` + ); + }); + } + ); - it("should ask for confirmation before proceeding with tool call", async () => { - expect(await moviesCollection.listIndexes().toArray()).toHaveLength(2); - mockElicitInput.confirmYes(); - await integration.mcpClient().callTool({ - name: "drop-index", - arguments: { database: "mflix", collection: "movies", indexName }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `year_1` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, - }); - expect(await moviesCollection.listIndexes().toArray()).toHaveLength(1); - }); + describe("when attempting to delete an index that does not exist", () => { + it("should fail with error", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-index", + arguments: vectorSearchEnabled + ? { + database: "mflix", + collection: "movies", + indexName: "non-existent", + type: "classic", + } + : { database: "mflix", collection: "movies", indexName: "non-existent" }, + }); + expect(response.isError).toBe(true); + const content = getResponseContent(response.content); + expect(content).toEqual( + `Error running drop-index: index not found with name [non-existent]` + ); + }); + }); - it("should not drop the index if the confirmation was not provided", async () => { - expect(await moviesCollection.listIndexes().toArray()).toHaveLength(2); - mockElicitInput.confirmNo(); - await integration.mcpClient().callTool({ - name: "drop-index", - arguments: { database: "mflix", collection: "movies", indexName }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `year_1` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + describe("when attempting to delete an index that exists", () => { + it("should succeed", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-index", + // The index is created in beforeEach + arguments: vectorSearchEnabled + ? { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "classic", + } + : { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + expect(response.isError).toBe(undefined); + const content = getResponseContent(response.content); + expect(content).toContain(`Successfully dropped the index from the provided namespace.`); + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ + indexName: getIndexName(), + namespace: "mflix.movies", + }); + }); + }); + }, + { + getUserConfig: () => ({ + ...defaultTestConfig, + voyageApiKey: vectorSearchEnabled ? "test-api-key" : "", + }), + } + ); + + const mockElicitInput = createMockElicitInput(); + describeWithMongoDB( + "dropping classic indexes through an elicitation enabled client", + (integration) => { + const { getMoviesCollection, getIndexName } = setupForClassicIndexes(integration); + afterEach(() => { + mockElicitInput.clear(); + }); + + it("should ask for confirmation before proceeding with tool call", async () => { + expect(await getMoviesCollection().listIndexes().toArray()).toHaveLength(2); + mockElicitInput.confirmYes(); + await integration.mcpClient().callTool({ + name: "drop-index", + arguments: vectorSearchEnabled + ? { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "classic", + } + : { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `year_1` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + expect(await getMoviesCollection().listIndexes().toArray()).toHaveLength(1); + }); + + it("should not drop the index if the confirmation was not provided", async () => { + expect(await getMoviesCollection().listIndexes().toArray()).toHaveLength(2); + mockElicitInput.confirmNo(); + await integration.mcpClient().callTool({ + name: "drop-index", + arguments: vectorSearchEnabled + ? { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "classic", + } + : { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `year_1` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + expect(await getMoviesCollection().listIndexes().toArray()).toHaveLength(2); + }); + }, + { + getUserConfig: () => ({ + ...defaultTestConfig, + voyageApiKey: vectorSearchEnabled ? "test-api-key" : "", + }), + getMockElicitationInput: () => mockElicitInput, + } + ); + + describe.skipIf(!vectorSearchEnabled)("dropping vector search indexes", () => { + describeWithMongoDB( + "when connected to MongoDB without search support", + (integration) => { + it("should fail with appropriate error when invoked", async () => { + await integration.connectMcpClient(); + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "any", collection: "foo", indexName: "default" }, + }); + const content = getResponseContent(response.content); + expect(response.isError).toBe(true); + expect(content).toEqual( + "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." + ); + }); + }, + { + getUserConfig: () => ({ ...defaultTestConfig, voyageApiKey: "test-api-key" }), + } + ); + + describeWithMongoDB( + "when connected to MongoDB with search support", + (integration) => { + const { getIndexName } = setupForVectorSearchIndexes(integration); + + describe("and attempting to delete a non-existent index", () => { + it("should fail with appropriate error", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "any", collection: "foo", indexName: "non-existent" }, + }); + expect(response.isError).toBe(true); + const content = getResponseContent(response.content); + expect(content).toContain("Index does not exist in the provided namespace."); + + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ + indexName: "non-existent", + namespace: "any.foo", + }); + }); + }); + + describe("and attempting to delete an existing index", () => { + it("should succeed in deleting the index", { timeout: SEARCH_TIMEOUT }, async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + const content = getResponseContent(response.content); + expect(content).toContain( + "Successfully dropped the index from the provided namespace." + ); + + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ + indexName: getIndexName(), + namespace: "mflix.movies", + }); + }); + }); + }, + { + getUserConfig: () => ({ ...defaultTestConfig, voyageApiKey: "test-api-key" }), + downloadOptions: { search: true }, + } + ); + + const mockElicitInput = createMockElicitInput(); + describeWithMongoDB( + "when invoked via an elicitation enabled client", + (integration) => { + const { getIndexName } = setupForVectorSearchIndexes(integration); + let dropSearchIndexSpy: MockInstance; + + beforeEach(() => { + // Note: Unlike drop-index tool test, we don't test the final state of + // indexes because of possible longer wait periods for changes to + // reflect, at-times taking >30 seconds. + dropSearchIndexSpy = vi.spyOn( + integration.mcpServer().session.serviceProvider, + "dropSearchIndex" + ); + }); + + afterEach(() => { + mockElicitInput.clear(); + }); + + it("should ask for confirmation before proceeding with tool call", async () => { + mockElicitInput.confirmYes(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + + expect(dropSearchIndexSpy).toHaveBeenCalledExactlyOnceWith( + "mflix", + "movies", + getIndexName() + ); + }); + + it("should not drop the index if the confirmation was not provided", async () => { + mockElicitInput.confirmNo(); + await integration.mcpClient().callTool({ + name: "drop-search-index", + arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + }); + expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); + expect(mockElicitInput.mock).toHaveBeenCalledWith({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + message: expect.stringContaining( + "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" + ), + requestedSchema: Elicitation.CONFIRMATION_SCHEMA, + }); + expect(dropSearchIndexSpy).not.toHaveBeenCalled(); + }); + }, + { + downloadOptions: { search: true }, + getMockElicitationInput: () => mockElicitInput, + } + ); }); - expect(await moviesCollection.listIndexes().toArray()).toHaveLength(2); }); - }, - { - getMockElicitationInput: () => mockElicitInput, } ); diff --git a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts b/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts deleted file mode 100644 index c5447534..00000000 --- a/tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts +++ /dev/null @@ -1,208 +0,0 @@ -import { beforeEach, afterEach, describe, expect, it, vi, type MockInstance } from "vitest"; -import { - databaseCollectionInvalidArgs, - databaseCollectionParameters, - getResponseContent, - validateThrowsForInvalidArguments, - validateToolMetadata, - waitUntilSearchManagementServiceIsReady, - waitUntilSearchIndexIsListed, - getDataFromUntrustedContent, -} from "../../../helpers.js"; -import { describeWithMongoDB } from "../mongodbHelpers.js"; -import type { Collection } from "mongodb"; -import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; -import { Elicitation } from "../../../../../src/elicitation.js"; - -const SEARCH_TIMEOUT = 20_000; - -describeWithMongoDB("drop-search-index tool - metadata and parameters", (integration) => { - validateToolMetadata( - integration, - "drop-search-index", - "Drop a search index or vector search index for the provided database and collection.", - [ - ...databaseCollectionParameters, - { - name: "indexName", - type: "string", - description: "The name of the search or vector search index to be dropped.", - required: true, - }, - ] - ); - - validateThrowsForInvalidArguments(integration, "drop-search-index", [ - ...databaseCollectionInvalidArgs, - { database: "test", collection: "testColl", indexName: null }, - { database: "test", collection: "testColl", indexName: undefined }, - { database: "test", collection: "testColl", indexName: [] }, - { database: "test", collection: "testColl", indexName: true }, - { database: "test", collection: "testColl", indexName: false }, - { database: "test", collection: "testColl", indexName: 0 }, - { database: "test", collection: "testColl", indexName: 12 }, - { database: "test", collection: "testColl", indexName: "" }, - ]); -}); - -describeWithMongoDB("drop-search-index tool - when connected to MongoDB without search support", (integration) => { - it("should fail with appropriate error when invoked", async () => { - await integration.connectMcpClient(); - const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "any", collection: "foo", indexName: "default" }, - }); - const content = getResponseContent(response.content); - expect(response.isError).toBe(true); - expect(content).toEqual( - "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." - ); - }); -}); - -describeWithMongoDB( - "drop-search-index tool - when connected to MongoDB with search support", - (integration) => { - beforeEach(async () => { - await integration.connectMcpClient(); - }); - - describe("when attempting to delete a non-existent index", () => { - it("should fail with appropriate error", async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "any", collection: "foo", indexName: "non-existent" }, - }); - expect(response.isError).toBe(true); - const content = getResponseContent(response.content); - expect(content).toContain("Index does not exist in the provided namespace."); - - const data = getDataFromUntrustedContent(content); - expect(JSON.parse(data)).toMatchObject({ indexName: "non-existent", namespace: "any.foo" }); - }); - }); - - describe("when attempting to delete an existing index", () => { - let moviesCollection: Collection; - beforeEach(async ({ signal }) => { - const mongoClient = integration.mongoClient(); - moviesCollection = mongoClient.db("mflix").collection("movies"); - await moviesCollection.insertMany([ - { - name: "Movie1", - plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", - }, - ]); - await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); - await moviesCollection.createSearchIndex({ - name: "searchIdx", - definition: { mappings: { dynamic: true } }, - }); - await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); - }); - - afterEach(async () => { - // dropping collection also drops the associated search indexes - await moviesCollection.drop(); - }); - - it("should succeed in deleting the index", { timeout: SEARCH_TIMEOUT }, async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, - }); - const content = getResponseContent(response.content); - expect(content).toContain("Successfully dropped the index from the provided namespace."); - - const data = getDataFromUntrustedContent(content); - expect(JSON.parse(data)).toMatchObject({ indexName: "searchIdx", namespace: "mflix.movies" }); - }); - }); - }, - { - downloadOptions: { search: true }, - } -); - -const mockElicitInput = createMockElicitInput(); -describeWithMongoDB( - "drop-search-index tool - when invoked via an elicitation enabled client", - (integration) => { - let moviesCollection: Collection; - let dropSearchIndexSpy: MockInstance; - - beforeEach(async ({ signal }) => { - const mongoClient = integration.mongoClient(); - moviesCollection = mongoClient.db("mflix").collection("movies"); - await moviesCollection.insertMany([ - { - name: "Movie1", - plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", - }, - ]); - await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); - await moviesCollection.createSearchIndex({ - name: "searchIdx", - definition: { mappings: { dynamic: true } }, - }); - await waitUntilSearchIndexIsListed(moviesCollection, "searchIdx", signal); - - await integration.mcpClient().callTool({ - name: "connect", - arguments: { - connectionString: integration.connectionString(), - }, - }); - - // Note: Unlike drop-index tool test, we don't test the final state of - // indexes because of possible longer wait periods for changes to - // reflect, at-times taking >30 seconds. - dropSearchIndexSpy = vi.spyOn(integration.mcpServer().session.serviceProvider, "dropSearchIndex"); - }); - - afterEach(async () => { - mockElicitInput.clear(); - // dropping collection also drops the associated search indexes - await moviesCollection.drop(); - }); - - it("should ask for confirmation before proceeding with tool call", async () => { - mockElicitInput.confirmYes(); - await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, - }); - - expect(dropSearchIndexSpy).toHaveBeenCalledExactlyOnceWith("mflix", "movies", "searchIdx"); - }); - - it("should not drop the index if the confirmation was not provided", async () => { - mockElicitInput.confirmNo(); - await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: "searchIdx" }, - }); - expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); - expect(mockElicitInput.mock).toHaveBeenCalledWith({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - message: expect.stringContaining( - "You are about to drop the `searchIdx` index from the `mflix.movies` namespace" - ), - requestedSchema: Elicitation.CONFIRMATION_SCHEMA, - }); - expect(dropSearchIndexSpy).not.toHaveBeenCalled(); - }); - }, - { - downloadOptions: { search: true }, - getMockElicitationInput: () => mockElicitInput, - } -); diff --git a/tests/integration/tools/mongodb/mongodbHelpers.ts b/tests/integration/tools/mongodb/mongodbHelpers.ts index 57959864..68dd12ba 100644 --- a/tests/integration/tools/mongodb/mongodbHelpers.ts +++ b/tests/integration/tools/mongodb/mongodbHelpers.ts @@ -77,7 +77,7 @@ export type TestSuiteConfig = { getClientCapabilities?: () => MockClientCapabilities; }; -const defaultTestSuiteConfig: TestSuiteConfig = { +export const defaultTestSuiteConfig: TestSuiteConfig = { getUserConfig: () => defaultTestConfig, getDriverOptions: () => defaultDriverOptions, downloadOptions: DEFAULT_MONGODB_PROCESS_OPTIONS, From 7d795caca716dd240c781b1a4dff6f917d2aff2f Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 15 Oct 2025 17:39:15 +0200 Subject: [PATCH 10/17] chore: use common logic from createIndex --- src/common/config.ts | 1 - src/helpers/searchErrorHandler.ts | 24 ++++++++++ src/server.ts | 8 +++- src/tools/mongodb/create/createIndex.ts | 25 ++-------- src/tools/mongodb/delete/dropIndex.ts | 23 ++------- src/tools/mongodb/tools.ts | 2 - .../tools/mongodb/delete/dropIndex.test.ts | 47 ++++++++++++++----- 7 files changed, 71 insertions(+), 59 deletions(-) create mode 100644 src/helpers/searchErrorHandler.ts diff --git a/src/common/config.ts b/src/common/config.ts index 6011e5a5..b7bf527b 100644 --- a/src/common/config.ts +++ b/src/common/config.ts @@ -204,7 +204,6 @@ export const defaultUserConfig: UserConfig = { "drop-collection", "delete-many", "drop-index", - "drop-search-index", ], transport: "stdio", httpPort: 3000, diff --git a/src/helpers/searchErrorHandler.ts b/src/helpers/searchErrorHandler.ts new file mode 100644 index 00000000..5e2eca71 --- /dev/null +++ b/src/helpers/searchErrorHandler.ts @@ -0,0 +1,24 @@ +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { type DbOperationArgs, MongoDBToolBase } from "../tools/mongodb/mongodbTool.js"; +import type { ToolArgs } from "../tools/tool.js"; + +export abstract class MongoDBToolWithSearchErrorHandler extends MongoDBToolBase { + protected handleError( + error: unknown, + args: ToolArgs + ): Promise | CallToolResult { + const CTA = this.server?.areLocalAtlasToolsAvailable() ? "`atlas-local` tools" : "Atlas CLI"; + if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { + return { + content: [ + { + text: `The connected MongoDB deployment does not support vector search indexes. Either connect to a MongoDB Atlas cluster or use the ${CTA} to create and manage a local Atlas deployment.`, + type: "text", + }, + ], + isError: true, + }; + } + return super.handleError(error, args); + } +} diff --git a/src/server.ts b/src/server.ts index f8aa3226..65baa08b 100644 --- a/src/server.ts +++ b/src/server.ts @@ -18,7 +18,7 @@ import { UnsubscribeRequestSchema, } from "@modelcontextprotocol/sdk/types.js"; import assert from "assert"; -import type { ToolBase, ToolConstructorParams } from "./tools/tool.js"; +import type { ToolBase, ToolCategory, ToolConstructorParams } from "./tools/tool.js"; import { validateConnectionString } from "./helpers/connectionOptions.js"; import { packageInfo } from "./common/packageInfo.js"; import { type ConnectionErrorHandler } from "./common/connectionErrorHandler.js"; @@ -174,6 +174,12 @@ export class Server { this.mcpServer.sendResourceListChanged(); } + public areLocalAtlasToolsAvailable(): boolean { + // TODO: remove hacky casts once we merge the local dev tools + const atlasLocalCategory = "atlas-local" as unknown as ToolCategory; + return !!this.tools.filter((tool) => tool.category === atlasLocalCategory).length; + } + public sendResourceUpdated(uri: string): void { this.session.logger.info({ id: LogId.resourceUpdateFailure, diff --git a/src/tools/mongodb/create/createIndex.ts b/src/tools/mongodb/create/createIndex.ts index f4ac313e..6074884b 100644 --- a/src/tools/mongodb/create/createIndex.ts +++ b/src/tools/mongodb/create/createIndex.ts @@ -1,11 +1,11 @@ import { z } from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; -import type { ToolCategory } from "../../tool.js"; +import { DbOperationArgs } from "../mongodbTool.js"; import { type ToolArgs, type OperationType, FeatureFlags } from "../../tool.js"; import type { IndexDirection } from "mongodb"; +import { MongoDBToolWithSearchErrorHandler } from "../../../helpers/searchErrorHandler.js"; -export class CreateIndexTool extends MongoDBToolBase { +export class CreateIndexTool extends MongoDBToolWithSearchErrorHandler { private vectorSearchIndexDefinition = z.object({ type: z.literal("vectorSearch"), fields: z @@ -113,25 +113,6 @@ export class CreateIndexTool extends MongoDBToolBase { break; case "vectorSearch": { - const isVectorSearchSupported = await this.session.isSearchSupported(); - if (!isVectorSearchSupported) { - // TODO: remove hacky casts once we merge the local dev tools - const isLocalAtlasAvailable = - (this.server?.tools.filter((t) => t.category === ("atlas-local" as unknown as ToolCategory)) - .length ?? 0) > 0; - - const CTA = isLocalAtlasAvailable ? "`atlas-local` tools" : "Atlas CLI"; - return { - content: [ - { - text: `The connected MongoDB deployment does not support vector search indexes. Either connect to a MongoDB Atlas cluster or use the ${CTA} to create and manage a local Atlas deployment.`, - type: "text", - }, - ], - isError: true, - }; - } - indexes = await provider.createSearchIndexes(database, collection, [ { name, diff --git a/src/tools/mongodb/delete/dropIndex.ts b/src/tools/mongodb/delete/dropIndex.ts index ac924fc2..e89c0444 100644 --- a/src/tools/mongodb/delete/dropIndex.ts +++ b/src/tools/mongodb/delete/dropIndex.ts @@ -1,11 +1,12 @@ import z from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; -import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; +import { DbOperationArgs } from "../mongodbTool.js"; import { type ToolArgs, type OperationType, formatUntrustedData, FeatureFlags } from "../../tool.js"; import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; +import { MongoDBToolWithSearchErrorHandler } from "../../../helpers/searchErrorHandler.js"; -export class DropIndexTool extends MongoDBToolBase { +export class DropIndexTool extends MongoDBToolWithSearchErrorHandler { public name = "drop-index"; protected description = "Drop an index for the provided database and collection."; protected argsShape = { @@ -90,22 +91,4 @@ export class DropIndexTool extends MongoDBToolBase { "**Do you confirm the execution of the action?**" ); } - - protected handleError( - error: unknown, - args: ToolArgs - ): Promise | CallToolResult { - if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { - return { - content: [ - { - text: "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search.", - type: "text", - }, - ], - isError: true, - }; - } - return super.handleError(error, args); - } } diff --git a/src/tools/mongodb/tools.ts b/src/tools/mongodb/tools.ts index e8564d9b..6e96b2ba 100644 --- a/src/tools/mongodb/tools.ts +++ b/src/tools/mongodb/tools.ts @@ -21,7 +21,6 @@ import { LogsTool } from "./metadata/logs.js"; import { ExportTool } from "./read/export.js"; import { ListSearchIndexesTool } from "./search/listSearchIndexes.js"; import { DropIndexTool } from "./delete/dropIndex.js"; -import { DropSearchIndexTool } from "./delete/dropSearchIndex.js"; export const MongoDbTools = [ ConnectTool, @@ -47,5 +46,4 @@ export const MongoDbTools = [ LogsTool, ExportTool, ListSearchIndexesTool, - DropSearchIndexTool, ]; diff --git a/tests/integration/tools/mongodb/delete/dropIndex.test.ts b/tests/integration/tools/mongodb/delete/dropIndex.test.ts index 018c2d55..d1a52f5c 100644 --- a/tests/integration/tools/mongodb/delete/dropIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropIndex.test.ts @@ -88,7 +88,7 @@ function setupForVectorSearchIndexes(integration: MongoDBIntegrationTestCase): { describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( "drop-index tool", ({ vectorSearchEnabled }) => { - describe(`when vector search is ${vectorSearchEnabled ? "enabled" : "disabled"}`, () => { + describe(`when vector search feature flag is ${vectorSearchEnabled ? "enabled" : "disabled"}`, () => { describeWithMongoDB( "tool metadata and parameters", (integration) => { @@ -322,13 +322,13 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( it("should fail with appropriate error when invoked", async () => { await integration.connectMcpClient(); const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "any", collection: "foo", indexName: "default" }, + name: "drop-index", + arguments: { database: "any", collection: "foo", indexName: "default", type: "search" }, }); const content = getResponseContent(response.content); expect(response.isError).toBe(true); - expect(content).toEqual( - "This MongoDB cluster does not support Search Indexes. Make sure you are using an Atlas Cluster, either remotely in Atlas or using the Atlas Local image, or your cluster supports MongoDB Search." + expect(content).toContain( + "The connected MongoDB deployment does not support vector search indexes" ); }); }, @@ -345,8 +345,13 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( describe("and attempting to delete a non-existent index", () => { it("should fail with appropriate error", async () => { const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "any", collection: "foo", indexName: "non-existent" }, + name: "drop-index", + arguments: { + database: "any", + collection: "foo", + indexName: "non-existent", + type: "search", + }, }); expect(response.isError).toBe(true); const content = getResponseContent(response.content); @@ -363,8 +368,13 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( describe("and attempting to delete an existing index", () => { it("should succeed in deleting the index", { timeout: SEARCH_TIMEOUT }, async () => { const response = await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + name: "drop-index", + arguments: { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "search", + }, }); const content = getResponseContent(response.content); expect(content).toContain( @@ -409,8 +419,13 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( it("should ask for confirmation before proceeding with tool call", async () => { mockElicitInput.confirmYes(); await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + name: "drop-index", + arguments: { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "search", + }, }); expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); expect(mockElicitInput.mock).toHaveBeenCalledWith({ @@ -431,8 +446,13 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( it("should not drop the index if the confirmation was not provided", async () => { mockElicitInput.confirmNo(); await integration.mcpClient().callTool({ - name: "drop-search-index", - arguments: { database: "mflix", collection: "movies", indexName: getIndexName() }, + name: "drop-index", + arguments: { + database: "mflix", + collection: "movies", + indexName: getIndexName(), + type: "search", + }, }); expect(mockElicitInput.mock).toHaveBeenCalledTimes(1); expect(mockElicitInput.mock).toHaveBeenCalledWith({ @@ -446,6 +466,7 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( }); }, { + getUserConfig: () => ({ ...defaultTestConfig, voyageApiKey: "test-api-key" }), downloadOptions: { search: true }, getMockElicitationInput: () => mockElicitInput, } From 7db2402464f6cc079d5bc097ab024945b629f28e Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Wed, 15 Oct 2025 20:24:34 +0200 Subject: [PATCH 11/17] chore: replace with vi.waitFor --- tests/integration/helpers.ts | 92 +++++-------------- .../tools/mongodb/delete/dropIndex.test.ts | 6 +- .../mongodb/search/listSearchIndexes.test.ts | 12 +-- 3 files changed, 30 insertions(+), 80 deletions(-) diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 6878be6c..d9ef23eb 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -23,8 +23,8 @@ import { Keychain } from "../../src/common/keychain.js"; import { Elicitation } from "../../src/elicitation.js"; import type { MockClientCapabilities, createMockElicitInput } from "../utils/elicitationMocks.js"; -const SEARCH_READY_CHECK_RETRIES = 200; -const SEARCH_INDEX_STATUS_CHECK_RETRIES = 100; +export const DEFAULT_WAIT_TIMEOUT = 1000; +export const DEFAULT_RETRY_INTERVAL = 100; export const driverOptions = setupDriverConfig({ config, @@ -422,88 +422,35 @@ export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } -export async function waitFor( - condition: () => boolean | Promise, - { - retries, - retryTimeout, - abortSignal, - shouldRetryOnError, - }: { - retries: number; - retryTimeout: number; - abortSignal?: AbortSignal; - shouldRetryOnError?: (error: unknown) => boolean | Promise; - } = { - retries: 100, - retryTimeout: 100, - } -): Promise { - for (let i = 0; i < retries && !abortSignal?.aborted; i++) { - try { - if (await condition()) { - return; - } - - await sleep(retryTimeout); - } catch (error) { - if (shouldRetryOnError && (await shouldRetryOnError(error))) { - await sleep(retryTimeout); - continue; - } - throw error; - } - } -} - export async function waitUntilSearchManagementServiceIsReady( collection: Collection, - abortSignal?: AbortSignal + timeout: number = DEFAULT_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL ): Promise { - await waitFor( - async (): Promise => { - await collection.listSearchIndexes({}).toArray(); - return true; - }, - { - retries: SEARCH_READY_CHECK_RETRIES, - retryTimeout: 100, - abortSignal, - shouldRetryOnError: (error: unknown) => { - return ( - error instanceof Error && - error.message.includes("Error connecting to Search Index Management service") - ); - }, - } - ); + await vi.waitFor(async () => await collection.listSearchIndexes({}).toArray(), { timeout, interval }); } async function waitUntilSearchIndexIs( collection: Collection, searchIndex: string, indexValidator: (index: { name: string; queryable: boolean }) => boolean, - abortSignal?: AbortSignal + timeout: number, + interval: number ): Promise { - await waitFor( - async (): Promise => { + await vi.waitFor( + async () => { const searchIndexes = (await collection.listSearchIndexes(searchIndex).toArray()) as { name: string; queryable: boolean; }[]; - return searchIndexes.some((index) => indexValidator(index)); + if (!searchIndexes.some((index) => indexValidator(index))) { + throw new Error("Search index did not pass validation"); + } }, { - retries: SEARCH_INDEX_STATUS_CHECK_RETRIES, - retryTimeout: 100, - abortSignal, - shouldRetryOnError: (error: unknown) => { - return ( - error instanceof Error && - error.message.includes("Error connecting to Search Index Management service") - ); - }, + timeout, + interval, } ); } @@ -511,20 +458,23 @@ async function waitUntilSearchIndexIs( export async function waitUntilSearchIndexIsListed( collection: Collection, searchIndex: string, - abortSignal?: AbortSignal + timeout: number = DEFAULT_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL ): Promise { - return waitUntilSearchIndexIs(collection, searchIndex, (index) => index.name === searchIndex, abortSignal); + return waitUntilSearchIndexIs(collection, searchIndex, (index) => index.name === searchIndex, timeout, interval); } export async function waitUntilSearchIndexIsQueryable( collection: Collection, searchIndex: string, - abortSignal?: AbortSignal + timeout: number = DEFAULT_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL ): Promise { return waitUntilSearchIndexIs( collection, searchIndex, (index) => index.name === searchIndex && index.queryable, - abortSignal + timeout, + interval ); } diff --git a/tests/integration/tools/mongodb/delete/dropIndex.test.ts b/tests/integration/tools/mongodb/delete/dropIndex.test.ts index d1a52f5c..18cc2d7e 100644 --- a/tests/integration/tools/mongodb/delete/dropIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropIndex.test.ts @@ -56,7 +56,7 @@ function setupForVectorSearchIndexes(integration: MongoDBIntegrationTestCase): { } { let moviesCollection: Collection; const indexName = "searchIdx"; - beforeEach(async ({ signal }) => { + beforeEach(async () => { await integration.connectMcpClient(); const mongoClient = integration.mongoClient(); moviesCollection = mongoClient.db("mflix").collection("movies"); @@ -66,12 +66,12 @@ function setupForVectorSearchIndexes(integration: MongoDBIntegrationTestCase): { plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", }, ]); - await waitUntilSearchManagementServiceIsReady(moviesCollection, signal); + await waitUntilSearchManagementServiceIsReady(moviesCollection, SEARCH_TIMEOUT); await moviesCollection.createSearchIndex({ name: indexName, definition: { mappings: { dynamic: true } }, }); - await waitUntilSearchIndexIsListed(moviesCollection, indexName, signal); + await waitUntilSearchIndexIsListed(moviesCollection, indexName, SEARCH_TIMEOUT); }); afterEach(async () => { diff --git a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts index 2dad468b..e2ff08e1 100644 --- a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts +++ b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts @@ -44,10 +44,10 @@ describeWithMongoDB( (integration) => { let fooCollection: Collection; - beforeEach(async ({ signal }) => { + beforeEach(async () => { await integration.connectMcpClient(); fooCollection = integration.mongoClient().db("any").collection("foo"); - await waitUntilSearchManagementServiceIsReady(fooCollection, signal); + await waitUntilSearchManagementServiceIsReady(fooCollection, SEARCH_TIMEOUT); }); describe("when the collection does not exist", () => { @@ -77,9 +77,9 @@ describeWithMongoDB( }); describe("when there are indexes", () => { - beforeEach(async ({ signal }) => { + beforeEach(async () => { await fooCollection.insertOne({ field1: "yay" }); - await waitUntilSearchManagementServiceIsReady(fooCollection, signal); + await waitUntilSearchManagementServiceIsReady(fooCollection, SEARCH_TIMEOUT); await fooCollection.createSearchIndexes([{ definition: { mappings: { dynamic: true } } }]); }); @@ -99,8 +99,8 @@ describeWithMongoDB( it( "returns the list of existing indexes and detects if they are queryable", { timeout: SEARCH_TIMEOUT }, - async ({ signal }) => { - await waitUntilSearchIndexIsQueryable(fooCollection, "default", signal); + async () => { + await waitUntilSearchIndexIsQueryable(fooCollection, "default", SEARCH_TIMEOUT); const response = await integration.mcpClient().callTool({ name: "list-search-indexes", From 2add1f88f27331f49e48ce8bf937a659645f52ad Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 13:30:30 +0200 Subject: [PATCH 12/17] chore: remove search error handler class --- src/helpers/searchErrorHandler.ts | 24 ------------------- src/tools/mongodb/delete/dropIndex.ts | 6 ++--- src/tools/mongodb/search/listSearchIndexes.ts | 2 +- 3 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 src/helpers/searchErrorHandler.ts diff --git a/src/helpers/searchErrorHandler.ts b/src/helpers/searchErrorHandler.ts deleted file mode 100644 index 5e2eca71..00000000 --- a/src/helpers/searchErrorHandler.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; -import { type DbOperationArgs, MongoDBToolBase } from "../tools/mongodb/mongodbTool.js"; -import type { ToolArgs } from "../tools/tool.js"; - -export abstract class MongoDBToolWithSearchErrorHandler extends MongoDBToolBase { - protected handleError( - error: unknown, - args: ToolArgs - ): Promise | CallToolResult { - const CTA = this.server?.areLocalAtlasToolsAvailable() ? "`atlas-local` tools" : "Atlas CLI"; - if (error instanceof Error && "codeName" in error && error.codeName === "SearchNotEnabled") { - return { - content: [ - { - text: `The connected MongoDB deployment does not support vector search indexes. Either connect to a MongoDB Atlas cluster or use the ${CTA} to create and manage a local Atlas deployment.`, - type: "text", - }, - ], - isError: true, - }; - } - return super.handleError(error, args); - } -} diff --git a/src/tools/mongodb/delete/dropIndex.ts b/src/tools/mongodb/delete/dropIndex.ts index e89c0444..7db551fd 100644 --- a/src/tools/mongodb/delete/dropIndex.ts +++ b/src/tools/mongodb/delete/dropIndex.ts @@ -1,12 +1,11 @@ import z from "zod"; import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; -import { DbOperationArgs } from "../mongodbTool.js"; +import { DbOperationArgs, MongoDBToolBase } from "../mongodbTool.js"; import { type ToolArgs, type OperationType, formatUntrustedData, FeatureFlags } from "../../tool.js"; import { ListSearchIndexesTool } from "../search/listSearchIndexes.js"; -import { MongoDBToolWithSearchErrorHandler } from "../../../helpers/searchErrorHandler.js"; -export class DropIndexTool extends MongoDBToolWithSearchErrorHandler { +export class DropIndexTool extends MongoDBToolBase { public name = "drop-index"; protected description = "Drop an index for the provided database and collection."; protected argsShape = { @@ -60,6 +59,7 @@ export class DropIndexTool extends MongoDBToolWithSearchErrorHandler { provider: NodeDriverServiceProvider, { database, collection, indexName }: ToolArgs ): Promise { + await this.ensureSearchIsSupported(); const searchIndexes = await ListSearchIndexesTool.getSearchIndexes(provider, database, collection); const indexDoesNotExist = !searchIndexes.find((index) => index.name === indexName); if (indexDoesNotExist) { diff --git a/src/tools/mongodb/search/listSearchIndexes.ts b/src/tools/mongodb/search/listSearchIndexes.ts index 920233ed..39af7685 100644 --- a/src/tools/mongodb/search/listSearchIndexes.ts +++ b/src/tools/mongodb/search/listSearchIndexes.ts @@ -60,7 +60,7 @@ export class ListSearchIndexesTool extends MongoDBToolBase { **/ return searchIndexes.map((index) => ({ name: (index["name"] ?? "default") as string, - type: (index["type"] ?? "UNKNOWN") as string, + type: (index["type"] ?? "UNKNOWN") as "search" | "vectorSearch", status: (index["status"] ?? "UNKNOWN") as string, queryable: (index["queryable"] ?? false) as boolean, latestDefinition: index["latestDefinition"] as Document, From 122f0624483b68e893f0177a034b81ea606f2347 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 14:29:01 +0200 Subject: [PATCH 13/17] chore: move isToolCategoryAvailable to Server --- src/server.ts | 6 ++---- src/tools/mongodb/mongodbTool.ts | 6 +----- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/server.ts b/src/server.ts index 65baa08b..bfef7c45 100644 --- a/src/server.ts +++ b/src/server.ts @@ -174,10 +174,8 @@ export class Server { this.mcpServer.sendResourceListChanged(); } - public areLocalAtlasToolsAvailable(): boolean { - // TODO: remove hacky casts once we merge the local dev tools - const atlasLocalCategory = "atlas-local" as unknown as ToolCategory; - return !!this.tools.filter((tool) => tool.category === atlasLocalCategory).length; + public isToolCategoryAvailable(name: ToolCategory): boolean { + return !!this.tools.filter((t) => t.category === name).length; } public sendResourceUpdated(uri: string): void { diff --git a/src/tools/mongodb/mongodbTool.ts b/src/tools/mongodb/mongodbTool.ts index dc134508..6c06df19 100644 --- a/src/tools/mongodb/mongodbTool.ts +++ b/src/tools/mongodb/mongodbTool.ts @@ -87,7 +87,7 @@ export abstract class MongoDBToolBase extends ToolBase { isError: true, }; case ErrorCodes.AtlasSearchNotSupported: { - const CTA = this.isToolCategoryAvailable("atlas-local" as unknown as ToolCategory) + const CTA = this.server?.isToolCategoryAvailable("atlas-local" as unknown as ToolCategory) ? "`atlas-local` tools" : "Atlas CLI"; return { @@ -123,8 +123,4 @@ export abstract class MongoDBToolBase extends ToolBase { return metadata; } - - protected isToolCategoryAvailable(name: ToolCategory): boolean { - return (this.server?.tools.filter((t) => t.category === name).length ?? 0) > 0; - } } From 3e578580c71102ef95952a1f2f1f376c5eb3ec8e Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 14:31:20 +0200 Subject: [PATCH 14/17] chore: refactor helpers to use vi.waitFor --- tests/integration/helpers.ts | 61 -------- .../tools/mongodb/create/createIndex.test.ts | 34 +++-- .../tools/mongodb/create/insertMany.test.ts | 72 ++++----- .../tools/mongodb/delete/dropIndex.test.ts | 17 ++- .../tools/mongodb/mongodbHelpers.ts | 141 ++++++++++-------- .../mongodb/search/listSearchIndexes.test.ts | 13 +- 6 files changed, 150 insertions(+), 188 deletions(-) diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 3c4d0de3..391804e8 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -1,4 +1,3 @@ -import type { Collection } from "mongodb"; import { CompositeLogger } from "../../src/common/logger.js"; import { ExportsManager } from "../../src/common/exportsManager.js"; import { Session } from "../../src/common/session.js"; @@ -24,9 +23,6 @@ import { Elicitation } from "../../src/elicitation.js"; import type { MockClientCapabilities, createMockElicitInput } from "../utils/elicitationMocks.js"; import { VectorSearchEmbeddingsManager } from "../../src/common/search/vectorSearchEmbeddingsManager.js"; -export const DEFAULT_WAIT_TIMEOUT = 1000; -export const DEFAULT_RETRY_INTERVAL = 100; - export const driverOptions = setupDriverConfig({ config, defaults: defaultDriverOptionsFromConfig, @@ -423,60 +419,3 @@ export function getDataFromUntrustedContent(content: string): string { export function sleep(ms: number): Promise { return new Promise((resolve) => setTimeout(resolve, ms)); } - -export async function waitUntilSearchManagementServiceIsReady( - collection: Collection, - timeout: number = DEFAULT_WAIT_TIMEOUT, - interval: number = DEFAULT_RETRY_INTERVAL -): Promise { - await vi.waitFor(async () => await collection.listSearchIndexes({}).toArray(), { timeout, interval }); -} - -async function waitUntilSearchIndexIs( - collection: Collection, - searchIndex: string, - indexValidator: (index: { name: string; queryable: boolean }) => boolean, - timeout: number, - interval: number -): Promise { - await vi.waitFor( - async () => { - const searchIndexes = (await collection.listSearchIndexes(searchIndex).toArray()) as { - name: string; - queryable: boolean; - }[]; - - if (!searchIndexes.some((index) => indexValidator(index))) { - throw new Error("Search index did not pass validation"); - } - }, - { - timeout, - interval, - } - ); -} - -export async function waitUntilSearchIndexIsListed( - collection: Collection, - searchIndex: string, - timeout: number = DEFAULT_WAIT_TIMEOUT, - interval: number = DEFAULT_RETRY_INTERVAL -): Promise { - return waitUntilSearchIndexIs(collection, searchIndex, (index) => index.name === searchIndex, timeout, interval); -} - -export async function waitUntilSearchIndexIsQueryable( - collection: Collection, - searchIndex: string, - timeout: number = DEFAULT_WAIT_TIMEOUT, - interval: number = DEFAULT_RETRY_INTERVAL -): Promise { - return waitUntilSearchIndexIs( - collection, - searchIndex, - (index) => index.name === searchIndex && index.queryable, - timeout, - interval - ); -} diff --git a/tests/integration/tools/mongodb/create/createIndex.test.ts b/tests/integration/tools/mongodb/create/createIndex.test.ts index ae41869e..cb468d9f 100644 --- a/tests/integration/tools/mongodb/create/createIndex.test.ts +++ b/tests/integration/tools/mongodb/create/createIndex.test.ts @@ -8,9 +8,8 @@ import { expectDefined, defaultTestConfig, } from "../../../helpers.js"; -import { ObjectId, type IndexDirection } from "mongodb"; -import { beforeEach, describe, expect, it } from "vitest"; -import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; +import { ObjectId, type Collection, type Document, type IndexDirection } from "mongodb"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; describeWithMongoDB("createIndex tool when search is not enabled", (integration) => { it("doesn't allow creating vector search indexes", async () => { @@ -403,12 +402,9 @@ describeWithMongoDB( describeWithMongoDB( "createIndex tool with vector search indexes", (integration) => { - let provider: NodeDriverServiceProvider; - - beforeEach(async ({ signal }) => { + beforeEach(async () => { await integration.connectMcpClient(); - provider = integration.mcpServer().session.serviceProvider; - await waitUntilSearchIsReady(provider, signal); + await waitUntilSearchIsReady(integration.mongoClient()); }); describe("when the collection does not exist", () => { @@ -457,14 +453,26 @@ describeWithMongoDB( }); describe("when the collection exists", () => { + let collectionName: string; + let collection: Collection; + beforeEach(async () => { + collectionName = new ObjectId().toString(); + collection = await integration + .mongoClient() + .db(integration.randomDbName()) + .createCollection(collectionName); + }); + + afterEach(async () => { + await collection.drop(); + }); + it("creates the index", async () => { - const collection = new ObjectId().toString(); - await provider.createCollection(integration.randomDbName(), collection); const response = await integration.mcpClient().callTool({ name: "create-index", arguments: { database: integration.randomDbName(), - collection, + collection: collectionName, name: "vector_1_vector", definition: [ { @@ -480,10 +488,10 @@ describeWithMongoDB( const content = getResponseContent(response.content); expect(content).toEqual( - `Created the index "vector_1_vector" on collection "${collection}" in database "${integration.randomDbName()}". Since this is a vector search index, it may take a while for the index to build. Use the \`list-indexes\` tool to check the index status.` + `Created the index "vector_1_vector" on collection "${collectionName}" in database "${integration.randomDbName()}". Since this is a vector search index, it may take a while for the index to build. Use the \`list-indexes\` tool to check the index status.` ); - const indexes = await provider.getSearchIndexes(integration.randomDbName(), collection); + const indexes = (await collection.listSearchIndexes().toArray()) as unknown as Document[]; expect(indexes).toHaveLength(1); expect(indexes[0]?.name).toEqual("vector_1_vector"); expect(indexes[0]?.type).toEqual("vectorSearch"); diff --git a/tests/integration/tools/mongodb/create/insertMany.test.ts b/tests/integration/tools/mongodb/create/insertMany.test.ts index d426a791..dea5ecef 100644 --- a/tests/integration/tools/mongodb/create/insertMany.test.ts +++ b/tests/integration/tools/mongodb/create/insertMany.test.ts @@ -1,7 +1,7 @@ import { - createVectorSearchIndexAndWait, describeWithMongoDB, validateAutoConnectBehavior, + createVectorSearchIndexAndWait, waitUntilSearchIsReady, } from "../mongodbHelpers.js"; @@ -14,8 +14,8 @@ import { getDataFromUntrustedContent, } from "../../../helpers.js"; import { beforeEach, afterEach, expect, it } from "vitest"; -import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import { ObjectId } from "bson"; +import type { Collection } from "mongodb"; describeWithMongoDB("insertMany tool when search is disabled", (integration) => { validateToolMetadata(integration, "insert-many", "Insert an array of documents into a MongoDB collection", [ @@ -109,35 +109,29 @@ describeWithMongoDB("insertMany tool when search is disabled", (integration) => describeWithMongoDB( "insertMany tool when search is enabled", (integration) => { - let provider: NodeDriverServiceProvider; + // let provider: NodeDriverServiceProvider; + let collection: Collection; - beforeEach(async ({ signal }) => { + beforeEach(async () => { await integration.connectMcpClient(); - provider = integration.mcpServer().session.serviceProvider; - await provider.createCollection(integration.randomDbName(), "test"); - await waitUntilSearchIsReady(provider, signal); + collection = await integration.mongoClient().db(integration.randomDbName()).createCollection("test"); + await waitUntilSearchIsReady(integration.mongoClient()); }); afterEach(async () => { - await provider.dropCollection(integration.randomDbName(), "test"); + await collection.drop(); }); - it("inserts a document when the embedding is correct", async ({ signal }) => { - await createVectorSearchIndexAndWait( - provider, - integration.randomDbName(), - "test", - [ - { - type: "vector", - path: "embedding", - numDimensions: 8, - similarity: "euclidean", - quantization: "scalar", - }, - ], - signal - ); + it("inserts a document when the embedding is correct", async () => { + await createVectorSearchIndexAndWait(integration.mongoClient(), integration.randomDbName(), "test", [ + { + type: "vector", + path: "embedding", + numDimensions: 8, + similarity: "euclidean", + quantization: "scalar", + }, + ]); const response = await integration.mcpClient().callTool({ name: "insert-many", @@ -152,26 +146,20 @@ describeWithMongoDB( const insertedIds = extractInsertedIds(content); expect(insertedIds).toHaveLength(1); - const docCount = await provider.countDocuments(integration.randomDbName(), "test", { _id: insertedIds[0] }); + const docCount = await collection.countDocuments({ _id: insertedIds[0] }); expect(docCount).toBe(1); }); - it("returns an error when there is a search index and quantisation is wrong", async ({ signal }) => { - await createVectorSearchIndexAndWait( - provider, - integration.randomDbName(), - "test", - [ - { - type: "vector", - path: "embedding", - numDimensions: 8, - similarity: "euclidean", - quantization: "scalar", - }, - ], - signal - ); + it("returns an error when there is a search index and quantisation is wrong", async () => { + await createVectorSearchIndexAndWait(integration.mongoClient(), integration.randomDbName(), "test", [ + { + type: "vector", + path: "embedding", + numDimensions: 8, + similarity: "euclidean", + quantization: "scalar", + }, + ]); const response = await integration.mcpClient().callTool({ name: "insert-many", @@ -189,7 +177,7 @@ describeWithMongoDB( "- Field embedding is an embedding with 8 dimensions and scalar quantization, and the provided value is not compatible." ); - const oopsieCount = await provider.countDocuments(integration.randomDbName(), "test", { + const oopsieCount = await collection.countDocuments({ embedding: "oopsie", }); expect(oopsieCount).toBe(0); diff --git a/tests/integration/tools/mongodb/delete/dropIndex.test.ts b/tests/integration/tools/mongodb/delete/dropIndex.test.ts index 18cc2d7e..d606de43 100644 --- a/tests/integration/tools/mongodb/delete/dropIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropIndex.test.ts @@ -8,15 +8,16 @@ import { getResponseContent, validateThrowsForInvalidArguments, validateToolMetadata, - waitUntilSearchIndexIsListed, - waitUntilSearchManagementServiceIsReady, } from "../../../helpers.js"; -import { describeWithMongoDB, type MongoDBIntegrationTestCase } from "../mongodbHelpers.js"; +import { + describeWithMongoDB, + waitUntilSearchIndexIsListed, + waitUntilSearchIsReady, + type MongoDBIntegrationTestCase, +} from "../mongodbHelpers.js"; import { createMockElicitInput } from "../../../../utils/elicitationMocks.js"; import { Elicitation } from "../../../../../src/elicitation.js"; -const SEARCH_TIMEOUT = 20_000; - function setupForClassicIndexes(integration: MongoDBIntegrationTestCase): { getMoviesCollection: () => Collection; getIndexName: () => string; @@ -66,12 +67,12 @@ function setupForVectorSearchIndexes(integration: MongoDBIntegrationTestCase): { plot: "This is a horrible movie about a database called BongoDB and how it tried to copy the OG MangoDB.", }, ]); - await waitUntilSearchManagementServiceIsReady(moviesCollection, SEARCH_TIMEOUT); + await waitUntilSearchIsReady(mongoClient); await moviesCollection.createSearchIndex({ name: indexName, definition: { mappings: { dynamic: true } }, }); - await waitUntilSearchIndexIsListed(moviesCollection, indexName, SEARCH_TIMEOUT); + await waitUntilSearchIndexIsListed(moviesCollection, indexName); }); afterEach(async () => { @@ -366,7 +367,7 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( }); describe("and attempting to delete an existing index", () => { - it("should succeed in deleting the index", { timeout: SEARCH_TIMEOUT }, async () => { + it("should succeed in deleting the index", async () => { const response = await integration.mcpClient().callTool({ name: "drop-index", arguments: { diff --git a/tests/integration/tools/mongodb/mongodbHelpers.ts b/tests/integration/tools/mongodb/mongodbHelpers.ts index e49ce25c..d53c97df 100644 --- a/tests/integration/tools/mongodb/mongodbHelpers.ts +++ b/tests/integration/tools/mongodb/mongodbHelpers.ts @@ -1,7 +1,7 @@ import path from "path"; import { fileURLToPath } from "url"; import fs from "fs/promises"; -import type { Document } from "mongodb"; +import type { Collection, Document } from "mongodb"; import { MongoClient, ObjectId } from "mongodb"; import type { IntegrationTest } from "../../helpers.js"; import { @@ -10,16 +10,17 @@ import { defaultTestConfig, defaultDriverOptions, getDataFromUntrustedContent, - sleep, } from "../../helpers.js"; import type { UserConfig, DriverOptions } from "../../../../src/common/config.js"; -import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { EJSON } from "bson"; import { MongoDBClusterProcess } from "./mongodbClusterProcess.js"; import type { MongoClusterConfiguration } from "./mongodbClusterProcess.js"; -import type { NodeDriverServiceProvider } from "@mongosh/service-provider-node-driver"; import type { createMockElicitInput, MockClientCapabilities } from "../../../utils/elicitationMocks.js"; +export const DEFAULT_WAIT_TIMEOUT = 1000; +export const DEFAULT_RETRY_INTERVAL = 100; + const __dirname = path.dirname(fileURLToPath(import.meta.url)); const testDataDumpPath = path.join(__dirname, "..", "..", "..", "accuracy", "test-data-dumps"); @@ -280,78 +281,100 @@ export async function getServerVersion(integration: MongoDBIntegrationTestCase): const serverStatus = await client.db("admin").admin().serverStatus(); return serverStatus.version as string; } - -const SEARCH_RETRIES = 200; -const SEARCH_WAITING_TICK = 100; +export const SEARCH_WAIT_TIMEOUT = 20_000; export async function waitUntilSearchIsReady( - provider: NodeDriverServiceProvider, - abortSignal: AbortSignal + mongoClient: MongoClient, + timeout: number = SEARCH_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL +): Promise { + await vi.waitFor( + async () => { + const testCollection = mongoClient.db("tempDB").collection("tempCollection"); + await testCollection.insertOne({ field1: "yay" }); + await testCollection.createSearchIndexes([{ definition: { mappings: { dynamic: true } } }]); + await testCollection.drop(); + }, + { timeout, interval } + ); +} + +async function waitUntilSearchIndexIs( + collection: Collection, + searchIndex: string, + indexValidator: (index: { name: string; queryable: boolean }) => boolean, + timeout: number, + interval: number, + getValidationFailedMessage: (searchIndexes: Document[]) => string = () => "Search index did not pass validation" ): Promise { - let lastError: unknown = null; - - for (let i = 0; i < SEARCH_RETRIES && !abortSignal.aborted; i++) { - try { - await provider.insertOne("tmp", "test", { field1: "yay" }); - await provider.createSearchIndexes("tmp", "test", [{ definition: { mappings: { dynamic: true } } }]); - await provider.dropCollection("tmp", "test"); - return; - } catch (err) { - lastError = err; - await sleep(100); + await vi.waitFor( + async () => { + const searchIndexes = (await collection.listSearchIndexes(searchIndex).toArray()) as { + name: string; + queryable: boolean; + }[]; + + if (!searchIndexes.some((index) => indexValidator(index))) { + throw new Error(getValidationFailedMessage(searchIndexes)); + } + }, + { + timeout, + interval, } - } + ); +} - throw new Error(`Search Management Index is not ready.\nlastError: ${JSON.stringify(lastError)}`); +export async function waitUntilSearchIndexIsListed( + collection: Collection, + searchIndex: string, + timeout: number = SEARCH_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL +): Promise { + return waitUntilSearchIndexIs( + collection, + searchIndex, + (index) => index.name === searchIndex, + timeout, + interval, + (searchIndexes) => + `Index ${searchIndex} is not yet in the index list (${searchIndexes.map(({ name }) => String(name)).join(", ")})` + ); } export async function waitUntilSearchIndexIsQueryable( - provider: NodeDriverServiceProvider, - database: string, - collection: string, - indexName: string, - abortSignal: AbortSignal + collection: Collection, + searchIndex: string, + timeout: number = SEARCH_WAIT_TIMEOUT, + interval: number = DEFAULT_RETRY_INTERVAL ): Promise { - let lastIndexStatus: unknown = null; - let lastError: unknown = null; - - for (let i = 0; i < SEARCH_RETRIES && !abortSignal.aborted; i++) { - try { - const [indexStatus] = await provider.getSearchIndexes(database, collection, indexName); - lastIndexStatus = indexStatus; - - if (indexStatus?.queryable === true) { - return; - } - } catch (err) { - lastError = err; - await sleep(SEARCH_WAITING_TICK); + return waitUntilSearchIndexIs( + collection, + searchIndex, + (index) => index.name === searchIndex && index.queryable, + timeout, + interval, + (searchIndexes) => { + const index = searchIndexes.find((index) => index.name === searchIndex); + return `Index ${searchIndex} in ${collection.dbName}.${collection.collectionName} is not ready. Last known status - ${JSON.stringify(index)}`; } - } - - throw new Error( - `Index ${indexName} in ${database}.${collection} is not ready: -lastIndexStatus: ${JSON.stringify(lastIndexStatus)} -lastError: ${JSON.stringify(lastError)}` ); } export async function createVectorSearchIndexAndWait( - provider: NodeDriverServiceProvider, + mongoClient: MongoClient, database: string, collection: string, - fields: Document[], - abortSignal: AbortSignal + fields: Document[] ): Promise { - await provider.createSearchIndexes(database, collection, [ - { - name: "default", - type: "vectorSearch", - definition: { - fields, - }, + const coll = await mongoClient.db(database).createCollection(collection); + await coll.createSearchIndex({ + name: "default", + type: "vectorSearch", + definition: { + fields, }, - ]); + }); - await waitUntilSearchIndexIsQueryable(provider, database, collection, "default", abortSignal); + await waitUntilSearchIndexIsQueryable(coll, "default"); } diff --git a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts index fb125844..39903796 100644 --- a/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts +++ b/tests/integration/tools/mongodb/search/listSearchIndexes.test.ts @@ -1,5 +1,10 @@ import type { Collection } from "mongodb"; -import { describeWithMongoDB, getSingleDocFromUntrustedContent } from "../mongodbHelpers.js"; +import { + describeWithMongoDB, + getSingleDocFromUntrustedContent, + waitUntilSearchIndexIsQueryable, + waitUntilSearchIsReady, +} from "../mongodbHelpers.js"; import { describe, it, expect, beforeEach } from "vitest"; import { getResponseContent, @@ -8,8 +13,6 @@ import { validateThrowsForInvalidArguments, databaseCollectionInvalidArgs, getDataFromUntrustedContent, - waitUntilSearchManagementServiceIsReady, - waitUntilSearchIndexIsQueryable, } from "../../../helpers.js"; import type { SearchIndexWithStatus } from "../../../../../src/tools/mongodb/search/listSearchIndexes.js"; @@ -47,7 +50,7 @@ describeWithMongoDB( beforeEach(async () => { await integration.connectMcpClient(); fooCollection = integration.mongoClient().db("any").collection("foo"); - await waitUntilSearchManagementServiceIsReady(fooCollection, SEARCH_TIMEOUT); + await waitUntilSearchIsReady(integration.mongoClient(), SEARCH_TIMEOUT); }); describe("when the collection does not exist", () => { @@ -79,7 +82,7 @@ describeWithMongoDB( describe("when there are indexes", () => { beforeEach(async () => { await fooCollection.insertOne({ field1: "yay" }); - await waitUntilSearchManagementServiceIsReady(fooCollection, SEARCH_TIMEOUT); + await waitUntilSearchIsReady(integration.mongoClient(), SEARCH_TIMEOUT); await fooCollection.createSearchIndexes([{ definition: { mappings: { dynamic: true } } }]); }); From d22309884435cae34934b6abbd5aae895688329c Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 14:51:28 +0200 Subject: [PATCH 15/17] chore: added a few more tests --- .../tools/mongodb/create/insertMany.test.ts | 1 - .../tools/mongodb/delete/dropIndex.test.ts | 57 ++++++++++++------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/tests/integration/tools/mongodb/create/insertMany.test.ts b/tests/integration/tools/mongodb/create/insertMany.test.ts index dea5ecef..0bb24c8a 100644 --- a/tests/integration/tools/mongodb/create/insertMany.test.ts +++ b/tests/integration/tools/mongodb/create/insertMany.test.ts @@ -109,7 +109,6 @@ describeWithMongoDB("insertMany tool when search is disabled", (integration) => describeWithMongoDB( "insertMany tool when search is enabled", (integration) => { - // let provider: NodeDriverServiceProvider; let collection: Collection; beforeEach(async () => { diff --git a/tests/integration/tools/mongodb/delete/dropIndex.test.ts b/tests/integration/tools/mongodb/delete/dropIndex.test.ts index d606de43..7a7b871e 100644 --- a/tests/integration/tools/mongodb/delete/dropIndex.test.ts +++ b/tests/integration/tools/mongodb/delete/dropIndex.test.ts @@ -343,28 +343,45 @@ describe.each([{ vectorSearchEnabled: false }, { vectorSearchEnabled: true }])( (integration) => { const { getIndexName } = setupForVectorSearchIndexes(integration); - describe("and attempting to delete a non-existent index", () => { - it("should fail with appropriate error", async () => { - const response = await integration.mcpClient().callTool({ - name: "drop-index", - arguments: { - database: "any", - collection: "foo", - indexName: "non-existent", - type: "search", - }, - }); - expect(response.isError).toBe(true); - const content = getResponseContent(response.content); - expect(content).toContain("Index does not exist in the provided namespace."); + describe.each([ + { + title: "an index from non-existent database", + database: "non-existent-db", + collection: "non-existent-coll", + indexName: "non-existent-index", + }, + { + title: "an index from non-existent collection", + database: "mflix", + collection: "non-existent-coll", + indexName: "non-existent-index", + }, + { + title: "a non-existent index", + database: "mflix", + collection: "movies", + indexName: "non-existent-index", + }, + ])( + "and attempting to delete $title (namespace - $database $collection)", + ({ database, collection, indexName }) => { + it("should fail with appropriate error", async () => { + const response = await integration.mcpClient().callTool({ + name: "drop-index", + arguments: { database, collection, indexName, type: "search" }, + }); + expect(response.isError).toBe(true); + const content = getResponseContent(response.content); + expect(content).toContain("Index does not exist in the provided namespace."); - const data = getDataFromUntrustedContent(content); - expect(JSON.parse(data)).toMatchObject({ - indexName: "non-existent", - namespace: "any.foo", + const data = getDataFromUntrustedContent(content); + expect(JSON.parse(data)).toMatchObject({ + indexName, + namespace: `${database}.${collection}`, + }); }); - }); - }); + } + ); describe("and attempting to delete an existing index", () => { it("should succeed in deleting the index", async () => { From 97810c6b3fcb84204cf2a93e1793359d8f9bae5c Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 15:07:34 +0200 Subject: [PATCH 16/17] chore: accuracy tests for dropping indexes --- tests/accuracy/dropIndex.test.ts | 177 ++++++++++++------ .../dropIndex.vectorSearchDisabled.test.ts | 96 ++++++++++ 2 files changed, 212 insertions(+), 61 deletions(-) create mode 100644 tests/accuracy/dropIndex.vectorSearchDisabled.test.ts diff --git a/tests/accuracy/dropIndex.test.ts b/tests/accuracy/dropIndex.test.ts index 82e76075..d5df1182 100644 --- a/tests/accuracy/dropIndex.test.ts +++ b/tests/accuracy/dropIndex.test.ts @@ -1,79 +1,134 @@ import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; import { describeAccuracyTests } from "./sdk/describeAccuracyTests.js"; import { Matcher } from "./sdk/matcher.js"; +import { formatUntrustedData } from "../../src/tools/tool.js"; // We don't want to delete actual indexes const mockedTools = { "drop-index": ({ indexName, database, collection }: Record): CallToolResult => { return { - content: [ - { - text: `Successfully dropped the index with name "${String(indexName)}" from the provided namespace "${String(database)}.${String(collection)}".`, - type: "text", - }, - ], + content: formatUntrustedData( + "Successfully dropped the index from the provided namespace.", + JSON.stringify({ + indexName, + namespace: `${database as string}.${collection as string}`, + }) + ), }; }, } as const; -describeAccuracyTests([ - { - prompt: "Delete the index called year_1 from mflix.movies namespace", - expectedToolCalls: [ - { - toolName: "drop-index", - parameters: { - database: "mflix", - collection: "movies", - indexName: "year_1", +describeAccuracyTests( + [ + { + prompt: "Delete the index called year_1 from mflix.movies namespace", + expectedToolCalls: [ + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: "year_1", + type: "classic", + }, }, - }, - ], - mockedTools, - }, - { - prompt: "First create a text index on field 'title' in 'mflix.movies' namespace and then drop all the indexes from 'mflix.movies' namespace", - expectedToolCalls: [ - { - toolName: "create-index", - parameters: { - database: "mflix", - collection: "movies", - name: Matcher.anyOf(Matcher.undefined, Matcher.string()), - definition: [ - { - keys: { - title: "text", + ], + mockedTools, + }, + { + prompt: "First create a text index on field 'title' in 'mflix.movies' namespace and then drop all the indexes from 'mflix.movies' namespace", + expectedToolCalls: [ + { + toolName: "create-index", + parameters: { + database: "mflix", + collection: "movies", + name: Matcher.anyOf(Matcher.undefined, Matcher.string()), + definition: [ + { + keys: { + title: "text", + }, + type: "classic", }, - type: "classic", - }, - ], + ], + }, }, - }, - { - toolName: "collection-indexes", - parameters: { - database: "mflix", - collection: "movies", + { + toolName: "collection-indexes", + parameters: { + database: "mflix", + collection: "movies", + }, }, - }, - { - toolName: "drop-index", - parameters: { - database: "mflix", - collection: "movies", - indexName: Matcher.string(), + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: Matcher.string(), + type: "classic", + }, }, - }, - { - toolName: "drop-index", - parameters: { - database: "mflix", - collection: "movies", - indexName: Matcher.string(), + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: Matcher.string(), + type: "classic", + }, }, - }, - ], - mockedTools, - }, -]); + ], + mockedTools, + }, + { + prompt: "Create a vector search index on 'mflix.movies' namespace on the 'plotSummary' field. The index should use 1024 dimensions. Confirm that its created and then drop the index.", + expectedToolCalls: [ + { + toolName: "create-index", + parameters: { + database: "mflix", + collection: "movies", + name: Matcher.anyOf(Matcher.undefined, Matcher.string()), + definition: [ + { + type: "vectorSearch", + fields: [ + { + type: "vector", + path: "plotSummary", + numDimensions: 1024, + }, + ], + }, + ], + }, + }, + { + toolName: "collection-indexes", + parameters: { + database: "mflix", + collection: "movies", + }, + }, + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: Matcher.string(), + type: "search", + }, + }, + ], + mockedTools, + }, + ], + { + userConfig: { + voyageApiKey: "voyage-api-key", + }, + clusterConfig: { search: true }, + } +); diff --git a/tests/accuracy/dropIndex.vectorSearchDisabled.test.ts b/tests/accuracy/dropIndex.vectorSearchDisabled.test.ts new file mode 100644 index 00000000..eca25090 --- /dev/null +++ b/tests/accuracy/dropIndex.vectorSearchDisabled.test.ts @@ -0,0 +1,96 @@ +/** + * Accuracy tests for when the vector search feature flag is disabled. + * + * TODO: Remove this file once we permanently enable the vector search feature. + */ +import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js"; +import { describeAccuracyTests } from "./sdk/describeAccuracyTests.js"; +import { Matcher } from "./sdk/matcher.js"; +import { formatUntrustedData } from "../../src/tools/tool.js"; + +// We don't want to delete actual indexes +const mockedTools = { + "drop-index": ({ indexName, database, collection }: Record): CallToolResult => { + return { + content: formatUntrustedData( + "Successfully dropped the index from the provided namespace.", + JSON.stringify({ + indexName, + namespace: `${database as string}.${collection as string}`, + }) + ), + }; + }, +} as const; + +describeAccuracyTests( + [ + { + prompt: "Delete the index called year_1 from mflix.movies namespace", + expectedToolCalls: [ + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: "year_1", + type: Matcher.anyOf(Matcher.undefined, Matcher.value("classic")), + }, + }, + ], + mockedTools, + }, + { + prompt: "First create a text index on field 'title' in 'mflix.movies' namespace and then drop all the indexes from 'mflix.movies' namespace", + expectedToolCalls: [ + { + toolName: "create-index", + parameters: { + database: "mflix", + collection: "movies", + name: Matcher.anyOf(Matcher.undefined, Matcher.string()), + definition: [ + { + keys: { + title: "text", + }, + type: "classic", + }, + ], + }, + }, + { + toolName: "collection-indexes", + parameters: { + database: "mflix", + collection: "movies", + }, + }, + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: Matcher.string(), + type: Matcher.anyOf(Matcher.undefined, Matcher.value("classic")), + }, + }, + { + toolName: "drop-index", + parameters: { + database: "mflix", + collection: "movies", + indexName: Matcher.string(), + type: Matcher.anyOf(Matcher.undefined, Matcher.value("classic")), + }, + }, + ], + mockedTools, + }, + ], + { + userConfig: { + voyageApiKey: "", + }, + } +); From 0817688b211bfe7e18890f8550155061e1fa6d70 Mon Sep 17 00:00:00 2001 From: Himanshu Singh Date: Thu, 16 Oct 2025 15:16:52 +0200 Subject: [PATCH 17/17] chore: replace another instance of provider with mongoclient --- tests/integration/tools/mongodb/create/createIndex.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/integration/tools/mongodb/create/createIndex.test.ts b/tests/integration/tools/mongodb/create/createIndex.test.ts index cb468d9f..161a8fb1 100644 --- a/tests/integration/tools/mongodb/create/createIndex.test.ts +++ b/tests/integration/tools/mongodb/create/createIndex.test.ts @@ -315,9 +315,7 @@ describeWithMongoDB( it("fails to create a vector search index", async () => { await integration.connectMcpClient(); const collection = new ObjectId().toString(); - await integration - .mcpServer() - .session.serviceProvider.createCollection(integration.randomDbName(), collection); + await integration.mongoClient().db(integration.randomDbName()).createCollection(collection); const response = await integration.mcpClient().callTool({ name: "create-index",