Skip to content

Commit 041ffe6

Browse files
committed
fix(files): bound the shared artifact reads at the widest consumer ceiling
Review round 3. The previous round bounded the compiled-artifact and cached-derivative reads at the rendered-document ceiling, which is half what the serving routes will return. That made the bound a policy rather than a backstop: an artifact between the two figures was refused even though the route accepts a response that size, and a derivative in that band read as a cache miss on every preview and re-transcoded the original each time. Both funnels now bound at the widest ceiling any of their consumers allows. A consumer that permits less still enforces its own limit on what it got back — the workspace download path continues to hold artifacts to the rendered-document ceiling.
1 parent 34f7aa0 commit 041ffe6

3 files changed

Lines changed: 19 additions & 8 deletions

File tree

‎apps/sim/lib/copilot/tools/server/files/doc-compiled-store.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import {
2020
storeCompiledDoc,
2121
} from '@/lib/copilot/tools/server/files/doc-compiled-store'
2222
import { PayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
23-
import { MAX_RENDERED_DOCUMENT_BYTES } from '@/lib/uploads/utils/file-utils'
23+
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
2424

2525
describe('compiled document publication', () => {
2626
beforeEach(() => {
@@ -81,7 +81,7 @@ describe('compiled document publication', () => {
8181
// The artifact is fetched separately from the source that names it, so a source
8282
// that cleared its own ceiling says nothing about how large this is.
8383
expect(mockDownloadFile.mock.calls[1]?.[0]).toEqual(
84-
expect.objectContaining({ maxBytes: MAX_RENDERED_DOCUMENT_BYTES })
84+
expect.objectContaining({ maxBytes: MAX_BUFFERED_TRANSFER_BYTES })
8585
)
8686
})
8787

@@ -95,8 +95,8 @@ describe('compiled document publication', () => {
9595
mockDownloadFile.mockRejectedValueOnce(
9696
new PayloadSizeLimitError({
9797
label: 'storage download',
98-
maxBytes: MAX_RENDERED_DOCUMENT_BYTES,
99-
observedBytes: MAX_RENDERED_DOCUMENT_BYTES + 1,
98+
maxBytes: MAX_BUFFERED_TRANSFER_BYTES,
99+
observedBytes: MAX_BUFFERED_TRANSFER_BYTES + 1,
100100
})
101101
)
102102

‎apps/sim/lib/copilot/tools/server/files/doc-compiled-store.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage, toError } from '@sim/utils/errors'
44
import { isPayloadSizeLimitError } from '@/lib/core/utils/stream-limits'
55
import { downloadFile, headObject, uploadFile } from '@/lib/uploads/core/storage-service'
6-
import { MAX_RENDERED_DOCUMENT_BYTES } from '@/lib/uploads/utils/file-utils'
6+
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
77

88
const logger = createLogger('CopilotDocCompiledStore')
99

@@ -75,6 +75,12 @@ async function loadPublishedArtifactPointer(key: string): Promise<PublishedArtif
7575
* about the size of this. Bounding it here rather than on the finished response is
7676
* what keeps an oversized artifact from being materialized before it is refused.
7777
*
78+
* The bound is the WIDEST ceiling any consumer of this funnel allows, because it is a
79+
* memory backstop and not a policy: a consumer that permits less enforces its own
80+
* limit on what it got back (the workspace download path holds artifacts to
81+
* `MAX_RENDERED_DOCUMENT_BYTES`, half of this). Using the tighter figure here instead
82+
* would reject artifacts the serving routes are willing to return.
83+
*
7884
* A size breach is rethrown rather than folded into `null`: null means "not built
7985
* yet", which callers answer with "still being prepared, try again", and an artifact
8086
* that is too large would retry forever behind that.
@@ -87,7 +93,7 @@ export async function loadCompiledDoc(
8793
): Promise<Buffer | null> {
8894
const key = compiledArtifactKey(workspaceId, source, ext, referencedInputIdentity)
8995
try {
90-
return await downloadFile({ key, context: 'copilot', maxBytes: MAX_RENDERED_DOCUMENT_BYTES })
96+
return await downloadFile({ key, context: 'copilot', maxBytes: MAX_BUFFERED_TRANSFER_BYTES })
9197
} catch (error) {
9298
if (isPayloadSizeLimitError(error)) throw error
9399
return null

‎apps/sim/lib/uploads/server/image-derivative.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { createLogger } from '@sim/logger'
33
import { getErrorMessage } from '@sim/utils/errors'
44
import { downloadFile, uploadFile } from '@/lib/uploads/core/storage-service'
55
import { isHevcHeifContainer, transcodeHeicToJpeg } from '@/lib/uploads/server/heic'
6-
import { MAX_RENDERED_DOCUMENT_BYTES } from '@/lib/uploads/utils/file-utils'
6+
import { MAX_BUFFERED_TRANSFER_BYTES } from '@/lib/uploads/shared/types'
77

88
const logger = createLogger('ImageDerivative')
99

@@ -21,13 +21,18 @@ function derivativeKey(storageKey: string): string {
2121
* A cache miss here is recoverable, so unlike the compiled-doc store this swallows a
2222
* size breach too: falling through re-transcodes the original, and the transcode is
2323
* bounded by the source read that produced its input.
24+
*
25+
* The ceiling matches what the serving routes will return for the same bytes. A
26+
* tighter one would not reject anything — it would turn every read of a derivative
27+
* in that band into a miss, re-transcoding the original on each preview, which costs
28+
* more than serving the cached copy would have.
2429
*/
2530
async function loadDerivative(storageKey: string): Promise<Buffer | null> {
2631
try {
2732
return await downloadFile({
2833
key: derivativeKey(storageKey),
2934
context: 'copilot',
30-
maxBytes: MAX_RENDERED_DOCUMENT_BYTES,
35+
maxBytes: MAX_BUFFERED_TRANSFER_BYTES,
3136
})
3237
} catch {
3338
return null

0 commit comments

Comments
 (0)