mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-07 06:59:26 +00:00
🐛 fix: Hydrate deferred attachments for later tool calls
An attachment accepted on one turn whose tool never ran carried neither an embedding nor a code reference, and every hydration query matches only files that already have one: getToolFilesByIds requires embedded, getUserCodeFiles requires a codeEnvRef. The file was therefore absent from later turns entirely, so asking to search or run code against it found nothing. Hydrating it back into attachments would have re-delivered earlier uploads to the model on every turn, so provisioning and delivery are now separate inputs: deferred records are fetched by their own query and reach the provisioning computation alone, never the returned attachments. Also provisions for the host create_file tool, whose name is distinct from write_file, and falls back to the primary context when a tool batch omits its agent id, matching what the tool loaders already do.
This commit is contained in:
parent
b8c7e0b109
commit
7132fdcf32
9 changed files with 259 additions and 9 deletions
|
|
@ -775,7 +775,11 @@ const executeOpenAIChatCompletion = async (envelope, { req, res }) => {
|
|||
agent never gains sandbox access even if the admin enabled the
|
||||
capability globally. */
|
||||
const toolExecuteOptions = {
|
||||
provisionFiles: createProvisionFilesCallback({ req, agentToolContexts }),
|
||||
provisionFiles: createProvisionFilesCallback({
|
||||
req,
|
||||
agentToolContexts,
|
||||
resolvePrimaryAgentId: () => primaryConfig.id,
|
||||
}),
|
||||
loadTools: async (toolNames, agentId, _configurable, callerCapabilityProjection) => {
|
||||
const ctx =
|
||||
agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {};
|
||||
|
|
|
|||
|
|
@ -1101,7 +1101,11 @@ const executeResponse = async (envelope, { req, res }) => {
|
|||
|
||||
// Create tool execute options for event-driven tool execution
|
||||
const toolExecuteOptions = {
|
||||
provisionFiles: createProvisionFilesCallback({ req, agentToolContexts }),
|
||||
provisionFiles: createProvisionFilesCallback({
|
||||
req,
|
||||
agentToolContexts,
|
||||
resolvePrimaryAgentId: () => primaryConfig.id,
|
||||
}),
|
||||
loadTools: async (toolNames, agentId, _configurable, callerCapabilityProjection) => {
|
||||
const ctx =
|
||||
agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {};
|
||||
|
|
@ -1315,7 +1319,11 @@ const executeResponse = async (envelope, { req, res }) => {
|
|||
});
|
||||
|
||||
const toolExecuteOptions = {
|
||||
provisionFiles: createProvisionFilesCallback({ req, agentToolContexts }),
|
||||
provisionFiles: createProvisionFilesCallback({
|
||||
req,
|
||||
agentToolContexts,
|
||||
resolvePrimaryAgentId: () => primaryConfig.id,
|
||||
}),
|
||||
loadTools: async (toolNames, agentId, _configurable, callerCapabilityProjection) => {
|
||||
const ctx =
|
||||
agentToolContexts.get(agentId) ?? agentToolContexts.get(primaryConfig.id) ?? {};
|
||||
|
|
|
|||
|
|
@ -453,7 +453,11 @@ const initializeClient = async ({
|
|||
}
|
||||
},
|
||||
...getSkillToolDeps(),
|
||||
provisionFiles: createProvisionFilesCallback({ req, agentToolContexts }),
|
||||
provisionFiles: createProvisionFilesCallback({
|
||||
req,
|
||||
agentToolContexts,
|
||||
resolvePrimaryAgentId: () => primaryConfig?.id,
|
||||
}),
|
||||
};
|
||||
|
||||
const summarizationOptions =
|
||||
|
|
@ -609,6 +613,7 @@ const initializeClient = async ({
|
|||
updateFilesUsage: db.updateFilesUsage,
|
||||
getUserKeyValues: db.getUserKeyValues,
|
||||
getUserCodeFiles: db.getUserCodeFiles,
|
||||
getDeferredProvisionFiles: db.getDeferredProvisionFiles,
|
||||
getToolFilesByIds: db.getToolFilesByIds,
|
||||
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
|
||||
filterFilesByAgentAccess,
|
||||
|
|
@ -697,6 +702,7 @@ const initializeClient = async ({
|
|||
updateFilesUsage: db.updateFilesUsage,
|
||||
getUserKeyValues: db.getUserKeyValues,
|
||||
getUserCodeFiles: db.getUserCodeFiles,
|
||||
getDeferredProvisionFiles: db.getDeferredProvisionFiles,
|
||||
getToolFilesByIds: db.getToolFilesByIds,
|
||||
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
|
||||
filterFilesByAgentAccess,
|
||||
|
|
@ -1130,6 +1136,7 @@ const initializeClient = async ({
|
|||
updateFilesUsage: db.updateFilesUsage,
|
||||
getUserKeyValues: db.getUserKeyValues,
|
||||
getUserCodeFiles: db.getUserCodeFiles,
|
||||
getDeferredProvisionFiles: db.getDeferredProvisionFiles,
|
||||
getToolFilesByIds: db.getToolFilesByIds,
|
||||
getCodeGeneratedFiles: db.getCodeGeneratedFiles,
|
||||
filterFilesByAgentAccess,
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
const { logger } = require('@librechat/data-schemas');
|
||||
const { Constants } = require('@librechat/agents');
|
||||
const { EToolResources } = require('librechat-data-provider');
|
||||
const { isAgentScopedFile } = require('@librechat/api');
|
||||
const { isAgentScopedFile, CREATE_FILE_TOOL_NAME } = require('@librechat/api');
|
||||
const { provisionToCodeEnv, provisionToVectorDB } = require('~/server/services/Files/provision');
|
||||
const db = require('~/models');
|
||||
|
||||
|
|
@ -13,11 +13,20 @@ const db = require('~/models');
|
|||
* @param {object} params
|
||||
* @param {object} params.req - Authenticated request, used for storage and Code API auth
|
||||
* @param {Map<string, object>} params.agentToolContexts - Per-agent contexts holding provisionState
|
||||
* @param {() => string | undefined} [params.resolvePrimaryAgentId] - Primary agent id,
|
||||
* read lazily because callers may build this callback before that config resolves
|
||||
* @returns {(toolNames: string[], agentId?: string) => Promise<void>}
|
||||
*/
|
||||
function createProvisionFilesCallback({ req, agentToolContexts }) {
|
||||
function createProvisionFilesCallback({ req, agentToolContexts, resolvePrimaryAgentId }) {
|
||||
return async function provisionFiles(toolNames, agentId) {
|
||||
const ctx = agentToolContexts.get(agentId);
|
||||
/* agentId is optional on this callback and a batch for the primary agent may omit
|
||||
* it, so fall back the way the tool loaders do. Otherwise the queue is missed and
|
||||
* the tool runs without its attachments. */
|
||||
const primaryAgentId = resolvePrimaryAgentId?.();
|
||||
const ctx =
|
||||
(agentId != null ? agentToolContexts.get(agentId) : undefined) ??
|
||||
(primaryAgentId != null ? agentToolContexts.get(primaryAgentId) : undefined) ??
|
||||
(agentToolContexts.size === 1 ? agentToolContexts.values().next().value : undefined);
|
||||
if (!ctx?.provisionState) {
|
||||
return;
|
||||
}
|
||||
|
|
@ -34,6 +43,7 @@ function createProvisionFilesCallback({ req, agentToolContexts }) {
|
|||
toolNames.includes(Constants.READ_FILE) ||
|
||||
toolNames.includes(Constants.EDIT_FILE) ||
|
||||
toolNames.includes(Constants.WRITE_FILE) ||
|
||||
toolNames.includes(CREATE_FILE_TOOL_NAME) ||
|
||||
toolNames.includes(Constants.BASH_PROGRAMMATIC_TOOL_CALLING);
|
||||
/** Programmatic tool calling orchestrates nested tools whose names never reach
|
||||
* this predicate, so a file_search reachable only through PTC would otherwise
|
||||
|
|
|
|||
|
|
@ -675,6 +675,7 @@ export interface InitializeAgentDbMethods extends EndpointDbMethods {
|
|||
) => Promise<unknown[]>;
|
||||
/** Get user-uploaded execute_code files by file IDs (from message.files in thread) */
|
||||
getUserCodeFiles?: (fileIds: string[], ownerScope: FileOwnerScope) => Promise<unknown[]>;
|
||||
getDeferredProvisionFiles?: (fileIds: string[], ownerScope: FileOwnerScope) => Promise<unknown[]>;
|
||||
/** Get messages for a conversation (supports select for field projection) */
|
||||
getMessages?: (
|
||||
filter: { conversationId: string },
|
||||
|
|
@ -1015,6 +1016,8 @@ export async function initializeAgent(
|
|||
),
|
||||
];
|
||||
const toolFileIds: string[] = [];
|
||||
/** Earlier-turn attachments still awaiting provisioning; provisioning input only. */
|
||||
let deferredProvisionFiles: IMongoFile[] = [];
|
||||
|
||||
/** Build the set of tool resources the agent has enabled */
|
||||
const toolResourceSet = new Set<EToolResources>();
|
||||
|
|
@ -1102,6 +1105,22 @@ export async function initializeAgent(
|
|||
: ([] as IMongoFile[]),
|
||||
]);
|
||||
|
||||
/* Attachments accepted on an earlier turn whose tool never ran are absent from
|
||||
* every query above, since those match only files that already carry the result
|
||||
* of provisioning. Fetched separately and kept out of the delivery set. */
|
||||
const wantsProvisioning = wantsCodeFiles || toolResourceSet.has(EToolResources.file_search);
|
||||
deferredProvisionFiles =
|
||||
wantsProvisioning &&
|
||||
db.getDeferredProvisionFiles &&
|
||||
requestFileOwnerScope &&
|
||||
threadFileIds &&
|
||||
threadFileIds.length > 0
|
||||
? ((await db.getDeferredProvisionFiles(
|
||||
threadFileIds,
|
||||
requestFileOwnerScope,
|
||||
)) as IMongoFile[])
|
||||
: [];
|
||||
|
||||
const allToolFiles = toolFiles.concat(codeGeneratedFiles, userCodeFiles);
|
||||
const snapshotFileIds = new Set(requestFileIds);
|
||||
for (const file of allToolFiles) {
|
||||
|
|
@ -1212,6 +1231,7 @@ export async function initializeAgent(
|
|||
enabledToolResources: toolResourceSet,
|
||||
checkSessionsAlive: db.checkSessionsAlive,
|
||||
loadCodeApiKey: db.loadCodeApiKey,
|
||||
provisionCandidates: deferredProvisionFiles as unknown as TFile[],
|
||||
});
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -2131,6 +2131,50 @@ describe('primeResources', () => {
|
|||
expect(searchResource?.files?.map((f) => f.file_id) ?? []).not.toContain('embedded-context');
|
||||
});
|
||||
|
||||
it('queues a deferred candidate for provisioning without delivering it again', async () => {
|
||||
process.env.CODEAPI_AUTH_PROVIDER = 'librechat-jwt';
|
||||
const deferred = makeCodeFile({ file_id: 'deferred-file' });
|
||||
|
||||
const result = await primeResources({
|
||||
req: mockReq,
|
||||
appConfig: mockAppConfig,
|
||||
getFiles: mockGetFiles,
|
||||
filterFiles: mockFilterFiles,
|
||||
tool_resources: {},
|
||||
attachments: Promise.resolve([]),
|
||||
requestFileSet,
|
||||
agentId: 'agent1',
|
||||
enabledToolResources: new Set([EToolResources.execute_code, EToolResources.file_search]),
|
||||
provisionCandidates: [deferred],
|
||||
});
|
||||
|
||||
expect(result.provisionState?.codeEnvFiles.map((f) => f.file_id)).toContain('deferred-file');
|
||||
expect(result.provisionState?.vectorDBFiles.map((f) => f.file_id)).toContain('deferred-file');
|
||||
/* The point of the separation: it must not become an attachment again. */
|
||||
expect(result.attachments?.map((f) => f?.file_id) ?? []).not.toContain('deferred-file');
|
||||
});
|
||||
|
||||
it('does not double-queue a candidate that is already an attachment', async () => {
|
||||
process.env.CODEAPI_AUTH_PROVIDER = 'librechat-jwt';
|
||||
const file = makeCodeFile({ file_id: 'shared-file' });
|
||||
|
||||
const result = await primeResources({
|
||||
req: mockReq,
|
||||
appConfig: mockAppConfig,
|
||||
getFiles: mockGetFiles,
|
||||
filterFiles: mockFilterFiles,
|
||||
tool_resources: {},
|
||||
attachments: Promise.resolve([file]),
|
||||
requestFileSet,
|
||||
agentId: 'agent1',
|
||||
enabledToolResources: new Set([EToolResources.execute_code]),
|
||||
provisionCandidates: [{ ...file }],
|
||||
});
|
||||
|
||||
const queued = result.provisionState?.codeEnvFiles.filter((f) => f.file_id === 'shared-file');
|
||||
expect(queued).toHaveLength(1);
|
||||
});
|
||||
|
||||
it('never queues a text-source record, which has no streamable backing', async () => {
|
||||
process.env.CODEAPI_AUTH_PROVIDER = 'librechat-jwt';
|
||||
const textFile = makeCodeFile({ file_id: 'text-record', source: FileSources.text });
|
||||
|
|
|
|||
|
|
@ -291,6 +291,26 @@ const categorizeFileForToolResources = ({
|
|||
const codeEnvRouteKey = (ref: CodeEnvRef): string =>
|
||||
ref.executionRouteKey ?? ref.executionProfile ?? 'default';
|
||||
|
||||
/** Attachments plus any deferred candidates not already present, deduped by file_id.
|
||||
* Only the provisioning computation sees this: the delivery list stays untouched. */
|
||||
const withDeferredCandidates = (
|
||||
attachments: Array<TFile>,
|
||||
candidates?: Array<TFile>,
|
||||
): Array<TFile> => {
|
||||
if (!candidates || candidates.length === 0) {
|
||||
return attachments;
|
||||
}
|
||||
const seen = new Set(attachments.map((file) => file.file_id));
|
||||
const merged = [...attachments];
|
||||
for (const candidate of candidates) {
|
||||
if (candidate?.file_id && !seen.has(candidate.file_id)) {
|
||||
seen.add(candidate.file_id);
|
||||
merged.push(candidate);
|
||||
}
|
||||
}
|
||||
return merged;
|
||||
};
|
||||
|
||||
/**
|
||||
* Lazy provisioning: instead of provisioning files now, compute which files need
|
||||
* provisioning. Actual provisioning happens at tool invocation time via the
|
||||
|
|
@ -443,6 +463,7 @@ export const primeResources = async ({
|
|||
enabledToolResources,
|
||||
checkSessionsAlive,
|
||||
loadCodeApiKey,
|
||||
provisionCandidates,
|
||||
}: {
|
||||
req?: ServerRequest;
|
||||
principal?: Pick<IUser, 'id' | 'role'>;
|
||||
|
|
@ -459,6 +480,10 @@ export const primeResources = async ({
|
|||
checkSessionsAlive?: TCheckSessionsAlive;
|
||||
/** Optional callback to load CODE_API_KEY once per request */
|
||||
loadCodeApiKey?: TLoadCodeApiKey;
|
||||
/** Attachments from earlier turns that were never provisioned. Considered for
|
||||
* provisioning only and never returned as attachments: re-delivering an earlier
|
||||
* upload to the model on every later turn is not the intent. */
|
||||
provisionCandidates?: Array<TFile>;
|
||||
}): Promise<{
|
||||
attachments: Array<TFile | undefined> | undefined;
|
||||
requestAttachments: Array<TFile | undefined> | undefined;
|
||||
|
|
@ -606,7 +631,7 @@ export const primeResources = async ({
|
|||
* provisioning here too, so a turn with no new attachment still primes them. */
|
||||
const contextProvisionState = await computeProvisionState({
|
||||
req,
|
||||
attachments,
|
||||
attachments: withDeferredCandidates(attachments, provisionCandidates),
|
||||
resourcePrincipal,
|
||||
enabledToolResources,
|
||||
tool_resources,
|
||||
|
|
@ -660,7 +685,7 @@ export const primeResources = async ({
|
|||
|
||||
const provisionState = await computeProvisionState({
|
||||
req,
|
||||
attachments,
|
||||
attachments: withDeferredCandidates(attachments, provisionCandidates),
|
||||
resourcePrincipal,
|
||||
enabledToolResources,
|
||||
tool_resources,
|
||||
|
|
|
|||
|
|
@ -705,6 +705,82 @@ describe('File Methods', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getDeferredProvisionFiles', () => {
|
||||
const makeFile = (
|
||||
fileId: string,
|
||||
ownerId: mongoose.Types.ObjectId,
|
||||
overrides: Record<string, unknown> = {},
|
||||
) =>
|
||||
runAsSystem(() =>
|
||||
fileMethods.createFile({
|
||||
file_id: fileId,
|
||||
user: ownerId,
|
||||
tenantId: 'tenant-a',
|
||||
conversationId: 'conversation-a',
|
||||
filename: `${fileId}.csv`,
|
||||
filepath: `/uploads/${fileId}.csv`,
|
||||
source: 'local',
|
||||
type: 'text/csv',
|
||||
bytes: 100,
|
||||
context: FileContext.message_attachment,
|
||||
...overrides,
|
||||
}),
|
||||
);
|
||||
|
||||
it('returns attachments that carry neither an embedding nor a code reference', async () => {
|
||||
const ownerId = new mongoose.Types.ObjectId();
|
||||
const deferredId = uuidv4();
|
||||
await makeFile(deferredId, ownerId);
|
||||
|
||||
const results = await fileMethods.getDeferredProvisionFiles([deferredId], {
|
||||
userId: ownerId.toString(),
|
||||
tenantId: 'tenant-a',
|
||||
});
|
||||
|
||||
expect(results.map((file) => file.file_id)).toEqual([deferredId]);
|
||||
});
|
||||
|
||||
it('omits files already embedded or already provisioned to the sandbox', async () => {
|
||||
const ownerId = new mongoose.Types.ObjectId();
|
||||
const embeddedId = uuidv4();
|
||||
const provisionedId = uuidv4();
|
||||
const generatedId = uuidv4();
|
||||
await makeFile(embeddedId, ownerId, { embedded: true });
|
||||
await makeFile(provisionedId, ownerId, {
|
||||
metadata: {
|
||||
codeEnvRef: {
|
||||
kind: 'user',
|
||||
id: ownerId.toString(),
|
||||
storage_session_id: 'session',
|
||||
file_id: provisionedId,
|
||||
},
|
||||
},
|
||||
});
|
||||
await makeFile(generatedId, ownerId, { context: FileContext.execute_code });
|
||||
|
||||
const results = await fileMethods.getDeferredProvisionFiles(
|
||||
[embeddedId, provisionedId, generatedId],
|
||||
{ userId: ownerId.toString(), tenantId: 'tenant-a' },
|
||||
);
|
||||
|
||||
expect(results).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not cross the authenticated owner scope', async () => {
|
||||
const ownerId = new mongoose.Types.ObjectId();
|
||||
const victimId = new mongoose.Types.ObjectId();
|
||||
const victimFileId = uuidv4();
|
||||
await makeFile(victimFileId, victimId);
|
||||
|
||||
const results = await fileMethods.getDeferredProvisionFiles([victimFileId], {
|
||||
userId: ownerId.toString(),
|
||||
tenantId: 'tenant-a',
|
||||
});
|
||||
|
||||
expect(results).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getUserCodeFiles', () => {
|
||||
it('returns only authenticated owner code-env uploads', async () => {
|
||||
const ownerId = new mongoose.Types.ObjectId();
|
||||
|
|
|
|||
|
|
@ -47,6 +47,10 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
|
|||
ownerScope?: FileOwnerScope,
|
||||
) => Promise<IMongoFile[]>;
|
||||
getUserCodeFiles: (fileIds: string[], ownerScope: FileOwnerScope) => Promise<IMongoFile[]>;
|
||||
getDeferredProvisionFiles: (
|
||||
fileIds: string[],
|
||||
ownerScope: FileOwnerScope,
|
||||
) => Promise<IMongoFile[]>;
|
||||
claimCodeFile: (data: {
|
||||
filename: string;
|
||||
conversationId: string;
|
||||
|
|
@ -267,6 +271,57 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieves conversation attachments that were accepted but never provisioned, so a
|
||||
* later turn can still queue them.
|
||||
*
|
||||
* Lazy provisioning defers the upload to the sandbox or vector store until a tool
|
||||
* actually runs. The other hydration queries only return files that already carry
|
||||
* the result of that work: `getToolFilesByIds` matches `embedded: true` for
|
||||
* file_search, and `getUserCodeFiles` requires an existing `codeEnvRef`. A file
|
||||
* whose tool was never called on its upload turn therefore satisfies neither and
|
||||
* disappears. This fills exactly that gap: attachments with no code reference and
|
||||
* no embedding.
|
||||
*
|
||||
* Kept separate from delivery hydration deliberately. These records are candidates
|
||||
* for provisioning only; feeding them back into the model's attachments would
|
||||
* re-send earlier uploads on every subsequent turn.
|
||||
*
|
||||
* @param fileIds - Candidate file IDs from the current thread
|
||||
* @param ownerScope - Authenticated owner scope
|
||||
* @returns Attachments still awaiting provisioning
|
||||
*/
|
||||
async function getDeferredProvisionFiles(
|
||||
fileIds: string[],
|
||||
ownerScope: FileOwnerScope,
|
||||
): Promise<IMongoFile[]> {
|
||||
if (!fileIds || fileIds.length === 0) {
|
||||
return [];
|
||||
}
|
||||
|
||||
try {
|
||||
const filter = withOwnerScope(
|
||||
{
|
||||
file_id: { $in: fileIds },
|
||||
context: { $ne: FileContext.execute_code },
|
||||
embedded: { $ne: true },
|
||||
'metadata.codeEnvRef': { $exists: false },
|
||||
'metadata.codeEnvRefs': { $exists: false },
|
||||
},
|
||||
ownerScope,
|
||||
);
|
||||
|
||||
const selectFields: SelectProjection = { text: 0 };
|
||||
const sortOptions = { createdAt: 1 as SortOrder };
|
||||
|
||||
const results = await getFiles(filter, sortOptions, selectFields);
|
||||
return results ?? [];
|
||||
} catch (error) {
|
||||
logger.error('[getDeferredProvisionFiles] Error retrieving deferred files:', error);
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieves user-uploaded execute_code files (not code-generated) by their file IDs.
|
||||
* These are files with fileIdentifier metadata but context is NOT execute_code (e.g., agents or message_attachment).
|
||||
|
|
@ -682,6 +737,7 @@ export function createFileMethods(mongoose: typeof import('mongoose')): {
|
|||
getToolFilesByIds,
|
||||
getCodeGeneratedFiles,
|
||||
getUserCodeFiles,
|
||||
getDeferredProvisionFiles,
|
||||
claimCodeFile,
|
||||
createFile,
|
||||
updateFile,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue