diff --git a/api/src/api/v2-files.test.ts b/api/src/api/v2-files.test.ts index 540b529b..6a142ee9 100644 --- a/api/src/api/v2-files.test.ts +++ b/api/src/api/v2-files.test.ts @@ -1,7 +1,8 @@ import { describe, expect, test } from 'bun:test'; import { config } from '../config'; +import { collectExecuteRequestInputFiles } from '../execution-manifest-request'; import type { TFile } from '../job'; -import { validateExecuteArguments, validateExecuteFiles } from './v2'; +import { validateExecuteArguments, validateExecuteFiles, deduplicateFilesByDestination } from './v2'; function messageOf(fn: () => void): string { try { @@ -85,4 +86,74 @@ describe('execute file validation', () => { ]; expect(() => validateExecuteFiles(files)).not.toThrow(); }); + + test('deduplicateFilesByDestination keeps the latest occurrence in surviving order', () => { + const files: TFile[] = [ + { name: 'data.csv', content: 'first' }, + { name: 'data.csv', content: 'second' }, + { name: 'other.csv', content: 'unique' }, + { name: 'data.csv', content: 'third' }, + ]; + const result = deduplicateFilesByDestination(files); + expect(result).toHaveLength(2); + expect(result[0].name).toBe('other.csv'); + expect(result[1].name).toBe('data.csv'); + expect(result[1].content).toBe('third'); + }); + + test('deduplicateFilesByDestination returns the same array when there are no duplicates', () => { + const files: TFile[] = [ + { name: 'a.csv', content: 'a' }, + { name: 'b.csv', content: 'b' }, + ]; + const result = deduplicateFilesByDestination(files); + expect(result).toEqual(files); + }); + + test('preserves the original destination of an unnamed file reference after deduplication', () => { + const files: TFile[] = [ + { name: 'main.py', content: 'print(1)' }, + { name: 'data.csv', content: 'old' }, + { name: 'data.csv', content: 'new' }, + { id: 'file-ref', storage_session_id: 'storage-session' } as TFile, + ]; + const deduped = deduplicateFilesByDestination(files); + expect(deduped.map(file => file.name)).toEqual(['main.py', 'data.csv', 'file3.code']); + expect(deduped[1].content).toBe('new'); + expect(collectExecuteRequestInputFiles({ files: deduped })).toEqual( + collectExecuteRequestInputFiles({ files }), + ); + expect(() => validateExecuteFiles(deduped)).not.toThrow(); + }); + + test('rejects malformed files even if another entry owns their destination', () => { + expect(messageOf(() => deduplicateFilesByDestination([ + { name: 'file1.code', content: 'source' }, + null as unknown as TFile, + ]))).toContain('files[1] must be an object'); + expect(messageOf(() => deduplicateFilesByDestination([ + { name: 'data.csv', content: 'old', encoding: 'invalid' as TFile['encoding'] }, + { name: 'data.csv', content: 'new' }, + ]))).toContain('files[0].encoding'); + expect(messageOf(() => deduplicateFilesByDestination([ + { name: 'data.csv', content: 'old' }, + { name: 'data.csv', id: 'file-ref' }, + ]))).toContain('files[1].storage_session_id'); + }); + + test('caps raw input count before dropping duplicates', () => { + const files = Array.from({ length: config.max_input_files + 1 }, () => ({ + name: 'data.csv', content: 'duplicate', + })); + expect(messageOf(() => deduplicateFilesByDestination(files))).toContain('cannot contain more than'); + }); + + test('deduplicateFilesByDestination allows validateExecuteFiles to accept previously-duplicate input', () => { + const files: TFile[] = [ + { name: 'data.csv', content: 'first' }, + { name: 'data.csv', content: 'second' }, + ]; + const deduped = deduplicateFilesByDestination(files); + expect(() => validateExecuteFiles(deduped)).not.toThrow(); + }); }); diff --git a/api/src/api/v2-session-binding.test.ts b/api/src/api/v2-session-binding.test.ts index 32a245f6..709b3b28 100644 --- a/api/src/api/v2-session-binding.test.ts +++ b/api/src/api/v2-session-binding.test.ts @@ -281,6 +281,52 @@ describe('per-request session binding', () => { } }); + test('uses the latest upload without renumbering later unnamed files', async () => { + config.session_workspace_enabled = false; + config.require_execution_manifest = false; + + const originalPrime = Job.prototype.prime; + const originalExecute = Job.prototype.execute; + const originalCleanup = Job.prototype.cleanup; + + let primedFiles: Array<{ name: string; content?: string }> = []; + Job.prototype.prime = async function captureFiles(): Promise { + primedFiles = this.files.map(({ name, content }) => ({ name, content })); + }; + Job.prototype.execute = async function executeWithoutSandbox() { + return {} as Awaited>; + }; + Job.prototype.cleanup = async function cleanupWithoutFilesystem(): Promise {}; + + try { + const response = await fetch(`${baseUrl}/api/v2/execute`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + language: testLanguage, + version: testVersion, + files: [ + { name: 'main.txt', content: 'source' }, + { name: 'data.csv', content: 'old upload' }, + { name: 'data.csv', content: 'new upload' }, + { content: 'unnamed source' }, + ], + }), + }); + + expect(response.status).toBe(200); + expect(primedFiles).toEqual([ + { name: 'main.txt', content: 'source' }, + { name: 'data.csv', content: 'new upload' }, + { name: 'file3.code', content: 'unnamed source' }, + ]); + } finally { + Job.prototype.prime = originalPrime; + Job.prototype.execute = originalExecute; + Job.prototype.cleanup = originalCleanup; + } + }); + test('a post-prime failure still reports the workspace as dirty', async () => { config.session_workspace_enabled = true; config.require_execution_manifest = false; diff --git a/api/src/api/v2.ts b/api/src/api/v2.ts index e2f252ce..fe1567bf 100644 --- a/api/src/api/v2.ts +++ b/api/src/api/v2.ts @@ -46,72 +46,109 @@ import { const router = express.Router(); const SYNTHETIC_PRINCIPAL_SOURCE = 'synthetic_test'; +/** + * Deduplicate files by destination name. Some callers (e.g. LibreChat) + * send the same file more than once per request when a user re-uploads a + * file in the same conversation. The sandbox rejects duplicate destinations, + * which surfaces to the end user as a confusing "sandbox is down" error. + * Keeps the latest occurrence, preserving the order of surviving files. + */ +export function deduplicateFilesByDestination(files: TFile[]): TFile[] { + if (files.length > config.max_input_files) { + throw { message: `files cannot contain more than ${config.max_input_files} destinations` }; + } + const seen = new Set(); + const deduped: TFile[] = []; + for (let i = files.length - 1; i >= 0; i--) { + const file = files[i]; + // Validate even entries that would otherwise be dropped as duplicates. + const destination = validateExecuteFile(file, i); + if (seen.has(destination)) continue; + seen.add(destination); + // Manifest claims use original indices; Job would otherwise renumber this file. + deduped.push(file.name ? file : { ...file, name: destination }); + } + deduped.reverse(); + if (deduped.length < files.length) { + logger.warn( + { original: files.length, deduped: deduped.length }, + 'Deduplicated file list before validation', + ); + } + return deduped; +} + function existingDestinationConflictMessage(existing: string, destination: string): string { return existing === destination ? `files contains duplicate destination "${destination}"` : `files contains conflicting destinations "${existing}" and "${destination}"`; } +function validateExecuteFile(value: TFile, i: number): string { + if (value == null || typeof value !== 'object' || Array.isArray(value)) { + throw { message: `files[${i}] must be an object` }; + } + const file = value as TFile; + const inline = typeof file.content === 'string'; + const byRef = typeof file.id === 'string' && file.id.length > 0; + if (inline === byRef) { + throw { + message: `files[${i}] must contain exactly one of non-empty id or string content`, + }; + } + if (file.id !== undefined && !byRef) { + throw { message: `files[${i}].id must be a non-empty string if provided` }; + } + if (byRef) { + if (typeof file.storage_session_id !== 'string' || file.storage_session_id.length === 0) { + throw { message: `files[${i}].storage_session_id is required as a non-empty string for file refs` }; + } + } else if (file.storage_session_id !== undefined || file.input_cache_key !== undefined) { + throw { + message: `files[${i}] inline content cannot include storage_session_id or input_cache_key`, + }; + } + if (file.name !== undefined && typeof file.name !== 'string') { + throw { message: `files[${i}].name must be a string if provided` }; + } + if ( + file.encoding !== undefined + && !(['base64', 'hex', 'utf8'] as const).includes(file.encoding) + ) { + throw { message: `files[${i}].encoding must be base64, hex, or utf8 if provided` }; + } + if (file.entity_id !== undefined && typeof file.entity_id !== 'string') { + throw { message: `files[${i}].entity_id must be a string if provided` }; + } + if ( + file.input_cache_key !== undefined && + ( + typeof file.input_cache_key !== 'string' || + !/^[0-9a-f]{64}$/.test(file.input_cache_key) + ) + ) { + throw { message: `files[${i}].input_cache_key must be a 64-character lowercase hex digest` }; + } + const destination = file.name || `file${i}.code`; + try { + validateFilePath(destination, '/tmp/codeapi-request-validation'); + } catch (error) { + throw { + message: error instanceof Error + ? `files[${i}].name is invalid: ${error.message}` + : `files[${i}].name is invalid`, + }; + } + return destination; +} + export function validateExecuteFiles(files: TFile[]): void { if (files.length > config.max_input_files) { throw { message: `files cannot contain more than ${config.max_input_files} destinations` }; } const destinations = new Set(); for (const [i, value] of files.entries()) { - if (value == null || typeof value !== 'object' || Array.isArray(value)) { - throw { message: `files[${i}] must be an object` }; - } - const file = value as TFile; - const inline = typeof file.content === 'string'; - const byRef = typeof file.id === 'string' && file.id.length > 0; - if (inline === byRef) { - throw { - message: `files[${i}] must contain exactly one of non-empty id or string content`, - }; - } - if (file.id !== undefined && !byRef) { - throw { message: `files[${i}].id must be a non-empty string if provided` }; - } - if (byRef) { - if (typeof file.storage_session_id !== 'string' || file.storage_session_id.length === 0) { - throw { message: `files[${i}].storage_session_id is required as a non-empty string for file refs` }; - } - } else if (file.storage_session_id !== undefined || file.input_cache_key !== undefined) { - throw { - message: `files[${i}] inline content cannot include storage_session_id or input_cache_key`, - }; - } - if (file.name !== undefined && typeof file.name !== 'string') { - throw { message: `files[${i}].name must be a string if provided` }; - } - if ( - file.encoding !== undefined - && !(['base64', 'hex', 'utf8'] as const).includes(file.encoding) - ) { - throw { message: `files[${i}].encoding must be base64, hex, or utf8 if provided` }; - } - if (file.entity_id !== undefined && typeof file.entity_id !== 'string') { - throw { message: `files[${i}].entity_id must be a string if provided` }; - } - if ( - file.input_cache_key !== undefined && - ( - typeof file.input_cache_key !== 'string' || - !/^[0-9a-f]{64}$/.test(file.input_cache_key) - ) - ) { - throw { message: `files[${i}].input_cache_key must be a 64-character lowercase hex digest` }; - } - const destination = file.name || `file${i}.code`; - try { - validateFilePath(destination, '/tmp/codeapi-request-validation'); - } catch (error) { - throw { - message: error instanceof Error - ? `files[${i}].name is invalid: ${error.message}` - : `files[${i}].name is invalid`, - }; - } + const destination = validateExecuteFile(value, i); const conflict = [...destinations].find( existing => existing === destination || @@ -251,12 +288,13 @@ function getJob( runtimeSessionHeader?: string | string[], ): Job { const { - session_id, language, version, args, stdin, files, + session_id, language, version, args, stdin, files: rawFiles, compile_memory_limit, run_memory_limit, compile_timeout, run_cpu_time, compile_cpu_time, env_vars, } = body; + let files = rawFiles; if (!language || typeof language !== 'string') { throw { message: 'language is required as a string' }; @@ -271,6 +309,7 @@ function getJob( throw { message: 'tool_call_socket must be a boolean if specified' }; } validateExecuteArguments(args, stdin); + files = deduplicateFilesByDestination(files); validateExecuteFiles(files); const rt = getLatestRuntimeMatchingLanguageVersion(language, version);