Skip to content

Feature/pinned chats#98

Open
andrii-ivanov wants to merge 5 commits intomainfrom
feature/pinned-chats
Open

Feature/pinned chats#98
andrii-ivanov wants to merge 5 commits intomainfrom
feature/pinned-chats

Conversation

@andrii-ivanov
Copy link
Collaborator

@andrii-ivanov andrii-ivanov commented Aug 12, 2025

PR Type

Enhancement, Tests


Description

Pinned Conversations Feature: Added comprehensive support for pinning and reordering conversations with dedicated UI components and state management
MCP (Model Context Protocol) Enhancements: Significantly improved MCP server management with OAuth authentication, connection handling, and comprehensive test coverage
Theme System Implementation: Added complete theming infrastructure with dark/light themes, RGB color definitions, and theme application utilities
Test Coverage Expansion: Added extensive test suites for MCP functionality, access middleware, tool formatting, memory agents, and OpenAI configurations
Configuration Schema Updates: Enhanced endpoint configurations with new parameters like verbosity, web_search, SearXNG support, and file search capabilities
Code Quality Improvements: Refactored various hooks and utilities for better state management, removed deprecated components, and improved error handling
Internationalization Setup: Added comprehensive i18n configuration supporting 26 languages with proper fallback strategies
Memory Agent Enhancements: Improved memory functionality with overflow handling and GPT-5+ model support


Diagram Walkthrough

flowchart LR
  A["Pinned Conversations"] --> B["Conversation Grouping"]
  A --> C["Reorder Mutations"]
  D["MCP Enhancements"] --> E["OAuth Handler"]
  D --> F["Server Manager"]
  D --> G["Connection Status"]
  H["Theme System"] --> I["Dark Theme"]
  H --> J["Light Theme"]
  H --> K["Theme Utils"]
  L["Test Coverage"] --> M["MCP Tests"]
  L --> N["Access Tests"]
  L --> O["Tool Tests"]
  P["Configuration"] --> Q["SearXNG Support"]
  P --> R["Web Search"]
  P --> S["File Search"]
Loading

File Walkthrough

Relevant files
Tests
13 files
zod.spec.ts
Enhanced MCP Zod schema tests with ref resolution               

packages/api/src/mcp/zod.spec.ts

• Updated imports to use convertWithResolvedRefs instead of
convertJsonSchemaToZod throughout test file
• Added comprehensive
tests for $ref resolution functionality with resolveJsonSchemaRefs

Added tests for handling bare object schemas as passthrough for
dynamic properties
• Added tests for additionalProperties with
anyOf/oneOf and allowEmptyObject scenarios

+776/-44
web.spec.ts
Updated web search tests with new imports and SearXNG support

packages/api/src/web/web.spec.ts

• Updated import paths to use relative imports and
librechat-data-provider
• Added searxngInstanceUrl and searxngApiKey
fields to all test configurations
• Fixed TypeScript type annotations
for authFields.forEach callback parameters
• Added comprehensive tests
for firecrawlOptions properties and timeout handling

+330/-23
access.spec.ts
Added comprehensive access middleware test suite                 

packages/api/src/middleware/access.spec.ts

• Added comprehensive test suite for access control middleware
functions
• Tests checkAccess, generateCheckAccess, and skipAgentCheck
functions
• Covers permission validation, role-based access, and
real-world usage patterns
• Includes error handling and edge case
scenarios for access control

+554/-0 
format.spec.ts
Added comprehensive tool formatting test suite                     

packages/api/src/tools/format.spec.ts

• Added comprehensive test suite for tool formatting utility functions

• Tests filterUniquePlugins, checkPluginAuth,
convertMCPToolsToPlugins, and getToolkitKey
• Covers plugin
deduplication, authentication validation, and MCP tool conversion

Includes edge cases and error handling for tool formatting operations

+461/-0 
mcp.spec.ts
Enhanced MCP tests with HTTP type and GitHub server support

packages/api/src/mcp/mcp.spec.ts

• Added tests for "http" type alias support in
StreamableHTTPOptionsSchema
• Added tests for processing
customUserVars in args field and headers
• Added GitHub MCP server
configuration tests with PAT_TOKEN placeholder handling
• Enhanced
test coverage for HTTP-based MCP server configurations

+146/-0 
memory.test.ts
Add comprehensive memory agent test suite                               

packages/api/src/agents/tests/memory.test.ts

• Added comprehensive test suite for memory agent functionality

Tests memory tool creation with overflow handling and token limits

Tests GPT-5+ model handling in processMemory function with temperature
removal
• Tests various model formats and parameter transformations

+469/-0 
llm.spec.ts
Add comprehensive OpenAI LLM configuration test suite       

packages/api/src/endpoints/openai/llm.spec.ts

• Added comprehensive test suite for getOpenAIConfig function
• Tests
model options, parameter separation, reasoning params, and Azure
configuration
• Tests GPT-5+ model handling with token parameter
transformations
• Tests verbosity parameter handling and various edge
cases

+424/-0 
crud.spec.ts
Add Mistral file deletion and cleanup tests                           

packages/api/src/files/mistral/crud.spec.ts

• Added deleteMistralFile function tests with error handling
• Added
comprehensive file cleanup tests for OCR processing
• Tests cleanup
behavior on success, failure, and error scenarios
• Tests graceful
handling of deletion errors without throwing

+386/-1 
content.spec.ts
Add content formatting test suite                                               

packages/api/src/format/content.spec.ts

• Added test suite for formatContentStrings function
• Tests
conversion of message content arrays to strings for different message
types
• Tests handling of mixed content types and edge cases
• Tests
real-world scenarios with text and image content

+340/-0 
key.test.ts
Add service key loading test suite                                             

packages/api/src/utils/key.test.ts

• Added comprehensive test suite for loadServiceKey function
• Tests
JSON parsing, file loading, URL fetching, and base64 decoding
• Tests
private key formatting and escaped newline handling
• Tests error
scenarios and edge cases

+182/-0 
handler.test.ts
Add MCP OAuth handler configuration tests                               

packages/api/src/mcp/oauth/handler.test.ts

• Adds comprehensive test suite for MCP OAuth handler configuration

Tests custom OAuth metadata fields like grant_types_supported,
token_endpoint_auth_methods_supported
• Validates default values and
custom configurations for OAuth parameters

+190/-0 
auth.test.ts
Add MCP authentication mapping tests                                         

packages/api/src/mcp/auth.test.ts

• Tests MCP authentication mapping functionality with various server
name formats
• Validates handling of special characters, spaces, and
Unicode in server names
• Tests edge cases like empty tools arrays and
database error handling

+168/-0 
convos.spec.ts
Add pinned conversation grouping tests                                     

client/src/utils/convos.spec.ts

• Adds comprehensive tests for pinned conversation handling in
groupConversationsByDate
• Tests pinned conversation sorting by
pinnedOrder and proper grouping
• Validates edge cases with no pinned
conversations

+80/-0   
Enhancement
33 files
useMCPServerManager.ts
Added comprehensive MCP server management hook                     

client/src/hooks/MCP/useMCPServerManager.ts

• Created comprehensive hook for managing MCP server connections and
authentication
• Implements OAuth flow handling with polling,
cancellation, and timeout management
• Provides server state
management, configuration dialog handling, and batch operations

Includes connection status monitoring and user plugin authentication
management

+566/-0 
manager.ts
Refactor MCP manager initialization and OAuth handling     

packages/api/src/mcp/manager.ts

• Refactored initializeMCP method to extract single server
initialization logic
• Added tracking for OAuth servers that require
authentication at startup
• Modified OAuth handling to skip
authentication during startup for better reliability
• Added utility
methods for getting user connections and OAuth servers

+212/-168
connection.ts
Improve MCP connection verification and error handling     

packages/api/src/mcp/connection.ts

• Improved connection verification logic with fallback methods

Enhanced isConnected method to handle servers without ping support

Removed error emission and cache invalidation logic
• Fixed transport
type detection for HTTP connections

+57/-103
memory.ts
Enhance memory agent with overflow handling and GPT-5+ support

packages/api/src/agents/memory.ts

• Enhanced memory tool with token overflow detection and error
artifacts
• Added GPT-5+ model parameter handling with temperature
removal
• Updated memory instructions to be more restrictive about
when to store memories
• Improved error handling and artifact
generation for memory operations

+92/-38 
useEventHandlers.ts
Improve SSE event handling and conversation state management

client/src/hooks/SSE/useEventHandlers.ts

• Enhanced error handling for cancelled streams and blank page
scenarios
• Improved conversation state management and navigation
logic
• Added draft saving for interrupted conversations
• Fixed agent
template application timing and parameters

+68/-25 
llm.ts
Enhance OpenAI LLM configuration with GPT-5+ and tool support

packages/api/src/endpoints/openai/llm.ts

• Added known OpenAI parameters set for better parameter separation

Enhanced GPT-5+ model handling with token parameter transformations

Added web search tool support and verbosity parameter handling

Improved Azure configuration with Responses API support

+145/-6 
convos.ts
Add pinned conversations support to conversation grouping

client/src/utils/convos.ts

• Added pinned conversations grouping and sorting functionality

Enhanced groupConversationsByDate to handle pinned conversations
separately
• Added pinned conversations at the top with custom
ordering
• Maintained existing date-based grouping for unpinned
conversations

+40/-6   
index.ts
Add comprehensive theme type definitions                                 

packages/client/src/theme/types/index.ts

• Defines comprehensive theme type interfaces for RGB color values,
CSS variables, and theme colors
• Includes interfaces for IThemeRGB,
IThemeVariables, IThemeColors, and Theme
• Covers text, surface,
border, brand, and utility color definitions

+189/-0 
zod.ts
Enhance JSON schema to Zod conversion with reference resolution

packages/api/src/mcp/zod.ts

• Removes inline type definitions and imports them from external types
module
• Adds resolveJsonSchemaRefs function for handling JSON schema
references
• Enhances schema conversion with better handling of bare
object schemas and additional properties
• Adds
convertWithResolvedRefs helper function for tests

+109/-18
object-traverse.ts
Add ESM-native object traversal utility                                   

packages/data-schemas/src/utils/object-traverse.ts

• Implements ESM-native object traversal utility with forEach
functionality
• Provides comprehensive traversal context with path
tracking, circular reference detection
• Includes type guards and safe
property manipulation methods

+178/-0 
mutations.ts
Enhance conversation pinning and reordering mutations       

client/src/data-provider/mutations.ts

• Updates conversation pinning mutation to use updateConversation
instead of dedicated pinConversation
• Adds
useReorderPinnedConversationsMutation for managing pinned conversation
order
• Enhances archive mutation to automatically unpin conversations
when archiving

+88/-26 
schemas.ts
Expand model parameters and conversation schema enhancements

packages/data-provider/src/schemas.ts

• Adds minimal option to ReasoningEffort enum and Verbosity enum

Updates penalty parameter ranges from 0-2 to -2-2 for OpenAI and
agents settings
• Adds web_search configuration for Anthropic settings
and schema updates
• Adds pinnedOrder field to conversation schema and
removes inline isPinned definition

+37/-12 
parsers.ts
Replace traverse library with custom ESM implementation   

packages/data-schemas/src/config/parsers.ts

• Replaces traverse library with custom ESM-native object traversal
utility
• Updates debug logging formatter to use new traversal context
interface
• Improves error handling and string concatenation in
logging

+54/-44 
parameterSettings.ts
Add new model parameters and web search options                   

packages/data-provider/src/parameterSettings.ts

• Adds minimal option to reasoning effort settings and verbosity
parameter for OpenAI
• Adds web_search toggle for OpenAI and Anthropic
endpoints
• Adds disableStreaming option for OpenAI endpoints

Updates parameter configurations across multiple model endpoints

+74/-5   
dark.ts
Add dark theme color definitions                                                 

packages/client/src/theme/themes/dark.ts

• Defines complete dark theme color palette with RGB values
• Maps
colors for text, surfaces, borders, brand elements, and utility colors

• Provides comprehensive dark mode color scheme implementation

+72/-0   
default.ts
Add default light theme color definitions                               

packages/client/src/theme/themes/default.ts

• Defines complete default light theme color palette with RGB values

Maps colors for text, surfaces, borders, brand elements, and utility
colors
• Provides comprehensive light mode color scheme implementation

+72/-0   
applyTheme.ts
Add theme application utility with validation                       

packages/client/src/theme/utils/applyTheme.ts

• Implements theme application utility with RGB validation
• Maps
theme RGB values to CSS variables and applies them to document root

Includes validation for RGB format and error handling

+115/-0 
handler.ts
Enhance MCP OAuth handler with configurable metadata         

packages/api/src/mcp/oauth/handler.ts

• Updates OAuth metadata discovery to use
discoverAuthorizationServerMetadata
• Adds configurable OAuth metadata
fields support with default values
• Enhances authorization flow with
resource parameter handling
• Improves error handling and logging
throughout OAuth process

+52/-9   
format.ts
Add plugin formatting and conversion utilities                     

packages/api/src/tools/format.ts

• Adds utility functions for filtering unique plugins and checking
plugin authentication
• Implements MCP tools to plugins conversion
functionality
• Adds toolkit key resolution for tool name matching

+142/-0 
useToolToggle.ts
Refactor tool toggle state management                                       

client/src/hooks/Plugins/useToolToggle.ts

• Refactors tool toggle logic to use ephemeral agent as single source
of truth
• Improves localStorage synchronization and removes infinite
loop prevention
• Enhances type safety with ToolValue type and better
value handling

+38/-26 
useArtifacts.ts
Update artifacts hook to use dedicated context                     

client/src/hooks/Artifacts/useArtifacts.ts

• Updates to use useArtifactsContext instead of useChatContext

Changes from latestMessage to latestMessageId and latestMessageText

Simplifies artifact detection and management logic

+21/-18 
crud.ts
Add file cleanup and fix service key configuration             

packages/api/src/files/mistral/crud.ts

• Adds deleteMistralFile function for cleaning up uploaded files

Updates OCR upload process to automatically delete temporary files
after processing
• Fixes Google service key path environment variable
name

+49/-5   
index.ts
Consolidate SVG icon exports                                                         

packages/client/src/svgs/index.ts

• Consolidates all SVG icon exports into a single index file
• Exports
67 different SVG components for consistent icon usage

+67/-0   
index.ts
Simplify component SVG exports to file-related icons         

client/src/components/svg/index.ts

• Reduces exports to only file-related SVG components
• Exports
CodePaths, FileIcon, FilePaths, SheetPaths, and TextPaths

+5/-66   
useAutoSave.ts
Refactor auto-save to use centralized draft utilities       

client/src/hooks/Input/useAutoSave.ts

• Refactors to use external getDraft, setDraft, and clearDraft
utilities
• Removes inline base64 encoding/decoding functions

Simplifies draft handling logic with centralized utilities

+9/-46   
i18n.ts
Add comprehensive internationalization configuration         

packages/client/src/locales/i18n.ts

• Sets up comprehensive i18n configuration with 26 language
translations
• Configures language detection and fallback strategies

Includes support for Chinese variants and proper fallback chains

+87/-0   
key.ts
Enhance Google service key loading with multiple formats 

packages/api/src/utils/key.ts

• Enhances Google service key loading to support base64 encoded and
stringified JSON
• Adds proper private key formatting with newline
handling
• Improves PEM format validation and correction

+51/-5   
useMCPSelect.ts
Enhance MCP selection with configuration-based filtering 

client/src/hooks/Plugins/useMCPSelect.ts

• Adds filtering based on startup config chatMenu setting
• Stabilizes
setMCPValues callback to prevent infinite render loops
• Integrates
with startup configuration for MCP server visibility

+23/-4   
web.ts
Add SearXNG search provider support                                           

packages/api/src/web/web.ts

• Adds SearXNG search provider configuration with instance URL and API
key
• Updates web search authentication to include SearXNG provider

Enhances scraper timeout configuration with Firecrawl options support

+21/-5   
useChatFunctions.ts
Simplify chat functions by removing artifacts configuration

client/src/hooks/Chat/useChatFunctions.ts

• Removes artifacts mode configuration from chat functions

Simplifies message submission by removing artifact-related parameters

• Updates response message ID generation logic for regeneration

+4/-11   
react-query-service.ts
Add MCP server management mutations and queries                   

packages/data-provider/src/react-query/react-query-service.ts

• Adds useReinitializeMCPServerMutation for server reinitialization

Adds useCancelMCPOAuthMutation for OAuth cancellation
• Adds
useMCPServerConnectionStatusQuery for connection status monitoring

+53/-0   
files.ts
Update file utility imports for SVG components                     

client/src/utils/files.ts

• Imports file-related SVG components from the component SVG module

+1/-4     
role.ts
Add file search permission to role types                                 

packages/data-schemas/src/types/role.ts

• Adds FILE_SEARCH permission type to role interface
• Extends
permissions structure with file search capabilities

+3/-0     
Configuration changes
1 files
config.ts
Enhance configuration schemas with new options                     

packages/data-provider/src/config.ts

• Added new title-related configuration options to base endpoint
schema
• Added fileSearch interface configuration option
• Enhanced
web search schema with SearXNG and Firecrawl options
• Added memory
character limit configuration and updated cache keys

+112/-28
Additional files
101 files
.env.example +43/-3   
backend-review.yml +1/-1     
client.yml +58/-0   
data-schemas.yml +1/-1     
frontend-review.yml +1/-1     
generate-release-changelog-pr.yml +0/-95   
generate-unreleased-changelog-pr.yml +0/-107 
i18n-unused-keys.yml +2/-1     
locize-i18n-sync.yml +1/-1     
unused-packages.yml +95/-6   
Dockerfile +1/-1     
Dockerfile.multi +11/-2   
README.md +1/-1     
BaseClient.js +14/-3   
OpenAIClient.js +3/-1     
formatMessages.js +0/-32   
BaseClient.test.js +40/-0   
DALLE3.js +7/-3     
OpenAIImageTools.js +22/-3   
DALLE3-proxy.spec.js +94/-0   
fileSearch.js +10/-2   
handleTools.js +15/-11 
cacheConfig.js +62/-0   
cacheConfig.spec.js +157/-0 
cacheFactory.js +108/-0 
cacheFactory.spec.js +432/-0 
getLogStores.js +54/-111
ioredisClient.js +0/-92   
keyvRedis.js +0/-109 
logViolation.js +3/-2     
redisClients.js +204/-0 
Agent.js +11/-13 
Agent.spec.js +40/-53 
Conversation.js +19/-11 
Conversation.spec.js +572/-0 
File.js +113/-3 
File.spec.js +264/-0 
Message.js +1/-1     
Message.spec.js +269/-4 
tx.js +15/-2   
tx.spec.js +104/-3 
package.json +13/-10 
ErrorController.js +12/-6   
ErrorController.spec.js +241/-0 
PluginController.js +26/-55 
PluginController.spec.js +520/-0 
TwoFactorController.js +26/-0   
UserController.js +14/-16 
client.js +196/-42
client.test.js +1190/-0
errors.js +0/-12   
request.js +22/-0   
v1.js +43/-22 
v1.spec.js +681/-0 
chatV1.js +3/-3     
chatV2.js +1/-1     
errors.js +2/-2     
helpers.js +10/-0   
v1.js +2/-2     
tools.js +1/-2     
index.js +5/-3     
index.spec.js +24/-1   
abortRun.js +2/-2     
buildEndpointOption.js +1/-1     
checkBan.js +1/-1     
concurrentLimiter.js +2/-2     
forkLimiters.js +79/-0   
importLimiters.js +13/-22 
index.js +2/-0     
loginLimiter.js +5/-14   
messageLimiters.js +7/-21   
registerLimiter.js +5/-14   
resetPasswordLimiter.js +3/-13   
sttLimiters.js +7/-21   
toolCallLimiter.js +5/-14   
ttsLimiters.js +7/-21   
uploadLimiters.js +7/-20   
verifyEmailLimiter.js +3/-13   
uaParser.js +3/-2     
validateEndpoint.js +1/-1     
validateModel.js +1/-1     
mcp.spec.js +1259/-0
static.spec.js +162/-0 
config.js +8/-1     
convos.js +47/-5   
files.agents.test.js +282/-0 
files.js +129/-12
files.test.js +302/-0 
mcp.js +399/-9 
memories.js +74/-18 
static.js +4/-1     
AppService.js +5/-3     
AppService.spec.js +205/-3 
update.js +20/-5   
update.spec.js +180/-2 
AssistantService.js +6/-4     
getCustomConfig.js +36/-40 
getEndpointsConfig.js +12/-2   
loadAsyncEndpoints.js +19/-19 
loadCustomConfig.js +8/-6     
Additional files not shown

Henri Cook and others added 5 commits August 5, 2025 11:35
# Conflicts:
#	.gitignore
#	Dockerfile.multi
#	README.md
#	package-lock.json
…/pinned-chats

# Conflicts:
#	api/models/Conversation.js
#	client/src/components/Conversations/Conversations.tsx
#	client/src/components/Conversations/Convo.tsx
#	client/src/components/Conversations/ConvoOptions/ConvoOptions.tsx
#	client/src/data-provider/mutations.ts
#	packages/data-provider/src/api-endpoints.ts
#	packages/data-provider/src/data-service.ts
@github-actions
Copy link

🚨 Unused NPM Packages Detected

The following unused dependencies were found:

📂 Root package.json

  • @e2b/code-interpreter

⚠️ Please remove these unused dependencies to keep your project clean.

Comment on lines 72 to 78
const buttonText = isServerInitializing
? localize('com_ui_loading')
: isReinit
? localize('com_ui_reinitialize')
: requiresOAuth
? localize('com_ui_authenticate')
: localize('com_ui_mcp_initialize');

Check warning

Code scanning / ESLint

Disallow nested ternary expressions Warning

Do not nest ternary expressions.
Comment on lines 74 to 78
: isReinit
? localize('com_ui_reinitialize')
: requiresOAuth
? localize('com_ui_authenticate')
: localize('com_ui_mcp_initialize');

Check warning

Code scanning / ESLint

Disallow nested ternary expressions Warning

Do not nest ternary expressions.
@qodo-code-review
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Test Coverage Shift

Many tests now rely on convertWithResolvedRefs and resolveJsonSchemaRefs. Ensure these helpers are robust and exported as used; verify behavior parity with the old convertJsonSchemaToZod where refs aren’t needed, and confirm unresolved-ref paths are intentionally different.

  describe('additionalProperties with anyOf/oneOf and allowEmptyObject', () => {
    it('should handle anyOf with object containing only additionalProperties when allowEmptyObject is false', () => {
      const schema: JsonSchemaType & { anyOf?: any } = {
        type: 'object',
        properties: {
          filter: {
            description: 'Filter field',
            anyOf: [
              {
                type: 'object',
                additionalProperties: {
                  type: 'object',
                  properties: {
                    _icontains: { type: 'string' },
                  },
                },
              },
              {
                type: 'null',
              },
            ],
          } as JsonSchemaType & { anyOf?: any },
        },
      };

      const zodSchema = convertWithResolvedRefs(schema, {
        allowEmptyObject: false,
        transformOneOfAnyOf: true,
      });

      expect(zodSchema).toBeDefined();

      const testData = {
        filter: {
          title: {
            _icontains: 'Pirate',
          },
        },
      };

      const result = zodSchema?.parse(testData);
      expect(result).toEqual(testData);
      expect(result?.filter).toBeDefined();
      expect(result?.filter?.title?._icontains).toBe('Pirate');
    });

    it('should not treat objects with additionalProperties as empty', () => {
      const schema: JsonSchemaType = {
        type: 'object',
        additionalProperties: {
          type: 'string',
        },
      };

      const zodSchemaWithoutAllow = convertWithResolvedRefs(schema, {
        allowEmptyObject: false,
      });

      // Should not return undefined because it has additionalProperties
      expect(zodSchemaWithoutAllow).toBeDefined();

      const testData = {
        customField: 'value',
      };

      expect(zodSchemaWithoutAllow?.parse(testData)).toEqual(testData);
    });

    it('should handle oneOf with object containing only additionalProperties', () => {
      const schema: JsonSchemaType & { oneOf?: any } = {
        type: 'object',
        properties: {},
        oneOf: [
          {
            type: 'object',
            additionalProperties: true,
          },
          {
            type: 'object',
            properties: {
              specificField: { type: 'string' },
            },
          },
        ],
      };

      const zodSchema = convertWithResolvedRefs(schema, {
        allowEmptyObject: false,
        transformOneOfAnyOf: true,
      });

      expect(zodSchema).toBeDefined();

      // Test with additional properties
      const testData1 = {
        randomField: 'value',
        anotherField: 123,
      };

      expect(zodSchema?.parse(testData1)).toEqual(testData1);

      // Test with specific field
      const testData2 = {
        specificField: 'test',
      };

      expect(zodSchema?.parse(testData2)).toEqual(testData2);
    });

    it('should handle complex nested schema with $ref-like structure', () => {
      const schema: JsonSchemaType & { anyOf?: any } = {
        type: 'object',
        properties: {
          query: {
            type: 'object',
            properties: {
              filter: {
                description: 'Filter conditions',
                anyOf: [
                  {
                    // This simulates a resolved $ref
                    anyOf: [
                      {
                        type: 'object',
                        properties: {
                          _or: {
                            type: 'array',
                            items: { type: 'object' },
                          },
                        },
                        required: ['_or'],
                      },
                      {
                        type: 'object',
                        additionalProperties: {
                          anyOf: [
                            {
                              type: 'object',
                              properties: {
                                _icontains: { type: 'string' },
                                _eq: { type: 'string' },
                              },
                            },
                          ],
                        },
                      },
                    ],
                  },
                  {
                    type: 'null',
                  },
                ],
              } as JsonSchemaType & { anyOf?: any },
            },
          },
        },
      };

      const zodSchema = convertWithResolvedRefs(schema, {
        allowEmptyObject: false,
        transformOneOfAnyOf: true,
      });

      expect(zodSchema).toBeDefined();

      const testData = {
        query: {
          filter: {
            title: {
              _icontains: 'Pirate',
            },
          },
        },
      };

      const result = zodSchema?.parse(testData);
      expect(result).toEqual(testData);
      expect(result?.query?.filter?.title?._icontains).toBe('Pirate');
    });
  });

  describe('$ref resolution with resolveJsonSchemaRefs', () => {
    it('should handle schemas with $ref references when resolved', () => {
      const schemaWithRefs = {
        type: 'object' as const,
        properties: {
          collection: {
            type: 'string' as const,
          },
          query: {
            type: 'object' as const,
            properties: {
              filter: {
                anyOf: [{ $ref: '#/$defs/__schema0' }, { type: 'null' as const }],
              },
            },
          },
        },
        required: ['collection', 'query'],
        $defs: {
          __schema0: {
            anyOf: [
              {
                type: 'object' as const,
                properties: {
                  _or: {
                    type: 'array' as const,
                    items: { $ref: '#/$defs/__schema0' },
                  },
                },
                required: ['_or'],
              },
              {
                type: 'object' as const,
                additionalProperties: {
                  anyOf: [
                    {
                      type: 'object' as const,
                      properties: {
                        _eq: {
                          anyOf: [
                            { type: 'string' as const },
                            { type: 'number' as const },
                            { type: 'null' as const },
                          ],
                        },
                      },
                    },
                  ],
                },
              },
            ],
          },
        },
      };

      // First test without resolving refs - should not work properly
      // Intentionally NOT using convertWithResolvedRefs here to test the behavior without ref resolution
      const zodSchemaUnresolved = convertJsonSchemaToZod(schemaWithRefs as any, {
        allowEmptyObject: true,
        transformOneOfAnyOf: true,
      });

      const testData = {
        collection: 'posts',
        query: {
          filter: {
            status: {
              _eq: 'draft',
            },
          },
        },
      };

      // Without resolving refs, the filter field won't work correctly
      const resultUnresolved = zodSchemaUnresolved?.parse(testData);
      expect(resultUnresolved?.query?.filter).toEqual({});

      // Now resolve refs first
      const resolvedSchema = resolveJsonSchemaRefs(schemaWithRefs);

      // Verify refs were resolved
      expect(resolvedSchema.properties?.query?.properties?.filter?.anyOf?.[0]).not.toHaveProperty(
        '$ref',
      );
      expect(resolvedSchema.properties?.query?.properties?.filter?.anyOf?.[0]).toHaveProperty(
        'anyOf',
      );

      // Already resolved manually above, so we use convertJsonSchemaToZod directly
      const zodSchemaResolved = convertJsonSchemaToZod(resolvedSchema as any, {
        allowEmptyObject: true,
        transformOneOfAnyOf: true,
      });

      // With resolved refs, it should work correctly
      const resultResolved = zodSchemaResolved?.parse(testData);
      expect(resultResolved).toEqual(testData);
      expect(resultResolved?.query?.filter?.status?._eq).toBe('draft');
    });

    it('should handle circular $ref references without infinite loops', () => {
      const schemaWithCircularRefs = {
        type: 'object' as const,
        properties: {
          node: { $ref: '#/$defs/TreeNode' },
        },
        $defs: {
          TreeNode: {
            type: 'object' as const,
            properties: {
              value: { type: 'string' as const },
              children: {
                type: 'array' as const,
                items: { $ref: '#/$defs/TreeNode' },
              },
            },
          },
        },
      };

      // Should not throw or hang
      const resolved = resolveJsonSchemaRefs(schemaWithCircularRefs);
      expect(resolved).toBeDefined();

      // The circular reference should be broken with a simple object schema
      // Already resolved manually above, so we use convertJsonSchemaToZod directly
      const zodSchema = convertJsonSchemaToZod(resolved as any, {
        allowEmptyObject: true,
        transformOneOfAnyOf: true,
      });

      expect(zodSchema).toBeDefined();

      const testData = {
        node: {
          value: 'root',
          children: [
            {
              value: 'child1',
              children: [],
            },
          ],
        },
      };

      expect(() => zodSchema?.parse(testData)).not.toThrow();
    });

    it('should handle various edge cases safely', () => {
      // Test with null/undefined
      expect(resolveJsonSchemaRefs(null as any)).toBeNull();
      expect(resolveJsonSchemaRefs(undefined as any)).toBeUndefined();

      // Test with non-object primitives
      expect(resolveJsonSchemaRefs('string' as any)).toBe('string');
      expect(resolveJsonSchemaRefs(42 as any)).toBe(42);
      expect(resolveJsonSchemaRefs(true as any)).toBe(true);

      // Test with arrays
      const arrayInput = [{ type: 'string' }, { $ref: '#/def' }];
      const arrayResult = resolveJsonSchemaRefs(arrayInput as any);
      expect(Array.isArray(arrayResult)).toBe(true);
      expect(arrayResult).toHaveLength(2);

      // Test with schema that has no refs
      const noRefSchema = {
        type: 'object' as const,
        properties: {
          name: { type: 'string' as const },
          nested: {
            type: 'object' as const,
            properties: {
              value: { type: 'number' as const },
            },
          },
        },
      };

      const resolvedNoRef = resolveJsonSchemaRefs(noRefSchema);
      expect(resolvedNoRef).toEqual(noRefSchema);

      // Test with invalid ref (non-existent)
      const invalidRefSchema = {
        type: 'object' as const,
        properties: {
          item: { $ref: '#/$defs/nonExistent' },
        },
        $defs: {
          other: { type: 'string' as const },
        },
      };

      const resolvedInvalid = resolveJsonSchemaRefs(invalidRefSchema);
      // Invalid refs should be preserved as-is
      expect(resolvedInvalid.properties?.item?.$ref).toBe('#/$defs/nonExistent');

      // Test with empty object
      expect(resolveJsonSchemaRefs({})).toEqual({});

      // Test with schema containing special JSON Schema keywords
      const schemaWithKeywords = {
        type: 'object' as const,
        properties: {
          value: {
            type: 'string' as const,
            minLength: 5,
            maxLength: 10,
            pattern: '^[A-Z]',
          },
        },
        additionalProperties: false,
        minProperties: 1,
      };

      const resolvedKeywords = resolveJsonSchemaRefs(schemaWithKeywords);
      expect(resolvedKeywords).toEqual(schemaWithKeywords);
      expect(resolvedKeywords.properties?.value?.minLength).toBe(5);
      expect(resolvedKeywords.additionalProperties).toBe(false);
    });
  });

  describe('Bare object schema handling for dynamic properties', () => {
    it('should handle object type without explicit properties but expecting dynamic field definitions', () => {
      // This simulates the Kintone add_fields tool schema
      const schema: JsonSchemaType = {
        type: 'object',
        properties: {
          app_id: {
            type: 'number',
            description: 'アプリID',
          },
          properties: {
            type: 'object',
            description: 'フィールドの設定(各フィールドには code, type, label の指定が必須)',
          },
        },
        required: ['app_id', 'properties'],
      };

      const zodSchema = convertWithResolvedRefs(schema);

      // Test case 1: Basic field definition
      const testData1 = {
        app_id: 810,
        properties: {
          minutes_id: {
            code: 'minutes_id',
            type: 'SINGLE_LINE_TEXT',
            label: 'minutes_id',
          },
        },
      };

      // WITH THE FIX: Bare object schemas now act as passthrough
      const result = zodSchema?.parse(testData1);
      expect(result).toEqual(testData1); // Properties pass through!
    });

    it('should work when properties field has additionalProperties true', () => {
      const schema: JsonSchemaType = {
        type: 'object',
        properties: {
          app_id: {
            type: 'number',
            description: 'アプリID',
          },
          properties: {
            type: 'object',
            description: 'フィールドの設定(各フィールドには code, type, label の指定が必須)',
            additionalProperties: true,
          },
        },
        required: ['app_id', 'properties'],
      };

      const zodSchema = convertWithResolvedRefs(schema);

      const testData = {
        app_id: 810,
        properties: {
          minutes_id: {
            code: 'minutes_id',
            type: 'SINGLE_LINE_TEXT',
            label: 'minutes_id',
          },
        },
      };

      const result = zodSchema?.parse(testData);
      expect(result).toEqual(testData);
      expect(result?.properties?.minutes_id).toBeDefined();
    });

    it('should work with proper field type definitions in additionalProperties', () => {
      const schema: JsonSchemaType = {
        type: 'object',
        properties: {
          app_id: {
            type: 'number',
            description: 'アプリID',
          },
          properties: {
            type: 'object',
            description: 'フィールドの設定(各フィールドには code, type, label の指定が必須)',
            additionalProperties: {
              type: 'object',
              properties: {
                type: { type: 'string' },
                code: { type: 'string' },
                label: { type: 'string' },
                required: { type: 'boolean' },
                options: {
                  type: 'object',
                  additionalProperties: {
                    type: 'object',
                    properties: {
                      label: { type: 'string' },
                      index: { type: 'string' },
                    },
                  },
                },
              },
              required: ['type', 'code', 'label'],
            },
          },
        },
        required: ['app_id', 'properties'],
      };

      const zodSchema = convertWithResolvedRefs(schema);

      // Test case 1: Simple text field
      const testData1 = {
        app_id: 810,
        properties: {
          minutes_id: {
            code: 'minutes_id',
            type: 'SINGLE_LINE_TEXT',
            label: 'minutes_id',
            required: false,
          },
        },
      };

      const result1 = zodSchema?.parse(testData1);
      expect(result1).toEqual(testData1);

      // Test case 2: Dropdown field with options
      const testData2 = {
        app_id: 820,
        properties: {
          status: {
            type: 'DROP_DOWN',
            code: 'status',
            label: 'Status',
            options: {
              'Not Started': {
                label: 'Not Started',
                index: '0',
              },
              'In Progress': {
                label: 'In Progress',
                index: '1',
              },
            },
          },
        },
      };

      const result2 = zodSchema?.parse(testData2);
      expect(result2).toEqual(testData2);

      // Test case 3: Multiple fields
      const testData3 = {
        app_id: 123,
        properties: {
          number_field: {
            type: 'NUMBER',
            code: 'number_field',
            label: '数値フィールド',
          },
          text_field: {
            type: 'SINGLE_LINE_TEXT',
            code: 'text_field',
            label: 'テキストフィールド',
          },
        },
      };

      const result3 = zodSchema?.parse(testData3);
      expect(result3).toEqual(testData3);
    });

    it('should handle the actual reported failing case', () => {
      // This is the exact schema that's failing for the user
      const schema: JsonSchemaType = {
        type: 'object',
        properties: {
          app_id: {
            type: 'number',
            description: 'アプリID',
          },
          properties: {
            type: 'object',
            description: 'フィールドの設定(各フィールドには code, type, label の指定が必須)',
          },
        },
        required: ['app_id', 'properties'],
      };

      const zodSchema = convertWithResolvedRefs(schema);

      // The exact data the user is trying to send
      const userData = {
        app_id: 810,
        properties: {
          minutes_id: {
            code: 'minutes_id',
            type: 'SINGLE_LINE_TEXT',
            label: 'minutes_id',
            required: false,
          },
        },
      };

      // WITH THE FIX: The properties now pass through correctly!
      const result = zodSchema?.parse(userData);
      expect(result).toEqual(userData);

      // This fixes the error "properties requires at least one field definition"
      // The MCP server now receives the full properties object
    });

    it('should demonstrate fix by treating bare object type as passthrough', () => {
      // Test what happens if we modify the conversion to treat bare object types
      // without properties as passthrough schemas
      const schema: JsonSchemaType = {
        type: 'object',
        properties: {
          app_id: {
            type: 'number',
            description: 'アプリID',
          },
          properties: {
            type: 'object',
            description: 'フィールドの設定(各フィールドには code, type, label の指定が必須)',
          },
        },
        required: ['app_id', 'properties'],
      };

      // For now, we'll simulate the fix by adding additionalProperties
      const fixedSchema: JsonSchemaType = {
        ...schema,
        properties: {
          ...schema.properties,
          properties: {
            ...(schema.properties!.properties as JsonSchemaType),
            additionalProperties: true,
          },
        },
      };

      const zodSchema = convertWithResolvedRefs(fixedSchema);

      const userData = {
        app_id: 810,
        properties: {
          minutes_id: {
            code: 'minutes_id',
            type: 'SINGLE_LINE_TEXT',
            label: 'minutes_id',
            required: false,
          },
        },
      };

      const result = zodSchema?.parse(userData);
      expect(result).toEqual(userData);
    });

    it('should NOT treat object schemas with $ref or complex properties as bare objects', () => {
      // This test ensures our fix doesn't affect schemas with $ref or other complex structures
      const schemaWithRef = {
        type: 'object' as const,
        properties: {
          data: {
            type: 'object' as const,
            // This has anyOf with $ref - should NOT be treated as a bare object
            anyOf: [{ $ref: '#/$defs/dataSchema' }, { type: 'null' as const }],
          },
        },
        $defs: {
          dataSchema: {
            type: 'object' as const,
            additionalProperties: {
              type: 'string' as const,
            },
          },
        },
      };

      // Convert without resolving refs
      const zodSchema = convertJsonSchemaToZod(schemaWithRef as any, {
        transformOneOfAnyOf: true,
      });

      const testData = {
        data: {
          field1: 'value1',
          field2: 'value2',
        },
      };

      // Without ref resolution, the data field should be stripped/empty
      const result = zodSchema?.parse(testData);
      expect(result?.data).toEqual({});
    });

    it('should NOT treat object schemas with oneOf/anyOf as bare objects', () => {
      // Ensure schemas with oneOf/anyOf are not treated as bare objects
      const schemaWithOneOf = {
        type: 'object' as const,
        properties: {
          config: {
            type: 'object' as const,
            // Empty properties but has oneOf - should NOT be passthrough
            oneOf: [
              { properties: { type: { const: 'A' } } },
              { properties: { type: { const: 'B' } } },
            ],
          } as any,
        },
      };

      const zodSchema = convertWithResolvedRefs(schemaWithOneOf as any, {
        transformOneOfAnyOf: true,
      });

      const testData = {
        config: {
          randomField: 'should not pass through',
        },
      };

      // The random field should be stripped because this isn't a bare object
      const result = zodSchema?.parse(testData);
      expect(result?.config).toEqual({});
    });
  });
});
Interval Cleanup

Polling uses setInterval and stores handles in state; verify all intervals are reliably cleared on success, timeout, error, and on unmount. Consider a useEffect cleanup to clear any remaining intervals to avoid leaks.

const startServerPolling = useCallback(
  (serverName: string) => {
    const pollInterval = setInterval(async () => {
      try {
        await queryClient.refetchQueries([QueryKeys.mcpConnectionStatus]);

        const freshConnectionData = queryClient.getQueryData([
          QueryKeys.mcpConnectionStatus,
        ]) as any;
        const freshConnectionStatus = freshConnectionData?.connectionStatus || {};

        const state = serverStates[serverName];
        const serverStatus = freshConnectionStatus[serverName];

        if (serverStatus?.connectionState === 'connected') {
          clearInterval(pollInterval);

          showToast({
            message: localize('com_ui_mcp_authenticated_success', { 0: serverName }),
            status: 'success',
          });

          const currentValues = mcpValuesRef.current ?? [];
          if (!currentValues.includes(serverName)) {
            setMCPValues([...currentValues, serverName]);
          }

          await queryClient.invalidateQueries([QueryKeys.tools]);

          // This delay is to ensure UI has updated with new connection status before cleanup
          // Otherwise servers will show as disconnected for a second after OAuth flow completes
          setTimeout(() => {
            cleanupServerState(serverName);
          }, 1000);
          return;
        }

        if (state?.oauthStartTime && Date.now() - state.oauthStartTime > 180000) {
          showToast({
            message: localize('com_ui_mcp_oauth_timeout', { 0: serverName }),
            status: 'error',
          });
          clearInterval(pollInterval);
          cleanupServerState(serverName);
          return;
        }

        if (serverStatus?.connectionState === 'error') {
          showToast({
            message: localize('com_ui_mcp_init_failed'),
            status: 'error',
          });
          clearInterval(pollInterval);
          cleanupServerState(serverName);
          return;
        }
      } catch (error) {
        console.error(`[MCP Manager] Error polling server ${serverName}:`, error);
        clearInterval(pollInterval);
        cleanupServerState(serverName);
        return;
      }
    }, 3500);

    updateServerState(serverName, { pollInterval });
  },
  [
    queryClient,
    serverStates,
    showToast,
    localize,
    setMCPValues,
    cleanupServerState,
    updateServerState,
  ],
);
Backward Compatibility

Schema changes add fields (verbosity, web_search, disableStreaming) and move isPinned/pinnedOrder. Validate API consumers and persistence layers tolerate these additions and optionality, and that min bounds for penalties changing to -2 don’t break UI validation.

export enum Verbosity {
  none = '',
  low = 'low',
  medium = 'medium',
  high = 'high',
}

export const imageDetailNumeric = {
  [ImageDetail.low]: 0,
  [ImageDetail.auto]: 1,

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Reuse exact redirect URI

Ensure redirectUri matches the value used during startAuthorization to prevent
authorization code mismatch errors. Persist and reuse the exact redirect URI
chosen for the flow instead of recomputing it here.

packages/api/src/mcp/oauth/handler.ts [372-379]

+const redirectUri =
+  metadata.redirectUri ??
+  metadata.clientInfo.redirect_uris?.[0] ??
+  this.getDefaultRedirectUri();
+
 const tokens = await exchangeAuthorization(metadata.serverUrl, {
-  redirectUri: metadata.clientInfo.redirect_uris?.[0] || this.getDefaultRedirectUri(),
+  redirectUri,
   metadata: metadata.metadata as unknown as SDKOAuthMetadata,
   clientInformation: metadata.clientInfo,
   codeVerifier: metadata.codeVerifier,
   authorizationCode,
   resource,
 });

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 9

__

Why: This is a critical fix for the OAuth flow, as using a mismatched redirect_uri during token exchange would cause the authorization to fail.

High
Verify connection before counting initialized

Avoid marking a server as initialized solely based on a resolved function call.
Verify the underlying connection state before counting it successful to prevent
false positives when OAuth is required or timeouts occur.

packages/api/src/mcp/manager.ts [44-84]

 public async initializeMCPs({
   mcpServers,
   flowManager,
   tokenMethods,
 }: {
   mcpServers: t.MCPServers;
   flowManager: FlowStateManager<MCPOAuthTokens | null>;
   tokenMethods?: TokenMethods;
 }): Promise<void> {
   this.mcpConfigs = mcpServers;
   const entries = Object.entries(mcpServers);
-  const initializedServers = new Set();
+  const initializedServers = new Set<number>();
   const connectionResults = await Promise.allSettled(
     entries.map(async ([serverName, config], i) => {
       try {
         await this.initializeMCP({
           serverName,
           config,
           flowManager,
           tokenMethods,
         });
-        initializedServers.add(i);
+        const conn = this.connections.get(serverName);
+        if (conn && (await conn.isConnected())) {
+          initializedServers.add(i);
+        } else {
+          logger.warn(`[MCP][${serverName}] Skipping success count: not connected`);
+        }
       } catch (error) {
         logger.error(`[MCP][${serverName}] Initialization failed`, error);
       }
     }),
   );
-  const successfulConnections = connectionResults.filter((result) => result.status === 'fulfilled').length;
+  const successfulConnections = initializedServers.size;
   logger.info(`[MCP] Initialized ${successfulConnections}/${entries.length} servers`);
   if (successfulConnections === 0) {
     throw new Error('No MCP servers initialized successfully');
   }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that initializeMCP can resolve without throwing an error even if the connection fails (e.g., due to skipped OAuth), leading to an incorrect count of successful initializations. Checking conn.isConnected() ensures only truly active connections are counted as successful, which is a critical fix for the new logic.

Medium
Fix polling state staleness and leaks

Avoid capturing stale serverStates inside the interval callback. Read the latest
state via a ref or pass timeout start in closure. Also ensure the interval is
cleared on unmount to prevent leaks if the component unmounts during polling.

client/src/hooks/MCP/useMCPServerManager.ts [136-211]

+const serverStatesRef = useRef(serverStates);
+useEffect(() => {
+  serverStatesRef.current = serverStates;
+}, [serverStates]);
+
+useEffect(() => {
+  return () => {
+    // cleanup all pending intervals on unmount
+    Object.values(serverStatesRef.current).forEach((s) => {
+      if (s?.pollInterval) clearInterval(s.pollInterval);
+    });
+  };
+}, []);
+
 const startServerPolling = useCallback(
   (serverName: string) => {
     const pollInterval = setInterval(async () => {
       try {
         await queryClient.refetchQueries([QueryKeys.mcpConnectionStatus]);
-
-        const freshConnectionData = queryClient.getQueryData([
-          QueryKeys.mcpConnectionStatus,
-        ]) as any;
+        const freshConnectionData = queryClient.getQueryData([QueryKeys.mcpConnectionStatus]) as any;
         const freshConnectionStatus = freshConnectionData?.connectionStatus || {};
 
-        const state = serverStates[serverName];
+        const state = serverStatesRef.current[serverName];
         const serverStatus = freshConnectionStatus[serverName];
 
         if (serverStatus?.connectionState === 'connected') {
           clearInterval(pollInterval);
-
-          showToast({
-            message: localize('com_ui_mcp_authenticated_success', { 0: serverName }),
-            status: 'success',
-          });
-
+          showToast({ message: localize('com_ui_mcp_authenticated_success', { 0: serverName }), status: 'success' });
           const currentValues = mcpValuesRef.current ?? [];
           if (!currentValues.includes(serverName)) {
             setMCPValues([...currentValues, serverName]);
           }
-
           await queryClient.invalidateQueries([QueryKeys.tools]);
-
-          // This delay is to ensure UI has updated with new connection status before cleanup
-          // Otherwise servers will show as disconnected for a second after OAuth flow completes
-          setTimeout(() => {
-            cleanupServerState(serverName);
-          }, 1000);
+          setTimeout(() => cleanupServerState(serverName), 1000);
           return;
         }
 
         if (state?.oauthStartTime && Date.now() - state.oauthStartTime > 180000) {
-          showToast({
-            message: localize('com_ui_mcp_oauth_timeout', { 0: serverName }),
-            status: 'error',
-          });
+          showToast({ message: localize('com_ui_mcp_oauth_timeout', { 0: serverName }), status: 'error' });
           clearInterval(pollInterval);
           cleanupServerState(serverName);
           return;
         }
 
         if (serverStatus?.connectionState === 'error') {
-          showToast({
-            message: localize('com_ui_mcp_init_failed'),
-            status: 'error',
-          });
+          showToast({ message: localize('com_ui_mcp_init_failed'), status: 'error' });
           clearInterval(pollInterval);
           cleanupServerState(serverName);
           return;
         }
       } catch (error) {
         console.error(`[MCP Manager] Error polling server ${serverName}:`, error);
         clearInterval(pollInterval);
         cleanupServerState(serverName);
         return;
       }
     }, 3500);
 
     updateServerState(serverName, { pollInterval });
   },
-  [
-    queryClient,
-    serverStates,
-    showToast,
-    localize,
-    setMCPValues,
-    cleanupServerState,
-    updateServerState,
-  ],
+  [queryClient, showToast, localize, setMCPValues, cleanupServerState, updateServerState],
 );

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a stale state capture in the setInterval callback and a missing cleanup for the interval, providing a robust fix using a ref and a useEffect for unmount cleanup.

Medium
Safely guard SharedArrayBuffer access

Accessing SharedArrayBuffer can throw or be undefined in some environments. Use
a typeof/safe check before referencing the constructor to avoid ReferenceErrors
in browsers without support.

packages/data-schemas/src/utils/object-traverse.ts [43]

-if (value instanceof SharedArrayBuffer) return false;
+// Check for SharedArrayBuffer when available
+// eslint-disable-next-line no-undef
+if (typeof SharedArrayBuffer !== 'undefined' && value instanceof SharedArrayBuffer) {
+  return false;
+}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly prevents a potential ReferenceError in environments where SharedArrayBuffer is not available, improving the utility's robustness and portability.

Medium
Guard against duplicate initialization

Prevent duplicate initialization when multiple clicks occur. Disable re-entry
per server while isInitializing or an OAuth flow is active to avoid racing
mutations and multiple tabs opening.

client/src/hooks/MCP/useMCPServerManager.ts [213-276]

 const initializeServer = useCallback(
   async (serverName: string, autoOpenOAuth: boolean = true) => {
+    const currentState = serverStatesRef.current[serverName];
+    if (currentState?.isInitializing || currentState?.oauthUrl) {
+      return;
+    }
+
     updateServerState(serverName, { isInitializing: true });
-
     try {
       const response = await reinitializeMutation.mutateAsync(serverName);
 
       if (response.success) {
         if (response.oauthRequired && response.oauthUrl) {
           updateServerState(serverName, {
             oauthUrl: response.oauthUrl,
             oauthStartTime: Date.now(),
             isCancellable: true,
             isInitializing: true,
           });
 
           if (autoOpenOAuth) {
             window.open(response.oauthUrl, '_blank', 'noopener,noreferrer');
           }
 
           startServerPolling(serverName);
         } else {
           await queryClient.refetchQueries([QueryKeys.mcpConnectionStatus]);
+          showToast({ message: localize('com_ui_mcp_initialized_success', { 0: serverName }), status: 'success' });
 
-          showToast({
-            message: localize('com_ui_mcp_initialized_success', { 0: serverName }),
-            status: 'success',
-          });
-
-          const currentValues = mcpValues ?? [];
+          const currentValues = mcpValuesRef.current ?? [];
           if (!currentValues.includes(serverName)) {
             setMCPValues([...currentValues, serverName]);
           }
 
           cleanupServerState(serverName);
         }
       } else {
-        showToast({
-          message: localize('com_ui_mcp_init_failed', { 0: serverName }),
-          status: 'error',
-        });
+        showToast({ message: localize('com_ui_mcp_init_failed', { 0: serverName }), status: 'error' });
         cleanupServerState(serverName);
       }
     } catch (error) {
       console.error(`[MCP Manager] Failed to initialize ${serverName}:`, error);
-      showToast({
-        message: localize('com_ui_mcp_init_failed', { 0: serverName }),
-        status: 'error',
-      });
+      showToast({ message: localize('com_ui_mcp_init_failed', { 0: serverName }), status: 'error' });
       cleanupServerState(serverName);
     }
   },
-  [
-    updateServerState,
-    reinitializeMutation,
-    startServerPolling,
-    queryClient,
-    showToast,
-    localize,
-    mcpValues,
-    cleanupServerState,
-    setMCPValues,
-  ],
+  [updateServerState, reinitializeMutation, startServerPolling, queryClient, showToast, localize, setMCPValues, cleanupServerState],
 );

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies a potential race condition and adds a guard to prevent initializeServer from being called multiple times, which improves the hook's robustness against rapid user input.

Low
Sanitize tool value types

Guard against non-boolean/string values stored in ephemeralAgent[toolKey] to
avoid unexpected UI states and localStorage pollution. Coerce only booleans and
strings, and ignore objects/numbers by falling back to false.

client/src/hooks/Plugins/useToolToggle.ts [75-85]

-const toolValue = useMemo(() => {
-  return ephemeralAgent?.[toolKey] ?? false;
+const toolValue = useMemo<ToolValue | false>(() => {
+  const value = ephemeralAgent?.[toolKey];
+  if (typeof value === 'boolean' || typeof value === 'string') {
+    return value as ToolValue;
+  }
+  return false;
 }, [ephemeralAgent, toolKey]);
 
 const isToolEnabled = useMemo(() => {
-  // For backward compatibility, treat truthy string values as enabled
   if (typeof toolValue === 'string') {
     return toolValue.length > 0;
   }
   return toolValue === true;
 }, [toolValue]);

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion improves robustness by ensuring the toolValue is always a boolean or string, preventing potential bugs from unexpected data types in the ephemeralAgent state.

Low
General
Fail safe when probes inconclusive

Relying on this.connectionState === 'connected' after a failed capability check
can mask disconnections. Return false immediately when all probes fail to avoid
stale connections being treated as healthy.

packages/api/src/mcp/connection.ts [586-641]

 public async isConnected(): Promise<boolean> {
-  // First check if we're in a connected state
   if (this.connectionState !== 'connected') {
     return false;
   }
   try {
-    // Try ping first as it's the lightest check
     await this.client.ping();
-    return this.connectionState === 'connected';
+    return true;
   } catch (error) {
-    // Check if the error is because ping is not supported (method not found)
     const pingUnsupported =
       error instanceof Error &&
-      ((error as Error)?.message.includes('-32601') ||
-        (error as Error)?.message.includes('invalid method ping') ||
-        (error as Error)?.message.includes('method not found'));
+      (error.message.includes('-32601') ||
+        error.message.includes('invalid method ping') ||
+        error.message.includes('method not found'));
     if (!pingUnsupported) {
       logger.error(`${this.getLogPrefix()} Ping failed:`, error);
       return false;
     }
-    // Ping is not supported by this server, try an alternative verification
-    logger.debug(
-      `${this.getLogPrefix()} Server does not support ping method, verifying connection with capabilities`,
-    );
+    logger.debug(`${this.getLogPrefix()} Ping unsupported; verifying via capabilities`);
     try {
-      // Get server capabilities to verify connection is truly active
       const capabilities = this.client.getServerCapabilities();
-      // If we have capabilities, try calling a supported method to verify connection
       if (capabilities?.tools) {
         await this.client.listTools();
-        return this.connectionState === 'connected';
-      } else if (capabilities?.resources) {
+        return true;
+      }
+      if (capabilities?.resources) {
         await this.client.listResources();
-        return this.connectionState === 'connected';
-      } else if (capabilities?.prompts) {
+        return true;
+      }
+      if (capabilities?.prompts) {
         await this.client.listPrompts();
-        return this.connectionState === 'connected';
-      } else {
-        // No capabilities to test, but we're in connected state and initialization succeeded
-        logger.debug(
-          `${this.getLogPrefix()} No capabilities to test, assuming connected based on state`,
-        );
-        return this.connectionState === 'connected';
+        return true;
       }
+      // No verifiable capability; treat as not connected to be safe
+      logger.warn(`${this.getLogPrefix()} No verifiable capability; treating as disconnected`);
+      return false;
     } catch (capabilityError) {
-      // If capability check fails, the connection is likely broken
       logger.error(`${this.getLogPrefix()} Connection verification failed:`, capabilityError);
       return false;
     }
   }
 }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that assuming a connection is active when no verification method (ping or capability checks) is available can lead to false positives. Returning false in this ambiguous case is a safer, more robust approach to prevent using a potentially stale connection.

Low
  • More

@jmaddington jmaddington force-pushed the main branch 2 times, most recently from 0a8419e to 772fa5a Compare January 1, 2026 18:01
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.

2 participants

Comments