Skip to content

Commit 2a80121

Browse files
committed
fix(providers): route 6-10MB attachments to the provider large-file path
The inline base64 cap was 10 MB of raw bytes, but the execution payload store refuses a single value above 8 MiB and base64 inflates by 4/3. Every raw file over 6 MiB therefore produced a base64 string the store rejected — and the rejection came from the base64 *cache* write, which threw and failed the run with "Execution memory limit exceeded" even though the bytes had already been read successfully. Because shouldUseLargeFilePath only fires above the inline cap, 6-10 MB attachments had no path at all on any provider: they never reached the OpenAI or Gemini Files API upload they were supposed to take. Derive the cap from the payload-store ceiling instead of hardcoding it, and degrade a refused cache write to "not cached" rather than failing a request whose bytes are in hand. Every other size guard in the chain compares raw bytes against maxBytes; only the Redis write sees the encoded size, which is why this went unnoticed — and why it failed only where Redis is configured. Also correct the provider ceilings against the vendors' current documentation: - openai: 50 MiB -> 50,000,000. The gate is `size > maxBytes`, so 50 MiB admitted 52,428,800 bytes; the docs say each file must be *under* 50 MB. - bedrock: had no entry and inherited the inline cap, which is above what Converse accepts (3.75 MB per image, 4.5 MB per document). - groq: 20 MiB -> 20,000,000, and modelled as the request cap the docs actually describe rather than a per-file MiB ceiling. - fireworks: had no entry; its 10 MB budget is on the base64 total, so the raw-byte equivalent is 7.5 MB. Add perRequestMaxBytes for the combined ceilings, enforced before any upload spend, and cover the OpenAI upload path end to end — it had no test at all.
1 parent 3de63c9 commit 2a80121

8 files changed

Lines changed: 325 additions & 30 deletions

File tree

apps/sim/executor/handlers/agent/agent-handler.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -970,8 +970,13 @@ export class AgentBlockHandler implements BlockHandler {
970970
(file) => !file.base64 && !shouldUseLargeFilePath(file, providerId)
971971
)
972972
if (missingFile) {
973+
const inlineMB = (INLINE_ATTACHMENT_THRESHOLD_BYTES / (1024 * 1024)).toFixed(0)
974+
const oversized =
975+
Number.isFinite(missingFile.size) && missingFile.size > INLINE_ATTACHMENT_THRESHOLD_BYTES
973976
throw new Error(
974-
`File "${missingFile.name}" could not be read for provider "${providerId}". The file may exceed the attachment size limit or may no longer be accessible.`
977+
oversized
978+
? `File "${missingFile.name}" (${(missingFile.size / (1024 * 1024)).toFixed(2)}MB) exceeds the ${inlineMB}MB inline attachment limit, and provider "${providerId}" has no large-file upload path for it.`
979+
: `File "${missingFile.name}" could not be read for provider "${providerId}". The file may no longer be accessible.`
975980
)
976981
}
977982

apps/sim/lib/uploads/utils/user-file-base64.server.test.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,40 @@ describe('hydrateUserFilesWithBase64', () => {
400400
expect(mockRedis.eval).toHaveBeenCalledOnce()
401401
})
402402

403+
/**
404+
* Reproduces the agent-attachment failure: a file under the inline limit whose base64 exceeds
405+
* the 8 MiB single-Redis-write cap. The bytes are already read by the time the cache is
406+
* written, so a refused cache write must degrade to "not cached", not fail the execution.
407+
*/
408+
it('still returns base64 when the value is too large to cache', async () => {
409+
mockGetRedisClient.mockReturnValue(mockRedis)
410+
const buffer = Buffer.alloc(9 * 1024 * 1024, 0x61)
411+
mockDownloadFile.mockResolvedValueOnce(buffer)
412+
const file: UserFile = {
413+
id: 'file-1',
414+
name: 'data_10mb.csv',
415+
key: 'execution/workspace/workflow/exec-1/data_10mb.csv',
416+
url: 'https://example.com/data_10mb.csv',
417+
size: buffer.length,
418+
type: 'text/csv',
419+
context: 'execution',
420+
}
421+
422+
const hydrated = await hydrateUserFilesWithBase64(
423+
{ file },
424+
{
425+
workspaceId: 'workspace',
426+
workflowId: 'workflow',
427+
executionId: 'exec-1',
428+
userId: 'user-1',
429+
maxBytes: 10 * 1024 * 1024,
430+
}
431+
)
432+
433+
expect(hydrated.file.base64).toBe(buffer.toString('base64'))
434+
expect(mockRedis.eval).not.toHaveBeenCalled()
435+
})
436+
403437
it('releases indexed budget entries even when cache keys already expired', async () => {
404438
mockGetRedisClient.mockReturnValue(mockRedis)
405439
mockRedis.hgetall.mockResolvedValueOnce({

apps/sim/lib/uploads/utils/user-file-base64.server.ts

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,7 @@ import {
2323
getExecutionRedisBudgetKeys,
2424
getExecutionRedisBudgetLimits,
2525
} from '@/lib/execution/redis-budget.server'
26-
import {
27-
ExecutionResourceLimitError,
28-
isExecutionResourceLimitError,
29-
} from '@/lib/execution/resource-errors'
26+
import { ExecutionResourceLimitError } from '@/lib/execution/resource-errors'
3027
import { isGeneratedDocumentSourceType } from '@/lib/uploads/utils/file-utils'
3128
import type { UserFile } from '@/executor/types'
3229

@@ -196,6 +193,24 @@ class InMemoryBase64Cache implements Base64Cache {
196193
}
197194
}
198195

196+
/**
197+
* The base64 cache only saves a repeat read from storage — the bytes it would have stored are
198+
* already in hand by the time it is written. Exceeding a Redis budget therefore means "do not
199+
* cache", never "fail the run": throwing here turned an oversized attachment into an opaque
200+
* "Execution memory limit exceeded" on a request that had already read the file successfully.
201+
*/
202+
function logSkippedCacheWrite(
203+
logger: Logger,
204+
requestId: string | undefined,
205+
file: UserFile,
206+
error: ExecutionResourceLimitError
207+
): void {
208+
logger.warn(
209+
`[${requestId ?? 'unknown'}] Skipping base64 cache write for ${file.name}: ${error.message}`,
210+
{ resource: error.resource, attemptedBytes: error.attemptedBytes }
211+
)
212+
}
213+
199214
function createBase64Cache(options: Base64HydrationOptions, logger: Logger): Base64Cache {
200215
const redis = getRedisClient()
201216
const { executionId } = options
@@ -228,11 +243,17 @@ function createBase64Cache(options: Base64HydrationOptions, logger: Logger): Bas
228243

229244
const limits = getExecutionRedisBudgetLimits()
230245
if (valueBytes > limits.maxSingleWriteBytes) {
231-
throw new ExecutionResourceLimitError({
232-
resource: 'redis_key_bytes',
233-
attemptedBytes: valueBytes,
234-
limitBytes: limits.maxSingleWriteBytes,
235-
})
246+
logSkippedCacheWrite(
247+
logger,
248+
options.requestId,
249+
file,
250+
new ExecutionResourceLimitError({
251+
resource: 'redis_key_bytes',
252+
attemptedBytes: valueBytes,
253+
limitBytes: limits.maxSingleWriteBytes,
254+
})
255+
)
256+
return
236257
}
237258
const cacheTtlSeconds = Math.max(ttlSeconds, limits.ttlSeconds)
238259
const budgetReservation: ExecutionRedisBudgetReservation = {
@@ -261,19 +282,21 @@ function createBase64Cache(options: Base64HydrationOptions, logger: Logger): Bas
261282
)) as [number, string, number | string | null]
262283
const [allowed, resource, current] = result
263284
if (allowed !== 1) {
264-
throw new ExecutionResourceLimitError({
265-
resource:
266-
resource === 'user_redis_bytes' ? 'user_redis_bytes' : 'execution_redis_bytes',
267-
attemptedBytes: valueBytes,
268-
currentBytes: Number(current ?? 0),
269-
limitBytes:
270-
resource === 'user_redis_bytes' ? limits.maxUserBytes : limits.maxExecutionBytes,
271-
})
285+
logSkippedCacheWrite(
286+
logger,
287+
options.requestId,
288+
file,
289+
new ExecutionResourceLimitError({
290+
resource:
291+
resource === 'user_redis_bytes' ? 'user_redis_bytes' : 'execution_redis_bytes',
292+
attemptedBytes: valueBytes,
293+
currentBytes: Number(current ?? 0),
294+
limitBytes:
295+
resource === 'user_redis_bytes' ? limits.maxUserBytes : limits.maxExecutionBytes,
296+
})
297+
)
272298
}
273299
} catch (error) {
274-
if (isExecutionResourceLimitError(error)) {
275-
throw error
276-
}
277300
logger.warn(`[${options.requestId}] Redis set failed, skipping cache`, error)
278301
}
279302
},

apps/sim/providers/attachments.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5+
import { LARGE_VALUE_THRESHOLD_BYTES } from '@/lib/execution/payloads/large-value-ref'
56
import type { UserFile } from '@/executor/types'
67
import {
78
buildAnthropicMessageContent,
@@ -286,6 +287,16 @@ describe('provider attachments', () => {
286287
})
287288

288289
describe('provider large-file capability', () => {
290+
/**
291+
* Guards the regression where the inline cap (10 MB) sat above what the payload store could
292+
* hold once base64 inflated it, so every 6-10 MB attachment died with "Execution memory limit
293+
* exceeded" instead of taking the provider's large-file path.
294+
*/
295+
it('keeps the inline cap inside the payload store ceiling once base64-encoded', () => {
296+
const encodedBytes = Math.ceil(INLINE_ATTACHMENT_THRESHOLD_BYTES / 3) * 4
297+
expect(encodedBytes).toBeLessThanOrEqual(LARGE_VALUE_THRESHOLD_BYTES)
298+
})
299+
289300
it('reports per-provider strategy and ceiling, defaulting others to inline', () => {
290301
expect(getProviderFileStrategy('openai')).toBe('files-api')
291302
expect(getProviderFileStrategy('google')).toBe('files-api')
@@ -298,8 +309,9 @@ describe('provider large-file capability', () => {
298309
expect(getProviderAttachmentMaxBytes('openai')).toBeGreaterThan(
299310
INLINE_ATTACHMENT_THRESHOLD_BYTES
300311
)
301-
expect(getProviderAttachmentMaxBytes('bedrock')).toBe(INLINE_ATTACHMENT_THRESHOLD_BYTES)
302312
expect(getProviderAttachmentMaxBytes('azure-openai')).toBe(INLINE_ATTACHMENT_THRESHOLD_BYTES)
313+
/** Bedrock Converse caps an image at 3.75MB — below the inline cap, so it needs its own entry. */
314+
expect(getProviderAttachmentMaxBytes('bedrock')).toBe(3_750_000)
303315
})
304316

305317
it('routes only oversized files on capable providers to the large-file path', () => {

apps/sim/providers/attachments.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ type ProviderFormattedMessage = {
7676
}
7777

7878
/**
79-
* Files at or below this size are inlined as base64, exactly as before. Larger files take
80-
* the provider's large-file path. Keeping the threshold at the legacy 10 MB cap guarantees
81-
* identical behaviour for existing attachments.
79+
* Files at or below this size are inlined as base64; larger files take the provider's
80+
* large-file path. Sized to the execution payload store, not to any provider — see
81+
* {@link INLINE_ATTACHMENT_MAX_BYTES}.
8282
*/
8383
export const INLINE_ATTACHMENT_THRESHOLD_BYTES = INLINE_ATTACHMENT_MAX_BYTES
8484

@@ -202,6 +202,17 @@ export function getProviderAttachmentMaxBytes(providerId: ProviderId | string):
202202
return getProviderFileAttachment(providerId).maxBytes
203203
}
204204

205+
/**
206+
* Combined attachment ceiling for one request, or `null` when the provider documents none.
207+
* Separate from {@link getProviderAttachmentMaxBytes}: a provider can accept a 50MB file yet
208+
* still reject three 20MB files in the same call.
209+
*/
210+
export function getProviderRequestAttachmentMaxBytes(
211+
providerId: ProviderId | string
212+
): number | null {
213+
return getProviderFileAttachment(providerId).perRequestMaxBytes ?? null
214+
}
215+
205216
export function inferAttachmentMimeType(file: UserFile): string {
206217
const explicitType = file.type?.trim().toLowerCase()
207218
return resolveFileType({
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { buildOpenAIMessageContent } from '@/providers/attachments'
6+
import {
7+
attachLargeFileRemoteUrls,
8+
uploadLargeFilesToProvider,
9+
} from '@/providers/file-attachments.server'
10+
import type { ProviderRequest } from '@/providers/types'
11+
12+
const {
13+
mockDownloadServableFileFromStorage,
14+
mockGeneratePresignedDownloadUrl,
15+
mockHasCloudStorage,
16+
mockVerifyFileAccess,
17+
} = vi.hoisted(() => ({
18+
mockDownloadServableFileFromStorage: vi.fn(),
19+
mockGeneratePresignedDownloadUrl: vi.fn(),
20+
mockHasCloudStorage: vi.fn(),
21+
mockVerifyFileAccess: vi.fn(),
22+
}))
23+
24+
vi.mock('@google/genai', () => ({
25+
FileState: { PROCESSING: 'PROCESSING', FAILED: 'FAILED' },
26+
GoogleGenAI: class {},
27+
}))
28+
29+
vi.mock('@/lib/uploads', () => ({
30+
StorageService: {
31+
hasCloudStorage: mockHasCloudStorage,
32+
generatePresignedDownloadUrl: mockGeneratePresignedDownloadUrl,
33+
},
34+
}))
35+
36+
vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
37+
downloadServableFileFromStorage: mockDownloadServableFileFromStorage,
38+
}))
39+
40+
vi.mock('@/app/api/files/authorization', () => ({
41+
verifyFileAccess: mockVerifyFileAccess,
42+
}))
43+
44+
/** The exact file from the reported failure: 9,591,617 bytes — over 6 MiB, under 50 MB. */
45+
const CSV_BYTES = 9_591_617
46+
47+
function makeRequest(size: number): ProviderRequest {
48+
return {
49+
model: 'gpt-4.1',
50+
apiKey: 'sk-test',
51+
userId: 'user-1',
52+
workflowId: 'workflow-1',
53+
messages: [
54+
{
55+
role: 'user',
56+
content: 'what does this say',
57+
files: [
58+
{
59+
id: 'file-1',
60+
name: 'data_10mb.csv',
61+
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/data_10mb.csv',
62+
url: '',
63+
size,
64+
type: 'text/csv',
65+
context: 'workspace',
66+
},
67+
],
68+
},
69+
],
70+
} as unknown as ProviderRequest
71+
}
72+
73+
describe('OpenAI large-file attachment lifecycle', () => {
74+
beforeEach(() => {
75+
vi.clearAllMocks()
76+
mockHasCloudStorage.mockReturnValue(true)
77+
mockVerifyFileAccess.mockResolvedValue(true)
78+
mockGeneratePresignedDownloadUrl.mockResolvedValue('https://storage.example.com/signed')
79+
mockDownloadServableFileFromStorage.mockResolvedValue({
80+
buffer: Buffer.alloc(CSV_BYTES, 0x61),
81+
contentType: 'text/csv',
82+
})
83+
vi.stubGlobal(
84+
'fetch',
85+
vi.fn(async () => new Response(JSON.stringify({ id: 'file-abc' }), { status: 200 }))
86+
)
87+
})
88+
89+
it('uploads to the Files API and references the file by id instead of inlining it', async () => {
90+
const request = makeRequest(CSV_BYTES)
91+
92+
await attachLargeFileRemoteUrls(request, 'openai')
93+
await uploadLargeFilesToProvider(request, 'openai')
94+
95+
const [url, init] = (fetch as unknown as ReturnType<typeof vi.fn>).mock.calls[0]
96+
expect(url).toBe('https://api.openai.com/v1/files')
97+
expect(init.method).toBe('POST')
98+
expect(init.headers.Authorization).toBe('Bearer sk-test')
99+
100+
const form = init.body as FormData
101+
expect(form.get('purpose')).toBe('user_data')
102+
expect(form.get('expires_after[anchor]')).toBe('created_at')
103+
expect(form.get('expires_after[seconds]')).toBe('3600')
104+
expect((form.get('file') as File).size).toBe(CSV_BYTES)
105+
106+
const file = request.messages?.[0].files?.[0]
107+
expect(file?.providerFileId).toBe('file-abc')
108+
109+
const content = buildOpenAIMessageContent(
110+
'what does this say',
111+
request.messages?.[0].files,
112+
'openai'
113+
)
114+
expect(content).toEqual([
115+
{ type: 'input_text', text: 'what does this say' },
116+
{ type: 'input_file', file_id: 'file-abc' },
117+
])
118+
})
119+
120+
it('leaves files at or below the inline cap on the base64 path', async () => {
121+
const request = makeRequest(5 * 1024 * 1024)
122+
123+
await attachLargeFileRemoteUrls(request, 'openai')
124+
await uploadLargeFilesToProvider(request, 'openai')
125+
126+
expect(fetch).not.toHaveBeenCalled()
127+
expect(request.messages?.[0].files?.[0].providerFileId).toBeUndefined()
128+
expect(request.messages?.[0].files?.[0].remoteUrl).toBeUndefined()
129+
})
130+
131+
it('rejects a request whose attachments together exceed the combined ceiling', async () => {
132+
const request = makeRequest(30 * 1024 * 1024)
133+
const [first] = request.messages?.[0].files ?? []
134+
request.messages?.[0].files?.push({ ...first, id: 'file-2', key: `${first.key}-2` })
135+
136+
await expect(attachLargeFileRemoteUrls(request, 'openai')).rejects.toThrow(
137+
/total 60.00MB, which exceeds the 48MB combined attachment limit/
138+
)
139+
expect(fetch).not.toHaveBeenCalled()
140+
})
141+
})

0 commit comments

Comments
 (0)