mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(files): serve rendered documents instead of source code (#6139)
* fix(files): serve rendered documents to attachments and workflow reads Generated documents store their generation source under a .pdf/.docx name and keep the compiled binary in a separate content-addressed artifact store, so any consumer doing a raw read handed out source text under a document name. - Route attachments and readUserFileContent through the servable resolver so they get the compiled artifact, and stop the internal generation-source MIME marker reaching providers as a content type. - Render on read when the artifact is missing. The artifact key is (workspace, source hash), so forking a workspace, moving a file, or editing the source outside a recompiling writer orphaned it permanently and reported "still being generated" forever. Rendering self-heals those and stores the result. - Fall back to serving stored bytes as application/octet-stream when a render fails, instead of failing forever, and remember not to retry those bytes. - Only compile without a workspace context when the file's type positively says it is generation source, so unrelated stored bytes are never executed. - Make the doc-not-ready error opt-in per caller and give it a 409 via HttpError, so output decoration degrades instead of failing completed work. * fix(files): keep render failures retryable and never relabel unrendered bytes Addresses the first review round. - Only memoize a render failure when it is deterministic. A DocCompileUserError means the source will never render, so remembering it is safe; sandbox outages, timeouts, and cancellations are transient and were stranding valid documents for the life of the process. Infra failures now propagate, which also restores DocCompileUserError reaching callers that map it to 409. - Refuse to hand back bytes the resolver could not render. readUserFileContent returns a string, so the resolver's honest application/octet-stream could not travel with it and attachment builders re-inferred a document MIME from the filename — shipping generation source to a provider as a PDF. The file-serve route keeps the graceful passthrough, where a human downloading the bytes is useful. - Normalize the declared type once so a padded or upper-cased source marker cannot pass the resolver gate on one code path and fail it on the other. - Import the doc-not-ready guard lazily. The static import pulled the doc-compile module graph (remote sandbox, task runner, execution limits) into every hydration consumer and broke an unrelated test's module mock in CI. * perf(files): coalesce concurrent renders of the same missing artifact An artifact miss is identical for every concurrent reader — a freshly forked workspace whose document several viewers open at once, or one request whose blocks read the same file — and each was paying for its own compile of the same bytes. Share one in-flight render per (workspace, source, ext) key and drop the entry as soon as it settles, so a later read still re-renders normally. * fix(files): refuse unrendered bytes at the download boundary Addresses the second review round. - Throw UnrenderableDocumentError from downloadServableFileFromStorage instead of returning bytes with an `unrendered` flag. Around 45 call sites (email attachments, cloud uploads, zip entries, provider attachments) receive only a Buffer and re-infer the type from the filename, so a flag they must remember to check is a flag they will not check. Those callers already handled the previous not-ready throw, so failing is the shape they expect. The file-serve route is unaffected — it resolves bytes directly and keeps the graceful passthrough, where a human downloading the file has a use for it. - Surface that failure through hydration: with throwOnDocNotReady set, the caller cannot use a file with no content, so an unrenderable document now reaches it verbatim instead of degrading to null and reporting a misleading "may exceed size limit or no longer accessible". - Stop a shared render inheriting one caller's cancellation. The coalesced run no longer carries any caller's signal; each caller races its own instead, so an aborting reader gives up promptly while the render finishes for the others and still lands in the cache. * fix(files): move the unrenderable error out of the 'use server' module file-utils.server.ts carries 'use server', whose exports must all be async functions, so exporting an error class from it failed the production build with 67 cascading errors. The class now lives in the plain file-utils.ts beside the other shared file helpers, which also lets the hydration path import it directly instead of through a dynamic import. Also bounds how long a failed render is remembered. The isolated-vm engine cannot tell a bad source from a sandbox outage, so a permanent entry let one transient failure block re-rendering that source for the life of the process. Entries now expire after five minutes: long enough to stop a read loop spending a sandbox run per read, short enough that an outage self-heals without a deploy. * fix(files): finish the render cancellation and failure-surfacing edges - Race the E2B render against the caller's signal too. Only the isolated-vm branch did, so an aborted request on the E2B path waited for the sandbox to finish and could return a success the caller no longer wanted. - Attach a terminal handler to the shared render. Every caller races it against its own signal, so all of them can walk away; a later rejection with no waiters left would otherwise surface as an unhandled rejection. - Stop narrowing what throwOnDocNotReady rethrows. readUserFileContent now runs document compiles and can fail in ways this module has no business enumerating; narrowing produced three consecutive review rounds of "this particular failure is still swallowed". The flag means "do not degrade". - Do not mark an unrendered response immutable. The serve route caches versioned responses for a year, which would pin a one-off render failure to that URL long after a later compile succeeds on the same version. * revert(files): drop the concurrent-render coalescing The coalescing was an optional efficiency win — rendering is content-addressed and idempotent, so duplicate concurrent renders produced the same artifact and cost only extra sandbox time on an artifact miss. It bought that at the price of the most intricate code in the change set, and produced three concurrency findings across two review rounds: a shared render inheriting one caller's cancellation, an E2B/isolated-vm asymmetry in how the signal was raced, and orphaned rejections once every caller could race away. Removing it also restores true cancellation on the isolated-vm path: the caller's signal now reaches runSandboxTask again, so an abort cancels the sandbox work rather than only abandoning the wait for it. * fix(review): simplify generated document attachments * fix(files): mock servable downloads in hydration tests * fix(files): preserve rendered attachment semantics * fix(files): preserve cached artifact size * fix(files): refuse unresolved xlsx source * fix(files): resolve execution artifact workspace
This commit is contained in:
@@ -541,8 +541,9 @@ export async function resolveServableDocBytes(args: {
|
||||
}
|
||||
}
|
||||
|
||||
// Reaches here only for xlsx, which has no isolated-vm fallback.
|
||||
if (!format) return { buffer: rawBuffer, contentType: getContentType(fileName) }
|
||||
// Reaches here only for xlsx, which has no isolated-vm fallback. Returning these
|
||||
// bytes would expose generation source as a spreadsheet.
|
||||
if (!format) throw new DocCompileUserError('Document is still being generated')
|
||||
|
||||
const cacheKey = sha256Hex(`${ext}${source}${workspaceId ?? ''}`)
|
||||
const cached = compiledDocCache.get(cacheKey)
|
||||
|
||||
@@ -158,14 +158,30 @@ describe('resolveServableDocBytes', () => {
|
||||
expect(mockRunSandboxTask).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns raw XLSX source when there is no workspaceId (xlsx has no isolated-vm path)', async () => {
|
||||
const result = await resolveServableDocBytes({
|
||||
rawBuffer: XLSX_SOURCE,
|
||||
fileName: 'sheet.xlsx',
|
||||
workspaceId: undefined,
|
||||
})
|
||||
it('throws instead of returning XLSX source when E2B is disabled', async () => {
|
||||
mockLoadCompiledDoc.mockResolvedValue(null)
|
||||
setEnvFlags({ isDocSandboxEnabled: false })
|
||||
|
||||
await expect(
|
||||
resolveServableDocBytes({
|
||||
rawBuffer: XLSX_SOURCE,
|
||||
fileName: 'sheet.xlsx',
|
||||
workspaceId: WORKSPACE_ID,
|
||||
})
|
||||
).rejects.toBeInstanceOf(DocCompileUserError)
|
||||
|
||||
expect(mockRunSandboxTask).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('throws instead of returning XLSX source when there is no workspaceId', async () => {
|
||||
await expect(
|
||||
resolveServableDocBytes({
|
||||
rawBuffer: XLSX_SOURCE,
|
||||
fileName: 'sheet.xlsx',
|
||||
workspaceId: undefined,
|
||||
})
|
||||
).rejects.toBeInstanceOf(DocCompileUserError)
|
||||
|
||||
expect(result.buffer).toBe(XLSX_SOURCE)
|
||||
expect(mockLoadCompiledDoc).not.toHaveBeenCalled()
|
||||
expect(mockRunSandboxTask).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
/**
|
||||
* @vitest-environment node
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockDownloadServableFileFromStorage, mockVerifyFileAccess } = vi.hoisted(() => ({
|
||||
mockDownloadServableFileFromStorage: vi.fn(),
|
||||
mockVerifyFileAccess: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
|
||||
downloadServableFileFromStorage: mockDownloadServableFileFromStorage,
|
||||
}))
|
||||
|
||||
vi.mock('@/app/api/files/authorization', () => ({
|
||||
verifyFileAccess: mockVerifyFileAccess,
|
||||
}))
|
||||
|
||||
import { readUserFileContent } from '@/lib/execution/payloads/materialization.server'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
|
||||
const PDF_SOURCE = Buffer.from('from reportlab.pdfgen import canvas')
|
||||
const PDF_BYTES = Buffer.from('%PDF-1.4 rendered bytes')
|
||||
|
||||
const generatedPdf: UserFile = {
|
||||
id: 'file-1',
|
||||
name: 'report.pdf',
|
||||
url: '',
|
||||
size: PDF_SOURCE.length,
|
||||
type: 'text/x-python-pdf',
|
||||
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/1700000000000-abc1234-report.pdf',
|
||||
}
|
||||
|
||||
describe('readUserFileContent', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
generatedPdf.size = PDF_SOURCE.length
|
||||
mockVerifyFileAccess.mockResolvedValue(true)
|
||||
mockDownloadServableFileFromStorage.mockResolvedValue({
|
||||
buffer: PDF_BYTES,
|
||||
contentType: 'application/pdf',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns the compiled artifact instead of the stored generation source', async () => {
|
||||
const content = await readUserFileContent(generatedPdf, {
|
||||
userId: 'user-1',
|
||||
encoding: 'base64',
|
||||
})
|
||||
|
||||
expect(mockDownloadServableFileFromStorage).toHaveBeenCalledOnce()
|
||||
expect(content).toBe(PDF_BYTES.toString('base64'))
|
||||
expect(content).not.toBe(PDF_SOURCE.toString('base64'))
|
||||
expect(generatedPdf.size).toBe(PDF_BYTES.length)
|
||||
})
|
||||
})
|
||||
@@ -10,8 +10,12 @@ import {
|
||||
} from '@/lib/execution/payloads/large-value-ref'
|
||||
import { ExecutionResourceLimitError } from '@/lib/execution/resource-errors'
|
||||
import type { StorageContext } from '@/lib/uploads'
|
||||
import { bufferToBase64, inferContextFromKey } from '@/lib/uploads/utils/file-utils'
|
||||
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
|
||||
import {
|
||||
bufferToBase64,
|
||||
inferContextFromKey,
|
||||
isGeneratedDocumentSourceType,
|
||||
} from '@/lib/uploads/utils/file-utils'
|
||||
import { downloadServableFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
|
||||
const logger = createLogger('ExecutionPayloadMaterialization')
|
||||
@@ -267,6 +271,11 @@ export async function assertUserFileContentAccess(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads the bytes a consumer should receive. For generated documents, updates the
|
||||
* file's size to the rendered artifact size so downstream attachment routing does
|
||||
* not make decisions from the smaller generation-source size.
|
||||
*/
|
||||
export async function readUserFileContent(
|
||||
file: unknown,
|
||||
options: ReadUserFileContentOptions
|
||||
@@ -291,9 +300,14 @@ export async function readUserFileContent(
|
||||
const requestId = options.requestId ?? 'unknown'
|
||||
|
||||
try {
|
||||
buffer = await downloadFileFromStorage(file, requestId, log, { maxBytes: maxSourceBytes })
|
||||
buffer = (
|
||||
await downloadServableFileFromStorage(file, requestId, log, { maxBytes: maxSourceBytes })
|
||||
).buffer
|
||||
} catch (error) {
|
||||
if (isPayloadSizeLimitError(error)) {
|
||||
if (isGeneratedDocumentSourceType(file.type) && error.observedBytes !== undefined) {
|
||||
file.size = error.observedBytes
|
||||
}
|
||||
throw new ExecutionResourceLimitError({
|
||||
resource: 'execution_payload_bytes',
|
||||
attemptedBytes: error.observedBytes ?? maxSourceBytes + 1,
|
||||
@@ -306,6 +320,9 @@ export async function readUserFileContent(
|
||||
if (!buffer) {
|
||||
throw new Error(`File content for ${file.name} is unavailable.`)
|
||||
}
|
||||
if (isGeneratedDocumentSourceType(file.type)) {
|
||||
file.size = buffer.length
|
||||
}
|
||||
if (buffer.length > maxSourceBytes) {
|
||||
throw new ExecutionResourceLimitError({
|
||||
resource: 'execution_payload_bytes',
|
||||
|
||||
@@ -3,27 +3,51 @@
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { mockDownloadFile } = vi.hoisted(() => ({
|
||||
mockDownloadFile: vi.fn(),
|
||||
}))
|
||||
const { mockDownloadFile, mockParseWorkspaceFileKey, mockResolveServableDocBytes } = vi.hoisted(
|
||||
() => ({
|
||||
mockDownloadFile: vi.fn(),
|
||||
mockParseWorkspaceFileKey: vi.fn(),
|
||||
mockResolveServableDocBytes: vi.fn(),
|
||||
})
|
||||
)
|
||||
|
||||
vi.mock('@/lib/uploads/core/storage-service', () => ({
|
||||
downloadFile: mockDownloadFile,
|
||||
hasCloudStorage: vi.fn(() => true),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/uploads/contexts/execution/execution-file-manager', () => ({
|
||||
downloadExecutionFile: mockDownloadFile,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/uploads/contexts/workspace/workspace-file-manager', () => ({
|
||||
parseWorkspaceFileKey: mockParseWorkspaceFileKey,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/copilot/tools/server/files/doc-compile', () => ({
|
||||
resolveServableDocBytes: mockResolveServableDocBytes,
|
||||
}))
|
||||
|
||||
vi.mock('@/app/api/files/authorization', () => ({
|
||||
verifyFileAccess: vi.fn(),
|
||||
}))
|
||||
|
||||
import { createLogger } from '@sim/logger'
|
||||
import { downloadFileFromStorage } from '@/lib/uploads/utils/file-utils.server'
|
||||
import {
|
||||
downloadFileFromStorage,
|
||||
downloadServableFileFromStorage,
|
||||
} from '@/lib/uploads/utils/file-utils.server'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
|
||||
describe('downloadFileFromStorage context derivation', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
mockDownloadFile.mockResolvedValue(Buffer.from('bytes'))
|
||||
mockParseWorkspaceFileKey.mockReturnValue(null)
|
||||
mockResolveServableDocBytes.mockImplementation(async ({ rawBuffer }) => ({
|
||||
buffer: rawBuffer,
|
||||
contentType: 'application/pdf',
|
||||
}))
|
||||
})
|
||||
|
||||
it('downloads with the key-derived context, ignoring a caller-supplied public context', async () => {
|
||||
@@ -44,4 +68,23 @@ describe('downloadFileFromStorage context derivation', () => {
|
||||
expect.objectContaining({ key: userFile.key, context: 'workspace' })
|
||||
)
|
||||
})
|
||||
|
||||
it('uses the workspace ID embedded in an execution key to resolve generated artifacts', async () => {
|
||||
const workspaceId = '2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f'
|
||||
const userFile: UserFile = {
|
||||
id: 'f1',
|
||||
name: 'report.pdf',
|
||||
url: '',
|
||||
size: 5,
|
||||
type: 'text/x-python-pdf',
|
||||
key: `execution/${workspaceId}/3f2e9d4c-6a7b-4d8e-9f0a-1b2c3d4e5f6a/4a3b2c1d-7e8f-4a9b-8c0d-1e2f3a4b5c6d/report.pdf`,
|
||||
context: 'execution',
|
||||
}
|
||||
|
||||
await downloadServableFileFromStorage(userFile, 'req-1', createLogger('test'))
|
||||
|
||||
expect(mockResolveServableDocBytes).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ workspaceId })
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -16,6 +16,7 @@ import { StorageService } from '@/lib/uploads'
|
||||
import { isExecutionFile } from '@/lib/uploads/contexts/execution/utils'
|
||||
import {
|
||||
extractStorageKey,
|
||||
extractWorkspaceIdFromExecutionKey,
|
||||
getFileExtension,
|
||||
getMimeTypeFromExtension,
|
||||
inferContextFromKey,
|
||||
@@ -384,7 +385,11 @@ export async function downloadServableFileFromStorage(
|
||||
const { parseWorkspaceFileKey } = await import(
|
||||
'@/lib/uploads/contexts/workspace/workspace-file-manager'
|
||||
)
|
||||
const workspaceId = userFile.key ? (parseWorkspaceFileKey(userFile.key) ?? undefined) : undefined
|
||||
const workspaceId = userFile.key
|
||||
? (parseWorkspaceFileKey(userFile.key) ??
|
||||
extractWorkspaceIdFromExecutionKey(userFile.key) ??
|
||||
undefined)
|
||||
: undefined
|
||||
|
||||
const { resolveServableDocBytes } = await import('@/lib/copilot/tools/server/files/doc-compile')
|
||||
const resolved = await resolveServableDocBytes({
|
||||
|
||||
@@ -9,24 +9,26 @@ import {
|
||||
} from '@/lib/uploads/utils/user-file-base64.server'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
|
||||
const { mockDownloadFile, mockRedis, mockVerifyFileAccess } = vi.hoisted(() => {
|
||||
const mockRedis = {
|
||||
get: vi.fn(),
|
||||
set: vi.fn(),
|
||||
hget: vi.fn(),
|
||||
hset: vi.fn(),
|
||||
hgetall: vi.fn(),
|
||||
expire: vi.fn(),
|
||||
scan: vi.fn(),
|
||||
del: vi.fn(),
|
||||
eval: vi.fn(),
|
||||
}
|
||||
return {
|
||||
mockDownloadFile: vi.fn(),
|
||||
mockRedis,
|
||||
mockVerifyFileAccess: vi.fn(),
|
||||
}
|
||||
})
|
||||
const { mockDownloadFile, mockDownloadServableFileFromStorage, mockRedis, mockVerifyFileAccess } =
|
||||
vi.hoisted(() => {
|
||||
const mockRedis = {
|
||||
get: vi.fn(),
|
||||
set: vi.fn(),
|
||||
hget: vi.fn(),
|
||||
hset: vi.fn(),
|
||||
hgetall: vi.fn(),
|
||||
expire: vi.fn(),
|
||||
scan: vi.fn(),
|
||||
del: vi.fn(),
|
||||
eval: vi.fn(),
|
||||
}
|
||||
return {
|
||||
mockDownloadFile: vi.fn(),
|
||||
mockDownloadServableFileFromStorage: vi.fn(),
|
||||
mockRedis,
|
||||
mockVerifyFileAccess: vi.fn(),
|
||||
}
|
||||
})
|
||||
|
||||
const mockGetRedisClient = redisConfigMockFns.mockGetRedisClient
|
||||
|
||||
@@ -44,6 +46,7 @@ vi.mock('@/lib/uploads/contexts/execution/execution-file-manager', () => ({
|
||||
|
||||
vi.mock('@/lib/uploads/utils/file-utils.server', () => ({
|
||||
downloadFileFromStorage: mockDownloadFile,
|
||||
downloadServableFileFromStorage: mockDownloadServableFileFromStorage,
|
||||
}))
|
||||
|
||||
vi.mock('@/app/api/files/authorization', () => ({
|
||||
@@ -64,6 +67,10 @@ describe('hydrateUserFilesWithBase64', () => {
|
||||
mockRedis.del.mockResolvedValue(1)
|
||||
mockRedis.eval.mockResolvedValue([1, 'ok', 0, 0])
|
||||
mockVerifyFileAccess.mockResolvedValue(true)
|
||||
mockDownloadServableFileFromStorage.mockImplementation(async (file: unknown) => ({
|
||||
buffer: await mockDownloadFile(file),
|
||||
contentType: 'application/octet-stream',
|
||||
}))
|
||||
})
|
||||
|
||||
it('strips existing base64 when it exceeds maxBytes', async () => {
|
||||
@@ -101,6 +108,84 @@ describe('hydrateUserFilesWithBase64', () => {
|
||||
expect(hydrated.file.base64).toBe(base64)
|
||||
})
|
||||
|
||||
it('uses rendered size when generated source metadata exceeds the inline limit', async () => {
|
||||
const rendered = Buffer.from('%PDF')
|
||||
mockDownloadServableFileFromStorage.mockResolvedValueOnce({
|
||||
buffer: rendered,
|
||||
contentType: 'application/pdf',
|
||||
})
|
||||
const file: UserFile = {
|
||||
id: 'file-1',
|
||||
name: 'report.pdf',
|
||||
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf',
|
||||
url: '',
|
||||
size: 11,
|
||||
type: 'text/x-python-pdf',
|
||||
}
|
||||
|
||||
const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' })
|
||||
|
||||
expect(hydrated.file.base64).toBe(rendered.toString('base64'))
|
||||
expect(hydrated.file.size).toBe(rendered.length)
|
||||
})
|
||||
|
||||
it('records rendered size when a generated document must use a provider upload path', async () => {
|
||||
mockDownloadServableFileFromStorage.mockResolvedValueOnce({
|
||||
buffer: Buffer.alloc(11),
|
||||
contentType: 'application/pdf',
|
||||
})
|
||||
const file: UserFile = {
|
||||
id: 'file-1',
|
||||
name: 'report.pdf',
|
||||
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf',
|
||||
url: '',
|
||||
size: 1,
|
||||
type: 'text/x-python-pdf',
|
||||
}
|
||||
|
||||
const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' })
|
||||
|
||||
expect(hydrated.file).not.toHaveProperty('base64')
|
||||
expect(hydrated.file.size).toBe(11)
|
||||
})
|
||||
|
||||
it('records cached rendered size when a generated document must use a provider upload path', async () => {
|
||||
mockGetRedisClient.mockReturnValue(mockRedis)
|
||||
mockRedis.get.mockResolvedValueOnce(Buffer.alloc(11).toString('base64'))
|
||||
const file: UserFile = {
|
||||
id: 'file-1',
|
||||
name: 'report.pdf',
|
||||
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf',
|
||||
url: '',
|
||||
size: 1,
|
||||
type: 'text/x-python-pdf',
|
||||
}
|
||||
|
||||
const hydrated = await hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' })
|
||||
|
||||
expect(hydrated.file).not.toHaveProperty('base64')
|
||||
expect(hydrated.file.size).toBe(11)
|
||||
expect(mockDownloadServableFileFromStorage).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('propagates generated documents that are still compiling', async () => {
|
||||
const notReady = new Error('Document is still being generated')
|
||||
notReady.name = 'DocCompileUserError'
|
||||
mockDownloadServableFileFromStorage.mockRejectedValueOnce(notReady)
|
||||
const file: UserFile = {
|
||||
id: 'file-1',
|
||||
name: 'report.pdf',
|
||||
key: 'workspace/2f1d8c3e-5b6a-4c7d-8e9f-0a1b2c3d4e5f/report.pdf',
|
||||
url: '',
|
||||
size: 1,
|
||||
type: 'text/x-python-pdf',
|
||||
}
|
||||
|
||||
await expect(
|
||||
hydrateUserFilesWithBase64({ file }, { maxBytes: 10, userId: 'user-1' })
|
||||
).rejects.toBe(notReady)
|
||||
})
|
||||
|
||||
it('does not hydrate URL-only internal file objects', async () => {
|
||||
const file: UserFile = {
|
||||
id: 'file-1',
|
||||
|
||||
@@ -27,6 +27,7 @@ import {
|
||||
ExecutionResourceLimitError,
|
||||
isExecutionResourceLimitError,
|
||||
} from '@/lib/execution/resource-errors'
|
||||
import { isGeneratedDocumentSourceType } from '@/lib/uploads/utils/file-utils'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
|
||||
const INLINE_BASE64_JSON_OVERHEAD_BYTES = 512 * 1024
|
||||
@@ -391,7 +392,11 @@ async function resolveBase64(
|
||||
const allowUnknownSize = options.allowUnknownSize ?? false
|
||||
const hasStableStorageKey = Boolean(file.key)
|
||||
|
||||
if (Number.isFinite(file.size) && file.size > maxBytes) {
|
||||
if (
|
||||
!isGeneratedDocumentSourceType(file.type) &&
|
||||
Number.isFinite(file.size) &&
|
||||
file.size > maxBytes
|
||||
) {
|
||||
logger.warn(
|
||||
`[${options.requestId}] Skipping base64 for ${file.name} (size ${file.size} exceeds ${maxBytes})`
|
||||
)
|
||||
@@ -420,9 +425,11 @@ async function resolveBase64(
|
||||
userId: options.userId,
|
||||
encoding: 'base64',
|
||||
maxBytes,
|
||||
maxSourceBytes: maxBytes,
|
||||
})
|
||||
} catch (error) {
|
||||
if (error instanceof Error && error.name === 'DocCompileUserError') {
|
||||
throw error
|
||||
}
|
||||
logger.warn(`[${requestId}] Failed to hydrate base64 for ${file.name}`, error)
|
||||
return null
|
||||
}
|
||||
@@ -456,7 +463,11 @@ async function hydrateUserFile(
|
||||
const cached = await state.cache.get(file)
|
||||
if (cached) {
|
||||
const maxBytes = options.maxBytes ?? DEFAULT_MAX_BASE64_BYTES
|
||||
if (Buffer.byteLength(cached, 'base64') > maxBytes) {
|
||||
const cachedBytes = Buffer.byteLength(cached, 'base64')
|
||||
if (isGeneratedDocumentSourceType(file.type)) {
|
||||
file.size = cachedBytes
|
||||
}
|
||||
if (cachedBytes > maxBytes) {
|
||||
return stripBase64(file)
|
||||
}
|
||||
return { ...file, base64: cached }
|
||||
|
||||
@@ -59,6 +59,15 @@ describe('provider attachments', () => {
|
||||
).toBe('image/png')
|
||||
})
|
||||
|
||||
it('infers MIME type from filename when file type is a generated-doc source marker', () => {
|
||||
expect(
|
||||
inferAttachmentMimeType({
|
||||
...pdfFile,
|
||||
type: 'text/x-python-pdf',
|
||||
})
|
||||
).toBe('application/pdf')
|
||||
})
|
||||
|
||||
it('formats OpenAI Responses content with text, image, and file parts', () => {
|
||||
const content = buildOpenAIMessageContent(
|
||||
'Analyze these files',
|
||||
@@ -301,6 +310,16 @@ describe('provider large-file capability', () => {
|
||||
expect(shouldUseLargeFilePath(large, 'bedrock')).toBe(false)
|
||||
})
|
||||
|
||||
it('does not expose generated source through a remote-url large-file path', () => {
|
||||
const generated = {
|
||||
...pdfFile,
|
||||
size: INLINE_ATTACHMENT_THRESHOLD_BYTES + 1,
|
||||
type: 'text/x-python-pdf',
|
||||
}
|
||||
expect(shouldUseLargeFilePath(generated, 'openai')).toBe(true)
|
||||
expect(shouldUseLargeFilePath(generated, 'anthropic')).toBe(false)
|
||||
})
|
||||
|
||||
it('references uploaded OpenAI files by file_id instead of inlining base64', () => {
|
||||
const content = buildOpenAIMessageContent(
|
||||
'Analyze',
|
||||
|
||||
@@ -6,9 +6,10 @@ import {
|
||||
getContentType,
|
||||
getExtensionFromMimeType,
|
||||
getFileExtension,
|
||||
getMimeTypeFromExtension,
|
||||
isGeneratedDocumentSourceType,
|
||||
MIME_TYPE_MAPPING,
|
||||
MODEL_SUPPORTED_IMAGE_MIME_TYPES,
|
||||
resolveFileType,
|
||||
} from '@/lib/uploads/utils/file-utils'
|
||||
import type { UserFile } from '@/executor/types'
|
||||
import {
|
||||
@@ -88,12 +89,18 @@ export function getProviderFileStrategy(providerId: ProviderId | string): Provid
|
||||
return getProviderFileAttachment(providerId).strategy
|
||||
}
|
||||
|
||||
/** True when a file exceeds the inline threshold and the provider has a large-file path. */
|
||||
/**
|
||||
* True when an oversized file has a safe provider path. Remote URLs point at the
|
||||
* primary storage object, so source-backed documents can only use artifact-aware
|
||||
* Files API uploads.
|
||||
*/
|
||||
export function shouldUseLargeFilePath(
|
||||
file: Pick<UserFile, 'size'>,
|
||||
file: Pick<UserFile, 'size' | 'type'>,
|
||||
providerId: ProviderId | string
|
||||
): boolean {
|
||||
if (getProviderFileAttachment(providerId).strategy === 'inline') return false
|
||||
const strategy = getProviderFileAttachment(providerId).strategy
|
||||
if (strategy === 'inline') return false
|
||||
if (strategy === 'remote-url' && isGeneratedDocumentSourceType(file.type)) return false
|
||||
return Number.isFinite(file.size) && file.size > INLINE_ATTACHMENT_THRESHOLD_BYTES
|
||||
}
|
||||
|
||||
@@ -197,12 +204,10 @@ export function getProviderAttachmentMaxBytes(providerId: ProviderId | string):
|
||||
|
||||
export function inferAttachmentMimeType(file: UserFile): string {
|
||||
const explicitType = file.type?.trim().toLowerCase()
|
||||
if (explicitType && explicitType !== 'application/octet-stream') {
|
||||
return explicitType
|
||||
}
|
||||
|
||||
const inferred = getMimeTypeFromExtension(getFileExtension(file.name))
|
||||
return inferred.toLowerCase()
|
||||
return resolveFileType({
|
||||
name: file.name,
|
||||
type: isGeneratedDocumentSourceType(explicitType) ? '' : (explicitType ?? ''),
|
||||
}).toLowerCase()
|
||||
}
|
||||
|
||||
function isTextDocumentMimeType(mimeType: string): boolean {
|
||||
|
||||
Reference in New Issue
Block a user