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.

Author: j15z

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

@@ -566,7 +566,7 @@ export async function resolveServableDocBytes(args: {
   isGeneratedSource?: boolean
   ownerKey?: string
   signal?: AbortSignal
-}): Promise<{ buffer: Buffer; contentType: string }> {
+}): Promise<{ buffer: Buffer; contentType: string; unrendered?: boolean }> {
   const { rawBuffer, fileName, workspaceId, isGeneratedSource, ownerKey, signal } = args
   const ext = fileName.slice(fileName.lastIndexOf('.')).toLowerCase()
   const extNoDot = ext.replace(/^\./, '')
@@ -595,7 +595,7 @@ export async function resolveServableDocBytes(args: {
    */
   const unrendered = (reason: string) => {
     logger.warn('Serving stored bytes unrendered', { fileName, workspaceId, reason })
-    return { buffer: rawBuffer, contentType: 'application/octet-stream' }
+    return { buffer: rawBuffer, contentType: 'application/octet-stream', unrendered: true }
   }
 
   if (workspaceId) {
@@ -626,8 +626,14 @@ export async function resolveServableDocBytes(args: {
     try {
       return await compileDoc({ source, fileName, workspaceId })
     } catch (error) {
+      // Only a script error is deterministic — the same bytes will never render, so
+      // remembering that is safe. Infra failures (sandbox create/timeout, S3, an
+      // aborted request) are transient: memoizing them would strand a perfectly good
+      // document for the life of the process, and swallowing them would report an
+      // outage as an unrenderable file. Let those propagate.
+      if (!(error instanceof DocCompileUserError)) throw error
       markUnrenderable(renderKey)
-      return unrendered(getErrorMessage(error, 'sandbox render failed'))
+      return unrendered(getErrorMessage(error, 'document source failed to render'))
     }
   }
 
@@ -648,7 +654,12 @@ export async function resolveServableDocBytes(args: {
     compiledCacheSet(renderKey, compiled)
     return { buffer: compiled, contentType: format.contentType }
   } catch (error) {
+    // Unlike the E2B engine, the isolated-vm task does not distinguish a script
+    // error from an infra one, so the only signal available here is cancellation —
+    // an aborted run says nothing about the source and must stay retryable rather
+    // than stranding a renderable document for the life of the process.
+    if (signal?.aborted) throw error
     markUnrenderable(renderKey)
-    return unrendered(getErrorMessage(error, 'sandbox render failed'))
+    return unrendered(getErrorMessage(error, 'document source failed to render'))
   }
 }

apps/sim/lib/execution/payloads/materialization.server.test.ts

@@ -114,6 +114,22 @@ describe('readUserFileContent generated-document resolution', () => {
     expect(isExecutionResourceLimitError(error)).toBe(false)
   })
 
+  it('refuses bytes the resolver could not render rather than letting them be relabelled', async () => {
+    // readUserFileContent returns only a string, so the resolver's honest
+    // application/octet-stream cannot travel with it — an attachment builder
+    // downstream would infer application/pdf from the name and ship source bytes
+    // as a document. Refusing is the only way to keep that from happening.
+    mockResolveServableDocBytes.mockResolvedValue({
+      buffer: Buffer.from('<html>not a pdf</html>'),
+      contentType: 'application/octet-stream',
+      unrendered: true,
+    })
+
+    await expect(
+      readUserFileContent(generatedDoc(), { userId: 'user-1', encoding: 'base64' })
+    ).rejects.toThrow(/could not be rendered/)
+  })
+
   it('passes a plain file through without consulting the document resolver', async () => {
     const plainText = Buffer.from('just notes', 'utf8')
     mockDownloadFile.mockResolvedValue(plainText)

apps/sim/lib/execution/payloads/materialization.server.ts

@@ -315,6 +315,19 @@ export async function readUserFileContent(
   if (!servable) {
     throw new Error(`File content for ${file.name} is unavailable.`)
   }
+  if (servable.unrendered) {
+    // The resolver could not produce the document these bytes claim to be and is
+    // serving them under a generic type. Every consumer here feeds the result to
+    // something that expects the real document — a provider attachment, a document
+    // parser, a function-block read — and this function returns only a string, so
+    // the honest content type cannot travel with it. Relabelling the source as a
+    // PDF downstream is the corruption this module exists to prevent, so refuse.
+    // (The file-serve route keeps the graceful passthrough: a human downloading
+    // the bytes and seeing what they actually are is useful.)
+    throw new Error(
+      `File ${file.name} could not be rendered; its stored bytes are not the format its name claims.`
+    )
+  }
   const { buffer } = servable
   if (buffer.length > maxSourceBytes) {
     throw new ExecutionResourceLimitError({

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

@@ -347,6 +347,13 @@ export async function downloadFileFromStorage(
 export interface ServableFile {
   buffer: Buffer
   contentType: string
+  /**
+   * Set when the bytes could not be rendered and are being served as-is under a
+   * generic content type. The bytes are NOT the document the filename claims, so a
+   * consumer that hands them to something expecting that format (a provider
+   * attachment, a document parser) must refuse rather than relabel them.
+   */
+  unrendered?: boolean
 }
 
 /**
@@ -384,8 +391,13 @@ export async function downloadServableFileFromStorage(
   // check on the actual stored bytes decides — real binaries pass through there
   // unchanged. (The files/download route gates type-first instead; its type is the
   // server-written DB record, which workflow state never is.)
-  const needsRendering =
-    isGeneratedDocumentSourceType(userFile.type) || isRenderableDocumentName(userFile.name)
+  // Normalize once: a stored type can arrive padded or upper-cased, and deciding the
+  // gate on the raw value while deciding `isGeneratedSource` on the normalized one
+  // would let a padded marker skip the resolver entirely when the name is not a
+  // renderable document.
+  const declaredType = userFile.type?.trim().toLowerCase()
+  const isGeneratedSource = isGeneratedDocumentSourceType(declaredType)
+  const needsRendering = isGeneratedSource || isRenderableDocumentName(userFile.name)
   if (!needsRendering) {
     const ext = getFileExtension(userFile.name)
     return { buffer, contentType: userFile.type || getMimeTypeFromExtension(ext) }
@@ -402,10 +414,8 @@ export async function downloadServableFileFromStorage(
     fileName: userFile.name,
     workspaceId,
     // Only the positive case carries meaning: the resolver reads this solely to decide
-    // whether compiling is warranted where no artifact lookup is possible. Normalize
-    // first — `inferAttachmentMimeType` lowercases before the same Set lookup, and a
-    // stored type can arrive padded or upper-cased.
-    isGeneratedSource: isGeneratedDocumentSourceType(userFile.type?.trim().toLowerCase()),
+    // whether compiling is warranted where no artifact lookup is possible.
+    isGeneratedSource,
     ownerKey: options.ownerKey,
     signal: options.signal,
   })

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

@@ -27,7 +27,6 @@ import {
   ExecutionResourceLimitError,
   isExecutionResourceLimitError,
 } from '@/lib/execution/resource-errors'
-import { isDocNotReadyError } from '@/lib/uploads/utils/servable-file-response'
 import type { UserFile } from '@/executor/types'
 
 const INLINE_BASE64_JSON_OVERHEAD_BYTES = 512 * 1024
@@ -443,8 +442,15 @@ async function resolveBase64(
     // actionable "still being generated" message. Callers that only decorate an
     // already-finished result opt out, so a late compile cannot retroactively
     // fail completed work.
-    if (options.throwOnDocNotReady && isDocNotReadyError(error)) {
-      throw error
+    if (options.throwOnDocNotReady) {
+      // Imported lazily: `servable-file-response` pulls in the doc-compile module
+      // graph (remote sandbox, sandbox task runner, execution limits), and a static
+      // import here would load all of it for every hydration consumer — mirroring
+      // the deliberate dynamic import in file-utils.server.ts.
+      const { isDocNotReadyError } = await import('@/lib/uploads/utils/servable-file-response')
+      if (isDocNotReadyError(error)) {
+        throw error
+      }
     }
     logger.warn(`[${requestId}] Failed to hydrate base64 for ${file.name}`, error)
     return null