From a95fea19bbbb1ec861891aca4fb4a9878e1226be Mon Sep 17 00:00:00 2001 From: David Newman Date: Wed, 14 Jan 2026 04:01:11 +1000 Subject: [PATCH] =?UTF-8?q?=F0=9F=8C=85=20fix:=20Agent=20Avatar=20S3=20URL?= =?UTF-8?q?=20Refresh=20Pagination=20and=20Persistence=20(#11323)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Refresh all S3 avatars for this user's accessible agent set, not the first page * Cleaner debug messages * Log errors as errors * refactor: avatar refresh logic to process agents in batches and improve error handling. Introduced new utility functions for refreshing S3 avatars and updating agent records. Updated tests to cover various scenarios including cache hits, user ownership checks, and error handling. Added constants for maximum refresh limits. * refactor: update avatar refresh logic to allow users with VIEW access to refresh avatars for all accessible agents. Removed checks for agent ownership and author presence, and updated related tests to reflect new behavior. * chore: Remove YouTube toolkit due to #11331 --------- Co-authored-by: Danny Avila --- api/server/controllers/agents/v1.js | 79 ++--- api/server/controllers/agents/v1.spec.js | 361 ++++++++++++++++++++++- api/server/services/start/tools.js | 3 +- packages/api/src/agents/avatars.spec.ts | 228 ++++++++++++++ packages/api/src/agents/avatars.ts | 122 ++++++++ packages/api/src/agents/index.ts | 1 + 6 files changed, 743 insertions(+), 51 deletions(-) create mode 100644 packages/api/src/agents/avatars.spec.ts create mode 100644 packages/api/src/agents/avatars.ts diff --git a/api/server/controllers/agents/v1.js b/api/server/controllers/agents/v1.js index 19a185279e..9f0a4a2279 100644 --- a/api/server/controllers/agents/v1.js +++ b/api/server/controllers/agents/v1.js @@ -5,7 +5,9 @@ const { logger } = require('@librechat/data-schemas'); const { agentCreateSchema, agentUpdateSchema, + refreshListAvatars, mergeAgentOcrConversion, + MAX_AVATAR_REFRESH_AGENTS, convertOcrToContextInPlace, } = require('@librechat/api'); const { @@ -56,46 +58,6 @@ const systemTools = { 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 @@ -544,6 +506,35 @@ const getListAgentsHandler = async (req, res) => { requiredPermissions: PermissionBits.VIEW, }); + /** + * Refresh all S3 avatars for this user's accessible agent set (not only the current page) + * This addresses page-size limits preventing refresh of agents beyond the first page + */ + const cache = getLogStores(CacheKeys.S3_EXPIRY_INTERVAL); + const refreshKey = `${userId}:agents_avatar_refresh`; + const alreadyChecked = await cache.get(refreshKey); + if (alreadyChecked) { + logger.debug('[/Agents] S3 avatar refresh already checked, skipping'); + } else { + try { + const fullList = await getListAgentsByAccess({ + accessibleIds, + otherParams: {}, + limit: MAX_AVATAR_REFRESH_AGENTS, + after: null, + }); + await refreshListAvatars({ + agents: fullList?.data ?? [], + userId, + refreshS3Url, + updateAgent, + }); + await cache.set(refreshKey, true, Time.THIRTY_MINUTES); + } catch (err) { + logger.error('[/Agents] Error refreshing avatars for full list: %o', err); + } + } + // Use the new ACL-aware function const data = await getListAgentsByAccess({ accessibleIds, @@ -571,15 +562,9 @@ const getListAgentsHandler = async (req, res) => { 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) { - logger.error('[/Agents] Error listing Agents', error); + logger.error('[/Agents] Error listing Agents: %o', error); res.status(500).json({ error: error.message }); } }; diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index 1bcf6c2fa3..8b2a57d903 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -1,8 +1,9 @@ const mongoose = require('mongoose'); -const { v4: uuidv4 } = require('uuid'); const { nanoid } = require('nanoid'); -const { MongoMemoryServer } = require('mongodb-memory-server'); +const { v4: uuidv4 } = require('uuid'); const { agentSchema } = require('@librechat/data-schemas'); +const { FileSources } = require('librechat-data-provider'); +const { MongoMemoryServer } = require('mongodb-memory-server'); // Only mock the dependencies that are not database-related jest.mock('~/server/services/Config', () => ({ @@ -54,6 +55,15 @@ jest.mock('~/models', () => ({ getCategoriesWithCounts: jest.fn(), })); +// Mock cache for S3 avatar refresh tests +const mockCache = { + get: jest.fn(), + set: jest.fn(), +}; +jest.mock('~/cache', () => ({ + getLogStores: jest.fn(() => mockCache), +})); + const { createAgent: createAgentHandler, updateAgent: updateAgentHandler, @@ -65,6 +75,8 @@ const { findPubliclyAccessibleResources, } = require('~/server/services/PermissionService'); +const { refreshS3Url } = require('~/server/services/Files/S3/crud'); + /** * @type {import('mongoose').Model} */ @@ -1207,4 +1219,349 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(response.data[0].is_promoted).toBe(true); }); }); + + describe('S3 Avatar Refresh', () => { + let userA, userB; + let agentWithS3Avatar, agentWithLocalAvatar, agentOwnedByOther; + + beforeEach(async () => { + await Agent.deleteMany({}); + jest.clearAllMocks(); + + // Reset cache mock + mockCache.get.mockResolvedValue(false); + mockCache.set.mockResolvedValue(undefined); + + userA = new mongoose.Types.ObjectId(); + userB = new mongoose.Types.ObjectId(); + + // Create agent with S3 avatar owned by userA + agentWithS3Avatar = await Agent.create({ + id: `agent_${nanoid(12)}`, + name: 'Agent with S3 Avatar', + description: 'Has S3 avatar', + provider: 'openai', + model: 'gpt-4', + author: userA, + avatar: { + source: FileSources.s3, + filepath: 'old-s3-path.jpg', + }, + versions: [ + { + name: 'Agent with S3 Avatar', + description: 'Has S3 avatar', + provider: 'openai', + model: 'gpt-4', + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + }); + + // Create agent with local avatar owned by userA + agentWithLocalAvatar = await Agent.create({ + id: `agent_${nanoid(12)}`, + name: 'Agent with Local Avatar', + description: 'Has local avatar', + provider: 'openai', + model: 'gpt-4', + author: userA, + avatar: { + source: 'local', + filepath: 'local-path.jpg', + }, + versions: [ + { + name: 'Agent with Local Avatar', + description: 'Has local avatar', + provider: 'openai', + model: 'gpt-4', + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + }); + + // Create agent with S3 avatar owned by userB + agentOwnedByOther = await Agent.create({ + id: `agent_${nanoid(12)}`, + name: 'Agent Owned By Other', + description: 'Owned by userB', + provider: 'openai', + model: 'gpt-4', + author: userB, + avatar: { + source: FileSources.s3, + filepath: 'other-s3-path.jpg', + }, + versions: [ + { + name: 'Agent Owned By Other', + description: 'Owned by userB', + provider: 'openai', + model: 'gpt-4', + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + }); + }); + + test('should skip avatar refresh if cache hit', async () => { + mockCache.get.mockResolvedValue(true); + findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Should not call refreshS3Url when cache hit + expect(refreshS3Url).not.toHaveBeenCalled(); + }); + + test('should refresh and persist S3 avatars on cache miss', async () => { + mockCache.get.mockResolvedValue(false); + findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockResolvedValue('new-s3-path.jpg'); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Verify S3 URL was refreshed + expect(refreshS3Url).toHaveBeenCalled(); + + // Verify cache was set + expect(mockCache.set).toHaveBeenCalled(); + + // Verify response was returned + expect(mockRes.json).toHaveBeenCalled(); + }); + + test('should refresh avatars for all accessible agents (VIEW permission)', async () => { + mockCache.get.mockResolvedValue(false); + // User A has access to both their own agent and userB's agent + findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id, agentOwnedByOther._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockResolvedValue('new-path.jpg'); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Should be called for both agents - any user with VIEW access can refresh + expect(refreshS3Url).toHaveBeenCalledTimes(2); + }); + + test('should skip non-S3 avatars', async () => { + mockCache.get.mockResolvedValue(false); + findAccessibleResources.mockResolvedValue([agentWithLocalAvatar._id, agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockResolvedValue('new-path.jpg'); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Should only be called for S3 avatar agent + expect(refreshS3Url).toHaveBeenCalledTimes(1); + }); + + test('should not update if S3 URL unchanged', async () => { + mockCache.get.mockResolvedValue(false); + findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + // Return the same path - no update needed + refreshS3Url.mockResolvedValue('old-s3-path.jpg'); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Verify refreshS3Url was called + expect(refreshS3Url).toHaveBeenCalled(); + + // Response should still be returned + expect(mockRes.json).toHaveBeenCalled(); + }); + + test('should handle S3 refresh errors gracefully', async () => { + mockCache.get.mockResolvedValue(false); + findAccessibleResources.mockResolvedValue([agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockRejectedValue(new Error('S3 error')); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + // Should not throw - handles error gracefully + await expect(getListAgentsHandler(mockReq, mockRes)).resolves.not.toThrow(); + + // Response should still be returned + expect(mockRes.json).toHaveBeenCalled(); + }); + + test('should process agents in batches', async () => { + mockCache.get.mockResolvedValue(false); + + // Create 25 agents (should be processed in batches of 20) + const manyAgents = []; + for (let i = 0; i < 25; i++) { + const agent = await Agent.create({ + id: `agent_${nanoid(12)}`, + name: `Agent ${i}`, + description: `Agent ${i} description`, + provider: 'openai', + model: 'gpt-4', + author: userA, + avatar: { + source: FileSources.s3, + filepath: `path${i}.jpg`, + }, + versions: [ + { + name: `Agent ${i}`, + description: `Agent ${i} description`, + provider: 'openai', + model: 'gpt-4', + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + }); + manyAgents.push(agent); + } + + const allAgentIds = manyAgents.map((a) => a._id); + findAccessibleResources.mockResolvedValue(allAgentIds); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockImplementation((avatar) => + Promise.resolve(avatar.filepath.replace('.jpg', '-new.jpg')), + ); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // All 25 should be processed + expect(refreshS3Url).toHaveBeenCalledTimes(25); + }); + + test('should skip agents without id or author', async () => { + mockCache.get.mockResolvedValue(false); + + // Create agent without proper id field (edge case) + const agentWithoutId = await Agent.create({ + id: `agent_${nanoid(12)}`, + name: 'Agent without ID field', + description: 'Testing', + provider: 'openai', + model: 'gpt-4', + author: userA, + avatar: { + source: FileSources.s3, + filepath: 'test-path.jpg', + }, + versions: [ + { + name: 'Agent without ID field', + description: 'Testing', + provider: 'openai', + model: 'gpt-4', + createdAt: new Date(), + updatedAt: new Date(), + }, + ], + }); + + findAccessibleResources.mockResolvedValue([agentWithoutId._id, agentWithS3Avatar._id]); + findPubliclyAccessibleResources.mockResolvedValue([]); + refreshS3Url.mockResolvedValue('new-path.jpg'); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Should still complete without errors + expect(mockRes.json).toHaveBeenCalled(); + }); + + test('should use MAX_AVATAR_REFRESH_AGENTS limit for full list query', async () => { + mockCache.get.mockResolvedValue(false); + findAccessibleResources.mockResolvedValue([]); + findPubliclyAccessibleResources.mockResolvedValue([]); + + const mockReq = { + user: { id: userA.toString(), role: 'USER' }, + query: {}, + }; + const mockRes = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + }; + + await getListAgentsHandler(mockReq, mockRes); + + // Verify that the handler completed successfully + expect(mockRes.json).toHaveBeenCalled(); + }); + }); }); diff --git a/api/server/services/start/tools.js b/api/server/services/start/tools.js index 4fd35755bc..dd2d69b274 100644 --- a/api/server/services/start/tools.js +++ b/api/server/services/start/tools.js @@ -5,7 +5,7 @@ const { Calculator } = require('@librechat/agents'); const { logger } = require('@librechat/data-schemas'); const { zodToJsonSchema } = require('zod-to-json-schema'); const { Tools, ImageVisionTool } = require('librechat-data-provider'); -const { getToolkitKey, oaiToolkit, ytToolkit, geminiToolkit } = require('@librechat/api'); +const { getToolkitKey, oaiToolkit, geminiToolkit } = require('@librechat/api'); const { toolkits } = require('~/app/clients/tools/manifest'); /** @@ -83,7 +83,6 @@ function loadAndFormatTools({ directory, adminFilter = [], adminIncluded = [] }) const basicToolInstances = [ new Calculator(), ...Object.values(oaiToolkit), - ...Object.values(ytToolkit), ...Object.values(geminiToolkit), ]; for (const toolInstance of basicToolInstances) { diff --git a/packages/api/src/agents/avatars.spec.ts b/packages/api/src/agents/avatars.spec.ts new file mode 100644 index 0000000000..ac97964837 --- /dev/null +++ b/packages/api/src/agents/avatars.spec.ts @@ -0,0 +1,228 @@ +import { FileSources } from 'librechat-data-provider'; +import type { Agent, AgentAvatar, AgentModelParameters } from 'librechat-data-provider'; +import type { RefreshS3UrlFn, UpdateAgentFn } from './avatars'; +import { + MAX_AVATAR_REFRESH_AGENTS, + AVATAR_REFRESH_BATCH_SIZE, + refreshListAvatars, +} from './avatars'; + +describe('refreshListAvatars', () => { + let mockRefreshS3Url: jest.MockedFunction; + let mockUpdateAgent: jest.MockedFunction; + const userId = 'user123'; + + beforeEach(() => { + mockRefreshS3Url = jest.fn(); + mockUpdateAgent = jest.fn(); + }); + + const createAgent = (overrides: Partial = {}): Agent => ({ + _id: 'obj1', + id: 'agent1', + name: 'Test Agent', + author: userId, + description: 'Test', + created_at: Date.now(), + avatar: { + source: FileSources.s3, + filepath: 'old-path.jpg', + }, + instructions: null, + provider: 'openai', + model: 'gpt-4', + model_parameters: {} as AgentModelParameters, + ...overrides, + }); + + it('should return empty stats for empty agents array', async () => { + const stats = await refreshListAvatars({ + agents: [], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.updated).toBe(0); + expect(mockRefreshS3Url).not.toHaveBeenCalled(); + expect(mockUpdateAgent).not.toHaveBeenCalled(); + }); + + it('should skip non-S3 avatars', async () => { + const agent = createAgent({ + avatar: { source: 'local', filepath: 'local-path.jpg' } as AgentAvatar, + }); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.not_s3).toBe(1); + expect(stats.updated).toBe(0); + expect(mockRefreshS3Url).not.toHaveBeenCalled(); + }); + + it('should skip agents without id', async () => { + const agent = createAgent({ id: '' }); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.no_id).toBe(1); + expect(mockRefreshS3Url).not.toHaveBeenCalled(); + }); + + it('should refresh avatars for agents owned by other users (VIEW access)', async () => { + const agent = createAgent({ author: 'otherUser' }); + mockRefreshS3Url.mockResolvedValue('new-path.jpg'); + mockUpdateAgent.mockResolvedValue({}); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.updated).toBe(1); + expect(mockRefreshS3Url).toHaveBeenCalled(); + expect(mockUpdateAgent).toHaveBeenCalled(); + }); + + it('should refresh and persist S3 avatars', async () => { + const agent = createAgent(); + mockRefreshS3Url.mockResolvedValue('new-path.jpg'); + mockUpdateAgent.mockResolvedValue({}); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.updated).toBe(1); + expect(mockRefreshS3Url).toHaveBeenCalledWith(agent.avatar); + expect(mockUpdateAgent).toHaveBeenCalledWith( + { id: 'agent1' }, + { avatar: { filepath: 'new-path.jpg', source: FileSources.s3 } }, + { updatingUserId: userId, skipVersioning: true }, + ); + }); + + it('should not update if S3 URL unchanged', async () => { + const agent = createAgent(); + mockRefreshS3Url.mockResolvedValue('old-path.jpg'); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.no_change).toBe(1); + expect(stats.updated).toBe(0); + expect(mockUpdateAgent).not.toHaveBeenCalled(); + }); + + it('should handle S3 refresh errors gracefully', async () => { + const agent = createAgent(); + mockRefreshS3Url.mockRejectedValue(new Error('S3 error')); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.s3_error).toBe(1); + expect(stats.updated).toBe(0); + }); + + it('should handle database persist errors gracefully', async () => { + const agent = createAgent(); + mockRefreshS3Url.mockResolvedValue('new-path.jpg'); + mockUpdateAgent.mockRejectedValue(new Error('DB error')); + + const stats = await refreshListAvatars({ + agents: [agent], + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.persist_error).toBe(1); + expect(stats.updated).toBe(0); + }); + + it('should process agents in batches', async () => { + const agents = Array.from({ length: 25 }, (_, i) => + createAgent({ + _id: `obj${i}`, + id: `agent${i}`, + avatar: { source: FileSources.s3, filepath: `path${i}.jpg` }, + }), + ); + + mockRefreshS3Url.mockImplementation((avatar) => + Promise.resolve(avatar.filepath.replace('.jpg', '-new.jpg')), + ); + mockUpdateAgent.mockResolvedValue({}); + + const stats = await refreshListAvatars({ + agents, + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.updated).toBe(25); + expect(mockRefreshS3Url).toHaveBeenCalledTimes(25); + expect(mockUpdateAgent).toHaveBeenCalledTimes(25); + }); + + it('should track mixed statistics correctly', async () => { + const agents = [ + createAgent({ id: 'agent1' }), + createAgent({ id: 'agent2', author: 'otherUser' }), + createAgent({ + id: 'agent3', + avatar: { source: 'local', filepath: 'local.jpg' } as AgentAvatar, + }), + createAgent({ id: '' }), // no id + ]; + + mockRefreshS3Url.mockResolvedValue('new-path.jpg'); + mockUpdateAgent.mockResolvedValue({}); + + const stats = await refreshListAvatars({ + agents, + userId, + refreshS3Url: mockRefreshS3Url, + updateAgent: mockUpdateAgent, + }); + + expect(stats.updated).toBe(2); // agent1 and agent2 (other user's agent now refreshed) + expect(stats.not_s3).toBe(1); // agent3 + expect(stats.no_id).toBe(1); // agent with empty id + }); +}); + +describe('Constants', () => { + it('should export MAX_AVATAR_REFRESH_AGENTS as 1000', () => { + expect(MAX_AVATAR_REFRESH_AGENTS).toBe(1000); + }); + + it('should export AVATAR_REFRESH_BATCH_SIZE as 20', () => { + expect(AVATAR_REFRESH_BATCH_SIZE).toBe(20); + }); +}); diff --git a/packages/api/src/agents/avatars.ts b/packages/api/src/agents/avatars.ts new file mode 100644 index 0000000000..7c92f352b2 --- /dev/null +++ b/packages/api/src/agents/avatars.ts @@ -0,0 +1,122 @@ +import { logger } from '@librechat/data-schemas'; +import { FileSources } from 'librechat-data-provider'; +import type { Agent, AgentAvatar } from 'librechat-data-provider'; + +const MAX_AVATAR_REFRESH_AGENTS = 1000; +const AVATAR_REFRESH_BATCH_SIZE = 20; + +export { MAX_AVATAR_REFRESH_AGENTS, AVATAR_REFRESH_BATCH_SIZE }; + +export type RefreshS3UrlFn = (avatar: AgentAvatar) => Promise; + +export type UpdateAgentFn = ( + searchParams: { id: string }, + updateData: { avatar: AgentAvatar }, + options: { updatingUserId: string; skipVersioning: boolean }, +) => Promise; + +export type RefreshListAvatarsParams = { + agents: Agent[]; + userId: string; + refreshS3Url: RefreshS3UrlFn; + updateAgent: UpdateAgentFn; +}; + +export type RefreshStats = { + updated: number; + not_s3: number; + no_id: number; + no_change: number; + s3_error: number; + persist_error: number; +}; + +/** + * Opportunistically refreshes S3-backed avatars for agent list responses. + * Processes agents in batches to prevent database connection pool exhaustion. + * 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 + * so we refresh once per interval at most. + * + * Any user with VIEW access to an agent can refresh its avatar URL. This ensures + * avatars remain accessible even when the owner hasn't logged in recently. + * The agents array should already be filtered to only include agents the user can access. + */ +export const refreshListAvatars = async ({ + agents, + userId, + refreshS3Url, + updateAgent, +}: RefreshListAvatarsParams): Promise => { + const stats: RefreshStats = { + updated: 0, + not_s3: 0, + no_id: 0, + no_change: 0, + s3_error: 0, + persist_error: 0, + }; + + if (!agents?.length) { + return stats; + } + + logger.debug('[refreshListAvatars] Refreshing S3 avatars for agents: %d', agents.length); + + for (let i = 0; i < agents.length; i += AVATAR_REFRESH_BATCH_SIZE) { + const batch = agents.slice(i, i + AVATAR_REFRESH_BATCH_SIZE); + + await Promise.all( + batch.map(async (agent) => { + if (agent?.avatar?.source !== FileSources.s3 || !agent?.avatar?.filepath) { + stats.not_s3++; + return; + } + + if (!agent?.id) { + logger.debug( + '[refreshListAvatars] Skipping S3 avatar refresh for agent: %s, ID is not set', + agent._id, + ); + stats.no_id++; + return; + } + + try { + logger.debug('[refreshListAvatars] Refreshing S3 avatar for agent: %s', agent._id); + const newPath = await refreshS3Url(agent.avatar); + + if (newPath && newPath !== agent.avatar.filepath) { + try { + await updateAgent( + { id: agent.id }, + { + avatar: { + filepath: newPath, + source: agent.avatar.source, + }, + }, + { + updatingUserId: userId, + skipVersioning: true, + }, + ); + stats.updated++; + } catch (persistErr) { + logger.error('[refreshListAvatars] Avatar refresh persist error: %o', persistErr); + stats.persist_error++; + } + } else { + stats.no_change++; + } + } catch (err) { + logger.error('[refreshListAvatars] S3 avatar refresh error: %o', err); + stats.s3_error++; + } + }), + ); + } + + logger.info('[refreshListAvatars] Avatar refresh summary: %o', stats); + return stats; +}; diff --git a/packages/api/src/agents/index.ts b/packages/api/src/agents/index.ts index 7f4be5f0ec..5efc22a397 100644 --- a/packages/api/src/agents/index.ts +++ b/packages/api/src/agents/index.ts @@ -1,3 +1,4 @@ +export * from './avatars'; export * from './chain'; export * from './edges'; export * from './initialize';