Skip to content

Commit 32fcc63

Browse files
authored
fix(files): stop promising immutable caching for documents compiled against other files (#8129)
* fix(files): stop promising immutable caching for documents compiled against other files A versioned serve URL (`?v=<updatedAt>`) was always answered with a one-year `immutable` Cache-Control. That holds for a stored source — a content write rotates the storage key, so a given key's bytes never change — but not for a response the route resolves against OTHER files: a document compiled against the files it references, or a sim page inlining its images, recompiles on every request. Those bytes change when a referenced file changes, while this file's key and `updatedAt` stay put, so the whole URL is unchanged and the browser served a stale render from cache until the document itself was edited. The resolver now reports when it read referenced content, and the route withholds the immutable lifetime for exactly those responses, keeping it for stored sources and self-contained artifacts. Also corrects three comments that claimed generated docs are edited in place under the same storage key. That stopped being true in #5545 (2026-07-13), which made every content write allocate a new key; the caching rule above was reasoned from the stale claim. * improvement(files): make serve cacheability a declared, required property An optional boolean let a branch added to the resolver inherit the cacheable default by saying nothing — the exact failure this change exists to prevent. Cacheability is now a required field every branch must declare, so forgetting it fails the build rather than silently promising a year of immutability. * improvement(files): answer a file revalidation with 304 instead of the whole body A response the browser is told to revalidate carried no validator, so every check re-sent the entire file. That is the cost a document compiled against other files now pays on each window focus: it cannot be given a cache lifetime, because its bytes really may have changed, so the only way to make the check cheap is to let the client prove what it already holds. Authorized serves now carry an ETag — the digest of the bytes about to be sent, which is exact by construction however those bytes were produced — and answer 304 to a matching If-None-Match. Matching is weak, per RFC 9110, so a cache that stored a weak validator still revalidates. Kept out of createFileResponse deliberately: digesting costs a pass over the buffer, up to the 100MB transfer ceiling, and a response served as immutable is never revalidated, so it would pay that pass and never collect. Public assets and the assistant-image path are unchanged. * fix(files): report a reference dependency from the isolated-VM compile path The isolated-VM fallback returns before the static reference scan, so a document it compiled never reported one — yet that path reads workspace files live through its broker, which is what `onWorkspaceFileAccess` records. A versioned request for such a document therefore still took a one-year immutable lifetime, and changing a referenced file left the browser serving a stale render. Fixed at the root rather than in that one branch: the flag is now required on CompiledDocResult, so every compile path must declare it and a new one cannot inherit a cacheable-forever answer by staying silent. Each site reports the union of what the source references statically and what the compile actually touched — neither alone is sufficient, since a failed read records no access and the broker reaches files the static scan cannot see. Making it required immediately surfaced a second case: the process-local compile cache is shared with compiles that carried a workspace, so a cached entry can hold contributor identities even when the reading call passes none. That branch now reads the cached identities instead of assuming independence. * improvement(files): compute a validator only where a response can be revalidated Both reviewers caught the same contradiction: the digest was documented as worth paying only where a 304 can be collected, then applied to every authorized serve including immutable ones, which are never revalidated. One place now decides, so an immutable response takes the plain path and spends no pass over its buffer. Also drops a redundant translation. The resolver was converting the compiler's boolean into a string union and the cache rule was converting it straight back; the producer's own required boolean now travels end to end, which is one fact in one shape and keeps the same build-time guarantee that a new branch must declare it.
1 parent 96b941f commit 32fcc63

8 files changed

Lines changed: 427 additions & 56 deletions

File tree

‎apps/sim/app/api/files/serve/[...path]/route.test.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ const {
3636
mockFindLocalFile,
3737
mockReadLocalFileWithinLimit,
3838
mockCreateFileResponse,
39+
mockCreateConditionalFileResponse,
3940
mockCreateErrorResponse,
4041
FileNotFoundError,
4142
serveLogger,
@@ -64,6 +65,7 @@ const {
6465
mockFindLocalFile: vi.fn(),
6566
mockReadLocalFileWithinLimit: vi.fn(),
6667
mockCreateFileResponse: vi.fn(),
68+
mockCreateConditionalFileResponse: vi.fn(),
6769
mockCreateErrorResponse: vi.fn(),
6870
FileNotFoundError: FileNotFoundErrorClass,
6971
}
@@ -129,6 +131,7 @@ vi.mock('@/lib/copilot/tools/server/files/doc-compile', () => ({
129131
vi.mock('@/app/api/files/utils', () => ({
130132
FileNotFoundError,
131133
createFileResponse: mockCreateFileResponse,
134+
createConditionalFileResponse: mockCreateConditionalFileResponse,
132135
createErrorResponse: mockCreateErrorResponse,
133136
getContentType: mockGetContentType,
134137
extractStorageKey: vi.fn().mockImplementation((path: string) => path.split('/').pop()),
@@ -192,6 +195,11 @@ describe('File Serve API Route', () => {
192195
})
193196
}
194197
)
198+
// Delegates so the existing assertions on the response payload — including its
199+
// Cache-Control — read the same call list whichever helper the route reached for.
200+
mockCreateConditionalFileResponse.mockImplementation((file: unknown) =>
201+
mockCreateFileResponse(file)
202+
)
195203
mockCreateErrorResponse.mockImplementation((error: Error) => {
196204
return new Response(JSON.stringify({ error: error.name, message: error.message }), {
197205
status: error.name === 'FileNotFoundError' ? 404 : 500,
@@ -439,6 +447,70 @@ describe('File Serve API Route', () => {
439447
expect(mockVerifyFileAccess).not.toHaveBeenCalled()
440448
})
441449

450+
describe('versioned cache lifetime', () => {
451+
const principal = {
452+
kind: 'delegated' as const,
453+
serviceId: 'executor' as const,
454+
subjectUserId: 'test-user-id',
455+
workspaceId: 'test-workspace-id',
456+
delegationId: 'delegation-1',
457+
audience: 'sim:workspace-files',
458+
issuedAt: new Date('2026-08-01T00:00:00Z'),
459+
expiresAt: new Date('2026-08-01T01:00:00Z'),
460+
delegationContext: {
461+
kind: 'workflow_execution' as const,
462+
workflowId: 'workflow-1',
463+
},
464+
}
465+
466+
async function serveVersionedDoc(dependsOnReferencedFiles: boolean) {
467+
mockResolveStoredFileContext.mockResolvedValue('workspace')
468+
mockParseWorkspaceFileKey.mockReturnValue('test-workspace-id')
469+
mockAuthenticateWorkspaceFile.mockResolvedValue(principal)
470+
mockResolveServableDocBytes.mockResolvedValue({
471+
buffer: Buffer.from('compiled'),
472+
contentType: 'application/pdf',
473+
...(dependsOnReferencedFiles ? { dependsOnReferencedFiles: true } : {}),
474+
})
475+
476+
const req = new NextRequest(
477+
'http://localhost:3000/api/files/serve/workspace/test-workspace-id/report.pdf?v=1756684800000'
478+
)
479+
await GET(req, {
480+
params: Promise.resolve({ path: ['workspace', 'test-workspace-id', 'report.pdf'] }),
481+
})
482+
return mockCreateFileResponse.mock.calls.at(-1)?.[0]
483+
}
484+
485+
it('caches a versioned document immutably when its bytes derive from the stored source alone', async () => {
486+
expect(await serveVersionedDoc(false)).toEqual(
487+
expect.objectContaining({ cacheControl: 'private, max-age=31536000, immutable' })
488+
)
489+
})
490+
491+
it('spends no validator on an immutable response, which is never revalidated', async () => {
492+
await serveVersionedDoc(false)
493+
expect(mockCreateConditionalFileResponse).not.toHaveBeenCalled()
494+
expect(mockCreateFileResponse).toHaveBeenCalled()
495+
})
496+
497+
it('attaches a validator to a revalidated response so the next check can be answered 304', async () => {
498+
await serveVersionedDoc(true)
499+
expect(mockCreateConditionalFileResponse).toHaveBeenCalled()
500+
})
501+
502+
it('keeps a versioned document revalidated when it was compiled against referenced files', async () => {
503+
/**
504+
* The URL carries the file's own `updatedAt`, which does not move when a
505+
* REFERENCED file changes — so an immutable lifetime would pin the stale
506+
* render in the browser cache until the document itself is edited.
507+
*/
508+
expect(await serveVersionedDoc(true)).toEqual(
509+
expect.objectContaining({ cacheControl: 'private, no-cache, must-revalidate' })
510+
)
511+
})
512+
})
513+
442514
it('serves a mothership chat attachment stored under a workspace key', async () => {
443515
/**
444516
* The attachment shares the `workspace/…` prefix but is recorded as

‎apps/sim/app/api/files/serve/[...path]/route.ts‎

Lines changed: 123 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ import { isSimPageSource, SIM_PAGE_CONTENT_TYPE } from '@/lib/workspace-files/pa
3030
import { renderSimPageDocumentWithAssets } from '@/lib/workspace-files/page-document.server'
3131
import { type KnowledgeFileAccess, verifyFileAccess } from '@/app/api/files/authorization'
3232
import {
33+
createConditionalFileResponse,
3334
createErrorResponse,
3435
createFileResponse,
3536
FileNotFoundError,
37+
type FileResponse,
3638
findLocalFile,
3739
getContentType,
3840
readLocalFileWithinLimit,
@@ -64,8 +66,25 @@ interface ServeOptions {
6466
raw: boolean
6567
/** `preview=1` — the caller renders these bytes rather than saving them. */
6668
preview: boolean
67-
/** `v=<updatedAt>` — the URL addresses content-immutable bytes. */
69+
/** `v=<updatedAt>` — the caller asserts the URL addresses one fixed content revision. */
6870
versioned: boolean
71+
/** The request's `If-None-Match`, so a revalidation can be answered 304 instead of re-sending. */
72+
ifNoneMatch: string | null
73+
}
74+
75+
interface ServableBytes {
76+
buffer: Buffer
77+
contentType: string
78+
/**
79+
* These bytes were resolved against OTHER files' current content — a page inlining its images,
80+
* or a document compiled against the files it references — so the same storage key can serve
81+
* different bytes over time while this file's own key and `updatedAt` stay put.
82+
*
83+
* Required, so a branch added to the resolver cannot inherit the cacheable answer by saying
84+
* nothing — the same reason the transfer ceiling is asserted where the branches converge rather
85+
* than inside each one.
86+
*/
87+
dependsOnReferencedFiles: boolean
6988
}
7089

7190
/**
@@ -95,12 +114,16 @@ async function resolveServableBytes(params: {
95114
/** The stored record's content type, where the caller has the record. */
96115
fileType?: string
97116
signal: AbortSignal | undefined
98-
}): Promise<{ buffer: Buffer; contentType: string }> {
117+
}): Promise<ServableBytes> {
99118
// `raw` is the stored source, already bounded by the read that produced it, but it
100119
// goes through the same check so the ceiling holds for everything this returns
101120
// rather than for every branch someone remembered to cover.
102-
const resolved = params.options.raw
103-
? { buffer: params.buffer, contentType: getContentType(params.filename) }
121+
const resolved: ServableBytes = params.options.raw
122+
? {
123+
buffer: params.buffer,
124+
contentType: getContentType(params.filename),
125+
dependsOnReferencedFiles: false,
126+
}
104127
: await resolveTransformedBytes(params)
105128
assertKnownSizeWithinLimit(
106129
resolved.buffer.length,
@@ -120,7 +143,7 @@ async function resolveTransformedBytes(params: {
120143
filePrincipal?: Principal
121144
fileType?: string
122145
signal: AbortSignal | undefined
123-
}): Promise<{ buffer: Buffer; contentType: string }> {
146+
}): Promise<ServableBytes> {
124147
const {
125148
buffer,
126149
filename,
@@ -146,25 +169,36 @@ async function resolveTransformedBytes(params: {
146169
await renderSimPageDocumentWithAssets(text, { workspaceId }),
147170
'utf8'
148171
)
149-
return { buffer: rendered, contentType: 'text/html' }
172+
// Inlines the workspace images the page references, read at their CURRENT content.
173+
return {
174+
buffer: rendered,
175+
contentType: 'text/html',
176+
dependsOnReferencedFiles: true,
177+
}
150178
}
151179
}
152180

153181
if (options.preview) {
154182
// Images resolve independently of the document path: a HEIF has no compiled-source
155183
// concept, so it never reaches the doc branch.
156184
const image = await resolveServableImageBytes(buffer, storageKey)
157-
if (image) return image
185+
// Transcoded from THIS file's stored bytes, so it lives and dies with the storage key.
186+
if (image) return { ...image, dependsOnReferencedFiles: false }
158187
}
159188

160-
return resolveServableDocBytes({
189+
const doc = await resolveServableDocBytes({
161190
rawBuffer: buffer,
162191
fileName: filename,
163192
workspaceId,
164193
filePrincipal,
165194
ownerKey,
166195
signal,
167196
})
197+
return {
198+
buffer: doc.buffer,
199+
contentType: doc.contentType,
200+
dependsOnReferencedFiles: doc.dependsOnReferencedFiles,
201+
}
168202
}
169203

170204
const STORAGE_KEY_PREFIX_RE = /^\d{13}-[a-z0-9]{7}-/
@@ -184,18 +218,41 @@ const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
184218
const PUBLIC_ASSET_CACHE_CONTROL = 'public, max-age=31536000'
185219

186220
/**
187-
* Cache-Control for a served file. A versioned request (`?v=<updatedAt>`) addresses
188-
* content-immutable bytes — generated docs are content-addressed and the version
189-
* bumps on every edit — so the browser may cache it indefinitely; re-opens and
190-
* focus refetches then resolve from cache with no round trip. Unversioned workspace
191-
* reads stay revalidated because the same storage key is edited in place.
221+
* Cache-Control for a served file.
222+
*
223+
* A versioned request (`?v=<updatedAt>`) normally addresses content-immutable bytes: a workspace
224+
* file's content write stores the new bytes under a NEW storage key, so a given key's stored
225+
* source never changes and the browser may cache it indefinitely — re-opens and focus refetches
226+
* then resolve from cache with no round trip.
227+
*
228+
* That promise does NOT hold when the response was resolved against other files' current content
229+
* (`derived-from-referenced-files`): a document compiled against the files it references, or a page
230+
* inlining its images, recompiles per request, so the same key serves different bytes once a
231+
* referenced file changes — while this file's key and `updatedAt`, and therefore the whole URL,
232+
* stay put. Promising immutability there pins a stale render in the browser cache for a year, so
233+
* those responses stay revalidated whether or not the request carried a version.
192234
*/
193235
function resolveServeCacheControl(
194236
versioned: boolean,
195-
context: string | undefined
237+
context: string | undefined,
238+
dependsOnReferencedFiles: boolean
196239
): string | undefined {
197-
if (versioned) return IMMUTABLE_CACHE_CONTROL
198-
return context === 'workspace' ? WORKSPACE_REVALIDATE_CACHE_CONTROL : undefined
240+
if (versioned && !dependsOnReferencedFiles) return IMMUTABLE_CACHE_CONTROL
241+
return context === 'workspace' || dependsOnReferencedFiles
242+
? WORKSPACE_REVALIDATE_CACHE_CONTROL
243+
: undefined
244+
}
245+
246+
/**
247+
* Sends a resolved file, attaching a validator only where the client may actually revalidate.
248+
*
249+
* An immutable response is never revalidated, so digesting its buffer — a pass over up to the
250+
* whole transfer ceiling — would cost the compute and never collect a single 304.
251+
*/
252+
function serveResolvedFile(file: FileResponse, ifNoneMatch: string | null): NextResponse {
253+
return file.cacheControl === IMMUTABLE_CACHE_CONTROL
254+
? createFileResponse(file)
255+
: createConditionalFileResponse(file, ifNoneMatch)
199256
}
200257

201258
export const GET = withRouteHandler(
@@ -281,6 +338,7 @@ export const GET = withRouteHandler(
281338
raw: query.raw === '1',
282339
preview: query.preview === '1',
283340
versioned: query.v != null,
341+
ifNoneMatch: request.headers.get('if-none-match'),
284342
}
285343

286344
if (workspacePrincipal) {
@@ -381,12 +439,19 @@ async function handleWorkspaceFile(
381439
workspaceId,
382440
size: resolved.buffer.length,
383441
})
384-
return createFileResponse({
385-
buffer: resolved.buffer,
386-
contentType: resolved.contentType,
387-
filename: file.name,
388-
cacheControl: resolveServeCacheControl(options.versioned, 'workspace'),
389-
})
442+
return serveResolvedFile(
443+
{
444+
buffer: resolved.buffer,
445+
contentType: resolved.contentType,
446+
filename: file.name,
447+
cacheControl: resolveServeCacheControl(
448+
options.versioned,
449+
'workspace',
450+
resolved.dependsOnReferencedFiles
451+
),
452+
},
453+
options.ifNoneMatch
454+
)
390455
}
391456

392457
async function handleLocalFile(
@@ -427,7 +492,11 @@ async function handleLocalFile(
427492
const segment = filename.split('/').pop() || filename
428493
const displayName = stripStorageKeyPrefix(segment)
429494
const workspaceId = getWorkspaceIdForCompile(filename)
430-
const { buffer: fileBuffer, contentType } = await resolveServableBytes({
495+
const {
496+
buffer: fileBuffer,
497+
contentType,
498+
dependsOnReferencedFiles,
499+
} = await resolveServableBytes({
431500
buffer: rawBuffer,
432501
filename: displayName,
433502
storageKey: filename,
@@ -439,12 +508,19 @@ async function handleLocalFile(
439508

440509
logger.info('Local file served', { userId, filename, size: fileBuffer.length })
441510

442-
return createFileResponse({
443-
buffer: fileBuffer,
444-
contentType,
445-
filename: displayName,
446-
cacheControl: resolveServeCacheControl(options.versioned, context),
447-
})
511+
return serveResolvedFile(
512+
{
513+
buffer: fileBuffer,
514+
contentType,
515+
filename: displayName,
516+
cacheControl: resolveServeCacheControl(
517+
options.versioned,
518+
context,
519+
dependsOnReferencedFiles
520+
),
521+
},
522+
options.ifNoneMatch
523+
)
448524
} catch (error) {
449525
logServeFailure('Error reading local file:', error)
450526
throw error
@@ -494,7 +570,11 @@ async function handleCloudProxy(
494570
const segment = cloudKey.split('/').pop() || 'download'
495571
const displayName = stripStorageKeyPrefix(segment)
496572
const workspaceId = getWorkspaceIdForCompile(cloudKey)
497-
const { buffer: fileBuffer, contentType } = await resolveServableBytes({
573+
const {
574+
buffer: fileBuffer,
575+
contentType,
576+
dependsOnReferencedFiles,
577+
} = await resolveServableBytes({
498578
buffer: rawBuffer,
499579
filename: displayName,
500580
storageKey: cloudKey,
@@ -511,12 +591,19 @@ async function handleCloudProxy(
511591
context,
512592
})
513593

514-
return createFileResponse({
515-
buffer: fileBuffer,
516-
contentType,
517-
filename: displayName,
518-
cacheControl: resolveServeCacheControl(options.versioned, context),
519-
})
594+
return serveResolvedFile(
595+
{
596+
buffer: fileBuffer,
597+
contentType,
598+
filename: displayName,
599+
cacheControl: resolveServeCacheControl(
600+
options.versioned,
601+
context,
602+
dependsOnReferencedFiles
603+
),
604+
},
605+
options.ifNoneMatch
606+
)
520607
} catch (error) {
521608
logServeFailure('Error downloading from cloud storage:', error)
522609
throw error

0 commit comments

Comments
 (0)