From ac4049edbbdab259e8c471ff11f3cebbd19b5c9f Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 1 Jun 2026 21:37:58 -0400 Subject: [PATCH] refactor: deepen skill authoring runtime wiring --- .../agents/__tests__/callbacks.spec.js | 3 + .../agents/__tests__/openai.spec.js | 56 ++++++++--- .../agents/__tests__/responses.unit.spec.js | 56 ++++++++--- api/server/controllers/agents/callbacks.js | 4 +- api/server/controllers/agents/openai.js | 56 ++--------- api/server/controllers/agents/responses.js | 67 +++---------- .../services/Endpoints/agents/initialize.js | 94 +++---------------- .../services/Endpoints/agents/skillDeps.js | 72 +++++++++++++- packages/api/src/agents/handlers.ts | 9 +- .../api/src/agents/skillConfigurable.spec.ts | 65 +++++++------ packages/api/src/agents/skillConfigurable.ts | 80 +++++++++++++--- packages/api/src/agents/tools.spec.ts | 10 +- packages/api/src/agents/tools.ts | 19 ++-- 13 files changed, 316 insertions(+), 275 deletions(-) diff --git a/api/server/controllers/agents/__tests__/callbacks.spec.js b/api/server/controllers/agents/__tests__/callbacks.spec.js index eb278e0823..65bcfccfe6 100644 --- a/api/server/controllers/agents/__tests__/callbacks.spec.js +++ b/api/server/controllers/agents/__tests__/callbacks.spec.js @@ -7,6 +7,9 @@ jest.mock('nanoid', () => ({ jest.mock('@librechat/api', () => ({ sendEvent: jest.fn(), + isCodeSessionToolName: jest.fn((name) => + ['execute_code', 'bash_tool', 'read_file', 'create_file', 'edit_file'].includes(name), + ), })); jest.mock('@librechat/data-schemas', () => ({ diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index b51e49a3b5..25a0c00c72 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -22,6 +22,34 @@ const mockBuildSkillPrimedIdsByName = jest.fn((manualSkillPrimes, alwaysApplySki return Object.keys(primed).length > 0 ? primed : undefined; }); const mockEnrichWithSkillConfigurable = jest.fn((result) => result); +const mockBuildAgentToolContext = jest.fn(({ agent, config }) => ({ + agent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + accessibleSkillIds: config.accessibleSkillIds, + activeSkillNames: config.activeSkillNames, + codeEnvAvailable: config.codeEnvAvailable, + skillAuthoringAvailable: config.skillAuthoringAvailable, + fileAuthoringToolNames: config.fileAuthoringToolNames, + skillPrimedIdsByName: + mockBuildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) ?? {}, +})); +const mockEnrichLoadedToolsWithAgentContext = jest.fn(({ result, req, ctx }) => + mockEnrichWithSkillConfigurable({ + result, + context: { + req, + accessibleSkillIds: ctx.accessibleSkillIds, + codeEnvAvailable: ctx.codeEnvAvailable === true, + skillPrimedIdsByName: ctx.skillPrimedIdsByName, + activeSkillNames: ctx.activeSkillNames, + skillAuthoringAvailable: ctx.skillAuthoringAvailable === true, + fileAuthoringToolNames: ctx.fileAuthoringToolNames, + }, + }), +); const mockGetSkillToolDeps = jest.fn(() => ({})); jest.mock('nanoid', () => ({ @@ -120,6 +148,8 @@ jest.mock('~/server/services/Endpoints/agents/skillDeps', () => ({ getSkillToolDeps: mockGetSkillToolDeps, enrichWithSkillConfigurable: mockEnrichWithSkillConfigurable, buildSkillPrimedIdsByName: mockBuildSkillPrimedIdsByName, + buildAgentToolContext: mockBuildAgentToolContext, + enrichLoadedToolsWithAgentContext: mockEnrichLoadedToolsWithAgentContext, })); jest.mock('~/cache', () => ({ @@ -452,19 +482,21 @@ describe('OpenAIChatCompletionController', () => { actionsEnabled: true, }), ); - expect(mockEnrichWithSkillConfigurable).toHaveBeenLastCalledWith( - expect.anything(), - req, - ['sub-skill-id'], - true, - { - 'sub-always-skill': 'sub-always-id', - 'sub-hidden-skill': 'sub-manual-id', + expect(mockEnrichWithSkillConfigurable).toHaveBeenLastCalledWith({ + result: expect.anything(), + context: { + req, + accessibleSkillIds: ['sub-skill-id'], + codeEnvAvailable: true, + skillPrimedIdsByName: { + 'sub-always-skill': 'sub-always-id', + 'sub-hidden-skill': 'sub-manual-id', + }, + activeSkillNames: ['sub-hidden-skill'], + skillAuthoringAvailable: true, + fileAuthoringToolNames: ['create_file', 'edit_file'], }, - ['sub-hidden-skill'], - true, - ['create_file', 'edit_file'], - ); + }); }); }); }); diff --git a/api/server/controllers/agents/__tests__/responses.unit.spec.js b/api/server/controllers/agents/__tests__/responses.unit.spec.js index 64b80a975c..2667790845 100644 --- a/api/server/controllers/agents/__tests__/responses.unit.spec.js +++ b/api/server/controllers/agents/__tests__/responses.unit.spec.js @@ -21,6 +21,34 @@ const mockBuildSkillPrimedIdsByName = jest.fn((manualSkillPrimes, alwaysApplySki return Object.keys(primed).length > 0 ? primed : undefined; }); const mockEnrichWithSkillConfigurable = jest.fn((result) => result); +const mockBuildAgentToolContext = jest.fn(({ agent, config }) => ({ + agent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + accessibleSkillIds: config.accessibleSkillIds, + activeSkillNames: config.activeSkillNames, + codeEnvAvailable: config.codeEnvAvailable, + skillAuthoringAvailable: config.skillAuthoringAvailable, + fileAuthoringToolNames: config.fileAuthoringToolNames, + skillPrimedIdsByName: + mockBuildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) ?? {}, +})); +const mockEnrichLoadedToolsWithAgentContext = jest.fn(({ result, req, ctx }) => + mockEnrichWithSkillConfigurable({ + result, + context: { + req, + accessibleSkillIds: ctx.accessibleSkillIds, + codeEnvAvailable: ctx.codeEnvAvailable === true, + skillPrimedIdsByName: ctx.skillPrimedIdsByName, + activeSkillNames: ctx.activeSkillNames, + skillAuthoringAvailable: ctx.skillAuthoringAvailable === true, + fileAuthoringToolNames: ctx.fileAuthoringToolNames, + }, + }), +); const mockGetSkillToolDeps = jest.fn(() => ({})); jest.mock('nanoid', () => ({ @@ -170,6 +198,8 @@ jest.mock('~/server/services/Endpoints/agents/skillDeps', () => ({ getSkillToolDeps: mockGetSkillToolDeps, enrichWithSkillConfigurable: mockEnrichWithSkillConfigurable, buildSkillPrimedIdsByName: mockBuildSkillPrimedIdsByName, + buildAgentToolContext: mockBuildAgentToolContext, + enrichLoadedToolsWithAgentContext: mockEnrichLoadedToolsWithAgentContext, })); jest.mock('~/cache', () => ({ @@ -542,19 +572,21 @@ describe('createResponse controller', () => { actionsEnabled: true, }), ); - expect(mockEnrichWithSkillConfigurable).toHaveBeenLastCalledWith( - expect.anything(), - req, - ['sub-skill-id'], - true, - { - 'sub-always-skill': 'sub-always-id', - 'sub-hidden-skill': 'sub-manual-id', + expect(mockEnrichWithSkillConfigurable).toHaveBeenLastCalledWith({ + result: expect.anything(), + context: { + req, + accessibleSkillIds: ['sub-skill-id'], + codeEnvAvailable: true, + skillPrimedIdsByName: { + 'sub-always-skill': 'sub-always-id', + 'sub-hidden-skill': 'sub-manual-id', + }, + activeSkillNames: ['sub-hidden-skill'], + skillAuthoringAvailable: true, + fileAuthoringToolNames: ['create_file', 'edit_file'], }, - ['sub-hidden-skill'], - true, - ['create_file', 'edit_file'], - ); + }); }); }); }); diff --git a/api/server/controllers/agents/callbacks.js b/api/server/controllers/agents/callbacks.js index 3cf862821e..b4cc8bd0f2 100644 --- a/api/server/controllers/agents/callbacks.js +++ b/api/server/controllers/agents/callbacks.js @@ -5,7 +5,6 @@ const { GraphEvents, GraphNodeKeys, ToolEndHandler, - CODE_EXECUTION_TOOLS, createContentAggregator, } = require('@librechat/agents'); const { @@ -13,13 +12,14 @@ const { GenerationJobManager, writeAttachmentEvent, createToolExecuteHandler, + isCodeSessionToolName, } = require('@librechat/api'); const { processFileCitations } = require('~/server/services/Files/Citations'); const { processCodeOutput, runPreviewFinalize } = require('~/server/services/Files/Code/process'); const { saveBase64Image } = require('~/server/services/Files/process'); function isCodeArtifactToolName(name) { - return CODE_EXECUTION_TOOLS.has(name) || name === 'create_file' || name === 'edit_file'; + return isCodeSessionToolName(name); } class ModelEndHandler { diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 5e5c435b28..e59e5f8b9b 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -48,8 +48,8 @@ const { } = require('~/server/services/PermissionService'); const { getSkillToolDeps, - enrichWithSkillConfigurable, - buildSkillPrimedIdsByName, + buildAgentToolContext, + enrichLoadedToolsWithAgentContext, } = require('~/server/services/Endpoints/agents/skillDeps'); const { getModelsConfig } = require('~/server/controllers/ModelController'); const { logViolation } = require('~/cache'); @@ -346,26 +346,11 @@ const OpenAIChatCompletionController = async (req, res) => { * actionsEnabled?: boolean, * }>} */ - const skillPrimedIdsByName = - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ) ?? {}; - const agentToolContexts = new Map(); - agentToolContexts.set(primaryConfig.id, { - agent, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, - accessibleSkillIds: primaryConfig.accessibleSkillIds, - activeSkillNames: primaryConfig.activeSkillNames, - codeEnvAvailable: primaryConfig.codeEnvAvailable, - skillAuthoringAvailable: primaryConfig.skillAuthoringAvailable, - fileAuthoringToolNames: primaryConfig.fileAuthoringToolNames, - skillPrimedIdsByName, - }); + agentToolContexts.set( + primaryConfig.id, + buildAgentToolContext({ agent, config: primaryConfig }), + ); // Only run BFS discovery (and pay `getModelsConfig` upfront) when the // primary has edges to follow — the common API case is single-agent. @@ -433,23 +418,7 @@ const OpenAIChatCompletionController = async (req, res) => { logViolation, db: dbMethods, onAgentInitialized: (agentId, handoffAgent, config) => { - agentToolContexts.set(agentId, { - agent: handoffAgent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - accessibleSkillIds: config.accessibleSkillIds, - activeSkillNames: config.activeSkillNames, - codeEnvAvailable: config.codeEnvAvailable, - skillAuthoringAvailable: config.skillAuthoringAvailable, - fileAuthoringToolNames: config.fileAuthoringToolNames, - skillPrimedIdsByName: - buildSkillPrimedIdsByName( - config.manualSkillPrimes, - config.alwaysApplySkillPrimes, - ) ?? {}, - }); + agentToolContexts.set(agentId, buildAgentToolContext({ agent: handoffAgent, config })); }, initializeAgent, }, @@ -515,16 +484,11 @@ const OpenAIChatCompletionController = async (req, res) => { tool_resources: ctx.tool_resources, actionsEnabled: ctx.actionsEnabled, }); - return enrichWithSkillConfigurable( + return enrichLoadedToolsWithAgentContext({ result, req, - ctx.accessibleSkillIds ?? primaryConfig.accessibleSkillIds, - ctx.codeEnvAvailable === true, - ctx.skillPrimedIdsByName ?? skillPrimedIdsByName, - ctx.activeSkillNames ?? primaryConfig.activeSkillNames, - ctx.skillAuthoringAvailable === true, - ctx.fileAuthoringToolNames ?? primaryConfig.fileAuthoringToolNames, - ); + ctx, + }); }, toolEndCallback, ...getSkillToolDeps(), diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 865b8e504f..f7e8796f93 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -57,8 +57,8 @@ const { } = require('~/server/services/PermissionService'); const { getSkillToolDeps, - enrichWithSkillConfigurable, - buildSkillPrimedIdsByName, + buildAgentToolContext, + enrichLoadedToolsWithAgentContext, } = require('~/server/services/Endpoints/agents/skillDeps'); const { getModelsConfig } = require('~/server/controllers/ModelController'); const { logViolation } = require('~/cache'); @@ -476,26 +476,11 @@ const createResponse = async (req, res) => { * actionsEnabled?: boolean, * }>} */ - const skillPrimedIdsByName = - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ) ?? {}; - const agentToolContexts = new Map(); - agentToolContexts.set(primaryConfig.id, { - agent, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, - accessibleSkillIds: primaryConfig.accessibleSkillIds, - activeSkillNames: primaryConfig.activeSkillNames, - codeEnvAvailable: primaryConfig.codeEnvAvailable, - skillAuthoringAvailable: primaryConfig.skillAuthoringAvailable, - fileAuthoringToolNames: primaryConfig.fileAuthoringToolNames, - skillPrimedIdsByName, - }); + agentToolContexts.set( + primaryConfig.id, + buildAgentToolContext({ agent, config: primaryConfig }), + ); // Only run BFS discovery (and pay `getModelsConfig` upfront) when the // primary has edges to follow — the common API case is single-agent. @@ -563,23 +548,7 @@ const createResponse = async (req, res) => { logViolation, db: dbMethods, onAgentInitialized: (agentId, handoffAgent, config) => { - agentToolContexts.set(agentId, { - agent: handoffAgent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - accessibleSkillIds: config.accessibleSkillIds, - activeSkillNames: config.activeSkillNames, - codeEnvAvailable: config.codeEnvAvailable, - skillAuthoringAvailable: config.skillAuthoringAvailable, - fileAuthoringToolNames: config.fileAuthoringToolNames, - skillPrimedIdsByName: - buildSkillPrimedIdsByName( - config.manualSkillPrimes, - config.alwaysApplySkillPrimes, - ) ?? {}, - }); + agentToolContexts.set(agentId, buildAgentToolContext({ agent: handoffAgent, config })); }, initializeAgent, }, @@ -707,16 +676,11 @@ const createResponse = async (req, res) => { tool_resources: ctx.tool_resources, actionsEnabled: ctx.actionsEnabled, }); - return enrichWithSkillConfigurable( + return enrichLoadedToolsWithAgentContext({ result, req, - ctx.accessibleSkillIds ?? primaryConfig.accessibleSkillIds, - ctx.codeEnvAvailable === true, - ctx.skillPrimedIdsByName ?? skillPrimedIdsByName, - ctx.activeSkillNames ?? primaryConfig.activeSkillNames, - ctx.skillAuthoringAvailable === true, - ctx.fileAuthoringToolNames ?? primaryConfig.fileAuthoringToolNames, - ); + ctx, + }); }, toolEndCallback, ...getSkillToolDeps(), @@ -886,16 +850,11 @@ const createResponse = async (req, res) => { tool_resources: ctx.tool_resources, actionsEnabled: ctx.actionsEnabled, }); - return enrichWithSkillConfigurable( + return enrichLoadedToolsWithAgentContext({ result, req, - ctx.accessibleSkillIds ?? primaryConfig.accessibleSkillIds, - ctx.codeEnvAvailable === true, - ctx.skillPrimedIdsByName ?? skillPrimedIdsByName, - ctx.activeSkillNames ?? primaryConfig.activeSkillNames, - ctx.skillAuthoringAvailable === true, - ctx.fileAuthoringToolNames ?? primaryConfig.fileAuthoringToolNames, - ); + ctx, + }); }, toolEndCallback, ...getSkillToolDeps(), diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 5e757817a0..424fcb8cc2 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -31,8 +31,8 @@ const { loadAgentTools, loadToolsForExecution } = require('~/server/services/Too const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions'); const { getSkillToolDeps, - enrichWithSkillConfigurable, - buildSkillPrimedIdsByName, + buildAgentToolContext, + enrichLoadedToolsWithAgentContext, } = require('./skillDeps'); const { getModelsConfig } = require('~/server/controllers/ModelController'); const { checkPermission, findAccessibleResources } = require('~/server/services/PermissionService'); @@ -222,16 +222,11 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { * 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( + return enrichLoadedToolsWithAgentContext({ result, req, - ctx.accessibleSkillIds, - ctx.codeEnvAvailable === true, - ctx.skillPrimedIdsByName, - ctx.activeSkillNames, - ctx.skillAuthoringAvailable === true, - ctx.fileAuthoringToolNames, - ); + ctx, + }); }, toolEndCallback, ...getSkillToolDeps(), @@ -361,30 +356,10 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { logger.debug( `[initializeClient] Storing tool context for ${primaryConfig.id}: ${primaryConfig.toolDefinitions?.length ?? 0} tools, registry size: ${primaryConfig.toolRegistry?.size ?? '0'}`, ); - /** Maps each primed skill name (manual `$` or always-apply) to the - * `_id` of the exact doc that was primed. Plumbed to - * `enrichWithSkillConfigurable` so the read_file handler can pin - * same-name collision lookups to the resolver's chosen doc AND relax - * the disable-model-invocation gate for skills whose body is already - * in this turn's context. */ - const skillPrimedIdsByName = - buildSkillPrimedIdsByName( - primaryConfig.manualSkillPrimes, - primaryConfig.alwaysApplySkillPrimes, - ) ?? {}; - agentToolContexts.set(primaryConfig.id, { - agent: primaryAgent, - toolRegistry: primaryConfig.toolRegistry, - userMCPAuthMap: primaryConfig.userMCPAuthMap, - tool_resources: primaryConfig.tool_resources, - actionsEnabled: primaryConfig.actionsEnabled, - accessibleSkillIds: primaryConfig.accessibleSkillIds, - activeSkillNames: primaryConfig.activeSkillNames, - codeEnvAvailable: primaryConfig.codeEnvAvailable, - skillAuthoringAvailable: primaryConfig.skillAuthoringAvailable, - fileAuthoringToolNames: primaryConfig.fileAuthoringToolNames, - skillPrimedIdsByName, - }); + agentToolContexts.set( + primaryConfig.id, + buildAgentToolContext({ agent: primaryAgent, config: primaryConfig }), + ); const { agentConfigs: discoveredConfigs, @@ -448,29 +423,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { // set. The per-agent tool context map is OK to keep populated even // for pruned ids: it's only read by closure in ON_TOOL_EXECUTE, // stale entries are unreachable at runtime. - // - // Handoff agents get the same `skillPrimedIdsByName` plumbing as the - // primary so `read_file` can pin same-name collisions to the exact - // primed doc AND relax the `disable-model-invocation: true` gate for - // skills whose body is already in this turn's context — matters for - // handoff agents that have their own always-apply skills bound or - // that the user `$`-invokes within the handoff flow. onAgentInitialized: (agentId, agent, config) => { - agentToolContexts.set(agentId, { - agent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - accessibleSkillIds: config.accessibleSkillIds, - activeSkillNames: config.activeSkillNames, - codeEnvAvailable: config.codeEnvAvailable, - skillAuthoringAvailable: config.skillAuthoringAvailable, - fileAuthoringToolNames: config.fileAuthoringToolNames, - skillPrimedIdsByName: - buildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) ?? - {}, - }); + agentToolContexts.set(agentId, buildAgentToolContext({ agent, config })); }, // Pass through the `@librechat/api` exports so that tests which // `jest.mock('@librechat/api')` can override the initializer/validator. @@ -517,18 +471,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { if (agentToolContexts.has(agentId)) { continue; } - agentToolContexts.set(agentId, { - agent: config, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - accessibleSkillIds: config.accessibleSkillIds, - activeSkillNames: config.activeSkillNames, - codeEnvAvailable: config.codeEnvAvailable, - skillAuthoringAvailable: config.skillAuthoringAvailable, - fileAuthoringToolNames: config.fileAuthoringToolNames, - }); + agentToolContexts.set(agentId, buildAgentToolContext({ agent: config, config })); } // `discoverConnectedAgents` always returns a concrete array, so no @@ -670,20 +613,7 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { }, ); agentConfigs.set(agentId, config); - agentToolContexts.set(agentId, { - agent, - toolRegistry: config.toolRegistry, - userMCPAuthMap: config.userMCPAuthMap, - tool_resources: config.tool_resources, - actionsEnabled: config.actionsEnabled, - accessibleSkillIds: config.accessibleSkillIds, - activeSkillNames: config.activeSkillNames, - codeEnvAvailable: config.codeEnvAvailable, - skillAuthoringAvailable: config.skillAuthoringAvailable, - fileAuthoringToolNames: config.fileAuthoringToolNames, - skillPrimedIdsByName: - buildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) ?? {}, - }); + agentToolContexts.set(agentId, buildAgentToolContext({ agent, config })); return config; } catch (err) { logger.error(`[processAgent] Error processing subagent ${agentId}:`, err); diff --git a/api/server/services/Endpoints/agents/skillDeps.js b/api/server/services/Endpoints/agents/skillDeps.js index ebdf7bfd2b..5df986304b 100644 --- a/api/server/services/Endpoints/agents/skillDeps.js +++ b/api/server/services/Endpoints/agents/skillDeps.js @@ -129,10 +129,9 @@ function grantSkillOwner({ req, skillId }) { } /** - * Builds the `skillPrimedIdsByName` map passed through to - * `enrichWithSkillConfigurable`. Centralized here so the four CJS call - * sites (`initialize.js`, `responses.js` x2, `openai.js`) share one - * source of truth — if `ResolvedManualSkill` ever renames `_id` or + * Builds the `skillPrimedIdsByName` map threaded through + * `buildAgentToolContext`. Centralized here so every runtime route shares + * one source of truth — if `ResolvedManualSkill` ever renames `_id` or * gains new identifying fields, only this helper changes. * * Combines both manual (`$`-popover) primes AND always-apply primes so @@ -179,6 +178,69 @@ function buildSkillPrimedIdsByName(manualSkillPrimes, alwaysApplySkillPrimes) { return out; } +/** + * Builds the per-agent context consumed by ON_TOOL_EXECUTE. Keeping this + * shape in one Adapter gives every runtime path the same configurable + * fields and the same primed-skill pinning behavior. + * + * @param {object} params + * @param {object} params.agent + * @param {object} params.config + * @returns {object} + */ +function buildAgentToolContext({ agent, config }) { + return { + agent, + toolRegistry: config.toolRegistry, + userMCPAuthMap: config.userMCPAuthMap, + tool_resources: config.tool_resources, + actionsEnabled: config.actionsEnabled, + accessibleSkillIds: config.accessibleSkillIds, + activeSkillNames: config.activeSkillNames, + codeEnvAvailable: config.codeEnvAvailable, + skillAuthoringAvailable: config.skillAuthoringAvailable, + fileAuthoringToolNames: config.fileAuthoringToolNames, + skillPrimedIdsByName: + buildSkillPrimedIdsByName(config.manualSkillPrimes, config.alwaysApplySkillPrimes) ?? {}, + }; +} + +function hasOwn(value, key) { + return Object.prototype.hasOwnProperty.call(value ?? {}, key); +} + +/** + * Applies per-agent runtime context to a loadToolsForExecution result. + * + * @param {object} params + * @param {{ loadedTools: unknown[], configurable?: Record }} params.result + * @param {object} params.req + * @param {object | undefined} params.ctx + * @param {object | undefined} [params.fallback] + * @returns {{ loadedTools: unknown[], configurable: Record }} + */ +function enrichLoadedToolsWithAgentContext({ result, req, ctx = {}, fallback = {} }) { + const codeEnvAvailable = hasOwn(ctx, 'codeEnvAvailable') + ? ctx.codeEnvAvailable === true + : fallback.codeEnvAvailable === true; + const skillAuthoringAvailable = hasOwn(ctx, 'skillAuthoringAvailable') + ? ctx.skillAuthoringAvailable === true + : fallback.skillAuthoringAvailable === true; + + return enrichWithSkillConfigurable({ + result, + context: { + req, + codeEnvAvailable, + accessibleSkillIds: ctx.accessibleSkillIds ?? fallback.accessibleSkillIds, + skillPrimedIdsByName: ctx.skillPrimedIdsByName ?? fallback.skillPrimedIdsByName, + activeSkillNames: ctx.activeSkillNames ?? fallback.activeSkillNames, + skillAuthoringAvailable, + fileAuthoringToolNames: ctx.fileAuthoringToolNames ?? fallback.fileAuthoringToolNames, + }, + }); +} + /** Skill-related properties for ToolExecuteOptions (stable references, allocated once). */ const skillToolDeps = { getSkillByName: db.getSkillByName, @@ -217,4 +279,6 @@ module.exports = { getSkillToolDeps, enrichWithSkillConfigurable, buildSkillPrimedIdsByName, + buildAgentToolContext, + enrichLoadedToolsWithAgentContext, }; diff --git a/packages/api/src/agents/handlers.ts b/packages/api/src/agents/handlers.ts index 09d8be0b4a..664d0f8c86 100644 --- a/packages/api/src/agents/handlers.ts +++ b/packages/api/src/agents/handlers.ts @@ -1,7 +1,7 @@ import yaml from 'js-yaml'; import { Types } from 'mongoose'; import { logger } from '@librechat/data-schemas'; -import { GraphEvents, Constants, CODE_EXECUTION_TOOLS } from '@librechat/agents'; +import { GraphEvents, Constants } from '@librechat/agents'; import type { LCTool, EventHandler, @@ -19,6 +19,7 @@ import { logAxiosError, runOutsideTracing } from '~/utils'; import { buildSkillPrimeMessage } from './skills'; import { cleanCodeToolOutput } from './cleanup'; import { primeSkillFiles } from './skillFiles'; +import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME, isCodeSessionToolName } from './tools'; import { parseFrontmatter } from '../skills/import'; export interface ToolEndCallbackData { @@ -238,8 +239,6 @@ const MAX_CACHE_BYTES = 512 * 1024; const MAX_AUTHORING_BYTES = 10 * 1024 * 1024; const MAX_TOOL_ERROR_MESSAGE_CHARS = 12_000; const MAX_TOOL_ERROR_STACK_CHARS = 4_000; -const CREATE_FILE_TOOL_NAME = 'create_file'; -const EDIT_FILE_TOOL_NAME = 'edit_file'; const SKILL_FILE_PREFIX = 'skills/'; const SKILL_MD = 'SKILL.md'; @@ -2904,7 +2903,7 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand turn: tc.turn, }; - if (tc.codeSessionContext && CODE_EXECUTION_TOOLS.has(tc.name)) { + if (tc.codeSessionContext && isCodeSessionToolName(tc.name)) { toolCallConfig.session_id = tc.codeSessionContext.session_id; if (tc.codeSessionContext.files && tc.codeSessionContext.files.length > 0) { toolCallConfig._injected_files = tc.codeSessionContext.files; @@ -3012,7 +3011,7 @@ export function createToolExecuteHandler(options: ToolExecuteOptions): EventHand // (model context, SSE forwarding, persistence) see it. // Non-code-execution tools pass through unchanged. const cleanedContent = - CODE_EXECUTION_TOOLS.has(tc.name) && typeof result.content === 'string' + isCodeSessionToolName(tc.name) && typeof result.content === 'string' ? cleanCodeToolOutput(result.content) : result.content; diff --git a/packages/api/src/agents/skillConfigurable.spec.ts b/packages/api/src/agents/skillConfigurable.spec.ts index 8c08df1625..94bc5dbd28 100644 --- a/packages/api/src/agents/skillConfigurable.spec.ts +++ b/packages/api/src/agents/skillConfigurable.spec.ts @@ -5,12 +5,10 @@ describe('enrichWithSkillConfigurable', () => { const accessibleSkillIds = ['skill-a', 'skill-b']; it('augments configurable with req, accessibleSkillIds, and codeEnvAvailable', () => { - const result = enrichWithSkillConfigurable( - { loadedTools: [], configurable: { other: 'value' } }, - req, - accessibleSkillIds, - true, - ); + const result = enrichWithSkillConfigurable({ + result: { loadedTools: [], configurable: { other: 'value' } }, + context: { req, accessibleSkillIds, codeEnvAvailable: true }, + }); expect(result.configurable).toEqual({ other: 'value', @@ -24,17 +22,30 @@ describe('enrichWithSkillConfigurable', () => { }); it('propagates codeEnvAvailable=false verbatim (not coerced)', () => { - const result = enrichWithSkillConfigurable( - { loadedTools: [], configurable: {} }, - req, - accessibleSkillIds, - false, - ); + const result = enrichWithSkillConfigurable({ + result: { loadedTools: [], configurable: {} }, + context: { req, accessibleSkillIds, codeEnvAvailable: false }, + }); expect(result.configurable.codeEnvAvailable).toBe(false); }); it('threads skillPrimedIdsByName through unchanged', () => { + const primed = { 'brand-guidelines': 'abc123' }; + const result = enrichWithSkillConfigurable({ + result: { loadedTools: [], configurable: {} }, + context: { + req, + accessibleSkillIds, + codeEnvAvailable: true, + skillPrimedIdsByName: primed, + }, + }); + + expect(result.configurable.skillPrimedIdsByName).toBe(primed); + }); + + it('supports the legacy positional shape', () => { const primed = { 'brand-guidelines': 'abc123' }; const result = enrichWithSkillConfigurable( { loadedTools: [], configurable: {} }, @@ -49,16 +60,16 @@ describe('enrichWithSkillConfigurable', () => { it('threads skill authoring gates through unchanged', () => { const fileAuthoringToolNames = new Set(['create_file', 'edit_file']); - const result = enrichWithSkillConfigurable( - { loadedTools: [], configurable: {} }, - req, - accessibleSkillIds, - true, - undefined, - undefined, - true, - fileAuthoringToolNames, - ); + const result = enrichWithSkillConfigurable({ + result: { loadedTools: [], configurable: {} }, + context: { + req, + accessibleSkillIds, + codeEnvAvailable: true, + skillAuthoringAvailable: true, + fileAuthoringToolNames, + }, + }); expect(result.configurable.skillAuthoringAvailable).toBe(true); expect(result.configurable.fileAuthoringToolNames).toBe(fileAuthoringToolNames); @@ -66,12 +77,10 @@ describe('enrichWithSkillConfigurable', () => { it('preserves loadedTools unchanged', () => { const tools = [{ name: 'x' }]; - const result = enrichWithSkillConfigurable( - { loadedTools: tools, configurable: undefined }, - req, - accessibleSkillIds, - false, - ); + const result = enrichWithSkillConfigurable({ + result: { loadedTools: tools, configurable: undefined }, + context: { req, accessibleSkillIds, codeEnvAvailable: false }, + }); expect(result.loadedTools).toBe(tools); }); diff --git a/packages/api/src/agents/skillConfigurable.ts b/packages/api/src/agents/skillConfigurable.ts index 8ded6ee349..ada280d6a0 100644 --- a/packages/api/src/agents/skillConfigurable.ts +++ b/packages/api/src/agents/skillConfigurable.ts @@ -24,17 +24,16 @@ * Empty/missing → no exception, the gate applies as normal and the * lookup uses the full ACL set. */ -export function enrichWithSkillConfigurable( - result: { loadedTools: unknown[]; configurable?: Record }, - req: { user?: { id?: string } }, - accessibleSkillIds: unknown[], - codeEnvAvailable: boolean, +export interface SkillConfigurableContext { + req: { user?: { id?: string } }; + accessibleSkillIds?: unknown[]; + 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, + skillPrimedIdsByName?: Record; /** * Names of skills the runtime can resolve, captured at agent init by * `injectSkillCatalog`. Lets `read_file` decide whether a @@ -45,23 +44,74 @@ export function enrichWithSkillConfigurable( * off, ephemeral badge off, or persisted `skills_enabled !== true`), * which is the same signal as `accessibleSkillIds.length === 0`. */ - activeSkillNames?: Set, + activeSkillNames?: Set; /** True when skills/{skillName}/... write paths are enabled for this run. */ - skillAuthoringAvailable?: boolean, + skillAuthoringAvailable?: boolean; /** Host file-authoring names registered by initializeAgent for this run. */ + fileAuthoringToolNames?: Set; +} + +export interface SkillConfigurableLoadResult { + loadedTools: unknown[]; + configurable?: Record; +} + +export interface EnrichWithSkillConfigurableParams { + result: SkillConfigurableLoadResult; + context: SkillConfigurableContext; +} + +export function enrichWithSkillConfigurable(params: EnrichWithSkillConfigurableParams): { + loadedTools: unknown[]; + configurable: Record; +}; +export function enrichWithSkillConfigurable( + result: SkillConfigurableLoadResult, + req: { user?: { id?: string } }, + accessibleSkillIds: unknown[] | undefined, + codeEnvAvailable: boolean, + skillPrimedIdsByName?: Record, + activeSkillNames?: Set, + skillAuthoringAvailable?: boolean, + fileAuthoringToolNames?: Set, +): { loadedTools: unknown[]; configurable: Record }; +export function enrichWithSkillConfigurable( + first: EnrichWithSkillConfigurableParams | SkillConfigurableLoadResult, + req?: { user?: { id?: string } }, + accessibleSkillIds?: unknown[], + codeEnvAvailable?: boolean, + skillPrimedIdsByName?: Record, + activeSkillNames?: Set, + skillAuthoringAvailable?: boolean, fileAuthoringToolNames?: Set, ): { loadedTools: unknown[]; configurable: Record } { + const { result, context } = + 'result' in first && 'context' in first + ? first + : { + result: first, + context: { + req: req ?? {}, + accessibleSkillIds, + codeEnvAvailable: codeEnvAvailable === true, + skillPrimedIdsByName, + activeSkillNames, + skillAuthoringAvailable, + fileAuthoringToolNames, + }, + }; + return { ...result, configurable: { ...result.configurable, - req, - codeEnvAvailable, - accessibleSkillIds, - skillPrimedIdsByName, - activeSkillNames, - skillAuthoringAvailable, - fileAuthoringToolNames, + req: context.req, + codeEnvAvailable: context.codeEnvAvailable, + accessibleSkillIds: context.accessibleSkillIds, + skillPrimedIdsByName: context.skillPrimedIdsByName, + activeSkillNames: context.activeSkillNames, + skillAuthoringAvailable: context.skillAuthoringAvailable, + fileAuthoringToolNames: context.fileAuthoringToolNames, }, }; } diff --git a/packages/api/src/agents/tools.spec.ts b/packages/api/src/agents/tools.spec.ts index 93708d2207..585a353cd3 100644 --- a/packages/api/src/agents/tools.spec.ts +++ b/packages/api/src/agents/tools.spec.ts @@ -47,6 +47,7 @@ import { registerCodeExecutionTools, registerFileAuthoringTools, isFileAuthoringToolDefinition, + isCodeSessionToolName, } from './tools'; describe('buildToolSet', () => { @@ -434,9 +435,12 @@ describe('registerCodeExecutionTools', () => { describe('registerFileAuthoringTools', () => { const makeRegistry = (): LCToolRegistry => new Map() as unknown as LCToolRegistry; - it('marks host-side file authoring tools as code-session-aware', () => { - expect(CODE_EXECUTION_TOOLS.has('create_file')).toBe(true); - expect(CODE_EXECUTION_TOOLS.has('edit_file')).toBe(true); + it('recognizes host-side file authoring tools as code-session-aware without mutating the shared set', () => { + expect(isCodeSessionToolName('bash_tool')).toBe(true); + expect(isCodeSessionToolName('create_file')).toBe(true); + expect(isCodeSessionToolName('edit_file')).toBe(true); + expect(CODE_EXECUTION_TOOLS.has('create_file')).toBe(false); + expect(CODE_EXECUTION_TOOLS.has('edit_file')).toBe(false); }); it('registers create_file and edit_file with skill-aware descriptions', () => { diff --git a/packages/api/src/agents/tools.ts b/packages/api/src/agents/tools.ts index b0963568f1..df43826006 100644 --- a/packages/api/src/agents/tools.ts +++ b/packages/api/src/agents/tools.ts @@ -8,19 +8,14 @@ import type { LCTool, LCToolRegistry } from '@librechat/agents'; export const CREATE_FILE_TOOL_NAME = 'create_file'; export const EDIT_FILE_TOOL_NAME = 'edit_file'; +export const FILE_AUTHORING_TOOL_NAMES: ReadonlySet = new Set([ + CREATE_FILE_TOOL_NAME, + EDIT_FILE_TOOL_NAME, +]); -/** - * `@librechat/agents` v3.2.x only treats the built-in code tools, `skill`, - * and `read_file` as code-session-aware event tools. LibreChat owns these - * host-side file-authoring definitions, so widen the shared runtime set here - * until the agents package exports first-class definitions/constants for - * `create_file`. - */ -const mutableCodeExecutionTools = CODE_EXECUTION_TOOLS as unknown as - | { add?: (name: string) => unknown } - | undefined; -mutableCodeExecutionTools?.add?.(CREATE_FILE_TOOL_NAME); -mutableCodeExecutionTools?.add?.(EDIT_FILE_TOOL_NAME); +export function isCodeSessionToolName(name: string): boolean { + return CODE_EXECUTION_TOOLS.has(name) || FILE_AUTHORING_TOOL_NAMES.has(name); +} interface ToolDefLike { name: string;