Main to Dev

[jmaddington]

User description

Update dev branch with changes from main.

---

PR Type

Enhancement, Tests

---

Description

• Major architectural enhancements: Added comprehensive MCP (Model Context Protocol) manager with OAuth 2.0 support, connection pooling, and token management
• New memory management system: Implemented AI agent memory management with token limits, validation, and artifact handling
• MeiliSearch integration: Added MongoDB-MeiliSearch synchronization plugin with batch processing and search capabilities
• Web search functionality: Added comprehensive web search type definitions and authentication handling for multiple providers
• Extensive test coverage: Added 1570+ lines of Mistral OCR tests, comprehensive share methods tests, web search authentication tests, and agent resources test suites
• LaTeX processing improvements: Enhanced LaTeX preprocessing with currency detection and mhchem command handling
• File operations: Added Mistral file operations and comprehensive OCR service testing
• Code cleanup: Removed legacy MCP package files and migrated functionality to new architecture

---

Changes walkthrough 📝

	Relevant files
Tests	9 files crud.spec.ts Add comprehensive test suite for Mistral OCR CRUD operations packages/api/src/files/mistral/crud.spec.ts • Added comprehensive test suite for Mistral OCR service with 1570+ lines of tests • Implemented mock setup for fs, form-data, axios, and data-schemas modules • Created tests for document upload, signed URL retrieval, OCR processing, and Azure Mistral OCR • Added extensive test coverage for configuration handling, OAuth flows, and error scenarios +1570/-0 share.test.ts Add comprehensive test suite for share methods functionality packages/data-schemas/src/methods/share.test.ts • Added comprehensive test suite for share methods with 1043 lines of tests • Implemented tests for creating, retrieving, updating, and deleting shared links • Added test coverage for message anonymization, pagination, and access control • Included edge cases, error handling, and concurrent operation testing +1043/-0 web.spec.ts Add comprehensive web search authentication test suite      packages/data-provider/specs/web.spec.ts • Added comprehensive test suite for web search authentication functionality • Tests cover extractWebSearchEnvVars, loadWebSearchAuth, and webSearchAuth functions • Includes tests for environment variable extraction, authentication validation, and service configuration • Tests handle various scenarios including OCR capabilities, user vs system authentication, and error handling +903/-0  resources.test.ts Add comprehensive agent resources test suite                          packages/api/src/agents/resources.test.ts • Added extensive test suite for the primeResources function in agent resources • Tests cover OCR file handling, attachment processing, and tool resource management • Includes tests for file categorization, duplicate prevention, and error handling • Tests various scenarios with different file types and authentication states +990/-0  mcp.spec.ts Extend MCP test suite with schema validation and environment processing packages/data-provider/specs/mcp.spec.ts • Extended MCP (Model Context Protocol) test suite with new schema validation tests • Added comprehensive tests for StreamableHTTPOptionsSchema validation • Added extensive tests for processMCPEnv function covering environment variable processing • Tests include user field processing, custom variables, and complex placeholder scenarios +661/-1  bedrock.spec.ts Add Bedrock input parser test suite                                            packages/data-provider/specs/bedrock.spec.ts • Added test suite for Bedrock input parser functionality • Tests focus on model matching for reasoning configuration • Covers various Claude model variants and their thinking/reasoning capabilities • Tests explicit configuration overrides and custom thinking budgets +114/-0  latex.spec.ts Refactor and expand LaTeX preprocessing test suite              client/src/utils/latex.spec.ts • Refactors test suite to focus on preprocessLaTeX function instead of processLaTeX • Adds comprehensive test cases for currency detection and escaping • Includes tests for mhchem command handling and complex mixed content scenarios • Expands coverage for edge cases like code blocks and malformed expressions +142/-127 useQueryParams.spec.ts Add comprehensive test suite for query parameters hook      client/src/hooks/Input/useQueryParams.spec.ts • Creates comprehensive test suite for URL query parameter processing hook • Tests auto-submission functionality with various parameter combinations • Includes timeout handling and settings application scenarios • Covers edge cases like empty parameters and mixed content types +489/-0  zod.spec.ts Expand Zod schema conversion tests with new options            packages/data-provider/src/zod.spec.ts • Adds test coverage for new dropFields option in JSON schema conversion • Implements tests for transformOneOfAnyOf option with union type handling • Includes comprehensive test cases for nested schema transformations • Tests interaction between multiple conversion options +417/-6
Enhancement	6 files manager.ts Implement MCP Manager for Model Context Protocol connections packages/api/src/mcp/manager.ts • Added new MCPManager class for managing Model Context Protocol connections • Implemented singleton pattern with connection pooling for app-level and user-specific connections • Added OAuth handling, token storage, and flow management capabilities • Included idle connection cleanup, tool mapping, and instruction formatting features +1108/-0 mongoMeili.ts Add MeiliSearch-MongoDB synchronization plugin                      packages/data-schemas/src/models/plugins/mongoMeili.ts • Added complete MeiliSearch-MongoDB synchronization plugin implementation • Includes batch processing, streaming sync, and cleanup functionality • Provides search capabilities with population and custom indexing • Implements hooks for document lifecycle management and error handling +756/-0  handler.ts Add MCP OAuth 2.0 authentication handler implementation    packages/api/src/mcp/oauth/handler.ts • Implements comprehensive OAuth 2.0 handler for MCP (Model Context Protocol) servers • Supports both pre-configured OAuth settings and auto-discovery of OAuth metadata • Provides methods for initiating OAuth flows, completing authorization, and refreshing tokens • Includes robust error handling and logging throughout the OAuth process +603/-0  connection.ts Enhance MCP connection with OAuth and streamable HTTP support packages/api/src/mcp/connection.ts • Adds OAuth token support and authentication error handling to MCP connections • Implements streamable HTTP transport support alongside existing transports • Enhances error handling with OAuth-specific error detection and retry logic • Improves connection management with better timeout handling and debugging +249/-42 web.ts Add comprehensive web search type definitions                        packages/data-provider/src/types/web.ts • Defines comprehensive type definitions for web search functionality • Includes types for multiple search providers (Serper, SearXNG) and result formats • Provides interfaces for search results, scraping, and reranking operations • Supports various search types including images, videos, news, and places +593/-0  memory.ts Add AI agent memory management system implementation          packages/api/src/agents/memory.ts • Implements comprehensive memory management system for AI agents • Provides tools for setting, deleting, and managing user-specific memories • Includes token limit enforcement and validation for memory operations • Features artifact handling and streaming response support +468/-0
Miscellaneous	2 files index.ts Add index exports for memory components                                    client/src/components/SidePanel/Memories/index.ts • Added index file to export MemoryViewer and MemoryEditDialog components • Simple barrel export pattern for memory-related components +2/-0      index.ts Add file operations export module                                                packages/api/src/files/index.ts • Creates simple export file for Mistral file operations +1/-0
Additional files	101 files devcontainer.json +17/-1    docker-compose.yml +5/-4      .env.example +100/-9  CONTRIBUTING.md +35/-17  BUG-REPORT.yml +2/-0      backend-review.yml +9/-5      deploy-dev.yml +11/-6    dev-branch-images.yml +72/-0    generate-release-changelog-pr.yml +3/-2      generate-unreleased-changelog-pr.yml +3/-2      helmcharts.yml +7/-0      i18n-unused-keys.yml +32/-8    unused-packages.yml +2/-0      CHANGELOG.md +223/-3  Dockerfile +2/-1      Dockerfile.multi +14/-13  AnthropicClient.js +28/-21  BaseClient.js +8/-8      ChatGPTClient.js +5/-5      GoogleClient.js +12/-9    OllamaClient.js +5/-4      OpenAIClient.js +31/-23  generators.js +0/-71    createLLM.js +1/-2      AnthropicClient.test.js +309/-4  BaseClient.test.js +5/-3      OpenAIClient.test.js +4/-4      PluginsClient.test.js +1/-1      OpenAPIPlugin.js +0/-184  OpenAPIPlugin.spec.js +0/-72    index.js +2/-0      manifest.json +14/-0    DALLE3.js +2/-2      OpenAIImageTools.js +519/-0  TavilySearchResults.js +33/-3    WordPress.js +21/-5    DALLE3.spec.js +26/-2    addOpenAPISpecs.js +0/-30    addOpenAPISpecs.spec.js +0/-76    fileSearch.js +1/-1      handleTools.js +75/-38  handleTools.test.js +36/-16  loadSpecs.js +0/-117  loadSpecs.spec.js +0/-101  banViolation.js +1/-3      banViolation.spec.js +20/-40  getLogStores.js +11/-1    keyvRedis.js +10/-0    index.js +3/-57    connect.js +4/-1      index.js +8/-0      indexSync.js +174/-0  models.js +5/-0      jest.config.js +3/-0      index.js +0/-4      indexSync.js +0/-89    Action.js +1/-4      Agent.js +323/-20 Agent.spec.js +2580/-254 Assistant.js +1/-4      Balance.js +0/-4      Banner.js +3/-6      Config.js +0/-86    Conversation.js +2/-5      ConversationTag.js +8/-13    File.js +27/-19  Key.js +0/-4      Message.js +13/-5    Message.spec.js +196/-117 Preset.js +2/-3      Project.js +1/-4      Prompt.js +2/-6      Role.js +3/-37    Role.spec.js +3/-1      Session.js +0/-275  Share.js +0/-342  Token.js +0/-199  ToolCall.js +1/-3      Transaction.js +59/-62  Transaction.spec.js +3/-4      User.js +0/-6      balanceMethods.js +4/-4      convoStructure.spec.js +2/-2      index.js +5/-48    inviteUser.js +3/-3      mongoMeili.js +0/-475  convoSchema.js +0/-18    messageSchema.js +0/-16    pluginAuthSchema.js +0/-6      presetSchema.js +0/-6      spendTokens.js +5/-6      spendTokens.spec.js +12/-7    tx.js +19/-2    tx.spec.js +100/-0  userMethods.js +4/-162  package.json +18/-14  cleanup.js +15/-15  AskController.js +4/-3      AuthController.js +29/-4    Balance.js +19/-4    Additional files not shown

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns OAuth token handling: The MCP OAuth implementation stores and manages sensitive authentication tokens. The token storage and refresh mechanisms in MCPTokenStorage and MCPOAuthHandler need careful review to ensure tokens are properly encrypted at rest and securely transmitted. The flow management system should prevent token leakage through logs or error messages.

⚡ Recommended focus areas for review Test Complexity This test file contains 1570+ lines with extensive mocking and complex test scenarios. The mock setup is intricate with multiple axios calls and file stream mocking. Review the test coverage to ensure it adequately tests error conditions and edge cases without being overly complex or brittle. // Mock setup must be hoisted jest.mock('fs'); jest.mock('form-data', () => { return jest.fn().mockImplementation(() => ({ append: jest.fn(), getHeaders: jest .fn() .mockReturnValue({ 'content-type': 'multipart/form-data; boundary=---boundary' }), getBuffer: jest.fn().mockReturnValue(Buffer.from('mock-form-data')), getLength: jest.fn().mockReturnValue(100), })); }); jest.mock('axios', () => { const mockAxiosInstance = { get: jest.fn().mockResolvedValue({ data: {} }), post: jest.fn().mockResolvedValue({ data: {} }), put: jest.fn().mockResolvedValue({ data: {} }), delete: jest.fn().mockResolvedValue({ data: {} }), interceptors: { request: { use: jest.fn(), eject: jest.fn(), clear: jest.fn() }, response: { use: jest.fn(), eject: jest.fn(), clear: jest.fn() }, }, defaults: { proxy: null, }, }; return { ...mockAxiosInstance, create: jest.fn().mockReturnValue(mockAxiosInstance), }; }); jest.mock('@librechat/data-schemas', () => ({ logger: { error: jest.fn(), }, })); jest.mock('~/utils/axios', () => ({ createAxiosInstance: () => jest.requireMock('axios'), logAxiosError: jest.fn(({ message }) => message || 'Error'), })); import * as fs from 'fs'; import axios from 'axios'; import type { Request as ExpressRequest } from 'express'; import type { Readable } from 'stream'; import type { MistralFileUploadResponse, MistralSignedUrlResponse, OCRResult } from '~/types'; import { logger as mockLogger } from '@librechat/data-schemas'; import { uploadDocumentToMistral, uploadMistralOCR, uploadAzureMistralOCR, getSignedUrl, performOCR, } from './crud'; interface MockReadStream extends Partial<Readable> { on: jest.Mock; pipe: jest.Mock; pause: jest.Mock; resume: jest.Mock; emit: jest.Mock; once: jest.Mock; destroy: jest.Mock; path?: string; fd?: number; flags?: string; mode?: number; autoClose?: boolean; bytesRead?: number; closed?: boolean; pending?: boolean; } const mockAxios = jest.mocked(axios); const mockLoadAuthValues = jest.fn(); describe('MistralOCR Service', () => { afterEach(() => { jest.clearAllMocks(); }); describe('uploadDocumentToMistral', () => { beforeEach(() => { // Create a more complete mock for file streams that FormData can work with const mockReadStream: MockReadStream = { on: jest.fn().mockImplementation(function ( this: MockReadStream, event: string, handler: () => void, ) { // Simulate immediate 'end' event to make FormData complete processing if (event === 'end') { handler(); } return this; }), pipe: jest.fn().mockImplementation(function (this: MockReadStream) { return this; }), pause: jest.fn(), resume: jest.fn(), emit: jest.fn(), once: jest.fn(), destroy: jest.fn(), path: '/path/to/test.pdf', fd: 1, flags: 'r', mode: 0o666, autoClose: true, bytesRead: 0, closed: false, pending: false, }; (jest.mocked(fs).createReadStream as jest.Mock).mockReturnValue(mockReadStream); }); it('should upload a document to Mistral API using file streaming', async () => { const mockResponse: { data: MistralFileUploadResponse } = { data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'test.pdf', purpose: 'ocr', }, }; mockAxios.post!.mockResolvedValueOnce(mockResponse); try { const result = await uploadDocumentToMistral({ filePath: '/path/to/test.pdf', fileName: 'test.pdf', apiKey: 'test-api-key', }); // Check that createReadStream was called with the correct file path expect(jest.mocked(fs).createReadStream).toHaveBeenCalledWith('/path/to/test.pdf'); // Since we're mocking FormData, we'll just check that axios was called correctly expect(mockAxios.post).toHaveBeenCalledWith( 'https://api.mistral.ai/v1/files', expect.anything(), expect.objectContaining({ headers: expect.objectContaining({ Authorization: 'Bearer test-api-key', }), maxBodyLength: Infinity, maxContentLength: Infinity, }), ); expect(result).toEqual(mockResponse.data); } catch (error) { console.error('Test error:', error); throw error; } }); it('should handle errors during document upload', async () => { const errorMessage = 'API error'; mockAxios.post!.mockRejectedValueOnce(new Error(errorMessage)); await expect( uploadDocumentToMistral({ filePath: '/path/to/test.pdf', fileName: 'test.pdf', apiKey: 'test-api-key', }), ).rejects.toThrow(errorMessage); }); }); describe('getSignedUrl', () => { it('should fetch signed URL from Mistral API', async () => { const mockResponse: { data: MistralSignedUrlResponse } = { data: { url: 'https://document-url.com', expires_at: Date.now() + 86400000, }, }; mockAxios.get!.mockResolvedValueOnce(mockResponse); const result = await getSignedUrl({ fileId: 'file-123', apiKey: 'test-api-key', }); expect(mockAxios.get).toHaveBeenCalledWith( 'https://api.mistral.ai/v1/files/file-123/url?expiry=24', { headers: { Authorization: 'Bearer test-api-key', }, }, ); expect(result).toEqual(mockResponse.data); }); it('should handle errors when fetching signed URL', async () => { const errorMessage = 'API error'; mockAxios.get!.mockRejectedValueOnce(new Error(errorMessage)); await expect( getSignedUrl({ fileId: 'file-123', apiKey: 'test-api-key', }), ).rejects.toThrow(); expect(mockLogger.error).toHaveBeenCalledWith('Error fetching signed URL:', errorMessage); }); }); describe('performOCR', () => { it('should perform OCR using Mistral API (document_url)', async () => { const mockResponse: { data: OCRResult } = { data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Page 1 content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, { index: 1, markdown: 'Page 2 content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 2, doc_size_bytes: 1024, }, }, }; mockAxios.post!.mockResolvedValueOnce(mockResponse); const result = await performOCR({ apiKey: 'test-api-key', url: 'https://document-url.com', model: 'mistral-ocr-latest', documentType: 'document_url', }); expect(mockAxios.post).toHaveBeenCalledWith( 'https://api.mistral.ai/v1/ocr', { model: 'mistral-ocr-latest', include_image_base64: false, image_limit: 0, document: { type: 'document_url', document_url: 'https://document-url.com', }, }, { headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-api-key', }, }, ); expect(result).toEqual(mockResponse.data); }); it('should perform OCR using Mistral API (image_url)', async () => { const mockResponse: { data: OCRResult } = { data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Image OCR content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 2048, }, }, }; mockAxios.post!.mockResolvedValueOnce(mockResponse); const result = await performOCR({ apiKey: 'test-api-key', url: 'https://image-url.com/image.png', model: 'mistral-ocr-latest', documentType: 'image_url', }); expect(mockAxios.post).toHaveBeenCalledWith( 'https://api.mistral.ai/v1/ocr', { model: 'mistral-ocr-latest', include_image_base64: false, image_limit: 0, document: { type: 'image_url', image_url: 'https://image-url.com/image.png', }, }, { headers: { 'Content-Type': 'application/json', Authorization: 'Bearer test-api-key', }, }, ); expect(result).toEqual(mockResponse.data); }); it('should handle errors during OCR processing', async () => { const errorMessage = 'OCR processing error'; mockAxios.post!.mockRejectedValueOnce(new Error(errorMessage)); await expect( performOCR({ apiKey: 'test-api-key', url: 'https://document-url.com', }), ).rejects.toThrow(); expect(mockLogger.error).toHaveBeenCalledWith('Error performing OCR:', errorMessage); }); }); describe('uploadMistralOCR', () => { beforeEach(() => { const mockReadStream: MockReadStream = { on: jest.fn().mockImplementation(function ( this: MockReadStream, event: string, handler: () => void, ) { // Simulate immediate 'end' event to make FormData complete processing if (event === 'end') { handler(); } return this; }), pipe: jest.fn().mockImplementation(function (this: MockReadStream) { return this; }), pause: jest.fn(), resume: jest.fn(), emit: jest.fn(), once: jest.fn(), destroy: jest.fn(), path: '/tmp/upload/file.pdf', fd: 1, flags: 'r', mode: 0o666, autoClose: true, bytesRead: 0, closed: false, pending: false, }; (jest.mocked(fs).createReadStream as jest.Mock).mockReturnValue(mockReadStream); }); it('should process OCR for a file with standard configuration', async () => { // Setup mocks mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'test-api-key', OCR_BASEURL: 'https://api.mistral.ai/v1', }); // Mock file upload response mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'document.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }); // Mock signed URL response mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); // Mock OCR response with text and images mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-medium', pages: [ { index: 0, markdown: 'Page 1 content', images: [ { id: 'img1', top_left_x: 0, top_left_y: 0, bottom_right_x: 100, bottom_right_y: 100, image_base64: 'base64image1', image_annotation: '', }, ], dimensions: { dpi: 300, height: 1100, width: 850 }, }, { index: 1, markdown: 'Page 2 content', images: [ { id: 'img2', top_left_x: 0, top_left_y: 0, bottom_right_x: 100, bottom_right_y: 100, image_base64: 'base64image2', image_annotation: '', }, ], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 2, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { // Use environment variable syntax to ensure loadAuthValues is called apiKey: '${OCR_API_KEY}', baseURL: '${OCR_BASEURL}', mistralModel: 'mistral-medium', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['OCR_BASEURL', 'OCR_API_KEY'], optional: expect.any(Set), }); // Verify OCR result expect(result).toEqual({ filename: 'document.pdf', bytes: expect.any(Number), filepath: 'mistral_ocr', text: expect.stringContaining('# PAGE 1'), images: ['base64image1', 'base64image2'], }); }); it('should process OCR for an image file and use image_url type', async () => { mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'test-api-key', OCR_BASEURL: 'https://api.mistral.ai/v1', }); // Mock file upload response mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-456', object: 'file', bytes: 2048, created_at: Date.now(), filename: 'image.png', purpose: 'ocr', } as MistralFileUploadResponse, }); // Mock signed URL response mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com/image.png', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); // Mock OCR response for image mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-medium', pages: [ { index: 0, markdown: 'Image OCR result', images: [ { id: 'img1', top_left_x: 0, top_left_y: 0, bottom_right_x: 100, bottom_right_y: 100, image_base64: 'imgbase64', image_annotation: '', }, ], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 2048, }, }, }); const req = { user: { id: 'user456' }, app: { locals: { ocr: { apiKey: '${OCR_API_KEY}', baseURL: '${OCR_BASEURL}', mistralModel: 'mistral-medium', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/image.png', originalname: 'image.png', mimetype: 'image/png', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/image.png', ); expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user456', authFields: ['OCR_BASEURL', 'OCR_API_KEY'], optional: expect.any(Set), }); // Check that the OCR API was called with image_url type expect(mockAxios.post).toHaveBeenCalledWith( 'https://api.mistral.ai/v1/ocr', expect.objectContaining({ document: expect.objectContaining({ type: 'image_url', image_url: 'https://signed-url.com/image.png', }), }), expect.any(Object), ); expect(result).toEqual({ filename: 'image.png', bytes: expect.any(Number), filepath: 'mistral_ocr', text: expect.stringContaining('Image OCR result'), images: ['imgbase64'], }); }); it('should process variable references in configuration', async () => { // Setup mocks with environment variables mockLoadAuthValues.mockResolvedValue({ CUSTOM_API_KEY: 'custom-api-key', CUSTOM_BASEURL: 'https://custom-api.mistral.ai/v1', }); // Mock API responses mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'document.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }); mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-large', pages: [ { index: 0, markdown: 'Content from custom API', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: '${CUSTOM_API_KEY}', baseURL: '${CUSTOM_BASEURL}', mistralModel: '${CUSTOM_MODEL}', }, }, }, } as unknown as ExpressRequest; // Set environment variable for model process.env.CUSTOM_MODEL = 'mistral-large'; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); // Verify that custom environment variables were extracted and used expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['CUSTOM_BASEURL', 'CUSTOM_API_KEY'], optional: expect.any(Set), }); // Check that mistral-large was used in the OCR API call expect(mockAxios.post).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ model: 'mistral-large', }), expect.anything(), ); expect(result.text).toEqual('Content from custom API\n\n'); }); it('should fall back to default values when variables are not properly formatted', async () => { mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'default-api-key', OCR_BASEURL: undefined, // Testing optional parameter }); mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'document.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }); mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Default API result', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { // Use environment variable syntax to ensure loadAuthValues is called apiKey: '${INVALID_FORMAT}', // Using valid env var format but with an invalid name baseURL: '${OCR_BASEURL}', // Using valid env var format mistralModel: 'mistral-ocr-latest', // Plain string value }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); // Should use the default values expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['OCR_BASEURL', 'INVALID_FORMAT'], optional: expect.any(Set), }); // Should use the default model when not using environment variable format expect(mockAxios.post).toHaveBeenCalledWith( expect.anything(), expect.objectContaining({ model: 'mistral-ocr-latest', }), expect.anything(), ); }); it('should handle API errors during OCR process', async () => { mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'test-api-key', }); // Mock file upload to fail mockAxios.post!.mockRejectedValueOnce(new Error('Upload failed')); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: 'OCR_API_KEY', baseURL: 'OCR_BASEURL', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await expect( uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }), ).rejects.toThrow('Error uploading document to Mistral OCR API'); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); }); it('should handle single page documents without page numbering', async () => { mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'test-api-key', OCR_BASEURL: 'https://api.mistral.ai/v1', // Make sure this is included }); // Clear all previous mocks mockAxios.post!.mockClear(); mockAxios.get!.mockClear(); // 1. First mock: File upload response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'single-page.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }), ); // 2. Second mock: Signed URL response mockAxios.get!.mockImplementationOnce(() => Promise.resolve({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }), ); // 3. Third mock: OCR response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Single page content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }), ); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: 'OCR_API_KEY', baseURL: 'OCR_BASEURL', mistralModel: 'mistral-ocr-latest', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'single-page.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); // Verify that single page documents don't include page numbering expect(result.text).not.toContain('# PAGE'); expect(result.text).toEqual('Single page content\n\n'); }); it('should use literal values in configuration when provided directly', async () => { // We'll still mock this but it should not be used for literal values mockLoadAuthValues.mockResolvedValue({}); // Clear all previous mocks mockAxios.post!.mockClear(); mockAxios.get!.mockClear(); // 1. First mock: File upload response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'direct-values.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }), ); // 2. Second mock: Signed URL response mockAxios.get!.mockImplementationOnce(() => Promise.resolve({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }), ); // 3. Third mock: OCR response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { model: 'mistral-direct-model', pages: [ { index: 0, markdown: 'Processed with literal config values', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }), ); const req = { user: { id: 'user123' }, app: { locals: { ocr: { // Direct values that should be used as-is, without variable substitution apiKey: 'actual-api-key-value', baseURL: 'https://direct-api-url.mistral.ai/v1', mistralModel: 'mistral-direct-model', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'direct-values.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); // Verify the correct URL was used with the direct baseURL value expect(mockAxios.post).toHaveBeenCalledWith( 'https://direct-api-url.mistral.ai/v1/files', expect.any(Object), expect.objectContaining({ headers: expect.objectContaining({ Authorization: 'Bearer actual-api-key-value', }), }), ); // Check the OCR call was made with the direct model value expect(mockAxios.post).toHaveBeenCalledWith( 'https://direct-api-url.mistral.ai/v1/ocr', expect.objectContaining({ model: 'mistral-direct-model', }), expect.any(Object), ); // Verify the result expect(result.text).toEqual('Processed with literal config values\n\n'); // Verify loadAuthValues was never called since we used direct values expect(mockLoadAuthValues).not.toHaveBeenCalled(); }); it('should handle empty configuration values and use defaults', async () => { // Set up the mock values to be returned by loadAuthValues mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'default-from-env-key', OCR_BASEURL: 'https://default-from-env.mistral.ai/v1', }); // Clear all previous mocks mockAxios.post!.mockClear(); mockAxios.get!.mockClear(); // 1. First mock: File upload response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'empty-config.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }), ); // 2. Second mock: Signed URL response mockAxios.get!.mockImplementationOnce(() => Promise.resolve({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }), ); // 3. Third mock: OCR response mockAxios.post!.mockImplementationOnce(() => Promise.resolve({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Content from default configuration', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }), ); const req = { user: { id: 'user123' }, app: { locals: { ocr: { // Empty string values - should fall back to defaults apiKey: '', baseURL: '', mistralModel: '', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'empty-config.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect((fs as jest.Mocked<typeof fs>).createReadStream).toHaveBeenCalledWith( '/tmp/upload/file.pdf', ); // Verify loadAuthValues was called with the default variable names expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['OCR_BASEURL', 'OCR_API_KEY'], optional: expect.any(Set), }); // Verify the API calls used the default values from loadAuthValues expect(mockAxios.post).toHaveBeenCalledWith( 'https://default-from-env.mistral.ai/v1/files', expect.any(Object), expect.objectContaining({ headers: expect.objectContaining({ Authorization: 'Bearer default-from-env-key', }), }), ); // Verify the OCR model defaulted to mistral-ocr-latest expect(mockAxios.post).toHaveBeenCalledWith( 'https://default-from-env.mistral.ai/v1/ocr', expect.objectContaining({ model: 'mistral-ocr-latest', }), expect.any(Object), ); // Check result expect(result.text).toEqual('Content from default configuration\n\n'); }); describe('Mixed env var and hardcoded configuration', () => { beforeEach(() => { const mockReadStream: MockReadStream = { on: jest.fn().mockImplementation(function ( this: MockReadStream, event: string, handler: () => void, ) { // Simulate immediate 'end' event to make FormData complete processing if (event === 'end') { handler(); } return this; }), pipe: jest.fn().mockImplementation(function (this: MockReadStream) { return this; }), pause: jest.fn(), resume: jest.fn(), emit: jest.fn(), once: jest.fn(), destroy: jest.fn(), path: '/tmp/upload/file.pdf', fd: 1, flags: 'r', mode: 0o666, autoClose: true, bytesRead: 0, closed: false, pending: false, }; (jest.mocked(fs).createReadStream as jest.Mock).mockReturnValue(mockReadStream); }); it('should preserve hardcoded baseURL when only apiKey is an env var', async () => { // This test demonstrates the current bug mockLoadAuthValues.mockResolvedValue({ AZURE_MISTRAL_OCR_API_KEY: 'test-api-key-from-env', // Note: OCR_BASEURL is not returned, simulating it not being set }); // Mock file upload response mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-123', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'document.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }); // Mock signed URL response mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); // Mock OCR response mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-2503', pages: [ { index: 0, markdown: 'Test content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: '${AZURE_MISTRAL_OCR_API_KEY}', baseURL: 'https://endpoint.models.ai.azure.com/v1', mistralModel: 'mistral-ocr-2503', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); // Check that loadAuthValues was called only with the env var field expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['AZURE_MISTRAL_OCR_API_KEY'], optional: expect.any(Set), }); // The fix: baseURL should be the hardcoded value const uploadCall = mockAxios.post!.mock.calls[0]; expect(uploadCall[0]).toBe('https://endpoint.models.ai.azure.com/v1/files'); }); it('should preserve hardcoded apiKey when only baseURL is an env var', async () => { // This test demonstrates the current bug mockLoadAuthValues.mockResolvedValue({ CUSTOM_OCR_BASEURL: 'https://custom-ocr-endpoint.com/v1', // Note: OCR_API_KEY is not returned, simulating it not being set }); // Mock file upload response mockAxios.post!.mockResolvedValueOnce({ data: { id: 'file-456', object: 'file', bytes: 1024, created_at: Date.now(), filename: 'document.pdf', purpose: 'ocr', } as MistralFileUploadResponse, }); // Mock signed URL response mockAxios.get!.mockResolvedValueOnce({ data: { url: 'https://signed-url.com', expires_at: Date.now() + 86400000, } as MistralSignedUrlResponse, }); // Mock OCR response mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Test content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user456' }, app: { locals: { ocr: { apiKey: 'hardcoded-api-key-12345', baseURL: '${CUSTOM_OCR_BASEURL}', mistralModel: 'mistral-ocr-latest', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await uploadMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); // Check that loadAuthValues was called only with the env var field expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user456', authFields: ['CUSTOM_OCR_BASEURL'], optional: expect.any(Set), }); // The fix: apiKey should be the hardcoded value const uploadCall = mockAxios.post!.mock.calls[0]; const authHeader = uploadCall[2]?.headers?.Authorization; expect(authHeader).toBe('Bearer hardcoded-api-key-12345'); }); }); }); describe('uploadAzureMistralOCR', () => { beforeEach(() => { (jest.mocked(fs).readFileSync as jest.Mock).mockReturnValue(Buffer.from('mock-file-content')); }); it('should process OCR using Azure Mistral with base64 encoding', async () => { mockLoadAuthValues.mockResolvedValue({ OCR_API_KEY: 'azure-api-key', OCR_BASEURL: 'https://azure.mistral.ai/v1', }); // Mock OCR response mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Azure OCR content', images: [ { id: 'azure1', top_left_x: 0, top_left_y: 0, bottom_right_x: 100, bottom_right_y: 100, image_base64: 'azure-base64', image_annotation: '', }, ], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: '${OCR_API_KEY}', baseURL: '${OCR_BASEURL}', mistralModel: 'mistral-ocr-latest', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/azure-file.pdf', originalname: 'azure-document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; const result = await uploadAzureMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); expect(jest.mocked(fs).readFileSync).toHaveBeenCalledWith('/tmp/upload/azure-file.pdf'); // Verify OCR was called with base64 data URL expect(mockAxios.post).toHaveBeenCalledWith( 'https://azure.mistral.ai/v1/ocr', expect.objectContaining({ document: expect.objectContaining({ type: 'document_url', document_url: expect.stringMatching(/^data:application\/pdf;base64,/), }), }), expect.any(Object), ); expect(result).toEqual({ filename: 'azure-document.pdf', bytes: expect.any(Number), filepath: 'azure_mistral_ocr', text: 'Azure OCR content\n\n', images: ['azure-base64'], }); }); describe('Mixed env var and hardcoded configuration', () => { it('should preserve hardcoded baseURL when only apiKey is an env var', async () => { // This test demonstrates the current bug mockLoadAuthValues.mockResolvedValue({ AZURE_MISTRAL_OCR_API_KEY: 'test-api-key-from-env', // Note: OCR_BASEURL is not returned, simulating it not being set }); // Mock OCR response mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-2503', pages: [ { index: 0, markdown: 'Test content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user123' }, app: { locals: { ocr: { apiKey: '${AZURE_MISTRAL_OCR_API_KEY}', baseURL: 'https://endpoint.models.ai.azure.com/v1', mistralModel: 'mistral-ocr-2503', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await uploadAzureMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); // Check that loadAuthValues was called only with the env var field expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user123', authFields: ['AZURE_MISTRAL_OCR_API_KEY'], optional: expect.any(Set), }); // The fix: baseURL should be the hardcoded value const ocrCall = mockAxios.post!.mock.calls[0]; expect(ocrCall[0]).toBe('https://endpoint.models.ai.azure.com/v1/ocr'); }); it('should preserve hardcoded apiKey when only baseURL is an env var', async () => { // This test demonstrates the current bug mockLoadAuthValues.mockResolvedValue({ CUSTOM_OCR_BASEURL: 'https://custom-ocr-endpoint.com/v1', // Note: OCR_API_KEY is not returned, simulating it not being set }); // Mock OCR response mockAxios.post!.mockResolvedValueOnce({ data: { model: 'mistral-ocr-latest', pages: [ { index: 0, markdown: 'Test content', images: [], dimensions: { dpi: 300, height: 1100, width: 850 }, }, ], document_annotation: '', usage_info: { pages_processed: 1, doc_size_bytes: 1024, }, }, }); const req = { user: { id: 'user456' }, app: { locals: { ocr: { apiKey: 'hardcoded-api-key-12345', baseURL: '${CUSTOM_OCR_BASEURL}', mistralModel: 'mistral-ocr-latest', }, }, }, } as unknown as ExpressRequest; const file = { path: '/tmp/upload/file.pdf', originalname: 'document.pdf', mimetype: 'application/pdf', } as Express.Multer.File; await uploadAzureMistralOCR({ req, file, loadAuthValues: mockLoadAuthValues, }); // Check that loadAuthValues was called only with the env var field expect(mockLoadAuthValues).toHaveBeenCalledWith({ userId: 'user456', authFields: ['CUSTOM_OCR_BASEURL'], optional: expect.any(Set), }); // The fix: apiKey should be the hardcoded value const ocrCall = mockAxios.post!.mock.calls[0]; const authHeader = ocrCall[2]?.headers?.Authorization; expect(authHeader).toBe('Bearer hardcoded-api-key-12345'); }); }); }); }); Memory Management The MCPManager singleton pattern with connection pooling and timeout management introduces complexity. The idle connection cleanup logic and user activity tracking need careful review for potential memory leaks or race conditions, especially around the checkIdleConnections method and user connection lifecycle. /** Check for and disconnect idle connections */ private checkIdleConnections(currentUserId?: string): void { const now = Date.now(); // Iterate through all users to check for idle ones for (const [userId, lastActivity] of this.userLastActivity.entries()) { if (currentUserId && currentUserId === userId) { continue; } if (now - lastActivity > this.USER_CONNECTION_IDLE_TIMEOUT) { logger.info( `[MCP][User: ${userId}] User idle for too long. Disconnecting all connections...`, ); // Disconnect all user connections asynchronously (fire and forget) this.disconnectUserConnections(userId).catch((err) => logger.error(`[MCP][User: ${userId}] Error disconnecting idle connections:`, err), ); } } } /** Updates the last activity timestamp for a user */ private updateUserLastActivity(userId: string): void { const now = Date.now(); this.userLastActivity.set(userId, now); logger.debug( `[MCP][User: ${userId}] Updated last activity timestamp: ${new Date(now).toISOString()}`, ); } Token Validation The memory processing system includes token counting and limits but the validation logic for token limits appears to have potential issues. The totalTokens parameter is used inconsistently and the token limit enforcement may not work correctly in all scenarios, particularly around line 108-114. const tokenCount = Tokenizer.getTokenCount(value, 'o200k_base'); if (tokenLimit && tokenCount > tokenLimit) { logger.warn( `Memory Agent failed to set memory: Value exceeds token limit. Value has ${tokenCount} tokens, but limit is ${tokenLimit}`, ); return `Memory value too large: ${tokenCount} tokens exceeds limit of ${tokenLimit}`; } if (tokenLimit && totalTokens + tokenCount > tokenLimit) { const remainingCapacity = tokenLimit - totalTokens; logger.warn( `Memory Agent failed to set memory: Would exceed total token limit. Current usage: ${totalTokens}, new memory: ${tokenCount} tokens, limit: ${tokenLimit}`, ); return `Cannot add memory: would exceed token limit. Current usage: ${totalTokens}/${tokenLimit} tokens. This memory requires ${tokenCount} tokens, but only ${remainingCapacity} tokens available.`; }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix OAuth handling race condition The OAuth handling logic has a potential race condition where oauthHandled flag is checked but the actual OAuth handling might fail silently. The code should verify that OAuth was actually successful before setting the flag and should handle the case where serverUrl is undefined. packages/api/src/mcp/manager.ts [300-314] /** Check if OAuth was already handled by the connection */ const errorWithFlag = error as (Error & { isOAuthError?: boolean }) | undefined; if (!oauthHandled && errorWithFlag?.isOAuthError) { - oauthHandled = true; logger.info(`${logPrefix} Handling OAuth`); const serverUrl = connection.url; if (serverUrl) { - await this.handleOAuthRequired({ - serverName: connection.serverName, - serverUrl, - flowManager, - }); + try { + await this.handleOAuthRequired({ + serverName: connection.serverName, + serverUrl, + flowManager, + }); + oauthHandled = true; + } catch (oauthError) { + logger.error(`${logPrefix} OAuth handling failed`, oauthError); + throw oauthError; + } + } else { + logger.error(`${logPrefix} OAuth required but server URL is undefined`); + throw new Error('OAuth required but server URL is undefined'); } } else { logger.info(`${logPrefix} OAuth already handled by connection`); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the oauthHandled flag is set prematurely, before the handleOAuthRequired async operation completes. Moving the flag assignment into a try block after the operation succeeds is a valid improvement that makes the state management more robust. It also correctly adds handling for a missing serverUrl.	Medium
	Fix incorrect type casting The type casting to t.IMessage for attachment objects is incorrect. Attachments should be typed as attachment objects, not message objects. This could lead to runtime errors when accessing attachment-specific properties. packages/data-schemas/src/methods/share.test.ts [362-367] -expect( - (result?.messages[0].attachments?.[0] as unknown as t.IMessage | undefined)?.messageId, -).toBe(result?.messages[0].messageId); -expect( - (result?.messages[0].attachments?.[0] as unknown as t.IMessage | undefined)?.conversationId, -).toBe(result?.conversationId); +expect(result?.messages[0].attachments?.[0]).toBeDefined(); +expect(result?.messages[0].attachments?.[0]).toMatchObject({ + file_id: 'file123', + filename: 'test.pdf', + type: 'application/pdf', +}); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies that the test is casting an attachment object to an IMessage type and checking for message properties. This is incorrect and makes the test's intent unclear. The proposed change to verify the actual attachment properties makes the test more accurate and robust.	Medium
General	Remove unnecessary try-catch wrapper The try-catch block with console.error and re-throw is unnecessary in Jest tests. Jest automatically handles and reports test failures, and this pattern can mask the actual test failure location and make debugging harder. packages/api/src/files/mistral/crud.spec.ts [135-161] -try { - const result = await uploadDocumentToMistral({ - filePath: '/path/to/test.pdf', - fileName: 'test.pdf', - apiKey: 'test-api-key', - }); - // ... expectations -} catch (error) { - console.error('Test error:', error); - throw error; -} +const result = await uploadDocumentToMistral({ + filePath: '/path/to/test.pdf', + fileName: 'test.pdf', + apiKey: 'test-api-key', +}); +// ... expectations [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion is correct. The try-catch block is redundant in a Jest test and can obscure stack traces. Removing it improves test clarity and debugging, as Jest handles promise rejections automatically.	Low
	Verify error logging behavior The test assumes specific error handling behavior without verifying the actual error path. The comment suggests the function falls back to a catch block, but this assumption may not hold if the implementation changes. packages/api/src/agents/resources.test.ts [927-943] const result = await primeResources({ req: reqWithoutLocals, getFiles: mockGetFiles, requestFileSet, attachments: undefined, tool_resources: { [EToolResources.ocr]: { file_ids: ['ocr-file-1'], }, }, }); expect(mockGetFiles).not.toHaveBeenCalled(); -// When app.locals is missing and there's an error accessing properties, -// the function falls back to the catch block which returns an empty array +expect(logger.error).toHaveBeenCalled(); expect(result.attachments).toEqual([]); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out that the test for handling missing app.locals should also verify that an error is logged, as implied by the test's comment. Adding expect(logger.error).toHaveBeenCalled() makes the test more complete by confirming the expected error handling side-effect.	Low
	Add error event handling The mock stream implementation only handles the 'end' event but real streams can emit 'error' events that should be handled to prevent unhandled promise rejections. Add error event handling to make the mock more robust and prevent potential test failures. packages/api/src/files/mistral/crud.spec.ts [89-117] const mockReadStream: MockReadStream = { on: jest.fn().mockImplementation(function ( this: MockReadStream, event: string, - handler: () => void, + handler: (error?: Error) => void, ) { // Simulate immediate 'end' event to make FormData complete processing if (event === 'end') { handler(); + } + // Handle error events to prevent unhandled rejections + if (event === 'error') { + // Don't emit error by default, but allow tests to trigger it if needed } return this; }), pipe: jest.fn().mockImplementation(function (this: MockReadStream) { return this; }), pause: jest.fn(), resume: jest.fn(), emit: jest.fn(), once: jest.fn(), destroy: jest.fn(), path: '/path/to/test.pdf', fd: 1, flags: 'r', mode: 0o666, autoClose: true, bytesRead: 0, closed: false, pending: false, }; [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that the mock stream should handle the 'error' event for robustness. This improves the quality and reusability of the test mock, even though no current tests rely on this behavior.	Low
	Remove unnecessary optional chaining The optional chaining operator ?. on connection is unnecessary here since connection is guaranteed to be defined at this point in the code flow. This could mask potential issues and makes the code less clear about the actual state. packages/api/src/mcp/manager.ts [565-567] -if (!(await connection?.isConnected())) { +if (!(await connection.isConnected())) { throw new Error('Failed to establish connection after initialization attempt.'); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that the optional chaining on connection is unnecessary, as connection is guaranteed to be defined at this point in the code. Removing it improves code clarity by asserting that the object is expected to exist, which is a good practice for readability and maintainability.	Low
	Make error logging assertions flexible The test expects two specific logger calls with exact error messages, but these assertions are fragile and may fail if the implementation's error handling or logging messages change slightly. packages/api/src/agents/resources.test.ts [910-915] -// Should log both the main error and the attachment error -expect(logger.error).toHaveBeenCalledWith('Error priming resources', error); -expect(logger.error).toHaveBeenCalledWith( - 'Error resolving attachments in catch block', - error, -); +// Should log errors appropriately +expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('Error'), error); +expect(logger.error).toHaveBeenCalledTimes(2); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 3 __ Why: While the suggestion correctly identifies that testing for exact error strings can be brittle, the proposed improved_code is too generic. It loses the specificity of the original test, which verifies that two distinct error messages are logged. The original assertion is stronger as it confirms more specific behavior.	Low
Update