Commit 8b199d7
authored
fix(files): serve rendered documents and stream workspace archives (#5995)
* fix(files): serve rendered documents instead of generator source
Generated docs (docx/pptx/pdf/xlsx) store their generation source as the primary
file; the rendered binary lives in a separate content-addressed artifact store.
resolveServableDocBytes is the chokepoint that swaps one for the other, and the
serve route, single-file download and ~50 tool routes all go through it.
Archive compression and the public v1 download read raw bytes instead, so a
generated document arrived as source text under a .docx name and Word reported
it as corrupt. Both now resolve through the servable reader.
Adds fetchServableWorkspaceFileBuffer beside the raw reader so the record to
UserFile mapping lives in one place, preserving storageContext; the raw reader's
doc comment now says it returns generation source, so the next call site has to
choose deliberately. v1 also has to send the resolved content type rather than
the record's source MIME, and returns a retryable 409 rather than a 500 when an
artifact is still compiling. docNotReadyMessage centralizes the 409 copy, and
isRenderableDocumentName is shared so the read path and its callers agree on
which extensions can expand.
* perf(files): stream workspace archives instead of buffering them
The bulk download route materialized every selected file before writing a byte,
so peak memory tracked the size of the selection. Ordinary files are now
appended as lazy streams: each opens its storage read only when the archiver
reaches it, so one entry is resident at a time rather than the whole archive.
Generated documents still resolve to buffers first. They are the only entries
whose bytes decide anything, and every status this route returns comes from
them — once the first byte is written the status code is committed, so those
decisions have to happen before the archive starts. The per-entry allowance and
byte budget therefore govern documents only.
archiver processes appended entries through a sequential queue; lazystream
defers each storage read until that entry's turn, since handing the archiver an
open stream per entry would hold more connections than the storage client pools.
nodeReadableToWebStream moves out of input-validation.server.ts into a shared
util rather than being written twice: Readable.toWeb throws ERR_INVALID_STATE
when a consumer cancels while the source is still flowing, which is exactly what
a cancelled download does.
Trade: a storage read failing mid-archive truncates the response rather than
returning 500, since the status is already sent. Documents cannot hit this. The
table export route already behaves this way.
* improvement(files): surface archive download errors in place
Bulk and folder downloads navigated to the API route, so any rejection replaced
the Files page with the raw JSON error body. Single-file download already
fetched and saved the blob, so the two paths had diverged.
That was survivable when the only failures were "too many files" and "too
large"; resolving rendered documents adds a 409 for a still-compiling artifact,
which is reachable in normal use. Both archive paths now fetch the zip and show
the server's message as a toast — the route writes that copy for a person, so it
is worth surfacing rather than discarding. Single-file download shows its error
too instead of only logging it.
* improvement(files): simplify the archive route and stop buffering real uploads
Four review passes over the branch converged on the same points.
Routing every office extension through the buffered path held an entire selection
of genuinely uploaded documents in memory — for a check the resolver settles on
the first few magic bytes. Only files stored under a generator-source content
type need resolving; a real .docx serves what is stored and now streams like
anything else, which is what the change was for.
With resolution sequential, the AbortController cancelled nothing (its signal
never reached the storage read), the success-path over-limit branch was
unreachable, and the cancellation guard in the catch could not fire. A plain loop
that returns at the point of failure replaces the outcome record, the sentinel,
the fan-out helper at limit 1, and four post-hoc scans. ZIP_MATERIALIZE_CONCURRENCY
was dead, and the aggregate over-limit message reported a byte count that was by
construction under the limit.
lazystream and its types are gone: Readable.from over an async generator defers
the open the same way and propagates source errors natively, so the PassThrough
relay went with them. The client uses requestRaw with the existing binary
contract instead of a hand-built query string and a re-typed error extractor, and
both archive call sites share one callback. The render headroom moves beside
isRenderableDocumentName, the servable reader stops minting a request id per
file, and the v1 download returns a view rather than a second full copy.
* improvement(files): keep the lazy entry stream in byte mode
Readable.from defaults to object mode; these chunks are bytes headed for an
archive, so the mode is now explicit. Also makes the fire-and-forget bulk
download call consistent with its sibling.
* improvement(files): correct the servable reader's 409 note
Batch callers build the response from docNotReadyMessage rather than through
docNotReadyResponse, so the doc comment now describes the outcome instead of
naming one of the two helpers.
* improvement(files): correct two comments that described the wrong behavior
The resolution loop's comment claimed at most one rendered document is resident.
It is not: every resolved buffer is held until the archive is assembled, bounded
by the request's remaining budget. The loop resolves one at a time, it does not
release one at a time.
RENDERED_DOCUMENT_HEADROOM_BYTES was documented as bounding the expansion beyond
the declared size, but it is passed straight through as maxBytes and is an
absolute ceiling on the rendered document — renamed to MAX_RENDERED_DOCUMENT_BYTES
so the name matches. Download failures also carry a fallback message, so a
transport error surfaces something better than a bare "Failed to fetch".
* fix(files): put streamed files and rendered documents on one byte budget
The budget only counted rendered documents, so streamed entries never reserved
their declared bytes. A mixed selection could clear the up-front declared-size
gate and still ship close to two full limits: ordinary files up to the cap, plus
documents drawing a fresh cap of their own.
Streamed entries ship exactly what they declared, so their share is known before
anything is read and is now reserved up front; documents draw from what is left.
The budget-exhausted rejection also quoted declared sizes, which for a selection
of small sources reads as a tiny total exceeding the limit. That case has no
knowable byte count — the documents have not been rendered — so it now says what
happened without inventing a number.
* fix(files): attach the download anchor and handle a finalize rejection
A detached anchor's click() works in current browsers, but every other download
helper in the app attaches first, and a silent no-op on this path would look
exactly like a download that never started. Not worth the ambiguity for two DOM
operations.
archive.finalize() was fired with void, so a failure after the response had
started became an unhandled rejection on top of the stream error event that
already fails the response. It is caught and logged now.
* fix(files): guard the archive download against concurrent clicks
Navigating to the route meant a second click just re-navigated. Fetching the
archive instead means each click starts another download that holds the whole
zip in tab memory, and the Download button gave no sign anything was happening.
The button now reflects the in-flight download alongside the existing move and
delete states. The ref guard is there as well as the state because two clicks in
the same tick would both pass a state check.
* fix(files): resolve every generated-document source type, not four of five
The archive keyed off a locally enumerated set of source content types that
omitted text/x-pdflibjs — the isolated-vm PDF generator. Those files failed the
check and streamed their generator source under a .pdf name, which is the exact
corruption this change exists to fix.
A canonical five-member set already existed in the file viewer; enumerating a
fourth copy was the mistake, not the missing string. The set moves to file-utils
beside the other file-type predicates, the viewer imports it, and the route asks
isGeneratedDocumentSourceType rather than carrying its own list. A parameterized
test pins all five, so adding a generator without extending the set fails.
* test(files): validate the produced archive with a real zip reader
Every existing test mocks the storage stream and reads the result back with the
same JS library that wrote it, which cannot catch the failure this route exists
to fix: an archive a real zip reader will not open.
This drives 5 MB of non-repeating bytes from an fs stream through archiver, the
lazy entry generator and the Node-to-web bridge, then checks the output with the
operating system's unzip and compares the extracted bytes. Skips rather than
fails where unzip is unavailable.
* fix(files): bound the markdown export's embedded-asset bundling
The embed list is produced by scanning the document body, so its length and the
bytes behind it are whatever the author put there. Every referenced asset was
downloaded at once with no concurrency bound, no per-file cap and no total cap,
so one request could materialize an unbounded number of unbounded files. Its
sibling bulk-download route already had a count cap and a byte cap.
Metadata is now resolved first, which bounds the download from declared sizes
before a byte is read and moves the authorization check off the download path.
Assets are then fetched with bounded concurrency and a per-file cap; a single
unreadable or oversized asset drops out of the bundle rather than failing the
export, which is what the previous allSettled did.
* fix(files): mount rendered documents into the sandbox, not generator source
Mounting a generated document handed the sandbox its generator source under a
.docx name, so a python-docx or openpyxl script failed on a file that looked
fine and the agent debugged code that was correct.
Both branches were wrong, not just the buffered one: with cloud storage the
mount presigns record.key, which is the raw source object, so swapping the
buffered read alone would have fixed only local storage. Source-backed documents
now always take the servable path — they are bounded by the render ceiling, so
routing them through the web process instead of presigning is affordable.
The text/binary decision also keyed off record.type, which for these files is
text/x-docxjs, so a resolved binary would have been decoded as UTF-8. It reads
the resolved content type now.
* chore(deps): regenerate the lockfile against staging's dependency rework
Staging reworked the OTel split, dropped dead deps and declared emcn peers, so
the lockfile is regenerated on top of that rather than carrying a stale merge.
The archive dependency is the only addition; lazystream stays as archiver's
transitive dep now that it is no longer declared directly.
Regenerated incrementally rather than from scratch: a clean resolve fails the
bunfig age gate because minimumReleaseAgeExcludes lists only the darwin and
linux-gnu @next/swc binaries, not the musl and windows variants that a full
re-resolve also pulls.
* fix(files): budget sandbox mounts on rendered bytes, not declared source size
A source-backed document declares the size of its generator, not of the document,
so the per-file and aggregate mount pre-checks were reading a number unrelated to
what was about to be mounted — tiny sources cleared the guard and only afterwards
was the running total updated with the real length.
Those pre-checks now apply only to files that serve what they declare. A document
is capped by the read instead, at the smaller of the per-file limit and the budget
left, and a size rejection is reported in the same terms as the other mount
limits. Same invariant the archive route already had to learn: declared size is
not a bound once bytes can be rendered.
* test(files): cover the two surfaces added without tests
The markdown export route had no test file at all, and the sandbox mount's
source-backed branch was exercised by nothing — both were changed in this PR and
one of them shipped a defect a reviewer caught within minutes.
Export: the embed-count rejection, the declared-bytes rejection landing before
any asset leaves storage, the per-asset download cap, an unreadable asset
dropping out rather than failing the export, and an unauthorized asset never
being read.
Mount: a generated document never presigning its raw key even on cloud storage,
its rendered bytes mounting as base64 rather than being utf-8 decoded off the
source MIME, and the budget rejecting on rendered length.
* fix(files): stop the export caps from rejecting documents that used to work
The caps I picked would have failed exports that previously succeeded: a
screenshot-heavy document can hold more than 100 embeds, and 100 embeds of
unoptimised PNGs can exceed 100 MB. Adding a limit to bound memory is right;
setting it below real usage turns a memory risk into a broken feature.
The byte ceiling now matches the bulk-download route, so the two export surfaces
reject at the same size rather than at two invented ones. The count is only a
guard on the metadata lookups that run before the byte check, so it moves well
above any hand-authored document instead of sitting where a legitimate one
could reach it.1 parent 6aa3e0e commit 8b199d7
20 files changed
Lines changed: 1374 additions & 199 deletions
File tree
- apps/sim
- app
- api
- files/export/[id]
- tools/file/manage
- v1/files/[fileId]
- workspaces/[id]/files/download
- workspace/[workspaceId]/files
- components/file-viewer
- lib
- copilot/tools/handlers
- core
- security
- utils
- uploads
- client
- contexts/workspace
- utils
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
17 | 18 | | |
| 19 | + | |
18 | 20 | | |
19 | 21 | | |
20 | 22 | | |
21 | 23 | | |
22 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
23 | 39 | | |
24 | 40 | | |
25 | 41 | | |
| |||
136 | 152 | | |
137 | 153 | | |
138 | 154 | | |
139 | | - | |
140 | | - | |
141 | | - | |
142 | | - | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
147 | | - | |
148 | | - | |
149 | | - | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
150 | 180 | | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
151 | 214 | | |
152 | 215 | | |
153 | 216 | | |
154 | 217 | | |
155 | 218 | | |
156 | | - | |
157 | | - | |
158 | | - | |
159 | | - | |
160 | | - | |
161 | | - | |
162 | | - | |
163 | | - | |
164 | | - | |
165 | | - | |
166 | | - | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
167 | 222 | | |
168 | 223 | | |
169 | 224 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| 14 | + | |
14 | 15 | | |
15 | 16 | | |
16 | 17 | | |
| |||
685 | 686 | | |
686 | 687 | | |
687 | 688 | | |
688 | | - | |
| 689 | + | |
| 690 | + | |
| 691 | + | |
| 692 | + | |
| 693 | + | |
689 | 694 | | |
690 | 695 | | |
691 | 696 | | |
| |||
864 | 869 | | |
865 | 870 | | |
866 | 871 | | |
| 872 | + | |
| 873 | + | |
| 874 | + | |
| 875 | + | |
| 876 | + | |
867 | 877 | | |
868 | 878 | | |
869 | 879 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
9 | | - | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
10 | 14 | | |
11 | 15 | | |
12 | 16 | | |
| |||
48 | 52 | | |
49 | 53 | | |
50 | 54 | | |
51 | | - | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
52 | 58 | | |
53 | 59 | | |
54 | 60 | | |
| |||
73 | 79 | | |
74 | 80 | | |
75 | 81 | | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
81 | | - | |
82 | | - | |
83 | | - | |
84 | | - | |
85 | | - | |
86 | | - | |
87 | | - | |
88 | | - | |
89 | | - | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
90 | 100 | | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
91 | 106 | | |
92 | 107 | | |
93 | 108 | | |
| |||
0 commit comments