feat(index-check): add index check functionality before query #309

Crushdada · 2025-06-18T14:43:04Z

Summary

This PR adds a new indexCheck configuration option that enforces MongoDB queries to use indexes, preventing collection scans (COLLSCAN) operations.

Changes

✅ Added indexCheck helper functions in src/helpers/indexCheck.ts
✅ Updated 5 MongoDB operations to support index checking: deleteMany, aggregate, count, find, updateMany
✅ Added configuration support in .smithery/smithery.yaml
✅ Updated README.md with comprehensive documentation
✅ Added comprehensive test suite with 7 passing tests

Features

Uses MongoDB's explain functionality to analyze query plans
Configurable via environment variables, CLI args, or platform config
Comprehensive error messages for debugging
Only applies to query operations (not inserts)

Testing

All existing tests pass
New unit tests achieve 57.89% code coverage
Integration tested with MCP server

Related Issues

Resolves #287

Documentation

Updated README.md with indexCheck configuration details
Added comprehensive examples and usage instructions
Updated .smithery/smithery.yaml following project patterns

kmruiz

Great job so far! Thanks a lot!

I've requested a few small changes that should make things more correct for newest MongoDB versions and one QoL that should simplify your code a bit.

Thanks a lot for contributing!

src/helpers/indexCheck.ts

Crushdada · 2025-06-20T15:06:38Z

@kmruiz :) Thank you for the detailed code review and suggestions! I have completed the following changes according to your feedback:

✅ Added ForbiddenCollscan error code and replaced generic Error with MongoDBError
✅ Extended index scan stage detection to support MongoDB 8.0+ stages (EXPRESS_IXSCAN, EXPRESS_CLUSTERED_IXSCAN, EXPRESS_UPDATE, EXPRESS_DELETE, IDHACK)
✅ Added comprehensive unit tests for all new index scan stages

Please review again, and feel free to provide any additional suggestions. Thank you again for your guidance !

kmruiz

Everything looks good @Crushdada ! I'm going to run the workflows and if everything is green and you don't mind, I'll merge the PR.

Thanks a lot for contributing! Really appreciate it!

kmruiz · 2025-06-20T16:03:55Z

@Crushdada I see that there are a few issues with the formatting and the usage of any. Would you mind fixing them?

You can run npm run check in your branch to see all the issues in your changes. Once you fix them I will run the tests and merge!

Thanks!

Crushdada · 2025-06-20T16:20:17Z

@kmruiz Thanks for catching that! Can I use "string | undefined" to solve the "any" type of problem?

kmruiz · 2025-06-20T16:26:11Z

Sure! ideally if you are working on an object with an unknown shape, ideally you should define a type and cast it so we know what is the basic shape for the object. For primitives, you can use the type and undefined. However we are not really strict here, if it's clear the purpose and the type, it will be fine.

himanshusinghs

Thank you for this @Crushdada 🚀 Added small comments. It would be nice to add a few integration tests for the added config change as well?

src/helpers/indexCheck.ts

himanshusinghs · 2025-06-20T16:17:35Z

src/tools/mongodb/delete/deleteMany.ts

+        // Check if delete operation uses an index if enabled
+        if (this.config.indexCheck) {
+            await checkIndexUsage(provider, database, collection, "deleteMany", async () => {
+                return provider.mongoClient.db(database).command({


The provider instance here provides interfaces (runCommand and runCommandWithCheck) for command execution. Please prefer those over accessing methods directly on MongoClient. The same applies to other instances of the MongoClient.db().command() usages.

I believe what you need here is runCommandWithCheck

Thanks for pointing out this issue, I have revised it. But I encountered a problem that confused me that the "code health" check failed. Should we increase the timeout at tests/integration/tools/mongodb/mongodbHelpers.ts:94 ?

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

kmruiz · 2025-06-26T10:34:25Z

I was looking at the failing test, and it seems to be failing with the latest changes. In main the test passes fine, so it's likely a regression we introduced with this PR related to the capability calculation of the server.

Crushdada · 2025-06-26T10:46:38Z

I was looking at the failing test, and it seems to be failing with the latest changes. In main the test passes fine, so it's likely a regression we introduced with this PR related to the capability calculation of the server.

Thank you. I will roll back the code to try to locate the cause of the test failure, but the test cannot even run normally on my mac. It always reports the error "mongodb cluster startup failure". Do you know how to solve it?

kmruiz · 2025-06-26T10:46:52Z

Seems that the issue is not related to this PR, but with an upgrade in the SDK that changes the default values for the MCP capabilities. The PR to fix the issue is here: #322. Whenever this is merged I'll run the checks again in your PR.

Sorry for the delay,

Crushdada · 2025-06-26T10:51:26Z

Seems that the issue is not related to this PR, but with an upgrade in the SDK that changes the default values for the MCP capabilities. The PR to fix the issue is here: #322. Whenever this is merged I'll run the checks again in your PR.看来问题与此 PR 无关，而是与 SDK 升级有关，该升级更改了 MCP 能力的默认值。修复问题的 PR 在这里： #322 。一旦合并，我会在你的 PR 中再次运行检查。

Sorry for the delay,抱歉延迟，

Wow, I see. Thank you very much. Once the merge of pr# 322 is completed, I will do the check I mentioned.

kmruiz · 2025-06-26T13:53:56Z

Thank you. I will roll back the code to try to locate the cause of the test failure, but the test cannot even run normally on my mac. It always reports the error "mongodb cluster startup failure". Do you know how to solve it?

Not sure! For running a mongodb cluster we use https://www.npmjs.com/package/mongodb-runner?activeTab=readme, which we maintain. Can you try to run the cluster manually using npx?

npx mongodb-runner start -t standalone
npx mongodb-runner stop --all

Edit: Before I forget, @Crushdada would you mind signing the contributor agreement if you haven't done it? Sorry for the bureaucracy we are still setting up everything for external contributors: https://www.mongodb.com/legal/contributor-agreement

@himanshusinghs , can you review again the PR? You are requesting changes, can you verify if it's ok to merge?

Crushdada · 2025-06-26T14:05:01Z

Thank you. I will roll back the code to try to locate the cause of the test failure, but the test cannot even run normally on my mac. It always reports the error "mongodb cluster startup failure". Do you know how to solve it?

Not sure! For running a mongodb cluster we use https://www.npmjs.com/package/mongodb-runner?activeTab=readme, which we maintain. Can you try to run the cluster manually using npx?
npx mongodb-runner start -t standalone
npx mongodb-runner stop --all
Edit: Before I forget, @Crushdada would you mind signing the contributor agreement if you haven't done it? Sorry for the bureaucracy we are still setting up everything for external contributors: https://www.mongodb.com/legal/contributor-agreement

@himanshusinghs , can you review again the PR? You are requesting changes, can you verify if it's ok to merge?

Done! I just submitted a contributor agreement.

himanshusinghs

Sorry for the delay here! Thank you for iterating on the feedback. It looks good to me 🚀

kmruiz · 2025-06-26T16:02:37Z

Merging! Thanks @Crushdada for the hard work and the patience to go through the feedback with us!

Crushdada requested a review from a team as a code owner June 18, 2025 14:43

kmruiz requested changes Jun 20, 2025

View reviewed changes

src/helpers/indexCheck.ts Show resolved Hide resolved

src/helpers/indexCheck.ts Outdated Show resolved Hide resolved

kmruiz approved these changes Jun 20, 2025

View reviewed changes

himanshusinghs requested changes Jun 20, 2025

View reviewed changes

Crushdada requested review from himanshusinghs and kmruiz June 21, 2025 03:45

Crushdada and others added 6 commits June 26, 2025 10:41

feat(index-check): add index check functionality before query

91e7b9b

feat(errors): enhance index check and proper error handling

92e68b0

fix(lint-errors): resolve ESLint formatting and type safety issues

e8e3065

Update src/helpers/indexCheck.ts

2dd9f95

Co-authored-by: Himanshu Singh <himanshu.singhs@outlook.in>

fix: use runCommandWithCheck for database commands

af5e401

feat: add integration tests for indexCheck config

3933710

Crushdada force-pushed the feature/index-check-before-query branch from c6b758e to 3933710 Compare June 26, 2025 02:44

fix: code style check failed

16a9130

Merge branch 'main' into feature/index-check-before-query

e5ce5fc

kmruiz approved these changes Jun 26, 2025

View reviewed changes

himanshusinghs approved these changes Jun 26, 2025

View reviewed changes

kmruiz merged commit d10b4e7 into mongodb-js:main Jun 26, 2025
17 checks passed

feat(index-check): add index check functionality before query #309

feat(index-check): add index check functionality before query #309

Conversation

Crushdada commented Jun 18, 2025

Summary

Changes

Features

Testing

Related Issues

Documentation

Uh oh!

kmruiz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Crushdada commented Jun 20, 2025

Uh oh!

kmruiz left a comment

Choose a reason for hiding this comment

Uh oh!

kmruiz commented Jun 20, 2025

Uh oh!

Crushdada commented Jun 20, 2025

Uh oh!

kmruiz commented Jun 20, 2025

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

himanshusinghs Jun 20, 2025

Choose a reason for hiding this comment

Uh oh!

Crushdada Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmruiz commented Jun 26, 2025

Uh oh!

Crushdada commented Jun 26, 2025

Uh oh!

kmruiz commented Jun 26, 2025

Uh oh!

Crushdada commented Jun 26, 2025

Uh oh!

kmruiz commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Crushdada commented Jun 26, 2025

Uh oh!

himanshusinghs left a comment

Choose a reason for hiding this comment

Uh oh!

kmruiz commented Jun 26, 2025

Uh oh!

Uh oh!

Uh oh!

Crushdada Jun 20, 2025 •

edited

Loading

kmruiz commented Jun 26, 2025 •

edited

Loading