From da0491d5dbec567f8e35f38c29fff5e7c206043d Mon Sep 17 00:00:00 2001 From: Ravi Kumar L Date: Tue, 18 Aug 2026 17:16:58 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=BB=20fix(agents):=20require=20Code=20?= =?UTF-8?q?Interpreter=20for=20programmatic=20MCP=20tools=20(#14977)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(agents): require code interpreter for programmatic MCP tools * test(data-provider): fix tool options fixture type * fix(agents): address programmatic tool review feedback * fix(agents): avoid no-op update on version revert --- api/server/controllers/agents/v1.js | 53 ++++- api/server/controllers/agents/v1.spec.js | 224 ++++++++++++++++++ .../SidePanel/Agents/AgentPanel.tsx | 5 +- .../SidePanel/Agents/MCPToolItem.tsx | 9 +- .../ItemDialog/__tests__/McpSection.spec.tsx | 53 ++++- .../Tools/ItemDialog/sections/McpSection.tsx | 21 +- .../Agents/Tools/ToolsMarketplaceDialog.tsx | 6 + .../SidePanel/Agents/Tools/ToolsSection.tsx | 12 +- .../__tests__/ToolsMarketplaceDialog.spec.tsx | 31 ++- .../Tools/__tests__/ToolsSection.spec.tsx | 31 +++ .../__tests__/AgentPanel.helpers.spec.ts | 26 ++ .../Agents/__tests__/MCPToolItem.spec.tsx | 25 +- client/src/locales/en/translation.json | 1 + .../src/agentToolOptions.spec.ts | 38 +++ .../data-provider/src/agentToolOptions.ts | 36 +++ packages/data-provider/src/index.ts | 1 + 16 files changed, 557 insertions(+), 15 deletions(-) create mode 100644 packages/data-provider/src/agentToolOptions.spec.ts create mode 100644 packages/data-provider/src/agentToolOptions.ts diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 296a36f908..4ccbdb44a1 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -37,6 +37,7 @@ const { AgentCapabilities, EModelEndpoint, resolveAllowedStatefulCodeEnvironments, + removeCodeExecutionCaller, removeNullishValues, } = require('librechat-data-provider'); const { @@ -271,6 +272,12 @@ const isSubagentsCapabilityEnabled = (req) => { return capabilities.includes(AgentCapabilities.subagents); }; +const isCodeInterpreterCapabilityEnabled = (req) => { + const capabilities = req.config?.endpoints?.[EModelEndpoint.agents]?.capabilities; + if (!Array.isArray(capabilities)) return false; + return capabilities.includes(AgentCapabilities.execute_code); +}; + /** Reject a newly selected stateful workspace scope that the deployment owner * has excluded. Disabled sessions and unrelated edits remain saveable so an * allowlist tightening never silently rewrites or strands an existing agent. */ @@ -533,6 +540,13 @@ const createAgentHandler = async (req, res) => { const validatedData = agentCreateSchema.parse(req.body); const { tools = [], ...agentData } = removeNullishValues(validatedData); + if ( + (!isCodeInterpreterCapabilityEnabled(req) || !tools.includes(Tools.execute_code)) && + agentData.tool_options != null + ) { + agentData.tool_options = removeCodeExecutionCaller(agentData.tool_options); + } + if ( !validateStatefulCodeEnvironment( req, @@ -818,7 +832,12 @@ const updateAgentHandler = async (req, res) => { updateData.stateful_code_sessions !== undefined || updateData.stateful_code_environment !== undefined; const includesToolsConfiguration = Array.isArray(updateData.tools); - if (includesStatefulConfiguration || includesToolsConfiguration) { + const includesToolOptionsConfiguration = updateData.tool_options !== undefined; + if ( + includesStatefulConfiguration || + includesToolsConfiguration || + includesToolOptionsConfiguration + ) { existingAgent = await db.getAgent({ id }); if (!existingAgent) { return res.status(404).json({ error: 'Agent not found' }); @@ -851,6 +870,18 @@ const updateAgentHandler = async (req, res) => { return; } } + + if (includesToolsConfiguration || includesToolOptionsConfiguration) { + const effectiveTools = updateData.tools ?? existingAgent.tools; + const effectiveToolOptions = updateData.tool_options ?? existingAgent.tool_options; + if ( + (!isCodeInterpreterCapabilityEnabled(req) || + !effectiveTools?.includes(Tools.execute_code)) && + effectiveToolOptions != null + ) { + updateData.tool_options = removeCodeExecutionCaller(effectiveToolOptions); + } + } } if (updateData.model_parameters && typeof updateData.model_parameters === 'object') { @@ -1265,6 +1296,14 @@ const duplicateAgentHandler = async (req, res) => { }); } + if ( + (!isCodeInterpreterCapabilityEnabled(req) || + !newAgentData.tools?.includes(Tools.execute_code)) && + newAgentData.tool_options != null + ) { + newAgentData.tool_options = removeCodeExecutionCaller(newAgentData.tool_options); + } + const newAgent = await db.createAgent(newAgentData); try { @@ -1757,6 +1796,18 @@ const revertAgentVersionHandler = async (req, res) => { } } + const effectiveRevertTools = revertUpdates.tools ?? updatedAgent.tools; + const hasCodeExecutionCaller = Object.values(updatedAgent.tool_options ?? {}).some((options) => + options.allowed_callers?.includes('code_execution'), + ); + if ( + (!isCodeInterpreterCapabilityEnabled(req) || + !effectiveRevertTools?.includes(Tools.execute_code)) && + hasCodeExecutionCaller + ) { + revertUpdates.tool_options = removeCodeExecutionCaller(updatedAgent.tool_options); + } + if (updatedAgent.tool_resources) { const removedCount = await pruneToolResourceFileIdsForAgent({ tool_resources: updatedAgent.tool_resources, diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index 435b0da43f..e0ed01c94f 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -182,6 +182,23 @@ describe('Agent Controllers - Mass Assignment Protection', () => { }); describe('createAgentHandler', () => { + test('removes programmatic tool options when Code Interpreter capability is disabled', async () => { + mockReq.body = { + name: 'Invalid Programmatic Agent', + provider: 'openai', + model: 'gpt-4', + tools: [Tools.execute_code, 'search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }; + + await createAgentHandler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(201); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + test('rejects a stateful environment excluded by deployment policy', async () => { mockReq.config = { endpoints: { @@ -824,6 +841,136 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(agentInDb.name).toBe('Updated Agent'); }); + test('removes newly added programmatic options when Code Interpreter capability is disabled', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { tools: [Tools.execute_code, 'search_mcp_example'] }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + + test('removes programmatic callers when Code Interpreter is disabled', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + tools: [Tools.execute_code, 'search_mcp_example'], + tool_options: { + search_mcp_example: { + allowed_callers: ['code_execution'], + defer_loading: true, + }, + }, + }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { tools: ['search_mcp_example'] }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({ + search_mcp_example: { defer_loading: true }, + }); + }); + + test('allows unrelated edits to a legacy inconsistent agent', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + tools: ['search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { description: 'Still saveable' }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].description).toBe('Still saveable'); + }); + + test('allows detaching a programmatic tool from a legacy inconsistent agent', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + tools: ['search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { tools: [] }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].tools).toEqual([]); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + + test('allows clearing programmatic options from a legacy inconsistent agent', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + tools: ['search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { tool_options: {} }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + + test('removes all newly submitted programmatic options from a legacy agent', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + tools: ['search_mcp_example', 'lookup_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }, + ); + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + lookup_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }; + + await updateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).not.toHaveBeenCalledWith(400); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + test('rejects selecting a stateful environment excluded by deployment policy', async () => { mockReq.user.id = existingAgentAuthorId.toString(); mockReq.params.id = existingAgentId; @@ -1495,6 +1642,83 @@ describe('Agent Controllers - Mass Assignment Protection', () => { const agentInDb = await Agent.findOne({ id: agent.id }).lean(); expect(agentInDb.tool_resources.file_search.file_ids).toEqual([ownedFileId, otherFileId]); }); + + test('duplicateAgentHandler removes programmatic options without Code Interpreter', async () => { + const sourceAgent = await Agent.create({ + id: `agent_${uuidv4()}`, + name: 'Legacy Programmatic Agent', + provider: 'openai', + model: 'gpt-4', + author: mockReq.user.id, + tools: ['search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }); + const db = require('~/models'); + jest.spyOn(db, 'getActions').mockResolvedValueOnce([]); + mockReq.params.id = sourceAgent.id; + + await duplicateAgentHandler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(201); + expect(mockRes.json.mock.calls[0][0].agent.tool_options).toEqual({}); + }); + + test('revertAgentVersionHandler removes restored programmatic options without Code Interpreter', async () => { + const agent = await Agent.create({ + id: `agent_${uuidv4()}`, + name: 'Current Agent', + provider: 'openai', + model: 'gpt-4', + author: mockReq.user.id, + versions: [ + { + name: 'Legacy Programmatic Agent', + provider: 'openai', + model: 'gpt-4', + tools: ['search_mcp_example'], + tool_options: { + search_mcp_example: { allowed_callers: ['code_execution'] }, + }, + }, + ], + }); + mockReq.params.id = agent.id; + mockReq.body = { version_index: 0 }; + + await revertAgentVersionHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + expect(mockRes.json.mock.calls[0][0].tool_options).toEqual({}); + }); + + test('revertAgentVersionHandler does not update unchanged tool options', async () => { + const agent = await Agent.create({ + id: `agent_${uuidv4()}`, + name: 'Current Agent', + provider: 'openai', + model: 'gpt-4', + author: mockReq.user.id, + versions: [ + { + name: 'Historical Agent', + provider: 'openai', + model: 'gpt-4', + tool_options: {}, + }, + ], + }); + const db = require('~/models'); + const updateAgentSpy = jest.spyOn(db, 'updateAgent'); + mockReq.params.id = agent.id; + mockReq.body = { version_index: 0 }; + + await revertAgentVersionHandler(mockReq, mockRes); + + expect(mockRes.json).toHaveBeenCalled(); + expect(updateAgentSpy).not.toHaveBeenCalled(); + }); }); describe('Mass Assignment Attack Scenarios', () => { diff --git a/client/src/components/SidePanel/Agents/AgentPanel.tsx b/client/src/components/SidePanel/Agents/AgentPanel.tsx index 4dc9226015..7f2517c278 100644 --- a/client/src/components/SidePanel/Agents/AgentPanel.tsx +++ b/client/src/components/SidePanel/Agents/AgentPanel.tsx @@ -11,6 +11,7 @@ import { ResourceType, EModelEndpoint, PermissionBits, + removeCodeExecutionCaller, resolveStatefulCodeEnvironment, isAssistantsEndpoint, } from 'librechat-data-provider'; @@ -91,6 +92,8 @@ export function composeAgentUpdatePayload(data: AgentForm, agent_id?: string | n * execute_code is disabled so a stale opt-in can't silently reactivate later. */ const normalizedStatefulCodeSessions = data.execute_code === true ? stateful_code_sessions : false; + const normalizedToolOptions = + data.execute_code === true ? tool_options : removeCodeExecutionCaller(tool_options); const normalizedStatefulCodeEnvironment = stateful_code_environment ?? 'user'; const shouldResetAvatar = @@ -118,7 +121,7 @@ export function composeAgentUpdatePayload(data: AgentForm, agent_id?: string | n recursion_limit, category, support_contact, - tool_options, + tool_options: normalizedToolOptions, skills, skills_enabled, /** A hidden stale 'agent' scope must not survive disabling memory — diff --git a/client/src/components/SidePanel/Agents/MCPToolItem.tsx b/client/src/components/SidePanel/Agents/MCPToolItem.tsx index d77caf4e7c..b8d971de06 100644 --- a/client/src/components/SidePanel/Agents/MCPToolItem.tsx +++ b/client/src/components/SidePanel/Agents/MCPToolItem.tsx @@ -18,6 +18,7 @@ interface MCPToolItemProps { intentDisabled: boolean; deferredToolsEnabled: boolean; programmaticToolsEnabled: boolean; + programmaticToolsAvailable: boolean; backgroundToolsEnabled: boolean; toolIntentsEnabled: boolean; onToggleSelect: () => void; @@ -44,6 +45,7 @@ export default function MCPToolItem({ onToggleIntent, deferredToolsEnabled, programmaticToolsEnabled, + programmaticToolsAvailable, backgroundToolsEnabled, toolIntentsEnabled, }: MCPToolItemProps) { @@ -95,8 +97,13 @@ export default function MCPToolItem({ icon={Code2} pressed={isProgrammatic} label={localize('com_ui_mcp_programmatic')} - tooltip={localize('com_ui_mcp_click_to_programmatic')} + tooltip={localize( + programmaticToolsAvailable + ? 'com_ui_mcp_click_to_programmatic' + : 'com_ui_mcp_programmatic_requires_code', + )} activeClass="text-violet-500" + disabled={!programmaticToolsAvailable && !isProgrammatic} onToggle={onToggleProgrammatic} /> )} diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx index 0a15331a12..eac90450d2 100644 --- a/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/__tests__/McpSection.spec.tsx @@ -12,7 +12,9 @@ const mockInitializeServer = jest.fn(); const mockIsConnectionDeferred = jest.fn((): boolean => false); const mockToggleIntentAll = jest.fn(); const mockIsToolProgrammaticOnly = jest.fn((_toolId: string): boolean => false); +const mockAreAllToolsProgrammatic = jest.fn((): boolean => false); const mockCapabilities = { + codeEnabled: false, deferredToolsEnabled: false, programmaticToolsEnabled: false, backgroundToolsEnabled: false, @@ -21,10 +23,19 @@ const mockCapabilities = { jest.mock('react-hook-form', () => ({ useFormContext: () => ({ control: {}, setValue: mockSetValue, getValues: mockGetValues }), - useWatch: ({ name }: { name: string }) => - name === 'tool_options' ? mockGetToolOptions() : mockGetValues(), + useWatch: ({ name }: { name: string }) => { + if (name === 'tool_options') { + return mockGetToolOptions(); + } + if (name === 'execute_code') { + return mockCodeInterpreterSelected(); + } + return mockGetValues(); + }, })); +const mockCodeInterpreterSelected = jest.fn((): boolean => false); + jest.mock('~/Providers', () => ({ useAgentPanelContext: () => ({ mcpServersMap: mockMcpServersMap() }), })); @@ -60,7 +71,7 @@ jest.mock('~/hooks', () => ({ toggleToolBackground: jest.fn(), toggleToolIntent: jest.fn(), areAllToolsDeferred: () => false, - areAllToolsProgrammatic: () => false, + areAllToolsProgrammatic: mockAreAllToolsProgrammatic, areAllToolsBackground: () => false, areAllToolsIntent: () => false, toggleDeferAll: jest.fn(), @@ -164,10 +175,18 @@ describe('McpSection', () => { mockToggleIntentAll.mockClear(); mockIsToolProgrammaticOnly.mockReset(); mockIsToolProgrammaticOnly.mockReturnValue(false); + mockAreAllToolsProgrammatic.mockReset(); + mockAreAllToolsProgrammatic.mockReturnValue(false); mockGetToolOptions.mockReset(); mockGetToolOptions.mockReturnValue(undefined); mockMcpServersMap.mockReset(); mockMcpServersMap.mockReturnValue(new Map()); + mockCodeInterpreterSelected.mockReset(); + mockCodeInterpreterSelected.mockReturnValue(false); + mockCapabilities.codeEnabled = false; + mockCapabilities.deferredToolsEnabled = false; + mockCapabilities.programmaticToolsEnabled = false; + mockCapabilities.backgroundToolsEnabled = false; mockCapabilities.toolIntentsEnabled = false; }); @@ -405,6 +424,34 @@ describe('McpSection', () => { expect(mockToggleIntentAll).toHaveBeenCalledWith(item.server.tools); }); + test('bulk programmatic toggle requires Code Interpreter to be available and selected', () => { + mockCapabilities.programmaticToolsEnabled = true; + const { unmount } = render(); + expect(screen.getByRole('button', { name: 'com_ui_mcp_programmatic_all' })).toHaveAttribute( + 'aria-disabled', + 'true', + ); + unmount(); + + mockCapabilities.codeEnabled = true; + mockCodeInterpreterSelected.mockReturnValue(true); + render(); + expect(screen.getByRole('button', { name: 'com_ui_mcp_programmatic_all' })).not.toHaveAttribute( + 'aria-disabled', + ); + }); + + test('bulk programmatic toggle can clear a legacy programmatic configuration', () => { + mockCapabilities.programmaticToolsEnabled = true; + mockAreAllToolsProgrammatic.mockReturnValue(true); + + render(); + + expect( + screen.getByRole('button', { name: 'com_ui_mcp_unprogrammatic_all' }), + ).not.toHaveAttribute('aria-disabled'); + }); + test('bulk intent skips programmatic-only tools (label can never reach them)', () => { mockCapabilities.toolIntentsEnabled = true; mockIsToolProgrammaticOnly.mockImplementation((toolId: string) => toolId === 'mcp:srv:a'); diff --git a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx index ee792c06f7..d03ed99630 100644 --- a/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ItemDialog/sections/McpSection.tsx @@ -3,6 +3,7 @@ import { Clock, Code2, Captions, Zap } from 'lucide-react'; import { useFormContext, useWatch } from 'react-hook-form'; import { Button, Spinner, Checkbox, Skeleton } from '@librechat/client'; import { + AgentCapabilities, Constants, splitMCPToolKey, normalizeServerName, @@ -83,11 +84,15 @@ export default function McpSection({ item }: Props) { const { mcpServersMap, mcpToolsLoading } = useAgentPanelContext(); const { agentsConfig } = useGetAgentsConfig(); const { + codeEnabled, deferredToolsEnabled, programmaticToolsEnabled, backgroundToolsEnabled, toolIntentsEnabled, } = useAgentCapabilities(agentsConfig?.capabilities); + const codeInterpreterSelected = useWatch({ control, name: AgentCapabilities.execute_code }); + const programmaticToolsAvailable = + codeEnabled && programmaticToolsEnabled && codeInterpreterSelected === true; const { isToolDeferred, isToolProgrammatic, @@ -274,6 +279,13 @@ export default function McpSection({ item }: Props) { const allSelected = hasTools && selectedTools.length === tools.length; const allDeferred = areAllToolsDeferred(tools); const allProgrammatic = areAllToolsProgrammatic(tools); + const programmaticBulkLabel = localize( + allProgrammatic ? 'com_ui_mcp_unprogrammatic_all' : 'com_ui_mcp_programmatic_all', + ); + const programmaticBulkTooltip = + programmaticToolsAvailable || allProgrammatic + ? programmaticBulkLabel + : localize('com_ui_mcp_programmatic_requires_code'); const allBackground = areAllToolsBackground(tools); /** Programmatic-only tools can never carry an intent label (the backend's * `canInjectIntentParam` skips non-direct tools), so both the bulk toggle @@ -450,12 +462,10 @@ export default function McpSection({ item }: Props) { icon={Code2} size="md" pressed={allProgrammatic} - label={localize( - allProgrammatic - ? 'com_ui_mcp_unprogrammatic_all' - : 'com_ui_mcp_programmatic_all', - )} + label={programmaticBulkLabel} activeClass="text-violet-600 dark:text-violet-500" + tooltip={programmaticBulkTooltip} + disabled={!programmaticToolsAvailable && !allProgrammatic} onToggle={() => toggleProgrammaticAll(tools)} /> )} @@ -533,6 +543,7 @@ export default function McpSection({ item }: Props) { intentDisabled={isToolProgrammaticOnly(tool.tool_id)} deferredToolsEnabled={deferredToolsEnabled} programmaticToolsEnabled={programmaticToolsEnabled} + programmaticToolsAvailable={programmaticToolsAvailable} backgroundToolsEnabled={backgroundToolsEnabled} toolIntentsEnabled={toolIntentsEnabled} onToggleSelect={() => toggleToolSelect(tool.tool_id)} diff --git a/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx b/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx index 9b95ec79e0..4f214167f9 100644 --- a/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ToolsMarketplaceDialog.tsx @@ -1,6 +1,7 @@ import { useState, useMemo, useCallback } from 'react'; import { Search } from 'lucide-react'; import { useFormContext } from 'react-hook-form'; +import { AgentCapabilities, removeCodeExecutionCaller } from 'librechat-data-provider'; import { Input, OGDialog, @@ -109,6 +110,11 @@ export default function ToolsMarketplaceDialog({ switch (patch.type) { case 'builtin': setValue(patch.field as keyof AgentForm, patch.value as never, { shouldDirty: true }); + if (patch.field === AgentCapabilities.execute_code && patch.value === false) { + setValue('tool_options', removeCodeExecutionCaller(getValues('tool_options')), { + shouldDirty: true, + }); + } break; case 'tool-add': { const current = (getValues('tools') ?? []) as string[]; diff --git a/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx b/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx index bf106b9bb4..882bb7a475 100644 --- a/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx +++ b/client/src/components/SidePanel/Agents/Tools/ToolsSection.tsx @@ -1,7 +1,12 @@ import { useState, useMemo, useCallback, useRef } from 'react'; import { Plus } from 'lucide-react'; import { useFormContext, useWatch } from 'react-hook-form'; -import { PermissionTypes, Permissions, AgentCapabilities } from 'librechat-data-provider'; +import { + PermissionTypes, + Permissions, + AgentCapabilities, + removeCodeExecutionCaller, +} from 'librechat-data-provider'; import { Label, Switch, @@ -153,6 +158,11 @@ export default function ToolsSection({ agentId }: Props) { switch (patch.type) { case 'builtin': setValue(patch.field as keyof AgentForm, patch.value as never, { shouldDirty: true }); + if (patch.field === AgentCapabilities.execute_code && patch.value === false) { + setValue('tool_options', removeCodeExecutionCaller(getValues('tool_options')), { + shouldDirty: true, + }); + } break; case 'tool-remove': { const current = (getValues('tools') ?? []) as string[]; diff --git a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx index 946c422343..80a257d2f3 100644 --- a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsMarketplaceDialog.spec.tsx @@ -3,8 +3,9 @@ import { fireEvent, render, screen } from '@testing-library/react'; import ToolsMarketplaceDialog from '../ToolsMarketplaceDialog'; const mockSetValue = jest.fn(); -const mockGetValues = jest.fn((): string[] => []); +const mockGetValues = jest.fn((_: string): unknown => []); let mockWatchedTools: string[] = []; +let mockExecuteCode = false; let mockMcpServersMap = new Map(); jest.mock('react-hook-form', () => ({ @@ -17,7 +18,7 @@ jest.mock('react-hook-form', () => ({ const map: Record = { tools: mockWatchedTools, skills: [], - execute_code: false, + execute_code: mockExecuteCode, web_search: false, file_search: false, artifacts: '', @@ -156,6 +157,7 @@ describe('ToolsMarketplaceDialog', () => { mockGetValues.mockClear(); mockGetValues.mockReturnValue([]); mockWatchedTools = []; + mockExecuteCode = false; mockMcpServersMap = new Map(); mockToggleFavorite.mockClear(); mockFavoriteKeys = new Set(); @@ -183,6 +185,31 @@ describe('ToolsMarketplaceDialog', () => { ); }); + test('disabling Code Interpreter clears programmatic MCP callers immediately', () => { + mockExecuteCode = true; + mockGetValues.mockImplementation((name: string) => + name === 'tool_options' + ? { + search: { allowed_callers: ['code_execution'], defer_loading: true }, + direct: { allowed_callers: ['direct'] }, + } + : [], + ); + + render(); + fireEvent.click(screen.getByRole('button', { name: /com_ui_run_code/ })); + + expect(mockSetValue).toHaveBeenCalledWith('execute_code', false, { shouldDirty: true }); + expect(mockSetValue).toHaveBeenCalledWith( + 'tool_options', + { + search: { defer_loading: true }, + direct: { allowed_callers: ['direct'] }, + }, + { shouldDirty: true }, + ); + }); + test('typing in search input filters the catalog', () => { render(); const input = screen.getByPlaceholderText('com_ui_tools_marketplace_search'); diff --git a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx index c9959ff2fc..38672bb5e4 100644 --- a/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx +++ b/client/src/components/SidePanel/Agents/Tools/__tests__/ToolsSection.spec.tsx @@ -198,6 +198,37 @@ describe('ToolsSection', () => { expect(screen.queryByTestId('item-dialog')).not.toBeInTheDocument(); expect(mockSetValue).toHaveBeenCalledWith('file_search', false, { shouldDirty: true }); }); + + test('clears programmatic MCP callers when Code Interpreter is removed', () => { + mockSelected = [ + { + kind: 'builtin', + id: 'execute_code', + name: 'Run Code', + description: '', + iconKey: 'execute_code', + }, + ]; + mockFormValues = { + tool_options: { + search: { allowed_callers: ['code_execution'], defer_loading: true }, + direct: { allowed_callers: ['direct'] }, + }, + }; + + render(); + fireEvent.click(screen.getByRole('button', { name: 'remove-execute_code' })); + + expect(mockSetValue).toHaveBeenCalledWith('execute_code', false, { shouldDirty: true }); + expect(mockSetValue).toHaveBeenCalledWith( + 'tool_options', + { + search: { defer_loading: true }, + direct: { allowed_callers: ['direct'] }, + }, + { shouldDirty: true }, + ); + }); }); describe('use all skills toggle', () => { diff --git a/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts b/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts index da3471824e..78f26921e2 100644 --- a/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts +++ b/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts @@ -85,6 +85,32 @@ describe('composeAgentUpdatePayload', () => { expect(payload.stateful_code_sessions).toBe(false); }); + it('removes programmatic callers when execute_code is disabled', () => { + const form = createForm(); + form.execute_code = false; + form.tool_options = { + search: { allowed_callers: ['code_execution'], defer_loading: true }, + }; + + const { payload } = composeAgentUpdatePayload(form, 'agent_123'); + + expect(payload.tool_options).toEqual({ search: { defer_loading: true } }); + }); + + it('preserves programmatic callers when execute_code is enabled', () => { + const form = createForm(); + form.execute_code = true; + form.tool_options = { + search: { allowed_callers: ['code_execution'] }, + }; + + const { payload } = composeAgentUpdatePayload(form, 'agent_123'); + + expect(payload.tool_options).toEqual({ + search: { allowed_callers: ['code_execution'] }, + }); + }); + it('preserves stateful_code_sessions when execute_code is enabled', () => { const form = createForm(); form.execute_code = true; diff --git a/client/src/components/SidePanel/Agents/__tests__/MCPToolItem.spec.tsx b/client/src/components/SidePanel/Agents/__tests__/MCPToolItem.spec.tsx index 1486374744..deedff10a7 100644 --- a/client/src/components/SidePanel/Agents/__tests__/MCPToolItem.spec.tsx +++ b/client/src/components/SidePanel/Agents/__tests__/MCPToolItem.spec.tsx @@ -35,6 +35,7 @@ function setup(overrides: Partial> = {} intentDisabled: false, deferredToolsEnabled: false, programmaticToolsEnabled: false, + programmaticToolsAvailable: false, backgroundToolsEnabled: false, toolIntentsEnabled: false, onToggleSelect: jest.fn(), @@ -108,12 +109,34 @@ describe('MCPToolItem', () => { }); test('programmatic is an inline button rendered only when enabled', () => { - const props = setup({ programmaticToolsEnabled: true }); + const props = setup({ programmaticToolsEnabled: true, programmaticToolsAvailable: true }); const programmaticButton = screen.getByRole('button', { name: 'com_ui_mcp_programmatic' }); fireEvent.click(programmaticButton); expect(props.onToggleProgrammatic).toHaveBeenCalledTimes(1); }); + test('programmatic is inert until Code Interpreter is selected', () => { + const props = setup({ programmaticToolsEnabled: true, programmaticToolsAvailable: false }); + const programmaticButton = screen.getByRole('button', { name: 'com_ui_mcp_programmatic' }); + + expect(programmaticButton).toHaveAttribute('aria-disabled', 'true'); + fireEvent.click(programmaticButton); + expect(props.onToggleProgrammatic).not.toHaveBeenCalled(); + }); + + test('an existing programmatic setting can be cleared after Code Interpreter is disabled', () => { + const props = setup({ + programmaticToolsEnabled: true, + programmaticToolsAvailable: false, + isProgrammatic: true, + }); + const programmaticButton = screen.getByRole('button', { name: 'com_ui_mcp_programmatic' }); + + expect(programmaticButton).not.toHaveAttribute('aria-disabled'); + fireEvent.click(programmaticButton); + expect(props.onToggleProgrammatic).toHaveBeenCalledTimes(1); + }); + test('background is an inline button rendered only when enabled', () => { const props = setup({ backgroundToolsEnabled: true }); const backgroundButton = screen.getByRole('button', { name: 'com_ui_mcp_background' }); diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index a9499bb3bc..f4fa831120 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -1505,6 +1505,7 @@ "com_ui_mcp_oauth_secret_reentry_required": "OAuth settings changed. Re-enter the client secret to save this MCP server.", "com_ui_mcp_oauth_timeout": "OAuth login timed out for {{0}}", "com_ui_mcp_programmatic": "Programmatic", + "com_ui_mcp_programmatic_requires_code": "Enable Code Interpreter before making MCP tools programmatic.", "com_ui_mcp_programmatic_all": "Mark all as programmatic", "com_ui_mcp_reauthentication_required": "MCP server '{{0}}' needs authentication. Reconnect to continue; if that fails, revoke its OAuth access and try again.", "com_ui_mcp_server": "MCP Server", diff --git a/packages/data-provider/src/agentToolOptions.spec.ts b/packages/data-provider/src/agentToolOptions.spec.ts new file mode 100644 index 0000000000..f0ce65814d --- /dev/null +++ b/packages/data-provider/src/agentToolOptions.spec.ts @@ -0,0 +1,38 @@ +import type { AgentToolOptions } from './types/assistants'; +import { removeCodeExecutionCaller } from './agentToolOptions'; + +describe('removeCodeExecutionCaller', () => { + it('removes a programmatic-only entry that has no other options', () => { + expect( + removeCodeExecutionCaller({ + search: { allowed_callers: ['code_execution'] }, + }), + ).toEqual({}); + }); + + it('preserves direct calling and unrelated options', () => { + expect( + removeCodeExecutionCaller({ + search: { + allowed_callers: ['direct', 'code_execution'], + defer_loading: true, + }, + }), + ).toEqual({ + search: { + allowed_callers: ['direct'], + defer_loading: true, + }, + }); + }); + + it('does not mutate its input', () => { + const input: AgentToolOptions = { + search: { allowed_callers: ['code_execution'], run_in_background: true }, + }; + + removeCodeExecutionCaller(input); + + expect(input.search.allowed_callers).toEqual(['code_execution']); + }); +}); diff --git a/packages/data-provider/src/agentToolOptions.ts b/packages/data-provider/src/agentToolOptions.ts new file mode 100644 index 0000000000..a611b49704 --- /dev/null +++ b/packages/data-provider/src/agentToolOptions.ts @@ -0,0 +1,36 @@ +import type { AgentToolOptions, AllowedCaller } from './types/assistants'; + +/** + * Removes Code Interpreter as an allowed caller without mutating the input. + * Tool entries and unrelated options are preserved; an empty entry is removed. + */ +export function removeCodeExecutionCaller( + toolOptions: AgentToolOptions | undefined, +): AgentToolOptions | undefined { + if (toolOptions == null) { + return toolOptions; + } + + const normalized: AgentToolOptions = {}; + for (const [toolName, options] of Object.entries(toolOptions)) { + const callers = options.allowed_callers; + if (callers?.includes('code_execution') !== true) { + normalized[toolName] = options; + continue; + } + + const allowedCallers = callers.filter( + (caller): caller is AllowedCaller => caller !== 'code_execution', + ); + const { allowed_callers: _removed, ...remainingOptions } = options; + const nextOptions = + allowedCallers.length > 0 + ? { ...remainingOptions, allowed_callers: allowedCallers } + : remainingOptions; + if (Object.keys(nextOptions).length > 0) { + normalized[toolName] = nextOptions; + } + } + + return normalized; +} diff --git a/packages/data-provider/src/index.ts b/packages/data-provider/src/index.ts index 73766b315f..8c98967cf3 100644 --- a/packages/data-provider/src/index.ts +++ b/packages/data-provider/src/index.ts @@ -58,5 +58,6 @@ export { default as createPayload } from './createPayload'; /* feedback */ export * from './feedback'; export * from './parameterSettings'; +export * from './agentToolOptions'; /* code-execution sandbox */ export * from './codeEnvRef';