diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index aeb20fd9a202..67b0221cc67f 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -414,6 +414,16 @@ 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); + const skillPrimedIdsByName = buildSkillPrimedIdsByName( + primaryConfig.manualSkillPrimes, + primaryConfig.alwaysApplySkillPrimes, + ); + const toolExecuteOptions = { loadTools: async (toolNames, agentId) => { const ctx = agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {}; @@ -432,11 +442,8 @@ const OpenAIChatCompletionController = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - undefined, - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ), + codeEnvAvailable, + skillPrimedIdsByName, ); }, toolEndCallback, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 051c7ad48b87..2954f8973c07 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -280,6 +280,7 @@ function convertMessagesToOutputItems(messages) { * @param {import('express').Response} res */ const createResponse = async (req, res) => { + const appConfig = req.config; const requestStartTime = Date.now(); // Validate request @@ -291,7 +292,7 @@ const createResponse = async (req, res) => { const request = validation.request; const agentId = request.model; const isStreaming = request.stream === true; - const summarizationConfig = req.config?.summarization; + const summarizationConfig = appConfig?.summarization; // Look up the agent const agent = await db.getAgent({ id: agentId }); @@ -344,7 +345,7 @@ const createResponse = async (req, res) => { // Build allowed providers set const allowedProviders = new Set( - req.config?.endpoints?.[EModelEndpoint.agents]?.allowedProviders, + appConfig?.endpoints?.[EModelEndpoint.agents]?.allowedProviders, ); // Create tool loader @@ -377,7 +378,6 @@ const createResponse = async (req, res) => { getSkillByName: db.getSkillByName, }; - const appConfig = req.config; const enabledCapabilities = new Set( appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities, ); @@ -557,6 +557,16 @@ const createResponse = async (req, res) => { indexTokenCountMap = primeResult.indexTokenCountMap; } + /* 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); + const skillPrimedIdsByName = buildSkillPrimedIdsByName( + manualSkillPrimes, + alwaysApplySkillPrimes, + ); + // Create tracker for streaming or aggregator for non-streaming const tracker = actuallyStreaming ? createResponseTracker() : null; const aggregator = actuallyStreaming ? null : createResponseAggregator(); @@ -614,11 +624,8 @@ const createResponse = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - undefined, - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ), + codeEnvAvailable, + skillPrimedIdsByName, ); }, toolEndCallback, @@ -665,7 +672,7 @@ const createResponse = async (req, res) => { initialSummary, runId: responseId, summarizationConfig, - appConfig: req.config, + appConfig, signal: abortController.signal, customHandlers: handlers, requestBody: { @@ -706,8 +713,8 @@ const createResponse = async (req, res) => { }); // Record token usage against balance - const balanceConfig = getBalanceConfig(req.config); - const transactionsConfig = getTransactionsConfig(req.config); + const balanceConfig = getBalanceConfig(appConfig); + const transactionsConfig = getTransactionsConfig(appConfig); recordCollectedUsage( { spendTokens: db.spendTokens, @@ -793,11 +800,8 @@ const createResponse = async (req, res) => { result, req, primaryConfig.accessibleSkillIds, - undefined, - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ), + codeEnvAvailable, + skillPrimedIdsByName, ); }, toolEndCallback, @@ -842,7 +846,7 @@ const createResponse = async (req, res) => { initialSummary, runId: responseId, summarizationConfig, - appConfig: req.config, + appConfig, signal: abortController.signal, customHandlers: handlers, requestBody: { @@ -882,8 +886,8 @@ const createResponse = async (req, res) => { }); // Record token usage against balance - const balanceConfig = getBalanceConfig(req.config); - const transactionsConfig = getTransactionsConfig(req.config); + const balanceConfig = getBalanceConfig(appConfig); + const transactionsConfig = getTransactionsConfig(appConfig); recordCollectedUsage( { spendTokens: db.spendTokens, diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 8f9693f4d849..bfd35320fb03 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -1,5 +1,5 @@ const { logger } = require('@librechat/data-schemas'); -const { EnvVar, createContentAggregator } = require('@librechat/agents'); +const { createContentAggregator } = require('@librechat/agents'); const { scopeSkillIds, loadSkillStates, @@ -25,7 +25,6 @@ const { getDefaultHandlers, } = require('~/server/controllers/agents/callbacks'); const { loadAgentTools, loadToolsForExecution } = require('~/server/services/ToolService'); -const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { getSkillToolDeps, @@ -121,6 +120,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { * - the agent has stored skills (scoped by scopeSkillIds later). */ const enabledCapabilities = new Set(appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities); const skillsCapabilityEnabled = enabledCapabilities.has(AgentCapabilities.skills); + const codeEnvAvailable = enabledCapabilities.has(AgentCapabilities.execute_code); const ephemeralSkillsToggle = req.body?.ephemeralAgent?.skills === true; const accessibleSkillIds = skillsCapabilityEnabled @@ -139,21 +139,6 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { accessibleSkillIds, }); - // Resolve code API key once for the entire run (shared by primeInvokedSkills - // and enrichWithSkillConfigurable) to avoid redundant auth lookups. - let codeApiKey; - if (skillsCapabilityEnabled && enabledCapabilities.has(AgentCapabilities.execute_code)) { - try { - const authValues = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); - codeApiKey = authValues[EnvVar.CODE_API_KEY]; - } catch { - // non-fatal — primeInvokedSkills and enrichWithSkillConfigurable will work without it - } - } - /** * Agent context store - populated after initialization, accessed by callback via closure. * Maps agentId -> { userMCPAuthMap, agent, tool_resources, toolRegistry, openAIApiKey } @@ -191,7 +176,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { result, req, ctx.accessibleSkillIds, - codeApiKey, + codeEnvAvailable, ctx.skillPrimedIdsByName, ); }, @@ -272,7 +257,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { accessibleSkillIds, ephemeralSkillsToggle ? undefined : primaryAgent.skills, ), - codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), + codeEnvAvailable, skillStates, defaultActiveOnShare, manualSkills, @@ -482,8 +467,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { req, payload, accessibleSkillIds, - codeApiKey, - loadAuthValues, + codeEnvAvailable, ...getSkillToolDeps(), }) : undefined; diff --git a/api/server/services/Endpoints/agents/skillDeps.js b/api/server/services/Endpoints/agents/skillDeps.js index 1cd1352dc04a..a05b6c5492dd 100644 --- a/api/server/services/Endpoints/agents/skillDeps.js +++ b/api/server/services/Endpoints/agents/skillDeps.js @@ -1,7 +1,6 @@ const { getStrategyFunctions } = require('~/server/services/Files/strategies'); const { batchUploadCodeEnvFiles } = require('~/server/services/Files/Code/crud'); const { getSessionInfo, checkIfActive } = require('~/server/services/Files/Code/process'); -const { loadAuthValues } = require('~/server/services/Tools/credentials'); const { enrichWithSkillConfigurable } = require('@librechat/api'); const db = require('~/models'); @@ -73,34 +72,8 @@ function getSkillToolDeps() { return skillToolDeps; } -/** - * Wraps the TS enrichWithSkillConfigurable with the CJS loadAuthValues dependency. - * @param {object} result - The result from loadToolsForExecution - * @param {object} req - The Express request object - * @param {Array} accessibleSkillIds - Pre-computed accessible skill IDs - * @param {string} [preResolvedCodeApiKey] - Pre-resolved code API key (skips redundant lookup) - * @param {Record} [skillPrimedIdsByName] - Map of name → skill id for skills primed this turn (manual `$`-popover invocation OR always-apply). Pins same-name collision lookups in `read_file` and relaxes the disable-model-invocation gate for the primed doc. - * @returns {Promise} Augmented result with skill configurable - */ -function enrichConfigurable( - result, - req, - accessibleSkillIds, - preResolvedCodeApiKey, - skillPrimedIdsByName, -) { - return enrichWithSkillConfigurable( - result, - req, - accessibleSkillIds, - loadAuthValues, - preResolvedCodeApiKey, - skillPrimedIdsByName, - ); -} - module.exports = { getSkillToolDeps, - enrichWithSkillConfigurable: enrichConfigurable, + enrichWithSkillConfigurable, buildSkillPrimedIdsByName, }; diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 79b562caec0e..8977c89d3efe 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -1273,34 +1273,13 @@ async function loadToolsForExecution({ const isBashTool = toolNames.includes(AgentConstants.BASH_TOOL); if (isBashTool) { try { - const authValues = await loadAuthValues({ - userId: req.user.id, - authFields: [EnvVar.CODE_API_KEY], - }); - const codeApiKey = authValues[EnvVar.CODE_API_KEY]; - - if (codeApiKey) { - const bashTool = createBashExecutionTool({ apiKey: codeApiKey }); - allLoadedTools.push(bashTool); - } else { - logger.debug('[loadToolsForExecution] bash_tool requested but CODE_API_KEY not available'); - allLoadedTools.push( - toolFn( - async () => [ - 'Code execution is not available. Use the read_file tool instead.', - undefined, - ], - { - name: AgentConstants.BASH_TOOL, - description: 'Bash execution (unavailable - no code API key configured)', - schema: { type: 'object', properties: { command: { type: 'string' } } }, - responseFormat: AgentConstants.CONTENT_AND_ARTIFACT, - }, - ), - ); - } + const bashTool = createBashExecutionTool({}); + allLoadedTools.push(bashTool); } catch (error) { - logger.error('[loadToolsForExecution] Error creating bash tool:', error); + logger.error( + '[loadToolsForExecution] Failed to create bash_tool — is LIBRECHAT_CODE_API_KEY set in the server environment?', + error, + ); } } diff --git a/packages/api/src/agents/handlers.spec.ts b/packages/api/src/agents/handlers.spec.ts index 06069b4e458d..92fe256959d7 100644 --- a/packages/api/src/agents/handlers.spec.ts +++ b/packages/api/src/agents/handlers.spec.ts @@ -571,4 +571,99 @@ describe('createToolExecuteHandler', () => { expect(lookupOptions).toEqual({}); }); }); + + describe('skill tool codeEnvAvailable gate (sandbox file priming)', () => { + const { Types } = jest.requireActual('mongoose') as typeof import('mongoose'); + const SKILL_ID = new Types.ObjectId(); + + function makeSkillHandlerWithFiles(params: { + codeEnvAvailable: boolean; + listSkillFiles: jest.Mock; + batchUploadCodeEnvFiles?: jest.Mock; + }) { + const getSkillByName = jest.fn(async () => ({ + _id: SKILL_ID as unknown as never, + name: 'brand-guidelines', + body: 'skill body', + fileCount: 2, + })); + /* `loadTools` injects `codeEnvAvailable` into the returned + `configurable`, which mirrors production flow through + `enrichWithSkillConfigurable`. `req` must be present for the + priming branch to enter (the handler guards on it). */ + const req = { user: { id: 'user-1' } }; + const loadTools: ToolExecuteOptions['loadTools'] = jest.fn(async () => ({ + loadedTools: [], + configurable: { codeEnvAvailable: params.codeEnvAvailable, req }, + })); + return createToolExecuteHandler({ + loadTools, + getSkillByName, + listSkillFiles: params.listSkillFiles as unknown as ToolExecuteOptions['listSkillFiles'], + batchUploadCodeEnvFiles: (params.batchUploadCodeEnvFiles ?? + jest.fn()) as unknown as ToolExecuteOptions['batchUploadCodeEnvFiles'], + getStrategyFunctions: jest.fn() as unknown as ToolExecuteOptions['getStrategyFunctions'], + }); + } + + 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'; + const listSkillFiles = jest.fn().mockResolvedValue([]); + const handler = makeSkillHandlerWithFiles({ + codeEnvAvailable: false, + listSkillFiles, + }); + + const [result] = await invokeHandler(handler, [ + { + id: 'call_gate_off', + name: Constants.SKILL_TOOL, + args: { skillName: 'brand-guidelines' }, + }, + ]); + + expect(result.status).toBe('success'); + 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'; + const listSkillFiles = jest.fn().mockResolvedValue([]); + const handler = makeSkillHandlerWithFiles({ + codeEnvAvailable: true, + listSkillFiles, + }); + + await invokeHandler(handler, [ + { id: 'call_gate_on', name: Constants.SKILL_TOOL, args: { skillName: 'brand-guidelines' } }, + ]); + + 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 97d951666d81..2e6f06dfd246 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 { GraphEvents, Constants, CODE_EXECUTION_TOOLS } from '@librechat/agents'; +import { EnvVar, GraphEvents, Constants, CODE_EXECUTION_TOOLS } from '@librechat/agents'; import type { LCTool, EventHandler, @@ -530,15 +530,19 @@ async function handleSkillToolCall( | { session_id: string; files: Array<{ id: string; session_id: string; name: string }> } | undefined; - // Prime skill files to code env when the skill has bundled files + // 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. + const codeEnvAvailable = mergedConfigurable?.codeEnvAvailable === true; if ( + codeEnvAvailable && skill.fileCount > 0 && req && listSkillFiles && getStrategyFunctions && batchUploadCodeEnvFiles ) { - const codeApiKey = (mergedConfigurable?.codeApiKey as string) ?? ''; + const codeApiKey = process.env[EnvVar.CODE_API_KEY] ?? ''; if (codeApiKey) { try { const skillFiles = await listSkillFiles(skill._id); diff --git a/packages/api/src/agents/skillConfigurable.spec.ts b/packages/api/src/agents/skillConfigurable.spec.ts new file mode 100644 index 000000000000..8b0d695d7fc9 --- /dev/null +++ b/packages/api/src/agents/skillConfigurable.spec.ts @@ -0,0 +1,70 @@ +import { enrichWithSkillConfigurable } from './skillConfigurable'; + +describe('enrichWithSkillConfigurable', () => { + const req = { user: { id: 'user-1' } }; + const accessibleSkillIds = ['skill-a', 'skill-b']; + + it('augments configurable with req, accessibleSkillIds, and codeEnvAvailable', () => { + const result = enrichWithSkillConfigurable( + { loadedTools: [], configurable: { other: 'value' } }, + req, + accessibleSkillIds, + true, + ); + + expect(result.configurable).toEqual({ + other: 'value', + req, + codeEnvAvailable: true, + accessibleSkillIds, + skillPrimedIdsByName: undefined, + }); + }); + + it('propagates codeEnvAvailable=false verbatim (not coerced)', () => { + const result = enrichWithSkillConfigurable( + { loadedTools: [], configurable: {} }, + req, + accessibleSkillIds, + false, + ); + + 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( + { loadedTools: [], configurable: {} }, + req, + accessibleSkillIds, + true, + primed, + ); + + expect(result.configurable.skillPrimedIdsByName).toBe(primed); + }); + + it('preserves loadedTools unchanged', () => { + const tools = [{ name: 'x' }]; + const result = enrichWithSkillConfigurable( + { loadedTools: tools, configurable: undefined }, + req, + accessibleSkillIds, + false, + ); + + expect(result.loadedTools).toBe(tools); + }); +}); diff --git a/packages/api/src/agents/skillConfigurable.ts b/packages/api/src/agents/skillConfigurable.ts index 960d390a52f2..793f4fe7e61b 100644 --- a/packages/api/src/agents/skillConfigurable.ts +++ b/packages/api/src/agents/skillConfigurable.ts @@ -1,65 +1,49 @@ -import { EnvVar } from '@librechat/agents'; -import { logger } from '@librechat/data-schemas'; - /** * Augments a loadTools result with skill-specific configurable properties. - * Loads the code API key and merges it with accessibleSkillIds and the request object. + * + * `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. * * `skillPrimedIdsByName` maps each primed skill name (manual `$` or - * always-apply) to the `_id` of the exact doc whose body was primed into - * the turn. Skill-tool handlers consult it to: - * 1. Relax the `disable-model-invocation` gate on `read_file` for any - * primed skill (so a `disable-model-invocation: true` skill whose - * body is in context can still load its `references/*` / `scripts/*` - * files). Without this, a user manually invoking — or auto-priming - * — a disabled skill would get a body that references files the - * model is forbidden to open. - * 2. Constrain the lookup to the primed `_id` on same-name collisions, - * so `read_file` reads from the same doc whose body got primed - * (otherwise a newer same-name duplicate could shadow the - * resolver's pick and cause body/file mismatch within a single - * turn). + * always-apply) to the `_id` of the exact doc whose body was primed + * into the turn. Skill-tool handlers consult it to: + * 1. Relax the `disable-model-invocation` gate on `read_file` for + * any primed skill (so a `disable-model-invocation: true` skill + * whose body is in context can still load its `references/*` / + * `scripts/*` files). Without this, a user manually invoking — + * or auto-priming — a disabled skill would get a body that + * references files the model is forbidden to open. + * 2. Constrain the lookup to the primed `_id` on same-name + * collisions, so `read_file` reads from the same doc whose body + * got primed (otherwise a newer same-name duplicate could shadow + * the resolver's pick and cause body/file mismatch within a + * single turn). * Empty/missing → no exception, the gate applies as normal and the * lookup uses the full ACL set. */ -export async function enrichWithSkillConfigurable( +export function enrichWithSkillConfigurable( result: { loadedTools: unknown[]; configurable?: Record }, req: { user?: { id?: string } }, accessibleSkillIds: unknown[], - loadAuthValues: (params: { - userId: string; - authFields: string[]; - }) => Promise>, - /** Pre-resolved code API key. When provided, loadAuthValues is skipped. */ - preResolvedCodeApiKey?: string, + codeEnvAvailable: boolean, /** * `{ [skillName]: skillIdString }` for every skill primed this turn * (manual or always-apply). The id pins same-name collision lookups to * the exact doc the resolver primed and relaxes the disable-model gate. */ skillPrimedIdsByName?: Record, -): Promise<{ loadedTools: unknown[]; configurable: Record }> { - let codeApiKey: string | undefined = preResolvedCodeApiKey; - if (!codeApiKey) { - try { - const authValues = await loadAuthValues({ - userId: req.user?.id ?? '', - authFields: [EnvVar.CODE_API_KEY], - }); - codeApiKey = authValues[EnvVar.CODE_API_KEY]; - } catch (err) { - logger.debug( - '[enrichWithSkillConfigurable] loadAuthValues failed:', - err instanceof Error ? err.message : err, - ); - } - } +): { loadedTools: unknown[]; configurable: Record } { return { ...result, configurable: { ...result.configurable, req, - codeApiKey, + codeEnvAvailable, accessibleSkillIds, skillPrimedIdsByName, }, diff --git a/packages/api/src/agents/skillFiles.spec.ts b/packages/api/src/agents/skillFiles.spec.ts new file mode 100644 index 000000000000..c96428ac6ced --- /dev/null +++ b/packages/api/src/agents/skillFiles.spec.ts @@ -0,0 +1,147 @@ +jest.mock('@librechat/data-schemas', () => ({ + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, +})); + +const mockExtract = jest.fn(); +jest.mock('./run', () => ({ + extractInvokedSkillsFromPayload: (...args: unknown[]) => mockExtract(...args), +})); + +import { Readable } from 'stream'; +import { Types } from 'mongoose'; +import { primeInvokedSkills } from './skillFiles'; +import type { PrimeInvokedSkillsDeps } from './skillFiles'; + +const SKILL_ID = new Types.ObjectId(); + +function makeDeps(overrides: Partial = {}): PrimeInvokedSkillsDeps { + const listSkillFiles = jest.fn().mockResolvedValue([]); + const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({ + session_id: 'session-x', + files: [], + }); + return { + req: { user: { id: 'user-1' } } as PrimeInvokedSkillsDeps['req'], + payload: [{ role: 'assistant', content: [] }], + accessibleSkillIds: [SKILL_ID], + codeEnvAvailable: true, + getSkillByName: jest.fn().mockResolvedValue({ + _id: SKILL_ID, + name: 'brand-guidelines', + body: 'skill body', + fileCount: 2, + }), + listSkillFiles, + getStrategyFunctions: jest.fn(), + batchUploadCodeEnvFiles, + ...overrides, + }; +} + +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'; + const deps = makeDeps({ codeEnvAvailable: false }); + + 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('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'; + const deps = makeDeps({ codeEnvAvailable: true }); + + await primeInvokedSkills(deps); + + 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'; + const fileRecords = [ + { + relativePath: 'references/style.md', + filename: 'style.md', + filepath: '/storage/brand-guidelines/references/style.md', + source: 's3', + bytes: 256, + }, + ]; + const listSkillFiles = jest.fn().mockResolvedValue(fileRecords); + const getStrategyFunctions = jest.fn().mockReturnValue({ + /* A real empty Readable — matches the production contract for + `getDownloadStream` and works even if `batchUploadCodeEnvFiles` + is later replaced with a partially-real implementation that + iterates the stream. */ + getDownloadStream: jest.fn().mockResolvedValue(Readable.from(Buffer.from(''))), + }); + const batchUploadCodeEnvFiles = jest.fn().mockResolvedValue({ + session_id: 'session-42', + files: [{ fileId: 'file-1', filename: 'brand-guidelines/references/style.md' }], + }); + + const deps = makeDeps({ + codeEnvAvailable: true, + listSkillFiles, + getStrategyFunctions, + batchUploadCodeEnvFiles, + }); + + await primeInvokedSkills(deps); + + expect(batchUploadCodeEnvFiles).toHaveBeenCalledTimes(1); + const [uploadArgs] = batchUploadCodeEnvFiles.mock.calls[0]; + expect(uploadArgs.apiKey).toBe('sk-from-env'); + expect(uploadArgs.entity_id).toBe(SKILL_ID.toString()); + /* One uploaded file per `fileRecords` entry plus the synthetic + SKILL.md that `primeSkillFiles` always prepends. */ + expect(uploadArgs.files).toHaveLength(fileRecords.length + 1); + expect(uploadArgs.files.map((f: { filename: string }) => f.filename)).toEqual( + expect.arrayContaining(['brand-guidelines/SKILL.md', 'brand-guidelines/references/style.md']), + ); + }); + + it('returns {} early when no skills were invoked, regardless of capability', async () => { + mockExtract.mockReturnValue(new Set()); + const deps = makeDeps({ codeEnvAvailable: true }); + + const result = await primeInvokedSkills(deps); + + expect(result).toEqual({}); + expect(deps.getSkillByName).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/api/src/agents/skillFiles.ts b/packages/api/src/agents/skillFiles.ts index 72156fcea0f7..3e33afbbb18e 100644 --- a/packages/api/src/agents/skillFiles.ts +++ b/packages/api/src/agents/skillFiles.ts @@ -225,12 +225,10 @@ export interface PrimeInvokedSkillsDeps { /** Raw message payload (before formatAgentMessages). Used to extract invoked skill names. */ payload: Array>; accessibleSkillIds: Types.ObjectId[]; - /** Pre-resolved code API key. When provided, loadAuthValues is not called (avoids redundant lookups). */ - codeApiKey?: string; - loadAuthValues: (params: { - userId: string; - authFields: string[]; - }) => Promise>; + /** `execute_code` capability flag for the run. When false, the batch-upload + * path is skipped entirely — skill bodies still reconstruct for history + * rebuilds, but no sandbox traffic is generated. */ + codeEnvAvailable: boolean; getSkillByName: ( name: string, accessibleIds: Types.ObjectId[], @@ -270,21 +268,7 @@ export async function primeInvokedSkills( return {}; } - let apiKey = deps.codeApiKey ?? ''; - if (!apiKey) { - try { - const authValues = await deps.loadAuthValues({ - userId: deps.req.user?.id ?? '', - authFields: [EnvVar.CODE_API_KEY], - }); - apiKey = authValues[EnvVar.CODE_API_KEY] ?? ''; - } catch (err) { - logger.debug( - '[primeInvokedSkills] loadAuthValues failed:', - err instanceof Error ? err.message : err, - ); - } - } + const apiKey = deps.codeEnvAvailable ? (process.env[EnvVar.CODE_API_KEY] ?? '') : ''; const skills = new Map();