From f00d968b5fae475118bf36c06dc9972c64be38b3 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 20 May 2026 13:40:40 -0400 Subject: [PATCH] fix: Clean up quota-blocked generated files --- api/server/services/Files/Code/crud.js | 44 ++++++++++ api/server/services/Files/Code/crud.spec.js | 52 ++++++++++- api/server/services/Files/process.js | 20 +++++ api/server/services/Files/process.spec.js | 96 ++++++++++++++++++++- api/server/services/Files/strategies.js | 4 +- packages/api/src/files/retention.spec.ts | 10 ++- packages/api/src/files/retention.ts | 3 + 7 files changed, 221 insertions(+), 8 deletions(-) diff --git a/api/server/services/Files/Code/crud.js b/api/server/services/Files/Code/crud.js index 035027b52d..0b8841002c 100644 --- a/api/server/services/Files/Code/crud.js +++ b/api/server/services/Files/Code/crud.js @@ -127,6 +127,49 @@ async function uploadCodeEnvFile({ req, stream, filename, kind, id, version }) { } } +/** + * Deletes a file from the Code Environment server. + * + * @param {ServerRequest} req - The authenticated request used for Code API auth. + * @param {import('librechat-data-provider').CodeEnvRef | { metadata?: { codeEnvRef?: import('librechat-data-provider').CodeEnvRef } }} file + * The code environment reference, or a file object containing one. + * @returns {Promise} + */ +async function deleteCodeEnvFile(req, file) { + const ref = file?.metadata?.codeEnvRef ?? file; + if (!ref?.storage_session_id || !ref?.file_id) { + return; + } + + try { + const baseURL = getCodeBaseURL(); + const query = buildCodeEnvDownloadQuery({ + kind: ref.kind, + id: ref.id, + ...(ref.kind === 'skill' ? { version: ref.version } : {}), + }); + const authHeaders = await getCodeApiAuthHeaders(req); + await axios({ + method: 'delete', + url: `${baseURL}/files/${ref.storage_session_id}/${ref.file_id}${query}`, + headers: { + 'User-Agent': 'LibreChat/1.0', + ...authHeaders, + }, + httpAgent: codeServerHttpAgent, + httpsAgent: codeServerHttpsAgent, + timeout: 15000, + }); + } catch (error) { + throw new Error( + logAxiosError({ + message: `Error deleting code environment file: ${error.message}`, + error, + }), + ); + } +} + /** * Uploads multiple files to the code execution environment in a single request. * Uses the /upload/batch endpoint which shares one session_id across all files. @@ -211,5 +254,6 @@ async function batchUploadCodeEnvFiles({ req, files, kind, id, version, read_onl module.exports = { getCodeOutputDownloadStream, uploadCodeEnvFile, + deleteCodeEnvFile, batchUploadCodeEnvFiles, }; diff --git a/api/server/services/Files/Code/crud.spec.js b/api/server/services/Files/Code/crud.spec.js index e26eb4dbbb..21239a08a7 100644 --- a/api/server/services/Files/Code/crud.spec.js +++ b/api/server/services/Files/Code/crud.spec.js @@ -61,7 +61,7 @@ const { codeServerHttpsAgent, getCodeApiAuthHeaders, } = require('@librechat/api'); -const { getCodeOutputDownloadStream, uploadCodeEnvFile } = require('./crud'); +const { getCodeOutputDownloadStream, uploadCodeEnvFile, deleteCodeEnvFile } = require('./crud'); describe('Code CRUD', () => { beforeEach(() => { @@ -327,4 +327,54 @@ describe('Code CRUD', () => { await expect(uploadCodeEnvFile(baseUploadParams)).rejects.toThrow(); }); }); + + describe('deleteCodeEnvFile', () => { + const req = { user: { id: 'user-123' } }; + const ref = { + kind: 'agent', + id: 'agent-123', + storage_session_id: 'sess-1', + file_id: 'fid-1', + }; + + it('deletes through the Code API with identity and auth headers', async () => { + getCodeApiAuthHeaders.mockResolvedValue({ Authorization: 'Bearer codeapi-token' }); + mockAxios.mockResolvedValue({ data: { message: 'File deleted successfully' } }); + + await deleteCodeEnvFile(req, ref); + + expect(getCodeApiAuthHeaders).toHaveBeenCalledWith(req); + expect(mockAxios).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'delete', + url: 'https://code-api.example.com/files/sess-1/fid-1?kind=agent&id=agent-123', + headers: expect.objectContaining({ + Authorization: 'Bearer codeapi-token', + 'User-Agent': 'LibreChat/1.0', + }), + httpAgent: codeServerHttpAgent, + httpsAgent: codeServerHttpsAgent, + timeout: 15000, + }), + ); + }); + + it('accepts a file object containing metadata.codeEnvRef', async () => { + mockAxios.mockResolvedValue({ data: { message: 'File deleted successfully' } }); + + await deleteCodeEnvFile(req, { metadata: { codeEnvRef: ref } }); + + expect(mockAxios).toHaveBeenCalledWith( + expect.objectContaining({ + url: 'https://code-api.example.com/files/sess-1/fid-1?kind=agent&id=agent-123', + }), + ); + }); + + it('no-ops when a code environment ref is missing', async () => { + await deleteCodeEnvFile(req, { metadata: {} }); + + expect(mockAxios).not.toHaveBeenCalled(); + }); + }); }); diff --git a/api/server/services/Files/process.js b/api/server/services/Files/process.js index f11d3f4bd5..c8b94c9474 100644 --- a/api/server/services/Files/process.js +++ b/api/server/services/Files/process.js @@ -123,6 +123,23 @@ const cleanupVectorFile = async ({ req, file }) => { } }; +const cleanupCodeEnvFile = async ({ req, file }) => { + if (!file?.metadata?.codeEnvRef) { + return; + } + + const { deleteFile } = getStrategyFunctions(FileSources.execute_code); + if (!deleteFile) { + return; + } + + try { + await deleteFile(req, file); + } catch (cleanupError) { + logger.error('[fileStorageLimit] Failed to clean up over-limit code env file:', cleanupError); + } +}; + const cleanupPersistedFile = async ({ req, file, openai }) => { await cleanupStoredFile({ req, file, openai }); if (!file?.file_id) { @@ -1158,6 +1175,9 @@ const processAgentFileUpload = async ({ req, res, metadata }) => { if (tool_resource === EToolResources.file_search && embedded) { await cleanupVectorFile({ req, file: fileInfo }); } + if (tool_resource === EToolResources.execute_code) { + await cleanupCodeEnvFile({ req, file: fileInfo }); + } } throw error; } diff --git a/api/server/services/Files/process.spec.js b/api/server/services/Files/process.spec.js index 89db793b7c..e14400f306 100644 --- a/api/server/services/Files/process.spec.js +++ b/api/server/services/Files/process.spec.js @@ -657,17 +657,19 @@ describe('processAgentFileUpload', () => { * runs in the same flow. Both must return a working * `handleFileUpload`. */ const codeEnvUpload = jest.fn().mockResolvedValue(uploaded); + const codeEnvDelete = jest.fn().mockResolvedValue(undefined); const localUpload = jest.fn().mockResolvedValue({ bytes: 0, filename: 'upload.bin', filepath: '/uploads/upload.bin', }); + const localDelete = jest.fn().mockResolvedValue(undefined); getStrategyFunctions.mockImplementation((src) => src === FileSources.execute_code - ? { handleFileUpload: codeEnvUpload } - : { handleFileUpload: localUpload, saveBuffer: jest.fn() }, + ? { handleFileUpload: codeEnvUpload, deleteFile: codeEnvDelete } + : { handleFileUpload: localUpload, deleteFile: localDelete, saveBuffer: jest.fn() }, ); - return codeEnvUpload; + return { codeEnvUpload, codeEnvDelete, localDelete }; }; it('persists kind:user codeEnvRef for chat attachments (messageAttachment=true)', async () => { @@ -744,6 +746,51 @@ describe('processAgentFileUpload', () => { const persisted = db.createFile.mock.calls[0][0]; expect(persisted.metadata).not.toHaveProperty('fileIdentifier'); }); + + it('rolls back code env uploads when final storage quota rejects', async () => { + const error = Object.assign(new Error('storage limit exceeded.'), { + code: 'FILE_STORAGE_LIMIT_EXCEEDED', + status: 413, + }); + const { codeEnvDelete, localDelete } = setupCodeEnvUpload({ + storage_session_id: 'sess-4', + file_id: 'fid-4', + }); + assertFileStorageLimit.mockResolvedValueOnce(undefined).mockRejectedValueOnce(error); + const req = makeReq(); + + await expect( + processAgentFileUpload({ + req, + res: mockRes, + metadata: { + agent_id: 'agent-abc', + tool_resource: EToolResources.execute_code, + file_id: 'file-uuid', + }, + }), + ).rejects.toThrow('storage limit exceeded'); + + expect(localDelete).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ filepath: '/uploads/upload.bin' }), + undefined, + ); + expect(codeEnvDelete).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + metadata: { + codeEnvRef: { + kind: 'agent', + id: 'agent-abc', + storage_session_id: 'sess-4', + file_id: 'fid-4', + }, + }, + }), + ); + expect(db.createFile).not.toHaveBeenCalled(); + }); }); }); @@ -1069,6 +1116,49 @@ describe('processFileURL', () => { expect(db.createFile).not.toHaveBeenCalled(); }); + it('passes request path config to local URL cleanup on storage limit failure', async () => { + const error = Object.assign(new Error('storage limit exceeded.'), { + code: 'FILE_STORAGE_LIMIT_EXCEEDED', + status: 413, + }); + const paths = { publicPath: '/srv/public', uploads: '/srv/uploads' }; + const saveURL = jest.fn().mockResolvedValue({ + filepath: '/images/user-123/image.png', + bytes: 512, + type: 'image/png', + }); + const deleteFile = jest.fn().mockResolvedValue(undefined); + getStrategyFunctions.mockReturnValue({ saveURL, deleteFile }); + assertFileStorageLimit.mockRejectedValueOnce(error); + + await expect( + processFileURL({ + fileStrategy: FileSources.local, + userId: 'user-123', + URL: 'https://example.com/image.png', + fileName: 'image.png', + basePath: 'images', + context: FileContext.image_generation, + tenantId: 'tenant-a', + req: { + user: { id: 'user-123', tenantId: 'tenant-a' }, + body: {}, + config: { fileConfig: { storageLimit: 1 }, paths }, + }, + }), + ).rejects.toThrow('storage limit exceeded'); + + expect(deleteFile).toHaveBeenCalledWith( + expect.objectContaining({ config: expect.objectContaining({ paths }) }), + expect.objectContaining({ + filepath: '/images/user-123/image.png', + source: FileSources.local, + }), + undefined, + ); + expect(db.createFile).not.toHaveBeenCalled(); + }); + it('applies retention metadata for generated images when retention mode is all', async () => { getRetentionExpiry.mockResolvedValueOnce({ expiredAt: new Date('2030-01-01T00:00:00.000Z'), diff --git a/api/server/services/Files/strategies.js b/api/server/services/Files/strategies.js index e5acbd6903..8ea4d07c33 100644 --- a/api/server/services/Files/strategies.js +++ b/api/server/services/Files/strategies.js @@ -72,7 +72,7 @@ const { processAzureAvatar, } = require('./Azure'); const { uploadOpenAIFile, deleteOpenAIFile, getOpenAIFileStream } = require('./OpenAI'); -const { getCodeOutputDownloadStream, uploadCodeEnvFile } = require('./Code'); +const { getCodeOutputDownloadStream, uploadCodeEnvFile, deleteCodeEnvFile } = require('./Code'); const { uploadVectors, deleteVectors } = require('./VectorDB'); /** @@ -222,7 +222,7 @@ const codeOutputStrategy = () => ({ /** @type {typeof prepareImagesLocal | null} */ prepareImagePayload: null, /** @type {typeof deleteLocalFile | null} */ - deleteFile: null, + deleteFile: deleteCodeEnvFile, handleFileUpload: uploadCodeEnvFile, getDownloadStream: getCodeOutputDownloadStream, }); diff --git a/packages/api/src/files/retention.spec.ts b/packages/api/src/files/retention.spec.ts index 65ede6a5b1..80eafca283 100644 --- a/packages/api/src/files/retention.spec.ts +++ b/packages/api/src/files/retention.spec.ts @@ -215,16 +215,22 @@ describe('retention helpers', () => { }); it('creates minimal retention requests for tool calls', () => { + const paths = { + imageOutput: '/srv/public/images', + publicPath: '/srv/public', + uploads: '/srv/uploads', + }; + expect( createMinimalRetentionRequest({ user: { id: 'user-1', tenantId: 'tenant-1' }, body: { conversationId: 'convo-1', isTemporary: 'true' }, - config: { interfaceConfig: { retentionMode: RetentionMode.TEMPORARY } }, + config: { interfaceConfig: { retentionMode: RetentionMode.TEMPORARY }, paths }, }), ).toEqual({ user: { id: 'user-1', tenantId: 'tenant-1' }, body: { conversationId: 'convo-1', isTemporary: 'true' }, - config: { interfaceConfig: { retentionMode: RetentionMode.TEMPORARY } }, + config: { interfaceConfig: { retentionMode: RetentionMode.TEMPORARY }, paths }, }); expect(createMinimalRetentionRequest()).toBeUndefined(); diff --git a/packages/api/src/files/retention.ts b/packages/api/src/files/retention.ts index 7d53584191..940665eeb1 100644 --- a/packages/api/src/files/retention.ts +++ b/packages/api/src/files/retention.ts @@ -4,6 +4,7 @@ import type { AppConfig } from '@librechat/data-schemas'; type InterfaceConfig = AppConfig['interfaceConfig']; type FileConfig = AppConfig['fileConfig']; +type PathsConfig = AppConfig['paths']; const retentionExpiryCache = new WeakMap< RetentionRequest, @@ -28,6 +29,7 @@ export type RetentionRequest = { }; config?: { interfaceConfig?: InterfaceConfig; + paths?: PathsConfig; fileConfig?: FileConfig; }; }; @@ -228,6 +230,7 @@ export const createMinimalRetentionRequest = ( }, config: { interfaceConfig: req.config?.interfaceConfig, + ...(req.config?.paths ? { paths: req.config.paths } : {}), ...(req.config?.fileConfig ? { fileConfig: req.config.fileConfig } : {}), }, };