diff --git a/api/server/controllers/agents/__tests__/resume.spec.js b/api/server/controllers/agents/__tests__/resume.spec.js index fcf040f763..79f31815c8 100644 --- a/api/server/controllers/agents/__tests__/resume.spec.js +++ b/api/server/controllers/agents/__tests__/resume.spec.js @@ -1530,7 +1530,7 @@ describe('ResumeAgentController (POST /agents/chat/resume)', () => { const res = await post(approveBody()); expect(res.status).toBe(400); - expect(mockGetActions).toHaveBeenCalledWith({ agent_id: { $in: [AGENT_ID] } }, false); + expect(mockGetActions).toHaveBeenCalledWith({ agentId: [AGENT_ID] }, false); expect(mockDecryptMetadata).not.toHaveBeenCalled(); expect(mockGenerationJobManager.approvals.resolve).not.toHaveBeenCalled(); expect(mockInitializeClient).not.toHaveBeenCalled(); diff --git a/api/server/controllers/agents/__tests__/v1.spec.js b/api/server/controllers/agents/__tests__/v1.spec.js index be2e4553f7..0b2b7722ae 100644 --- a/api/server/controllers/agents/__tests__/v1.spec.js +++ b/api/server/controllers/agents/__tests__/v1.spec.js @@ -68,7 +68,7 @@ describe('duplicateAgent', () => { await duplicateAgent(req, res); expect(getAgent).toHaveBeenCalledWith({ id: 'agent_123' }); - expect(getActions).toHaveBeenCalledWith({ agent_id: 'agent_123' }, true); + expect(getActions).toHaveBeenCalledWith({ agentId: 'agent_123' }, true); expect(createAgent).toHaveBeenCalledWith( expect.objectContaining({ id: 'agent_new_123', diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 20c1478697..cd455dc67b 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -1465,7 +1465,7 @@ const duplicateAgentHandler = async (req, res) => { return res.status(subagentReferenceError.status).json(subagentReferenceError.body); } - const originalActions = (await db.getActions({ agent_id: id }, true)) ?? []; + const originalActions = (await db.getActions({ agentId: id }, true)) ?? []; const sanitizedActions = originalActions.map((action) => { const metadata = { ...(action.metadata || {}) }; for (const field of sensitiveFields) { @@ -1550,7 +1550,7 @@ const duplicateAgentHandler = async (req, res) => { const fullActionId = `${domain}${actionDelimiter}${newActionId}`; const newAction = await db.updateAction( - { action_id: newActionId, agent_id: newAgentId }, + { actionId: newActionId, agentId: newAgentId }, { metadata: action.metadata, agent_id: newAgentId, @@ -1575,7 +1575,7 @@ const duplicateAgentHandler = async (req, res) => { try { newAgent = await db.createAgent(newAgentData); } catch (error) { - await db.deleteActions({ agent_id: newAgentId, user: userId }).catch((cleanupError) => { + await db.deleteActions({ agentId: newAgentId, user: userId }).catch((cleanupError) => { logger.error( '[/agents/:id/duplicate] Failed to clean up cloned Actions after Agent creation failed:', cleanupError, @@ -2065,7 +2065,7 @@ const revertAgentVersionHandler = async (req, res) => { .filter(Boolean); const actions = actionIds.length > 0 - ? ((await db.getActions({ agent_id: id, action_id: { $in: actionIds } }, true)) ?? []) + ? ((await db.getActions({ agentId: id, actionId: actionIds }, true)) ?? []) : []; if ( diff --git a/api/server/controllers/assistants/v1.js b/api/server/controllers/assistants/v1.js index 2f53c07632..425fb98de8 100644 --- a/api/server/controllers/assistants/v1.js +++ b/api/server/controllers/assistants/v1.js @@ -104,7 +104,7 @@ const createAssistant = async (req, res) => { createData.append_current_datetime = append_current_datetime; } - const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData); + const document = await updateAssistantDoc({ assistantId: assistant.id }, createData); if (azureModelIdentifier) { assistant.model = azureModelIdentifier; @@ -220,14 +220,14 @@ const patchAssistant = async (req, res) => { if (conversation_starters !== undefined) { const conversationStartersUpdate = await updateAssistantDoc( - { assistant_id }, + { assistantId: assistant_id }, { conversation_starters }, ); updatedAssistant.conversation_starters = conversationStartersUpdate.conversation_starters; } if (append_current_datetime !== undefined) { - await updateAssistantDoc({ assistant_id }, { append_current_datetime }); + await updateAssistantDoc({ assistantId: assistant_id }, { append_current_datetime }); updatedAssistant.append_current_datetime = append_current_datetime; } @@ -418,7 +418,7 @@ const uploadAssistantAvatar = async (req, res) => { const promises = []; promises.push( updateAssistantDoc( - { assistant_id }, + { assistantId: assistant_id }, { avatar: { filepath: image.filepath, diff --git a/api/server/controllers/assistants/v2.js b/api/server/controllers/assistants/v2.js index e6e9a5d0ef..063354d946 100644 --- a/api/server/controllers/assistants/v2.js +++ b/api/server/controllers/assistants/v2.js @@ -94,7 +94,7 @@ const createAssistant = async (req, res) => { createData.append_current_datetime = append_current_datetime; } - const document = await updateAssistantDoc({ assistant_id: assistant.id }, createData); + const document = await updateAssistantDoc({ assistantId: assistant.id }, createData); if (azureModelIdentifier) { assistant.model = azureModelIdentifier; @@ -135,7 +135,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => { if (updateData?.conversation_starters) { const conversationStartersUpdate = await updateAssistantDoc( - { assistant_id: assistant_id }, + { assistantId: assistant_id }, { conversation_starters: updateData.conversation_starters }, ); conversation_starters = conversationStartersUpdate.conversation_starters; @@ -145,7 +145,7 @@ const updateAssistant = async ({ req, openai, assistant_id, updateData }) => { if (updateData?.append_current_datetime !== undefined) { await updateAssistantDoc( - { assistant_id: assistant_id }, + { assistantId: assistant_id }, { append_current_datetime: updateData.append_current_datetime }, ); delete updateData.append_current_datetime; diff --git a/api/server/middleware/assistants/validateAuthor.js b/api/server/middleware/assistants/validateAuthor.js index 024d6abbe3..6ffc8382b4 100644 --- a/api/server/middleware/assistants/validateAuthor.js +++ b/api/server/middleware/assistants/validateAuthor.js @@ -40,7 +40,7 @@ const validateAuthor = async ({ req, openai, overrideEndpoint, overrideAssistant return; } - const assistantDoc = await getAssistant({ assistant_id, user: req.user.id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id, user: req.user.id }); if (assistantDoc) { return; } diff --git a/api/server/middleware/spec/validateImages.spec.js b/api/server/middleware/spec/validateImages.spec.js index ba04a27310..7eab98fa69 100644 --- a/api/server/middleware/spec/validateImages.spec.js +++ b/api/server/middleware/spec/validateImages.spec.js @@ -311,13 +311,11 @@ describe('validateImageRequest middleware', () => { expect(next).toHaveBeenCalled(); expect(getAssistant).toHaveBeenCalledWith( { - 'avatar.filepath': { - $in: [ - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png', - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false', - '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true', - ], - }, + avatarFilepath: [ + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png', + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=false', + '/images/65cfb246f7ecadb8b1e8036c/assistant-avatar.png?manual=true', + ], }, { _id: 1, assistant_id: 1, endpoint: 1 }, ); diff --git a/api/server/routes/agents/actions.js b/api/server/routes/agents/actions.js index 124fe7209a..9846a2b899 100644 --- a/api/server/routes/agents/actions.js +++ b/api/server/routes/agents/actions.js @@ -68,9 +68,7 @@ router.get('/', async (req, res) => { const editableAgentIds = agentsResponse.data.map((agent) => agent.id); const actions = - editableAgentIds.length > 0 - ? await db.getActions({ agent_id: { $in: editableAgentIds } }) - : []; + editableAgentIds.length > 0 ? await db.getActions({ agentId: editableAgentIds }) : []; for (const action of actions) { if (blockFilteredActionProjection(req.config?.filters, res, action)) { @@ -171,7 +169,7 @@ router.post( // Permissions already validated by middleware - load agent directly initialPromises.push(db.getAgent({ id: agent_id })); if (requestedActionId) { - initialPromises.push(db.getActions({ action_id: requestedActionId }, true)); + initialPromises.push(db.getActions({ actionId: requestedActionId }, true)); } /** @type {[Agent, [Action|undefined]]} */ @@ -254,7 +252,7 @@ router.post( /** @type {Action} */ const updatedAction = await db.updateAction( - { action_id: requestedActionId ?? action_id, agent_id }, + { actionId: requestedActionId ?? action_id, agentId: agent_id }, actionUpdateData, ); @@ -323,7 +321,7 @@ router.delete( { tools: updatedTools, actions: updatedActions }, { updatingUserId: req.user.id, forceVersion: true }, ); - const deleted = await db.deleteAction({ action_id, agent_id }); + const deleted = await db.deleteAction({ actionId: action_id, agentId: agent_id }); if (!deleted) { logger.warn('[Agent Action Delete] No matching action document found', { action_id, diff --git a/api/server/routes/assistants/actions.js b/api/server/routes/assistants/actions.js index 42e37f3b47..e3e952a603 100644 --- a/api/server/routes/assistants/actions.js +++ b/api/server/routes/assistants/actions.js @@ -70,9 +70,9 @@ router.post('/:assistant_id', async (req, res) => { const { openai } = await getOpenAIClient({ req, res }); - initialPromises.push(db.getAssistant({ assistant_id })); + initialPromises.push(db.getAssistant({ assistantId: assistant_id })); initialPromises.push(openai.beta.assistants.retrieve(assistant_id)); - !!_action_id && initialPromises.push(db.getActions({ action_id }, true)); + !!_action_id && initialPromises.push(db.getActions({ actionId: action_id }, true)); /** @type {[AssistantDocument, Assistant, [Action|undefined]]} */ const [assistant_data, assistant, actions_result] = await Promise.all(initialPromises); @@ -150,7 +150,7 @@ router.post('/:assistant_id', async (req, res) => { if (!assistant_data) { assistantUpdateData.user = req.user.id; } - promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData)); + promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData)); // Only update user field for new actions const actionUpdateData = { metadata, assistant_id }; @@ -158,7 +158,9 @@ router.post('/:assistant_id', async (req, res) => { // For new actions, use the assistant owner's user ID actionUpdateData.user = assistant_user || req.user.id; } - promises.push(db.updateAction({ action_id, assistant_id }, actionUpdateData)); + promises.push( + db.updateAction({ actionId: action_id, assistantId: assistant_id }, actionUpdateData), + ); /** @type {[AssistantDocument, Action]} */ let [assistantDocument, updatedAction] = await Promise.all(promises); @@ -200,7 +202,7 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => { const { openai } = await getOpenAIClient({ req, res }); const initialPromises = []; - initialPromises.push(db.getAssistant({ assistant_id })); + initialPromises.push(db.getAssistant({ assistantId: assistant_id })); initialPromises.push(openai.beta.assistants.retrieve(assistant_id)); /** @type {[AssistantDocument, Assistant]} */ @@ -238,8 +240,8 @@ router.delete('/:assistant_id/:action_id/:model', async (req, res) => { if (!assistant_data) { assistantUpdateData.user = req.user.id; } - promises.push(db.updateAssistantDoc({ assistant_id }, assistantUpdateData)); - promises.push(db.deleteAction({ action_id, assistant_id })); + promises.push(db.updateAssistantDoc({ assistantId: assistant_id }, assistantUpdateData)); + promises.push(db.deleteAction({ actionId: action_id, assistantId: assistant_id })); const [, deletedAction] = await Promise.all(promises); if (!deletedAction) { diff --git a/api/server/routes/files/files.js b/api/server/routes/files/files.js index 281afd1d54..daa13a8a9a 100644 --- a/api/server/routes/files/files.js +++ b/api/server/routes/files/files.js @@ -289,7 +289,7 @@ router.delete('/', async (req, res) => { /* Handle assistant unlinking even if no valid files to delete */ if (req.body.assistant_id && req.body.tool_resource && dbFiles.length === 0) { const assistant = await db.getAssistant({ - assistant_id: req.body.assistant_id, + assistantId: req.body.assistant_id, }); const toolResourceFiles = assistant?.tool_resources?.[req.body.tool_resource]?.file_ids ?? []; diff --git a/api/server/services/ActionService.js b/api/server/services/ActionService.js index 7caa89064e..9f5e7f9942 100644 --- a/api/server/services/ActionService.js +++ b/api/server/services/ActionService.js @@ -57,7 +57,7 @@ const validateAndUpdateTool = async ({ req, tool, assistant_id }) => { } if (!toolNameRegex.test(tool.function.name)) { const [functionName, domain] = tool.function.name.split(actionDelimiter); - actions = await getActions({ assistant_id, user: req.user.id }, true); + actions = await getActions({ assistantId: assistant_id, user: req.user.id }, true); const matchingActions = actions.filter((action) => { const metadata = action.metadata; if (!metadata) { @@ -157,14 +157,11 @@ async function domainParser(domain, inverse = false) { /** * Loads action sets based on the user and assistant ID. * - * @param {Object} searchParams - The parameters for loading action sets. - * @param {string} searchParams.user - The user identifier. - * @param {string} [searchParams.agent_id]- The agent identifier. - * @param {string} [searchParams.assistant_id]- The assistant identifier. + * @param {import('@librechat/data-schemas').ActionQuery} query - The criteria for loading action sets. * @returns {Promise} A promise that resolves to an array of actions or `null` if no match. */ -async function loadActionSets(searchParams) { - return await getActions(searchParams, true); +async function loadActionSets(query) { + return await getActions(query, true); } /** @@ -513,8 +510,8 @@ async function decryptMetadata(metadata) { */ const deleteAssistantActions = async ({ req, assistant_id }) => { try { - await deleteActions({ assistant_id, user: req.user.id }); - await deleteAssistant({ assistant_id, user: req.user.id }); + await deleteActions({ assistantId: assistant_id, user: req.user.id }); + await deleteAssistant({ assistantId: assistant_id, user: req.user.id }); } catch (error) { const message = 'Trouble deleting Assistant Actions for Assistant ID: ' + assistant_id; logger.error(message, error); diff --git a/api/server/services/Endpoints/assistants/build.js b/api/server/services/Endpoints/assistants/build.js index 85f7090211..c05fa9f249 100644 --- a/api/server/services/Endpoints/assistants/build.js +++ b/api/server/services/Endpoints/assistants/build.js @@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => { }); if (assistant_id) { - const assistantDoc = await getAssistant({ assistant_id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id }); if (assistantDoc) { // Create a clean assistant object with only the needed properties diff --git a/api/server/services/Endpoints/azureAssistants/build.js b/api/server/services/Endpoints/azureAssistants/build.js index 315447ed7f..bc04ae73d2 100644 --- a/api/server/services/Endpoints/azureAssistants/build.js +++ b/api/server/services/Endpoints/azureAssistants/build.js @@ -16,7 +16,7 @@ const buildOptions = async (endpoint, parsedBody) => { }); if (assistant_id) { - const assistantDoc = await getAssistant({ assistant_id }); + const assistantDoc = await getAssistant({ assistantId: assistant_id }); if (assistantDoc) { endpointOption.assistant = { append_current_datetime: assistantDoc.append_current_datetime, diff --git a/api/server/services/ToolService.js b/api/server/services/ToolService.js index 3fd5b38b8a..61fb486acb 100644 --- a/api/server/services/ToolService.js +++ b/api/server/services/ToolService.js @@ -199,7 +199,7 @@ const prepareActionSnapshotForTools = async ({ agentId, toolNames, filters, decr if (!toolNames.some((toolName) => isActionTool(toolName))) { return null; } - const storedActions = (await loadActionSets({ agent_id: agentId })) ?? []; + const storedActions = (await loadActionSets({ agentId })) ?? []; if (storedActions.length === 0) { return { storedActions, actionSets: [] }; } @@ -518,7 +518,7 @@ async function processRequiredActions(client, requiredActions) { /** @type {Action[]} */ const storedActions = (await loadActionSets({ - assistant_id: client.req.body.assistant_id, + assistantId: client.req.body.assistant_id, })) ?? []; const actionSets = await prepareStoredActionsForUse({ actions: storedActions, diff --git a/api/server/services/__tests__/ToolService.spec.js b/api/server/services/__tests__/ToolService.spec.js index 500017c591..825ed48356 100644 --- a/api/server/services/__tests__/ToolService.spec.js +++ b/api/server/services/__tests__/ToolService.spec.js @@ -2330,7 +2330,7 @@ describe('ToolService - Action Capability Gating', () => { definitionsOnly: false, }); - expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' }); + expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' }); }); it('blocks persisted action metadata before generic tool initialization', async () => { @@ -3013,7 +3013,7 @@ describe('ToolService - Action Capability Gating', () => { actionsEnabled: true, }); - expect(mockLoadActionSets).toHaveBeenCalledWith({ agent_id: 'agent_123' }); + expect(mockLoadActionSets).toHaveBeenCalledWith({ agentId: 'agent_123' }); }); it('blocks decrypted persisted action secrets before execution-time domain work', async () => { diff --git a/packages/api/src/agents/hitl/protection.ts b/packages/api/src/agents/hitl/protection.ts index c8ace4a5c5..e0dfedaa6c 100644 --- a/packages/api/src/agents/hitl/protection.ts +++ b/packages/api/src/agents/hitl/protection.ts @@ -165,7 +165,7 @@ export interface ResumeContentProtectionDependencies { getFiles: ResumeContentInspectionInput['getFiles']; getAgent: (filter: { id: string }) => Promise; getActions: ( - filter: { agent_id: { $in: string[] } }, + query: { agentId: string[] }, includeSensitive: boolean, ) => Promise; getUserMemories: (params: { userId: string; agentId?: string }) => Promise; @@ -642,7 +642,7 @@ async function getResumeActionSnapshots( ENCRYPTED_ACTION_METADATA_FIELDS, ); const actions = - (await dependencies.getActions({ agent_id: { $in: agentIds } }, needsEncryptedMetadata)) ?? []; + (await dependencies.getActions({ agentId: agentIds }, needsEncryptedMetadata)) ?? []; const withFunctions = actions.map((action) => { const rawSpec = action.metadata?.raw_spec; if (typeof rawSpec !== 'string') { diff --git a/packages/api/src/images/authorization.spec.ts b/packages/api/src/images/authorization.spec.ts index 66bf6a77c3..8f68909b8d 100644 --- a/packages/api/src/images/authorization.spec.ts +++ b/packages/api/src/images/authorization.spec.ts @@ -301,9 +301,11 @@ describe('createImageAuthorizationMiddleware', () => { expect(next).toHaveBeenCalledTimes(1); expect(deps.getAssistant).toHaveBeenCalledWith( { - 'avatar.filepath': { - $in: [assistantPath, `${assistantPath}?manual=false`, `${assistantPath}?manual=true`], - }, + avatarFilepath: [ + assistantPath, + `${assistantPath}?manual=false`, + `${assistantPath}?manual=true`, + ], }, { _id: 1, assistant_id: 1, endpoint: 1 }, ); diff --git a/packages/api/src/images/authorization.ts b/packages/api/src/images/authorization.ts index 87a7e26b2f..20f0b5cb09 100644 --- a/packages/api/src/images/authorization.ts +++ b/packages/api/src/images/authorization.ts @@ -8,7 +8,7 @@ import { runAsSystem, tenantStorage, } from '@librechat/data-schemas'; -import type { IAgent, IAssistant, SystemCapability } from '@librechat/data-schemas'; +import type { IAgent, IAssistant, AssistantQuery, SystemCapability } from '@librechat/data-schemas'; import type { Request, Response, NextFunction, RequestHandler } from 'express'; import type { FilterQuery, ProjectionType, Types } from 'mongoose'; @@ -72,7 +72,7 @@ export interface ImageAuthorizationDeps { projection?: ProjectionType, ) => Promise | null>; getAssistant: ( - query: FilterQuery, + query: AssistantQuery, projection?: ProjectionType, ) => Promise; getImageConfig?: (params: { userId: string; user: ImageUser }) => Promise<{ @@ -334,7 +334,7 @@ async function canViewAssistantAvatar( deps: ImageAuthorizationDeps, ): Promise { const assistant = await deps.getAssistant( - { 'avatar.filepath': { $in: getStoredPathCandidates(imagePath.canonicalPath) } }, + { avatarFilepath: getStoredPathCandidates(imagePath.canonicalPath) }, { _id: 1, assistant_id: 1, endpoint: 1 }, ); if (!assistant) { diff --git a/packages/data-schemas/src/methods/action.spec.ts b/packages/data-schemas/src/methods/action.spec.ts new file mode 100644 index 0000000000..7f03d251be --- /dev/null +++ b/packages/data-schemas/src/methods/action.spec.ts @@ -0,0 +1,125 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import type { IAction } from '~/types'; +import { createActionMethods } from './action'; +import { createModels } from '~/models'; + +jest.mock('~/config/winston', () => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +})); + +let mongoServer: InstanceType; +let methods: ReturnType; +let Action: mongoose.Model; + +const userA = new mongoose.Types.ObjectId(); +const userB = new mongoose.Types.ObjectId(); + +const seed = (overrides: Partial) => + Action.create({ + user: userA, + action_id: 'act-1', + type: 'action_prototype', + metadata: { domain: 'example.com', auth: { type: 'none', token_exchange_method: null } }, + ...overrides, + } as Partial); + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + Object.assign(mongoose.models, createModels(mongoose)); + Action = mongoose.models.Action; + methods = createActionMethods(mongoose); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await Action.deleteMany({}); +}); + +describe('action criteria', () => { + it('finds by a single action id', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-2', agent_id: 'agent-1' }); + + const found = await methods.getActions({ actionId: 'act-1' }, true); + + expect(found.map((a) => a.action_id)).toEqual(['act-1']); + }); + + it('finds by any of several ids', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-2', agent_id: 'agent-1' }); + await seed({ action_id: 'act-3', agent_id: 'agent-1' }); + + const found = await methods.getActions({ actionId: ['act-1', 'act-3'] }, true); + + expect(found.map((a) => a.action_id).sort()).toEqual(['act-1', 'act-3']); + }); + + it('combines criteria with AND', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-1', agent_id: 'agent-2' }); + + const found = await methods.getActions({ actionId: 'act-1', agentId: 'agent-2' }, true); + + expect(found).toHaveLength(1); + expect(found[0].agent_id).toBe('agent-2'); + }); + + it('scopes deletes to the owning user', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1', user: userA }); + await seed({ action_id: 'act-2', agent_id: 'agent-1', user: userB }); + + const deleted = await methods.deleteActions({ agentId: 'agent-1', user: String(userB) }); + + expect(deleted).toBe(1); + const remaining = await methods.getActions({}, true); + expect(remaining.map((a) => a.action_id)).toEqual(['act-1']); + }); + + it('rejects an unrecognized criterion instead of matching every action', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + + await expect( + methods.getActions({ action_id: 'act-1' } as unknown as { actionId?: string }, true), + ).rejects.toThrow("Unknown query criterion: 'action_id'"); + }); + + it('does not delete every action when given an unrecognized criterion', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-2', agent_id: 'agent-2' }); + + await expect( + methods.deleteActions({ agent_id: 'agent-1' } as unknown as { agentId?: string }), + ).rejects.toThrow("Unknown query criterion: 'agent_id'"); + await expect(Action.countDocuments({})).resolves.toBe(2); + }); + + it('does not delete every action when a misspelled criterion carries undefined', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-2', agent_id: 'agent-2' }); + + const absent: string | undefined = undefined; + await expect( + methods.deleteActions({ agent_id: absent } as unknown as { agentId?: string }), + ).rejects.toThrow("Unknown query criterion: 'agent_id'"); + await expect(Action.countDocuments({})).resolves.toBe(2); + }); + + it('still omits a recognized criterion the caller left undefined', async () => { + await seed({ action_id: 'act-1', agent_id: 'agent-1' }); + await seed({ action_id: 'act-2', agent_id: 'agent-2' }); + + const found = await methods.getActions({ actionId: 'act-1', agentId: undefined }, true); + + expect(found.map((a) => a.action_id)).toEqual(['act-1']); + }); +}); diff --git a/packages/data-schemas/src/methods/action.ts b/packages/data-schemas/src/methods/action.ts index b86509800f..e7e5c33824 100644 --- a/packages/data-schemas/src/methods/action.ts +++ b/packages/data-schemas/src/methods/action.ts @@ -1,42 +1,46 @@ import type { FilterQuery, Model } from 'mongoose'; -import type { IAction } from '~/types'; +import type { ActionQuery, IAction } from '~/types'; +import { buildFilter, type FieldMap } from '~/utils/criteria'; const sensitiveFields = ['api_key', 'oauth_client_id', 'oauth_client_secret'] as const; +const ACTION_FIELDS: FieldMap = { + actionId: 'action_id', + agentId: 'agent_id', + assistantId: 'assistant_id', + user: 'user', +}; + +/** Translates domain action criteria into a Mongo filter. */ +function actionFilter(query: ActionQuery): FilterQuery { + return buildFilter>(query, ACTION_FIELDS); +} + export function createActionMethods(mongoose: typeof import('mongoose')): { - getActions: ( - searchParams: FilterQuery, - includeSensitive?: boolean, - ) => Promise; - updateAction: ( - searchParams: FilterQuery, - updateData: Partial, - ) => Promise; - deleteAction: (searchParams: FilterQuery) => Promise; - deleteActions: (searchParams: FilterQuery) => Promise; + getActions: (query: ActionQuery, includeSensitive?: boolean) => Promise; + updateAction: (query: ActionQuery, updateData: Partial) => Promise; + deleteAction: (query: ActionQuery) => Promise; + deleteActions: (query: ActionQuery) => Promise; } { /** * Update an action with new data without overwriting existing properties, * or create a new action if it doesn't exist. */ async function updateAction( - searchParams: FilterQuery, + query: ActionQuery, updateData: Partial, ): Promise { const Action = mongoose.models.Action as Model; const options = { new: true, upsert: true }; - return await Action.findOneAndUpdate(searchParams, updateData, options).lean(); + return await Action.findOneAndUpdate(actionFilter(query), updateData, options).lean(); } /** - * Retrieves all actions that match the given search parameters. + * Retrieves all actions that match the given criteria. */ - async function getActions( - searchParams: FilterQuery, - includeSensitive = false, - ): Promise { + async function getActions(query: ActionQuery, includeSensitive = false): Promise { const Action = mongoose.models.Action as Model; - const actions = await Action.find(searchParams).lean(); + const actions = await Action.find(actionFilter(query)).lean(); if (!includeSensitive) { for (let i = 0; i < actions.length; i++) { @@ -57,19 +61,19 @@ export function createActionMethods(mongoose: typeof import('mongoose')): { } /** - * Deletes an action by params. + * Deletes an action by criteria. */ - async function deleteAction(searchParams: FilterQuery): Promise { + async function deleteAction(query: ActionQuery): Promise { const Action = mongoose.models.Action as Model; - return await Action.findOneAndDelete(searchParams).lean(); + return await Action.findOneAndDelete(actionFilter(query)).lean(); } /** - * Deletes actions by params. + * Deletes actions by criteria. */ - async function deleteActions(searchParams: FilterQuery): Promise { + async function deleteActions(query: ActionQuery): Promise { const Action = mongoose.models.Action as Model; - const result = await Action.deleteMany(searchParams); + const result = await Action.deleteMany(actionFilter(query)); return result.deletedCount; } diff --git a/packages/data-schemas/src/methods/agent.ts b/packages/data-schemas/src/methods/agent.ts index 13706a7782..ea8efb175c 100644 --- a/packages/data-schemas/src/methods/agent.ts +++ b/packages/data-schemas/src/methods/agent.ts @@ -8,7 +8,7 @@ import { } from 'librechat-data-provider'; import type { FilterQuery, Model, PipelineStage, ProjectionType, Types } from 'mongoose'; import type { AgentToolResources } from 'librechat-data-provider'; -import type { IAgent, IAclEntry } from '~/types'; +import type { IAgent, IAclEntry, ActionQuery } from '~/types'; import { withCodeEnvironmentReference } from './codeEnvironment'; import { filterExistingSkillIds } from './skill'; import logger from '~/config/winston'; @@ -116,10 +116,7 @@ export interface AgentDeps { /** Removes all ACL permissions for a resource. Injected from PermissionService. */ removeAllPermissions: (params: { resourceType: string; resourceId: unknown }) => Promise; /** Gets actions. Created by createActionMethods. */ - getActions: ( - searchParams: FilterQuery, - includeSensitive?: boolean, - ) => Promise; + getActions: (query: ActionQuery, includeSensitive?: boolean) => Promise; /** Returns resource IDs solely owned by the given user. From createAclEntryMethods. */ getSoleOwnedResourceIds: ( userObjectId: Types.ObjectId, @@ -802,7 +799,7 @@ export function createAgentMethods( if (actionIds.length > 0) { try { - const actions = await getActions({ action_id: { $in: actionIds } }, true); + const actions = await getActions({ actionId: actionIds }, true); actionsHash = await generateActionMetadataHash( currentAgent.actions, diff --git a/packages/data-schemas/src/methods/assistant.ts b/packages/data-schemas/src/methods/assistant.ts index 5751293f1c..00e94ebce4 100644 --- a/packages/data-schemas/src/methods/assistant.ts +++ b/packages/data-schemas/src/methods/assistant.ts @@ -1,20 +1,32 @@ import type { FilterQuery, Model, ProjectionType } from 'mongoose'; -import type { IAssistant } from '~/types'; +import type { AssistantQuery, IAssistant } from '~/types'; +import { buildFilter, type FieldMap } from '~/utils/criteria'; import { createIndexesWithRetry } from '~/utils/retry'; +const ASSISTANT_FIELDS: FieldMap = { + assistantId: 'assistant_id', + user: 'user', + avatarFilepath: 'avatar.filepath', +}; + +/** Translates domain assistant criteria into a Mongo filter. */ +function assistantFilter(query: AssistantQuery): FilterQuery { + return buildFilter>(query, ASSISTANT_FIELDS); +} + export function createAssistantMethods(mongoose: typeof import('mongoose')): { updateAssistantDoc: ( - searchParams: FilterQuery, + query: AssistantQuery, updateData: Partial, ) => Promise; - deleteAssistant: (searchParams: FilterQuery) => Promise; - deleteAssistants: (searchParams: FilterQuery) => Promise; + deleteAssistant: (query: AssistantQuery) => Promise; + deleteAssistants: (query: AssistantQuery) => Promise; getAssistants: ( - searchParams: FilterQuery, + query: AssistantQuery, select?: string | Record | null, ) => Promise; getAssistant: ( - searchParams: FilterQuery, + query: AssistantQuery, projection?: ProjectionType, ) => Promise; ensureAssistantIndexes: () => Promise; @@ -37,50 +49,50 @@ export function createAssistantMethods(mongoose: typeof import('mongoose')): { * or create a new assistant if it doesn't exist. */ async function updateAssistantDoc( - searchParams: FilterQuery, + query: AssistantQuery, updateData: Partial, ): Promise { const options = { new: true, upsert: true }; - return await Assistant().findOneAndUpdate(searchParams, updateData, options).lean(); + return await Assistant() + .findOneAndUpdate(assistantFilter(query), updateData, options) + .lean(); } /** * Retrieves an assistant document based on the provided search params. */ async function getAssistant( - searchParams: FilterQuery, + query: AssistantQuery, projection?: ProjectionType, ): Promise { await ensureAssistantIndexes(); - return await Assistant().findOne(searchParams, projection).lean(); + return await Assistant().findOne(assistantFilter(query), projection).lean(); } /** * Retrieves all assistants that match the given search parameters. */ async function getAssistants( - searchParams: FilterQuery, + query: AssistantQuery, select: string | Record | null = null, ): Promise { - const query = Assistant().find(searchParams); + const search = Assistant().find(assistantFilter(query)); - return await (select ? query.select(select) : query).lean(); + return await (select ? search.select(select) : search).lean(); } /** * Deletes an assistant based on the provided search params. */ - async function deleteAssistant( - searchParams: FilterQuery, - ): Promise { - return await Assistant().findOneAndDelete(searchParams); + async function deleteAssistant(query: AssistantQuery): Promise { + return await Assistant().findOneAndDelete(assistantFilter(query)); } /** * Deletes all assistants matching the given search parameters. */ - async function deleteAssistants(searchParams: FilterQuery): Promise { - const result = await Assistant().deleteMany(searchParams); + async function deleteAssistants(query: AssistantQuery): Promise { + const result = await Assistant().deleteMany(assistantFilter(query)); return result.deletedCount; } diff --git a/packages/data-schemas/src/schema/assistant.spec.ts b/packages/data-schemas/src/schema/assistant.spec.ts index 9d1edc064f..9cff86a0de 100644 --- a/packages/data-schemas/src/schema/assistant.spec.ts +++ b/packages/data-schemas/src/schema/assistant.spec.ts @@ -18,8 +18,8 @@ describe('assistantSchema', () => { } as unknown as typeof import('mongoose'); const methods = createAssistantMethods(mongoose); - await methods.getAssistant({ assistant_id: 'assistant-id' }); - await methods.getAssistant({ assistant_id: 'assistant-id' }); + await methods.getAssistant({ assistantId: 'assistant-id' }); + await methods.getAssistant({ assistantId: 'assistant-id' }); expect(createIndexes).toHaveBeenCalledTimes(1); }); diff --git a/packages/data-schemas/src/types/action.ts b/packages/data-schemas/src/types/action.ts index 841d6c95e5..29537b3939 100644 --- a/packages/data-schemas/src/types/action.ts +++ b/packages/data-schemas/src/types/action.ts @@ -1,4 +1,5 @@ import mongoose, { Document } from 'mongoose'; +import type { OneOrMany } from './query'; export interface IAction extends Document { user: mongoose.Types.ObjectId; @@ -27,3 +28,15 @@ export interface IAction extends Document { }; tenantId?: string; } + +/** + * Domain criteria for locating actions. Fields are combined with AND; an array + * value matches any of its entries. Deliberately storage-agnostic — it names + * domain concepts, not stored field names or query operators. + */ +export interface ActionQuery { + actionId?: OneOrMany; + agentId?: OneOrMany; + assistantId?: OneOrMany; + user?: string; +} diff --git a/packages/data-schemas/src/types/assistant.ts b/packages/data-schemas/src/types/assistant.ts index 065e0cd6b8..55e1430e8c 100644 --- a/packages/data-schemas/src/types/assistant.ts +++ b/packages/data-schemas/src/types/assistant.ts @@ -1,4 +1,5 @@ import { Document, Types } from 'mongoose'; +import type { OneOrMany } from './query'; export interface IAssistant extends Document { user: Types.ObjectId; @@ -15,3 +16,14 @@ export interface IAssistant extends Document { append_current_datetime?: boolean; tenantId?: string; } + +/** + * Domain criteria for locating assistants. Fields are combined with AND; an + * array value matches any of its entries. + */ +export interface AssistantQuery { + assistantId?: OneOrMany; + user?: string; + /** Matches the stored avatar path, used to authorize avatar image reads. */ + avatarFilepath?: OneOrMany; +} diff --git a/packages/data-schemas/src/types/index.ts b/packages/data-schemas/src/types/index.ts index 3dea872423..92ace38060 100644 --- a/packages/data-schemas/src/types/index.ts +++ b/packages/data-schemas/src/types/index.ts @@ -20,6 +20,7 @@ export * from './agentApiKey'; export * from './agentCategory'; export * from './codeEnvironment'; export * from './role'; +export * from './query'; export * from './action'; export * from './assistant'; export * from './file'; diff --git a/packages/data-schemas/src/types/query.ts b/packages/data-schemas/src/types/query.ts new file mode 100644 index 0000000000..adbff0188c --- /dev/null +++ b/packages/data-schemas/src/types/query.ts @@ -0,0 +1,7 @@ +/** + * A criterion that matches a single value or any one of several. + * + * Storage-agnostic on purpose: it is part of the domain query vocabulary, not + * of any engine's filter language. + */ +export type OneOrMany = T | T[]; diff --git a/packages/data-schemas/src/utils/criteria.spec.ts b/packages/data-schemas/src/utils/criteria.spec.ts new file mode 100644 index 0000000000..2862a6cc76 --- /dev/null +++ b/packages/data-schemas/src/utils/criteria.spec.ts @@ -0,0 +1,130 @@ +import { buildFilter, matchAny, type FieldMap } from './criteria'; + +type SampleQuery = { + actionId?: string | string[]; + agentId?: string | string[]; + user?: string; +}; + +const FIELDS: FieldMap = { + actionId: 'action_id', + agentId: 'agent_id', + user: 'user', +}; + +describe('matchAny', () => { + it('passes a scalar through unchanged', () => { + expect(matchAny('abc')).toBe('abc'); + }); + + it('translates a list into an $in expression', () => { + expect(matchAny(['a', 'b'])).toEqual({ $in: ['a', 'b'] }); + }); + + it('translates an empty list into an $in that matches nothing', () => { + expect(matchAny([])).toEqual({ $in: [] }); + }); +}); + +describe('buildFilter', () => { + it('maps criteria onto their stored field names', () => { + expect(buildFilter>({ actionId: 'a1' }, FIELDS)).toEqual({ + action_id: 'a1', + }); + }); + + it('combines multiple criteria', () => { + const filter = buildFilter>( + { agentId: 'agent-1', user: 'user-1' }, + FIELDS, + ); + expect(filter).toEqual({ agent_id: 'agent-1', user: 'user-1' }); + }); + + it('translates list criteria into $in', () => { + const filter = buildFilter>( + { agentId: ['a', 'b'] }, + FIELDS, + ); + expect(filter).toEqual({ agent_id: { $in: ['a', 'b'] } }); + }); + + it('omits criteria the caller left undefined', () => { + const filter = buildFilter>( + { actionId: 'a1', agentId: undefined }, + FIELDS, + ); + expect(filter).toEqual({ action_id: 'a1' }); + }); + + it('builds an empty filter from an empty query, so "list all" stays expressible', () => { + expect(buildFilter>({}, FIELDS)).toEqual({}); + }); + + it('throws on an unrecognized criterion rather than silently widening the filter', () => { + const query = { assistant_id: 'asst_1' } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Unknown query criterion: 'assistant_id'", + ); + }); + + it('throws even when a recognized criterion is also present', () => { + const query = { user: 'user-1', id: 'asst_1' } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Unknown query criterion: 'id'", + ); + }); + + it('throws on an unrecognized criterion whose value is undefined', () => { + /* A JS caller misspelling a criterion whose value is absent must not slip + through the undefined-omission rule and produce a filter matching everything. */ + const query = { agent_id: undefined } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Unknown query criterion: 'agent_id'", + ); + }); + + it('throws on a criterion that only resolves through the field map prototype', () => { + const query = { toString: 'x' } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Unknown query criterion: 'toString'", + ); + }); + + it('refuses a Mongo operator object smuggled in as a criterion value', () => { + const query = { agentId: { $ne: null } } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Invalid value for query criterion 'agentId'", + ); + }); + + it('refuses an operator object hidden inside a list criterion', () => { + const query = { agentId: ['a', { $ne: null }] } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Invalid value for query criterion 'agentId'", + ); + }); + + it('refuses null, which would match documents missing the field', () => { + const query = { user: null } as unknown as SampleQuery; + expect(() => buildFilter>(query, FIELDS)).toThrow( + "Invalid value for query criterion 'user'", + ); + }); + + it('accepts the scalar shapes a domain criterion may carry', () => { + type Mixed = { actionId?: string; count?: number; active?: boolean; since?: Date }; + const fields: FieldMap = { + actionId: 'action_id', + count: 'count', + active: 'active', + since: 'since', + }; + const since = new Date('2026-01-01T00:00:00.000Z'); + const filter = buildFilter>( + { actionId: 'a1', count: 2, active: true, since }, + fields, + ); + expect(filter).toEqual({ action_id: 'a1', count: 2, active: true, since }); + }); +}); diff --git a/packages/data-schemas/src/utils/criteria.ts b/packages/data-schemas/src/utils/criteria.ts new file mode 100644 index 0000000000..2f8f21d1b5 --- /dev/null +++ b/packages/data-schemas/src/utils/criteria.ts @@ -0,0 +1,78 @@ +import type { OneOrMany } from '~/types/query'; + +/** Maps every criterion of a domain query to the field it is stored under. */ +export type FieldMap = Readonly>; + +/** The value shapes a domain criterion may carry. */ +type CriterionValue = string | number | boolean | Date; + +function isCriterionValue(value: unknown): value is CriterionValue { + const type = typeof value; + return type === 'string' || type === 'number' || type === 'boolean' || value instanceof Date; +} + +/** + * Rejects anything that is not a scalar or a list of scalars. + * + * A criterion is a domain value, never a query fragment. Without this a + * JavaScript caller could pass `{ agentId: { $ne: null } }` and have the + * operator copied verbatim into the filter, widening the scope the field map + * was supposed to fix. + */ +function assertCriterionValue( + key: string, + value: unknown, +): asserts value is OneOrMany { + const values = Array.isArray(value) ? value : [value]; + for (const entry of values) { + if (!isCriterionValue(entry)) { + throw new Error( + `Invalid value for query criterion '${key}': expected a scalar or array of scalars`, + ); + } + } +} + +/** + * Translates a scalar-or-list criterion into a Mongo match expression. Domain + * query types express "any of these" as an array; the engine spells it. + */ +export function matchAny(value: OneOrMany): T | { $in: T[] } { + return Array.isArray(value) ? { $in: value } : value; +} + +/** + * Builds a Mongo filter from a domain query, omitting criteria the caller left + * undefined. + * + * Throws on any criterion the field map does not cover, and on any value that + * is not a scalar or list of scalars. Callers in `api/` are JavaScript and get + * no compile-time checking, and a silently accepted bad criterion would widen + * the filter — an unscoped `findOne` returns an arbitrary document rather than + * none, and an unscoped `deleteMany` removes every document — so this fails + * closed instead. + * + * Keys are validated before the undefined-omission rule is applied: a + * misspelled criterion that happens to carry `undefined` must still be + * rejected, not quietly skipped. The lookup is an own-property check so that + * inherited names like `toString` cannot resolve through the field map's + * prototype. + */ +export function buildFilter( + query: TQuery, + fields: FieldMap, +): TFilter { + const filter: Record = {}; + for (const key of Object.keys(query) as Array) { + if (!Object.prototype.hasOwnProperty.call(fields, key)) { + throw new Error(`Unknown query criterion: '${key}'`); + } + const value = query[key]; + if (value === undefined) { + continue; + } + assertCriterionValue(key, value); + filter[fields[key]] = matchAny(value); + } + return filter as TFilter; +}