Skip to content

fix(files): serve rendered documents instead of source code - #6139

Open
j15z wants to merge 13 commits into
stagingfrom
agent-block-file-issue
Open

fix(files): serve rendered documents instead of source code#6139
j15z wants to merge 13 commits into
stagingfrom
agent-block-file-issue

Conversation

@j15z

@j15z j15z commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Route readUserFileContent through the existing servable-file resolver so Agent and workflow file reads receive a generated document's compiled artifact instead of its editable source
  • Derive artifact workspace IDs from both workspace and execution storage keys so workflow-scoped generated files resolve the same compiled artifact
  • Treat internal generation-source MIME markers such as text/x-python-pdf as storage metadata and infer the provider-facing MIME from the filename
  • Use the rendered artifact's byte count for inline-vs-provider-upload routing, and never send editable source through a remote-URL attachment path
  • Preserve the retryable "still generating" error when an artifact is not ready and refuse the remaining XLSX raw-source fallback
  • Keep the existing write-time compilation and artifact lifecycle unchanged; this PR does not add render-on-read or new caches

Pairs with simstudioai/mothership#385, which propagates DocCompiler to the file subagent's provider request so load_skill can expose Python document skills. The two changes are independent to deploy.

Type of Change

  • Bug fix

Testing

  • 49 focused tests passing across generated-document resolution, storage-key derivation, materialization, base64 hydration, and provider attachments
  • Full Sim TypeScript check passing
  • Lint and the full audit suite passing (check:boundaries, check:api-validation:strict, check:utils, check:zustand-v5, check:react-query, check:client-boundary, check:bare-icons, check:icon-paths, check:realtime-prune, skills:check, agent-stream-docs:check)
  • No migrations in this change

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

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.
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Aug 1, 2026 3:40am

Request Review

@cursor

cursor Bot commented Jul 31, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes attachment routing, execution payload materialization, and document resolution paths where wrong bytes or sizes could leak source or mis-route large files; behavior is covered by focused tests but touches security-sensitive file serving.

Overview
Generated documents (PDF, DOCX, etc.) are stored as editable generation source, not the final binary. This PR routes workflow reads, base64 hydration, and provider attachments through the same servable-file resolver the file-serve route already uses, so consumers get the compiled artifact instead of source text.

readUserFileContent now uses downloadServableFileFromStorage and, for generated-doc MIME markers like text/x-python-pdf, updates file.size to the rendered byte length so inline vs provider-upload routing does not rely on the smaller source metadata. Workspace IDs for artifact lookup are derived from execution storage keys as well as workspace keys.

Provider attachment logic treats generation-source types as storage metadata (MIME inferred from filename), blocks remote-URL large-file paths for those files (URLs would point at source in primary storage), and keeps Files API upload as the safe oversized path where supported.

XLSX no longer falls back to returning raw source when compilation is unavailable; it throws the same retryable DocCompileUserError as other formats still generating.

Reviewed by Cursor Bugbot for commit cb21c93. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread apps/sim/lib/execution/payloads/materialization.server.ts Outdated
Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/uploads/utils/file-utils.server.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Routes generated-document reads and provider attachments through rendered artifacts instead of editable source.

  • Derives artifact workspace IDs from workspace and execution storage keys.
  • Uses rendered byte sizes for inline-versus-upload routing and infers provider MIME types from filenames when internal source markers are present.
  • Rejects unavailable XLSX artifacts rather than returning generation source.
  • Adds focused coverage for artifact resolution, materialization, base64 hydration, MIME inference, and provider routing.

Confidence Score: 5/5

The pull request appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
apps/sim/lib/copilot/tools/server/files/doc-compile.ts Removes the remaining XLSX generation-source fallback and preserves artifact-first resolution with a retryable not-ready error.
apps/sim/lib/uploads/utils/file-utils.server.ts Extends artifact workspace derivation to execution storage keys before resolving generated documents.
apps/sim/lib/execution/payloads/materialization.server.ts Routes workflow reads through the servable-file resolver and records rendered artifact sizes.
apps/sim/lib/uploads/utils/user-file-base64.server.ts Hydrates generated documents from rendered bytes, updates cached rendered sizes, and propagates compilation readiness errors.
apps/sim/providers/attachments.ts Prevents generated source from using remote-URL attachment paths and derives provider-facing MIME types from filenames.

Sequence Diagram

sequenceDiagram
  participant Consumer as Workflow/Attachment Consumer
  participant Reader as File Reader
  participant Storage as Object Storage
  participant Resolver as Servable Document Resolver
  participant Artifact as Compiled Artifact Store
  Consumer->>Reader: Read generated document
  Reader->>Storage: Download stored source
  Reader->>Resolver: Resolve servable bytes
  Resolver->>Artifact: Load compiled artifact using workspace ID
  alt Artifact is ready
    Artifact-->>Resolver: Rendered document bytes
    Resolver-->>Reader: Rendered bytes and provider MIME
    Reader-->>Consumer: Content with rendered byte size
  else Artifact is unavailable
    Artifact-->>Resolver: No artifact
    Resolver-->>Consumer: Document still being generated
  end
Loading

Reviews (11): Last reviewed commit: "fix(files): resolve execution artifact w..." | Re-trigger Greptile

Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/execution/payloads/materialization.server.ts Outdated
Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
…ed 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.
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.
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/uploads/utils/user-file-base64.server.ts
Comment thread apps/sim/lib/uploads/utils/file-utils.server.ts Outdated
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.
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/utils/user-file-base64.server.ts
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.
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/executor/handlers/agent/agent-handler.ts Outdated
Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
- 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.
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/execution/payloads/materialization.server.ts Outdated
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/execution/payloads/materialization.server.ts
Comment thread apps/sim/lib/execution/payloads/materialization.server.ts
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/uploads/utils/user-file-base64.server.ts
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/copilot/tools/server/files/doc-compile.ts Outdated
Comment thread apps/sim/lib/uploads/utils/user-file-base64.server.ts
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/execution/payloads/materialization.server.ts
@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@j15z

j15z commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator Author

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit cb21c93. Configure here.

@j15z j15z changed the title fix(files): serve rendered documents to attachments and workflow reads fix(files): serve rendered documents instead of source code Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant