From 3e064c2f2b533db9281390db9c0a4906dcf646fe Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 16 Apr 2026 18:19:33 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=AF=20feat:=20Per-Agent=20Skill=20Sele?= =?UTF-8?q?ction=20in=20Builder=20and=20Runtime=20Scoping=20(#12689)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: per-agent skill selection in builder and runtime scoping Wire skills persistence on the Agent model and enable the skills section in the agents builder panel. At runtime, scope the skill catalog to only the skills configured on each agent (intersected with user ACL). When no skills are configured, the full user catalog is used as the default. The ephemeral chat toggle overrides per-agent scoping to provide the full catalog. * fix: add scopeSkillIds to @librechat/api mock in responses unit test The test mocks @librechat/api but was missing the newly imported scopeSkillIds, causing createResponse to throw before reaching the assertions. Added a passthrough mock that returns the input array. * fix: scope primeInvokedSkills by agent's configured skills primeInvokedSkills was receiving the full unscoped accessibleSkillIds, bypassing the per-agent skill scoping applied to initializeAgent. This allowed previously invoked skills from message history to be resolved and primed even when excluded from the agent's configured skill set. Apply the same scopeSkillIds filtering to match the initializeAgent calls, so skill resolution is consistent across catalog injection and history priming. * fix: preserve agent skills through form reset and union prime scope Two related bugs in the per-agent skill selection flow: 1. resetAgentForm dropped the persisted skills array because the generic fall-through at the end of the loop excludes object/array values. Combined with composeAgentUpdatePayload always emitting skills, this caused any save of a previously-configured agent to silently overwrite skills with an empty array. Add an explicit case for skills mirroring the agent_ids handling. 2. primeInvokedSkills processes the full conversation payload, including prior handoff-agent invocations. Scoping it to only primaryAgent.skills meant a skill invoked by a handoff agent in a prior turn could not be resolved when the current primary agent had a different scope, leaving message history reconstruction incomplete. Union the per-agent scoped accessibleSkillIds across primary plus all loaded handoff agents so any skill any active agent could invoke is resolvable from history. * fix: mark inline skill removals as dirty The inline X button on the skills list called setValue without shouldDirty: true, so removing a skill via this control did not mark the skills field as dirty in react-hook-form state. When a user removed a skill with the X button and also staged an avatar upload in the same save, isAvatarUploadOnlyDirty returned true and onSubmit short-circuited to avatar-only upload, silently dropping the PATCH that would persist the skill removal. The dialog path (SkillSelectDialog) already passes shouldDirty: true on add/remove; this aligns the inline control with that behavior. * fix: restore full ACL scope for primeInvokedSkills history reconstruction Reverting the earlier scoping of primeInvokedSkills to the active-agent union. That change conflated runtime invocation scoping (which correctly gates what the model can call now) with history reconstruction (which restores bodies the model already saw in prior turns). Per-agent scoping still applies at: - Catalog injection (injectSkillCatalog via initializeAgent) - Runtime invocation (handleSkillToolCall via enrichWithSkillConfigurable, using each agent's scoped accessibleSkillIds in agentToolContexts) History priming is a read of past context, not a grant of new capability. Scoping it causes historical skill bodies to vanish from formatAgentMessages when an agent's skills list is edited mid-conversation or when the ephemeral toggle flips, which breaks message reconstruction and drops code-env file continuity for /mnt/data/{skillName}/ references. The user's ACL-accessible set is the correct and sufficient gate for history reconstruction. * fix: close openai.js skill gap and pin undefined vs [] semantics Three related gaps surfaced in review: 1. api/server/controllers/agents/openai.js was a third skill resolution site alongside responses.js and initialize.js, but still used the old activation gate (required ephemeralAgent.skills === true) and never passed accessibleSkillIds through scopeSkillIds. Per-agent scoping silently did not apply on this route. Mirror the same pattern used in responses.js so all three routes behave identically. 2. scopeSkillIds previously collapsed undefined and [] into the same "full catalog" fallback, making it impossible for a user to express "this agent has no skills." Tighten the semantics before any data is written under the old behavior: - undefined / null = not configured, full catalog - [] = explicitly none, returns [] - non-empty = intersection with ACL-accessible set Update defaultAgentFormValues.skills from [] to undefined so a brand new agent whose skills UI was never touched does not accidentally persist "explicit none" on first save (removeNullishValues strips undefined from the payload server side). 3. Add direct unit tests for scopeSkillIds covering all five cases (undefined, null, empty, disjoint, overlap, exact match, empty accessible set). 16 tests total in skills.test.ts pass. * fix: add scopeSkillIds to @librechat/api mock in openai unit test Same pattern as the earlier responses.unit.spec.js fix: the test mocks @librechat/api with an explicit object, so each newly imported symbol must be added to the mock. Without scopeSkillIds, OpenAIChatCompletion controller throws on destructuring before reaching recordCollectedUsage, causing the token usage assertions to fail. --- .../agents/__tests__/openai.spec.js | 1 + .../agents/__tests__/responses.unit.spec.js | 1 + api/server/controllers/agents/openai.js | 13 ++-- api/server/controllers/agents/responses.js | 13 ++-- .../services/Endpoints/agents/initialize.js | 26 +++++-- .../SidePanel/Agents/AgentConfig.tsx | 37 +++++----- .../SidePanel/Agents/AgentPanel.tsx | 2 + .../SidePanel/Agents/AgentSelect.tsx | 9 +++ .../api/src/agents/__tests__/skills.test.ts | 67 +++++++++++++++++++ packages/api/src/agents/discovery.ts | 9 +++ packages/api/src/agents/skills.ts | 25 +++++++ packages/api/src/agents/validation.ts | 1 + packages/data-provider/src/schemas.ts | 6 +- packages/data-schemas/src/schema/agent.ts | 4 ++ packages/data-schemas/src/types/agent.ts | 1 + 15 files changed, 178 insertions(+), 37 deletions(-) diff --git a/api/server/controllers/agents/__tests__/openai.spec.js b/api/server/controllers/agents/__tests__/openai.spec.js index a444b18863..fd876f03ea 100644 --- a/api/server/controllers/agents/__tests__/openai.spec.js +++ b/api/server/controllers/agents/__tests__/openai.spec.js @@ -40,6 +40,7 @@ jest.mock('@librechat/api', () => ({ }), createChunk: jest.fn().mockReturnValue({}), buildToolSet: jest.fn().mockReturnValue(new Set()), + scopeSkillIds: jest.fn().mockImplementation((ids) => ids), sendFinalChunk: jest.fn(), createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }), validateRequest: jest diff --git a/api/server/controllers/agents/__tests__/responses.unit.spec.js b/api/server/controllers/agents/__tests__/responses.unit.spec.js index cc2086766f..720bd1e4f3 100644 --- a/api/server/controllers/agents/__tests__/responses.unit.spec.js +++ b/api/server/controllers/agents/__tests__/responses.unit.spec.js @@ -41,6 +41,7 @@ jest.mock('@librechat/api', () => ({ processStream: jest.fn().mockResolvedValue(undefined), }), buildToolSet: jest.fn().mockReturnValue(new Set()), + scopeSkillIds: jest.fn().mockImplementation((ids) => ids), createSafeUser: jest.fn().mockReturnValue({ id: 'user-123' }), initializeAgent: jest.fn().mockResolvedValue({ id: 'agent-123', diff --git a/api/server/controllers/agents/openai.js b/api/server/controllers/agents/openai.js index 9826adfad2..13b1fa0d72 100644 --- a/api/server/controllers/agents/openai.js +++ b/api/server/controllers/agents/openai.js @@ -13,6 +13,7 @@ const { createRun, createChunk, buildToolSet, + scopeSkillIds, sendFinalChunk, createSafeUser, validateRequest, @@ -244,10 +245,9 @@ const OpenAIChatCompletionController = async (req, res) => { }; const enabledCapabilities = new Set(agentsEConfig?.capabilities); - const ephemeralAgent = req.body?.ephemeralAgent; - const skillsEnabled = - enabledCapabilities.has(AgentCapabilities.skills) && ephemeralAgent?.skills === true; - const accessibleSkillIds = skillsEnabled + const skillsCapabilityEnabled = enabledCapabilities.has(AgentCapabilities.skills); + const ephemeralSkillsToggle = req.body?.ephemeralAgent?.skills === true; + const accessibleSkillIds = skillsCapabilityEnabled ? await findAccessibleResources({ userId: req.user.id, role: req.user.role, @@ -268,7 +268,10 @@ const OpenAIChatCompletionController = async (req, res) => { endpointOption, allowedProviders, isInitialAgent: true, - accessibleSkillIds, + accessibleSkillIds: scopeSkillIds( + accessibleSkillIds, + ephemeralSkillsToggle ? undefined : agent.skills, + ), codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), }, dbMethods, diff --git a/api/server/controllers/agents/responses.js b/api/server/controllers/agents/responses.js index 3993be345f..772b564fcc 100644 --- a/api/server/controllers/agents/responses.js +++ b/api/server/controllers/agents/responses.js @@ -12,6 +12,7 @@ const { const { createRun, buildToolSet, + scopeSkillIds, createSafeUser, initializeAgent, getBalanceConfig, @@ -373,10 +374,9 @@ const createResponse = async (req, res) => { const enabledCapabilities = new Set( appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities, ); - const ephemeralAgent = req.body?.ephemeralAgent; - const skillsEnabled = - enabledCapabilities.has(AgentCapabilities.skills) && ephemeralAgent?.skills === true; - const accessibleSkillIds = skillsEnabled + const skillsCapabilityEnabled = enabledCapabilities.has(AgentCapabilities.skills); + const ephemeralSkillsToggle = req.body?.ephemeralAgent?.skills === true; + const accessibleSkillIds = skillsCapabilityEnabled ? await findAccessibleResources({ userId: req.user.id, role: req.user.role, @@ -397,7 +397,10 @@ const createResponse = async (req, res) => { endpointOption, allowedProviders, isInitialAgent: true, - accessibleSkillIds, + accessibleSkillIds: scopeSkillIds( + accessibleSkillIds, + ephemeralSkillsToggle ? undefined : agent.skills, + ), codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), }, dbMethods, diff --git a/api/server/services/Endpoints/agents/initialize.js b/api/server/services/Endpoints/agents/initialize.js index 1cabe402fc..08a4eb03ac 100644 --- a/api/server/services/Endpoints/agents/initialize.js +++ b/api/server/services/Endpoints/agents/initialize.js @@ -1,6 +1,7 @@ const { logger } = require('@librechat/data-schemas'); const { EnvVar, createContentAggregator } = require('@librechat/agents'); const { + scopeSkillIds, initializeAgent, primeInvokedSkills, validateAgentModel, @@ -107,11 +108,12 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { const toolEndCallback = createToolEndCallback({ req, res, artifactPromises, streamId }); /** Query accessible skill IDs once per run (shared across all agents). - * Requires both admin capability AND per-conversation toggle (if ephemeral). */ + * Skills activate when the admin capability is enabled AND either: + * - the per-conversation toggle is on (ephemeral), OR + * - the agent has stored skills (scoped by scopeSkillIds later). */ const enabledCapabilities = new Set(appConfig?.endpoints?.[EModelEndpoint.agents]?.capabilities); - const ephemeralSkillsToggle = req.body?.ephemeralAgent?.skills; - const skillsCapabilityEnabled = - enabledCapabilities.has(AgentCapabilities.skills) && ephemeralSkillsToggle === true; + const skillsCapabilityEnabled = enabledCapabilities.has(AgentCapabilities.skills); + const ephemeralSkillsToggle = req.body?.ephemeralAgent?.skills === true; const accessibleSkillIds = skillsCapabilityEnabled ? await findAccessibleResources({ @@ -236,7 +238,10 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { endpointOption, allowedProviders, isInitialAgent: true, - accessibleSkillIds, + accessibleSkillIds: scopeSkillIds( + accessibleSkillIds, + ephemeralSkillsToggle ? undefined : primaryAgent.skills, + ), codeEnvAvailable: enabledCapabilities.has(AgentCapabilities.execute_code), }, { @@ -283,6 +288,8 @@ const initializeClient = async ({ req, res, signal, endpointOption }) => { requestFiles, conversationId, parentMessageId, + computeAccessibleSkillIds: (agent) => + scopeSkillIds(accessibleSkillIds, ephemeralSkillsToggle ? undefined : agent.skills), }, { getAgent: db.getAgent, @@ -399,6 +406,15 @@ 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. */ const handlePrimeInvokedSkills = skillsCapabilityEnabled ? (payload) => primeInvokedSkills({ diff --git a/client/src/components/SidePanel/Agents/AgentConfig.tsx b/client/src/components/SidePanel/Agents/AgentConfig.tsx index 2ac6bc7b48..8495da8027 100644 --- a/client/src/components/SidePanel/Agents/AgentConfig.tsx +++ b/client/src/components/SidePanel/Agents/AgentConfig.tsx @@ -74,17 +74,25 @@ export default function AgentConfig() { const skills = useWatch({ control, name: 'skills' }); const agent_id = useWatch({ control, name: 'id' }); + const { + codeEnabled, + toolsEnabled, + contextEnabled, + actionsEnabled, + skillsEnabled, + artifactsEnabled, + webSearchEnabled, + fileSearchEnabled, + } = useAgentCapabilities(agentsConfig?.capabilities); + const hasSkillsAccess = useHasAccess({ permissionType: PermissionTypes.SKILLS, permission: Permissions.USE, }); - const { data: skillsData } = useListSkillsQuery({ limit: 100 }, { enabled: false }); + const showSkills = hasSkillsAccess && skillsEnabled; + const { data: skillsData } = useListSkillsQuery({ limit: 100 }, { enabled: showSkills }); const skillsMap = useMemo(() => { const map = new Map(); - // Backend list response: `{ skills: TSkillSummary[]; ... }` (renamed - // from `.data` in the CRUD PR). This integration is gated behind - // `false &&` below so this map is currently unreachable — kept here - // so the section compiles for when agent-skills wiring lands. for (const skill of skillsData?.skills ?? []) { map.set(skill._id, skill.name); } @@ -103,16 +111,6 @@ export default function AgentConfig() { return newFileMap; }, [fileMap, agentFiles]); - const { - codeEnabled, - toolsEnabled, - contextEnabled, - actionsEnabled, - artifactsEnabled, - webSearchEnabled, - fileSearchEnabled, - } = useAgentCapabilities(agentsConfig?.capabilities); - const context_files = useMemo(() => { if (typeof agent === 'string') { return []; @@ -340,8 +338,7 @@ export default function AgentConfig() { /> )} - {/* WIP: Skills — remove `false &&` to re-enable */} - {false && hasSkillsAccess && ( + {showSkills && (