mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 14:57:42 +00:00
🧹 fix: Clean Up Orphaned Agent File Stubs After Deletion (#12781)
* 🧹 fix: Prune Orphaned File References on File Deletion Deleting a file via the Manage Files tab left its file_id in every agent's tool_resources.*.file_ids. Stubs accumulate until the frontend dedupe keys them as duplicates and blocks all new uploads (issue #12776). - Add removeAgentResourceFilesFromAllAgents in packages/data-schemas: a single updateMany/$pullAll across every EToolResources category. - Invoke it from processDeleteRequest after db.deleteFiles so every referencing agent is cleaned up, not just the one passed in req.body. - Wrap the cleanup in try/catch so a stale agent update cannot mask a successful file deletion. * 🧼 fix: Prune Orphaned File References on Agent Update Already-affected agents would stay broken even after the delete-time fix: the stubs sit on the agent document until something strips them. Heal them on the next save (issue #12776). - Add collectToolResourceFileIds + stripFileIdsFromToolResources helpers in @librechat/api — centralizing the tool_resources traversal used by the controller and the follow-up migration script. - In updateAgentHandler, check the effective tool_resources against the files collection. When orphans are found, either strip them from the incoming tool_resources (if the update sets them) or run the bulk cleanup (if the update leaves tool_resources untouched). * 🧰 chore: Add Migration to Clean Up Orphaned Agent File References Complements the delete-time and save-time fixes by healing agents that already accumulated orphan stubs before the upgrade (issue #12776). The script is idempotent — re-running it on a clean database is a no-op. - Add config/migrate-orphaned-agent-files.js following the existing migrate-*.js convention: --dry-run by default omitted (writes by default) and --batch-size= tuning knob. Streams agents via cursor. - Register migrate:orphaned-agent-files and :dry-run npm scripts. - Reuse collectToolResourceFileIds from @librechat/api so migration and runtime share the same traversal logic. * 🩹 fix: Address Codex/Copilot Review on Orphaned Agent File Cleanup Refines the #12776 fix series based on automated review feedback. - Scope save-time pruning to the current agent only. When a PATCH carries tool_resources, strip orphans from the incoming payload and pay the DB round-trip only then. Removes the collection-wide updateMany previously triggered when tool_resources was absent (Codex P2 / Copilot). - Wrap the orphan check in try/catch so a transient db.getFiles failure can't turn a good save into a 500 (comprehensive review #1). - Replace Object.values(EToolResources) casts with an explicit list of agent-side categories in both orphans.ts and agent.ts. code_interpreter belongs to the Assistants API and isn't a key of AgentToolResources — including it was a type lie and generated dead MongoDB clauses (comprehensive review #3, #8). - Export TOOL_RESOURCE_KEYS from @librechat/api and consume it in the migration script, dropping one duplicated definition (#4). - Cap migration results.details at 50 sample entries so the memory footprint stays bounded on deployments with thousands of corrupted agents (Codex P3). - Add migrate:orphaned-agent-files:batch npm script to match the convention set by migrate-agent-permissions / migrate-prompt-permissions (#7). - Add controller-level tests covering the three orphan-pruning paths: strip from incoming tool_resources, leave alone when tool_resources is absent, swallow db.getFiles errors and still save (#6). - Back pre-existing "should validate tool_resources in updates" test's file_ids with real File docs — the new pruning would otherwise strip them, and that test is about OCR conversion / schema filtering, not file existence. Register the File model in beforeAll so the fixture works. * 🩹 fix: Tighten TOOL_RESOURCE_KEYS Type and Align Migration Sample Output Two follow-ups from the second review pass. - Type data-schemas' TOOL_RESOURCE_KEYS as ReadonlyArray<keyof AgentToolResources> instead of readonly string[]. Data-schemas depends on data-provider, so the import is clean. Catches typos and aligns with the matching export in @librechat/api — doesn't guarantee exhaustiveness, but that's a TypeScript limitation, not a workspace one. - Align the migration's console output with DETAIL_SAMPLE_LIMIT: print every collected detail (up to 50) and, when more agents were affected than the sample size allowed, show a truncation notice. The old hard cap of 25 meant affected agents in the 26-50 range were collected but never shown. * ✅ test: Add Integration Coverage for Orphan Cleanup Paths (#12776) Exercise the delete-time and migration paths end-to-end against a real in-memory Mongo. Catches integration bugs the isolated unit tests on each layer couldn't. - api/server/services/Files/process.integration.spec.js — the primary repro: seed an Agent + File, call processDeleteRequest, assert the file_id disappears from every referencing agent's tool_resources while unrelated agents stay untouched. Also covers the no-op case and confirms a failure in the new cleanup step cannot roll back the file deletion itself. - api/test/migrate-orphaned-agent-files.spec.js — drives the migration module: --dry-run reports without writing, apply mode prunes across every tool_resource category, re-running is idempotent, and DETAIL_SAMPLE_LIMIT caps the in-memory sample on wide corruption. Mocks only the connect helper (the spec owns the mongoose instance) so the real migration code path — cursor, $pullAll, reduce — runs. * 🔒 fix: Run Orphan Cleanup Migration in System Tenant Context Codex P2 catch: under TENANT_ISOLATION_STRICT=true, the migration throws on the very first Agent.countDocuments() because the tenant isolation plugin fail-closes on queries without tenant context — which makes migrate:orphaned-agent-files unusable on the exact deployments most likely to have accumulated corruption. - Wrap the scan/prune body in runAsSystem so queries bypass the tenant filter (SYSTEM_TENANT_ID sentinel). The migration legitimately needs cross-tenant visibility — this is the same pattern seedDatabase and the S3 refresh job already use. - Add a regression test that spies on Agent.countDocuments() and asserts the active tenantStorage context is SYSTEM_TENANT_ID during the call. Pins the wrap against future regressions without the brittleness of toggling the strict-mode env var (which caches on first read). Note: the delete-time and save-time paths already run inside an authenticated HTTP request where tenantStorage.run is set by auth middleware, so the cleanup naturally scopes to the current tenant — which is the correct behavior there since file ownership is tenant-scoped. * 🧹 chore: Drop Unused path Import From Process Integration Spec Leftover from an earlier iteration that resolved the migration path via path.resolve before I switched to a relative require. The import does nothing now — removing it.
This commit is contained in:
Danny Avila
GitHub
parent
40742f9191
commit
181d705579
12 changed files with 978 additions and 3 deletions
|
|
@ -10,7 +10,9 @@ const {
|
|||
collectEdgeAgentIds,
|
||||
mergeAgentOcrConversion,
|
||||
MAX_AVATAR_REFRESH_AGENTS,
|
||||
collectToolResourceFileIds,
|
||||
convertOcrToContextInPlace,
|
||||
stripFileIdsFromToolResources,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Time,
|
||||
|
|
@ -387,6 +389,38 @@ const updateAgentHandler = async (req, res) => {
|
|||
updateData.tools = ocrConversion.tools;
|
||||
}
|
||||
|
||||
/*
|
||||
* Strip orphaned file_id stubs from the incoming payload (see issue #12776).
|
||||
* Scoped to updates that actually touch tool_resources: if the save does not
|
||||
* modify that field, the delete-time cleanup in processDeleteRequest and the
|
||||
* one-off migration already cover pre-existing corruption, so there's no
|
||||
* reason to pay an extra DB round-trip here. Wrapped in try/catch so a
|
||||
* transient failure in this integrity check never turns a good save into 500.
|
||||
*/
|
||||
if (updateData.tool_resources) {
|
||||
try {
|
||||
const referencedFileIds = collectToolResourceFileIds(updateData.tool_resources);
|
||||
if (referencedFileIds.length > 0) {
|
||||
const existingFiles = await db.getFiles({ file_id: { $in: referencedFileIds } }, null, {
|
||||
file_id: 1,
|
||||
});
|
||||
const existingIds = new Set((existingFiles ?? []).map((f) => f.file_id));
|
||||
const orphans = referencedFileIds.filter((id) => !existingIds.has(id));
|
||||
if (orphans.length > 0) {
|
||||
logger.warn(
|
||||
`[/Agents/:id] Pruning ${orphans.length} orphaned file reference(s) from agent ${id}`,
|
||||
);
|
||||
stripFileIdsFromToolResources(updateData.tool_resources, orphans);
|
||||
}
|
||||
}
|
||||
} catch (orphanCheckError) {
|
||||
logger.warn(
|
||||
'[/Agents/:id] Orphan file check failed, skipping cleanup for this request',
|
||||
orphanCheckError,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if (updateData.tools) {
|
||||
const existingToolSet = new Set(existingAgent.tools ?? []);
|
||||
const newMCPTools = updateData.tools.filter(
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
const mongoose = require('mongoose');
|
||||
const { nanoid } = require('nanoid');
|
||||
const { v4: uuidv4 } = require('uuid');
|
||||
const { agentSchema } = require('@librechat/data-schemas');
|
||||
const { agentSchema, fileSchema } = require('@librechat/data-schemas');
|
||||
const { FileSources, PermissionBits } = require('librechat-data-provider');
|
||||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
|
||||
|
|
@ -99,6 +99,9 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
const mongoUri = mongoServer.getUri();
|
||||
await mongoose.connect(mongoUri);
|
||||
Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema);
|
||||
// Register File so orphan-pruning tests (and the tool_resources validation
|
||||
// test, which now needs real File docs for its ids) have a working model.
|
||||
mongoose.models.File || mongoose.model('File', fileSchema);
|
||||
}, 20000);
|
||||
|
||||
afterAll(async () => {
|
||||
|
|
@ -542,6 +545,23 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
});
|
||||
|
||||
test('should validate tool_resources in updates', async () => {
|
||||
// Back these ids with real File docs so the orphan-pruning added for
|
||||
// issue #12776 does not strip them — this test is about OCR conversion
|
||||
// and schema filtering, not file existence.
|
||||
const File = mongoose.models.File;
|
||||
for (const id of ['ocr1', 'ocr2', 'img1']) {
|
||||
await File.create({
|
||||
file_id: id,
|
||||
user: existingAgentAuthorId,
|
||||
filename: `${id}.txt`,
|
||||
filepath: `/tmp/${id}`,
|
||||
object: 'file',
|
||||
type: 'text/plain',
|
||||
bytes: 1,
|
||||
source: FileSources.local,
|
||||
});
|
||||
}
|
||||
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = {
|
||||
|
|
@ -729,6 +749,93 @@ describe('Agent Controllers - Mass Assignment Protection', () => {
|
|||
}),
|
||||
);
|
||||
});
|
||||
|
||||
describe('orphan file_id pruning (issue #12776)', () => {
|
||||
const File = () => mongoose.models.File;
|
||||
|
||||
const createFileDoc = async (file_id, userId) =>
|
||||
File().create({
|
||||
file_id,
|
||||
user: userId,
|
||||
filename: `${file_id}.txt`,
|
||||
filepath: `/tmp/${file_id}`,
|
||||
object: 'file',
|
||||
type: 'text/plain',
|
||||
bytes: 1,
|
||||
source: FileSources.local,
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await File().deleteMany({});
|
||||
});
|
||||
|
||||
test('strips orphan file_ids from incoming tool_resources before persisting', async () => {
|
||||
const keeper = `file_${uuidv4()}`;
|
||||
const orphan = `file_${uuidv4()}`;
|
||||
await createFileDoc(keeper, existingAgentAuthorId);
|
||||
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = {
|
||||
tool_resources: {
|
||||
file_search: { file_ids: [keeper, orphan] },
|
||||
},
|
||||
};
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
|
||||
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([keeper]);
|
||||
});
|
||||
|
||||
test('leaves tool_resources alone when the update omits it', async () => {
|
||||
const orphan = `file_${uuidv4()}`;
|
||||
await Agent.updateOne(
|
||||
{ id: existingAgentId },
|
||||
{ $set: { tool_resources: { file_search: { file_ids: [orphan] } } } },
|
||||
);
|
||||
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = { name: 'Unrelated Rename' };
|
||||
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
|
||||
expect(agentInDb.name).toBe('Unrelated Rename');
|
||||
// Save-time pruning is intentionally scoped to tool_resources updates.
|
||||
// The delete-time fix and migration script cover the untouched case.
|
||||
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([orphan]);
|
||||
});
|
||||
|
||||
test('swallows errors from the file-existence check and still completes the save', async () => {
|
||||
const db = require('~/models');
|
||||
const originalGetFiles = db.getFiles;
|
||||
db.getFiles = jest.fn().mockRejectedValue(new Error('transient DB error'));
|
||||
|
||||
const orphan = `file_${uuidv4()}`;
|
||||
mockReq.user.id = existingAgentAuthorId.toString();
|
||||
mockReq.params.id = existingAgentId;
|
||||
mockReq.body = {
|
||||
name: 'Save Succeeds',
|
||||
tool_resources: { file_search: { file_ids: [orphan] } },
|
||||
};
|
||||
|
||||
try {
|
||||
await updateAgentHandler(mockReq, mockRes);
|
||||
|
||||
expect(mockRes.status).not.toHaveBeenCalledWith(500);
|
||||
expect(mockRes.json).toHaveBeenCalled();
|
||||
const agentInDb = await Agent.findOne({ id: existingAgentId }).lean();
|
||||
expect(agentInDb.name).toBe('Save Succeeds');
|
||||
// Cleanup skipped on error, so the id remains — the delete-time path
|
||||
// or the next successful save will reconcile it.
|
||||
expect(agentInDb.tool_resources.file_search.file_ids).toEqual([orphan]);
|
||||
} finally {
|
||||
db.getFiles = originalGetFiles;
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('Mass Assignment Attack Scenarios', () => {
|
||||
|
|
|
|||
203
api/server/services/Files/process.integration.spec.js
Normal file
203
api/server/services/Files/process.integration.spec.js
Normal file
|
|
@ -0,0 +1,203 @@
|
|||
/**
|
||||
* Integration test for the delete-time path of issue #12776.
|
||||
*
|
||||
* Covers the full flow through `processDeleteRequest`:
|
||||
* 1. Real Agent + File docs in an in-memory Mongo.
|
||||
* 2. Invoke the delete service.
|
||||
* 3. Assert both the File record is gone and every agent's
|
||||
* tool_resources.*.file_ids no longer references the deleted id.
|
||||
*
|
||||
* Uses FileSources.text so the strategy layer (disk / S3 / OpenAI) is a
|
||||
* no-op — we don't need real filesystem access to exercise the agent
|
||||
* reference cleanup, which is what issue #12776 is about.
|
||||
*/
|
||||
|
||||
const mongoose = require('mongoose');
|
||||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
const { agentSchema, fileSchema, createMethods } = require('@librechat/data-schemas');
|
||||
const { FileSources } = require('librechat-data-provider');
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => {
|
||||
const actual = jest.requireActual('@librechat/data-schemas');
|
||||
return {
|
||||
...actual,
|
||||
logger: { warn: jest.fn(), debug: jest.fn(), error: jest.fn(), info: jest.fn() },
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('@librechat/agents', () => ({
|
||||
EnvVar: { CODE_API_KEY: 'CODE_API_KEY' },
|
||||
}));
|
||||
|
||||
jest.mock('@librechat/api', () => ({
|
||||
sanitizeFilename: jest.fn((n) => n),
|
||||
parseText: jest.fn().mockResolvedValue({ text: '', bytes: 0 }),
|
||||
processAudioFile: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/controllers/assistants/v2', () => ({
|
||||
addResourceFileId: jest.fn(),
|
||||
deleteResourceFileId: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/controllers/assistants/helpers', () => ({
|
||||
getOpenAIClient: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Tools/credentials', () => ({
|
||||
loadAuthValues: jest.fn(),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/strategies', () => ({
|
||||
getStrategyFunctions: jest.fn(() => ({ deleteFile: jest.fn().mockResolvedValue(undefined) })),
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Files/Audio/STTService', () => ({
|
||||
STTService: { getInstance: jest.fn() },
|
||||
}));
|
||||
|
||||
jest.mock('~/server/services/Config', () => ({
|
||||
checkCapability: jest.fn().mockResolvedValue(true),
|
||||
}));
|
||||
|
||||
jest.mock('~/cache', () => ({
|
||||
getLogStores: jest.fn(() => ({ get: jest.fn(), set: jest.fn(), delete: jest.fn() })),
|
||||
}));
|
||||
|
||||
// Replace the mocked `~/models` from the sibling process.spec.js with real,
|
||||
// mongoose-backed methods. All our in-memory models share this module.
|
||||
jest.mock('~/models', () => {
|
||||
const mongoose = require('mongoose');
|
||||
const { createMethods } = require('@librechat/data-schemas');
|
||||
return createMethods(mongoose, {
|
||||
removeAllPermissions: jest.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
});
|
||||
|
||||
require('module-alias/register');
|
||||
const { processDeleteRequest } = require('./process');
|
||||
|
||||
describe('processDeleteRequest — agent reference cleanup (issue #12776)', () => {
|
||||
let mongoServer;
|
||||
let Agent;
|
||||
let File;
|
||||
|
||||
beforeAll(async () => {
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
await mongoose.connect(mongoServer.getUri());
|
||||
|
||||
// createMethods (via ~/models) registers the File model as a side-effect,
|
||||
// but we also need the Agent model registered before any queries run.
|
||||
Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema);
|
||||
File = mongoose.models.File || mongoose.model('File', fileSchema);
|
||||
// Touch createMethods once so the migration/setup side-effects run.
|
||||
createMethods(mongoose, { removeAllPermissions: jest.fn() });
|
||||
}, 30000);
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await Agent.deleteMany({});
|
||||
await File.deleteMany({});
|
||||
});
|
||||
|
||||
const seedFile = async (file_id, userId) =>
|
||||
File.create({
|
||||
file_id,
|
||||
user: userId,
|
||||
filename: `${file_id}.txt`,
|
||||
filepath: `/tmp/${file_id}`,
|
||||
object: 'file',
|
||||
type: 'text/plain',
|
||||
bytes: 1,
|
||||
source: FileSources.text,
|
||||
});
|
||||
|
||||
const seedAgent = async (authorId, tool_resources) =>
|
||||
Agent.create({
|
||||
id: `agent_${Math.random().toString(36).slice(2, 10)}`,
|
||||
name: 'Integration Test Agent',
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: authorId,
|
||||
tool_resources,
|
||||
});
|
||||
|
||||
const buildReq = (fileDocs, extraBody = {}) => ({
|
||||
user: { id: fileDocs[0].user.toString() },
|
||||
body: { files: fileDocs, ...extraBody },
|
||||
config: { fileStrategy: 'local', fileConfig: {}, endpoints: {} },
|
||||
});
|
||||
|
||||
test('strips deleted file_ids from every agent that referenced them', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const keeperId = `file_keeper_${Math.random().toString(36).slice(2, 10)}`;
|
||||
const deletedId = `file_deleted_${Math.random().toString(36).slice(2, 10)}`;
|
||||
|
||||
const deletedFile = await seedFile(deletedId, userId);
|
||||
await seedFile(keeperId, userId);
|
||||
|
||||
// Two agents both reference the file that's about to be deleted, plus the
|
||||
// keeper. A third, unrelated agent has a different file_id and must not be
|
||||
// touched by the cleanup.
|
||||
const agentA = await seedAgent(userId, {
|
||||
file_search: { file_ids: [deletedId, keeperId] },
|
||||
});
|
||||
const agentB = await seedAgent(userId, {
|
||||
execute_code: { file_ids: [deletedId] },
|
||||
});
|
||||
const untouchedAgent = await seedAgent(userId, {
|
||||
context: { file_ids: [keeperId] },
|
||||
});
|
||||
|
||||
await processDeleteRequest({ req: buildReq([deletedFile.toObject()]), files: [deletedFile] });
|
||||
|
||||
expect(await File.findOne({ file_id: deletedId })).toBeNull();
|
||||
expect(await File.findOne({ file_id: keeperId })).not.toBeNull();
|
||||
|
||||
const updatedA = await Agent.findOne({ id: agentA.id }).lean();
|
||||
const updatedB = await Agent.findOne({ id: agentB.id }).lean();
|
||||
const updatedUntouched = await Agent.findOne({ id: untouchedAgent.id }).lean();
|
||||
|
||||
expect(updatedA.tool_resources.file_search.file_ids).toEqual([keeperId]);
|
||||
expect(updatedB.tool_resources.execute_code.file_ids).toEqual([]);
|
||||
expect(updatedUntouched.tool_resources.context.file_ids).toEqual([keeperId]);
|
||||
});
|
||||
|
||||
test('is a no-op when no agent references the deleted file', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const loneId = `file_lone_${Math.random().toString(36).slice(2, 10)}`;
|
||||
const loneFile = await seedFile(loneId, userId);
|
||||
const unrelatedAgent = await seedAgent(userId, {
|
||||
file_search: { file_ids: ['other_id'] },
|
||||
});
|
||||
|
||||
await processDeleteRequest({ req: buildReq([loneFile.toObject()]), files: [loneFile] });
|
||||
|
||||
expect(await File.findOne({ file_id: loneId })).toBeNull();
|
||||
const after = await Agent.findOne({ id: unrelatedAgent.id }).lean();
|
||||
expect(after.tool_resources.file_search.file_ids).toEqual(['other_id']);
|
||||
});
|
||||
|
||||
test('still deletes the file when the agent cleanup step throws', async () => {
|
||||
const userId = new mongoose.Types.ObjectId();
|
||||
const targetId = `file_target_${Math.random().toString(36).slice(2, 10)}`;
|
||||
const targetFile = await seedFile(targetId, userId);
|
||||
|
||||
const db = require('~/models');
|
||||
const original = db.removeAgentResourceFilesFromAllAgents;
|
||||
db.removeAgentResourceFilesFromAllAgents = jest
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('simulated cleanup failure'));
|
||||
|
||||
try {
|
||||
await processDeleteRequest({ req: buildReq([targetFile.toObject()]), files: [targetFile] });
|
||||
expect(await File.findOne({ file_id: targetId })).toBeNull();
|
||||
} finally {
|
||||
db.removeAgentResourceFilesFromAllAgents = original;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -219,6 +219,14 @@ const processDeleteRequest = async ({ req, files }) => {
|
|||
|
||||
await Promise.allSettled(promises);
|
||||
await db.deleteFiles(resolvedFileIds);
|
||||
|
||||
if (resolvedFileIds.length > 0) {
|
||||
try {
|
||||
await db.removeAgentResourceFilesFromAllAgents({ file_ids: resolvedFileIds });
|
||||
} catch (error) {
|
||||
logger.error('Error cleaning up orphaned agent file references', error);
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
|
|||
184
api/test/migrate-orphaned-agent-files.spec.js
Normal file
184
api/test/migrate-orphaned-agent-files.spec.js
Normal file
|
|
@ -0,0 +1,184 @@
|
|||
/**
|
||||
* Integration test for the orphan-cleanup migration script used to heal
|
||||
* agents corrupted before the delete-time and save-time fixes for issue
|
||||
* #12776 shipped. Exercises the full module end-to-end:
|
||||
* - dry-run reports orphans without writing
|
||||
* - apply mode removes them
|
||||
* - re-running on a cleaned database is a no-op (idempotent)
|
||||
* - the DETAIL_SAMPLE_LIMIT truncation kicks in on wide corruption
|
||||
*/
|
||||
|
||||
// Replace the migration's `./connect` helper — it opens its own connection
|
||||
// via the mongo URI env var, but the test already owns the mongoose instance.
|
||||
jest.mock('../../config/connect', () => jest.fn(async () => undefined));
|
||||
|
||||
jest.mock('@librechat/data-schemas', () => {
|
||||
const actual = jest.requireActual('@librechat/data-schemas');
|
||||
return {
|
||||
...actual,
|
||||
logger: { warn: jest.fn(), debug: jest.fn(), error: jest.fn(), info: jest.fn() },
|
||||
};
|
||||
});
|
||||
|
||||
const mongoose = require('mongoose');
|
||||
const { MongoMemoryServer } = require('mongodb-memory-server');
|
||||
const { agentSchema, fileSchema } = require('@librechat/data-schemas');
|
||||
const { FileSources } = require('librechat-data-provider');
|
||||
|
||||
const { migrateOrphanedAgentFiles } = require('../../config/migrate-orphaned-agent-files');
|
||||
|
||||
describe('migrate-orphaned-agent-files (issue #12776)', () => {
|
||||
let mongoServer;
|
||||
let Agent;
|
||||
let File;
|
||||
const userId = () => new mongoose.Types.ObjectId();
|
||||
|
||||
beforeAll(async () => {
|
||||
mongoServer = await MongoMemoryServer.create();
|
||||
await mongoose.connect(mongoServer.getUri());
|
||||
Agent = mongoose.models.Agent || mongoose.model('Agent', agentSchema);
|
||||
File = mongoose.models.File || mongoose.model('File', fileSchema);
|
||||
}, 30000);
|
||||
|
||||
afterAll(async () => {
|
||||
await mongoose.disconnect();
|
||||
await mongoServer.stop();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
await Agent.deleteMany({});
|
||||
await File.deleteMany({});
|
||||
});
|
||||
|
||||
const seedFile = (file_id) =>
|
||||
File.create({
|
||||
file_id,
|
||||
user: userId(),
|
||||
filename: `${file_id}.txt`,
|
||||
filepath: `/tmp/${file_id}`,
|
||||
object: 'file',
|
||||
type: 'text/plain',
|
||||
bytes: 1,
|
||||
source: FileSources.text,
|
||||
});
|
||||
|
||||
const seedAgent = (tool_resources) =>
|
||||
Agent.create({
|
||||
id: `agent_${Math.random().toString(36).slice(2, 10)}`,
|
||||
name: `Test Agent ${Math.random().toString(36).slice(2, 6)}`,
|
||||
provider: 'test',
|
||||
model: 'test-model',
|
||||
author: userId(),
|
||||
tool_resources,
|
||||
});
|
||||
|
||||
test('dry-run reports orphans without mutating any agent', async () => {
|
||||
const keeperId = 'keeper';
|
||||
await seedFile(keeperId);
|
||||
const agent = await seedAgent({
|
||||
file_search: { file_ids: [keeperId, 'orphan_1', 'orphan_2'] },
|
||||
});
|
||||
|
||||
const result = await migrateOrphanedAgentFiles({ dryRun: true });
|
||||
|
||||
expect(result.dryRun).toBe(true);
|
||||
expect(result.scannedAgents).toBe(1);
|
||||
expect(result.agentsWithOrphans).toBe(1);
|
||||
expect(result.totalOrphansRemoved).toBe(2);
|
||||
// Dry-run reports what would change without writing — no updates counted.
|
||||
expect(result.agentsUpdated).toBe(0);
|
||||
|
||||
const after = await Agent.findOne({ id: agent.id }).lean();
|
||||
expect(after.tool_resources.file_search.file_ids).toEqual([keeperId, 'orphan_1', 'orphan_2']);
|
||||
});
|
||||
|
||||
test('apply mode removes orphans across every tool_resource category', async () => {
|
||||
const keeperA = 'k_a';
|
||||
const keeperB = 'k_b';
|
||||
await seedFile(keeperA);
|
||||
await seedFile(keeperB);
|
||||
|
||||
const agent = await seedAgent({
|
||||
file_search: { file_ids: [keeperA, 'o1'] },
|
||||
execute_code: { file_ids: ['o2', keeperB] },
|
||||
context: { file_ids: ['o3'] },
|
||||
});
|
||||
|
||||
const result = await migrateOrphanedAgentFiles({ dryRun: false });
|
||||
|
||||
expect(result.dryRun).toBe(false);
|
||||
expect(result.agentsWithOrphans).toBe(1);
|
||||
expect(result.agentsUpdated).toBe(1);
|
||||
expect(result.totalOrphansRemoved).toBe(3);
|
||||
|
||||
const after = await Agent.findOne({ id: agent.id }).lean();
|
||||
expect(after.tool_resources.file_search.file_ids).toEqual([keeperA]);
|
||||
expect(after.tool_resources.execute_code.file_ids).toEqual([keeperB]);
|
||||
expect(after.tool_resources.context.file_ids).toEqual([]);
|
||||
});
|
||||
|
||||
test('is idempotent — re-running on a clean database is a no-op', async () => {
|
||||
await seedFile('keeper');
|
||||
await seedAgent({ file_search: { file_ids: ['keeper', 'orphan'] } });
|
||||
|
||||
await migrateOrphanedAgentFiles({ dryRun: false });
|
||||
const second = await migrateOrphanedAgentFiles({ dryRun: false });
|
||||
|
||||
expect(second.agentsWithOrphans).toBe(0);
|
||||
expect(second.agentsUpdated).toBe(0);
|
||||
expect(second.totalOrphansRemoved).toBe(0);
|
||||
});
|
||||
|
||||
test('leaves agents without orphans completely alone', async () => {
|
||||
await seedFile('only');
|
||||
const agent = await seedAgent({ file_search: { file_ids: ['only'] } });
|
||||
|
||||
const result = await migrateOrphanedAgentFiles({ dryRun: false });
|
||||
|
||||
expect(result.scannedAgents).toBe(1);
|
||||
expect(result.agentsWithOrphans).toBe(0);
|
||||
const after = await Agent.findOne({ id: agent.id }).lean();
|
||||
expect(after.tool_resources.file_search.file_ids).toEqual(['only']);
|
||||
});
|
||||
|
||||
test('sample array is bounded on wide corruption (DETAIL_SAMPLE_LIMIT)', async () => {
|
||||
// Seed more than the cap (50) so the truncation branch is exercised.
|
||||
const agents = [];
|
||||
for (let i = 0; i < 55; i++) {
|
||||
agents.push(
|
||||
await seedAgent({
|
||||
file_search: { file_ids: [`orphan_${i}`] },
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const result = await migrateOrphanedAgentFiles({ dryRun: true });
|
||||
|
||||
expect(result.agentsWithOrphans).toBe(55);
|
||||
expect(result.details.length).toBeLessThanOrEqual(50);
|
||||
expect(result.details.length).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
test('runs the body inside a system tenant context (strict-mode safe)', async () => {
|
||||
// Pins the runAsSystem wrap: without it the migration throws under
|
||||
// TENANT_ISOLATION_STRICT=true on the very first Agent.countDocuments(),
|
||||
// blocking the intended remediation path for corrupted agents.
|
||||
const { SYSTEM_TENANT_ID, tenantStorage } = require('@librechat/data-schemas');
|
||||
await seedFile('keeper');
|
||||
await seedAgent({ file_search: { file_ids: ['keeper', 'orphan'] } });
|
||||
|
||||
const contextsObserved = [];
|
||||
const originalCountDocuments = Agent.countDocuments.bind(Agent);
|
||||
Agent.countDocuments = jest.fn((...args) => {
|
||||
contextsObserved.push(tenantStorage.getStore()?.tenantId);
|
||||
return originalCountDocuments(...args);
|
||||
});
|
||||
|
||||
try {
|
||||
await migrateOrphanedAgentFiles({ dryRun: false });
|
||||
expect(contextsObserved).toContain(SYSTEM_TENANT_ID);
|
||||
} finally {
|
||||
Agent.countDocuments = originalCountDocuments;
|
||||
}
|
||||
});
|
||||
});
|
||||
160
config/migrate-orphaned-agent-files.js
Normal file
160
config/migrate-orphaned-agent-files.js
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
const path = require('path');
|
||||
const { logger, runAsSystem } = require('@librechat/data-schemas');
|
||||
const { TOOL_RESOURCE_KEYS, collectToolResourceFileIds } = require('@librechat/api');
|
||||
|
||||
require('module-alias')({ base: path.resolve(__dirname, '..', 'api') });
|
||||
const connect = require('./connect');
|
||||
|
||||
const { Agent, File } = require('~/db/models');
|
||||
|
||||
/**
|
||||
* Cap on the number of per-agent entries we retain in `results.details`. Larger
|
||||
* runs still update every affected agent and still report accurate aggregate
|
||||
* counts — we just stop accumulating sample data past this threshold to keep
|
||||
* memory bounded on deployments with thousands of corrupted agents.
|
||||
*/
|
||||
const DETAIL_SAMPLE_LIMIT = 50;
|
||||
|
||||
/**
|
||||
* Cleans up orphaned file_id references from agent `tool_resources` — that is,
|
||||
* file_ids that remain on an agent after the underlying File document has
|
||||
* already been deleted (see issue #12776). These stubs otherwise accumulate and
|
||||
* eventually block new uploads with "Duplicate file detected."
|
||||
*
|
||||
* Safe to re-run — if there are no orphans, nothing is written.
|
||||
*
|
||||
* @param {{ dryRun?: boolean, batchSize?: number }} [options]
|
||||
*/
|
||||
async function migrateOrphanedAgentFiles({ dryRun = true, batchSize = 100 } = {}) {
|
||||
await connect();
|
||||
|
||||
logger.info('Starting Orphaned Agent Files Migration', { dryRun, batchSize });
|
||||
|
||||
/*
|
||||
* Scan and heal across every tenant. Without this wrapper the tenant
|
||||
* isolation plugin either scopes queries to a (non-existent) tenant or
|
||||
* throws under TENANT_ISOLATION_STRICT=true, making the script unusable
|
||||
* as the intended remediation path for corrupted agents.
|
||||
*/
|
||||
return runAsSystem(async () => {
|
||||
const totalAgents = await Agent.countDocuments();
|
||||
logger.info(`Scanning ${totalAgents} agent(s) for orphaned file references`);
|
||||
|
||||
const results = {
|
||||
dryRun,
|
||||
scannedAgents: 0,
|
||||
agentsWithOrphans: 0,
|
||||
agentsUpdated: 0,
|
||||
totalOrphansRemoved: 0,
|
||||
errors: 0,
|
||||
details: [],
|
||||
};
|
||||
|
||||
const cursor = Agent.find({}, { id: 1, name: 1, tool_resources: 1 })
|
||||
.lean()
|
||||
.cursor({ batchSize });
|
||||
|
||||
for await (const agent of cursor) {
|
||||
results.scannedAgents++;
|
||||
|
||||
try {
|
||||
const referencedFileIds = collectToolResourceFileIds(agent.tool_resources);
|
||||
if (referencedFileIds.length === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const existing = await File.find(
|
||||
{ file_id: { $in: referencedFileIds } },
|
||||
{ file_id: 1, _id: 0 },
|
||||
).lean();
|
||||
const existingIds = new Set(existing.map((f) => f.file_id));
|
||||
const orphans = referencedFileIds.filter((id) => !existingIds.has(id));
|
||||
if (orphans.length === 0) {
|
||||
continue;
|
||||
}
|
||||
|
||||
results.agentsWithOrphans++;
|
||||
results.totalOrphansRemoved += orphans.length;
|
||||
if (results.details.length < DETAIL_SAMPLE_LIMIT) {
|
||||
results.details.push({
|
||||
agentId: agent.id,
|
||||
name: agent.name,
|
||||
orphanCount: orphans.length,
|
||||
orphans,
|
||||
});
|
||||
}
|
||||
|
||||
if (dryRun) {
|
||||
logger.debug(`[dry-run] Would prune ${orphans.length} orphan(s) from agent ${agent.id}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const pullAllOps = {};
|
||||
for (const key of TOOL_RESOURCE_KEYS) {
|
||||
pullAllOps[`tool_resources.${key}.file_ids`] = orphans;
|
||||
}
|
||||
const updateResult = await Agent.updateOne({ _id: agent._id }, { $pullAll: pullAllOps });
|
||||
if (updateResult.modifiedCount > 0) {
|
||||
results.agentsUpdated++;
|
||||
logger.info(
|
||||
`Pruned ${orphans.length} orphan(s) from agent "${agent.name}" (${agent.id})`,
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
results.errors++;
|
||||
logger.error(`Failed to process agent ${agent.id}`, { error: error.message });
|
||||
}
|
||||
}
|
||||
|
||||
logger.info('Orphaned Agent Files Migration completed', {
|
||||
dryRun,
|
||||
scannedAgents: results.scannedAgents,
|
||||
agentsWithOrphans: results.agentsWithOrphans,
|
||||
agentsUpdated: results.agentsUpdated,
|
||||
totalOrphansRemoved: results.totalOrphansRemoved,
|
||||
errors: results.errors,
|
||||
});
|
||||
|
||||
return results;
|
||||
});
|
||||
}
|
||||
|
||||
if (require.main === module) {
|
||||
const dryRun = process.argv.includes('--dry-run');
|
||||
const batchSize =
|
||||
parseInt(process.argv.find((arg) => arg.startsWith('--batch-size='))?.split('=')[1]) || 100;
|
||||
|
||||
migrateOrphanedAgentFiles({ dryRun, batchSize })
|
||||
.then((result) => {
|
||||
console.log(`\n=== ${dryRun ? 'DRY RUN ' : ''}RESULTS ===`);
|
||||
console.log(`Agents scanned: ${result.scannedAgents}`);
|
||||
console.log(`Agents with orphans: ${result.agentsWithOrphans}`);
|
||||
console.log(
|
||||
`Orphan references ${dryRun ? 'to remove' : 'removed'}: ${result.totalOrphansRemoved}`,
|
||||
);
|
||||
if (!dryRun) {
|
||||
console.log(`Agents updated: ${result.agentsUpdated}`);
|
||||
}
|
||||
if (result.errors > 0) {
|
||||
console.log(`Errors: ${result.errors}`);
|
||||
}
|
||||
if (result.details.length > 0) {
|
||||
console.log('\nAffected agents:');
|
||||
result.details.forEach((d, i) => {
|
||||
console.log(` ${i + 1}. "${d.name}" (${d.agentId}) — ${d.orphanCount} orphan(s)`);
|
||||
});
|
||||
if (result.agentsWithOrphans > result.details.length) {
|
||||
console.log(
|
||||
` ... and ${result.agentsWithOrphans - result.details.length} more (sample capped at ${DETAIL_SAMPLE_LIMIT})`,
|
||||
);
|
||||
}
|
||||
}
|
||||
process.exit(0);
|
||||
})
|
||||
.catch((error) => {
|
||||
console.error('Orphaned agent files migration failed:', error);
|
||||
process.exit(1);
|
||||
});
|
||||
}
|
||||
|
||||
module.exports = { migrateOrphanedAgentFiles };
|
||||
|
|
@ -93,7 +93,10 @@
|
|||
"migrate:agent-permissions:batch": "node config/migrate-agent-permissions.js --batch-size=50",
|
||||
"migrate:prompt-permissions:dry-run": "node config/migrate-prompt-permissions.js --dry-run",
|
||||
"migrate:prompt-permissions": "node config/migrate-prompt-permissions.js",
|
||||
"migrate:prompt-permissions:batch": "node config/migrate-prompt-permissions.js --batch-size=50"
|
||||
"migrate:prompt-permissions:batch": "node config/migrate-prompt-permissions.js --batch-size=50",
|
||||
"migrate:orphaned-agent-files:dry-run": "node config/migrate-orphaned-agent-files.js --dry-run",
|
||||
"migrate:orphaned-agent-files": "node config/migrate-orphaned-agent-files.js",
|
||||
"migrate:orphaned-agent-files:batch": "node config/migrate-orphaned-agent-files.js --batch-size=50"
|
||||
},
|
||||
"repository": {
|
||||
"type": "git",
|
||||
|
|
|
|||
|
|
@ -9,6 +9,7 @@ export * from './handlers';
|
|||
export * from './initialize';
|
||||
export * from './legacy';
|
||||
export * from './memory';
|
||||
export * from './orphans';
|
||||
export * from './migration';
|
||||
export * from './openai';
|
||||
export * from './transactions';
|
||||
|
|
|
|||
53
packages/api/src/agents/orphans.spec.ts
Normal file
53
packages/api/src/agents/orphans.spec.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import { EToolResources } from 'librechat-data-provider';
|
||||
import type { AgentToolResources } from 'librechat-data-provider';
|
||||
import { collectToolResourceFileIds, stripFileIdsFromToolResources } from './orphans';
|
||||
|
||||
const makeResources = (): AgentToolResources => ({
|
||||
[EToolResources.file_search]: { file_ids: ['a', 'b', 'c'] },
|
||||
[EToolResources.execute_code]: { file_ids: ['b', 'd'] },
|
||||
[EToolResources.context]: { file_ids: ['e'] },
|
||||
});
|
||||
|
||||
describe('collectToolResourceFileIds', () => {
|
||||
it('returns empty array for nullish input', () => {
|
||||
expect(collectToolResourceFileIds(undefined)).toEqual([]);
|
||||
expect(collectToolResourceFileIds(null)).toEqual([]);
|
||||
});
|
||||
|
||||
it('gathers and de-duplicates file_ids across every category', () => {
|
||||
const ids = collectToolResourceFileIds(makeResources());
|
||||
expect(new Set(ids)).toEqual(new Set(['a', 'b', 'c', 'd', 'e']));
|
||||
});
|
||||
|
||||
it('skips categories without a file_ids array', () => {
|
||||
const resources: AgentToolResources = {
|
||||
[EToolResources.file_search]: { file_ids: ['a'] },
|
||||
[EToolResources.context]: {},
|
||||
};
|
||||
expect(collectToolResourceFileIds(resources)).toEqual(['a']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('stripFileIdsFromToolResources', () => {
|
||||
it('removes matching ids from every category and reports the count', () => {
|
||||
const resources = makeResources();
|
||||
const { removedCount } = stripFileIdsFromToolResources(resources, ['b', 'e']);
|
||||
|
||||
expect(removedCount).toBe(3);
|
||||
expect(resources[EToolResources.file_search]?.file_ids).toEqual(['a', 'c']);
|
||||
expect(resources[EToolResources.execute_code]?.file_ids).toEqual(['d']);
|
||||
expect(resources[EToolResources.context]?.file_ids).toEqual([]);
|
||||
});
|
||||
|
||||
it('is a no-op when no ids are provided', () => {
|
||||
const resources = makeResources();
|
||||
const { removedCount } = stripFileIdsFromToolResources(resources, []);
|
||||
expect(removedCount).toBe(0);
|
||||
expect(resources[EToolResources.file_search]?.file_ids).toEqual(['a', 'b', 'c']);
|
||||
});
|
||||
|
||||
it('handles nullish tool_resources safely', () => {
|
||||
const { removedCount } = stripFileIdsFromToolResources(undefined, ['a']);
|
||||
expect(removedCount).toBe(0);
|
||||
});
|
||||
});
|
||||
70
packages/api/src/agents/orphans.ts
Normal file
70
packages/api/src/agents/orphans.ts
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
import { EToolResources } from 'librechat-data-provider';
|
||||
import type { AgentToolResources } from 'librechat-data-provider';
|
||||
|
||||
/**
|
||||
* Every `EToolResources` member that can carry `file_ids` on an agent document.
|
||||
* `code_interpreter` is intentionally omitted — it's part of `EToolResources`
|
||||
* for the Assistants API but not a key of the agent-side `AgentToolResources`
|
||||
* shape, so including it would be a type lie and generate dead MongoDB clauses.
|
||||
*/
|
||||
export const TOOL_RESOURCE_KEYS: ReadonlyArray<keyof AgentToolResources> = [
|
||||
EToolResources.execute_code,
|
||||
EToolResources.file_search,
|
||||
EToolResources.image_edit,
|
||||
EToolResources.context,
|
||||
EToolResources.ocr,
|
||||
];
|
||||
|
||||
/**
|
||||
* Collects every file_id referenced across all tool_resource categories.
|
||||
* Duplicates are de-duplicated across categories.
|
||||
*/
|
||||
export function collectToolResourceFileIds(
|
||||
tool_resources: AgentToolResources | undefined | null,
|
||||
): string[] {
|
||||
if (!tool_resources) {
|
||||
return [];
|
||||
}
|
||||
const seen = new Set<string>();
|
||||
for (const key of TOOL_RESOURCE_KEYS) {
|
||||
const ids = tool_resources[key]?.file_ids;
|
||||
if (!Array.isArray(ids)) {
|
||||
continue;
|
||||
}
|
||||
for (const id of ids) {
|
||||
if (typeof id === 'string') {
|
||||
seen.add(id);
|
||||
}
|
||||
}
|
||||
}
|
||||
return Array.from(seen);
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the given file_ids from every tool_resource category on the provided
|
||||
* tool_resources object. Mutates in place and also returns the same reference
|
||||
* for convenience. Returns the count of removed references.
|
||||
*/
|
||||
export function stripFileIdsFromToolResources(
|
||||
tool_resources: AgentToolResources | undefined | null,
|
||||
idsToRemove: Iterable<string>,
|
||||
): { tool_resources: AgentToolResources | undefined | null; removedCount: number } {
|
||||
if (!tool_resources) {
|
||||
return { tool_resources, removedCount: 0 };
|
||||
}
|
||||
const removeSet = idsToRemove instanceof Set ? idsToRemove : new Set(idsToRemove);
|
||||
if (removeSet.size === 0) {
|
||||
return { tool_resources, removedCount: 0 };
|
||||
}
|
||||
let removedCount = 0;
|
||||
for (const key of TOOL_RESOURCE_KEYS) {
|
||||
const resource = tool_resources[key];
|
||||
if (!resource || !Array.isArray(resource.file_ids)) {
|
||||
continue;
|
||||
}
|
||||
const before = resource.file_ids.length;
|
||||
resource.file_ids = resource.file_ids.filter((id) => !removeSet.has(id));
|
||||
removedCount += before - resource.file_ids.length;
|
||||
}
|
||||
return { tool_resources, removedCount };
|
||||
}
|
||||
|
|
@ -51,6 +51,7 @@ let deleteUserAgents: AgentMethods['deleteUserAgents'];
|
|||
let revertAgentVersion: AgentMethods['revertAgentVersion'];
|
||||
let addAgentResourceFile: AgentMethods['addAgentResourceFile'];
|
||||
let removeAgentResourceFiles: AgentMethods['removeAgentResourceFiles'];
|
||||
let removeAgentResourceFilesFromAllAgents: AgentMethods['removeAgentResourceFilesFromAllAgents'];
|
||||
let getListAgentsByAccess: AgentMethods['getListAgentsByAccess'];
|
||||
let generateActionMetadataHash: AgentMethods['generateActionMetadataHash'];
|
||||
|
||||
|
|
@ -93,6 +94,7 @@ beforeAll(async () => {
|
|||
revertAgentVersion = methods.revertAgentVersion;
|
||||
addAgentResourceFile = methods.addAgentResourceFile;
|
||||
removeAgentResourceFiles = methods.removeAgentResourceFiles;
|
||||
removeAgentResourceFilesFromAllAgents = methods.removeAgentResourceFilesFromAllAgents;
|
||||
getListAgentsByAccess = methods.getListAgentsByAccess;
|
||||
generateActionMetadataHash = methods.generateActionMetadataHash;
|
||||
|
||||
|
|
@ -2645,6 +2647,109 @@ describe('Agent Methods', () => {
|
|||
).rejects.toThrow('Agent not found for removing resource files');
|
||||
});
|
||||
|
||||
describe('removeAgentResourceFilesFromAllAgents', () => {
|
||||
beforeEach(async () => {
|
||||
await Agent.deleteMany({});
|
||||
});
|
||||
|
||||
test('should strip deleted file_ids from every agent that references them', async () => {
|
||||
const sharedFileId = `file_${uuidv4()}`;
|
||||
const keeperFileId = `file_${uuidv4()}`;
|
||||
|
||||
const agentA = await createBasicAgent();
|
||||
const agentB = await createBasicAgent();
|
||||
const untouchedAgent = await createBasicAgent();
|
||||
|
||||
await addAgentResourceFile({
|
||||
agent_id: agentA.id,
|
||||
tool_resource: EToolResources.file_search,
|
||||
file_id: sharedFileId,
|
||||
});
|
||||
await addAgentResourceFile({
|
||||
agent_id: agentA.id,
|
||||
tool_resource: EToolResources.file_search,
|
||||
file_id: keeperFileId,
|
||||
});
|
||||
await addAgentResourceFile({
|
||||
agent_id: agentB.id,
|
||||
tool_resource: EToolResources.execute_code,
|
||||
file_id: sharedFileId,
|
||||
});
|
||||
await addAgentResourceFile({
|
||||
agent_id: untouchedAgent.id,
|
||||
tool_resource: EToolResources.context,
|
||||
file_id: keeperFileId,
|
||||
});
|
||||
|
||||
const result = await removeAgentResourceFilesFromAllAgents({
|
||||
file_ids: [sharedFileId],
|
||||
});
|
||||
|
||||
expect(result.matchedCount).toBe(2);
|
||||
expect(result.modifiedCount).toBe(2);
|
||||
|
||||
const updatedA = await getAgent({ id: agentA.id });
|
||||
const updatedB = await getAgent({ id: agentB.id });
|
||||
const updatedUntouched = await getAgent({ id: untouchedAgent.id });
|
||||
|
||||
const aFileIds = (updatedA!.tool_resources as Record<string, { file_ids: string[] }>)
|
||||
.file_search.file_ids;
|
||||
const bFileIds = (updatedB!.tool_resources as Record<string, { file_ids: string[] }>)
|
||||
.execute_code.file_ids;
|
||||
const untouchedFileIds = (
|
||||
updatedUntouched!.tool_resources as Record<string, { file_ids: string[] }>
|
||||
).context.file_ids;
|
||||
|
||||
expect(aFileIds).not.toContain(sharedFileId);
|
||||
expect(aFileIds).toContain(keeperFileId);
|
||||
expect(bFileIds).not.toContain(sharedFileId);
|
||||
expect(untouchedFileIds).toEqual([keeperFileId]);
|
||||
});
|
||||
|
||||
test('should remove file_ids across multiple tool_resource types on the same agent', async () => {
|
||||
const fileId = `file_${uuidv4()}`;
|
||||
const agent = await createBasicAgent();
|
||||
|
||||
await addAgentResourceFile({
|
||||
agent_id: agent.id,
|
||||
tool_resource: EToolResources.file_search,
|
||||
file_id: fileId,
|
||||
});
|
||||
await addAgentResourceFile({
|
||||
agent_id: agent.id,
|
||||
tool_resource: EToolResources.ocr,
|
||||
file_id: fileId,
|
||||
});
|
||||
|
||||
await removeAgentResourceFilesFromAllAgents({ file_ids: [fileId] });
|
||||
|
||||
const updated = await getAgent({ id: agent.id });
|
||||
const resources = updated!.tool_resources as Record<string, { file_ids: string[] }>;
|
||||
expect(resources.file_search.file_ids).not.toContain(fileId);
|
||||
expect(resources.ocr.file_ids).not.toContain(fileId);
|
||||
});
|
||||
|
||||
test('should no-op and not throw when file_ids is empty', async () => {
|
||||
const result = await removeAgentResourceFilesFromAllAgents({ file_ids: [] });
|
||||
expect(result).toEqual({ matchedCount: 0, modifiedCount: 0 });
|
||||
});
|
||||
|
||||
test('should no-op when no agent references the given file_ids', async () => {
|
||||
const fileId = `file_${uuidv4()}`;
|
||||
const agent = await createBasicAgent();
|
||||
|
||||
await addAgentResourceFile({
|
||||
agent_id: agent.id,
|
||||
tool_resource: EToolResources.file_search,
|
||||
file_id: `different_${uuidv4()}`,
|
||||
});
|
||||
|
||||
const result = await removeAgentResourceFilesFromAllAgents({ file_ids: [fileId] });
|
||||
expect(result.matchedCount).toBe(0);
|
||||
expect(result.modifiedCount).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
test('should handle updateAgent with complex nested updates', async () => {
|
||||
const agentId = `agent_${uuidv4()}`;
|
||||
const authorId = new mongoose.Types.ObjectId();
|
||||
|
|
|
|||
|
|
@ -1,11 +1,26 @@
|
|||
import crypto from 'node:crypto';
|
||||
import { Constants, ResourceType, actionDelimiter } from 'librechat-data-provider';
|
||||
import { Constants, EToolResources, ResourceType, actionDelimiter } from 'librechat-data-provider';
|
||||
import type { AgentToolResources } from 'librechat-data-provider';
|
||||
import type { FilterQuery, Model, Types } from 'mongoose';
|
||||
import type { IAgent, IAclEntry } from '~/types';
|
||||
import logger from '~/config/winston';
|
||||
|
||||
const { mcp_delimiter } = Constants;
|
||||
|
||||
/**
|
||||
* Mirrors `TOOL_RESOURCE_KEYS` in `@librechat/api` — the subset of
|
||||
* `EToolResources` that actually carries `file_ids` on an agent document.
|
||||
* `code_interpreter` is excluded (it belongs to the Assistants API, not
|
||||
* `AgentToolResources`) to avoid emitting dead MongoDB clauses.
|
||||
*/
|
||||
const TOOL_RESOURCE_KEYS: ReadonlyArray<keyof AgentToolResources> = [
|
||||
EToolResources.execute_code,
|
||||
EToolResources.file_search,
|
||||
EToolResources.image_edit,
|
||||
EToolResources.context,
|
||||
EToolResources.ocr,
|
||||
];
|
||||
|
||||
export interface AgentDeps {
|
||||
/** Removes all ACL permissions for a resource. Injected from PermissionService. */
|
||||
removeAllPermissions: (params: { resourceType: string; resourceId: unknown }) => Promise<void>;
|
||||
|
|
@ -477,6 +492,37 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag
|
|||
return agentAfterPull;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes the given file_ids from every agent's `tool_resources.*.file_ids`
|
||||
* so file deletion cannot leave orphaned stubs behind (see issue #12776).
|
||||
*/
|
||||
async function removeAgentResourceFilesFromAllAgents({
|
||||
file_ids,
|
||||
}: {
|
||||
file_ids: string[];
|
||||
}): Promise<{ matchedCount: number; modifiedCount: number }> {
|
||||
if (!file_ids || file_ids.length === 0) {
|
||||
return { matchedCount: 0, modifiedCount: 0 };
|
||||
}
|
||||
|
||||
const Agent = mongoose.models.Agent as Model<IAgent>;
|
||||
|
||||
const orQuery = TOOL_RESOURCE_KEYS.map((key) => ({
|
||||
[`tool_resources.${key}.file_ids`]: { $in: file_ids },
|
||||
}));
|
||||
|
||||
const pullAllOps = TOOL_RESOURCE_KEYS.reduce<Record<string, string[]>>((acc, key) => {
|
||||
acc[`tool_resources.${key}.file_ids`] = file_ids;
|
||||
return acc;
|
||||
}, {});
|
||||
|
||||
const result = await Agent.updateMany({ $or: orQuery }, { $pullAll: pullAllOps });
|
||||
return {
|
||||
matchedCount: result.matchedCount ?? 0,
|
||||
modifiedCount: result.modifiedCount ?? 0,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes an agent based on the provided search parameter.
|
||||
*/
|
||||
|
|
@ -774,6 +820,7 @@ export function createAgentMethods(mongoose: typeof import('mongoose'), deps: Ag
|
|||
removeAgentResourceFiles,
|
||||
generateActionMetadataHash,
|
||||
removeAgentFromUserFavorites,
|
||||
removeAgentResourceFilesFromAllAgents,
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue