mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-25 11:47:32 +00:00
🗂️ fix: Resolve Code Interpreter Destination Collisions (#15449)
* 🗂️ fix: Resolve Code Interpreter Destination Collisions
Codeapi mounts each `/exec` input at a destination derived from its `name`
and rejects the whole request when two entries land on one. LibreChat never
enforced that: `primeFiles` sent `name: file.filename` verbatim, and the
graph seed deduped by `(storage_session_id, id)` — identity, not destination.
Two conversation files sharing a filename therefore killed code execution for
the rest of the conversation. The rejected request never reaches the sandbox,
so no result comes back to collapse the pair, and every later turn re-primes
both and fails identically. Only code-generated outputs are covered by the
`(filename, conversationId, context, tenantId)` partial unique index; uploads
carry `context: message_attachment`, so one conversation can hold several
records sharing a filename — two uploads of `image.png`, or an upload whose
name a later execution wrote back in place.
`packages/api/src/files/code/destinations.ts` now owns the name → destination
mapping for every contributor to an `/exec` file list, mirroring codeapi's
conflict rule including directory-prefix conflicts:
- `primeFiles` claims per pushed file, disambiguating with a `-2`, `-3`
counter and echoing the resolved path into the tool context, so the model
is told where each file actually mounts. Claim order comes from immutable
`createdAt`, not `getFiles`'s `updatedAt` sort, which usage accounting and
re-upload both bump — order-dependent destinations would silently repoint
a path an earlier turn had already handed the model. Newest wins the bare
name, matching `ToolNode.updateCodeSession` and giving an in-place rewrite
precedence over the original it superseded.
- `seedCodeFilesIntoSessions` drops a later file whose destination is already
claimed. Renaming there would invent a path nothing told the model about,
and a conflict at that layer means one logical file arrived twice under
different storage pointers.
Fixes #15443
* 🧹 chore: Tolerate Holes in the Destination Priority Sort
`primeFiles` sorts the raw `getFiles` result before its own per-entry guard
runs, so the comparator has to survive a null row rather than throw ahead of
the loop that was already defending against one.
Also tightens the tool-context assertion in the upload-vs-output case, which
was passing on a trailing space rather than on the path.
* 🔁 fix: Address Codex Review on Destination Assignment
Four findings from the review of `b0f61d0`.
**Identity-derived suffixes instead of a collision counter.** A counter is
assigned from whatever is in the set, so a third file arriving under the
literal name a displaced file had been given pushed that file along, and code
the model wrote in an earlier turn would silently start reading the newcomer.
The fallback now hashes the file's own `file_id`, so a displaced file keeps
its destination for the life of the conversation and only the winner of the
bare name can change. A trailing counter still guards a hash collision so the
search terminates on any input.
**Rank by last content write, not record creation.** `processCodeOutput`
claims one row per `(filename, conversationId)` and rewrites it in place, so
a repeatedly written output keeps its original `createdAt` while recording the
latest writer in `metadata.sourceDispatchedAt`. Ranking on `createdAt` alone
handed the bare path to an upload the output had since been rewritten over.
Recency is now `max(sourceDispatchedAt, createdAt)` — still never `updatedAt`,
which usage accounting bumps without any content changing.
**Advertise a displaced generated output.** Generated files are left out of
the tool context because the model already knows what it wrote — which stops
being true once a newer same-named file takes the bare path and the output
mounts somewhere the model has never seen. Displaced outputs are now listed
with the name they were written as, and displaced uploads name their original
too.
**Rank conversation files above an agent's private ones.** Each agent claims
destinations over its own candidate set — the conversation's files plus its
own — so a private file taking a bare name from a shared file in one agent but
not in another left two agents advertising different paths for the same file
into one shared mount namespace. Shared files now rank first, so the majority
that every agent sees resolves identically. The private-versus-private case
still drops at the merge and is documented there; closing it needs one
assignment across contributors, made before any agent renders its context.
* 📌 test: Pin Alias Ownership on an In-Place Rewrite
Codex round 2 flagged that the identity alias still falls back to a counter
when the alias itself is literally claimed, so a displaced file can move after
all. The mechanism is real and my JSDoc overstated the guarantee, which this
corrects.
The behaviour stays as it is. The realistic way a file arrives under another
file's alias is the model rewriting the displaced file in place: it was told
`<stem>-<hash>.ext`, wrote back to it, and `processCodeOutput` registered an
output under that literal name. Keeping the alias with the newest holder is
what makes the model's familiar path resolve to its own edit; reserving it
against the newcomer would hand that path back to the bytes it replaced. The
test pins that ordering so a later change cannot invert it silently.
This commit is contained in:
Danny Avila
GitHub
parent
09722f4b16
commit
224512851a
8 changed files with 989 additions and 20 deletions
|
|
@ -22,7 +22,10 @@ const {
|
|||
getStorageMetadata,
|
||||
getCodeExecutionBaseUrl,
|
||||
buildCodeEnvDownloadQuery,
|
||||
claimCodeDestination,
|
||||
createCodeDestinationSet,
|
||||
CODE_API_EXPECTED_PROFILE_HEADER,
|
||||
sortCodeFilesByDestinationPriority,
|
||||
} = require('@librechat/api');
|
||||
const {
|
||||
Tools,
|
||||
|
|
@ -1030,13 +1033,26 @@ const getPreviewContextSuffix = (file) => {
|
|||
: ' (preview unavailable)';
|
||||
};
|
||||
|
||||
const getVisibleCodeFileContextLine = (file, agentResourceIds) => {
|
||||
if (file.context === FileContext.execute_code) {
|
||||
/**
|
||||
* A generated output is normally left out — the model already knows what it
|
||||
* wrote. That only holds while the file is still where it wrote it: once a
|
||||
* newer same-named file takes the bare path, the output mounts under a
|
||||
* suffixed name the model has never seen, and silence would leave it reading
|
||||
* the newcomer or failing to find its own artifact.
|
||||
*/
|
||||
const getVisibleCodeFileContextLine = (file, agentResourceIds, destination) => {
|
||||
const displaced = destination !== file.filename;
|
||||
if (file.context === FileContext.execute_code && !displaced) {
|
||||
return '';
|
||||
}
|
||||
|
||||
const fileSuffix = agentResourceIds.has(file.file_id) ? '' : ' (attached by user)';
|
||||
return `\n\t- /mnt/data/${file.filename}${fileSuffix}${getPreviewContextSuffix(file)}`;
|
||||
const origin =
|
||||
file.context === FileContext.execute_code
|
||||
? ` (written earlier as ${file.filename})`
|
||||
: `${agentResourceIds.has(file.file_id) ? '' : ' (attached by user)'}${
|
||||
displaced ? ` (uploaded as ${file.filename})` : ''
|
||||
}`;
|
||||
return `\n\t- /mnt/data/${destination}${origin}${getPreviewContextSuffix(file)}`;
|
||||
};
|
||||
|
||||
const appendVisibleCodeFileContext = (toolContext, contextLine) => {
|
||||
|
|
@ -1163,6 +1179,17 @@ const primeFiles = async (options) => {
|
|||
const sessions = new Map();
|
||||
let toolContext = '';
|
||||
|
||||
/* Claim order decides which record keeps the bare `/mnt/data/<name>` path
|
||||
* when several share a filename, so it is fixed here rather than inherited
|
||||
* from `getFiles`'s `updatedAt` sort — usage accounting and re-upload both
|
||||
* bump `updatedAt`, which would repoint paths between turns. `file_ids` are
|
||||
* this agent's own resources; every other candidate came from the
|
||||
* conversation and is therefore seen by every agent in the run, so shared
|
||||
* files rank first and land on the same destination whichever agent primes
|
||||
* them. */
|
||||
const orderedFiles = sortCodeFilesByDestinationPriority(dbFiles, agentResourceIds);
|
||||
const destinations = createCodeDestinationSet();
|
||||
|
||||
/* Per-file path counters — emitted at the bottom so a single
|
||||
* grep on `[primeCodeFiles]` shows the input volume, the per-file
|
||||
* paths taken, and the final dispatch summary in one trace. */
|
||||
|
|
@ -1171,8 +1198,8 @@ const primeFiles = async (options) => {
|
|||
let requiredCodeFiles = 0;
|
||||
const reuploadFailureCategories = new Set();
|
||||
|
||||
for (let i = 0; i < dbFiles.length; i++) {
|
||||
const file = dbFiles[i];
|
||||
for (let i = 0; i < orderedFiles.length; i++) {
|
||||
const file = orderedFiles[i];
|
||||
if (!file) {
|
||||
continue;
|
||||
}
|
||||
|
|
@ -1205,9 +1232,19 @@ const primeFiles = async (options) => {
|
|||
* tenant prefix from auth context).
|
||||
*/
|
||||
const pushFile = (overrideSessionId, overrideId) => {
|
||||
/* Claimed here rather than up front so files that never reach the
|
||||
* sandbox — no code-env ref, or a failed re-upload — do not reserve a
|
||||
* name and push a file that does reach it onto a counter. */
|
||||
const destination = claimCodeDestination(destinations, file.filename, file.file_id);
|
||||
if (destination !== file.filename) {
|
||||
logger.debug(
|
||||
`[primeCodeFiles] file=${file.file_id} destination=${destination} ` +
|
||||
`reason=name-collision filename=${file.filename}`,
|
||||
);
|
||||
}
|
||||
toolContext = appendVisibleCodeFileContext(
|
||||
toolContext,
|
||||
getVisibleCodeFileContextLine(file, agentResourceIds),
|
||||
getVisibleCodeFileContextLine(file, agentResourceIds, destination),
|
||||
);
|
||||
/* `id` is the storage file_id (drives codeapi's upload-key
|
||||
* existence check), `resource_id` is the entity that owns
|
||||
|
|
@ -1219,7 +1256,7 @@ const primeFiles = async (options) => {
|
|||
id: overrideId ?? id,
|
||||
resource_id: sourceRef.id,
|
||||
storage_session_id: overrideSessionId ?? session_id,
|
||||
name: file.filename,
|
||||
name: destination,
|
||||
kind: sourceRef.kind,
|
||||
...(sourceRef.kind === 'skill' ? { version: sourceRef.version } : {}),
|
||||
});
|
||||
|
|
|
|||
|
|
@ -113,6 +113,42 @@ jest.mock('@librechat/api', () => {
|
|||
}),
|
||||
codeServerHttpAgent: new http.Agent({ keepAlive: false }),
|
||||
codeServerHttpsAgent: new https.Agent({ keepAlive: false }),
|
||||
/* Sandbox destination assignment, mirrored the same way the identity
|
||||
* helpers above are. The real policy — directory-prefix conflicts, the
|
||||
* byte cap, flattening, the hashed suffix — lives in
|
||||
* `packages/api/src/files/code/destinations.ts` under its own
|
||||
* `destinations.spec.ts`; these stubs carry just enough of its shape
|
||||
* (an identity-derived suffix, shared-then-newest ordering) for the
|
||||
* `primeFiles` tests to assert that it is wired to `name` and to the tool
|
||||
* context at all. The suffix here is the raw identity rather than a
|
||||
* digest so the expectations below read as names. */
|
||||
createCodeDestinationSet: () => new Set(),
|
||||
claimCodeDestination: (set, name, identity) => {
|
||||
const dot = name.lastIndexOf('.');
|
||||
const stem = dot > 0 ? name.slice(0, dot) : name;
|
||||
const extension = dot > 0 ? name.slice(dot) : '';
|
||||
let destination = name;
|
||||
for (let counter = 1; set.has(destination); counter++) {
|
||||
const tail = counter === 1 ? '' : `-${counter}`;
|
||||
destination = `${stem}-${identity}${tail}${extension}`;
|
||||
}
|
||||
set.add(destination);
|
||||
return destination;
|
||||
},
|
||||
sortCodeFilesByDestinationPriority: (files, privateFileIds) => {
|
||||
const isPrivate = (file) => (privateFileIds?.has(file?.file_id) ? 1 : 0);
|
||||
const contentTime = (file) =>
|
||||
Math.max(
|
||||
file?.metadata?.sourceDispatchedAt ?? 0,
|
||||
new Date(file?.createdAt ?? 0).getTime() || 0,
|
||||
);
|
||||
return [...files].sort((a, b) => {
|
||||
const scope = isPrivate(a) - isPrivate(b);
|
||||
if (scope !== 0) return scope;
|
||||
const delta = contentTime(b) - contentTime(a);
|
||||
return delta !== 0 ? delta : (a?.file_id ?? '').localeCompare(b?.file_id ?? '');
|
||||
});
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
|
|
@ -2847,4 +2883,276 @@ describe('Code Process', () => {
|
|||
expect(mockAxios).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('primeFiles resolves sandbox destination collisions (#15443)', () => {
|
||||
/**
|
||||
* Codeapi mounts each input at a destination derived from its `name` and
|
||||
* rejects the whole `/exec` request when two entries land on one — and a
|
||||
* rejected request never reaches the sandbox, so nothing comes back to
|
||||
* collapse the pair. Every later turn re-primes both files and fails the
|
||||
* same way, which is why one duplicate filename kills code execution for
|
||||
* the rest of the conversation.
|
||||
*
|
||||
* Only code-generated outputs are covered by the `(filename,
|
||||
* conversationId, context, tenantId)` partial unique index; uploads carry
|
||||
* `context: message_attachment`, so a conversation can hold several
|
||||
* records sharing a filename.
|
||||
*/
|
||||
const { getFiles } = require('~/models');
|
||||
const { getStrategyFunctions } = require('~/server/services/Files/strategies');
|
||||
|
||||
const codeFile = ({
|
||||
file_id,
|
||||
filename,
|
||||
storage_session_id,
|
||||
createdAt,
|
||||
context,
|
||||
sourceDispatchedAt,
|
||||
}) => ({
|
||||
file_id,
|
||||
filename,
|
||||
createdAt,
|
||||
context: context ?? 'message_attachment',
|
||||
source: 'local',
|
||||
filepath: `/uploads/${file_id}`,
|
||||
metadata: {
|
||||
...(sourceDispatchedAt != null ? { sourceDispatchedAt } : {}),
|
||||
codeEnvRef: {
|
||||
kind: 'user',
|
||||
id: 'user-123',
|
||||
storage_session_id,
|
||||
file_id: `${file_id}-sandbox`,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
/** Freshness probe answers "uploaded just now", so every file takes the
|
||||
* cache-hit path and none of them re-upload. */
|
||||
function setupActiveSessions() {
|
||||
getStrategyFunctions.mockImplementation(() => ({
|
||||
getDownloadStream: jest.fn(),
|
||||
handleFileUpload: jest.fn(),
|
||||
}));
|
||||
mockAxios.mockResolvedValue({ data: { lastModified: new Date().toISOString() } });
|
||||
}
|
||||
|
||||
const prime = (tool_resources) =>
|
||||
primeFiles({
|
||||
req: { user: { id: 'user-123', role: 'USER' } },
|
||||
tool_resources: tool_resources ?? { execute_code: { file_ids: ['older', 'newer'] } },
|
||||
agentId: 'agent-id',
|
||||
});
|
||||
|
||||
const bySession = (result) =>
|
||||
Object.fromEntries(result.files.map((f) => [f.storage_session_id, f.name]));
|
||||
|
||||
it('gives two uploads sharing a filename distinct destinations', async () => {
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'image.png',
|
||||
storage_session_id: 'sess-older',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
}),
|
||||
codeFile({
|
||||
file_id: 'newer',
|
||||
filename: 'image.png',
|
||||
storage_session_id: 'sess-newer',
|
||||
createdAt: new Date('2026-01-02T00:00:00Z'),
|
||||
}),
|
||||
]);
|
||||
|
||||
const { files, toolContext } = await prime();
|
||||
|
||||
expect(files).toHaveLength(2);
|
||||
expect(new Set(files.map((f) => f.name)).size).toBe(2);
|
||||
/* The newest record keeps the bare path: when an execution rewrote an
|
||||
* uploaded file in place, the model's next read of that path has to
|
||||
* find its own edit rather than the superseded original. */
|
||||
expect(bySession({ files })).toEqual({
|
||||
'sess-newer': 'image.png',
|
||||
'sess-older': 'image-older.png',
|
||||
});
|
||||
expect(toolContext).toContain('/mnt/data/image.png');
|
||||
/* The displaced file is advertised at the path it actually mounts on,
|
||||
* alongside the name the user knows it by. */
|
||||
expect(toolContext).toContain('/mnt/data/image-older.png');
|
||||
expect(toolContext).toContain('(uploaded as image.png)');
|
||||
});
|
||||
|
||||
it('assigns the same destinations regardless of the order getFiles returns', async () => {
|
||||
/**
|
||||
* `getFiles` sorts by `updatedAt` desc by default, and usage accounting
|
||||
* and re-upload both bump `updatedAt` — so claim order cannot come from
|
||||
* the query. If it did, a path a previous turn told the model about
|
||||
* would silently point at the other file.
|
||||
*/
|
||||
const older = codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'image.png',
|
||||
storage_session_id: 'sess-older',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
});
|
||||
const newer = codeFile({
|
||||
file_id: 'newer',
|
||||
filename: 'image.png',
|
||||
storage_session_id: 'sess-newer',
|
||||
createdAt: new Date('2026-01-02T00:00:00Z'),
|
||||
});
|
||||
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([older, newer]);
|
||||
const ascending = await prime();
|
||||
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([newer, older]);
|
||||
const descending = await prime();
|
||||
|
||||
expect(bySession(ascending)).toEqual(bySession(descending));
|
||||
expect(bySession(ascending)).toEqual({
|
||||
'sess-newer': 'image.png',
|
||||
'sess-older': 'image-older.png',
|
||||
});
|
||||
});
|
||||
|
||||
it('separates a code output from the upload whose name it reused', async () => {
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-upload',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
}),
|
||||
codeFile({
|
||||
file_id: 'newer',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-output',
|
||||
createdAt: new Date('2026-01-02T00:00:00Z'),
|
||||
context: 'execute_code',
|
||||
}),
|
||||
]);
|
||||
|
||||
const { files, toolContext } = await prime();
|
||||
|
||||
expect(bySession({ files })).toEqual({
|
||||
'sess-output': 'data.csv',
|
||||
'sess-upload': 'data-older.csv',
|
||||
});
|
||||
/* The output keeps the path it wrote, so it stays out of the context
|
||||
* exactly as an undisplaced generated file does. */
|
||||
expect(toolContext).toContain('/mnt/data/data-older.csv');
|
||||
expect(toolContext).not.toContain('/mnt/data/data.csv');
|
||||
});
|
||||
|
||||
it('advertises a generated output that a newer upload displaced', async () => {
|
||||
/**
|
||||
* The model only knows it wrote `/mnt/data/report.png`. Once a newer
|
||||
* upload takes that path, silence would leave it reading the upload or
|
||||
* failing to find its own artifact.
|
||||
*/
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'report.png',
|
||||
storage_session_id: 'sess-output',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
context: 'execute_code',
|
||||
}),
|
||||
codeFile({
|
||||
file_id: 'newer',
|
||||
filename: 'report.png',
|
||||
storage_session_id: 'sess-upload',
|
||||
createdAt: new Date('2026-01-02T00:00:00Z'),
|
||||
}),
|
||||
]);
|
||||
|
||||
const { toolContext } = await prime();
|
||||
|
||||
expect(toolContext).toContain('/mnt/data/report-older.png (written earlier as report.png)');
|
||||
});
|
||||
|
||||
it('ranks a rewritten output above an upload created after it', async () => {
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-output',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
context: 'execute_code',
|
||||
sourceDispatchedAt: Date.parse('2026-03-01T00:00:00Z'),
|
||||
}),
|
||||
codeFile({
|
||||
file_id: 'newer',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-upload',
|
||||
createdAt: new Date('2026-02-01T00:00:00Z'),
|
||||
}),
|
||||
]);
|
||||
|
||||
const { files } = await prime();
|
||||
|
||||
expect(bySession({ files })).toEqual({
|
||||
'sess-output': 'data.csv',
|
||||
'sess-upload': 'data-newer.csv',
|
||||
});
|
||||
});
|
||||
|
||||
it("lets a conversation file outrank the agent's own file of the same name", async () => {
|
||||
/**
|
||||
* Every agent in a run primes the conversation's files plus its own, so
|
||||
* a private file taking the bare path in one agent and not in another
|
||||
* would leave two agents advertising different paths for the same
|
||||
* shared file into one mount namespace.
|
||||
*/
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'shared',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-shared',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
}),
|
||||
codeFile({
|
||||
file_id: 'agent-own',
|
||||
filename: 'data.csv',
|
||||
storage_session_id: 'sess-agent',
|
||||
createdAt: new Date('2026-06-01T00:00:00Z'),
|
||||
}),
|
||||
]);
|
||||
|
||||
const { files } = await prime({
|
||||
execute_code: {
|
||||
file_ids: ['agent-own'],
|
||||
files: [{ file_id: 'shared' }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(bySession({ files })).toEqual({
|
||||
'sess-shared': 'data.csv',
|
||||
'sess-agent': 'data-agent-own.csv',
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves a single file on its own filename', async () => {
|
||||
setupActiveSessions();
|
||||
getFiles.mockResolvedValue([
|
||||
codeFile({
|
||||
file_id: 'older',
|
||||
filename: 'report.pdf',
|
||||
storage_session_id: 'sess-only',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
}),
|
||||
]);
|
||||
|
||||
const { files, toolContext } = await prime();
|
||||
|
||||
expect(files.map((f) => f.name)).toEqual(['report.pdf']);
|
||||
expect(toolContext).toContain('/mnt/data/report.pdf');
|
||||
expect(toolContext).not.toContain('uploaded as');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -132,21 +132,63 @@ describe('seedCodeFilesIntoSessions', () => {
|
|||
expect(entry.files!.map((f) => f.id).sort()).toEqual(['new-1', 'shared-1', 'skill-1']);
|
||||
});
|
||||
|
||||
it('treats same name + same session as a duplicate; same name + different sessions as distinct', () => {
|
||||
it('keeps distinct identities that mount at distinct destinations', () => {
|
||||
/**
|
||||
* The dedupe key is `(session_id, id)` — not `name` alone. Two
|
||||
* primed uploads can legitimately share a filename when they live
|
||||
* in different sandbox sessions (e.g. each agent re-uploaded the
|
||||
* same source file). Both should land in the seed.
|
||||
* The dedupe key is `(session_id, id)` — not `name` alone. Two primed
|
||||
* uploads are separate files even when one was re-uploaded into a new
|
||||
* sandbox session, and both belong in the seed as long as the caller
|
||||
* resolved them to different mount paths.
|
||||
*/
|
||||
const a = file('id-A', 'sess-A', 'data.csv');
|
||||
const b = file('id-B', 'sess-B', 'data.csv');
|
||||
const b = file('id-B', 'sess-B', 'data-2.csv');
|
||||
const result = seedCodeFilesIntoSessions([a, a, b], undefined);
|
||||
const entry = result!.get(Constants.EXECUTE_CODE) as CodeSessionContext;
|
||||
expect(entry.files).toHaveLength(2);
|
||||
expect(entry.files!.map((f) => f.storage_session_id).sort()).toEqual(['sess-A', 'sess-B']);
|
||||
});
|
||||
|
||||
it('drops a distinct identity that would mount at an already-claimed destination', () => {
|
||||
/**
|
||||
* Regression for #15443. The identity key cannot see this: one file
|
||||
* re-uploaded by one agent and cache-hit by another arrives twice with
|
||||
* different `storage_session_id`s under a single `name`. Codeapi rejects
|
||||
* the whole `/exec` request on the duplicate destination, and because the
|
||||
* call never reaches the sandbox nothing comes back to collapse the pair
|
||||
* — every later turn re-primes it and fails the same way.
|
||||
*/
|
||||
const primed = file('id-A', 'sess-A', 'data.csv');
|
||||
const reuploaded = file('id-B', 'sess-B', 'data.csv');
|
||||
const result = seedCodeFilesIntoSessions([primed, reuploaded], undefined);
|
||||
const entry = result!.get(Constants.EXECUTE_CODE) as CodeSessionContext;
|
||||
expect(entry.files).toHaveLength(1);
|
||||
expect(entry.files![0].storage_session_id).toBe('sess-A');
|
||||
});
|
||||
|
||||
it('lets the prior partition keep a destination an incoming file also wants', () => {
|
||||
const existing: ToolSessionMap = new Map();
|
||||
existing.set(Constants.EXECUTE_CODE, {
|
||||
session_id: 'skill-sess',
|
||||
files: [file('skill-1', 'skill-sess', 'skills/report/run.py')],
|
||||
lastUpdated: 1,
|
||||
} satisfies CodeSessionContext);
|
||||
|
||||
const result = seedCodeFilesIntoSessions(
|
||||
[file('user-1', 'user-sess', 'skills/report/run.py'), file('user-2', 'user-sess', 'ok.csv')],
|
||||
existing,
|
||||
);
|
||||
const entry = result!.get(Constants.EXECUTE_CODE) as CodeSessionContext;
|
||||
expect(entry.files!.map((f) => f.id)).toEqual(['skill-1', 'user-2']);
|
||||
});
|
||||
|
||||
it('rejects an incoming file whose destination is a directory of a claimed one', () => {
|
||||
const result = seedCodeFilesIntoSessions(
|
||||
[file('id-A', 'sess-A', 'data/rows.csv'), file('id-B', 'sess-B', 'data')],
|
||||
undefined,
|
||||
);
|
||||
const entry = result!.get(Constants.EXECUTE_CODE) as CodeSessionContext;
|
||||
expect(entry.files!.map((f) => f.id)).toEqual(['id-A']);
|
||||
});
|
||||
|
||||
it('seeds only the requested code-session partition', () => {
|
||||
const statefulKey = 'execute_code:stateful:v1:user';
|
||||
const existing: ToolSessionMap = new Map();
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import { Constants } from '@librechat/agents';
|
||||
import { logger } from '@librechat/data-schemas';
|
||||
import type { FileRefs, CodeEnvFile, ToolSessionMap, CodeSessionContext } from '@librechat/agents';
|
||||
import type { StatefulCodeEnvironment } from 'librechat-data-provider';
|
||||
import {
|
||||
|
|
@ -6,6 +7,7 @@ import {
|
|||
resolveCodeExecutionContext,
|
||||
type CodeExecutionContext,
|
||||
} from './execution';
|
||||
import { createCodeDestinationSet, reserveCodeDestination } from '~/files/code/destinations';
|
||||
|
||||
/**
|
||||
* Minimal shape for an agent that may contribute primed code files to its
|
||||
|
|
@ -125,6 +127,27 @@ export function collectCodeExecutionProfileRoutes(
|
|||
* dedupe `_injected_files` would grow proportionally to agent count and
|
||||
* inflate every `/exec` POST. First-seen wins so the original ordering /
|
||||
* source is preserved.
|
||||
*
|
||||
* Identity dedupe alone cannot keep the seed valid: codeapi rejects the
|
||||
* whole `/exec` request when two entries mount at one destination, and a
|
||||
* file re-uploaded by one agent but cache-hit by another arrives twice
|
||||
* under different `storage_session_id`s with a single `name`. Sources are
|
||||
* merged in trust order — skill seed, then primary agent, then the rest —
|
||||
* so first-seen also wins the destination, and the later copy of an
|
||||
* already-mounted name is dropped rather than renamed to a path nothing
|
||||
* told the model about.
|
||||
*
|
||||
* Each agent resolves destinations over its own candidate set, so this can
|
||||
* still drop a genuinely distinct file: two agents whose *private*
|
||||
* resources share a filename each claim the bare name locally, and only the
|
||||
* first survives here while the second agent's tool context keeps
|
||||
* advertising its own file at that path. `sortCodeFilesByDestinationPriority`
|
||||
* ranks conversation-scoped files above private ones precisely so the shared
|
||||
* majority cannot diverge that way; closing the private-versus-private case
|
||||
* needs one assignment across contributors, which means resolving
|
||||
* destinations before any agent renders its tool context. Until then this
|
||||
* drop is the failure floor — before it, the pair reached codeapi together
|
||||
* and took the whole run down with a rejected request.
|
||||
*/
|
||||
export function seedCodeFilesIntoSessions(
|
||||
files: CodeEnvFile[] | undefined,
|
||||
|
|
@ -139,16 +162,26 @@ export function seedCodeFilesIntoSessions(
|
|||
const prior = sessions.get(sessionKey) as CodeSessionContext | undefined;
|
||||
|
||||
/**
|
||||
* Compose `(storage_session_id, id)` as a stable identity. `name` alone
|
||||
* isn't sufficient — two distinct primed uploads can share a filename
|
||||
* (different storage sessions, different file_ids). The composite stays
|
||||
* cheap to compute and the keys are short uuids.
|
||||
* Identity is `(storage_session_id, id)`, not `name` — two distinct primed
|
||||
* uploads can share a filename across different storage sessions and
|
||||
* file_ids, and collapsing those would drop a file the caller resolved to
|
||||
* its own mount path. The composite stays cheap to compute and the keys
|
||||
* are short uuids. Destination uniqueness is enforced separately below,
|
||||
* against the sandbox's one-file-per-path constraint.
|
||||
*/
|
||||
const seenKeys = new Set<string>();
|
||||
const destinations = createCodeDestinationSet();
|
||||
const mergedFiles: FileRefs = [];
|
||||
const pushIfFresh = (f: { id?: string; storage_session_id?: string; name?: string }): void => {
|
||||
const key = `${f.storage_session_id ?? ''}\0${f.id ?? ''}`;
|
||||
if (seenKeys.has(key)) return;
|
||||
if (f.name != null && !reserveCodeDestination(destinations, f.name)) {
|
||||
logger.debug(
|
||||
`[seedCodeFilesIntoSessions] dropped id=${f.id} name=${f.name} ` +
|
||||
`reason=destination-taken sessionKey=${sessionKey}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
seenKeys.add(key);
|
||||
mergedFiles.push(f as FileRefs[number]);
|
||||
};
|
||||
|
|
|
|||
307
packages/api/src/files/code/destinations.spec.ts
Normal file
307
packages/api/src/files/code/destinations.spec.ts
Normal file
|
|
@ -0,0 +1,307 @@
|
|||
import {
|
||||
claimCodeDestination,
|
||||
reserveCodeDestination,
|
||||
createCodeDestinationSet,
|
||||
sortCodeFilesByDestinationPriority,
|
||||
} from './destinations';
|
||||
import { deterministicHexSuffix } from '~/utils/files';
|
||||
|
||||
/** The suffix the implementation derives for a displaced file, spelled the
|
||||
* same way so the expectations read as names rather than as digests. */
|
||||
const suffixFor = (identity: string): string => `-${deterministicHexSuffix(identity)}`;
|
||||
|
||||
describe('claimCodeDestination', () => {
|
||||
it('returns the name untouched when nothing has claimed it', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
expect(claimCodeDestination(set, 'image.png', 'file-a')).toBe('image.png');
|
||||
});
|
||||
|
||||
it('moves a displaced file onto its identity suffix, keeping the extension', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
expect(claimCodeDestination(set, 'image.png', 'file-a')).toBe('image.png');
|
||||
expect(claimCodeDestination(set, 'image.png', 'file-b')).toBe(
|
||||
`image${suffixFor('file-b')}.png`,
|
||||
);
|
||||
expect(claimCodeDestination(set, 'image.png', 'file-c')).toBe(
|
||||
`image${suffixFor('file-c')}.png`,
|
||||
);
|
||||
});
|
||||
|
||||
/**
|
||||
* The reason the suffix is not a collision counter. With `-2`, `-3`, a
|
||||
* third file arriving under the literal name a displaced file had been
|
||||
* given would push that file along, and code the model wrote in an earlier
|
||||
* turn would silently start reading the newcomer.
|
||||
*/
|
||||
it('gives a displaced file the same destination whatever else is present', () => {
|
||||
const alone = createCodeDestinationSet();
|
||||
claimCodeDestination(alone, 'image.png', 'winner');
|
||||
const withoutNewcomer = claimCodeDestination(alone, 'image.png', 'displaced');
|
||||
|
||||
const crowded = createCodeDestinationSet();
|
||||
claimCodeDestination(crowded, 'image.png', 'winner');
|
||||
claimCodeDestination(crowded, 'image-2.png', 'newcomer');
|
||||
claimCodeDestination(crowded, `image${suffixFor('unrelated')}.png`, 'unrelated');
|
||||
const withNewcomer = claimCodeDestination(crowded, 'image.png', 'displaced');
|
||||
|
||||
expect(withNewcomer).toBe(withoutNewcomer);
|
||||
});
|
||||
|
||||
/**
|
||||
* The one case where a displaced file does move, and why that is right.
|
||||
* The model is told the displaced file is at `<stem>-<hash>.png`, rewrites
|
||||
* it in place, and `processCodeOutput` registers the output under that
|
||||
* literal name. Next turn the output is the newest holder of that path, so
|
||||
* it keeps it and the superseded original steps aside — the model's
|
||||
* familiar path resolves to its own edit rather than to the bytes it
|
||||
* replaced.
|
||||
*/
|
||||
it('leaves an alias with the later file that rewrote it, not the superseded original', () => {
|
||||
const firstTurn = createCodeDestinationSet();
|
||||
claimCodeDestination(firstTurn, 'image.png', 'newer-upload');
|
||||
const alias = claimCodeDestination(firstTurn, 'image.png', 'original');
|
||||
expect(alias).toBe(`image${suffixFor('original')}.png`);
|
||||
|
||||
const secondTurn = createCodeDestinationSet();
|
||||
const rewrite = claimCodeDestination(secondTurn, alias, 'rewrite-output');
|
||||
claimCodeDestination(secondTurn, 'image.png', 'newer-upload');
|
||||
const originalNow = claimCodeDestination(secondTurn, 'image.png', 'original');
|
||||
|
||||
expect(rewrite).toBe(alias);
|
||||
expect(originalNow).toBe(`image${suffixFor('original')}-2.png`);
|
||||
});
|
||||
|
||||
it('preserves directory structure when disambiguating', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'out/plots/fig.png', 'file-a');
|
||||
expect(claimCodeDestination(set, 'out/plots/fig.png', 'file-b')).toBe(
|
||||
`out/plots/fig${suffixFor('file-b')}.png`,
|
||||
);
|
||||
});
|
||||
|
||||
it('appends to names without an extension', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'README', 'file-a');
|
||||
expect(claimCodeDestination(set, 'README', 'file-b')).toBe(`README${suffixFor('file-b')}`);
|
||||
});
|
||||
|
||||
it('treats a leading dot as part of the name, not an extension', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, '_.env', 'file-a');
|
||||
expect(claimCodeDestination(set, '_.env', 'file-b')).toBe(`_${suffixFor('file-b')}.env`);
|
||||
});
|
||||
|
||||
it('falls back to a counter when two claims share an identity', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'image.png', 'same');
|
||||
expect(claimCodeDestination(set, 'image.png', 'same')).toBe(`image${suffixFor('same')}.png`);
|
||||
expect(claimCodeDestination(set, 'image.png', 'same')).toBe(`image${suffixFor('same')}-2.png`);
|
||||
});
|
||||
|
||||
it('rejects a destination that a claimed directory prefix would swallow', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'data/rows.csv', 'file-a');
|
||||
expect(claimCodeDestination(set, 'data', 'file-b')).toBe(`data${suffixFor('file-b')}`);
|
||||
});
|
||||
|
||||
it('flattens a destination nested under a claimed file', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'data', 'file-a');
|
||||
expect(claimCodeDestination(set, 'data/rows.csv', 'file-b')).toBe('data__rows.csv');
|
||||
});
|
||||
|
||||
it('still disambiguates once a nested destination has been flattened', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
claimCodeDestination(set, 'data', 'file-a');
|
||||
claimCodeDestination(set, 'data__rows.csv', 'file-b');
|
||||
expect(claimCodeDestination(set, 'data/rows.csv', 'file-c')).toBe(
|
||||
`data__rows${suffixFor('file-c')}.csv`,
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps a disambiguated leaf inside the per-segment byte budget', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
const longName = `${'a'.repeat(251)}.png`;
|
||||
expect(Buffer.byteLength(longName, 'utf8')).toBe(255);
|
||||
claimCodeDestination(set, longName, 'file-a');
|
||||
const second = claimCodeDestination(set, longName, 'file-b');
|
||||
expect(second).not.toBe(longName);
|
||||
expect(Buffer.byteLength(second, 'utf8')).toBeLessThanOrEqual(255);
|
||||
expect(second.endsWith(`${suffixFor('file-b')}.png`)).toBe(true);
|
||||
});
|
||||
|
||||
/**
|
||||
* The search only terminates while trimming a name to the byte cap leaves
|
||||
* the suffix intact. An earlier cut-from-the-end trim handed back the
|
||||
* original name for every attempt and hung the request thread, so this
|
||||
* asserts distinctness at the cap rather than trusting the composition.
|
||||
*/
|
||||
it('keeps destinations distinct at the byte cap across repeated collisions', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
const longName = `${'a'.repeat(251)}.png`;
|
||||
const claimed = new Set<string>();
|
||||
for (let i = 0; i < 12; i++) {
|
||||
const destination = claimCodeDestination(set, longName, `file-${i}`);
|
||||
expect(Buffer.byteLength(destination, 'utf8')).toBeLessThanOrEqual(255);
|
||||
expect(claimed.has(destination)).toBe(false);
|
||||
claimed.add(destination);
|
||||
}
|
||||
expect(claimed.size).toBe(12);
|
||||
});
|
||||
|
||||
/**
|
||||
* A name that arrives over budget is passed through untouched — capping
|
||||
* every name would rewrite paths the caller never had a collision on. Only
|
||||
* the disambiguated forms are held to the cap, and they must stay distinct
|
||||
* even when the extension leaves no stem to trim.
|
||||
*/
|
||||
it('keeps destinations distinct when the extension consumes the whole budget', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
const longExtension = `_.${'b'.repeat(260)}`;
|
||||
expect(claimCodeDestination(set, longExtension, 'file-0')).toBe(longExtension);
|
||||
|
||||
const claimed = new Set<string>();
|
||||
for (let i = 1; i < 6; i++) {
|
||||
const destination = claimCodeDestination(set, longExtension, `file-${i}`);
|
||||
expect(Buffer.byteLength(destination, 'utf8')).toBeLessThanOrEqual(255);
|
||||
expect(claimed.has(destination)).toBe(false);
|
||||
claimed.add(destination);
|
||||
}
|
||||
expect(claimed.size).toBe(5);
|
||||
});
|
||||
});
|
||||
|
||||
describe('reserveCodeDestination', () => {
|
||||
it('accepts a free name and refuses the repeat', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
expect(reserveCodeDestination(set, 'image.png')).toBe(true);
|
||||
expect(reserveCodeDestination(set, 'image.png')).toBe(false);
|
||||
});
|
||||
|
||||
it('refuses either direction of a directory-prefix conflict', () => {
|
||||
const nested = createCodeDestinationSet();
|
||||
reserveCodeDestination(nested, 'data/rows.csv');
|
||||
expect(reserveCodeDestination(nested, 'data')).toBe(false);
|
||||
|
||||
const flat = createCodeDestinationSet();
|
||||
reserveCodeDestination(flat, 'data');
|
||||
expect(reserveCodeDestination(flat, 'data/rows.csv')).toBe(false);
|
||||
});
|
||||
|
||||
it('leaves unrelated names alone', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
reserveCodeDestination(set, 'data/rows.csv');
|
||||
expect(reserveCodeDestination(set, 'database.csv')).toBe(true);
|
||||
expect(reserveCodeDestination(set, 'data/other.csv')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not reserve a name it refused', () => {
|
||||
const set = createCodeDestinationSet();
|
||||
reserveCodeDestination(set, 'data');
|
||||
expect(reserveCodeDestination(set, 'data/rows.csv')).toBe(false);
|
||||
expect(claimCodeDestination(set, 'data/rows.csv', 'file-b')).toBe('data__rows.csv');
|
||||
});
|
||||
});
|
||||
|
||||
describe('sortCodeFilesByDestinationPriority', () => {
|
||||
it('puts the newest record first so it keeps the bare name', () => {
|
||||
const older = { file_id: 'a', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
const newer = { file_id: 'b', createdAt: new Date('2026-02-01T00:00:00Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([older, newer])).toEqual([newer, older]);
|
||||
});
|
||||
|
||||
it('accepts serialized dates and epoch millis', () => {
|
||||
const older = { file_id: 'a', createdAt: '2026-01-01T00:00:00.000Z' };
|
||||
const newer = { file_id: 'b', createdAt: Date.parse('2026-02-01T00:00:00.000Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([older, newer])).toEqual([newer, older]);
|
||||
});
|
||||
|
||||
/**
|
||||
* `processCodeOutput` claims one row per `(filename, conversationId)` and
|
||||
* rewrites it in place with `$setOnInsert`, so a repeatedly written output
|
||||
* keeps its original `createdAt`. Ranking on that alone would hand the bare
|
||||
* path to an upload the output has since been rewritten over.
|
||||
*/
|
||||
it('ranks a rewritten output by its last content write, not its creation', () => {
|
||||
const output = {
|
||||
file_id: 'output',
|
||||
createdAt: new Date('2026-01-01T00:00:00Z'),
|
||||
metadata: { sourceDispatchedAt: Date.parse('2026-03-01T00:00:00Z') },
|
||||
};
|
||||
const upload = { file_id: 'upload', createdAt: new Date('2026-02-01T00:00:00Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([upload, output])).toEqual([output, upload]);
|
||||
});
|
||||
|
||||
it('never lets a stale write stamp drag a record below its creation', () => {
|
||||
const output = {
|
||||
file_id: 'output',
|
||||
createdAt: new Date('2026-03-01T00:00:00Z'),
|
||||
metadata: { sourceDispatchedAt: Date.parse('2026-01-01T00:00:00Z') },
|
||||
};
|
||||
const upload = { file_id: 'upload', createdAt: new Date('2026-02-01T00:00:00Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([upload, output])).toEqual([output, upload]);
|
||||
});
|
||||
|
||||
/**
|
||||
* Every agent in a run primes the conversation's files plus its own. If a
|
||||
* private file could outrank a shared one, two agents would advertise
|
||||
* different paths for the same shared file into one mount namespace.
|
||||
*/
|
||||
it('sinks contributor-private files below shared ones regardless of age', () => {
|
||||
const shared = { file_id: 'shared', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
const priv = { file_id: 'private', createdAt: new Date('2026-06-01T00:00:00Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([priv, shared], new Set(['private']))).toEqual([
|
||||
shared,
|
||||
priv,
|
||||
]);
|
||||
});
|
||||
|
||||
it('still ranks by recency within each scope', () => {
|
||||
const sharedOld = { file_id: 's1', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
const sharedNew = { file_id: 's2', createdAt: new Date('2026-02-01T00:00:00Z') };
|
||||
const privOld = { file_id: 'p1', createdAt: new Date('2026-03-01T00:00:00Z') };
|
||||
const privNew = { file_id: 'p2', createdAt: new Date('2026-04-01T00:00:00Z') };
|
||||
expect(
|
||||
sortCodeFilesByDestinationPriority(
|
||||
[privOld, sharedOld, privNew, sharedNew],
|
||||
new Set(['p1', 'p2']),
|
||||
),
|
||||
).toEqual([sharedNew, sharedOld, privNew, privOld]);
|
||||
});
|
||||
|
||||
it('breaks ties on file_id so the order is stable across turns', () => {
|
||||
const createdAt = new Date('2026-01-01T00:00:00Z');
|
||||
const first = { file_id: 'aaa', createdAt };
|
||||
const second = { file_id: 'bbb', createdAt };
|
||||
expect(sortCodeFilesByDestinationPriority([second, first])).toEqual([first, second]);
|
||||
expect(sortCodeFilesByDestinationPriority([first, second])).toEqual([first, second]);
|
||||
});
|
||||
|
||||
it('sorts records without a timestamp last rather than dropping them', () => {
|
||||
const dated = { file_id: 'a', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
const undatedEarly = { file_id: 'b' };
|
||||
const unparsable = { file_id: 'c', createdAt: 'not-a-date' };
|
||||
expect(sortCodeFilesByDestinationPriority([undatedEarly, unparsable, dated])).toEqual([
|
||||
dated,
|
||||
undatedEarly,
|
||||
unparsable,
|
||||
]);
|
||||
});
|
||||
|
||||
it('preserves holes instead of throwing on them', () => {
|
||||
const dated = { file_id: 'a', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
expect(sortCodeFilesByDestinationPriority([null, dated, undefined])).toEqual([
|
||||
dated,
|
||||
null,
|
||||
undefined,
|
||||
]);
|
||||
});
|
||||
|
||||
it('does not mutate its input', () => {
|
||||
const older = { file_id: 'a', createdAt: new Date('2026-01-01T00:00:00Z') };
|
||||
const newer = { file_id: 'b', createdAt: new Date('2026-02-01T00:00:00Z') };
|
||||
const input = [older, newer];
|
||||
sortCodeFilesByDestinationPriority(input);
|
||||
expect(input).toEqual([older, newer]);
|
||||
});
|
||||
});
|
||||
222
packages/api/src/files/code/destinations.ts
Normal file
222
packages/api/src/files/code/destinations.ts
Normal file
|
|
@ -0,0 +1,222 @@
|
|||
import {
|
||||
appendLeafSuffix,
|
||||
flattenArtifactPath,
|
||||
deterministicHexSuffix,
|
||||
FILENAME_SEGMENT_MAX_BYTES,
|
||||
} from '~/utils/files';
|
||||
|
||||
/**
|
||||
* Sandbox input files mount at a destination derived from their `name`, and
|
||||
* codeapi rejects the whole `/exec` request when two entries resolve to the
|
||||
* same destination — or when one destination is a directory prefix of
|
||||
* another (`files contains duplicate destination "x"`, `files contains
|
||||
* conflicting destinations "a" and "a/b"`). A rejection is fatal for the
|
||||
* rest of the conversation: the request never reaches the sandbox, so no
|
||||
* result comes back to collapse the colliding refs, and every later turn
|
||||
* re-primes the same pair.
|
||||
*
|
||||
* Nothing about a LibreChat file record guarantees that uniqueness. Only
|
||||
* code-generated outputs are covered by the `(filename, conversationId,
|
||||
* context, tenantId)` partial unique index; user uploads carry
|
||||
* `context: message_attachment`, so one conversation can hold several
|
||||
* records sharing a `filename` — two uploads of `image.png`, or an upload
|
||||
* whose name a later execution wrote back in place.
|
||||
*
|
||||
* This module owns the name → destination mapping so every contributor to
|
||||
* an `/exec` file list agrees on it.
|
||||
*/
|
||||
export interface CodeDestinationSet {
|
||||
readonly names: Set<string>;
|
||||
readonly ancestors: Set<string>;
|
||||
}
|
||||
|
||||
export function createCodeDestinationSet(): CodeDestinationSet {
|
||||
return { names: new Set<string>(), ancestors: new Set<string>() };
|
||||
}
|
||||
|
||||
function collectAncestors(destination: string): string[] {
|
||||
const segments = destination.split('/');
|
||||
const ancestors: string[] = [];
|
||||
for (let i = 1; i < segments.length; i++) {
|
||||
ancestors.push(segments.slice(0, i).join('/'));
|
||||
}
|
||||
return ancestors;
|
||||
}
|
||||
|
||||
/** True when a claimed file sits at one of `destination`'s parent
|
||||
* directories, so every name under that directory conflicts. */
|
||||
function hasTakenAncestor(set: CodeDestinationSet, destination: string): boolean {
|
||||
const ancestors = collectAncestors(destination);
|
||||
for (let i = 0; i < ancestors.length; i++) {
|
||||
if (set.names.has(ancestors[i])) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Mirrors codeapi's `validateExecuteFiles` conflict rule: an exact match, or
|
||||
* either destination being a directory prefix of the other. */
|
||||
function isTaken(set: CodeDestinationSet, destination: string): boolean {
|
||||
if (set.names.has(destination) || set.ancestors.has(destination)) {
|
||||
return true;
|
||||
}
|
||||
return hasTakenAncestor(set, destination);
|
||||
}
|
||||
|
||||
function take(set: CodeDestinationSet, destination: string): void {
|
||||
set.names.add(destination);
|
||||
const ancestors = collectAncestors(destination);
|
||||
for (let i = 0; i < ancestors.length; i++) {
|
||||
set.ancestors.add(ancestors[i]);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Inserts `suffix` before the leaf's extension, preserving directory
|
||||
* structure (`a/b/c.txt` -> `a/b/c-9f2a11.txt`). The leaf is held to the same
|
||||
* per-segment byte budget `sanitizeFilename` applies, since codeapi caps
|
||||
* whole-path length and a name already sitting at the cap would otherwise
|
||||
* grow past it. The suffix survives that trim, so distinct suffixes stay
|
||||
* distinct and the search below always terminates.
|
||||
*/
|
||||
function withSuffix(destination: string, suffix: string): string {
|
||||
const slash = destination.lastIndexOf('/');
|
||||
const dir = slash === -1 ? '' : destination.slice(0, slash + 1);
|
||||
const leaf = destination.slice(slash + 1);
|
||||
return `${dir}${appendLeafSuffix(leaf, suffix, FILENAME_SEGMENT_MAX_BYTES)}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Claims a destination for `name`, falling back to `<stem>-<identity hash>`
|
||||
* when it is already spoken for. Callers that own what the model is told
|
||||
* about the sandbox use this: both files stay reachable, at names the caller
|
||||
* can echo into the tool context.
|
||||
*
|
||||
* The fallback hashes `identity` — a stable per-file value such as `file_id`
|
||||
* — rather than counting collisions, so a displaced file keeps the same
|
||||
* destination for the life of the conversation. A counter would be assigned
|
||||
* from the current set: a third file arriving under the literal name a
|
||||
* displaced file had been given would push that file along, and code the
|
||||
* model wrote in an earlier turn would silently start reading the newcomer.
|
||||
* The hash does not depend on what else is present, so a file that has been
|
||||
* told to the model at one path stays there.
|
||||
*
|
||||
* One case still moves it, and deliberately: when a *later* file is itself
|
||||
* named the alias, it takes it and the older file falls to a counter. In
|
||||
* practice that later file is the model's own in-place rewrite of the
|
||||
* displaced one — it was told `<stem>-<hash>.ext`, wrote back to it, and
|
||||
* `processCodeOutput` registered an output under that literal name — so the
|
||||
* path the model has been using keeps resolving to the newest content at
|
||||
* that path, which is the whole point of the recency ordering. Reserving the
|
||||
* alias against the newcomer instead would hand that path back to the
|
||||
* superseded original. An *unrelated* file reaching this branch would have
|
||||
* to be named for the hex digest of another file's `file_id`, which is not
|
||||
* a name a user can construct.
|
||||
*/
|
||||
export function claimCodeDestination(
|
||||
set: CodeDestinationSet,
|
||||
name: string,
|
||||
identity: string,
|
||||
): string {
|
||||
/* A claimed file sitting at a parent directory makes every name beneath it
|
||||
* conflict, so suffixing the leaf can never clear it. Flattening lifts the
|
||||
* path out from under that directory and leaves only whole-name conflicts,
|
||||
* which the suffix does clear. */
|
||||
const base = hasTakenAncestor(set, name)
|
||||
? flattenArtifactPath(name, FILENAME_SEGMENT_MAX_BYTES)
|
||||
: name;
|
||||
if (!isTaken(set, base)) {
|
||||
take(set, base);
|
||||
return base;
|
||||
}
|
||||
const identitySuffix = `-${deterministicHexSuffix(identity)}`;
|
||||
let destination = withSuffix(base, identitySuffix);
|
||||
for (let counter = 2; isTaken(set, destination); counter++) {
|
||||
destination = withSuffix(base, `${identitySuffix}-${counter}`);
|
||||
}
|
||||
take(set, destination);
|
||||
return destination;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reserves `name` only if it is free, reporting whether the caller may keep
|
||||
* the file. Merge points use this rather than {@link claimCodeDestination}:
|
||||
* a renamed destination there would be one the model was never told about,
|
||||
* and a conflict at that layer means the same logical file arrived twice
|
||||
* under different storage pointers, so dropping the later copy loses
|
||||
* nothing.
|
||||
*/
|
||||
export function reserveCodeDestination(set: CodeDestinationSet, name: string): boolean {
|
||||
if (isTaken(set, name)) {
|
||||
return false;
|
||||
}
|
||||
take(set, name);
|
||||
return true;
|
||||
}
|
||||
|
||||
interface CodeDestinationCandidate {
|
||||
file_id?: string;
|
||||
createdAt?: Date | string | number;
|
||||
/** Last content write for a reused code-output record. `processCodeOutput`
|
||||
* claims one row per `(filename, conversationId)` and rewrites it in
|
||||
* place, so `createdAt` marks when the name was first produced, not when
|
||||
* the bytes behind it last changed. */
|
||||
metadata?: { sourceDispatchedAt?: number } | null;
|
||||
}
|
||||
|
||||
function toTime(value: Date | string | number | undefined): number {
|
||||
if (value == null) {
|
||||
return 0;
|
||||
}
|
||||
const time = value instanceof Date ? value.getTime() : new Date(value).getTime();
|
||||
return Number.isFinite(time) ? time : 0;
|
||||
}
|
||||
|
||||
function contentTime(file: CodeDestinationCandidate | null | undefined): number {
|
||||
return Math.max(toTime(file?.metadata?.sourceDispatchedAt), toTime(file?.createdAt));
|
||||
}
|
||||
|
||||
/**
|
||||
* Orders files so the one holding the newest content claims the bare name and
|
||||
* the rest take an identity suffix. Newest-wins matches
|
||||
* `ToolNode.updateCodeSession`, which collapses by name the same way once
|
||||
* results come back, and it is the right answer when an execution rewrote an
|
||||
* uploaded file in place: the model's next read of that path gets its own
|
||||
* edit, not the superseded original.
|
||||
*
|
||||
* Recency is `max(metadata.sourceDispatchedAt, createdAt)`, never `updatedAt`.
|
||||
* `updatedAt` — the default `getFiles` sort — is bumped by usage accounting
|
||||
* and by re-upload, so destinations keyed on it would shuffle between turns
|
||||
* and silently repoint paths that code written in an earlier turn still
|
||||
* reads. `sourceDispatchedAt` moves only when a generated output's bytes are
|
||||
* rewritten, which is exactly when the ranking should change.
|
||||
*
|
||||
* `privateFileIds` names files that belong to a single contributor rather
|
||||
* than to the conversation, and sinks them below the shared ones. Each agent
|
||||
* in a run claims destinations over its own set — the conversation's files
|
||||
* plus its own — so without this a private file could take a bare name from a
|
||||
* shared file in one agent and not in another, leaving two agents advertising
|
||||
* different paths for the same file into one shared mount namespace.
|
||||
*
|
||||
* Holes are tolerated and preserved rather than filtered: callers hand this
|
||||
* a raw query result and keep their own per-entry guard.
|
||||
*/
|
||||
export function sortCodeFilesByDestinationPriority<T extends CodeDestinationCandidate>(
|
||||
files: Array<T | null | undefined>,
|
||||
privateFileIds?: ReadonlySet<string>,
|
||||
): Array<T | null | undefined> {
|
||||
const isPrivate = (file: T | null | undefined): number =>
|
||||
privateFileIds != null && file?.file_id != null && privateFileIds.has(file.file_id) ? 1 : 0;
|
||||
return [...files].sort((a, b) => {
|
||||
const scope = isPrivate(a) - isPrivate(b);
|
||||
if (scope !== 0) {
|
||||
return scope;
|
||||
}
|
||||
const delta = contentTime(b) - contentTime(a);
|
||||
if (delta !== 0) {
|
||||
return delta;
|
||||
}
|
||||
return (a?.file_id ?? '').localeCompare(b?.file_id ?? '');
|
||||
});
|
||||
}
|
||||
|
|
@ -1,4 +1,5 @@
|
|||
export * from './classify';
|
||||
export * from './destinations';
|
||||
export * from './extract';
|
||||
export * from './form';
|
||||
export * from './identity';
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ const USER_FACING_UPLOAD_ERRORS = [
|
|||
|
||||
const ASCII_FILENAME_SAFE_PATTERN = /^[a-zA-Z0-9._-]$/;
|
||||
const UNSAFE_UNICODE_FILENAME_PATTERN = /[^\p{L}\p{M}\p{N}\p{Emoji}\u200d._-]/gu;
|
||||
const FILENAME_SEGMENT_MAX_BYTES = 255;
|
||||
export const FILENAME_SEGMENT_MAX_BYTES = 255;
|
||||
|
||||
function sanitizeFilenameSegment(segment: string): string {
|
||||
const asciiSanitized = Array.from(segment.normalize('NFC'), (char) => {
|
||||
|
|
@ -63,6 +63,25 @@ function truncateLeafWithSuffix(leaf: string, suffix: string, maxBytes: number):
|
|||
return truncateUtf8Bytes(stem, stemBudget) + suffix + ext;
|
||||
}
|
||||
|
||||
/**
|
||||
* Composes a leaf as `<stem><suffix><ext>`, trimming the stem so the result
|
||||
* stays inside `maxBytes`. Unlike {@link truncateLeafWithSuffix}, whose suffix
|
||||
* marks a truncation and is therefore only applied when one happens, this
|
||||
* always applies the suffix — callers use it to disambiguate one name from
|
||||
* another, so a trim that dropped the suffix would hand back the name they
|
||||
* were trying to move away from.
|
||||
*/
|
||||
export function appendLeafSuffix(leaf: string, suffix: string, maxBytes: number): string {
|
||||
const ext = path.extname(leaf);
|
||||
const stem = path.basename(leaf, ext);
|
||||
const suffixBytes = utf8ByteLength(suffix);
|
||||
const extBytes = utf8ByteLength(ext);
|
||||
if (extBytes + suffixBytes >= maxBytes) {
|
||||
return truncateUtf8Bytes(`${stem}${suffix}${ext}`, maxBytes);
|
||||
}
|
||||
return `${truncateUtf8Bytes(stem, maxBytes - extBytes - suffixBytes)}${suffix}${ext}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a user-facing error message from a file upload error.
|
||||
* When redaction is enabled, maps known error patterns to fixed messages
|
||||
|
|
@ -161,7 +180,7 @@ const ARTIFACT_PATH_TOTAL_MAX_BYTES = 512;
|
|||
* collision becomes likely, vs. the realistic ceiling of single-digit
|
||||
* artifacts per turn).
|
||||
*/
|
||||
function deterministicHexSuffix(input: string): string {
|
||||
export function deterministicHexSuffix(input: string): string {
|
||||
return crypto.createHash('sha256').update(input).digest('hex').slice(0, 6);
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue