From 7f59a1815c174120d128c61f2e235e16696eb0fc Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 21 Jan 2026 15:01:04 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=A7=20fix:=20Agent=20Deletion=20Logic?= =?UTF-8?q?=20to=20Update=20User=20Favorites=20(#11466)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔧 fix: Agent Deletion Logic to Update User Favorites * Added functionality to remove agents from user favorites when an agent is deleted. * Implemented updates in the deleteAgent and deleteUserAgents functions to ensure user favorites are correctly modified. * Added comprehensive tests to verify that agents are removed from user favorites across multiple scenarios, ensuring data integrity and user experience. * 🔧 test: Enhance deleteUserAgents Functionality Tests * Added comprehensive tests for the deleteUserAgents function to ensure it correctly removes agents from user favorites across various scenarios. * Verified that user favorites are updated appropriately when agents are deleted, including cases where agents are shared among multiple users and when users have no favorites. * Ensured that existing agents remain unaffected when no agents are associated with the author being deleted. * 🔧 refactor: Remove Deprecated getListAgents Functionality * Removed the deprecated getListAgents function from the Agent model, encouraging the use of getListAgentsByAccess for ACL-aware agent listing. * Updated related tests in Agent.spec.js to eliminate references to getListAgents, ensuring code cleanliness and maintainability. * Adjusted imports and exports accordingly to reflect the removal of the deprecated function. --- api/models/Agent.js | 79 ++---- api/models/Agent.spec.js | 543 ++++++++++++++++++++++++++++++++++----- 2 files changed, 499 insertions(+), 123 deletions(-) diff --git a/api/models/Agent.js b/api/models/Agent.js index 3df2bcbec2..11789ca63b 100644 --- a/api/models/Agent.js +++ b/api/models/Agent.js @@ -11,17 +11,15 @@ const { isEphemeralAgentId, encodeEphemeralAgentId, } = require('librechat-data-provider'); -const { GLOBAL_PROJECT_NAME, mcp_all, mcp_delimiter } = - require('librechat-data-provider').Constants; +const { mcp_all, mcp_delimiter } = require('librechat-data-provider').Constants; const { removeAgentFromAllProjects, removeAgentIdsFromProject, addAgentIdsToProject, - getProjectByName, } = require('./Project'); const { removeAllPermissions } = require('~/server/services/PermissionService'); const { getMCPServerTools } = require('~/server/services/Config'); -const { Agent, AclEntry } = require('~/db/models'); +const { Agent, AclEntry, User } = require('~/db/models'); const { getActions } = require('./Action'); /** @@ -600,6 +598,14 @@ const deleteAgent = async (searchParameter) => { } catch (error) { logger.error('[deleteAgent] Error removing agent from handoff edges', error); } + try { + await User.updateMany( + { 'favorites.agentId': agent.id }, + { $pull: { favorites: { agentId: agent.id } } }, + ); + } catch (error) { + logger.error('[deleteAgent] Error removing agent from user favorites', error); + } } return agent; }; @@ -629,6 +635,15 @@ const deleteUserAgents = async (userId) => { resourceId: { $in: agentObjectIds }, }); + try { + await User.updateMany( + { 'favorites.agentId': { $in: agentIds } }, + { $pull: { favorites: { agentId: { $in: agentIds } } } }, + ); + } catch (error) { + logger.error('[deleteUserAgents] Error removing agents from user favorites', error); + } + await Agent.deleteMany({ author: userId }); } catch (error) { logger.error('[deleteUserAgents] General error:', error); @@ -735,59 +750,6 @@ const getListAgentsByAccess = async ({ }; }; -/** - * Get all agents. - * @deprecated Use getListAgentsByAccess for ACL-aware agent listing - * @param {Object} searchParameter - The search parameters to find matching agents. - * @param {string} searchParameter.author - The user ID of the agent's author. - * @returns {Promise} A promise that resolves to an object containing the agents data and pagination info. - */ -const getListAgents = async (searchParameter) => { - const { author, ...otherParams } = searchParameter; - - let query = Object.assign({ author }, otherParams); - - const globalProject = await getProjectByName(GLOBAL_PROJECT_NAME, ['agentIds']); - if (globalProject && (globalProject.agentIds?.length ?? 0) > 0) { - const globalQuery = { id: { $in: globalProject.agentIds }, ...otherParams }; - delete globalQuery.author; - query = { $or: [globalQuery, query] }; - } - const agents = ( - await Agent.find(query, { - id: 1, - _id: 1, - name: 1, - avatar: 1, - author: 1, - projectIds: 1, - description: 1, - // @deprecated - isCollaborative replaced by ACL permissions - isCollaborative: 1, - category: 1, - }).lean() - ).map((agent) => { - if (agent.author?.toString() !== author) { - delete agent.author; - } - if (agent.author) { - agent.author = agent.author.toString(); - } - return agent; - }); - - const hasMore = agents.length > 0; - const firstId = agents.length > 0 ? agents[0].id : null; - const lastId = agents.length > 0 ? agents[agents.length - 1].id : null; - - return { - data: agents, - has_more: hasMore, - first_id: firstId, - last_id: lastId, - }; -}; - /** * Updates the projects associated with an agent, adding and removing project IDs as specified. * This function also updates the corresponding projects to include or exclude the agent ID. @@ -953,12 +915,11 @@ module.exports = { updateAgent, deleteAgent, deleteUserAgents, - getListAgents, revertAgentVersion, updateAgentProjects, + countPromotedAgents, addAgentResourceFile, getListAgentsByAccess, removeAgentResourceFiles, generateActionMetadataHash, - countPromotedAgents, }; diff --git a/api/models/Agent.spec.js b/api/models/Agent.spec.js index 51256f8cf1..baceb3e8f3 100644 --- a/api/models/Agent.spec.js +++ b/api/models/Agent.spec.js @@ -22,17 +22,17 @@ const { createAgent, updateAgent, deleteAgent, - getListAgents, - getListAgentsByAccess, + deleteUserAgents, revertAgentVersion, updateAgentProjects, addAgentResourceFile, + getListAgentsByAccess, removeAgentResourceFiles, generateActionMetadataHash, } = require('./Agent'); const permissionService = require('~/server/services/PermissionService'); const { getCachedTools, getMCPServerTools } = require('~/server/services/Config'); -const { AclEntry } = require('~/db/models'); +const { AclEntry, User } = require('~/db/models'); /** * @type {import('mongoose').Model} @@ -59,6 +59,7 @@ describe('models/Agent', () => { beforeEach(async () => { await Agent.deleteMany({}); + await User.deleteMany({}); }); test('should add tool_resource to tools if missing', async () => { @@ -575,43 +576,488 @@ describe('models/Agent', () => { expect(sourceAgentAfter.edges).toHaveLength(0); }); - test('should list agents by author', async () => { + test('should remove agent from user favorites when agent is deleted', async () => { + const agentId = `agent_${uuidv4()}`; + const authorId = new mongoose.Types.ObjectId(); + const userId = new mongoose.Types.ObjectId(); + + // Create agent + await createAgent({ + id: agentId, + name: 'Agent To Delete', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create user with the agent in favorites + await User.create({ + _id: userId, + name: 'Test User', + email: `test-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agentId }, { model: 'gpt-4', endpoint: 'openAI' }], + }); + + // Verify user has agent in favorites + const userBefore = await User.findById(userId); + expect(userBefore.favorites).toHaveLength(2); + expect(userBefore.favorites.some((f) => f.agentId === agentId)).toBe(true); + + // Delete the agent + await deleteAgent({ id: agentId }); + + // Verify agent is deleted + const agentAfterDelete = await getAgent({ id: agentId }); + expect(agentAfterDelete).toBeNull(); + + // Verify agent is removed from user favorites + const userAfter = await User.findById(userId); + expect(userAfter.favorites).toHaveLength(1); + expect(userAfter.favorites.some((f) => f.agentId === agentId)).toBe(false); + expect(userAfter.favorites.some((f) => f.model === 'gpt-4')).toBe(true); + }); + + test('should remove agent from multiple users favorites when agent is deleted', async () => { + const agentId = `agent_${uuidv4()}`; + const authorId = new mongoose.Types.ObjectId(); + const user1Id = new mongoose.Types.ObjectId(); + const user2Id = new mongoose.Types.ObjectId(); + + // Create agent + await createAgent({ + id: agentId, + name: 'Agent To Delete', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create two users with the agent in favorites + await User.create({ + _id: user1Id, + name: 'Test User 1', + email: `test1-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agentId }], + }); + + await User.create({ + _id: user2Id, + name: 'Test User 2', + email: `test2-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agentId }, { agentId: `agent_${uuidv4()}` }], + }); + + // Delete the agent + await deleteAgent({ id: agentId }); + + // Verify agent is removed from both users' favorites + const user1After = await User.findById(user1Id); + const user2After = await User.findById(user2Id); + + expect(user1After.favorites).toHaveLength(0); + expect(user2After.favorites).toHaveLength(1); + expect(user2After.favorites.some((f) => f.agentId === agentId)).toBe(false); + }); + + test('should preserve other agents in database when one agent is deleted', async () => { + const agentToDeleteId = `agent_${uuidv4()}`; + const agentToKeep1Id = `agent_${uuidv4()}`; + const agentToKeep2Id = `agent_${uuidv4()}`; + const authorId = new mongoose.Types.ObjectId(); + + // Create multiple agents + await createAgent({ + id: agentToDeleteId, + name: 'Agent To Delete', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agentToKeep1Id, + name: 'Agent To Keep 1', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agentToKeep2Id, + name: 'Agent To Keep 2', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Verify all agents exist + expect(await getAgent({ id: agentToDeleteId })).not.toBeNull(); + expect(await getAgent({ id: agentToKeep1Id })).not.toBeNull(); + expect(await getAgent({ id: agentToKeep2Id })).not.toBeNull(); + + // Delete one agent + await deleteAgent({ id: agentToDeleteId }); + + // Verify only the deleted agent is removed, others remain intact + expect(await getAgent({ id: agentToDeleteId })).toBeNull(); + const keptAgent1 = await getAgent({ id: agentToKeep1Id }); + const keptAgent2 = await getAgent({ id: agentToKeep2Id }); + expect(keptAgent1).not.toBeNull(); + expect(keptAgent1.name).toBe('Agent To Keep 1'); + expect(keptAgent2).not.toBeNull(); + expect(keptAgent2.name).toBe('Agent To Keep 2'); + }); + + test('should preserve other agents in user favorites when one agent is deleted', async () => { + const agentToDeleteId = `agent_${uuidv4()}`; + const agentToKeep1Id = `agent_${uuidv4()}`; + const agentToKeep2Id = `agent_${uuidv4()}`; + const authorId = new mongoose.Types.ObjectId(); + const userId = new mongoose.Types.ObjectId(); + + // Create multiple agents + await createAgent({ + id: agentToDeleteId, + name: 'Agent To Delete', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agentToKeep1Id, + name: 'Agent To Keep 1', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agentToKeep2Id, + name: 'Agent To Keep 2', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create user with all three agents in favorites + await User.create({ + _id: userId, + name: 'Test User', + email: `test-${uuidv4()}@example.com`, + provider: 'local', + favorites: [ + { agentId: agentToDeleteId }, + { agentId: agentToKeep1Id }, + { agentId: agentToKeep2Id }, + ], + }); + + // Verify user has all three agents in favorites + const userBefore = await User.findById(userId); + expect(userBefore.favorites).toHaveLength(3); + + // Delete one agent + await deleteAgent({ id: agentToDeleteId }); + + // Verify only the deleted agent is removed from favorites + const userAfter = await User.findById(userId); + expect(userAfter.favorites).toHaveLength(2); + expect(userAfter.favorites.some((f) => f.agentId === agentToDeleteId)).toBe(false); + expect(userAfter.favorites.some((f) => f.agentId === agentToKeep1Id)).toBe(true); + expect(userAfter.favorites.some((f) => f.agentId === agentToKeep2Id)).toBe(true); + }); + + test('should not affect users who do not have deleted agent in favorites', async () => { + const agentToDeleteId = `agent_${uuidv4()}`; + const otherAgentId = `agent_${uuidv4()}`; + const authorId = new mongoose.Types.ObjectId(); + const userWithDeletedAgentId = new mongoose.Types.ObjectId(); + const userWithoutDeletedAgentId = new mongoose.Types.ObjectId(); + + // Create agents + await createAgent({ + id: agentToDeleteId, + name: 'Agent To Delete', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: otherAgentId, + name: 'Other Agent', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create user with the agent to be deleted + await User.create({ + _id: userWithDeletedAgentId, + name: 'User With Deleted Agent', + email: `user1-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agentToDeleteId }, { model: 'gpt-4', endpoint: 'openAI' }], + }); + + // Create user without the agent to be deleted + await User.create({ + _id: userWithoutDeletedAgentId, + name: 'User Without Deleted Agent', + email: `user2-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: otherAgentId }, { model: 'claude-3', endpoint: 'anthropic' }], + }); + + // Delete the agent + await deleteAgent({ id: agentToDeleteId }); + + // Verify user with deleted agent has it removed + const userWithDeleted = await User.findById(userWithDeletedAgentId); + expect(userWithDeleted.favorites).toHaveLength(1); + expect(userWithDeleted.favorites.some((f) => f.agentId === agentToDeleteId)).toBe(false); + expect(userWithDeleted.favorites.some((f) => f.model === 'gpt-4')).toBe(true); + + // Verify user without deleted agent is completely unaffected + const userWithoutDeleted = await User.findById(userWithoutDeletedAgentId); + expect(userWithoutDeleted.favorites).toHaveLength(2); + expect(userWithoutDeleted.favorites.some((f) => f.agentId === otherAgentId)).toBe(true); + expect(userWithoutDeleted.favorites.some((f) => f.model === 'claude-3')).toBe(true); + }); + + test('should remove all user agents from favorites when deleteUserAgents is called', async () => { const authorId = new mongoose.Types.ObjectId(); const otherAuthorId = new mongoose.Types.ObjectId(); + const userId = new mongoose.Types.ObjectId(); - const agentIds = []; - for (let i = 0; i < 5; i++) { - const id = `agent_${uuidv4()}`; - agentIds.push(id); - await createAgent({ - id, - name: `Agent ${i}`, - provider: 'test', - model: 'test-model', - author: authorId, - }); - } + const agent1Id = `agent_${uuidv4()}`; + const agent2Id = `agent_${uuidv4()}`; + const otherAuthorAgentId = `agent_${uuidv4()}`; - for (let i = 0; i < 3; i++) { - await createAgent({ - id: `other_agent_${uuidv4()}`, - name: `Other Agent ${i}`, - provider: 'test', - model: 'test-model', - author: otherAuthorId, - }); - } + // Create agents by the author to be deleted + await createAgent({ + id: agent1Id, + name: 'Author Agent 1', + provider: 'test', + model: 'test-model', + author: authorId, + }); - const result = await getListAgents({ author: authorId.toString() }); + await createAgent({ + id: agent2Id, + name: 'Author Agent 2', + provider: 'test', + model: 'test-model', + author: authorId, + }); - expect(result).toBeDefined(); - expect(result.data).toBeDefined(); - expect(result.data).toHaveLength(5); - expect(result.has_more).toBe(true); + // Create agent by different author (should not be deleted) + await createAgent({ + id: otherAuthorAgentId, + name: 'Other Author Agent', + provider: 'test', + model: 'test-model', + author: otherAuthorId, + }); - for (const agent of result.data) { - expect(agent.author).toBe(authorId.toString()); - } + // Create user with all agents in favorites + await User.create({ + _id: userId, + name: 'Test User', + email: `test-${uuidv4()}@example.com`, + provider: 'local', + favorites: [ + { agentId: agent1Id }, + { agentId: agent2Id }, + { agentId: otherAuthorAgentId }, + { model: 'gpt-4', endpoint: 'openAI' }, + ], + }); + + // Verify user has all favorites + const userBefore = await User.findById(userId); + expect(userBefore.favorites).toHaveLength(4); + + // Delete all agents by the author + await deleteUserAgents(authorId.toString()); + + // Verify author's agents are deleted from database + expect(await getAgent({ id: agent1Id })).toBeNull(); + expect(await getAgent({ id: agent2Id })).toBeNull(); + + // Verify other author's agent still exists + expect(await getAgent({ id: otherAuthorAgentId })).not.toBeNull(); + + // Verify user favorites: author's agents removed, others remain + const userAfter = await User.findById(userId); + expect(userAfter.favorites).toHaveLength(2); + expect(userAfter.favorites.some((f) => f.agentId === agent1Id)).toBe(false); + expect(userAfter.favorites.some((f) => f.agentId === agent2Id)).toBe(false); + expect(userAfter.favorites.some((f) => f.agentId === otherAuthorAgentId)).toBe(true); + expect(userAfter.favorites.some((f) => f.model === 'gpt-4')).toBe(true); + }); + + test('should handle deleteUserAgents when agents are in multiple users favorites', async () => { + const authorId = new mongoose.Types.ObjectId(); + const user1Id = new mongoose.Types.ObjectId(); + const user2Id = new mongoose.Types.ObjectId(); + const user3Id = new mongoose.Types.ObjectId(); + + const agent1Id = `agent_${uuidv4()}`; + const agent2Id = `agent_${uuidv4()}`; + const unrelatedAgentId = `agent_${uuidv4()}`; + + // Create agents by the author + await createAgent({ + id: agent1Id, + name: 'Author Agent 1', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agent2Id, + name: 'Author Agent 2', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create users with various favorites configurations + await User.create({ + _id: user1Id, + name: 'User 1', + email: `user1-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agent1Id }, { agentId: agent2Id }], + }); + + await User.create({ + _id: user2Id, + name: 'User 2', + email: `user2-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: agent1Id }, { model: 'claude-3', endpoint: 'anthropic' }], + }); + + await User.create({ + _id: user3Id, + name: 'User 3', + email: `user3-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: unrelatedAgentId }, { model: 'gpt-4', endpoint: 'openAI' }], + }); + + // Delete all agents by the author + await deleteUserAgents(authorId.toString()); + + // Verify all users' favorites are correctly updated + const user1After = await User.findById(user1Id); + expect(user1After.favorites).toHaveLength(0); + + const user2After = await User.findById(user2Id); + expect(user2After.favorites).toHaveLength(1); + expect(user2After.favorites.some((f) => f.agentId === agent1Id)).toBe(false); + expect(user2After.favorites.some((f) => f.model === 'claude-3')).toBe(true); + + // User 3 should be completely unaffected + const user3After = await User.findById(user3Id); + expect(user3After.favorites).toHaveLength(2); + expect(user3After.favorites.some((f) => f.agentId === unrelatedAgentId)).toBe(true); + expect(user3After.favorites.some((f) => f.model === 'gpt-4')).toBe(true); + }); + + test('should handle deleteUserAgents when user has no agents', async () => { + const authorWithNoAgentsId = new mongoose.Types.ObjectId(); + const otherAuthorId = new mongoose.Types.ObjectId(); + const userId = new mongoose.Types.ObjectId(); + + const existingAgentId = `agent_${uuidv4()}`; + + // Create agent by different author + await createAgent({ + id: existingAgentId, + name: 'Existing Agent', + provider: 'test', + model: 'test-model', + author: otherAuthorId, + }); + + // Create user with favorites + await User.create({ + _id: userId, + name: 'Test User', + email: `test-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ agentId: existingAgentId }, { model: 'gpt-4', endpoint: 'openAI' }], + }); + + // Delete agents for user with no agents (should be a no-op) + await deleteUserAgents(authorWithNoAgentsId.toString()); + + // Verify existing agent still exists + expect(await getAgent({ id: existingAgentId })).not.toBeNull(); + + // Verify user favorites are unchanged + const userAfter = await User.findById(userId); + expect(userAfter.favorites).toHaveLength(2); + expect(userAfter.favorites.some((f) => f.agentId === existingAgentId)).toBe(true); + expect(userAfter.favorites.some((f) => f.model === 'gpt-4')).toBe(true); + }); + + test('should handle deleteUserAgents when agents are not in any favorites', async () => { + const authorId = new mongoose.Types.ObjectId(); + const userId = new mongoose.Types.ObjectId(); + + const agent1Id = `agent_${uuidv4()}`; + const agent2Id = `agent_${uuidv4()}`; + + // Create agents by the author + await createAgent({ + id: agent1Id, + name: 'Agent 1', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + await createAgent({ + id: agent2Id, + name: 'Agent 2', + provider: 'test', + model: 'test-model', + author: authorId, + }); + + // Create user with favorites that don't include these agents + await User.create({ + _id: userId, + name: 'Test User', + email: `test-${uuidv4()}@example.com`, + provider: 'local', + favorites: [{ model: 'gpt-4', endpoint: 'openAI' }], + }); + + // Verify agents exist + expect(await getAgent({ id: agent1Id })).not.toBeNull(); + expect(await getAgent({ id: agent2Id })).not.toBeNull(); + + // Delete all agents by the author + await deleteUserAgents(authorId.toString()); + + // Verify agents are deleted + expect(await getAgent({ id: agent1Id })).toBeNull(); + expect(await getAgent({ id: agent2Id })).toBeNull(); + + // Verify user favorites are unchanged + const userAfter = await User.findById(userId); + expect(userAfter.favorites).toHaveLength(1); + expect(userAfter.favorites.some((f) => f.model === 'gpt-4')).toBe(true); }); test('should update agent projects', async () => { @@ -733,26 +1179,6 @@ describe('models/Agent', () => { expect(result).toBe(expected); }); - test('should handle getListAgents with invalid author format', async () => { - try { - const result = await getListAgents({ author: 'invalid-object-id' }); - expect(result.data).toEqual([]); - } catch (error) { - expect(error).toBeDefined(); - } - }); - - test('should handle getListAgents with no agents', async () => { - const authorId = new mongoose.Types.ObjectId(); - const result = await getListAgents({ author: authorId.toString() }); - - expect(result).toBeDefined(); - expect(result.data).toEqual([]); - expect(result.has_more).toBe(false); - expect(result.first_id).toBeNull(); - expect(result.last_id).toBeNull(); - }); - test('should handle updateAgentProjects with non-existent agent', async () => { const nonExistentId = `agent_${uuidv4()}`; const userId = new mongoose.Types.ObjectId(); @@ -2366,17 +2792,6 @@ describe('models/Agent', () => { expect(result).toBeNull(); }); - test('should handle getListAgents with no agents', async () => { - const authorId = new mongoose.Types.ObjectId(); - const result = await getListAgents({ author: authorId.toString() }); - - expect(result).toBeDefined(); - expect(result.data).toEqual([]); - expect(result.has_more).toBe(false); - expect(result.first_id).toBeNull(); - expect(result.last_id).toBeNull(); - }); - test('should handle updateAgent with MongoDB operators mixed with direct updates', async () => { const agentId = `agent_${uuidv4()}`; const authorId = new mongoose.Types.ObjectId();