diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index d623603a28..b7b2dbf367 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -11,7 +11,6 @@ const { const { Tools, Constants, - SystemRoles, FileSources, ResourceType, AccessRoleIds, @@ -20,6 +19,8 @@ const { PermissionBits, actionDelimiter, removeNullishValues, + CacheKeys, + Time, } = require('librechat-data-provider'); const { getListAgentsByAccess, @@ -45,6 +46,7 @@ const { updateAction, getActions } = require('~/models/Action'); const { getCachedTools } = require('~/server/services/Config'); const { deleteFileByFilter } = require('~/models/File'); const { getCategoriesWithCounts } = require('~/models'); +const { getLogStores } = require('~/cache'); const systemTools = { [Tools.execute_code]: true, @@ -52,6 +54,49 @@ const systemTools = { [Tools.web_search]: true, }; +const MAX_SEARCH_LEN = 100; +const escapeRegex = (str = '') => str.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + +/** + * Opportunistically refreshes S3-backed avatars for agent list responses. + * Only list responses are refreshed because they're the highest-traffic surface and + * the avatar URLs have a short-lived TTL. The refresh is cached per-user for 30 minutes + * via {@link CacheKeys.S3_EXPIRY_INTERVAL} so we refresh once per interval at most. + * @param {Array} agents - Agents being enriched with S3-backed avatars + * @param {string} userId - User identifier used for the cache refresh key + */ +const refreshListAvatars = async (agents, userId) => { + if (!agents?.length) { + return; + } + + const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL); + const refreshKey = `${userId}:agents_list`; + const alreadyChecked = await cache.get(refreshKey); + if (alreadyChecked) { + return; + } + + await Promise.all( + agents.map(async (agent) => { + if (agent?.avatar?.source !== FileSources.s3 || !agent?.avatar?.filepath) { + return; + } + + try { + const newPath = await refreshS3Url(agent.avatar); + if (newPath && newPath !== agent.avatar.filepath) { + agent.avatar = { ...agent.avatar, filepath: newPath }; + } + } catch (err) { + logger.debug('[/Agents] Avatar refresh error for list item', err); + } + }), + ); + + await cache.set(refreshKey, true, Time.THIRTY_MINUTES); +}; + /** * Creates an Agent. * @route POST /Agents @@ -142,10 +187,13 @@ const getAgentHandler = async (req, res, expandProperties = false) => { agent.version = agent.versions ? agent.versions.length : 0; if (agent.avatar && agent.avatar?.source === FileSources.s3) { - const originalUrl = agent.avatar.filepath; - agent.avatar.filepath = await refreshS3Url(agent.avatar); - if (originalUrl !== agent.avatar.filepath) { - await updateAgent({ id }, { avatar: agent.avatar }, { updatingUserId: req.user.id }); + try { + agent.avatar = { + ...agent.avatar, + filepath: await refreshS3Url(agent.avatar), + }; + } catch (e) { + logger.warn('[/Agents/:id] Failed to refresh S3 URL', e); } } @@ -209,7 +257,12 @@ const updateAgentHandler = async (req, res) => { try { const id = req.params.id; const validatedData = agentUpdateSchema.parse(req.body); - const { _id, ...updateData } = removeNullishValues(validatedData); + // Preserve explicit null for avatar to allow resetting the avatar + const { avatar: avatarField, _id, ...rest } = validatedData; + const updateData = removeNullishValues(rest); + if (avatarField === null) { + updateData.avatar = avatarField; + } // Convert OCR to context in incoming updateData convertOcrToContextInPlace(updateData); @@ -342,21 +395,21 @@ const duplicateAgentHandler = async (req, res) => { const [domain] = action.action_id.split(actionDelimiter); const fullActionId = `${domain}${actionDelimiter}${newActionId}`; + // Sanitize sensitive metadata before persisting + const filteredMetadata = { ...(action.metadata || {}) }; + for (const field of sensitiveFields) { + delete filteredMetadata[field]; + } + const newAction = await updateAction( { action_id: newActionId }, { - metadata: action.metadata, + metadata: filteredMetadata, agent_id: newAgentId, user: userId, }, ); - const filteredMetadata = { ...newAction.metadata }; - for (const field of sensitiveFields) { - delete filteredMetadata[field]; - } - - newAction.metadata = filteredMetadata; newActionsList.push(newAction); return fullActionId; }; @@ -463,13 +516,13 @@ const getListAgentsHandler = async (req, res) => { filter.is_promoted = { $ne: true }; } - // Handle search filter + // Handle search filter (escape regex and cap length) if (search && search.trim() !== '') { - filter.$or = [ - { name: { $regex: search.trim(), $options: 'i' } }, - { description: { $regex: search.trim(), $options: 'i' } }, - ]; + const safeSearch = escapeRegex(search.trim().slice(0, MAX_SEARCH_LEN)); + const regex = new RegExp(safeSearch, 'i'); + filter.$or = [{ name: regex }, { description: regex }]; } + // Get agent IDs the user has VIEW access to via ACL const accessibleIds = await findAccessibleResources({ userId, @@ -477,10 +530,12 @@ const getListAgentsHandler = async (req, res) => { resourceType: ResourceType.AGENT, requiredPermissions: requiredPermission, }); + const publiclyAccessibleIds = await findPubliclyAccessibleResources({ resourceType: ResourceType.AGENT, requiredPermissions: PermissionBits.VIEW, }); + // Use the new ACL-aware function const data = await getListAgentsByAccess({ accessibleIds, @@ -488,13 +543,31 @@ const getListAgentsHandler = async (req, res) => { limit, after: cursor, }); - if (data?.data?.length) { - data.data = data.data.map((agent) => { - if (publiclyAccessibleIds.some((id) => id.equals(agent._id))) { + + const agents = data?.data ?? []; + if (!agents.length) { + return res.json(data); + } + + const publicSet = new Set(publiclyAccessibleIds.map((oid) => oid.toString())); + + data.data = agents.map((agent) => { + try { + if (agent?._id && publicSet.has(agent._id.toString())) { agent.isPublic = true; } - return agent; - }); + } catch (e) { + // Silently ignore mapping errors + void e; + } + return agent; + }); + + // Opportunistically refresh S3 avatar URLs for list results with caching + try { + await refreshListAvatars(data.data, req.user.id); + } catch (err) { + logger.debug('[/Agents] Skipping avatar refresh for list', err); } return res.json(data); } catch (error) { @@ -517,28 +590,21 @@ const getListAgentsHandler = async (req, res) => { const uploadAgentAvatarHandler = async (req, res) => { try { const appConfig = req.config; + if (!req.file) { + return res.status(400).json({ message: 'No file uploaded' }); + } filterFile({ req, file: req.file, image: true, isAvatar: true }); const { agent_id } = req.params; if (!agent_id) { return res.status(400).json({ message: 'Agent ID is required' }); } - const isAdmin = req.user.role === SystemRoles.ADMIN; const existingAgent = await getAgent({ id: agent_id }); if (!existingAgent) { return res.status(404).json({ error: 'Agent not found' }); } - const isAuthor = existingAgent.author.toString() === req.user.id.toString(); - const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor; - - if (!hasEditPermission) { - return res.status(403).json({ - error: 'You do not have permission to modify this non-collaborative agent', - }); - } - const buffer = await fs.readFile(req.file.path); const fileStrategy = getFileStrategy(appConfig, { isAvatar: true }); const resizedBuffer = await resizeAvatar({ @@ -571,8 +637,6 @@ const uploadAgentAvatarHandler = async (req, res) => { } } - const promises = []; - const data = { avatar: { filepath: image.filepath, @@ -580,17 +644,16 @@ const uploadAgentAvatarHandler = async (req, res) => { }, }; - promises.push( - await updateAgent({ id: agent_id }, data, { - updatingUserId: req.user.id, - }), - ); - - const resolved = await Promise.all(promises); - res.status(201).json(resolved[0]); + const updatedAgent = await updateAgent({ id: agent_id }, data, { + updatingUserId: req.user.id, + }); + res.status(201).json(updatedAgent); } catch (error) { const message = 'An error occurred while updating the Agent Avatar'; - logger.error(message, error); + logger.error( + `[/:agent_id/avatar] ${message} (${req.params?.agent_id ?? 'unknown agent'})`, + error, + ); res.status(500).json({ message }); } finally { try { @@ -629,21 +692,13 @@ const revertAgentVersionHandler = async (req, res) => { return res.status(400).json({ error: 'version_index is required' }); } - const isAdmin = req.user.role === SystemRoles.ADMIN; const existingAgent = await getAgent({ id }); if (!existingAgent) { return res.status(404).json({ error: 'Agent not found' }); } - const isAuthor = existingAgent.author.toString() === req.user.id.toString(); - const hasEditPermission = existingAgent.isCollaborative || isAdmin || isAuthor; - - if (!hasEditPermission) { - return res.status(403).json({ - error: 'You do not have permission to modify this non-collaborative agent', - }); - } + // Permissions are enforced via route middleware (ACL EDIT) const updatedAgent = await revertAgentVersion({ id }, version_index); diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index b8d4d50ee6..bfdee7eb79 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -47,6 +47,7 @@ jest.mock('~/server/services/PermissionService', () => ({ findPubliclyAccessibleResources: jest.fn().mockResolvedValue([]), grantPermission: jest.fn(), hasPublicPermission: jest.fn().mockResolvedValue(false), + checkPermission: jest.fn().mockResolvedValue(true), })); jest.mock('~/models', () => ({ @@ -573,6 +574,68 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(updatedAgent.version).toBe(agentInDb.versions.length); }); + test('should allow resetting avatar when value is explicitly null', async () => { + await Agent.updateOne( + { id: existingAgentId }, + { + avatar: { + filepath: 'https://example.com/avatar.png', + source: 's3', + }, + }, + ); + + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + avatar: null, + }; + + await updateAgentHandler(mockReq, mockRes); + + const updatedAgent = mockRes.json.mock.calls[0][0]; + expect(updatedAgent.avatar).toBeNull(); + + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(agentInDb.avatar).toBeNull(); + }); + + test('should ignore avatar field when value is undefined', async () => { + const originalAvatar = { + filepath: 'https://example.com/original.png', + source: 's3', + }; + await Agent.updateOne({ id: existingAgentId }, { avatar: originalAvatar }); + + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + avatar: undefined, + }; + + await updateAgentHandler(mockReq, mockRes); + + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(agentInDb.avatar.filepath).toBe(originalAvatar.filepath); + expect(agentInDb.avatar.source).toBe(originalAvatar.source); + }); + + test('should not bump version when no mutable fields change', async () => { + const existingAgent = await Agent.findOne({ id: existingAgentId }); + const originalVersionCount = existingAgent.versions.length; + + mockReq.user.id = existingAgentAuthorId.toString(); + mockReq.params.id = existingAgentId; + mockReq.body = { + avatar: undefined, + }; + + await updateAgentHandler(mockReq, mockRes); + + const agentInDb = await Agent.findOne({ id: existingAgentId }); + expect(agentInDb.versions.length).toBe(originalVersionCount); + }); + test('should handle validation errors properly', async () => { mockReq.user.id = existingAgentAuthorId.toString(); mockReq.params.id = existingAgentId; diff --git a/api/server/routes/agents/v1.js b/api/server/routes/agents/v1.js index ef0535c4db..1e4f1c0118 100644 --- a/api/server/routes/agents/v1.js +++ b/api/server/routes/agents/v1.js @@ -146,7 +146,15 @@ router.delete( * @param {number} req.body.version_index - Index of the version to revert to. * @returns {Agent} 200 - success response - application/json */ -router.post('/:id/revert', checkGlobalAgentShare, v1.revertAgentVersion); +router.post( + '/:id/revert', + checkGlobalAgentShare, + canAccessAgentResource({ + requiredPermission: PermissionBits.EDIT, + resourceIdParam: 'id', + }), + v1.revertAgentVersion, +); /** * Returns a list of agents. diff --git a/client/src/common/agents-types.ts b/client/src/common/agents-types.ts index 43448a478f..9ac6b440a3 100644 --- a/client/src/common/agents-types.ts +++ b/client/src/common/agents-types.ts @@ -41,4 +41,8 @@ export type AgentForm = { recursion_limit?: number; support_contact?: SupportContact; category: string; + // Avatar management fields + avatar_file?: File | null; + avatar_preview?: string | null; + avatar_action?: 'upload' | 'reset' | null; } & TAgentCapabilities; diff --git a/client/src/components/SidePanel/Agents/AgentAvatar.tsx b/client/src/components/SidePanel/Agents/AgentAvatar.tsx index ff396a6be2..bb1d44dfdc 100644 --- a/client/src/components/SidePanel/Agents/AgentAvatar.tsx +++ b/client/src/components/SidePanel/Agents/AgentAvatar.tsx @@ -1,202 +1,101 @@ -import { useState, useEffect, useRef } from 'react'; -import * as Popover from '@radix-ui/react-popover'; +import { useEffect, useCallback } from 'react'; import { useToastContext } from '@librechat/client'; -import { useQueryClient } from '@tanstack/react-query'; -import { - QueryKeys, - mergeFileConfig, - fileConfig as defaultFileConfig, -} from 'librechat-data-provider'; -import type { UseMutationResult } from '@tanstack/react-query'; -import type { - Agent, - AgentAvatar, - AgentCreateParams, - AgentListResponse, -} from 'librechat-data-provider'; -import { - useUploadAgentAvatarMutation, - useGetFileConfig, - allAgentViewAndEditQueryKeys, - invalidateAgentMarketplaceQueries, -} from '~/data-provider'; +import { useFormContext, useWatch } from 'react-hook-form'; +import { mergeFileConfig, fileConfig as defaultFileConfig } from 'librechat-data-provider'; +import type { AgentAvatar } from 'librechat-data-provider'; +import type { AgentForm } from '~/common'; import { AgentAvatarRender, NoImage, AvatarMenu } from './Images'; +import { useGetFileConfig } from '~/data-provider'; import { useLocalize } from '~/hooks'; -import { formatBytes } from '~/utils'; -function Avatar({ - agent_id = '', - avatar, - createMutation, -}: { - agent_id: string | null; - avatar: null | AgentAvatar; - createMutation: UseMutationResult; -}) { - const queryClient = useQueryClient(); - const [menuOpen, setMenuOpen] = useState(false); - const [previewUrl, setPreviewUrl] = useState(''); - const [progress, setProgress] = useState(1); - const [input, setInput] = useState(null); - const lastSeenCreatedId = useRef(null); +function Avatar({ avatar }: { avatar: AgentAvatar | null }) { + const localize = useLocalize(); + const { showToast } = useToastContext(); + const { control, setValue } = useFormContext(); + const avatarPreview = useWatch({ control, name: 'avatar_preview' }) ?? ''; + const avatarAction = useWatch({ control, name: 'avatar_action' }); const { data: fileConfig = defaultFileConfig } = useGetFileConfig({ select: (data) => mergeFileConfig(data), }); - const localize = useLocalize(); - const { showToast } = useToastContext(); - - const { mutate: uploadAvatar } = useUploadAgentAvatarMutation({ - onMutate: () => { - setProgress(0.4); - }, - onSuccess: (data) => { - if (lastSeenCreatedId.current !== createMutation.data?.id) { - lastSeenCreatedId.current = createMutation.data?.id ?? ''; - } - showToast({ message: localize('com_ui_upload_agent_avatar') }); - - setInput(null); - const newUrl = data.avatar?.filepath ?? ''; - setPreviewUrl(newUrl); - - ((keys) => { - keys.forEach((key) => { - const res = queryClient.getQueryData([QueryKeys.agents, key]); - - if (!res?.data) { - return; - } - - const agents = res.data.map((agent) => { - if (agent.id === agent_id) { - return { - ...agent, - ...data, - }; - } - return agent; - }); - - queryClient.setQueryData([QueryKeys.agents, key], { - ...res, - data: agents, - }); - }); - })(allAgentViewAndEditQueryKeys); - invalidateAgentMarketplaceQueries(queryClient); - setProgress(1); - }, - onError: (error) => { - console.error('Error:', error); - setInput(null); - setPreviewUrl(''); - showToast({ message: localize('com_ui_upload_error'), status: 'error' }); - setProgress(1); - }, - }); + // Derive whether agent has a remote avatar from the avatar prop + const hasRemoteAvatar = Boolean(avatar?.filepath); useEffect(() => { - if (input) { - const reader = new FileReader(); - reader.onloadend = () => { - setPreviewUrl(reader.result as string); - }; - reader.readAsDataURL(input); - } - }, [input]); - - useEffect(() => { - if (avatar && avatar.filepath) { - setPreviewUrl(avatar.filepath); - } else { - setPreviewUrl(''); - } - }, [avatar]); - - useEffect(() => { - /** Experimental: Condition to prime avatar upload before Agent Creation - * - If the createMutation state Id was last seen (current) and the createMutation is successful - * we can assume that the avatar upload has already been initiated and we can skip the upload - * - * The mutation state is not reset until the user deliberately selects a new agent or an agent is deleted - * - * This prevents the avatar from being uploaded multiple times before the user selects a new agent - * while allowing the user to upload to prime the avatar and other values before the agent is created. - */ - const sharedUploadCondition = !!( - createMutation.isSuccess && - input && - previewUrl && - previewUrl.includes('base64') - ); - if (sharedUploadCondition && lastSeenCreatedId.current === createMutation.data.id) { + if (avatarAction) { return; } - if (sharedUploadCondition && createMutation.data.id) { - const formData = new FormData(); - formData.append('file', input, input.name); - formData.append('agent_id', createMutation.data.id); - - uploadAvatar({ - agent_id: createMutation.data.id, - formData, - }); + if (avatar?.filepath && avatarPreview !== avatar.filepath) { + setValue('avatar_preview', avatar.filepath); } - }, [createMutation.data, createMutation.isSuccess, input, previewUrl, uploadAvatar]); - const handleFileChange = (event: React.ChangeEvent): void => { - const file = event.target.files?.[0]; - const sizeLimit = fileConfig.avatarSizeLimit ?? 0; + if (!avatar?.filepath && avatarPreview !== '') { + setValue('avatar_preview', ''); + } + }, [avatar?.filepath, avatarAction, avatarPreview, setValue]); - if (sizeLimit && file && file.size <= sizeLimit) { - setInput(file); - setMenuOpen(false); + const handleFileChange = useCallback( + (event: React.ChangeEvent) => { + const file = event.target.files?.[0]; + const sizeLimit = fileConfig.avatarSizeLimit ?? 0; - const currentId = agent_id ?? ''; - if (!currentId) { + if (!file) { return; } - const formData = new FormData(); - formData.append('file', file, file.name); - formData.append('agent_id', currentId); - - if (typeof avatar === 'object') { - formData.append('avatar', JSON.stringify(avatar)); + if (sizeLimit && file.size > sizeLimit) { + const limitInMb = sizeLimit / (1024 * 1024); + const displayLimit = Number.isInteger(limitInMb) + ? limitInMb + : parseFloat(limitInMb.toFixed(1)); + showToast({ + message: localize('com_ui_upload_invalid_var', { 0: displayLimit }), + status: 'error', + }); + return; } - uploadAvatar({ - agent_id: currentId, - formData, - }); - } else { - const megabytes = sizeLimit ? formatBytes(sizeLimit) : 2; - showToast({ - message: localize('com_ui_upload_invalid_var', { 0: megabytes + '' }), - status: 'error', - }); - } + const reader = new FileReader(); + reader.onloadend = () => { + setValue('avatar_file', file, { shouldDirty: true }); + setValue('avatar_preview', (reader.result as string) ?? '', { shouldDirty: true }); + setValue('avatar_action', 'upload', { shouldDirty: true }); + }; + reader.readAsDataURL(file); + }, + [fileConfig.avatarSizeLimit, localize, setValue, showToast], + ); - setMenuOpen(false); - }; + const handleReset = useCallback(() => { + const remoteAvatarExists = Boolean(avatar?.filepath); + setValue('avatar_preview', '', { shouldDirty: true }); + setValue('avatar_file', null, { shouldDirty: true }); + setValue('avatar_action', remoteAvatarExists ? 'reset' : null, { shouldDirty: true }); + }, [avatar?.filepath, setValue]); + + const hasIcon = Boolean(avatarPreview) || hasRemoteAvatar; + const canReset = hasIcon; return ( - + <>
- - - + + {avatarPreview ? : } + + } + handleFileChange={handleFileChange} + onReset={handleReset} + canReset={canReset} + />
- {} - + ); } diff --git a/client/src/components/SidePanel/Agents/AgentConfig.tsx b/client/src/components/SidePanel/Agents/AgentConfig.tsx index 342b8c0da7..abee8f0c23 100644 --- a/client/src/components/SidePanel/Agents/AgentConfig.tsx +++ b/client/src/components/SidePanel/Agents/AgentConfig.tsx @@ -2,7 +2,7 @@ import React, { useState, useMemo, useCallback } from 'react'; import { useToastContext } from '@librechat/client'; import { Controller, useWatch, useFormContext } from 'react-hook-form'; import { EModelEndpoint, getEndpointField } from 'librechat-data-provider'; -import type { AgentForm, AgentPanelProps, IconComponentTypes } from '~/common'; +import type { AgentForm, IconComponentTypes } from '~/common'; import { removeFocusOutlines, processAgentOption, @@ -37,7 +37,7 @@ const inputClass = cn( removeFocusOutlines, ); -export default function AgentConfig({ createMutation }: Pick) { +export default function AgentConfig() { const localize = useLocalize(); const fileMap = useFileMapContext(); const { showToast } = useToastContext(); @@ -183,11 +183,7 @@ export default function AgentConfig({ createMutation }: Pick {/* Avatar & Name */}
- +
); }; export function AvatarMenu({ + trigger, handleFileChange, + onReset, + canReset, }: { + trigger: ReactElement; handleFileChange: (event: React.ChangeEvent) => void; + onReset: () => void; + canReset: boolean; }) { const localize = useLocalize(); const fileInputRef = useRef(null); + const [isOpen, setIsOpen] = useState(false); const onItemClick = () => { if (fileInputRef.current) { @@ -98,40 +78,61 @@ export function AvatarMenu({ fileInputRef.current?.click(); }; + const uploadLabel = localize('com_ui_upload_image'); + + const items: MenuItemProps[] = [ + { + id: 'upload-avatar', + label: uploadLabel, + onClick: () => onItemClick(), + }, + ]; + + if (canReset) { + items.push( + { separate: true }, + { + id: 'reset-avatar', + label: localize('com_ui_reset_var', { 0: 'Avatar' }), + onClick: () => { + if (fileInputRef.current) { + fileInputRef.current.value = ''; + } + onReset(); + }, + }, + ); + } + return ( - - - - {/* - Use DALL·E - */} - - - + <> + } + items={items} + isOpen={isOpen} + setIsOpen={setIsOpen} + menuId="agent-avatar-menu" + placement="bottom" + gutter={8} + portal + mountByState + /> + { + handleFileChange(event); + if (fileInputRef.current) { + fileInputRef.current.value = ''; + } else { + event.currentTarget.value = ''; + } + }} + ref={fileInputRef} + tabIndex={-1} + /> + ); } diff --git a/client/src/components/SidePanel/Agents/__tests__/AgentAvatar.spec.tsx b/client/src/components/SidePanel/Agents/__tests__/AgentAvatar.spec.tsx new file mode 100644 index 0000000000..c12caf42d4 --- /dev/null +++ b/client/src/components/SidePanel/Agents/__tests__/AgentAvatar.spec.tsx @@ -0,0 +1,95 @@ +/** + * @jest-environment jsdom + */ +/* eslint-disable i18next/no-literal-string */ +import { describe, it, expect } from '@jest/globals'; +import { render, fireEvent } from '@testing-library/react'; +import { FormProvider, useForm, type UseFormReturn } from 'react-hook-form'; +import type { AgentForm } from '~/common'; +import AgentAvatar from '../AgentAvatar'; + +jest.mock('@librechat/client', () => ({ + useToastContext: () => ({ + showToast: jest.fn(), + }), +})); + +jest.mock('~/data-provider', () => ({ + useGetFileConfig: () => ({ + data: { avatarSizeLimit: 1024 * 1024 }, + }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +jest.mock('../Images', () => ({ + AgentAvatarRender: () =>
, + NoImage: () =>
, + AvatarMenu: ({ onReset }: { onReset: () => void }) => ( + + ), +})); + +const defaultFormValues: AgentForm = { + agent: undefined, + id: 'agent_123', + name: 'Agent', + description: null, + instructions: null, + model: 'gpt-4', + model_parameters: {}, + tools: [], + provider: 'openai', + agent_ids: [], + edges: [], + end_after_tools: false, + hide_sequential_outputs: false, + recursion_limit: undefined, + category: 'general', + support_contact: undefined, + artifacts: '', + execute_code: false, + file_search: false, + web_search: false, + avatar_file: null, + avatar_preview: '', + avatar_action: null, +}; + +describe('AgentAvatar reset menu', () => { + it('clears preview and file state when reset is triggered', () => { + let methodsRef: UseFormReturn; + const Wrapper = () => { + methodsRef = useForm({ + defaultValues: { + ...defaultFormValues, + avatar_preview: 'data:image/png;base64,abc', + avatar_file: new File(['avatar'], 'avatar.png', { type: 'image/png' }), + avatar_action: 'upload', + }, + }); + + return ( + + + + ); + }; + + const { getByTestId } = render(); + fireEvent.click(getByTestId('reset-avatar')); + + expect(methodsRef.getValues('avatar_preview')).toBe(''); + expect(methodsRef.getValues('avatar_file')).toBeNull(); + expect(methodsRef.getValues('avatar_action')).toBe('reset'); + }); +}); diff --git a/client/src/components/SidePanel/Agents/__tests__/AgentFooter.spec.tsx b/client/src/components/SidePanel/Agents/__tests__/AgentFooter.spec.tsx index d861baee9f..3425f5a75c 100644 --- a/client/src/components/SidePanel/Agents/__tests__/AgentFooter.spec.tsx +++ b/client/src/components/SidePanel/Agents/__tests__/AgentFooter.spec.tsx @@ -157,7 +157,7 @@ jest.mock('../DuplicateAgent', () => ({ ), })); -jest.mock('~/components', () => ({ +jest.mock('@librechat/client', () => ({ Spinner: () =>
, })); @@ -225,6 +225,7 @@ describe('AgentFooter', () => { updateMutation: mockUpdateMutation, setActivePanel: mockSetActivePanel, setCurrentAgentId: mockSetCurrentAgentId, + isAvatarUploading: false, }; beforeEach(() => { @@ -275,14 +276,14 @@ describe('AgentFooter', () => { expect(screen.queryByTestId('admin-settings')).not.toBeInTheDocument(); expect(screen.getByTestId('grant-access-dialog')).toBeInTheDocument(); expect(screen.getByTestId('duplicate-button')).toBeInTheDocument(); - expect(document.querySelector('.spinner')).not.toBeInTheDocument(); + expect(screen.queryByTestId('spinner')).not.toBeInTheDocument(); }); test('handles loading states for createMutation', () => { const { unmount } = render( , ); - expect(document.querySelector('.spinner')).toBeInTheDocument(); + expect(screen.getByTestId('spinner')).toBeInTheDocument(); expect(screen.queryByText('Save')).not.toBeInTheDocument(); // Find the submit button (the one with aria-busy attribute) const buttons = screen.getAllByRole('button'); @@ -294,9 +295,18 @@ describe('AgentFooter', () => { test('handles loading states for updateMutation', () => { render(); - expect(document.querySelector('.spinner')).toBeInTheDocument(); + expect(screen.getByTestId('spinner')).toBeInTheDocument(); expect(screen.queryByText('Save')).not.toBeInTheDocument(); }); + + test('handles loading state when avatar upload is in progress', () => { + render(); + expect(screen.getByTestId('spinner')).toBeInTheDocument(); + const buttons = screen.getAllByRole('button'); + const submitButton = buttons.find((button) => button.getAttribute('type') === 'submit'); + expect(submitButton).toBeDisabled(); + expect(submitButton).toHaveAttribute('aria-busy', 'true'); + }); }); describe('Conditional Rendering', () => { diff --git a/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts b/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts new file mode 100644 index 0000000000..988796cdc3 --- /dev/null +++ b/client/src/components/SidePanel/Agents/__tests__/AgentPanel.helpers.spec.ts @@ -0,0 +1,141 @@ +/** + * @jest-environment jsdom + */ +import { describe, it, expect, jest } from '@jest/globals'; +import { Constants, type Agent } from 'librechat-data-provider'; +import type { FieldNamesMarkedBoolean } from 'react-hook-form'; +import type { AgentForm } from '~/common'; +import { + composeAgentUpdatePayload, + persistAvatarChanges, + isAvatarUploadOnlyDirty, +} from '../AgentPanel'; + +const createForm = (): AgentForm => ({ + agent: undefined, + id: 'agent_123', + name: 'Agent', + description: null, + instructions: null, + model: 'gpt-4', + model_parameters: {}, + tools: [], + provider: 'openai', + agent_ids: [], + edges: [], + end_after_tools: false, + hide_sequential_outputs: false, + recursion_limit: undefined, + category: 'general', + support_contact: undefined, + artifacts: '', + execute_code: false, + file_search: false, + web_search: false, + avatar_file: null, + avatar_preview: '', + avatar_action: null, +}); + +describe('composeAgentUpdatePayload', () => { + it('includes avatar: null when resetting a persistent agent', () => { + const form = createForm(); + form.avatar_action = 'reset'; + + const { payload } = composeAgentUpdatePayload(form, 'agent_123'); + + expect(payload.avatar).toBeNull(); + }); + + it('omits avatar when resetting an ephemeral agent', () => { + const form = createForm(); + form.avatar_action = 'reset'; + + const { payload } = composeAgentUpdatePayload(form, Constants.EPHEMERAL_AGENT_ID); + + expect(payload.avatar).toBeUndefined(); + }); + + it('never adds avatar during upload actions', () => { + const form = createForm(); + form.avatar_action = 'upload'; + + const { payload } = composeAgentUpdatePayload(form, 'agent_123'); + + expect(payload.avatar).toBeUndefined(); + }); +}); + +describe('persistAvatarChanges', () => { + it('returns false for ephemeral agents', async () => { + const uploadAvatar = jest.fn(); + const result = await persistAvatarChanges({ + agentId: Constants.EPHEMERAL_AGENT_ID, + avatarActionState: 'upload', + avatarFile: new File(['avatar'], 'avatar.png', { type: 'image/png' }), + uploadAvatar, + }); + + expect(result).toBe(false); + expect(uploadAvatar).not.toHaveBeenCalled(); + }); + + it('returns false when no upload is pending', async () => { + const uploadAvatar = jest.fn(); + const result = await persistAvatarChanges({ + agentId: 'agent_123', + avatarActionState: null, + avatarFile: null, + uploadAvatar, + }); + + expect(result).toBe(false); + expect(uploadAvatar).not.toHaveBeenCalled(); + }); + + it('uploads avatar when all prerequisites are met', async () => { + const uploadAvatar = jest.fn().mockResolvedValue({} as Agent); + const file = new File(['avatar'], 'avatar.png', { type: 'image/png' }); + + const result = await persistAvatarChanges({ + agentId: 'agent_123', + avatarActionState: 'upload', + avatarFile: file, + uploadAvatar, + }); + + expect(result).toBe(true); + expect(uploadAvatar).toHaveBeenCalledTimes(1); + const callArgs = uploadAvatar.mock.calls[0][0]; + expect(callArgs.agent_id).toBe('agent_123'); + expect(callArgs.formData).toBeInstanceOf(FormData); + }); +}); + +describe('isAvatarUploadOnlyDirty', () => { + it('detects avatar-only dirty state', () => { + const dirtyFields = { + avatar_action: true, + avatar_preview: true, + } as FieldNamesMarkedBoolean; + + expect(isAvatarUploadOnlyDirty(dirtyFields)).toBe(true); + }); + + it('ignores agent field when checking dirty state', () => { + const dirtyFields = { + agent: { value: true } as any, + avatar_file: true, + } as FieldNamesMarkedBoolean; + + expect(isAvatarUploadOnlyDirty(dirtyFields)).toBe(true); + }); + + it('returns false when other fields are dirty', () => { + const dirtyFields = { + name: true, + } as FieldNamesMarkedBoolean; + + expect(isAvatarUploadOnlyDirty(dirtyFields)).toBe(false); + }); +}); diff --git a/client/src/data-provider/Agents/mutations.ts b/client/src/data-provider/Agents/mutations.ts index 7ffcfa10ba..f49c910c98 100644 --- a/client/src/data-provider/Agents/mutations.ts +++ b/client/src/data-provider/Agents/mutations.ts @@ -188,9 +188,41 @@ export const useUploadAgentAvatarMutation = ( t.AgentAvatarVariables, // request unknown // context > => { - return useMutation([MutationKeys.agentAvatarUpload], { + const queryClient = useQueryClient(); + return useMutation({ + mutationKey: [MutationKeys.agentAvatarUpload], mutationFn: (variables: t.AgentAvatarVariables) => dataService.uploadAgentAvatar(variables), - ...(options || {}), + onMutate: (variables) => options?.onMutate?.(variables), + onError: (error, variables, context) => options?.onError?.(error, variables, context), + onSuccess: (updatedAgent, variables, context) => { + ((keys: t.AgentListParams[]) => { + keys.forEach((key) => { + const listRes = queryClient.getQueryData([QueryKeys.agents, key]); + if (!listRes) { + return; + } + + queryClient.setQueryData([QueryKeys.agents, key], { + ...listRes, + data: listRes.data.map((agent) => { + if (agent.id === variables.agent_id) { + return updatedAgent; + } + return agent; + }), + }); + }); + })(allAgentViewAndEditQueryKeys); + + queryClient.setQueryData([QueryKeys.agent, variables.agent_id], updatedAgent); + queryClient.setQueryData( + [QueryKeys.agent, variables.agent_id, 'expanded'], + updatedAgent, + ); + invalidateAgentMarketplaceQueries(queryClient); + + return options?.onSuccess?.(updatedAgent, variables, context); + }, }); }; diff --git a/client/src/utils/forms.tsx b/client/src/utils/forms.tsx index 68e9db2fde..2b13388f46 100644 --- a/client/src/utils/forms.tsx +++ b/client/src/utils/forms.tsx @@ -52,6 +52,9 @@ export const getDefaultAgentFormValues = () => ({ ...defaultAgentFormValues, model: localStorage.getItem(LocalStorageKeys.LAST_AGENT_MODEL) ?? '', provider: createProviderOption(localStorage.getItem(LocalStorageKeys.LAST_AGENT_PROVIDER) ?? ''), + avatar_file: null, + avatar_preview: '', + avatar_action: null, }); export const processAgentOption = ({ diff --git a/packages/api/src/agents/validation.ts b/packages/api/src/agents/validation.ts index cbbdebb76e..4798ffeb80 100644 --- a/packages/api/src/agents/validation.ts +++ b/packages/api/src/agents/validation.ts @@ -81,6 +81,7 @@ export const agentCreateSchema = agentBaseSchema.extend({ /** Update schema extends base with all fields optional and additional update-only fields */ export const agentUpdateSchema = agentBaseSchema.extend({ + avatar: z.union([agentAvatarSchema, z.null()]).optional(), provider: z.string().optional(), model: z.string().nullable().optional(), projectIds: z.array(z.string()).optional(), diff --git a/packages/client/src/components/DropdownPopup.tsx b/packages/client/src/components/DropdownPopup.tsx index 30ed030677..a3014c8f7f 100644 --- a/packages/client/src/components/DropdownPopup.tsx +++ b/packages/client/src/components/DropdownPopup.tsx @@ -93,7 +93,7 @@ const Menu: React.FC = ({ .map((item, index) => { const { subItems } = item; if (item.separate === true) { - return ; + return ; } if (subItems && subItems.length > 0) { return (