Skip to content

Commit 9f5b711

Browse files
committed
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.
1 parent 6ff2969 commit 9f5b711

3 files changed

Lines changed: 102 additions & 17 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ describe('collectReferencedFileIds', () => {
169169
).resolves.toEqual({
170170
buffer: Buffer.from('%PDF-built'),
171171
contentType: 'application/pdf',
172+
dependsOnReferencedFiles: true,
172173
contributingFiles: [
173174
{
174175
fileId: ID,
@@ -219,6 +220,7 @@ describe('collectReferencedFileIds', () => {
219220
).resolves.toEqual({
220221
buffer: Buffer.from('%PDF-cached'),
221222
contentType: 'application/pdf',
223+
dependsOnReferencedFiles: true,
222224
contributingFiles: [
223225
{
224226
fileId: ID,

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

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,24 @@ export interface CompiledDocResult {
152152
* function of this file's stored source alone: the same storage key compiles to different bytes
153153
* once a referenced file changes, with nothing about this file changing. A caller that assigns
154154
* the response a cache lifetime must not promise immutability for it.
155+
*
156+
* Required, so a compile path added or changed later cannot omit it and silently inherit a
157+
* cacheable-forever answer — which is exactly what the isolated-VM fallback did while this was
158+
* optional, despite reading workspace files live through its broker.
155159
*/
156-
dependsOnReferencedFiles?: boolean
160+
dependsOnReferencedFiles: boolean
161+
}
162+
163+
/**
164+
* Whether a compile result must be treated as resolved against other files.
165+
*
166+
* The union of what the source ASKS for and what the compile actually TOUCHED. Neither alone is
167+
* sufficient: a statically detectable reference whose read failed records no access, and the
168+
* isolated-VM broker reaches files the static scan cannot see. Either one means these bytes can
169+
* change while this file's own stored source does not.
170+
*/
171+
function touchesReferencedFiles(source: string, accessedCount: number): boolean {
172+
return accessedCount > 0 || collectReferencedFileIds(source).size > 0
157173
}
158174

159175
function referencedImageIdentities(
@@ -574,6 +590,7 @@ async function buildCompiledDoc(
574590
return {
575591
buffer,
576592
contentType: fmt.contentType,
593+
dependsOnReferencedFiles: touchesReferencedFiles(args.source, contributingFiles.length),
577594
...(contributingFiles.length > 0 ? { contributingFiles } : {}),
578595
}
579596
}
@@ -664,6 +681,10 @@ async function compileDocInLegacySandbox(
664681
return {
665682
buffer: cached.buffer,
666683
contentType: fmt.contentType,
684+
dependsOnReferencedFiles: touchesReferencedFiles(
685+
args.source,
686+
cached.contributingFiles?.length ?? 0
687+
),
667688
...(cached.contributingFiles && cached.contributingFiles.length > 0
668689
? { contributingFiles: cached.contributingFiles }
669690
: {}),
@@ -686,6 +707,7 @@ async function compileDocInLegacySandbox(
686707
return {
687708
buffer,
688709
contentType: fmt.contentType,
710+
dependsOnReferencedFiles: touchesReferencedFiles(args.source, contributingFiles.size),
689711
...(contributingFiles.size > 0 ? { contributingFiles: [...contributingFiles.values()] } : {}),
690712
}
691713
}
@@ -729,6 +751,7 @@ export async function compileDoc(args: CompileArgs): Promise<CompiledDocResult>
729751
return {
730752
buffer: existing,
731753
contentType: fmt.contentType,
754+
dependsOnReferencedFiles: touchesReferencedFiles(source, contributingFiles.length),
732755
...(contributingFiles.length > 0 ? { contributingFiles } : {}),
733756
}
734757
}
@@ -898,11 +921,19 @@ export async function resolveServableDocBytes(args: {
898921
// explicitly alongside the table-driven formats.
899922
const magic = format?.magic ?? (extNoDot === 'xlsx' ? ZIP_MAGIC : undefined)
900923
if (magic && bufferStartsWith(rawBuffer, magic)) {
901-
return { buffer: rawBuffer, contentType: getContentType(fileName) }
924+
return {
925+
buffer: rawBuffer,
926+
contentType: getContentType(fileName),
927+
dependsOnReferencedFiles: false,
928+
}
902929
}
903930

904931
if (!format && extNoDot !== 'xlsx') {
905-
return { buffer: rawBuffer, contentType: getContentType(fileName) }
932+
return {
933+
buffer: rawBuffer,
934+
contentType: getContentType(fileName),
935+
dependsOnReferencedFiles: false,
936+
}
906937
}
907938

908939
const source = rawBuffer.toString('utf-8')
@@ -924,21 +955,15 @@ export async function resolveServableDocBytes(args: {
924955
'Referenced document resolution requires an authorized workspace file principal'
925956
)
926957
}
927-
const compiled = await compileDoc({
928-
source,
929-
fileName,
930-
workspaceId,
931-
filePrincipal,
932-
ownerKey,
933-
signal,
934-
})
935-
return { ...compiled, dependsOnReferencedFiles: true }
958+
return compileDoc({ source, fileName, workspaceId, filePrincipal, ownerKey, signal })
936959
}
937960
const stored = await loadCompiledDocByExt(workspaceId, source, extNoDot, {
938961
allowLegacyReferencedArtifact: true,
939962
filePrincipal,
940963
})
941-
if (stored) return stored
964+
// Reached only where the source references nothing, so the artifact is keyed by the source
965+
// alone and cannot change while that source does not.
966+
if (stored) return { ...stored, dependsOnReferencedFiles: false }
942967
throw new DocCompileUserError('Document is still being generated', { pending: true })
943968
}
944969

@@ -952,6 +977,12 @@ export async function resolveServableDocBytes(args: {
952977
return {
953978
buffer: cached.buffer,
954979
contentType: format.contentType,
980+
// The process-local cache is shared with compiles that DID carry a workspace, so a cached
981+
// entry can hold contributor identities even though this call passed none.
982+
dependsOnReferencedFiles: touchesReferencedFiles(
983+
source,
984+
cached.contributingFiles?.length ?? 0
985+
),
955986
...(cached.contributingFiles && cached.contributingFiles.length > 0
956987
? { contributingFiles: cached.contributingFiles }
957988
: {}),
@@ -964,5 +995,9 @@ export async function resolveServableDocBytes(args: {
964995
{ ownerKey, signal }
965996
)
966997
compiledCacheSet(cacheKey, compiled)
967-
return { buffer: compiled, contentType: format.contentType }
998+
return {
999+
buffer: compiled,
1000+
contentType: format.contentType,
1001+
dependsOnReferencedFiles: touchesReferencedFiles(source, 0),
1002+
}
9681003
}

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

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -379,11 +379,49 @@ describe('resolveServableDocBytes', () => {
379379
fileName: 'report.pdf',
380380
workspaceId: WORKSPACE_ID,
381381
})
382-
).resolves.toEqual({ buffer: compiled, contentType: 'application/pdf' })
382+
).resolves.toEqual({
383+
buffer: compiled,
384+
contentType: 'application/pdf',
385+
// The isolated-VM path, but this source references nothing and the compile touched
386+
// nothing — so it stays cacheable. The flag tracks dependency, not which backend ran.
387+
dependsOnReferencedFiles: false,
388+
})
383389
expect(mockLoadCompiledDoc).not.toHaveBeenCalled()
384390
expect(mockStoreCompiledDoc).not.toHaveBeenCalled()
385391
})
386392

393+
it('reports a reference dependency when the isolated-VM broker reads a workspace file', async () => {
394+
/**
395+
* The isolated-VM fallback returns before the static reference scan, so nothing about the
396+
* SOURCE marks it as dependent — only the broker access does. Missing that is what let a
397+
* versioned request take a one-year immutable lifetime for bytes that change when the
398+
* referenced file changes.
399+
*/
400+
const compiled = Buffer.from('%PDF-broker-read')
401+
const contributor = {
402+
fileId: 'broker-reference-1',
403+
key: 'workspace/workspace-1/broker-reference-1.png',
404+
context: 'workspace' as const,
405+
contentUpdatedAt: new Date('2026-08-07T01:00:00.000Z'),
406+
}
407+
setEnvFlags({ isDocSandboxEnabled: false })
408+
mockRunSandboxTask.mockImplementationOnce((...args: unknown[]) => {
409+
const options = args[2] as {
410+
onWorkspaceFileAccess?: (identity: typeof contributor) => void
411+
}
412+
options.onWorkspaceFileAccess?.(contributor)
413+
return Promise.resolve(compiled)
414+
})
415+
416+
const result = await resolveServableDocBytes({
417+
rawBuffer: Buffer.from('const viaBroker = true'),
418+
fileName: 'report.pdf',
419+
workspaceId: WORKSPACE_ID,
420+
})
421+
422+
expect(result.dependsOnReferencedFiles).toBe(true)
423+
})
424+
387425
it('preserves contributor identities when a servable document hits the local compile cache', async () => {
388426
const source = 'const cacheContributor = true'
389427
const compiled = Buffer.from('%PDF-cached-with-contributor')
@@ -413,6 +451,7 @@ describe('resolveServableDocBytes', () => {
413451
).resolves.toEqual({
414452
buffer: compiled,
415453
contentType: 'application/pdf',
454+
dependsOnReferencedFiles: true,
416455
contributingFiles: [contributor],
417456
})
418457

@@ -425,6 +464,7 @@ describe('resolveServableDocBytes', () => {
425464
).resolves.toEqual({
426465
buffer: compiled,
427466
contentType: 'application/pdf',
467+
dependsOnReferencedFiles: true,
428468
contributingFiles: [contributor],
429469
})
430470
expect(mockRunSandboxTask).toHaveBeenCalledTimes(1)
@@ -443,7 +483,11 @@ describe('resolveServableDocBytes', () => {
443483
workspaceId: WORKSPACE_ID,
444484
})
445485

446-
expect(result).toEqual({ buffer: compiled, contentType: 'application/pdf' })
486+
expect(result).toEqual({
487+
buffer: compiled,
488+
contentType: 'application/pdf',
489+
dependsOnReferencedFiles: true,
490+
})
447491
expect(mockExecuteInSandbox).not.toHaveBeenCalled()
448492
expect(mockRunSandboxTask).toHaveBeenCalledWith(
449493
'pdf-generate',
@@ -467,7 +511,11 @@ describe('resolveServableDocBytes', () => {
467511
fileName: 'report.pdf',
468512
workspaceId: WORKSPACE_ID,
469513
})
470-
).resolves.toEqual({ buffer: compiled, contentType: 'application/pdf' })
514+
).resolves.toEqual({
515+
buffer: compiled,
516+
contentType: 'application/pdf',
517+
dependsOnReferencedFiles: true,
518+
})
471519
expect(mockRunSandboxTask).toHaveBeenCalledTimes(1)
472520
expect(mockReadWorkspaceFileMetadata).not.toHaveBeenCalled()
473521
expect(mockStoreCompiledDoc).not.toHaveBeenCalled()

0 commit comments

Comments
 (0)