🧩 fix: Harden Agent Skill Lifecycles End to End (#14429)

* test: cover agent skill lifecycles end to end

* style: sort agent skill imports

api/server/services/Endpoints/agents/addedConvo.js

@@ -10,7 +10,7 @@ const {
 const { isEphemeralAgentId } = require('librechat-data-provider');
 const { filterFilesByAgentAccess } = require('~/server/services/Files/permissions');
 const { getMCPServerTools } = require('~/server/services/Config');
-const { canAuthorSkillFiles } = require('./skillDeps');
+const { getSkillDbMethods, canAuthorSkillFiles } = require('./skillDeps');
 const db = require('~/models');
 
 const loadAddedAgent = (params) =>
@@ -97,6 +97,7 @@ const processAddedConvo = async ({
   });
 
   try {
+    const skillDbMethods = getSkillDbMethods();
     const addedAgent = await loadAddedAgent({ req, conversation: addedConvo, primaryAgent });
     if (!addedAgent) {
       return { userMCPAuthMap };
@@ -138,7 +139,7 @@ const processAddedConvo = async ({
         const resolvedSkillIds = await resolveModelSpecSkillIds({
           names: selectedModelSpec.skills,
           accessibleSkillIds,
-          getSkillByName: db.getSkillByName,
+          getSkillByName: skillDbMethods.getSkillByName,
         });
         addedAgent.skills_enabled = true;
         addedAgent.skills = resolvedSkillIds.map((id) => id.toString());
@@ -195,9 +196,9 @@ const processAddedConvo = async ({
         getToolFilesByIds: db.getToolFilesByIds,
         getCodeGeneratedFiles: db.getCodeGeneratedFiles,
         filterFilesByAgentAccess,
-        listSkillsByAccess: db.listSkillsByAccess,
-        listAlwaysApplySkills: db.listAlwaysApplySkills,
-        getSkillByName: db.getSkillByName,
+        listSkillsByAccess: skillDbMethods.listSkillsByAccess,
+        listAlwaysApplySkills: skillDbMethods.listAlwaysApplySkills,
+        getSkillByName: skillDbMethods.getSkillByName,
       },
     );
 

api/server/services/Endpoints/agents/addedConvo.spec.js

@@ -4,8 +4,12 @@ const mockLoadAddedAgent = jest.fn();
 const mockResolveAgentScopedSkillIds = jest.fn();
 const mockResolveModelSpecSkillIds = jest.fn();
 const mockCanAuthorSkillFiles = jest.fn();
+const mockGetSkillDbMethods = jest.fn();
 const mockGetAgent = jest.fn();
 const mockGetMCPServerTools = jest.fn();
+const mockRegistryGetSkillByName = jest.fn();
+const mockRegistryListSkillsByAccess = jest.fn();
+const mockRegistryListAlwaysApplySkills = jest.fn();
 
 jest.mock('@librechat/data-schemas', () => ({
   logger: {
@@ -35,6 +39,7 @@ jest.mock('~/server/services/Config', () => ({
 
 jest.mock('./skillDeps', () => ({
   canAuthorSkillFiles: (...args) => mockCanAuthorSkillFiles(...args),
+  getSkillDbMethods: () => mockGetSkillDbMethods(),
 }));
 
 jest.mock('~/models', () => ({
@@ -45,7 +50,6 @@ jest.mock('~/models', () => ({
 }));
 
 const { processAddedConvo } = require('./addedConvo');
-const db = require('~/models');
 const { Constants } = require('librechat-data-provider');
 
 const makeReq = () => ({ user: { id: 'u1', role: 'USER' } });
@@ -72,6 +76,11 @@ describe('processAddedConvo', () => {
     );
     mockResolveModelSpecSkillIds.mockResolvedValue([]);
     mockCanAuthorSkillFiles.mockReturnValue(false);
+    mockGetSkillDbMethods.mockReturnValue({
+      getSkillByName: mockRegistryGetSkillByName,
+      listSkillsByAccess: mockRegistryListSkillsByAccess,
+      listAlwaysApplySkills: mockRegistryListAlwaysApplySkills,
+    });
   });
 
   const baseParams = (overrides = {}) => ({
@@ -125,6 +134,75 @@ describe('processAddedConvo', () => {
     );
   });
 
+  it('keeps deployment-aware skill metadata on a persisted added-agent config', async () => {
+    const deploymentSkillId = { toString: () => 'deployment-skill' };
+    const agentConfigs = new Map();
+    const initializedConfig = {
+      id: 'persisted-added-agent',
+      additional_instructions: '<skill_catalog>deployment-skill</skill_catalog>',
+      manualSkillPrimes: [],
+      alwaysApplySkillPrimes: [
+        {
+          _id: 'deployment-skill',
+          name: 'deployment-skill',
+          body: 'deployment skill body',
+        },
+      ],
+      toolDefinitions: [{ name: 'skill' }],
+      userMCPAuthMap: undefined,
+    };
+
+    mockLoadAddedAgent.mockResolvedValue({
+      id: 'persisted-added-agent',
+      provider: 'openai',
+      skills_enabled: true,
+      skills: ['deployment-skill'],
+    });
+    mockResolveAgentScopedSkillIds.mockReturnValue([deploymentSkillId]);
+    mockInitializeAgent.mockResolvedValue(initializedConfig);
+
+    await processAddedConvo(
+      baseParams({
+        accessibleSkillIds: [deploymentSkillId],
+        editableSkillIds: [deploymentSkillId],
+        skillsCapabilityEnabled: true,
+        agentConfigs,
+      }),
+    );
+
+    expect(mockResolveModelSpecSkillIds).not.toHaveBeenCalled();
+    expect(mockInitializeAgent).toHaveBeenCalledWith(
+      expect.objectContaining({
+        agent: expect.objectContaining({
+          id: 'persisted-added-agent',
+          skills_enabled: true,
+          skills: ['deployment-skill'],
+        }),
+        accessibleSkillIds: [deploymentSkillId],
+      }),
+      expect.objectContaining({
+        listSkillsByAccess: mockRegistryListSkillsByAccess,
+        listAlwaysApplySkills: mockRegistryListAlwaysApplySkills,
+        getSkillByName: mockRegistryGetSkillByName,
+      }),
+    );
+    expect(agentConfigs.get('persisted-added-agent')).toBe(initializedConfig);
+    expect(agentConfigs.get('persisted-added-agent')).toEqual(
+      expect.objectContaining({
+        additional_instructions: '<skill_catalog>deployment-skill</skill_catalog>',
+        manualSkillPrimes: [],
+        alwaysApplySkillPrimes: [
+          expect.objectContaining({
+            name: 'deployment-skill',
+            body: 'deployment skill body',
+          }),
+        ],
+        toolDefinitions: [expect.objectContaining({ name: 'skill' })],
+      }),
+    );
+    expect(mockGetSkillDbMethods).toHaveBeenCalledTimes(1);
+  });
+
   it('resolves and forwards model-spec skill scope for added ephemeral agents', async () => {
     const accessibleSkillId = { toString: () => 'accessible-skill' };
     const editableSkillId = { toString: () => 'editable-skill' };
@@ -181,7 +259,7 @@ describe('processAddedConvo', () => {
     expect(mockResolveModelSpecSkillIds).toHaveBeenCalledWith({
       names: ['finance-analyst'],
       accessibleSkillIds: [accessibleSkillId],
-      getSkillByName: db.getSkillByName,
+      getSkillByName: mockRegistryGetSkillByName,
     });
     expect(mockResolveAgentScopedSkillIds).toHaveBeenNthCalledWith(1, {
       agent: expect.objectContaining({
@@ -222,10 +300,11 @@ describe('processAddedConvo', () => {
         defaultActiveOnShare: true,
       }),
       expect.objectContaining({
-        listSkillsByAccess: db.listSkillsByAccess,
-        listAlwaysApplySkills: db.listAlwaysApplySkills,
-        getSkillByName: db.getSkillByName,
+        listSkillsByAccess: mockRegistryListSkillsByAccess,
+        listAlwaysApplySkills: mockRegistryListAlwaysApplySkills,
+        getSkillByName: mockRegistryGetSkillByName,
       }),
     );
+    expect(mockGetSkillDbMethods).toHaveBeenCalledTimes(1);
   });
 });

api/server/services/Endpoints/agents/initialize.js

@@ -402,7 +402,7 @@ const initializeClient = async ({ req, res, signal, endpointOption, jobCreatedAt
       const resolvedSkillIds = await resolveModelSpecSkillIds({
         names: selectedModelSpec.skills,
         accessibleSkillIds,
-        getSkillByName: db.getSkillByName,
+        getSkillByName: skillDbMethods.getSkillByName,
       });
       primaryAgent.skills_enabled = true;
       primaryAgent.skills = resolvedSkillIds.map((id) => id.toString());

api/server/services/Endpoints/agents/initialize.spec.js

@@ -79,7 +79,7 @@ jest.mock('~/cache', () => ({
 }));
 
 const { initializeClient } = require('./initialize');
-const { getSkillToolDeps } = require('./skillDeps');
+const { getSkillDbMethods, getSkillToolDeps } = require('./skillDeps');
 const { getModelsConfig } = require('~/server/controllers/ModelController');
 const { logger } = require('@librechat/data-schemas');
 const { User, AclEntry } = require('~/db/models');
@@ -360,6 +360,63 @@ describe('initializeClient — processAgent ACL gate', () => {
       canCreateSkillSpy.mockRestore();
     }
   });
+
+  it('resolves model-spec skill names through deployment-aware skill methods', async () => {
+    const deploymentSkillId = new mongoose.Types.ObjectId();
+    await AclEntry.create({
+      principalType: PrincipalType.USER,
+      principalId: testUser._id,
+      principalModel: PrincipalModel.USER,
+      resourceType: ResourceType.SKILL,
+      resourceId: new mongoose.Types.ObjectId(),
+      permBits: PermissionBits.VIEW,
+      grantedBy: testUser._id,
+    });
+    const endpointOption = makeEndpointOption();
+    endpointOption.spec = 'spec-deployment-skill';
+    endpointOption.agent = Promise.resolve({
+      id: Constants.EPHEMERAL_AGENT_ID,
+      name: 'Ephemeral Primary',
+      provider: 'openai',
+      model: 'gpt-4',
+      tools: [],
+    });
+    mockInitializeAgent.mockResolvedValue(makePrimaryConfig([]));
+    const req = makeReq();
+    req.config.endpoints.agents = { capabilities: ['skills'] };
+    req.config.modelSpecs = {
+      list: [{ name: 'spec-deployment-skill', skills: ['deployment-skill'] }],
+    };
+    const getSkillByNameSpy = jest.spyOn(getSkillDbMethods(), 'getSkillByName').mockResolvedValue({
+      _id: deploymentSkillId,
+      name: 'deployment-skill',
+      source: 'deployment',
+    });
+    const canCreateSkillSpy = jest
+      .spyOn(getSkillToolDeps(), 'canCreateSkill')
+      .mockResolvedValue(false);
+
+    try {
+      await initializeClient({
+        req,
+        res: {},
+        signal: new AbortController().signal,
+        endpointOption,
+      });
+
+      expect(getSkillByNameSpy).toHaveBeenCalledWith(
+        'deployment-skill',
+        expect.any(Array),
+        expect.any(Object),
+      );
+      const initializeParams = mockInitializeAgent.mock.calls[0][0];
+      expect(initializeParams.agent.skills_enabled).toBe(true);
+      expect(initializeParams.agent.skills).toEqual([deploymentSkillId.toString()]);
+    } finally {
+      getSkillByNameSpy.mockRestore();
+      canCreateSkillSpy.mockRestore();
+    }
+  });
 });
 
 describe('initializeClient — subagent loading', () => {