Skip to content

fix: use dynamic database lookup for search index check#1095

Open
nirinchev wants to merge 6 commits intomainfrom
ni/search-tests
Open

fix: use dynamic database lookup for search index check#1095
nirinchev wants to merge 6 commits intomainfrom
ni/search-tests

Conversation

@nirinchev
Copy link
Copy Markdown
Collaborator

Based on #1091 - uses a dynamic list of databases to check for search index support instead of the hardcoded #mongodb-mcp which may not be accessible for the db user.

Fixes #1022

Will-hxw and others added 3 commits April 21, 2026 06:45
…check

Use listDatabases() to find an accessible non-system database instead of
hardcoded '#mongodb-mcp' which may not exist or be accessible when the
connection string restricts database access.
Copilot AI review requested due to automatic review settings April 23, 2026 21:19
@nirinchev nirinchev requested a review from a team as a code owner April 23, 2026 21:19
@nirinchev nirinchev requested review from jeroenvervaeke and removed request for a team April 23, 2026 21:19
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Atlas Search capability detection to avoid relying on a single hardcoded probe database, improving compatibility with users whose connection string/roles restrict database access (Issue #1022).

Changes:

  • Update ConnectionStateConnected.isSearchSupported() to probe multiple candidate databases (initial DB + listDatabases() results + fallback) and treat SearchNotEnabled as the definitive “not supported” signal.
  • Add logging for the search capability probe and thread a logger through Session.isSearchSupported().
  • Expand unit/integration test coverage, including spinning up auth-enabled MongoDB test clusters with multiple users and restricted privileges.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/common/connectionManager.ts Implements the new multi-database probe logic and caches the probe result.
src/common/session.ts Passes the session logger into the connected state’s isSearchSupported() call.
src/common/logging/loggingDefinitions.ts Adds a new log ID for probe-related debug logging.
tests/unit/common/session.test.ts Updates mocks and assertions for the new probe behavior and error handling.
tests/integration/common/connectionManager.test.ts Adds integration coverage for probe candidate selection, failure modes, and caching.
tests/integration/tools/mongodb/mongodbHelpers.ts Adds a helper to build per-user connection strings for restricted-user scenarios.
tests/integration/tools/mongodb/mongodbClusterProcess.ts Adds support for creating multiple users on mongo-runner clusters and generating per-user URIs.
api-extractor/reports/web.public.api.md Updates public API report for the new isSearchSupported(logger) signature.
api-extractor/reports/mongodb-mcp-server.public.api.md Updates public API report for the new isSearchSupported(logger) signature.

Comment thread src/common/connectionManager.ts
Comment thread src/common/connectionManager.ts
Comment thread tests/integration/tools/mongodb/mongodbClusterProcess.ts Outdated
Comment thread tests/integration/tools/mongodb/mongodbHelpers.ts
@coveralls
Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 24860576670

Warning

No base build found for commit 47fb11c on main.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 81.942%

Details

  • Patch coverage: 37 of 37 lines across 2 files are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 3609
Covered Lines: 3147
Line Coverage: 87.2%
Relevant Branches: 2261
Covered Branches: 1663
Branch Coverage: 73.55%
Branches in Coverage %: Yes
Coverage Strength: 175.62 hits per line

💛 - Coveralls

@Will-hxw
Copy link
Copy Markdown

Will-hxw commented Apr 24, 2026

I opened #1097 as a small compatibility patch on top of this branch. It keeps ConnectionStateConnected.isSearchSupported() callable without a logger by falling back to NullLogger, updates the API reports, and adds a focused unit test for the direct no-argument call path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: $search support detection relies on hard coded dummy db name

5 participants