diff --git a/.env.example b/.env.example index 7779276bcc..07918a8652 100644 --- a/.env.example +++ b/.env.example @@ -810,13 +810,6 @@ HELP_AND_FAQ_URL=https://librechat.ai #=====================================================# OPENWEATHER_API_KEY= -#====================================# -# LibreChat Code Interpreter API # -#====================================# - -# https://code.librechat.ai -# LIBRECHAT_CODE_API_KEY=your-key - #======================# # Web Search # #======================# diff --git a/api/app/clients/tools/util/handleTools.js b/api/app/clients/tools/util/handleTools.js index 8adb43f945..1c5e03340c 100644 --- a/api/app/clients/tools/util/handleTools.js +++ b/api/app/clients/tools/util/handleTools.js @@ -1,10 +1,5 @@ const { logger } = require('@librechat/data-schemas'); -const { - EnvVar, - Calculator, - createSearchTool, - createCodeExecutionTool, -} = require('@librechat/agents'); +const { Calculator, createSearchTool, createCodeExecutionTool } = require('@librechat/agents'); const { checkAccess, toolkitParent, @@ -265,28 +260,14 @@ const loadTools = async ({ for (const tool of tools) { if (tool === Tools.execute_code) { requestedTools[tool] = async () => { - const authValues = await loadAuthValues({ - userId: user, - authFields: [EnvVar.CODE_API_KEY], + const { files, toolContext } = await primeCodeFiles({ + ...options, + agentId: agent?.id, }); - const codeApiKey = authValues[EnvVar.CODE_API_KEY]; - const { files, toolContext } = await primeCodeFiles( - { - ...options, - agentId: agent?.id, - }, - codeApiKey, - ); if (toolContext) { toolContextMap[tool] = toolContext; } - const CodeExecutionTool = createCodeExecutionTool({ - user_id: user, - files, - ...authValues, - }); - CodeExecutionTool.apiKey = codeApiKey; - return CodeExecutionTool; + return createCodeExecutionTool({ user_id: user, files }); }; continue; } else if (tool === Tools.file_search) { diff --git a/api/package.json b/api/package.json index dad4f545a2..eafb9dbd45 100644 --- a/api/package.json +++ b/api/package.json @@ -44,7 +44,7 @@ "@google/genai": "^1.19.0", "@keyv/redis": "^4.3.3", "@langchain/core": "^0.3.80", - "@librechat/agents": "^3.1.68-dev.0", + "@librechat/agents": "^3.1.68-dev.1", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", diff --git a/api/server/controllers/__tests__/tools.verifyToolAuth.spec.js b/api/server/controllers/__tests__/tools.verifyToolAuth.spec.js new file mode 100644 index 0000000000..03965021c4 --- /dev/null +++ b/api/server/controllers/__tests__/tools.verifyToolAuth.spec.js @@ -0,0 +1,102 @@ +jest.mock('@librechat/data-schemas', () => ({ + logger: { debug: jest.fn(), error: jest.fn(), warn: jest.fn() }, +})); + +jest.mock('@librechat/api', () => ({ + checkAccess: jest.fn(), + loadWebSearchAuth: jest.fn(), +})); + +jest.mock('~/models', () => ({ + getRoleByName: jest.fn(), + createToolCall: jest.fn(), + getToolCallsByConvo: jest.fn(), + getMessage: jest.fn(), +})); + +jest.mock('~/server/services/Files/process', () => ({ + processFileURL: jest.fn(), + uploadImageBuffer: jest.fn(), +})); + +jest.mock('~/server/services/Files/Code/process', () => ({ + processCodeOutput: jest.fn(), +})); + +jest.mock('~/server/services/Tools/credentials', () => ({ + loadAuthValues: jest.fn(), +})); + +jest.mock('~/app/clients/tools/util', () => ({ + loadTools: jest.fn(), +})); + +const { Tools, AuthType } = require('librechat-data-provider'); +const { verifyToolAuth } = require('../tools'); + +/** + * Phase 8 behavioral pin: `verifyToolAuth(execute_code)` unconditionally + * returns system-authenticated. Sandbox auth moved server-side into the + * agents library, so the per-user `CODE_API_KEY` check that previously + * gated this endpoint is gone. The deployment contract is: if the + * admin enabled the `execute_code` capability, the sandbox is + * reachable. This endpoint does not probe reachability (would be too + * expensive per UI-gate query); failures surface at execution time. + * + * A regression where someone re-adds an auth check here would + * resurrect the per-user key-entry dialog on the client, which Phase 8 + * explicitly removed. Pin the contract. + */ +describe('verifyToolAuth — execute_code system-auth contract', () => { + const makeReq = (toolId) => ({ + params: { toolId }, + user: { id: 'user-1' }, + config: {}, + }); + + const makeRes = () => { + const res = {}; + res.status = jest.fn().mockReturnValue(res); + res.json = jest.fn().mockReturnValue(res); + return res; + }; + + it('returns authenticated: true with SYSTEM_DEFINED for execute_code', async () => { + const res = makeRes(); + await verifyToolAuth(makeReq(Tools.execute_code), res); + + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ + authenticated: true, + message: AuthType.SYSTEM_DEFINED, + }); + }); + + it('returns 404 for unknown tool ids (not in directCallableTools)', async () => { + const res = makeRes(); + await verifyToolAuth(makeReq('not_a_real_tool'), res); + + expect(res.status).toHaveBeenCalledWith(404); + expect(res.json).toHaveBeenCalledWith({ message: 'Tool not found' }); + }); + + it('does NOT invoke loadAuthValues for execute_code (no per-user credential check)', async () => { + /* Regression guard: a future refactor that threads per-user auth back + in would resurface the key-entry dialog on the client. Pin that + the auth path is never consulted. */ + const { loadAuthValues } = require('~/server/services/Tools/credentials'); + loadAuthValues.mockClear(); + + await verifyToolAuth(makeReq(Tools.execute_code), makeRes()); + + expect(loadAuthValues).not.toHaveBeenCalled(); + }); + + it('does NOT reference AuthType.USER_PROVIDED in the response (Phase 8 removed the path)', async () => { + const res = makeRes(); + await verifyToolAuth(makeReq(Tools.execute_code), res); + + const payload = res.json.mock.calls[0][0]; + expect(payload.message).not.toBe(AuthType.USER_PROVIDED); + }); +}); diff --git a/api/server/controllers/agents/__tests__/client.memory.spec.js b/api/server/controllers/agents/__tests__/client.memory.spec.js new file mode 100644 index 0000000000..de8eeb2153 --- /dev/null +++ b/api/server/controllers/agents/__tests__/client.memory.spec.js @@ -0,0 +1,74 @@ +const { EModelEndpoint, AgentCapabilities } = require('librechat-data-provider'); + +/** + * Pins the capability-flag derivation that `AgentClient::useMemory` uses when + * it calls `initializeAgent` for the memory-extraction agent. The expression + * is trivial but lives in a controller path that's otherwise hard to unit- + * test, so a focused regression guard at the pure-logic layer ensures any + * drift in config-key names (`agents`, `capabilities`) or capability enum + * values (`execute_code`) surfaces here instead of silently stripping + * `bash_tool` + `read_file` from memory agents in production. + * + * The expression mirrored below is the one in + * `api/server/controllers/agents/client.js::useMemory`: + * + * new Set(appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities) + * .has(AgentCapabilities.execute_code) + */ +function deriveMemoryCodeEnvAvailable(appConfig) { + return new Set(appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities).has( + AgentCapabilities.execute_code, + ); +} + +describe('AgentClient::useMemory — codeEnvAvailable derivation', () => { + it('returns true when appConfig lists execute_code under the agents endpoint capabilities', () => { + expect( + deriveMemoryCodeEnvAvailable({ + endpoints: { + [EModelEndpoint.agents]: { + capabilities: [AgentCapabilities.execute_code, AgentCapabilities.file_search], + }, + }, + }), + ).toBe(true); + }); + + it('returns false when the agents endpoint omits execute_code', () => { + expect( + deriveMemoryCodeEnvAvailable({ + endpoints: { + [EModelEndpoint.agents]: { + capabilities: [AgentCapabilities.file_search, AgentCapabilities.web_search], + }, + }, + }), + ).toBe(false); + }); + + it('returns false when the capabilities array is absent', () => { + expect(deriveMemoryCodeEnvAvailable({ endpoints: { [EModelEndpoint.agents]: {} } })).toBe( + false, + ); + }); + + it('returns false when the agents endpoint config is absent', () => { + expect(deriveMemoryCodeEnvAvailable({ endpoints: {} })).toBe(false); + }); + + it('returns false when appConfig is null / undefined', () => { + /* Defensive — `req.config` can be unset in edge-case test harnesses and + ephemeral-agent flows; the memory path must not throw on access. */ + expect(deriveMemoryCodeEnvAvailable(null)).toBe(false); + expect(deriveMemoryCodeEnvAvailable(undefined)).toBe(false); + }); + + it('matches the literal string "execute_code" — catches enum rename drift', () => { + /* Pins the capability enum value so a rename of `AgentCapabilities.execute_code` + that doesn't propagate to the controllers surfaces here. If this test breaks, + update the underlying expression in `useMemory` and the helpers in + `initialize.js` / `openai.js` / `responses.js` to match. */ + expect(AgentCapabilities.execute_code).toBe('execute_code'); + expect(EModelEndpoint.agents).toBe('agents'); + }); +}); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 40483ebdc3..160df4ea95 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -2,8 +2,6 @@ const { nanoid } = require('nanoid'); const { logger } = require('@librechat/data-schemas'); const { Tools, StepTypes, FileContext, ErrorTypes } = require('librechat-data-provider'); const { - EnvVar, - Constants, GraphEvents, GraphNodeKeys, ToolEndHandler, @@ -17,7 +15,6 @@ const { } = require('@librechat/api'); const { processFileCitations } = require('~/server/services/Files/Citations'); const { processCodeOutput } = require('~/server/services/Files/Code/process'); -const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { saveBase64Image } = require('~/server/services/Files/process'); class ModelEndHandler { @@ -456,15 +453,10 @@ function createToolEndCallback({ req, res, artifactPromises, streamId = null }) const { id, name } = file; artifactPromises.push( (async () => { - const result = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); const fileMetadata = await processCodeOutput({ req, id, name, - apiKey: result[EnvVar.CODE_API_KEY], messageId: metadata.run_id, toolCallId: output.tool_call_id, conversationId: metadata.thread_id, @@ -662,15 +654,10 @@ function createResponsesToolEndCallback({ req, res, tracker, artifactPromises }) const { id, name } = file; artifactPromises.push( (async () => { - const result = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); const fileMetadata = await processCodeOutput({ req, id, name, - apiKey: result[EnvVar.CODE_API_KEY], messageId: metadata.run_id, toolCallId: output.tool_call_id, conversationId: metadata.thread_id, diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 3669933cae..cf48a8d87b 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -47,6 +47,7 @@ const { ContentTypes, EModelEndpoint, PermissionTypes, + AgentCapabilities, isAgentsEndpoint, isEphemeralAgentId, removeNullishValues, @@ -489,6 +490,13 @@ class AgentClient extends BaseClient { return; } + /** Forward the same `execute_code` capability gate the chat flow uses — + * memory agents are unlikely to list `execute_code`, but if one does, + * Phase 8 relies on this flag to expand the string into + * `bash_tool` + `read_file` (pre-Phase 8 the legacy `execute_code` + * tool registered unconditionally; without this passthrough the + * memory path would silently lose code-execution tooling). */ + const memoryCapabilities = new Set(appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities); const agent = await initializeAgent( { req: this.options.req, @@ -500,6 +508,7 @@ class AgentClient extends BaseClient { ? EModelEndpoint.agents : memoryConfig.agent?.provider, }, + codeEnvAvailable: memoryCapabilities.has(AgentCapabilities.execute_code), }, { getFiles: db.getFiles, diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index d26be8117e..9d98600ef7 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -314,6 +314,7 @@ const OpenAIChatCompletionController = async (req, res) => { userMCPAuthMap: primaryConfig.userMCPAuthMap, tool_resources: primaryConfig.tool_resources, actionsEnabled: primaryConfig.actionsEnabled, + codeEnvAvailable: primaryConfig.codeEnvAvailable, }); // Only run BFS discovery (and pay `getModelsConfig` upfront) when the @@ -343,6 +344,8 @@ const OpenAIChatCompletionController = async (req, res) => { // sub-agent must clear the same sharing boundary, not the looser // in-app AGENT one. resourceType: ResourceType.REMOTE_AGENT, + /** @see DiscoverConnectedAgentsParams.codeEnvAvailable */ + codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), }, { getAgent: db.getAgent, @@ -369,6 +372,7 @@ const OpenAIChatCompletionController = async (req, res) => { userMCPAuthMap: config.userMCPAuthMap, tool_resources: config.tool_resources, actionsEnabled: config.actionsEnabled, + codeEnvAvailable: config.codeEnvAvailable, }); }, initializeAgent, @@ -414,11 +418,13 @@ const OpenAIChatCompletionController = async (req, res) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId: null }); - /* Stable for the turn: the capability set is the admin config, and - the prime lists are fixed once `initializeAgent` resolves. Hoisting - these out of `loadTools` avoids recomputing them on every tool - execution (and keeps the call-site lean). */ - const codeEnvAvailable = enabledCapabilities.has(AgentCapabilities.execute_code); + /* Stable for the turn: the prime lists are fixed once + `initializeAgent` resolves. Hoisted out of `loadTools` so tool + execution doesn't recompute them. `codeEnvAvailable` is read + per-agent from the stored tool context (admin cap AND that + agent's `tools` list includes `execute_code`) — a skills-only + agent never gains sandbox access even if the admin enabled the + capability globally. */ const skillPrimedIdsByName = buildSkillPrimedIdsByName( primaryConfig.manualSkillPrimes, primaryConfig.alwaysApplySkillPrimes, @@ -442,7 +448,7 @@ const OpenAIChatCompletionController = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - codeEnvAvailable, + ctx.codeEnvAvailable === true, skillPrimedIdsByName, ); }, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 49d56e01e8..08ca17bde5 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -444,6 +444,7 @@ const createResponse = async (req, res) => { userMCPAuthMap: primaryConfig.userMCPAuthMap, tool_resources: primaryConfig.tool_resources, actionsEnabled: primaryConfig.actionsEnabled, + codeEnvAvailable: primaryConfig.codeEnvAvailable, }); // Only run BFS discovery (and pay `getModelsConfig` upfront) when the @@ -473,6 +474,8 @@ const createResponse = async (req, res) => { // sub-agent must clear the same sharing boundary, not the looser // in-app AGENT one. resourceType: ResourceType.REMOTE_AGENT, + /** @see DiscoverConnectedAgentsParams.codeEnvAvailable */ + codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), }, { getAgent: db.getAgent, @@ -499,6 +502,7 @@ const createResponse = async (req, res) => { userMCPAuthMap: config.userMCPAuthMap, tool_resources: config.tool_resources, actionsEnabled: config.actionsEnabled, + codeEnvAvailable: config.codeEnvAvailable, }); }, initializeAgent, @@ -567,11 +571,14 @@ const createResponse = async (req, res) => { } } - /* Stable for the turn: the capability set is the admin config, and - the prime lists are fixed once `initializeAgent` resolves. Hoisted - here so both the streaming and non-streaming `loadTools` closures - below read the same values without recomputing per tool execution. */ - const codeEnvAvailable = enabledCapabilities.has(AgentCapabilities.execute_code); + /* Stable for the turn: the prime lists are fixed once + `initializeAgent` resolves. Hoisted here so both the streaming + and non-streaming `loadTools` closures below reuse it without + recomputing per tool execution. `codeEnvAvailable` is read + per-agent from the stored tool context (admin cap AND that + agent's `tools` list includes `execute_code`) — a skills-only + agent never gains sandbox access even if the admin enabled the + capability globally. */ const skillPrimedIdsByName = buildSkillPrimedIdsByName( manualSkillPrimes, alwaysApplySkillPrimes, @@ -634,7 +641,7 @@ const createResponse = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - codeEnvAvailable, + ctx.codeEnvAvailable === true, skillPrimedIdsByName, ); }, @@ -810,7 +817,7 @@ const createResponse = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - codeEnvAvailable, + ctx.codeEnvAvailable === true, skillPrimedIdsByName, ); }, diff --git a/api/server/controllers/tools.js b/api/server/controllers/tools.js index 1df11b1059..c173e2981e 100644 --- a/api/server/controllers/tools.js +++ b/api/server/controllers/tools.js @@ -1,5 +1,4 @@ const { nanoid } = require('nanoid'); -const { EnvVar } = require('@librechat/agents'); const { logger } = require('@librechat/data-schemas'); const { checkAccess, loadWebSearchAuth } = require('@librechat/api'); const { @@ -15,9 +14,12 @@ const { processCodeOutput } = require('~/server/services/Files/Code/process'); const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { loadTools } = require('~/app/clients/tools/util'); -const fieldsMap = { - [Tools.execute_code]: [EnvVar.CODE_API_KEY], -}; +/** + * Tools that are callable directly via `POST /tools/:toolId/call`. + * `execute_code` is the only entry today; the tool runs server-side via + * the agents library / sandbox service without any per-user credential. + */ +const directCallableTools = new Set([Tools.execute_code]); const toolAccessPermType = { [Tools.execute_code]: PermissionTypes.RUN_CODE, @@ -65,37 +67,23 @@ const verifyToolAuth = async (req, res) => { if (toolId === Tools.web_search) { return await verifyWebSearchAuth(req, res); } - const authFields = fieldsMap[toolId]; - if (!authFields) { + if (!directCallableTools.has(toolId)) { res.status(404).json({ message: 'Tool not found' }); return; } - let result; - try { - result = await loadAuthValues({ - userId: req.user.id, - authFields, - throwError: false, - }); - } catch (error) { - logger.error('Error loading auth values', error); - res.status(200).json({ authenticated: false, message: AuthType.USER_PROVIDED }); - return; - } - let isUserProvided = false; - for (const field of authFields) { - if (!result[field]) { - res.status(200).json({ authenticated: false, message: AuthType.USER_PROVIDED }); - return; - } - if (!isUserProvided && process.env[field] !== result[field]) { - isUserProvided = true; - } - } - res.status(200).json({ - authenticated: true, - message: isUserProvided ? AuthType.USER_PROVIDED : AuthType.SYSTEM_DEFINED, - }); + /** + * `execute_code` no longer requires a per-user credential — sandbox + * auth is handled server-side by the agents library. Always report + * system-authenticated so the client proceeds straight to the call + * without a key-entry dialog. + * + * Deployment contract: reachability of the sandbox service is the + * admin's responsibility. This endpoint does not probe the service + * (a per-auth-check network hop would be too expensive for what is + * a UI-gate query). If the sandbox is unreachable, the call path + * surfaces the error at execution time instead of here. + */ + res.status(200).json({ authenticated: true, message: AuthType.SYSTEM_DEFINED }); } catch (error) { res.status(500).json({ message: error.message }); } @@ -111,7 +99,7 @@ const callTool = async (req, res) => { try { const appConfig = req.config; const { toolId = '' } = req.params; - if (!fieldsMap[toolId]) { + if (!directCallableTools.has(toolId)) { logger.warn(`[${toolId}/call] User ${req.user.id} attempted call to invalid tool`); res.status(404).json({ message: 'Tool not found' }); return; @@ -199,7 +187,6 @@ const callTool = async (req, res) => { req, id, name, - apiKey: tool.apiKey, messageId, toolCallId, conversationId, diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index eb13ecdc31..5c26f65b81 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -1,6 +1,5 @@ const fs = require('fs').promises; const express = require('express'); -const { EnvVar } = require('@librechat/agents'); const { logger, SystemCapabilities } = require('@librechat/data-schemas'); const { refreshS3FileUrls, @@ -29,7 +28,6 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { getOpenAIClient } = require('~/server/controllers/assistants/helpers'); const { hasCapability } = require('~/server/middleware/roles/capabilities'); const { checkPermission } = require('~/server/services/PermissionService'); -const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { hasAccessToFilesViaAgent } = require('~/server/services/Files'); const { cleanFileName } = require('~/server/utils/files'); const { getLogStores } = require('~/cache'); @@ -287,13 +285,8 @@ router.get('/code/download/:session_id/:fileId', async (req, res) => { return res.status(501).send('Not Implemented'); } - const result = await loadAuthValues({ userId: req.user.id, authFields: [EnvVar.CODE_API_KEY] }); - /** @type {AxiosResponse | undefined} */ - const response = await getDownloadStream( - `${session_id}/${fileId}`, - result[EnvVar.CODE_API_KEY], - ); + const response = await getDownloadStream(`${session_id}/${fileId}`); res.set(response.headers); response.data.pipe(res); } catch (error) { diff --git a/api/server/services/Endpoints/agents/addedConvo.js b/api/server/services/Endpoints/agents/addedConvo.js index 7561053f8f..2a2cd9ca30 100644 --- a/api/server/services/Endpoints/agents/addedConvo.js +++ b/api/server/services/Endpoints/agents/addedConvo.js @@ -40,6 +40,9 @@ const loadAddedAgent = (params) => * @param {Map} params.agentConfigs - Map of agent configs to add to * @param {string} params.primaryAgentId - The primary agent ID * @param {Object|undefined} params.userMCPAuthMap - User MCP auth map to merge into + * @param {boolean} [params.codeEnvAvailable] - `execute_code` capability flag; + * forwarded verbatim to the added agent's `initializeAgent`. @see + * InitializeAgentParams.codeEnvAvailable for full semantics. * @returns {Promise<{userMCPAuthMap: Object|undefined}>} The updated userMCPAuthMap */ const processAddedConvo = async ({ @@ -57,6 +60,7 @@ const processAddedConvo = async ({ primaryAgentId, primaryAgent, userMCPAuthMap, + codeEnvAvailable, }) => { const addedConvo = endpointOption.addedConvo; if (addedConvo == null) { @@ -101,6 +105,7 @@ const processAddedConvo = async ({ agent: addedAgent, endpointOption, allowedProviders, + codeEnvAvailable, }, { getFiles: db.getFiles, diff --git a/api/server/services/Endpoints/agents/addedConvo.spec.js b/api/server/services/Endpoints/agents/addedConvo.spec.js new file mode 100644 index 0000000000..b5c9427690 --- /dev/null +++ b/api/server/services/Endpoints/agents/addedConvo.spec.js @@ -0,0 +1,108 @@ +const mockInitializeAgent = jest.fn(); +const mockValidateAgentModel = jest.fn(); +const mockLoadAddedAgent = jest.fn(); +const mockGetAgent = jest.fn(); +const mockGetMCPServerTools = jest.fn(); + +jest.mock('@librechat/data-schemas', () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +jest.mock('@librechat/api', () => ({ + ADDED_AGENT_ID: '__added_agent__', + initializeAgent: (...args) => mockInitializeAgent(...args), + validateAgentModel: (...args) => mockValidateAgentModel(...args), + loadAddedAgent: (params) => mockLoadAddedAgent(params), +})); + +jest.mock('~/server/services/Files/permissions', () => ({ + filterFilesByAgentAccess: jest.fn(), +})); + +jest.mock('~/server/services/Config', () => ({ + getMCPServerTools: (...args) => mockGetMCPServerTools(...args), +})); + +jest.mock('~/models', () => ({ + getAgent: (...args) => mockGetAgent(...args), +})); + +const { processAddedConvo } = require('./addedConvo'); + +const makeReq = () => ({ user: { id: 'u1', role: 'USER' } }); + +/** + * Phase 8 pins `processAddedConvo` forwarding the run's `codeEnvAvailable` to + * the added-convo `initializeAgent` call. Without this, parallel multi-convo + * agents with `tools: ['execute_code']` silently drop `bash_tool` + `read_file` + * even though the primary had them — pre-Phase-8 the legacy + * `CodeExecutionToolDefinition` landed in their `toolDefinitions` via the + * registry regardless of any explicit flag. + */ +describe('processAddedConvo — codeEnvAvailable passthrough', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockValidateAgentModel.mockResolvedValue({ isValid: true }); + mockInitializeAgent.mockResolvedValue({ + id: 'added-agent', + userMCPAuthMap: undefined, + }); + mockLoadAddedAgent.mockResolvedValue({ id: 'added-agent', provider: 'openai' }); + }); + + const baseParams = (overrides = {}) => ({ + req: makeReq(), + res: {}, + endpointOption: { addedConvo: { model: 'gpt-4o', agent_id: 'added-agent' } }, + modelsConfig: { openai: ['gpt-4o'] }, + logViolation: jest.fn(), + loadTools: jest.fn(), + requestFiles: [], + conversationId: 'conv-1', + parentMessageId: null, + allowedProviders: new Set(['openai']), + agentConfigs: new Map(), + primaryAgentId: 'primary-id', + primaryAgent: { id: 'primary-id' }, + userMCPAuthMap: undefined, + ...overrides, + }); + + it('forwards codeEnvAvailable=true to the added-convo initializeAgent call', async () => { + await processAddedConvo(baseParams({ codeEnvAvailable: true })); + + expect(mockInitializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ codeEnvAvailable: true }), + expect.anything(), + ); + }); + + it('forwards codeEnvAvailable=false verbatim (not coerced to undefined)', async () => { + /* Symmetric coverage: if the runtime gate is off for the primary, the + parallel agent must not accidentally re-enable code execution via a + defaulting bug in the destructuring. */ + await processAddedConvo(baseParams({ codeEnvAvailable: false })); + + expect(mockInitializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ codeEnvAvailable: false }), + expect.anything(), + ); + }); + + it('forwards codeEnvAvailable=undefined when caller omits it (no silent default)', async () => { + /* Backstop for the "caller didn't update after Phase 8" case — the + added-convo path must not invent a truthy value out of thin air. + Matches `initializeAgent`'s own "explicit opt-in" semantics. */ + await processAddedConvo(baseParams()); + + expect(mockInitializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ codeEnvAvailable: undefined }), + expect.anything(), + ); + }); +}); diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index bfd35320fb..3baf6b8554 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -172,11 +172,16 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { }); logger.debug(`[ON_TOOL_EXECUTE] loaded ${result.loadedTools?.length ?? 0} tools`); + /** Per-agent narrowed flag (admin capability AND agent.tools + * includes execute_code), captured in `agentToolContexts` when + * the agent initialized. Falls back to `false` on any stray + * ctx miss so a skills-only agent never gains sandbox access + * even if capability lookup somehow skips. */ return enrichWithSkillConfigurable( result, req, ctx.accessibleSkillIds, - codeEnvAvailable, + ctx.codeEnvAvailable === true, ctx.skillPrimedIdsByName, ); }, @@ -299,6 +304,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { tool_resources: primaryConfig.tool_resources, actionsEnabled: primaryConfig.actionsEnabled, accessibleSkillIds: primaryConfig.accessibleSkillIds, + codeEnvAvailable: primaryConfig.codeEnvAvailable, skillPrimedIdsByName, }); @@ -323,6 +329,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { scopeSkillIds(accessibleSkillIds, ephemeralSkillsToggle ? undefined : agent.skills), skillStates, defaultActiveOnShare, + codeEnvAvailable, }, { getAgent: db.getAgent, @@ -364,6 +371,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { tool_resources: config.tool_resources, actionsEnabled: config.actionsEnabled, accessibleSkillIds: config.accessibleSkillIds, + codeEnvAvailable: config.codeEnvAvailable, skillPrimedIdsByName: buildSkillPrimedIdsByName( config.manualSkillPrimes, config.alwaysApplySkillPrimes, @@ -404,6 +412,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { parentMessageId, allowedProviders, primaryAgentId: primaryConfig.id, + codeEnvAvailable, }); if (updatedMCPAuthMap) { @@ -421,6 +430,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { tool_resources: config.tool_resources, actionsEnabled: config.actionsEnabled, accessibleSkillIds: config.accessibleSkillIds, + codeEnvAvailable: config.codeEnvAvailable, }); } @@ -452,22 +462,18 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { modelLabel: endpointOption.model_parameters.modelLabel, }); - /** primeInvokedSkills reconstructs bodies of skills invoked in prior turns so - * formatAgentMessages can rebuild HumanMessages and re-prime code-env files. - * Unlike catalog injection and runtime invocation (both scoped per-agent), - * history priming must use the user's full ACL-accessible set: historical - * skill calls can reference skills no longer in any active agent's scope - * (agent.skills edited, ephemeral toggle flipped), and scoping those out - * would drop prior skill context and break file references in follow-up - * turns. The ACL check remains the security gate; handleSkillToolCall is - * where per-agent scoping prevents NEW invocations. */ + /** History priming uses the user's full ACL-accessible skill set (not + * per-agent scoped) because prior turns may reference skills no longer + * in any active agent's scope; the ACL check is the security gate. + * `codeEnvAvailable` comes from `primaryConfig` — @see + * `InitializedAgent.codeEnvAvailable` for the per-agent narrowing. */ const handlePrimeInvokedSkills = skillsCapabilityEnabled ? (payload) => primeInvokedSkills({ req, payload, accessibleSkillIds, - codeEnvAvailable, + codeEnvAvailable: primaryConfig.codeEnvAvailable === true, ...getSkillToolDeps(), }) : undefined; diff --git a/api/server/services/Files/Code/crud.js b/api/server/services/Files/Code/crud.js index 5bf028702a..8130f4f095 100644 --- a/api/server/services/Files/Code/crud.js +++ b/api/server/services/Files/Code/crud.js @@ -15,11 +15,10 @@ const MAX_FILE_SIZE = 150 * 1024 * 1024; /** * Retrieves a download stream for a specified file. * @param {string} fileIdentifier - The identifier for the file (e.g., "session_id/fileId"). - * @param {string} apiKey - The API key for authentication. * @returns {Promise} A promise that resolves to a readable stream of the file content. * @throws {Error} If there's an error during the download process. */ -async function getCodeOutputDownloadStream(fileIdentifier, apiKey) { +async function getCodeOutputDownloadStream(fileIdentifier) { try { const baseURL = getCodeBaseURL(); /** @type {import('axios').AxiosRequestConfig} */ @@ -29,7 +28,6 @@ async function getCodeOutputDownloadStream(fileIdentifier, apiKey) { responseType: 'stream', headers: { 'User-Agent': 'LibreChat/1.0', - 'X-API-Key': apiKey, }, httpAgent: codeServerHttpAgent, httpsAgent: codeServerHttpsAgent, @@ -54,12 +52,11 @@ async function getCodeOutputDownloadStream(fileIdentifier, apiKey) { * @param {ServerRequest} params.req - The request object from Express. It should have a `user` property with an `id` representing the user * @param {import('fs').ReadStream | import('stream').Readable} params.stream - The read stream for the file. * @param {string} params.filename - The name of the file. - * @param {string} params.apiKey - The API key for authentication. * @param {string} [params.entity_id] - Optional entity ID for the file. * @returns {Promise} * @throws {Error} If there's an error during the upload process. */ -async function uploadCodeEnvFile({ req, stream, filename, apiKey, entity_id = '' }) { +async function uploadCodeEnvFile({ req, stream, filename, entity_id = '' }) { try { const form = new FormData(); if (entity_id.length > 0) { @@ -75,7 +72,6 @@ async function uploadCodeEnvFile({ req, stream, filename, apiKey, entity_id = '' 'Content-Type': 'multipart/form-data', 'User-Agent': 'LibreChat/1.0', 'User-Id': req.user.id, - 'X-API-Key': apiKey, }, httpAgent: codeServerHttpAgent, httpsAgent: codeServerHttpsAgent, @@ -115,12 +111,11 @@ async function uploadCodeEnvFile({ req, stream, filename, apiKey, entity_id = '' * @param {object} params * @param {import('express').Request & { user: { id: string } }} params.req - The request object. * @param {Array<{ stream: NodeJS.ReadableStream; filename: string }>} params.files - Files to upload. - * @param {string} params.apiKey - The API key for authentication. * @param {string} [params.entity_id] - Optional entity ID. * @returns {Promise<{ session_id: string; files: Array<{ fileId: string; filename: string }> }>} * @throws {Error} If the batch upload fails entirely. */ -async function batchUploadCodeEnvFiles({ req, files, apiKey, entity_id = '' }) { +async function batchUploadCodeEnvFiles({ req, files, entity_id = '' }) { try { const form = new FormData(); if (entity_id.length > 0) { @@ -138,7 +133,6 @@ async function batchUploadCodeEnvFiles({ req, files, apiKey, entity_id = '' }) { 'Content-Type': 'multipart/form-data', 'User-Agent': 'LibreChat/1.0', 'User-Id': req.user.id, - 'X-API-Key': apiKey, }, httpAgent: codeServerHttpAgent, httpsAgent: codeServerHttpsAgent, diff --git a/api/server/services/Files/Code/crud.spec.js b/api/server/services/Files/Code/crud.spec.js index 261f0f052b..aa2fd8e3e7 100644 --- a/api/server/services/Files/Code/crud.spec.js +++ b/api/server/services/Files/Code/crud.spec.js @@ -33,7 +33,7 @@ describe('Code CRUD', () => { const mockResponse = { data: Readable.from(['chunk']) }; mockAxios.mockResolvedValue(mockResponse); - await getCodeOutputDownloadStream('session-1/file-1', 'test-key'); + await getCodeOutputDownloadStream('session-1/file-1'); const callConfig = mockAxios.mock.calls[0][0]; expect(callConfig.httpAgent).toBe(codeServerHttpAgent); @@ -47,19 +47,18 @@ describe('Code CRUD', () => { it('should request stream response from the correct URL', async () => { mockAxios.mockResolvedValue({ data: Readable.from(['chunk']) }); - await getCodeOutputDownloadStream('session-1/file-1', 'test-key'); + await getCodeOutputDownloadStream('session-1/file-1'); const callConfig = mockAxios.mock.calls[0][0]; expect(callConfig.url).toBe('https://code-api.example.com/download/session-1/file-1'); expect(callConfig.responseType).toBe('stream'); expect(callConfig.timeout).toBe(15000); - expect(callConfig.headers['X-API-Key']).toBe('test-key'); }); it('should throw on network error', async () => { mockAxios.mockRejectedValue(new Error('ECONNREFUSED')); - await expect(getCodeOutputDownloadStream('s/f', 'key')).rejects.toThrow(); + await expect(getCodeOutputDownloadStream('s/f')).rejects.toThrow(); }); }); @@ -68,7 +67,6 @@ describe('Code CRUD', () => { req: { user: { id: 'user-123' } }, stream: Readable.from(['file-content']), filename: 'data.csv', - apiKey: 'test-key', }; it('should pass dedicated keepAlive:false agents to axios', async () => { diff --git a/api/server/services/Files/Code/process.js b/api/server/services/Files/Code/process.js index d7076f75f2..028b8c1872 100644 --- a/api/server/services/Files/Code/process.js +++ b/api/server/services/Files/Code/process.js @@ -70,7 +70,6 @@ const createDownloadFallback = ({ * @param {ServerRequest} params.req - The Express request object. * @param {string} params.id - The file ID from the code environment. * @param {string} params.name - The filename. - * @param {string} params.apiKey - The code execution API key. * @param {string} params.toolCallId - The tool call ID that generated the file. * @param {string} params.session_id - The code execution session ID. * @param {string} params.conversationId - The current conversation ID. @@ -81,7 +80,6 @@ const processCodeOutput = async ({ req, id, name, - apiKey, toolCallId, conversationId, messageId, @@ -108,7 +106,6 @@ const processCodeOutput = async ({ responseType: 'arraybuffer', headers: { 'User-Agent': 'LibreChat/1.0', - 'X-API-Key': apiKey, }, httpAgent: codeServerHttpAgent, httpsAgent: codeServerHttpsAgent, @@ -280,15 +277,13 @@ function checkIfActive(dateString) { /** * Retrieves the `lastModified` time string for a specified file from Code Execution Server. * - * @param {Object} params - The parameters object. - * @param {string} params.fileIdentifier - The identifier for the file (e.g., "session_id/fileId"). - * @param {string} params.apiKey - The API key for authentication. + * @param {string} fileIdentifier - The identifier for the file (e.g., "session_id/fileId"). * * @returns {Promise} * A promise that resolves to the `lastModified` time string of the file if successful, or null if there is an * error in initialization or fetching the info. */ -async function getSessionInfo(fileIdentifier, apiKey) { +async function getSessionInfo(fileIdentifier) { try { const baseURL = getCodeBaseURL(); const [path, queryString] = fileIdentifier.split('?'); @@ -304,7 +299,6 @@ async function getSessionInfo(fileIdentifier, apiKey) { params: queryParams, headers: { 'User-Agent': 'LibreChat/1.0', - 'X-API-Key': apiKey, }, httpAgent: codeServerHttpAgent, httpsAgent: codeServerHttpsAgent, @@ -327,13 +321,12 @@ async function getSessionInfo(fileIdentifier, apiKey) { * @param {ServerRequest} options.req * @param {Agent['tool_resources']} options.tool_resources * @param {string} [options.agentId] - The agent ID for file access control - * @param {string} apiKey * @returns {Promise<{ * files: Array<{ id: string; session_id: string; name: string }>, * toolContext: string, * }>} */ -const primeFiles = async (options, apiKey) => { +const primeFiles = async (options) => { const { tool_resources, req, agentId } = options; const file_ids = tool_resources?.[EToolResources.execute_code]?.file_ids ?? []; const agentResourceIds = new Set(file_ids); @@ -414,7 +407,6 @@ const primeFiles = async (options, apiKey) => { stream, filename: file.filename, entity_id: queryParams.entity_id, - apiKey, }); // Preserve existing metadata when adding fileIdentifier @@ -436,7 +428,7 @@ const primeFiles = async (options, apiKey) => { ); } }; - const uploadTime = await getSessionInfo(file.metadata.fileIdentifier, apiKey); + const uploadTime = await getSessionInfo(file.metadata.fileIdentifier); if (!uploadTime) { logger.warn(`Failed to get upload time for file ${id} in session ${session_id}`); await reuploadFile(); diff --git a/api/server/services/Files/process.js b/api/server/services/Files/process.js index f9891483d4..07c101fc33 100644 --- a/api/server/services/Files/process.js +++ b/api/server/services/Files/process.js @@ -18,7 +18,6 @@ const { getEndpointFileConfig, documentParserMimeTypes, } = require('librechat-data-provider'); -const { EnvVar } = require('@librechat/agents'); const { logger } = require('@librechat/data-schemas'); const { sanitizeFilename, parseText, processAudioFile } = require('@librechat/api'); const { @@ -503,13 +502,11 @@ const processAgentFileUpload = async ({ req, res, metadata }) => { throw new Error('Code execution is not enabled for Agents'); } const { handleFileUpload: uploadCodeEnvFile } = getStrategyFunctions(FileSources.execute_code); - const result = await loadAuthValues({ userId: req.user.id, authFields: [EnvVar.CODE_API_KEY] }); const stream = fs.createReadStream(file.path); const fileIdentifier = await uploadCodeEnvFile({ req, stream, filename: file.originalname, - apiKey: result[EnvVar.CODE_API_KEY], entity_id, }); fileInfoMetadata = { fileIdentifier }; diff --git a/api/server/services/Files/process.spec.js b/api/server/services/Files/process.spec.js index 39300161a8..88f2bb7b6b 100644 --- a/api/server/services/Files/process.spec.js +++ b/api/server/services/Files/process.spec.js @@ -4,9 +4,7 @@ jest.mock('@librechat/data-schemas', () => ({ logger: { warn: jest.fn(), debug: jest.fn(), error: jest.fn() }, })); -jest.mock('@librechat/agents', () => ({ - EnvVar: { CODE_API_KEY: 'CODE_API_KEY' }, -})); +jest.mock('@librechat/agents', () => ({})); jest.mock('@librechat/api', () => ({ sanitizeFilename: jest.fn((n) => n), diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 8977c89d3e..690af7f4e4 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -2,7 +2,6 @@ const { logger } = require('@librechat/data-schemas'); const { tool: toolFn, DynamicStructuredTool } = require('@langchain/core/tools'); const { sleep, - EnvVar, StepTypes, GraphEvents, createToolSearch, @@ -60,7 +59,6 @@ const { primeFiles: primeSearchFiles } = require('~/app/clients/tools/util/fileS const { primeFiles: primeCodeFiles } = require('~/server/services/Files/Code/process'); const { manifestToolMap, toolkits } = require('~/app/clients/tools/manifest'); const { createOnSearchResults } = require('~/server/services/Tools/search'); -const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { reinitMCPServer } = require('~/server/services/Tools/mcp'); const { resolveConfigServers } = require('~/server/services/MCP'); const { recordUsage } = require('~/server/services/Threads'); @@ -715,7 +713,6 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to }, { isBuiltInTool, - loadAuthValues, getOrFetchMCPServerTools, getActionToolDefinitions, }, @@ -770,7 +767,6 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to }, { isBuiltInTool, - loadAuthValues, getOrFetchMCPServerTools, getActionToolDefinitions, }, @@ -793,20 +789,9 @@ async function loadToolDefinitionsWrapper({ req, res, agent, streamId = null, to if (hasExecuteCode && tool_resources) { try { - const authValues = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); - const codeApiKey = authValues[EnvVar.CODE_API_KEY]; - - if (codeApiKey) { - const { toolContext } = await primeCodeFiles( - { req, tool_resources, agentId: agent.id }, - codeApiKey, - ); - if (toolContext) { - toolContextMap[Tools.execute_code] = toolContext; - } + const { toolContext } = await primeCodeFiles({ req, tool_resources, agentId: agent.id }); + if (toolContext) { + toolContextMap[Tools.execute_code] = toolContext; } } catch (error) { logger.error('[loadToolDefinitionsWrapper] Error priming code files:', error); @@ -992,7 +977,6 @@ async function loadAgentTools({ agentId: agent.id, agentToolOptions: agent.tool_options, deferredToolsEnabled, - loadAuthValues, }); const agentTools = []; @@ -1253,18 +1237,12 @@ async function loadToolsForExecution({ if (isPTC && toolRegistry) { configurable.toolRegistry = toolRegistry; try { - const authValues = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); - const codeApiKey = authValues[EnvVar.CODE_API_KEY]; - - if (codeApiKey) { - const ptcTool = createProgrammaticToolCallingTool({ apiKey: codeApiKey }); - allLoadedTools.push(ptcTool); - } else { - logger.warn('[loadToolsForExecution] PTC requested but CODE_API_KEY not available'); - } + /** + * PTC auth is handled by the agents library / sandbox service + * directly; LibreChat no longer threads a per-run credential. + */ + const ptcTool = createProgrammaticToolCallingTool({}); + allLoadedTools.push(ptcTool); } catch (error) { logger.error('[loadToolsForExecution] Error creating PTC tool:', error); } @@ -1276,10 +1254,7 @@ async function loadToolsForExecution({ const bashTool = createBashExecutionTool({}); allLoadedTools.push(bashTool); } catch (error) { - logger.error( - '[loadToolsForExecution] Failed to create bash_tool — is LIBRECHAT_CODE_API_KEY set in the server environment?', - error, - ); + logger.error('[loadToolsForExecution] Failed to create bash_tool', error); } } diff --git a/client/src/Providers/BadgeRowContext.tsx b/client/src/Providers/BadgeRowContext.tsx index 05f6528145..025532f0c6 100644 --- a/client/src/Providers/BadgeRowContext.tsx +++ b/client/src/Providers/BadgeRowContext.tsx @@ -6,7 +6,6 @@ import { useMCPServerManager, useSearchApiKeyForm, useGetAgentsConfig, - useCodeApiKeyForm, useToolToggle, } from '~/hooks'; import { getTimestampedValue } from '~/utils/timestamps'; @@ -22,7 +21,6 @@ interface BadgeRowContextType { artifacts: ReturnType; fileSearch: ReturnType; codeInterpreter: ReturnType; - codeApiKeyForm: ReturnType; searchApiKeyForm: ReturnType; mcpServerManager: ReturnType; } @@ -199,20 +197,14 @@ export default function BadgeRowProvider({ } }, [storageSuffix, specName, isSubmitting, setEphemeralAgent]); - /** CodeInterpreter hooks */ - const codeApiKeyForm = useCodeApiKeyForm({}); - const { setIsDialogOpen: setCodeDialogOpen } = codeApiKeyForm; - + /** CodeInterpreter hook — sandbox auth is handled server-side by the + * agents library, so the toggle no longer has an auth dialog gate. */ const codeInterpreter = useToolToggle({ conversationId, storageContextKey, - setIsDialogOpen: setCodeDialogOpen, toolKey: Tools.execute_code, localStorageKey: LocalStorageKeys.LAST_CODE_TOGGLE_, - authConfig: { - toolId: Tools.execute_code, - queryOptions: { retry: 1 }, - }, + isAuthenticated: true, }); /** WebSearch hooks */ @@ -268,7 +260,6 @@ export default function BadgeRowProvider({ agentsConfig, conversationId, storageContextKey, - codeApiKeyForm, codeInterpreter, searchApiKeyForm, mcpServerManager, diff --git a/client/src/components/Chat/Input/CodeInterpreter.tsx b/client/src/components/Chat/Input/CodeInterpreter.tsx index c534648837..488f081f32 100644 --- a/client/src/components/Chat/Input/CodeInterpreter.tsx +++ b/client/src/components/Chat/Input/CodeInterpreter.tsx @@ -9,7 +9,6 @@ function CodeInterpreter() { const localize = useLocalize(); const context = useBadgeRowContext(); const { toggleState: runCode, debouncedChange, isPinned } = context?.codeInterpreter ?? {}; - const { badgeTriggerRef } = context?.codeApiKeyForm ?? {}; const canRunCode = useHasAccess({ permissionType: PermissionTypes.RUN_CODE, @@ -23,7 +22,6 @@ function CodeInterpreter() { return ( (runCode || isPinned) && ( webSearchAuthData?.authTypes ?? [], [webSearchAuthData?.authTypes], ); - const codeAuthType = useMemo(() => codeAuthData?.message ?? false, [codeAuthData?.message]); - if (!searchApiKeyForm || !codeApiKeyForm) { + if (!searchApiKeyForm) { return null; } @@ -29,41 +25,18 @@ function ToolDialogs() { menuTriggerRef: searchMenuTriggerRef, } = searchApiKeyForm; - const { - methods: codeMethods, - onSubmit: codeOnSubmit, - isDialogOpen: codeDialogOpen, - setIsDialogOpen: setCodeDialogOpen, - handleRevokeApiKey: codeHandleRevoke, - badgeTriggerRef: codeBadgeTriggerRef, - menuTriggerRef: codeMenuTriggerRef, - } = codeApiKeyForm; - return ( - <> - - - + ); } diff --git a/client/src/components/Chat/Input/ToolsDropdown.tsx b/client/src/components/Chat/Input/ToolsDropdown.tsx index 6b06d5f3cc..a0b969ce59 100644 --- a/client/src/components/Chat/Input/ToolsDropdown.tsx +++ b/client/src/components/Chat/Input/ToolsDropdown.tsx @@ -62,13 +62,10 @@ const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { artifacts, fileSearch, mcpServerManager, - codeApiKeyForm, codeInterpreter, searchApiKeyForm, } = context ?? {}; - const { setIsDialogOpen: setIsCodeDialogOpen, menuTriggerRef: codeMenuTriggerRef } = - codeApiKeyForm ?? {}; const { setIsDialogOpen: setIsSearchDialogOpen, menuTriggerRef: searchMenuTriggerRef } = searchApiKeyForm ?? {}; const { @@ -76,11 +73,7 @@ const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { setIsPinned: setIsSearchPinned, authData: webSearchAuthData, } = webSearch ?? {}; - const { - isPinned: isCodePinned, - setIsPinned: setIsCodePinned, - authData: codeAuthData, - } = codeInterpreter ?? {}; + const { isPinned: isCodePinned, setIsPinned: setIsCodePinned } = codeInterpreter ?? {}; const { isPinned: isFileSearchPinned, setIsPinned: setIsFileSearchPinned } = fileSearch ?? {}; const { isPinned: isArtifactsPinned, setIsPinned: setIsArtifactsPinned } = artifacts ?? {}; const { isPinned: isSkillsPinned, setIsPinned: setIsSkillsPinned } = skills ?? {}; @@ -91,11 +84,6 @@ const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { return !authTypes.every(([, authType]) => authType === AuthType.SYSTEM_DEFINED); }, [webSearchAuthData?.authTypes]); - const showCodeSettings = useMemo( - () => codeAuthData?.message !== AuthType.SYSTEM_DEFINED, - [codeAuthData?.message], - ); - const handleWebSearchToggle = useCallback(() => { const newValue = !webSearch?.toggleState; webSearch?.debouncedChange({ value: newValue }); @@ -276,26 +264,6 @@ const ToolsDropdown = ({ disabled }: ToolsDropdownProps) => { {localize('com_assistants_code_interpreter')}
- {showCodeSettings && ( - - )} ); - return ( - <> - {iconOnly ? : button} - - - ); + return iconOnly ? : button; }, ); diff --git a/client/src/components/SidePanel/Agents/Code/Action.tsx b/client/src/components/SidePanel/Agents/Code/Action.tsx index f6ca3ca1aa..371f65e361 100644 --- a/client/src/components/SidePanel/Agents/Code/Action.tsx +++ b/client/src/components/SidePanel/Agents/Code/Action.tsx @@ -1,7 +1,5 @@ -import { useRef } from 'react'; -import { KeyRoundIcon } from 'lucide-react'; -import { AuthType, AgentCapabilities } from 'librechat-data-provider'; -import { useFormContext, Controller, useWatch } from 'react-hook-form'; +import { AgentCapabilities } from 'librechat-data-provider'; +import { useFormContext, Controller } from 'react-hook-form'; import { Checkbox, HoverCard, @@ -11,121 +9,62 @@ import { HoverCardTrigger, } from '@librechat/client'; import type { AgentForm } from '~/common'; -import { useLocalize, useCodeApiKeyForm } from '~/hooks'; -import ApiKeyDialog from './ApiKeyDialog'; +import { useLocalize } from '~/hooks'; import { ESide } from '~/common'; -import { cn } from '~/utils'; -export default function Action({ authType = '', isToolAuthenticated = false }) { +export default function Action() { const localize = useLocalize(); const methods = useFormContext(); const { control, setValue } = methods; - const apiKeyButtonRef = useRef(null); - const { - onSubmit, - isDialogOpen, - setIsDialogOpen, - handleRevokeApiKey, - methods: keyFormMethods, - } = useCodeApiKeyForm({ - onSubmit: () => { - setValue(AgentCapabilities.execute_code, true, { shouldDirty: true }); - setTimeout(() => apiKeyButtonRef.current?.focus(), 100); - }, - onRevoke: () => { - setValue(AgentCapabilities.execute_code, false, { shouldDirty: true }); - setTimeout(() => apiKeyButtonRef.current?.focus(), 100); - }, - }); - - const runCodeIsEnabled = useWatch({ control, name: AgentCapabilities.execute_code }); - const isUserProvided = authType === AuthType.USER_PROVIDED; - - const handleCheckboxChange = (checked: boolean) => { - if (isToolAuthenticated) { - setValue(AgentCapabilities.execute_code, checked, { shouldDirty: true }); - } else if (runCodeIsEnabled) { - setValue(AgentCapabilities.execute_code, false, { shouldDirty: true }); - } else { - setIsDialogOpen(true); - } - }; return ( - <> - -
- ( - - )} - /> - -
- {isUserProvided && ( - - )} - - - -
- - -
-

- {localize('com_agents_code_interpreter')} -

-
- - + +
+ ( + + setValue(AgentCapabilities.execute_code, checked === true, { shouldDirty: true }) + } + className="relative float-left mr-2 inline-flex h-4 w-4 cursor-pointer" + value={field.value.toString()} + aria-labelledby="execute-code-label" + /> + )} + /> + +
+ + +
- - - + + +
+

+ {localize('com_agents_code_interpreter')} +

+
+ + +
+ ); } diff --git a/client/src/components/SidePanel/Agents/Code/ApiKeyDialog.tsx b/client/src/components/SidePanel/Agents/Code/ApiKeyDialog.tsx deleted file mode 100644 index 70f114ae44..0000000000 --- a/client/src/components/SidePanel/Agents/Code/ApiKeyDialog.tsx +++ /dev/null @@ -1,117 +0,0 @@ -import { OGDialogTemplate, Input, Button, OGDialog } from '@librechat/client'; -import type { UseFormRegister, UseFormHandleSubmit } from 'react-hook-form'; -import type { ApiKeyFormData } from '~/common'; -import type { RefObject } from 'react'; -import { useLocalize } from '~/hooks'; - -export default function ApiKeyDialog({ - isOpen, - onSubmit, - onRevoke, - onOpenChange, - isUserProvided, - isToolAuthenticated, - register, - handleSubmit, - triggerRef, - triggerRefs, -}: { - isOpen: boolean; - onOpenChange: (open: boolean) => void; - onSubmit: (data: { apiKey: string }) => void; - onRevoke: () => void; - isUserProvided: boolean; - isToolAuthenticated: boolean; - register: UseFormRegister; - handleSubmit: UseFormHandleSubmit; - triggerRef?: RefObject; - triggerRefs?: RefObject[]; -}) { - const localize = useLocalize(); - const languageIcons = [ - 'python.svg', - 'nodedotjs.svg', - 'tsnode.svg', - 'rust.svg', - 'go.svg', - 'c.svg', - 'cplusplus.svg', - 'php.svg', - 'fortran.svg', - 'r.svg', - ]; - - return ( - - -
- {localize('com_ui_librechat_code_api_title')} -
-
- {localize('com_ui_librechat_code_api_subtitle')} -
- {/* Language Icons Stack */} -
-
- {languageIcons.map((icon) => ( -
- -
- ))} -
- - {localize('com_ui_librechat_code_api_key')} - -
-
- (e.target.readOnly = false)} - {...register('apiKey', { required: true })} - /> -
- - } - selection={{ - selectHandler: handleSubmit(onSubmit), - selectClasses: 'bg-green-500 hover:bg-green-600 text-white', - selectText: localize('com_ui_save'), - }} - buttons={ - isUserProvided && - isToolAuthenticated && ( - - ) - } - showCancelButton={true} - /> - - ); -} diff --git a/client/src/components/SidePanel/Agents/Code/Form.tsx b/client/src/components/SidePanel/Agents/Code/Form.tsx index 735dfa4537..24389e6a76 100644 --- a/client/src/components/SidePanel/Agents/Code/Form.tsx +++ b/client/src/components/SidePanel/Agents/Code/Form.tsx @@ -1,6 +1,4 @@ -import { Tools } from 'librechat-data-provider'; import type { ExtendedFile } from '~/common'; -import { useVerifyAgentToolAuth } from '~/data-provider'; import { useLocalize } from '~/hooks'; import Action from './Action'; import Files from './Files'; @@ -13,7 +11,6 @@ export default function CodeForm({ files?: [string, ExtendedFile][]; }) { const localize = useLocalize(); - const { data } = useVerifyAgentToolAuth({ toolId: Tools.execute_code }); return (
@@ -30,7 +27,7 @@ export default function CodeForm({
- +
diff --git a/client/src/hooks/Plugins/index.ts b/client/src/hooks/Plugins/index.ts index 9262503016..fc360b759d 100644 --- a/client/src/hooks/Plugins/index.ts +++ b/client/src/hooks/Plugins/index.ts @@ -1,5 +1,3 @@ export * from './useToolToggle'; -export { default as useAuthCodeTool } from './useAuthCodeTool'; -export { default as useCodeApiKeyForm } from './useCodeApiKeyForm'; export { default as useSearchApiKeyForm } from './useSearchApiKeyForm'; export { default as usePluginDialogHelpers } from './usePluginDialogHelpers'; diff --git a/client/src/hooks/Plugins/useAuthCodeTool.ts b/client/src/hooks/Plugins/useAuthCodeTool.ts deleted file mode 100644 index f523fd01ef..0000000000 --- a/client/src/hooks/Plugins/useAuthCodeTool.ts +++ /dev/null @@ -1,51 +0,0 @@ -import { useCallback } from 'react'; -import { useQueryClient } from '@tanstack/react-query'; -import { AuthType, Tools, QueryKeys } from 'librechat-data-provider'; -import { useUpdateUserPluginsMutation } from 'librechat-data-provider/react-query'; - -const useAuthCodeTool = (options?: { isEntityTool: boolean }) => { - const queryClient = useQueryClient(); - const isEntityTool = options?.isEntityTool ?? true; - const updateUserPlugins = useUpdateUserPluginsMutation({ - onMutate: (vars) => { - queryClient.setQueryData([QueryKeys.toolAuth, Tools.execute_code], () => ({ - authenticated: vars.action === 'install', - message: AuthType.USER_PROVIDED, - })); - }, - onSuccess: () => { - queryClient.invalidateQueries([QueryKeys.toolAuth, Tools.execute_code]); - }, - onError: () => { - queryClient.invalidateQueries([QueryKeys.toolAuth, Tools.execute_code]); - }, - }); - - const installTool = useCallback( - (apiKey: string) => { - updateUserPlugins.mutate({ - pluginKey: Tools.execute_code, - action: 'install', - auth: { LIBRECHAT_CODE_API_KEY: apiKey }, - isEntityTool, - }); - }, - [updateUserPlugins, isEntityTool], - ); - - const removeTool = useCallback(() => { - updateUserPlugins.mutate({ - pluginKey: Tools.execute_code, - action: 'uninstall', - auth: { LIBRECHAT_CODE_API_KEY: null }, - isEntityTool, - }); - }, [updateUserPlugins, isEntityTool]); - - return { - removeTool, - installTool, - }; -}; - -export default useAuthCodeTool; diff --git a/client/src/hooks/Plugins/useCodeApiKeyForm.ts b/client/src/hooks/Plugins/useCodeApiKeyForm.ts deleted file mode 100644 index 32120c8ab2..0000000000 --- a/client/src/hooks/Plugins/useCodeApiKeyForm.ts +++ /dev/null @@ -1,47 +0,0 @@ -// client/src/hooks/Plugins/useCodeApiKeyForm.ts -import { useRef, useState, useCallback } from 'react'; -import { useForm } from 'react-hook-form'; -import type { ApiKeyFormData } from '~/common'; -import useAuthCodeTool from '~/hooks/Plugins/useAuthCodeTool'; - -export default function useCodeApiKeyForm({ - onSubmit, - onRevoke, -}: { - onSubmit?: () => void; - onRevoke?: () => void; -}) { - const methods = useForm(); - const menuTriggerRef = useRef(null); - const badgeTriggerRef = useRef(null); - const [isDialogOpen, setIsDialogOpen] = useState(false); - const { installTool, removeTool } = useAuthCodeTool({ isEntityTool: true }); - const { reset } = methods; - - const onSubmitHandler = useCallback( - (data: { apiKey: string }) => { - reset(); - installTool(data.apiKey); - setIsDialogOpen(false); - onSubmit?.(); - }, - [onSubmit, reset, installTool], - ); - - const handleRevokeApiKey = useCallback(() => { - reset(); - removeTool(); - setIsDialogOpen(false); - onRevoke?.(); - }, [reset, onRevoke, removeTool]); - - return { - methods, - isDialogOpen, - setIsDialogOpen, - handleRevokeApiKey, - onSubmit: onSubmitHandler, - badgeTriggerRef, - menuTriggerRef, - }; -} diff --git a/client/src/locales/en/translation.json b/client/src/locales/en/translation.json index 2728fd1ff2..b61f4bc6a6 100644 --- a/client/src/locales/en/translation.json +++ b/client/src/locales/en/translation.json @@ -651,7 +651,6 @@ "com_ui_action_button": "Action Button", "com_ui_active": "Active", "com_ui_add": "Add", - "com_ui_add_code_interpreter_api_key": "Add Code Interpreter API Key", "com_ui_add_first_bookmark": "Click on a chat to add one", "com_ui_add_first_mcp_server": "Create your first MCP server to get started", "com_ui_add_first_prompt": "Create your first prompt to get started", @@ -1120,9 +1119,6 @@ "com_ui_latest_footer": "Every AI for Everyone.", "com_ui_latest_version": "Latest version", "com_ui_leave_blank_to_keep": "Leave blank to keep existing", - "com_ui_librechat_code_api_key": "Get your LibreChat Code Interpreter API key", - "com_ui_librechat_code_api_subtitle": "Secure. Multi-language. Input/Output Files.", - "com_ui_librechat_code_api_title": "Run AI Code", "com_ui_light_theme_enabled": "Light theme enabled", "com_ui_link_copied": "Link copied", "com_ui_link_refreshed": "Link refreshed", diff --git a/package-lock.json b/package-lock.json index 6aea881865..d3013b350d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -59,7 +59,7 @@ "@google/genai": "^1.19.0", "@keyv/redis": "^4.3.3", "@langchain/core": "^0.3.80", - "@librechat/agents": "^3.1.68-dev.0", + "@librechat/agents": "^3.1.68-dev.1", "@librechat/api": "*", "@librechat/data-schemas": "*", "@microsoft/microsoft-graph-client": "^3.0.7", @@ -11894,9 +11894,9 @@ } }, "node_modules/@librechat/agents": { - "version": "3.1.68-dev.0", - "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.1.68-dev.0.tgz", - "integrity": "sha512-xpPU5kEYe8/vUcZKAHUxJz6sHxHP0iqLb9kyziioGAzP1v5Bd8OTKOdasUQlDF3P0trkRhVgIs3ga6trGJuz6Q==", + "version": "3.1.68-dev.1", + "resolved": "https://registry.npmjs.org/@librechat/agents/-/agents-3.1.68-dev.1.tgz", + "integrity": "sha512-AYQB20CrqwC9VXyFkXXEBRfLH5WTkSJzLyOHs9Rt++BT8lzjc+1wxJ9aDDu9gO4SQQNE+dvBlWeuY+lyh1utqA==", "license": "MIT", "dependencies": { "@anthropic-ai/sdk": "^0.73.0", @@ -44232,7 +44232,7 @@ "@google/genai": "^1.19.0", "@keyv/redis": "^4.3.3", "@langchain/core": "^0.3.80", - "@librechat/agents": "^3.1.68-dev.0", + "@librechat/agents": "^3.1.68-dev.1", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.29.0", "@smithy/node-http-handler": "^4.4.5", diff --git a/packages/api/package.json b/packages/api/package.json index b7aa39ef32..b2d820db0b 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -95,7 +95,7 @@ "@google/genai": "^1.19.0", "@keyv/redis": "^4.3.3", "@langchain/core": "^0.3.80", - "@librechat/agents": "^3.1.68-dev.0", + "@librechat/agents": "^3.1.68-dev.1", "@librechat/data-schemas": "*", "@modelcontextprotocol/sdk": "^1.29.0", "@smithy/node-http-handler": "^4.4.5", diff --git a/packages/api/src/agents/__tests__/initialize.test.ts b/packages/api/src/agents/__tests__/initialize.test.ts index 51fcc6d5ba..8ed8218f68 100644 --- a/packages/api/src/agents/__tests__/initialize.test.ts +++ b/packages/api/src/agents/__tests__/initialize.test.ts @@ -940,3 +940,201 @@ describe('initializeAgent — skill `allowed-tools` union (Phase 6)', () => { expect(loadTools.mock.calls[0][0].tools).toEqual([]); }); }); + +describe('initializeAgent — execute_code capability expansion', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('expands execute_code into bash_tool + read_file when codeEnvAvailable=true', async () => { + const { agent, req, res, loadTools, db } = createMocks(); + agent.tools = ['execute_code']; + + const result = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: true, + }, + db, + ); + + const names = (result.toolDefinitions ?? []).map((d) => d.name); + expect(names).toContain('bash_tool'); + expect(names).toContain('read_file'); + /* The legacy `execute_code` tool def is no longer registered by this + path — the string stays in `agent.tools` as the capability trigger + but never appears in the tool definitions the LLM sees. */ + expect(names).not.toContain('execute_code'); + }); + + it('does not register bash_tool + read_file when codeEnvAvailable=false', async () => { + const { agent, req, res, loadTools, db } = createMocks(); + agent.tools = ['execute_code']; + + const result = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: false, + }, + db, + ); + + const names = (result.toolDefinitions ?? []).map((d) => d.name); + expect(names).not.toContain('bash_tool'); + expect(names).not.toContain('read_file'); + }); + + it('does not register bash_tool + read_file when agent does not request execute_code', async () => { + const { agent, req, res, loadTools, db } = createMocks(); + agent.tools = ['web_search']; + + const result = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: true, + }, + db, + ); + + const names = (result.toolDefinitions ?? []).map((d) => d.name); + expect(names).not.toContain('bash_tool'); + expect(names).not.toContain('read_file'); + }); + + it('narrows codeEnvAvailable on InitializedAgent to the per-agent effective value', async () => { + /* The admin-level `params.codeEnvAvailable` is AND-ed with + `agent.tools.includes('execute_code')` and stored on the returned + agent. Downstream runtime code (JS controllers, `primeInvokedSkills`) + reads the narrowed value from the stored context so skills-only + agents never accidentally trip sandbox-side logic. */ + const { agent, req, res, loadTools, db } = createMocks(); + + // Admin cap on, agent asks for execute_code → effective true. + agent.tools = ['execute_code']; + const execAgent = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: true, + }, + db, + ); + expect(execAgent.codeEnvAvailable).toBe(true); + + // Admin cap on, agent does NOT ask for execute_code → effective false. + agent.tools = ['web_search']; + const skillsOnlyAgent = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: true, + }, + db, + ); + expect(skillsOnlyAgent.codeEnvAvailable).toBe(false); + + // Admin cap off, agent asks for execute_code → still effective false. + agent.tools = ['execute_code']; + const capOffAgent = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: false, + }, + db, + ); + expect(capOffAgent.codeEnvAvailable).toBe(false); + + // Neither → effective false. + agent.tools = ['web_search']; + const neitherAgent = await initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.OPENAI]), + isInitialAgent: true, + codeEnvAvailable: false, + }, + db, + ); + expect(neitherAgent.codeEnvAvailable).toBe(false); + }); + + it('trips GOOGLE_TOOL_CONFLICT on Google/Vertex when execute_code expands alongside provider tools', async () => { + /* Pre-Phase 8, an `execute_code`-only agent on Google/Vertex with + `options.tools` populated would throw GOOGLE_TOOL_CONFLICT because + `CodeExecutionToolDefinition` populated `toolDefinitions` and + `hasAgentTools` was true. After dropping that registry entry, the + check is now gated on the runtime-expanded `bash_tool` + `read_file` + pair — so the expansion MUST happen before `hasAgentTools` is + computed or the guard silently goes away for this scenario. */ + const { agent, req, res, loadTools, db } = createMocks({ + provider: Providers.GOOGLE, + overrideProvider: Providers.GOOGLE, + }); + agent.tools = ['execute_code']; + + /* Surface an options.tools array from the provider config — this is + the `google_search` / `url_context` built-in LLM tooling that + Google/Vertex exposes via provider options. */ + mockGetProviderConfig.mockReturnValue({ + getOptions: jest.fn().mockResolvedValue({ + llmConfig: { model: 'test-model', maxTokens: 4096 }, + tools: [{ google_search: {} }], + } satisfies InitializeResultBase), + overrideProvider: Providers.GOOGLE, + }); + + await expect( + initializeAgent( + { + req, + res, + agent, + loadTools, + endpointOption: { endpoint: EModelEndpoint.agents }, + allowedProviders: new Set([Providers.GOOGLE]), + isInitialAgent: true, + codeEnvAvailable: true, + }, + db, + ), + ).rejects.toThrow(/google_tool_conflict/); + }); +}); diff --git a/packages/api/src/agents/__tests__/skills.test.ts b/packages/api/src/agents/__tests__/skills.test.ts index 85fedf2eec..b56b28667a 100644 --- a/packages/api/src/agents/__tests__/skills.test.ts +++ b/packages/api/src/agents/__tests__/skills.test.ts @@ -697,6 +697,66 @@ describe('injectSkillCatalog', () => { expect(definedNames).toContain('bash_tool'); expect(definedNames).not.toContain('skill'); }); + + it('does NOT register bash_tool when codeEnvAvailable is false (skills-only agent)', async () => { + /* Narrowing regression: `initializeAgent` now passes the per-agent + effective flag (admin cap AND `agent.tools.includes('execute_code')`). + A skills-only agent passes `false` here, and `bash_tool` must stay + out of the registered toolDefinitions even with an active skill + catalog. `read_file` still registers — manually-primed skills + read their `references/*` from storage without a sandbox. */ + const owned = makeSkill('owned-skill', userObjectId); + const listSkillsByAccess = buildPager([[owned]]); + const result = await injectSkillCatalog( + baseParams({ listSkillsByAccess, codeEnvAvailable: false }), + ); + const definedNames = (result.toolDefinitions ?? []).map((d) => d.name); + expect(definedNames).toContain('read_file'); + expect(definedNames).toContain('skill'); + expect(definedNames).not.toContain('bash_tool'); + }); + + it('does not duplicate bash_tool/read_file already registered by the execute_code path', async () => { + /* Simulates the Phase 8 dedupe: when an agent has both the + `execute_code` capability (registers bash_tool+read_file via + `registerCodeExecutionTools` before catalog injection) AND skills + active, `injectSkillCatalog` must see the existing entries in the + registry and skip re-adding. One copy of each reaches the LLM. */ + const owned = makeSkill('owned-skill', userObjectId); + const listSkillsByAccess = buildPager([[owned]]); + type ToolRegistryArg = NonNullable[0]['toolRegistry']>; + type ToolDef = Parameters[1]; + const preBash: ToolDef = { + name: 'bash_tool', + description: 'pre', + parameters: { type: 'object', properties: {} }, + }; + const preRead: ToolDef = { + name: 'read_file', + description: 'pre', + parameters: { type: 'object', properties: {} }, + responseFormat: 'content', + }; + const preRegistry = new Map() as unknown as ToolRegistryArg; + preRegistry.set('bash_tool', preBash); + preRegistry.set('read_file', preRead); + const result = await injectSkillCatalog( + baseParams({ + listSkillsByAccess, + codeEnvAvailable: true, + toolRegistry: preRegistry, + toolDefinitions: [preBash, preRead], + }), + ); + const names = (result.toolDefinitions ?? []).map((d) => d.name); + const bashOccurrences = names.filter((n) => n === 'bash_tool').length; + const readOccurrences = names.filter((n) => n === 'read_file').length; + expect(bashOccurrences).toBe(1); + expect(readOccurrences).toBe(1); + /* Skill tool still gets registered because there is at least one + catalog-visible skill. */ + expect(names).toContain('skill'); + }); }); describe('buildSkillPrimeMessage', () => { diff --git a/packages/api/src/agents/discovery.spec.ts b/packages/api/src/agents/discovery.spec.ts index 5b86cd194e..70a9012478 100644 --- a/packages/api/src/agents/discovery.spec.ts +++ b/packages/api/src/agents/discovery.spec.ts @@ -236,6 +236,75 @@ describe('discoverConnectedAgents', () => { expect(initArgs.endpointOption.endpoint).toBe(EModelEndpoint.agents); }); + it('forwards codeEnvAvailable to every handoff initializeAgent call', async () => { + /* Pre-Phase 8, a handoff sub-agent with `tools: ['execute_code']` + got `CodeExecutionToolDefinition` registered unconditionally via + the legacy registry path. Phase 8 replaced that with a + `params.codeEnvAvailable`-gated expansion inside `initializeAgent`; + if discovery forgets to forward the primary's capability flag, + handoff agents lose `bash_tool` + `read_file` even though the + primary had them. Pin the pass-through so regressions surface. */ + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + codeEnvAvailable: true, + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(mockInitializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ codeEnvAvailable: true }), + expect.anything(), + ); + }); + + it('forwards codeEnvAvailable=false verbatim so handoff agents respect disabled capability', async () => { + /* Symmetric to the "true" case: when the primary resolved + `codeEnvAvailable = false`, handoffs must NOT accidentally + re-enable code execution. The passthrough must preserve `false` + distinctly from `undefined`. */ + const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); + const getAgent = jest.fn(async () => makeAgent('B', [])); + const checkPermission = jest.fn().mockResolvedValue(true); + + await discoverConnectedAgents( + { + req: makeReq(), + res: makeRes(), + primaryConfig, + allowedProviders: new Set(), + modelsConfig: { openai: ['gpt-4o'] }, + loadTools: jest.fn(), + codeEnvAvailable: false, + }, + { + getAgent, + checkPermission, + logViolation: jest.fn(), + db: {} as never, + }, + ); + + expect(mockInitializeAgent).toHaveBeenCalledWith( + expect.objectContaining({ codeEnvAvailable: false }), + expect.anything(), + ); + }); + it('passes the configured resourceType (e.g. REMOTE_AGENT) to checkPermission', async () => { const primaryConfig = makeConfig('A', [{ from: 'A', to: 'B', edgeType: 'handoff' }]); const getAgent = jest.fn(async () => makeAgent('B', [])); diff --git a/packages/api/src/agents/discovery.ts b/packages/api/src/agents/discovery.ts index f50e6d27e8..57321a4cb3 100644 --- a/packages/api/src/agents/discovery.ts +++ b/packages/api/src/agents/discovery.ts @@ -73,6 +73,16 @@ export interface DiscoverConnectedAgentsParams { skillStates?: InitializeAgentParams['skillStates']; /** Default active-on-share flag, forwarded to each sub-agent. */ defaultActiveOnShare?: InitializeAgentParams['defaultActiveOnShare']; + /** + * Whether the `execute_code` capability is enabled for the run. Forwarded + * verbatim to each handoff sub-agent so `registerCodeExecutionTools` can + * expand `agent.tools: ['execute_code']` into the `bash_tool` + `read_file` + * pair. Omitted (or `undefined`) → the expansion is skipped, matching the + * primary-agent gate; callers that already resolved the capability set + * for the primary SHOULD forward the same value here or sub-agents lose + * code-execution tooling even though their parent had it. + */ + codeEnvAvailable?: InitializeAgentParams['codeEnvAvailable']; } export interface DiscoverConnectedAgentsDeps { @@ -140,6 +150,7 @@ export async function discoverConnectedAgents( computeAccessibleSkillIds, skillStates, defaultActiveOnShare, + codeEnvAvailable, } = params; const { @@ -240,6 +251,7 @@ export async function discoverConnectedAgents( accessibleSkillIds: computeAccessibleSkillIds?.(agent), skillStates, defaultActiveOnShare, + codeEnvAvailable, }, db, ); diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index 92fe256959..0f11e4e7dd 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -606,17 +606,7 @@ describe('createToolExecuteHandler', () => { }); } - const ORIGINAL_KEY = process.env.LIBRECHAT_CODE_API_KEY; - afterEach(() => { - if (ORIGINAL_KEY === undefined) { - delete process.env.LIBRECHAT_CODE_API_KEY; - } else { - process.env.LIBRECHAT_CODE_API_KEY = ORIGINAL_KEY; - } - }); - - it('does NOT call listSkillFiles when codeEnvAvailable is false (even when env key is set)', async () => { - process.env.LIBRECHAT_CODE_API_KEY = 'present'; + it('does NOT call listSkillFiles when codeEnvAvailable is false', async () => { const listSkillFiles = jest.fn().mockResolvedValue([]); const handler = makeSkillHandlerWithFiles({ codeEnvAvailable: false, @@ -635,8 +625,7 @@ describe('createToolExecuteHandler', () => { expect(listSkillFiles).not.toHaveBeenCalled(); }); - it('calls listSkillFiles when codeEnvAvailable is true AND the env key is set', async () => { - process.env.LIBRECHAT_CODE_API_KEY = 'present'; + it('calls listSkillFiles when codeEnvAvailable is true', async () => { const listSkillFiles = jest.fn().mockResolvedValue([]); const handler = makeSkillHandlerWithFiles({ codeEnvAvailable: true, @@ -649,21 +638,5 @@ describe('createToolExecuteHandler', () => { expect(listSkillFiles).toHaveBeenCalledWith(SKILL_ID); }); - - it('does NOT call listSkillFiles when codeEnvAvailable is true but env key is unset (admin misconfig)', async () => { - delete process.env.LIBRECHAT_CODE_API_KEY; - const listSkillFiles = jest.fn().mockResolvedValue([]); - const handler = makeSkillHandlerWithFiles({ - codeEnvAvailable: true, - listSkillFiles, - }); - - const [result] = await invokeHandler(handler, [ - { id: 'call_no_env', name: Constants.SKILL_TOOL, args: { skillName: 'brand-guidelines' } }, - ]); - - expect(result.status).toBe('success'); - expect(listSkillFiles).not.toHaveBeenCalled(); - }); }); }); diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 2e6f06dfd2..eb8cbb06b8 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -1,5 +1,5 @@ import { logger } from '@librechat/data-schemas'; -import { EnvVar, GraphEvents, Constants, CODE_EXECUTION_TOOLS } from '@librechat/agents'; +import { GraphEvents, Constants, CODE_EXECUTION_TOOLS } from '@librechat/agents'; import type { LCTool, EventHandler, @@ -86,11 +86,10 @@ export interface ToolExecuteOptions { batchUploadCodeEnvFiles?: (params: { req: ServerRequest; files: Array<{ stream: NodeJS.ReadableStream; filename: string }>; - apiKey: string; entity_id?: string; }) => Promise<{ session_id: string; files: Array<{ fileId: string; filename: string }> }>; /** Checks if a code env file is still active. Returns lastModified or null. */ - getSessionInfo?: (fileIdentifier: string, apiKey: string) => Promise; + getSessionInfo?: (fileIdentifier: string) => Promise; /** 23-hour freshness check */ checkIfActive?: (dateString: string) => boolean; /** Persists codeEnvIdentifiers on skill files after upload */ @@ -532,7 +531,7 @@ async function handleSkillToolCall( // Prime skill files to code env — only when the `execute_code` capability // is enabled for this run. The flag is threaded via configurable upstream - // so this gate cannot be bypassed by a stray env var. + // so this gate cannot be bypassed. const codeEnvAvailable = mergedConfigurable?.codeEnvAvailable === true; if ( codeEnvAvailable && @@ -542,30 +541,26 @@ async function handleSkillToolCall( getStrategyFunctions && batchUploadCodeEnvFiles ) { - const codeApiKey = process.env[EnvVar.CODE_API_KEY] ?? ''; - if (codeApiKey) { - try { - const skillFiles = await listSkillFiles(skill._id); - const primeResult = await primeSkillFiles({ - skill, - skillFiles, - req, - apiKey: codeApiKey, - getStrategyFunctions, - batchUploadCodeEnvFiles, - getSessionInfo, - checkIfActive, - updateSkillFileCodeEnvIds, - }); - if (primeResult) { - artifact = primeResult; - } - } catch (error) { - logger.error( - `[handleSkillToolCall] Failed to prime files for skill "${args.skillName}":`, - error instanceof Error ? error.message : error, - ); + try { + const skillFiles = await listSkillFiles(skill._id); + const primeResult = await primeSkillFiles({ + skill, + skillFiles, + req, + getStrategyFunctions, + batchUploadCodeEnvFiles, + getSessionInfo, + checkIfActive, + updateSkillFileCodeEnvIds, + }); + if (primeResult) { + artifact = primeResult; } + } catch (error) { + logger.error( + `[handleSkillToolCall] Failed to prime files for skill "${args.skillName}":`, + error instanceof Error ? error.message : error, + ); } } diff --git a/packages/api/src/agents/initialize.ts b/packages/api/src/agents/initialize.ts index 0d0b9aa953..2f9f1bdf9f 100644 --- a/packages/api/src/agents/initialize.ts +++ b/packages/api/src/agents/initialize.ts @@ -1,6 +1,7 @@ import { Providers } from '@librechat/agents'; import { logger } from '@librechat/data-schemas'; import { + Tools, Constants, ErrorTypes, EModelEndpoint, @@ -38,6 +39,7 @@ import { unionPrimeAllowedTools, MAX_PRIMED_SKILLS_PER_TURN, } from './skills'; +import { registerCodeExecutionTools } from './tools'; import { primeResources } from './resources'; import type { ResolvedManualSkill, ResolvedAlwaysApplySkill } from './skills'; import type { TFilterFilesByAgentAccess } from './resources'; @@ -75,6 +77,18 @@ export type InitializedAgent = Agent & { actionsEnabled?: boolean; /** Maximum characters allowed in a single tool result before truncation. */ maxToolResultChars?: number; + /** + * Whether the code-execution environment is available *for this agent*. + * Narrower than the incoming `params.codeEnvAvailable` admin flag — this + * is `admin_capability_enabled && agent.tools.includes('execute_code')`, + * computed once here so downstream code (`injectSkillCatalog`, + * `enrichWithSkillConfigurable`, `primeInvokedSkills`) doesn't have to + * re-scan the tool list on every runtime handler invocation. + * Authoritative for both persisted and ephemeral agents: the + * ephemeral-agent toggle is reconciled into `agent.tools` upstream + * (`packages/api/src/agents/added.ts`), so the check is uniform. + */ + codeEnvAvailable: boolean; /** Accessible skill IDs for ACL checking at execute time */ accessibleSkillIds?: import('mongoose').Types.ObjectId[]; /** Number of skills in the catalog (used to determine if SkillTool should be registered) */ @@ -696,6 +710,54 @@ export async function initializeAgent( agent.provider = options.provider; } + /** + * Unify code-execution tools around `bash_tool` + `read_file` when the + * agent explicitly lists `execute_code` in its tools and the admin + * capability is enabled for the run. The legacy `execute_code` tool + * (backed by `CodeExecutionToolDefinition` + `primeCodeFiles`) is no + * longer registered; the string `execute_code` on the agent document + * stays as the capability-trigger marker but expands into the + * skill-flavored tool pair here. + * + * `effectiveCodeEnvAvailable` is the per-agent truth: the admin-level + * `params.codeEnvAvailable` AND the agent actually asking for code + * execution. Computed once and reused by the expansion block below, + * the `injectSkillCatalog` call, and the returned `InitializedAgent`. + * Downstream handlers (runtime `configurable`, `primeInvokedSkills`) + * read it from the stored per-agent value so a skills-only agent + * never accidentally registers `bash_tool` or primes sandbox files + * just because the admin globally enabled code execution. + * + * Done BEFORE the `hasAgentTools` / GOOGLE_TOOL_CONFLICT gate so + * execute-code-only agents on Google/Vertex still trip the conflict + * guard when provider-specific tools are also configured. Also before + * `injectSkillCatalog` so the skill path's own + * `registerCodeExecutionTools` call becomes a no-op via the registry + * `.has()` dedupe — exactly one copy of each tool reaches the LLM. + */ + const agentRequestsCodeExec = (agent.tools ?? []).includes(Tools.execute_code); + const effectiveCodeEnvAvailable = params.codeEnvAvailable === true && agentRequestsCodeExec; + if (effectiveCodeEnvAvailable) { + const codeExecResult = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions, + includeBash: true, + }); + toolDefinitions = codeExecResult.toolDefinitions; + } else if (agentRequestsCodeExec) { + /** + * Agent asked for `execute_code` but the admin-level gate is off — + * surface a debug log so operators tracing "why isn't code + * interpreter working?" get a clear signal. The event-driven tool + * loader (`loadToolDefinitionsWrapper`) doesn't log capability- + * disabled warnings for the definitions-only path, so without this, + * the tool silently vanishes from the LLM's definitions with no trace. + */ + logger.debug( + `[initializeAgent] Agent "${agent.id}" requests execute_code but codeEnvAvailable=${String(params.codeEnvAvailable)}; skipping bash_tool + read_file registration.`, + ); + } + /** Check for tool presence from either full instances or definitions (event-driven mode) */ const hasAgentTools = (structuredTools?.length ?? 0) > 0 || (toolDefinitions?.length ?? 0) > 0; @@ -756,7 +818,7 @@ export async function initializeAgent( accessibleSkillIds, contextWindowTokens: Number(agentMaxContextTokens) || 200_000, listSkillsByAccess: db?.listSkillsByAccess, - codeEnvAvailable: params.codeEnvAvailable, + codeEnvAvailable: effectiveCodeEnvAvailable, userId: req.user?.id, skillStates: params.skillStates, defaultActiveOnShare: params.defaultActiveOnShare, @@ -797,6 +859,7 @@ export async function initializeAgent( hasDeferredTools, actionsEnabled, baseContextTokens, + codeEnvAvailable: effectiveCodeEnvAvailable, skillCount, accessibleSkillIds: executableSkillIds, manualSkillPrimes, diff --git a/packages/api/src/agents/openai/service.ts b/packages/api/src/agents/openai/service.ts index 90190ce7ce..0d637f7a50 100644 --- a/packages/api/src/agents/openai/service.ts +++ b/packages/api/src/agents/openai/service.ts @@ -19,6 +19,7 @@ * ``` */ import { nanoid } from 'nanoid'; +import { AgentCapabilities } from 'librechat-data-provider'; import type { Response as ServerResponse, Request } from 'express'; import type { ChatCompletionResponse, @@ -66,7 +67,17 @@ export interface ChatCompletionDependencies { ) => Promise; /** Create agent run */ createRun?: CreateRunFn; - /** App config */ + /** + * App config. Optional, but required for agents with `execute_code` in + * their tools: the helper derives `codeEnvAvailable` from + * `appConfig?.endpoints?.agents?.capabilities` and forwards it into + * `deps.initializeAgent`. When `appConfig` is omitted, the resolved + * `codeEnvAvailable` is `undefined`, so `initializeAgent` skips the + * `execute_code` → `bash_tool` + `read_file` expansion entirely and + * code-requesting agents silently lose sandbox tools. Pass `appConfig` + * (even a minimal shape with just `endpoints.agents.capabilities`) to + * keep code execution working. + */ appConfig?: AppConfig; /** Tool execute options for event-driven tool execution */ toolExecuteOptions?: ToolExecuteOptions; @@ -123,6 +134,15 @@ interface InitializeAgentParams { endpointOption?: Record; allowedProviders: Set; isInitialAgent?: boolean; + /** + * Whether the `execute_code` capability is enabled for the run. + * `initializeAgent` uses this to expand `agent.tools: ['execute_code']` + * into the `bash_tool` + `read_file` pair — if the caller's injected + * `initializeAgent` implementation consults this flag, agents configured + * for code execution will keep working post-Phase-8. Absent / `undefined` + * skips the expansion (same semantics as the in-repo controllers). + */ + codeEnvAvailable?: boolean; } /** @@ -400,6 +420,26 @@ export async function createAgentChatCompletion( // Build allowed providers set (empty = all allowed) const allowedProviders = new Set(); + /** + * Derive `codeEnvAvailable` from the caller-supplied `appConfig` so + * `agent.tools: ['execute_code']` still produces `bash_tool` + + * `read_file` in the initialized agent's `toolDefinitions` (Phase 8 + * removed the legacy `execute_code` tool definition, so the + * capability flag is the sole gate). Uses the + * `AgentCapabilities.execute_code` enum value rather than a string + * literal so an enum rename propagates here automatically. Falls + * back to `undefined` when the caller doesn't provide `appConfig` — + * matching the "explicit opt-in" semantics the in-repo controllers + * use. + */ + const agentsConfig = (deps.appConfig?.endpoints as Record | undefined)?.agents; + const codeEnvAvailable = + agentsConfig != null && typeof agentsConfig === 'object' + ? ((agentsConfig as { capabilities?: string[] }).capabilities ?? []).includes( + AgentCapabilities.execute_code, + ) + : undefined; + // Initialize the agent first to check for disableStreaming const initializedAgent = await deps.initializeAgent({ req, @@ -414,6 +454,7 @@ export async function createAgentChatCompletion( }, allowedProviders, isInitialAgent: true, + codeEnvAvailable, }); // Determine if streaming is enabled (check both request and agent config) diff --git a/packages/api/src/agents/skillConfigurable.spec.ts b/packages/api/src/agents/skillConfigurable.spec.ts index 8b0d695d7f..5b22a23cfa 100644 --- a/packages/api/src/agents/skillConfigurable.spec.ts +++ b/packages/api/src/agents/skillConfigurable.spec.ts @@ -32,17 +32,6 @@ describe('enrichWithSkillConfigurable', () => { expect(result.configurable.codeEnvAvailable).toBe(false); }); - it('does not inject a codeApiKey key (per-user lookup removed)', () => { - const result = enrichWithSkillConfigurable( - { loadedTools: [], configurable: {} }, - req, - accessibleSkillIds, - true, - ); - - expect(result.configurable).not.toHaveProperty('codeApiKey'); - }); - it('threads skillPrimedIdsByName through unchanged', () => { const primed = { 'brand-guidelines': 'abc123' }; const result = enrichWithSkillConfigurable( diff --git a/packages/api/src/agents/skillConfigurable.ts b/packages/api/src/agents/skillConfigurable.ts index 793f4fe7e6..f9202f57e4 100644 --- a/packages/api/src/agents/skillConfigurable.ts +++ b/packages/api/src/agents/skillConfigurable.ts @@ -4,10 +4,8 @@ * `codeEnvAvailable` is threaded as a boolean (true when the agent's * `execute_code` capability is enabled). Downstream skill consumers — * the skill-tool handler (for file priming) and `primeInvokedSkills` - * (for history re-priming) — gate sandbox uploads on this flag rather - * than on API-key presence, so no sandbox traffic occurs for agents - * that lack code-execution capability even if - * `process.env.LIBRECHAT_CODE_API_KEY` happens to be set. + * (for history re-priming) — gate sandbox uploads on this flag so no + * sandbox traffic occurs for agents that lack code-execution capability. * * `skillPrimedIdsByName` maps each primed skill name (manual `$` or * always-apply) to the `_id` of the exact doc whose body was primed diff --git a/packages/api/src/agents/skillFiles.spec.ts b/packages/api/src/agents/skillFiles.spec.ts index c96428ac6c..6c44d3c7f7 100644 --- a/packages/api/src/agents/skillFiles.spec.ts +++ b/packages/api/src/agents/skillFiles.spec.ts @@ -44,23 +44,12 @@ function makeDeps(overrides: Partial = {}): PrimeInvoked } describe('primeInvokedSkills — execute_code capability gate', () => { - const ORIGINAL_KEY = process.env.LIBRECHAT_CODE_API_KEY; - beforeEach(() => { jest.clearAllMocks(); mockExtract.mockReturnValue(new Set(['brand-guidelines'])); }); - afterEach(() => { - if (ORIGINAL_KEY === undefined) { - delete process.env.LIBRECHAT_CODE_API_KEY; - } else { - process.env.LIBRECHAT_CODE_API_KEY = ORIGINAL_KEY; - } - }); - - it('skips the batch-upload path when codeEnvAvailable is false (even if env key is set)', async () => { - process.env.LIBRECHAT_CODE_API_KEY = 'present'; + it('skips the batch-upload path when codeEnvAvailable is false', async () => { const deps = makeDeps({ codeEnvAvailable: false }); const result = await primeInvokedSkills(deps); @@ -70,19 +59,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => { expect(deps.batchUploadCodeEnvFiles).not.toHaveBeenCalled(); }); - it('skips the batch-upload path when codeEnvAvailable is true but env key is unset', async () => { - delete process.env.LIBRECHAT_CODE_API_KEY; - const deps = makeDeps({ codeEnvAvailable: true }); - - const result = await primeInvokedSkills(deps); - - expect(result.skills?.get('brand-guidelines')).toBe('skill body'); - expect(deps.listSkillFiles).not.toHaveBeenCalled(); - expect(deps.batchUploadCodeEnvFiles).not.toHaveBeenCalled(); - }); - - it('enters the batch-upload path when codeEnvAvailable is true and env key is set', async () => { - process.env.LIBRECHAT_CODE_API_KEY = 'present'; + it('enters the batch-upload path when codeEnvAvailable is true', async () => { const deps = makeDeps({ codeEnvAvailable: true }); await primeInvokedSkills(deps); @@ -90,8 +67,7 @@ describe('primeInvokedSkills — execute_code capability gate', () => { expect(deps.listSkillFiles).toHaveBeenCalledWith(SKILL_ID); }); - it('actually calls batchUploadCodeEnvFiles with the env-sourced apiKey when files are returned', async () => { - process.env.LIBRECHAT_CODE_API_KEY = 'sk-from-env'; + it('calls batchUploadCodeEnvFiles without an apiKey when files are returned', async () => { const fileRecords = [ { relativePath: 'references/style.md', @@ -125,7 +101,10 @@ describe('primeInvokedSkills — execute_code capability gate', () => { expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1); const [uploadArgs] = batchUploadCodeEnvFiles.mock.calls[0]; - expect(uploadArgs.apiKey).toBe('sk-from-env'); + /* Phase 8 deprecation: LibreChat no longer threads an apiKey through + the sandbox upload path. The agents library / sandbox service owns + auth internally. */ + expect(uploadArgs).not.toHaveProperty('apiKey'); expect(uploadArgs.entity_id).toBe(SKILL_ID.toString()); /* One uploaded file per `fileRecords` entry plus the synthetic SKILL.md that `primeSkillFiles` always prepends. */ diff --git a/packages/api/src/agents/skillFiles.ts b/packages/api/src/agents/skillFiles.ts index 3e33afbbb1..a1467f75e7 100644 --- a/packages/api/src/agents/skillFiles.ts +++ b/packages/api/src/agents/skillFiles.ts @@ -1,5 +1,5 @@ import { Readable } from 'stream'; -import { Constants, EnvVar } from '@librechat/agents'; +import { Constants } from '@librechat/agents'; import { logger } from '@librechat/data-schemas'; import type { ToolSessionMap, CodeSessionContext } from '@librechat/agents'; import type { Types } from 'mongoose'; @@ -19,7 +19,6 @@ export interface PrimeSkillFilesParams { skill: { body: string; name: string; _id: Types.ObjectId | string }; skillFiles: SkillFileRecord[]; req: ServerRequest; - apiKey: string; getStrategyFunctions: (source: string) => { getDownloadStream?: (req: ServerRequest, filepath: string) => Promise; [key: string]: unknown; @@ -27,14 +26,13 @@ export interface PrimeSkillFilesParams { batchUploadCodeEnvFiles: (params: { req: ServerRequest; files: Array<{ stream: NodeJS.ReadableStream; filename: string }>; - apiKey: string; entity_id?: string; }) => Promise<{ session_id: string; files: Array<{ fileId: string; filename: string }>; }>; /** Checks if a code env file is still active. Returns lastModified timestamp or null. */ - getSessionInfo?: (fileIdentifier: string, apiKey: string) => Promise; + getSessionInfo?: (fileIdentifier: string) => Promise; /** 23-hour freshness check */ checkIfActive?: (dateString: string) => boolean; /** Persists codeEnvIdentifier on skill files after upload */ @@ -69,7 +67,6 @@ export async function primeSkillFiles( skill, skillFiles, req, - apiKey, getStrategyFunctions, batchUploadCodeEnvFiles, getSessionInfo, @@ -99,7 +96,7 @@ export async function primeSkillFiles( if (!representative) { return false; } - const lastModified = await getSessionInfo(representative.codeEnvIdentifier!, apiKey); + const lastModified = await getSessionInfo(representative.codeEnvIdentifier!); return !!(lastModified && checkIfActive(lastModified)); }), ); @@ -163,7 +160,6 @@ export async function primeSkillFiles( const result = await batchUploadCodeEnvFiles({ req, files: filesToUpload, - apiKey, entity_id: entityId, }); // Exclude SKILL.md from the returned files array — it is uploaded to disk @@ -268,8 +264,6 @@ export async function primeInvokedSkills( return {}; } - const apiKey = deps.codeEnvAvailable ? (process.env[EnvVar.CODE_API_KEY] ?? '') : ''; - const skills = new Map(); // Phase 1: Resolve all skills in parallel (DB lookups) @@ -299,7 +293,7 @@ export async function primeInvokedSkills( let sessions: ToolSessionMap | undefined; const skillsWithFiles = resolvedSkills.filter((s) => s.fileCount > 0); - if (apiKey && skillsWithFiles.length > 0) { + if (deps.codeEnvAvailable && skillsWithFiles.length > 0) { // Parallel file list lookups (R2 fix) const fileListResults = await Promise.all( skillsWithFiles.map(async (skill) => ({ @@ -329,10 +323,7 @@ export async function primeInvokedSkills( ); if (!representative) return true; try { - const lastModified = await deps.getSessionInfo?.( - representative.codeEnvIdentifier!, - apiKey, - ); + const lastModified = await deps.getSessionInfo?.(representative.codeEnvIdentifier!); return !!(lastModified && deps.checkIfActive?.(lastModified)); } catch { return false; @@ -378,7 +369,6 @@ export async function primeInvokedSkills( skill, skillFiles: files, req: deps.req, - apiKey, getStrategyFunctions: deps.getStrategyFunctions, batchUploadCodeEnvFiles: deps.batchUploadCodeEnvFiles, getSessionInfo: deps.getSessionInfo, diff --git a/packages/api/src/agents/skills.ts b/packages/api/src/agents/skills.ts index 9a8745befd..c5ecd39eb9 100644 --- a/packages/api/src/agents/skills.ts +++ b/packages/api/src/agents/skills.ts @@ -1,16 +1,12 @@ import { logger } from '@librechat/data-schemas'; import { HumanMessage } from '@langchain/core/messages'; -import { - formatSkillCatalog, - SkillToolDefinition, - ReadFileToolDefinition, - BashExecutionToolDefinition, -} from '@librechat/agents'; +import { formatSkillCatalog, SkillToolDefinition } from '@librechat/agents'; import type { LCToolRegistry, LCTool, InjectedMessage } from '@librechat/agents'; import type { BaseMessage } from '@langchain/core/messages'; import type { Agent } from 'librechat-data-provider'; import type { Types } from 'mongoose'; import type { InitializeAgentDbMethods } from './initialize'; +import { registerCodeExecutionTools } from './tools'; const SKILL_CATALOG_LIMIT = 100; /** Max pages scanned per run when filtering out inactive skills. */ @@ -355,43 +351,30 @@ export async function injectSkillCatalog( parameters: SkillToolDefinition.parameters as unknown as LCTool['parameters'], }; - const readFileDef: LCTool = { - name: ReadFileToolDefinition.name, - description: ReadFileToolDefinition.description, - parameters: ReadFileToolDefinition.parameters as unknown as LCTool['parameters'], - responseFormat: ReadFileToolDefinition.responseFormat, - }; - - const bashToolDef: LCTool = { - name: BashExecutionToolDefinition.name, - description: BashExecutionToolDefinition.description, - parameters: BashExecutionToolDefinition.schema as unknown as LCTool['parameters'], - }; - /** - * `skill` tool is conditional on having anything for the model to invoke; - * `read_file` is always registered when any active skill is in scope - * (manually-primed disabled skills still need it); `bash_tool` follows - * code-env availability as before. + * `skill` tool is conditional on having anything for the model to invoke. + * `read_file` + `bash_tool` go through `registerCodeExecutionTools` so + * a prior registration from `initializeAgent` (for the `execute_code` + * capability) doesn't produce a duplicate copy. `read_file` is always + * included — manually-primed `disable-model-invocation: true` skills + * still need it to load their `references/*` from storage. `bash_tool` + * follows `codeEnvAvailable` as before. */ - const defs: LCTool[] = []; + let workingDefs: LCTool[] = [...(inputDefs ?? [])]; if (catalogVisibleSkills.length > 0) { - defs.push(skillToolDef); - } - defs.push(readFileDef); - if (codeEnvAvailable) { - defs.push(bashToolDef); + workingDefs.push(skillToolDef); + toolRegistry?.set(skillToolDef.name, skillToolDef); } - const toolDefinitions = [...(inputDefs ?? []), ...defs]; - if (toolRegistry) { - for (const def of defs) { - toolRegistry.set(def.name, def); - } - } + const codeExecResult = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: workingDefs, + includeBash: codeEnvAvailable === true, + }); + workingDefs = codeExecResult.toolDefinitions; return { - toolDefinitions, + toolDefinitions: workingDefs, skillCount: catalogVisibleSkills.length, activeSkillIds: executableSkills.map((s) => s._id), }; diff --git a/packages/api/src/agents/tools.spec.ts b/packages/api/src/agents/tools.spec.ts index 49887fbb02..161849dac6 100644 --- a/packages/api/src/agents/tools.spec.ts +++ b/packages/api/src/agents/tools.spec.ts @@ -1,4 +1,26 @@ -import { buildToolSet, BuildToolSetConfig } from './tools'; +/** + * `@librechat/agents` may ship without the skill-flavored tool definitions on + * older installed versions. Stub them so `registerCodeExecutionTools` (which + * consumes only the three exports below) can be exercised deterministically. + * Mirrors the same pattern used in `__tests__/skills.test.ts`. + */ +jest.mock('@librechat/agents', () => ({ + ...jest.requireActual('@librechat/agents'), + ReadFileToolDefinition: { + name: 'read_file', + description: 'read file', + parameters: { type: 'object', properties: {} }, + responseFormat: 'content', + }, + BashExecutionToolDefinition: { + name: 'bash_tool', + description: 'bash', + schema: { type: 'object', properties: {} }, + }, +})); + +import type { LCTool, LCToolRegistry } from '@librechat/agents'; +import { buildToolSet, BuildToolSetConfig, registerCodeExecutionTools } from './tools'; describe('buildToolSet', () => { describe('event-driven mode (toolDefinitions)', () => { @@ -124,3 +146,124 @@ describe('buildToolSet', () => { }); }); }); + +describe('registerCodeExecutionTools', () => { + const makeRegistry = (): LCToolRegistry => new Map() as unknown as LCToolRegistry; + + describe('fresh run (no pre-existing defs or registry entries)', () => { + it('registers read_file + bash_tool when includeBash=true', () => { + const toolRegistry = makeRegistry(); + const result = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: [], + includeBash: true, + }); + + const names = result.toolDefinitions.map((d) => d.name).sort(); + expect(names).toEqual(['bash_tool', 'read_file']); + expect(result.registered.sort()).toEqual(['bash_tool', 'read_file']); + expect(toolRegistry.has('read_file')).toBe(true); + expect(toolRegistry.has('bash_tool')).toBe(true); + }); + + it('registers read_file only when includeBash=false', () => { + const toolRegistry = makeRegistry(); + const result = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: [], + includeBash: false, + }); + + expect(result.toolDefinitions.map((d) => d.name)).toEqual(['read_file']); + expect(result.registered).toEqual(['read_file']); + expect(toolRegistry.has('read_file')).toBe(true); + expect(toolRegistry.has('bash_tool')).toBe(false); + }); + + it('preserves pre-existing unrelated tool definitions', () => { + const toolRegistry = makeRegistry(); + const existing: LCTool[] = [ + { name: 'calculator', description: 'calc', parameters: undefined } as LCTool, + ]; + const result = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: existing, + includeBash: true, + }); + + const names = result.toolDefinitions.map((d) => d.name); + expect(names).toEqual(['calculator', 'read_file', 'bash_tool']); + }); + }); + + describe('idempotence (second call in same run)', () => { + it('is a no-op when both tools already live in the registry', () => { + const toolRegistry = makeRegistry(); + const first = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: [], + includeBash: true, + }); + /* Second call simulates skills-path + execute_code-path overlap. */ + const second = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: first.toolDefinitions, + includeBash: true, + }); + + expect(second.registered).toEqual([]); + expect(second.toolDefinitions).toHaveLength(2); + const names = second.toolDefinitions.map((d) => d.name).sort(); + expect(names).toEqual(['bash_tool', 'read_file']); + }); + + it('is a no-op when tools already live in toolDefinitions (no registry available)', () => { + const existing: LCTool[] = [ + { name: 'read_file', description: 'pre', parameters: undefined } as LCTool, + { name: 'bash_tool', description: 'pre', parameters: undefined } as LCTool, + ]; + const result = registerCodeExecutionTools({ + toolRegistry: undefined, + toolDefinitions: existing, + includeBash: true, + }); + + expect(result.registered).toEqual([]); + expect(result.toolDefinitions).toEqual(existing); + }); + + it('only adds the missing half when one is already registered', () => { + const toolRegistry = makeRegistry(); + toolRegistry.set('read_file', { + name: 'read_file', + description: 'prev', + parameters: undefined, + } as LCTool); + const result = registerCodeExecutionTools({ + toolRegistry, + toolDefinitions: [], + includeBash: true, + }); + + expect(result.registered).toEqual(['bash_tool']); + const names = result.toolDefinitions.map((d) => d.name); + expect(names).toEqual(['bash_tool']); + expect(toolRegistry.has('read_file')).toBe(true); + expect(toolRegistry.has('bash_tool')).toBe(true); + }); + }); + + describe('no-registry variant', () => { + it('still returns merged toolDefinitions when toolRegistry is undefined', () => { + const result = registerCodeExecutionTools({ + toolRegistry: undefined, + toolDefinitions: [], + includeBash: true, + }); + + const names = result.toolDefinitions.map((d) => d.name).sort(); + expect(names).toEqual(['bash_tool', 'read_file']); + expect(result.registered.sort()).toEqual(['bash_tool', 'read_file']); + }); + }); +}); diff --git a/packages/api/src/agents/tools.ts b/packages/api/src/agents/tools.ts index ad1a724e4f..5582724ba9 100644 --- a/packages/api/src/agents/tools.ts +++ b/packages/api/src/agents/tools.ts @@ -1,3 +1,6 @@ +import { BashExecutionToolDefinition, ReadFileToolDefinition } from '@librechat/agents'; +import type { LCTool, LCToolRegistry } from '@librechat/agents'; + interface ToolDefLike { name: string; [key: string]: unknown; @@ -37,3 +40,98 @@ export function buildToolSet(agentConfig: BuildToolSetConfig | null | undefined) return new Set(toolNames.filter((name): name is string => Boolean(name))); } + +export interface RegisterCodeExecutionToolsParams { + toolRegistry: LCToolRegistry | undefined; + toolDefinitions: LCTool[] | undefined; + /** + * When `true`, register `bash_tool` alongside `read_file`. When `false`, + * register `read_file` only — manually-primed skills still need it to + * load `references/*` files from storage even without a sandbox. + * + * Callers: + * - `initializeAgent` passes `true` iff the `execute_code` capability + * is enabled for the run. + * - `injectSkillCatalog` passes whatever `codeEnvAvailable` resolved to + * for the run. + * + * Both callers reach this helper in the same `initializeAgent` run + * sequentially; the registry `.has()` check keeps the second call a + * no-op so there is exactly one copy of each tool in `toolDefinitions`. + */ + includeBash: boolean; +} + +export interface RegisterCodeExecutionToolsResult { + toolDefinitions: LCTool[]; + /** Tool names newly registered (skipped names that already existed). */ + registered: string[]; +} + +/** + * Hoisted module-level definitions so `registerCodeExecutionTools` doesn't + * re-allocate on every call (including the common no-op second call in the + * same run). The shapes are derived entirely from static + * `@librechat/agents` exports — no per-request state — so a single frozen + * object per tool is safe to share across every agent init. + */ +const READ_FILE_DEF: LCTool = Object.freeze({ + name: ReadFileToolDefinition.name, + description: ReadFileToolDefinition.description, + parameters: ReadFileToolDefinition.parameters as unknown as LCTool['parameters'], + responseFormat: ReadFileToolDefinition.responseFormat, +}) as LCTool; + +const BASH_TOOL_DEF: LCTool = Object.freeze({ + name: BashExecutionToolDefinition.name, + description: BashExecutionToolDefinition.description, + parameters: BashExecutionToolDefinition.schema as unknown as LCTool['parameters'], +}) as LCTool; + +/** + * Idempotently registers the skill-flavored code-execution tool pair + * (`bash_tool` + `read_file`) into the run's tool registry and + * tool-definition list. + * + * Replaces the legacy `CodeExecutionToolDefinition` / `execute_code` + * registration. `execute_code` as a capability name and as an + * `agent.tools` entry is preserved — it just expands into this tool + * pair at load time so there is only one code-execution tool path + * end-to-end (no same-run dedupe surprises for agents with both + * `execute_code` capability AND skills active). + */ +export function registerCodeExecutionTools( + params: RegisterCodeExecutionToolsParams, +): RegisterCodeExecutionToolsResult { + const { toolRegistry, toolDefinitions, includeBash } = params; + + const candidates: LCTool[] = includeBash ? [READ_FILE_DEF, BASH_TOOL_DEF] : [READ_FILE_DEF]; + + const existingNames = new Set((toolDefinitions ?? []).map((d) => d.name)); + + const registered: string[] = []; + const newDefs: LCTool[] = []; + for (const def of candidates) { + const inRegistry = toolRegistry?.has(def.name) === true; + const inDefs = existingNames.has(def.name); + if (inRegistry || inDefs) { + continue; + } + toolRegistry?.set(def.name, def); + newDefs.push(def); + registered.push(def.name); + } + + /** + * Skip the array spread on the common second-call no-op path (both tools + * already registered by the first caller in the same run). Returns the + * input array by reference; callers treat the return value as immutable. + */ + if (newDefs.length === 0) { + return { toolDefinitions: toolDefinitions ?? [], registered }; + } + return { + toolDefinitions: [...(toolDefinitions ?? []), ...newDefs], + registered, + }; +} diff --git a/packages/api/src/tools/classification.spec.ts b/packages/api/src/tools/classification.spec.ts index 2d6fe222ec..dd89f2b426 100644 --- a/packages/api/src/tools/classification.spec.ts +++ b/packages/api/src/tools/classification.spec.ts @@ -135,8 +135,6 @@ describe('classification.ts', () => { }); describe('buildToolClassification with deferredToolsEnabled', () => { - const mockLoadAuthValues = jest.fn().mockResolvedValue({}); - const createMCPTool = (name: string, description?: string) => ({ name, @@ -163,7 +161,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: false, - loadAuthValues: mockLoadAuthValues, }); expect(result.hasDeferredTools).toBe(false); @@ -184,7 +181,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: false, - loadAuthValues: mockLoadAuthValues, }); expect(result.toolRegistry).toBeDefined(); @@ -206,7 +202,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.hasDeferredTools).toBe(true); @@ -227,7 +222,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.hasDeferredTools).toBe(true); @@ -247,7 +241,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: false, - loadAuthValues: mockLoadAuthValues, }); expect(result.hasDeferredTools).toBe(false); @@ -266,7 +259,6 @@ describe('classification.ts', () => { userId: 'user1', agentId: 'agent1', agentToolOptions, - loadAuthValues: mockLoadAuthValues, }); expect(result.hasDeferredTools).toBe(true); @@ -282,7 +274,6 @@ describe('classification.ts', () => { userId: 'user1', agentId: 'agent1', deferredToolsEnabled: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.toolRegistry).toBeUndefined(); @@ -292,8 +283,6 @@ describe('classification.ts', () => { }); describe('buildToolClassification with definitionsOnly', () => { - const mockLoadAuthValues = jest.fn().mockResolvedValue({ CODE_API_KEY: 'test-key' }); - const createMCPTool = (name: string, description?: string) => ({ name, @@ -320,7 +309,6 @@ describe('classification.ts', () => { agentToolOptions, deferredToolsEnabled: true, definitionsOnly: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.additionalTools.length).toBe(0); @@ -340,7 +328,6 @@ describe('classification.ts', () => { agentToolOptions, deferredToolsEnabled: true, definitionsOnly: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.toolDefinitions.some((d) => d.name === 'tool_search')).toBe(true); @@ -361,7 +348,6 @@ describe('classification.ts', () => { agentToolOptions, deferredToolsEnabled: true, definitionsOnly: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.toolDefinitions.some((d) => d.name === 'run_tools_with_code')).toBe(true); @@ -369,46 +355,6 @@ describe('classification.ts', () => { expect(result.additionalTools.length).toBe(0); }); - it('should NOT call loadAuthValues for PTC when definitionsOnly=true', async () => { - const loadedTools: GenericTool[] = [createMCPTool('tool1')]; - - const agentToolOptions: AgentToolOptions = { - tool1: { allowed_callers: ['code_execution'] }, - }; - - await buildToolClassification({ - loadedTools, - userId: 'user1', - agentId: 'agent1', - agentToolOptions, - deferredToolsEnabled: true, - definitionsOnly: true, - loadAuthValues: mockLoadAuthValues, - }); - - expect(mockLoadAuthValues).not.toHaveBeenCalled(); - }); - - it('should call loadAuthValues for PTC when definitionsOnly=false', async () => { - const loadedTools: GenericTool[] = [createMCPTool('tool1')]; - - const agentToolOptions: AgentToolOptions = { - tool1: { allowed_callers: ['code_execution'] }, - }; - - await buildToolClassification({ - loadedTools, - userId: 'user1', - agentId: 'agent1', - agentToolOptions, - deferredToolsEnabled: true, - definitionsOnly: false, - loadAuthValues: mockLoadAuthValues, - }); - - expect(mockLoadAuthValues).toHaveBeenCalled(); - }); - it('should create tool instances when definitionsOnly=false (default)', async () => { const loadedTools: GenericTool[] = [createMCPTool('tool1')]; @@ -422,7 +368,6 @@ describe('classification.ts', () => { agentId: 'agent1', agentToolOptions, deferredToolsEnabled: true, - loadAuthValues: mockLoadAuthValues, }); expect(result.additionalTools.some((t) => t.name === 'tool_search')).toBe(true); diff --git a/packages/api/src/tools/classification.ts b/packages/api/src/tools/classification.ts index 2c65076f6f..e945b43101 100644 --- a/packages/api/src/tools/classification.ts +++ b/packages/api/src/tools/classification.ts @@ -8,7 +8,6 @@ import { logger } from '@librechat/data-schemas'; import { Constants } from 'librechat-data-provider'; import { - EnvVar, createToolSearch, ToolSearchToolDefinition, createProgrammaticToolCallingTool, @@ -188,11 +187,6 @@ export interface BuildToolClassificationParams { deferredToolsEnabled?: boolean; /** When true, skip creating tool instances (for event-driven mode) */ definitionsOnly?: boolean; - /** Function to load auth values (dependency injection) */ - loadAuthValues: (params: { - userId: string; - authFields: string[]; - }) => Promise>; } /** Result from building tool classification */ @@ -252,13 +246,11 @@ export async function buildToolClassification( params: BuildToolClassificationParams, ): Promise { const { - userId, agentId, loadedTools, agentToolOptions, definitionsOnly = false, deferredToolsEnabled = true, - loadAuthValues, } = params; const additionalTools: GenericTool[] = []; @@ -331,7 +323,6 @@ export async function buildToolClassification( logger.debug(`[buildToolClassification] Tool Search enabled for agent ${agentId}`); } - /** PTC requires CODE_API_KEY for sandbox execution */ if (!hasProgrammaticTools) { return { toolRegistry, toolDefinitions, additionalTools, hasDeferredTools }; } @@ -354,18 +345,7 @@ export async function buildToolClassification( } try { - const authValues = await loadAuthValues({ - userId, - authFields: [EnvVar.CODE_API_KEY], - }); - const codeApiKey = authValues[EnvVar.CODE_API_KEY]; - - if (!codeApiKey) { - logger.warn('[buildToolClassification] PTC configured but CODE_API_KEY not available'); - return { toolRegistry, toolDefinitions, additionalTools, hasDeferredTools }; - } - - const ptcTool = createProgrammaticToolCallingTool({ apiKey: codeApiKey }); + const ptcTool = createProgrammaticToolCallingTool({}); additionalTools.push(ptcTool); /** Add PTC definition for event-driven mode */ diff --git a/packages/api/src/tools/definitions.spec.ts b/packages/api/src/tools/definitions.spec.ts index e7ba2f5ce9..e297024ddf 100644 --- a/packages/api/src/tools/definitions.spec.ts +++ b/packages/api/src/tools/definitions.spec.ts @@ -8,7 +8,6 @@ import type { } from './definitions'; describe('definitions.ts', () => { - const mockLoadAuthValues = jest.fn().mockResolvedValue({}); const mockGetOrFetchMCPServerTools = jest.fn().mockResolvedValue(null); const mockIsBuiltInTool = jest.fn().mockReturnValue(false); @@ -27,7 +26,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -65,7 +63,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; @@ -106,7 +103,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; @@ -142,7 +138,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; @@ -165,7 +160,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; @@ -188,7 +182,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -198,7 +191,13 @@ describe('definitions.ts', () => { expect(calcDef?.parameters).toBeDefined(); }); - it('should include parameters for execute_code native tool', async () => { + it('does not resolve `execute_code` as a builtin tool definition (registered by initializeAgent instead)', async () => { + /* Phase 8: the legacy `CodeExecutionToolDefinition` is no longer in + the registry. `execute_code` stays in `agent.tools` as the + capability-trigger marker, but its tool definitions (`bash_tool` + + `read_file`) are added by `registerCodeExecutionTools` during + `initializeAgent` — not here. `loadToolDefinitions` must silently + drop the name so nothing shadows that path. */ mockIsBuiltInTool.mockImplementation((name) => name === 'execute_code'); const params: LoadToolDefinitionsParams = { @@ -210,18 +209,13 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); const execCodeDef = result.toolDefinitions.find((d) => d.name === 'execute_code'); - expect(execCodeDef).toBeDefined(); - expect(execCodeDef?.parameters).toBeDefined(); - expect(execCodeDef?.parameters?.properties).toHaveProperty('lang'); - expect(execCodeDef?.parameters?.properties).toHaveProperty('code'); - expect(execCodeDef?.parameters?.required).toContain('lang'); - expect(execCodeDef?.parameters?.required).toContain('code'); + expect(execCodeDef).toBeUndefined(); + expect(result.toolRegistry.has('execute_code')).toBe(false); }); it('should include parameters for web_search native tool', async () => { @@ -236,7 +230,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -260,7 +253,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -284,7 +276,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -306,7 +297,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -360,7 +350,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -419,7 +408,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -463,7 +451,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -495,7 +482,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -522,7 +508,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -548,7 +533,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, }; const result = await loadToolDefinitions(params, deps); @@ -603,7 +587,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; @@ -636,7 +619,6 @@ describe('definitions.ts', () => { const deps: LoadToolDefinitionsDeps = { getOrFetchMCPServerTools: mockGetOrFetchMCPServerTools, isBuiltInTool: mockIsBuiltInTool, - loadAuthValues: mockLoadAuthValues, getActionToolDefinitions: mockGetActionToolDefinitions, }; diff --git a/packages/api/src/tools/definitions.ts b/packages/api/src/tools/definitions.ts index 8ca60e9aaf..d56c299304 100644 --- a/packages/api/src/tools/definitions.ts +++ b/packages/api/src/tools/definitions.ts @@ -48,11 +48,6 @@ export interface LoadToolDefinitionsDeps { getOrFetchMCPServerTools: (userId: string, serverName: string) => Promise; /** Checks if a tool name is a known built-in tool */ isBuiltInTool: (toolName: string) => boolean; - /** Loads auth values for tool search (passed to buildToolClassification) */ - loadAuthValues: (params: { - userId: string; - authFields: string[]; - }) => Promise>; /** Loads action tool definitions (schemas) from OpenAPI specs */ getActionToolDefinitions?: ( agentId: string, @@ -77,8 +72,7 @@ export async function loadToolDefinitions( deps: LoadToolDefinitionsDeps, ): Promise { const { userId, agentId, tools, toolOptions = {}, deferredToolsEnabled = false } = params; - const { getOrFetchMCPServerTools, isBuiltInTool, loadAuthValues, getActionToolDefinitions } = - deps; + const { getOrFetchMCPServerTools, isBuiltInTool, getActionToolDefinitions } = deps; const emptyResult: LoadToolDefinitionsResult = { toolDefinitions: [], @@ -196,7 +190,6 @@ export async function loadToolDefinitions( userId, agentId, loadedTools, - loadAuthValues, deferredToolsEnabled, definitionsOnly: true, agentToolOptions: toolOptions, diff --git a/packages/api/src/tools/registry/definitions.ts b/packages/api/src/tools/registry/definitions.ts index b0d03199bd..5e953ce547 100644 --- a/packages/api/src/tools/registry/definitions.ts +++ b/packages/api/src/tools/registry/definitions.ts @@ -1,8 +1,4 @@ -import { - WebSearchToolDefinition, - CalculatorToolDefinition, - CodeExecutionToolDefinition, -} from '@librechat/agents'; +import { WebSearchToolDefinition, CalculatorToolDefinition } from '@librechat/agents'; import { geminiToolkit } from '~/tools/toolkits/gemini'; import { oaiToolkit } from '~/tools/toolkits/oai'; @@ -451,7 +447,17 @@ export const toolDefinitions: Record = { }, }; -/** Tool definitions from @librechat/agents */ +/** + * Tool definitions from @librechat/agents. + * + * `CodeExecutionToolDefinition` (the legacy `execute_code` tool) is + * intentionally absent — the `execute_code` capability now expands into + * the skill-flavored `bash_tool` + `read_file` pair, registered at + * initialize-time by `registerCodeExecutionTools`. Agents whose `tools` + * array contains the literal string `execute_code` continue to work: + * the capability gate still filters on that string, and the runtime + * registers the tool pair on match. + */ const agentToolDefinitions: Record = { [CalculatorToolDefinition.name]: { name: CalculatorToolDefinition.name, @@ -459,12 +465,6 @@ const agentToolDefinitions: Record = { schema: CalculatorToolDefinition.schema as unknown as ExtendedJsonSchema, toolType: 'builtin', }, - [CodeExecutionToolDefinition.name]: { - name: CodeExecutionToolDefinition.name, - description: CodeExecutionToolDefinition.description, - schema: CodeExecutionToolDefinition.schema as unknown as ExtendedJsonSchema, - toolType: 'builtin', - }, [WebSearchToolDefinition.name]: { name: WebSearchToolDefinition.name, description: WebSearchToolDefinition.description,