Skip to content

Commit ee582e0

Browse files
committed
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.
1 parent f0b79c5 commit ee582e0

14 files changed

Lines changed: 845 additions & 58 deletions

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -783,6 +783,9 @@ export class AgentBlockHandler implements BlockHandler {
783783
userId: ctx.userId,
784784
logger,
785785
maxBytes: INLINE_ATTACHMENT_THRESHOLD_BYTES,
786+
// These files are about to become provider attachments, so a document that
787+
// is still compiling must fail loudly rather than reach the model empty.
788+
throwOnDocNotReady: true,
786789
})
787790

788791
const missingFile = hydratedFiles.find(

apps/sim/executor/handlers/mothership/mothership-handler.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,56 @@ describe('MothershipBlockHandler', () => {
588588
])
589589
})
590590

591+
it('attaches a generated document under its rendered MIME type, not its generation-source marker', async () => {
592+
const fileContent = Buffer.from('%PDF-1.4 ...', 'utf8').toString('base64')
593+
mockGenerateId.mockReturnValueOnce('chat-uuid')
594+
mockGenerateId.mockReturnValueOnce('message-uuid')
595+
mockGenerateId.mockReturnValueOnce('request-uuid')
596+
mockReadUserFileContent.mockResolvedValueOnce(fileContent)
597+
598+
fetchMock.mockResolvedValue(
599+
new Response(
600+
JSON.stringify({
601+
content: 'analyzed',
602+
model: 'mothership',
603+
conversationId: 'chat-uuid',
604+
tokens: {},
605+
toolCalls: [],
606+
}),
607+
{
608+
status: 200,
609+
headers: { 'Content-Type': 'application/json' },
610+
}
611+
)
612+
)
613+
614+
await handler.execute(context, block, {
615+
prompt: 'Analyze this file',
616+
files: [
617+
{
618+
name: 'report.pdf',
619+
key: 'workspace/workspace-1/report.pdf',
620+
size: 16,
621+
type: 'text/x-python-pdf',
622+
},
623+
],
624+
})
625+
626+
const [, options] = fetchMock.mock.calls[0] as [string, RequestInit]
627+
const body = JSON.parse(String(options.body))
628+
expect(body.fileAttachments).toEqual([
629+
{
630+
type: 'document',
631+
source: {
632+
type: 'base64',
633+
media_type: 'application/pdf',
634+
data: fileContent,
635+
},
636+
filename: 'report.pdf',
637+
},
638+
])
639+
})
640+
591641
it('propagates local aborts to the mothership request', async () => {
592642
const abortController = new AbortController()
593643
context.abortSignal = abortController.signal

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type {
2323
StreamingExecution,
2424
} from '@/executor/types'
2525
import { buildAPIUrl, buildAuthHeaders, extractAPIErrorMessage } from '@/executor/utils/http'
26+
import { inferAttachmentMimeType } from '@/providers/attachments'
2627
import type { SerializedBlock } from '@/serializer/types'
2728

2829
const logger = createLogger('MothershipBlockHandler')
@@ -309,7 +310,7 @@ async function buildMothershipFileAttachments(
309310
maxSourceBytes: MAX_MOTHERSHIP_ATTACHMENT_BYTES,
310311
})
311312

312-
const content = createFileContentFromBase64(base64, userFile.type)
313+
const content = createFileContentFromBase64(base64, inferAttachmentMimeType(userFile))
313314
if (!content) {
314315
throw new Error(`File type is not supported for Mothership attachments: ${userFile.name}`)
315316
}

apps/sim/executor/variables/resolvers/reference-async.server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ async function hydrateExplicitBase64(
8181
allowLargeValueWorkflowScope: context.executionContext.allowLargeValueWorkflowScope,
8282
userId: context.executionContext.userId,
8383
maxBytes: context.executionContext.base64MaxBytes,
84+
// An explicit `<file.base64>` reference has no degraded mode — the resolver throws
85+
// when content is missing. Opt in so a still-compiling document reports that
86+
// instead of the generic size/availability message below.
87+
throwOnDocNotReady: true,
8488
})
8589
if (!hydrated.base64) {
8690
throw new Error(

apps/sim/lib/copilot/tools/server/files/doc-compile.ts

Lines changed: 115 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createLogger } from '@sim/logger'
22
import { sha256Hex } from '@sim/security/hash'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { isDocSandboxEnabled } from '@/lib/core/config/env-flags'
5+
import { HttpError } from '@/lib/core/utils/http-error'
56
import { CodeLanguage } from '@/lib/execution/languages'
67
import {
78
executeInSandbox,
@@ -26,7 +27,18 @@ const logger = createLogger('CopilotDocCompile')
2627
* Errors so callers can return 5xx instead of telling the agent its script was
2728
* wrong.
2829
*/
29-
export class DocCompileUserError extends Error {
30+
export class DocCompileUserError extends HttpError {
31+
/**
32+
* Mirrors the 409 that `docNotReadyResponse` (servable-file-response) already
33+
* returns at the API boundary, so the executor's generic `.statusCode` mapping
34+
* surfaces this as a retryable conflict rather than an opaque 500. Extending
35+
* {@link HttpError} rather than `Error` also lets `withRouteHandler` map it — that
36+
* boundary matches on `instanceof HttpError`, not on a duck-typed `statusCode`, so
37+
* a plain field alone would still leak a 500 from any route that forgets the
38+
* explicit `isDocNotReadyError` check.
39+
*/
40+
readonly statusCode = 409
41+
3042
constructor(message: string) {
3143
super(message)
3244
this.name = 'DocCompileUserError'
@@ -482,6 +494,21 @@ function compiledCacheSet(key: string, buffer: Buffer): void {
482494
compiledDocCache.set(key, buffer)
483495
}
484496

497+
/**
498+
* Sources that failed to render, so a read loop over a file that is not really a
499+
* generated document cannot spend a sandbox run per read. Keyed identically to
500+
* {@link compiledDocCache}, so an edit to the file produces a new key and gets a
501+
* fresh attempt.
502+
*/
503+
const unrenderableSources = new Set<string>()
504+
505+
function markUnrenderable(key: string): void {
506+
if (unrenderableSources.size >= MAX_COMPILED_DOC_CACHE) {
507+
unrenderableSources.delete(unrenderableSources.values().next().value as string)
508+
}
509+
unrenderableSources.add(key)
510+
}
511+
485512
/**
486513
* Resolves the bytes a consumer should actually serve/attach for a stored file —
487514
* the single source of truth shared by the file-serve route and every tool that
@@ -498,63 +525,130 @@ function compiledCacheSet(key: string, buffer: Buffer): void {
498525
* - Bytes already carry the format magic (`%PDF`/ZIP) → real uploaded/binary file,
499526
* serve as-is.
500527
* - Generated-doc source → load the content-addressed compiled artifact.
501-
* - Artifact missing in the E2B regime → the doc is still being generated; throw
502-
* {@link DocCompileUserError} so callers signal "not ready / retry" instead of
503-
* shipping source.
504-
* - E2B disabled → compile the committed JS source via isolated-vm (cached).
528+
* - Artifact missing → render it now and store it, so the next read is a lookup. An
529+
* artifact miss is not evidence that a compile is in flight: the artifact key is
530+
* (workspace, source hash), so forking a workspace, moving a file, or editing the
531+
* source outside a recompiling writer orphans it permanently. Rendering on read
532+
* self-heals all of those.
533+
* - Render fails → serve the stored bytes as `application/octet-stream` and remember
534+
* not to retry them. A file whose bytes are neither the format's binary nor
535+
* renderable source (a `.docx`-named legacy `.doc`, an HTML error page saved as
536+
* `.pdf`) is served as what it is instead of failing forever.
505537
* - Non-doc files → pass through with the extension-derived content type.
538+
* - No workspace context AND no positive evidence of generation source → pass the
539+
* stored bytes through rather than execute them (see `isGeneratedSource`).
506540
*
507-
* It never falls back to attaching the raw source bytes for a generated doc.
541+
* It never hands back generation source under a document content type, and it never
542+
* leaves a caller in a state that only a retry-forever could clear.
508543
*/
509544
export async function resolveServableDocBytes(args: {
510545
rawBuffer: Buffer
511546
fileName: string
512547
workspaceId: string | undefined
548+
/**
549+
* `true` when the caller has positive evidence the stored bytes are generation
550+
* source (the file's MIME marker). Anything else — `false` or `undefined` — means
551+
* "no such evidence".
552+
*
553+
* This flag may only ever WITHHOLD work, never authorize serving source. It is
554+
* read in exactly one place: the no-workspace-context branch, where no artifact
555+
* lookup is possible and the only remaining action would be executing the stored
556+
* bytes as a program. Declining to compile there is safe — the bytes pass through
557+
* untouched.
558+
*
559+
* It deliberately does NOT gate the workspace branches. The value derives from
560+
* `UserFile.type`, which travels through workflow state and can be rewritten by a
561+
* caller that never knew about the internal marker; letting a negative value
562+
* short-circuit an artifact lookup or a compile would serve generation source
563+
* under a binary content type — the corruption this whole module exists to
564+
* prevent.
565+
*/
566+
isGeneratedSource?: boolean
513567
ownerKey?: string
514568
signal?: AbortSignal
515569
}): Promise<{ buffer: Buffer; contentType: string }> {
516-
const { rawBuffer, fileName, workspaceId, ownerKey, signal } = args
570+
const { rawBuffer, fileName, workspaceId, isGeneratedSource, ownerKey, signal } = args
517571
const ext = fileName.slice(fileName.lastIndexOf('.')).toLowerCase()
518572
const extNoDot = ext.replace(/^\./, '')
519573
const format = COMPILABLE_FORMATS[ext]
574+
const passthrough = () => ({ buffer: rawBuffer, contentType: getContentType(fileName) })
520575

521576
// xlsx isn't in COMPILABLE_FORMATS (no isolated-vm path), so match its ZIP magic
522577
// explicitly alongside the table-driven formats.
523578
const magic = format?.magic ?? (extNoDot === 'xlsx' ? ZIP_MAGIC : undefined)
524579
if (magic && bufferStartsWith(rawBuffer, magic)) {
525-
return { buffer: rawBuffer, contentType: getContentType(fileName) }
580+
return passthrough()
526581
}
527582

528583
if (!format && extNoDot !== 'xlsx') {
529-
return { buffer: rawBuffer, contentType: getContentType(fileName) }
584+
return passthrough()
530585
}
531586

532587
const source = rawBuffer.toString('utf-8')
588+
const renderKey = sha256Hex(`${ext}${source}${workspaceId ?? ''}`)
589+
590+
/**
591+
* Last resort when the bytes cannot be rendered. Deliberately does NOT claim the
592+
* extension's content type: at this point the magic check has already failed and
593+
* rendering has failed too, so labelling source text `application/pdf` would hand
594+
* back a document nothing can open — the corruption this module exists to prevent.
595+
*/
596+
const unrendered = (reason: string) => {
597+
logger.warn('Serving stored bytes unrendered', { fileName, workspaceId, reason })
598+
return { buffer: rawBuffer, contentType: 'application/octet-stream' }
599+
}
533600

534601
if (workspaceId) {
535602
const stored = await loadCompiledDocByExt(workspaceId, source, extNoDot)
536603
if (stored) {
537604
return { buffer: stored.buffer, contentType: stored.contentType }
538605
}
539-
if (isDocSandboxEnabled && (await getE2BDocFormat(fileName))) {
540-
throw new DocCompileUserError('Document is still being generated')
606+
} else if (!isGeneratedSource) {
607+
// No workspace id (e.g. an execution-scratch key), so no artifact lookup is
608+
// possible and the only branch left would hand these bytes to the sandbox as a
609+
// program. Require positive evidence first. These bytes are whatever was stored,
610+
// so the extension-derived type is still the best available answer.
611+
return passthrough()
612+
}
613+
614+
if (unrenderableSources.has(renderKey)) {
615+
return unrendered('previous render attempt for these bytes failed')
616+
}
617+
618+
if (workspaceId && isDocSandboxEnabled && (await getE2BDocFormat(fileName))) {
619+
// Render on read. The artifact store is keyed by (workspace, source hash), so a
620+
// miss is NOT proof that a compile is still in flight — forking a workspace,
621+
// moving a file, or editing the source outside a recompiling writer all orphan
622+
// the artifact permanently. Rendering here self-heals every one of those and
623+
// stores the result, so the next read is a lookup. Compiling is idempotent
624+
// (content-addressed), so racing a still-running write-time compile is wasteful
625+
// but correct.
626+
try {
627+
return await compileDoc({ source, fileName, workspaceId })
628+
} catch (error) {
629+
markUnrenderable(renderKey)
630+
return unrendered(getErrorMessage(error, 'sandbox render failed'))
541631
}
542632
}
543633

544634
// Reaches here only for xlsx, which has no isolated-vm fallback.
545-
if (!format) return { buffer: rawBuffer, contentType: getContentType(fileName) }
635+
if (!format) return passthrough()
546636

547-
const cacheKey = sha256Hex(`${ext}${source}${workspaceId ?? ''}`)
548-
const cached = compiledDocCache.get(cacheKey)
637+
const cached = compiledDocCache.get(renderKey)
549638
if (cached) {
550639
return { buffer: cached, contentType: format.contentType }
551640
}
552641

553-
const compiled = await runSandboxTask(
554-
format.taskId,
555-
{ code: source, workspaceId: workspaceId || '' },
556-
{ ownerKey, signal }
557-
)
558-
compiledCacheSet(cacheKey, compiled)
559-
return { buffer: compiled, contentType: format.contentType }
642+
try {
643+
const compiled = await runSandboxTask(
644+
format.taskId,
645+
{ code: source, workspaceId: workspaceId || '' },
646+
{ ownerKey, signal }
647+
)
648+
compiledCacheSet(renderKey, compiled)
649+
return { buffer: compiled, contentType: format.contentType }
650+
} catch (error) {
651+
markUnrenderable(renderKey)
652+
return unrendered(getErrorMessage(error, 'sandbox render failed'))
653+
}
560654
}

0 commit comments

Comments
 (0)