vector-search tool added with embedding model support #591
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds vector search functionality to the MCP server with embedding model support. It introduces new tools for performing vector similarity searches using MongoDB's $vectorSearch aggregation stage, along with Azure AI Inference embedding provider integration and Azure Managed Identity authentication for HTTP transport.
Key changes:
- Two vector search tool variants (V1 and V2) with different configuration approaches
- Azure AI Inference embedding provider with retry logic and factory pattern
- Azure Managed Identity authentication middleware with JWT verification
- Enhanced configuration options for embedding models and authentication
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/mongodb/read/vectorSearchv1.ts | V1 vector search tool requiring path/index arguments |
| src/tools/mongodb/read/vectorSearchv2.ts | V2 vector search tool using pre-configured defaults |
| src/embedding/* | Embedding provider interface, factory, and Azure AI implementation |
| src/transports/azureManagedIdentityAuth.ts | JWT authentication middleware for HTTP transport |
| src/common/config.ts | Configuration schema updates for vector search and auth |
| tests/* | Comprehensive test coverage for new functionality |
| attempt++; | ||
| } | ||
|
|
||
| throw new Error(`Embedding request ultimately failed after ${maxRetries + 1} attempt(s): ${lastError?.message}`); |
There was a problem hiding this comment.
The error message could be more informative by including the original request details. Consider adding context about what was being embedded when the failure occurred.
| throw new Error(`Embedding request ultimately failed after ${maxRetries + 1} attempt(s): ${lastError?.message}`); | |
| throw new Error( | |
| `Embedding request ultimately failed after ${maxRetries + 1} attempt(s): ${lastError?.message}\n` + | |
| `Request details: input.length=${input.length}, input[0]=${JSON.stringify(input[0])}, model/deployment=${this.config.deployment}, endpoint=${this.config.endpoint}` | |
| ); |
| if (!expectedAud) { | ||
| logger.warning({ | ||
| id: 0 as any, | ||
| context: "azureManagedIdentityAuth", | ||
| message: "No audience or clientId configured; 'aud' claim will not be enforced.", | ||
| }); | ||
| } |
There was a problem hiding this comment.
The log ID is hardcoded as 0 as any which bypasses the type system. This should use a proper LogId constant like LogId.azureManagedIdentityAuthError for consistency with the logging framework.
| userConfig.azureManagedIdentityRequiredRoles = commaSeparatedToArray( | ||
| userConfig.azureManagedIdentityRequiredRoles | ||
| ); | ||
| userConfig.azureManagedIdentityAllowedAppIds = commaSeparatedToArray( | ||
| userConfig.azureManagedIdentityAllowedAppIds | ||
| ); |
There was a problem hiding this comment.
The configuration setup repeats similar patterns for array processing. Consider extracting this into a helper function to reduce code duplication and make the configuration setup more maintainable.
1 participant
Proposed changes
Checklist