feat: adds support for dropping vector search indexes in drop-index tool MCP-239 #642

himanshusinghs · 2025-10-13T12:57:01Z

Proposed changes

Adds support for dropping vector search indexes in drop-index tool. The functionality is behind a feature flag.

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.

Checklist

I have signed the MongoDB CLA

Copilot

Pull Request Overview

This PR introduces a new drop-search-index tool for dropping search indexes from MongoDB collections. The tool is currently restricted to test environments and includes elicitation (confirmation) functionality for safety.

Introduces a new drop-search-index tool with proper validation, error handling, and elicitation support
Refactors existing search index functionality to support code reuse between list and drop operations
Adds comprehensive test coverage including unit, integration, and elicitation tests

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/tools/mongodb/delete/dropSearchIndex.ts	New tool implementation for dropping search indexes with validation and confirmation
src/tools/mongodb/search/listSearchIndexes.ts	Refactored to extract reusable search index retrieval logic
src/tools/mongodb/tools.ts	Added the new tool to the exports list
src/common/config.ts	Added drop-search-index to confirmation required tools list
tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts	Comprehensive test suite for the new tool
tests/integration/tools/mongodb/search/listSearchIndexes.test.ts	Updated to use refactored helper functions
tests/integration/helpers.ts	Added reusable helper functions for search index operations
tests/integration/tools/mongodb/mongodbClusterProcess.ts	Added directConnection parameter to connection string

src/tools/mongodb/delete/dropSearchIndex.ts

tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts

coveralls · 2025-10-13T13:14:40Z

Pull Request Test Coverage Report for Build 18466847504

Details

89 of 89 (100.0%) changed or added relevant lines in 4 files are covered.
2 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.1%) to 82.334%

Files with Coverage Reduction	New Missed Lines	%
src/common/atlas/apiClientError.ts	2	87.65%

Totals
Change from base Build 18382799942:	0.1%
Covered Lines:	5686
Relevant Lines:	6778

💛 - Coveralls

- 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.

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

kmruiz · 2025-10-15T15:59:15Z

tests/integration/tools/mongodb/mongodbClusterProcess.ts

                () => runningContainer.stop(),
-                () => `mongodb://${runningContainer.getHost()}:${runningContainer.getMappedPort(27017)}`
+                () =>
+                    `mongodb://${runningContainer.getHost()}:${runningContainer.getMappedPort(27017)}/?directConnection=true`


Why do we need the direct connection here? It shouldn't be necessary.

Unfortunately it is. Without it the connection just keeps hanging because driver tries to resolve involved clusters and since the internal ports are never exposed the driver is unable to connect. BTW this is also what is recommended by the CLI when you kick start a deployment.

I see that in your PR you're using the service provider from server instance itself and because our connection code handles it internally (in generateConnectionInfoFromCliArgs) it works fine but for tests I think it would be better to continue using the test mongo client provided by the MdbIntegration.

tests/integration/helpers.ts

kmruiz · 2025-10-15T16:01:47Z

tests/integration/helpers.ts

+    }
+}
+
+export async function waitUntilSearchManagementServiceIsReady(


Why aren't we using waitUntilSearchIsReady

Probably you should take a look at mongodbHelpers.ts and see what can be reused and refactor.

These are the same methods. I just renamed / refactored them to re-use parts of the logic.

kmruiz · 2025-10-15T16:04:28Z

src/helpers/searchErrorHandler.ts

+import { type DbOperationArgs, MongoDBToolBase } from "../tools/mongodb/mongodbTool.js";
+import type { ToolArgs } from "../tools/tool.js";
+
+export abstract class MongoDBToolWithSearchErrorHandler extends MongoDBToolBase {


Not sure we want this. In #626 we are using the MongoDBToolBase, and it would be best to have all errors centralised in one single place.

I just now stumbled upon it while I was going through your PR. Certainly that's more preferable.

himanshusinghs requested a review from a team as a code owner October 13, 2025 12:57

Copilot AI review requested due to automatic review settings October 13, 2025 12:57

Copilot AI reviewed Oct 13, 2025

View reviewed changes

src/tools/mongodb/delete/dropSearchIndex.ts Outdated Show resolved Hide resolved

tests/integration/tools/mongodb/delete/dropSearchIndex.test.ts Outdated Show resolved Hide resolved

himanshusinghs changed the title ~~feat: introduces drop-search-index tool~~ feat: introduces drop-search-index tool MCP-239 Oct 13, 2025

himanshusinghs force-pushed the feat/MCP-239-delete-vector-search-indexes branch from 4e0e3e0 to 3df48c4 Compare October 13, 2025 13:04

himanshusinghs force-pushed the feat/MCP-239-delete-vector-search-indexes branch 4 times, most recently from 10f49db to d6f1b91 Compare October 15, 2025 11:55

himanshusinghs added 10 commits October 15, 2025 17:39

chore: fix isError property placement with tests

1065ee8

chore: fix isError property on listSearchIndexes tool with tests

f26ae1a

chore: remove redundant call

8960ffa

chore: remove unused import

eed4479

chore: addresses a few common concerns

52dc7c5

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

chore: use Zod.string instead of CommonArgs.string

746643b

chore: adapt test for new interface

00f2212

chore: merge drop-search-index into drop-index tool

716d9cb

chore: use common logic from createIndex

7d795ca

himanshusinghs force-pushed the feat/MCP-239-delete-vector-search-indexes branch from effaac6 to 7d795ca Compare October 15, 2025 15:39

himanshusinghs changed the title ~~feat: introduces drop-search-index tool MCP-239~~ feat: adds vector search drop support to drop-index tool MCP-239 Oct 15, 2025

himanshusinghs changed the title ~~feat: adds vector search drop support to drop-index tool MCP-239~~ feat: adds support for dropping vector search indexes in drop-index tool MCP-239 Oct 15, 2025

kmruiz reviewed Oct 15, 2025

View reviewed changes

tests/integration/helpers.ts Outdated Show resolved Hide resolved

kmruiz reviewed Oct 15, 2025

View reviewed changes

chore: replace with vi.waitFor

7db2402

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: adds support for dropping vector search indexes in drop-index tool MCP-239 #642

feat: adds support for dropping vector search indexes in drop-index tool MCP-239 #642

himanshusinghs commented Oct 13, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

coveralls commented Oct 13, 2025 •

edited

Loading

Uh oh!

kmruiz Oct 15, 2025

Uh oh!

himanshusinghs Oct 15, 2025

Uh oh!

himanshusinghs Oct 15, 2025

Uh oh!

Uh oh!

kmruiz Oct 15, 2025

Uh oh!

kmruiz Oct 15, 2025

Uh oh!

himanshusinghs Oct 15, 2025

Uh oh!

kmruiz Oct 15, 2025

Uh oh!

himanshusinghs Oct 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: adds support for dropping vector search indexes in drop-index tool MCP-239 #642

Are you sure you want to change the base?

feat: adds support for dropping vector search indexes in drop-index tool MCP-239 #642

Conversation

himanshusinghs commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

coveralls commented Oct 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18466847504

Details

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

himanshusinghs commented Oct 13, 2025 •

edited

Loading

coveralls commented Oct 13, 2025 •

edited

Loading