Skip to content

Commit 144c506

Browse files
committed
improvement(files): cache stream binding meta, tighten file cache-control, prune dead persist path
- Cache the agent-stream ProseMirror↔Yjs binding metadata per session and reuse it across streamed frames instead of rebuilding it (O(doc)) every frame; matches how y-tiptap's own binding maintains the mapping in place. Safe because the shadow doc only ever sees the agent's own reconciles. - Default createFileResponse to a private, no-cache policy so auth-gated bytes are never stored in a shared cache; genuinely-public serve routes opt into public caching explicitly. - Serve content-addressed (key=) embedded images with an immutable private cache to avoid re-downloading them on every doc re-open; fileId= embeds and public shares keep revalidating (the underlying key can change / a share can be revoked). - Drop the dead conflict.version field from the collab-doc persist result and remove the wasted getWorkspaceFile re-read on the conflict path (the relay treats missing and conflict identically and never read version). - Decorate-sort the files/folders lists (compute each sort key once) and index the move-menu subtree build (O(N) vs O(N^2)); ordering is preserved exactly.
1 parent 10bfb5d commit 144c506

14 files changed

Lines changed: 179 additions & 89 deletions

File tree

apps/realtime/src/handlers/file-doc-app.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,13 @@ export async function fetchFileDocMerge(
7777
* Result of a persist attempt (mirrors the app's `persistFileDoc` contract):
7878
* - `persisted` — written; `version` is the new durable version the relay records as synced.
7979
* - `missing` — the file is gone.
80-
* - `conflict` — the file changed out-of-band since `expectedVersion`; NOT written. `version` is the
81-
* current durable version the relay adopts as its new If-Match to re-persist the current live stream.
80+
* - `conflict` — the file changed out-of-band since `expectedVersion`; NOT written. The relay leaves the
81+
* durable content authoritative and reads nothing off this result beyond the status.
8282
*/
8383
export type PersistResult =
8484
| { status: 'persisted'; version: number }
8585
| { status: 'missing' }
86-
| { status: 'conflict'; version: number }
86+
| { status: 'conflict' }
8787
| { status: 'deferred' }
8888

8989
/**

apps/realtime/src/handlers/file-doc.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,6 @@ describe('setupWorkspaceFileDocHandlers', () => {
338338
// authoritative; a later flush projects the converged stream once the merge lands.
339339
mockFetchFileDocPersist.mockResolvedValue({
340340
status: 'conflict',
341-
version: 999,
342341
})
343342
const { io } = createIo()
344343
const { handlers } = setup('socket-1', io)

apps/sim/app/api/files/serve-inline-image.ts

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,20 +8,29 @@ import { createFileResponse, FileNotFoundError } from '@/app/api/files/utils'
88
const logger = createLogger('InlineImageServe')
99

1010
/**
11-
* A shared/edited/deleted file must never serve stale bytes from its fixed inline URL, so every inline
12-
* image revalidates on each request.
11+
* A `fileId=` embed (or a shared/revocable audience) must never serve stale bytes from its fixed inline
12+
* URL, so it revalidates on each request. See `immutable` below for the cacheable case.
1313
*/
1414
const INLINE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
1515

16+
/**
17+
* A `key=` embed addresses a CONTENT-ADDRESSED, immutable storage key (a re-upload mints a new key), so
18+
* its bytes never change — safe to cache hard in the (private) browser cache, avoiding a re-download of
19+
* every embedded image on each doc re-open/re-render. NEVER use this for the public-share route (a share
20+
* can be revoked) or a `fileId=` embed (the underlying key can change under a stable fileId).
21+
*/
22+
const INLINE_IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable'
23+
1624
/**
1725
* Download and respond with an already-workspace-scoped inline image — the single serving tail for both
1826
* the in-app and public inline routes. When `sniff` is set (public shares, a less-trusted audience), the
1927
* served content type is derived from the bytes and non-raster content is refused with 404; otherwise the
20-
* stored content type is served, matching the in-app serve route.
28+
* stored content type is served, matching the in-app serve route. `immutable` opts a content-addressed
29+
* (`key=`) in-app embed into a long private cache; leave it false for `fileId=` embeds and public shares.
2130
*/
2231
export async function serveInlineImage(
2332
image: ResolvedInlineImage,
24-
{ sniff }: { sniff: boolean }
33+
{ sniff, immutable = false }: { sniff: boolean; immutable?: boolean }
2534
): Promise<NextResponse> {
2635
const buffer = await downloadFile({ key: image.key, context: 'workspace' })
2736

@@ -39,6 +48,6 @@ export async function serveInlineImage(
3948
buffer,
4049
contentType,
4150
filename: image.filename,
42-
cacheControl: INLINE_CACHE_CONTROL,
51+
cacheControl: immutable ? INLINE_IMMUTABLE_CACHE_CONTROL : INLINE_CACHE_CONTROL,
4352
})
4453
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ function getWorkspaceIdForCompile(key: string): string | undefined {
6060

6161
const IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable'
6262
const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate'
63+
/** For the genuinely-public, pre-auth asset routes (avatars, OG images, workspace logos) — these are
64+
* intentionally shared-cacheable. Passed EXPLICITLY so the default response cache stays `private`. */
65+
const PUBLIC_ASSET_CACHE_CONTROL = 'public, max-age=31536000'
6366

6467
/**
6568
* Cache-Control for a served file. A versioned request (`?v=<updatedAt>`) addresses
@@ -314,6 +317,7 @@ async function handleCloudProxyPublic(
314317
buffer: fileBuffer,
315318
contentType,
316319
filename,
320+
cacheControl: PUBLIC_ASSET_CACHE_CONTROL,
317321
})
318322
} catch (error) {
319323
logger.error('Error serving public cloud file:', error)
@@ -338,6 +342,7 @@ async function handleLocalFilePublic(filename: string): Promise<NextResponse> {
338342
buffer: fileBuffer,
339343
contentType,
340344
filename,
345+
cacheControl: PUBLIC_ASSET_CACHE_CONTROL,
341346
})
342347
} catch (error) {
343348
logger.error('Error reading public local file:', error)

apps/sim/app/api/files/utils.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,26 @@ describe('extractFilename', () => {
173173
expect(response.headers.get('Content-Security-Policy')).toBeNull()
174174
})
175175

176+
it('defaults to a PRIVATE cache so access-verified content is never shared-cached', () => {
177+
const response = createFileResponse({
178+
buffer: Buffer.from('fake-image-data'),
179+
contentType: 'image/png',
180+
filename: 'safe-image.png',
181+
})
182+
// No explicit cacheControl → must NOT be `public` (a shared cache/CDN could re-serve authed bytes).
183+
expect(response.headers.get('Cache-Control')).toBe('private, no-cache')
184+
})
185+
186+
it('honors an explicit cacheControl (e.g. public assets opt in)', () => {
187+
const response = createFileResponse({
188+
buffer: Buffer.from('fake-image-data'),
189+
contentType: 'image/png',
190+
filename: 'avatar.png',
191+
cacheControl: 'public, max-age=31536000',
192+
})
193+
expect(response.headers.get('Cache-Control')).toBe('public, max-age=31536000')
194+
})
195+
176196
it('should serve PDFs inline safely', () => {
177197
const response = createFileResponse({
178198
buffer: Buffer.from('fake-pdf-data'),

apps/sim/app/api/files/utils.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,10 @@ export function createFileResponse(file: FileResponse): NextResponse {
211211
const headers: Record<string, string> = {
212212
'Content-Type': contentType,
213213
'Content-Disposition': `${disposition}; ${encodeFilenameForHeader(file.filename)}`,
214-
'Cache-Control': file.cacheControl || 'public, max-age=31536000',
214+
// Default to PRIVATE: this response is served only after access verification, so it must never be
215+
// stored by a shared cache/CDN and re-served cross-user. Genuinely public assets (avatars, OG images,
216+
// workspace logos) pass an explicit `cacheControl` (see PUBLIC_ASSET_CACHE_CONTROL in the serve route).
217+
'Cache-Control': file.cacheControl || 'private, no-cache',
215218
'X-Content-Type-Options': 'nosniff',
216219
}
217220

apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,19 @@ describe('GET /api/workspaces/[id]/files/inline', () => {
3838
mockDownloadFile.mockResolvedValue(PNG)
3939
})
4040

41-
it('serves a workspace-scoped image by fileId', async () => {
41+
it('serves a workspace-scoped image by fileId (revalidated — the key can change on re-upload)', async () => {
4242
const res = await GET(req('fileId=wf_abc'), params)
4343
expect(res.status).toBe(200)
4444
expect(mockResolveImage).toHaveBeenCalledWith('ws-1', { fileId: 'wf_abc' })
45+
// A fileId points at whatever bytes are current, so it must NOT be cached immutably.
46+
expect(res.headers.get('Cache-Control')).toBe('private, no-cache, must-revalidate')
4547
})
4648

47-
it('serves a workspace-scoped image by key', async () => {
49+
it('serves a workspace-scoped image by key with an immutable (content-addressed) cache', async () => {
4850
const res = await GET(req(`key=${encodeURIComponent('workspace/ws-1/x-photo.png')}`), params)
4951
expect(res.status).toBe(200)
52+
// A `key=` embed addresses an immutable storage key → cache hard (privately) to avoid re-downloads.
53+
expect(res.headers.get('Cache-Control')).toBe('private, max-age=31536000, immutable')
5054
})
5155

5256
it('404s when the reference does not resolve in the workspace (cross-workspace)', async () => {

apps/sim/app/api/workspaces/[id]/files/inline/route.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ export const GET = withRouteHandler(
4747
throw new FileNotFoundError('Not found')
4848
}
4949

50-
return await serveInlineImage(image, { sniff: false })
50+
// A `key=` embed addresses a content-addressed, immutable storage key → cache it hard (privately)
51+
// so re-opening a doc doesn't re-download every embedded image. A `fileId=` embed can point at new
52+
// bytes after a re-upload, so it must keep revalidating.
53+
return await serveInlineImage(image, { sniff: false, immutable: 'key' in ref })
5154
} catch (error) {
5255
if (error instanceof FileNotFoundError) {
5356
return createErrorResponse(error)

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.test.ts

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import { afterEach, beforeAll, describe, expect, it } from 'vitest'
77
import { Awareness } from 'y-protocols/awareness'
88
import * as Y from 'yjs'
99
import { createMarkdownEditorExtensions } from '../editor-extensions'
10-
import { applyAgentStreamFrame, beginAgentStream, endAgentStream } from './apply-streamed-markdown'
10+
import {
11+
AGENT_STREAM_ORIGIN,
12+
applyAgentStreamFrame,
13+
beginAgentStream,
14+
endAgentStream,
15+
} from './apply-streamed-markdown'
1116

1217
beforeAll(() => {
1318
// jsdom does not implement elementFromPoint; the Placeholder extension's viewport tracking calls it
@@ -186,4 +191,35 @@ describe('agent-stream applier', () => {
186191
expect(live).toContain('EDITED')
187192
expect(live).toContain('Gamma paragraph')
188193
})
194+
195+
it('reuses cached binding metadata across frames, still emitting minimal per-frame deltas', () => {
196+
// The binding `meta` is built ONCE (first frame) and reused — `updateYFragment` maintains it in place,
197+
// so we skip an O(doc) `initProseMirrorDoc` rebuild per frame. This guards that caching preserves the
198+
// minimal-delta + no-duplication behavior across frames (a stale/rebuilt mapping would re-emit existing
199+
// paragraphs → duplication).
200+
const { editor, doc } = track(makeCollabEditor())
201+
const session = beginAgentStream(editor)!
202+
203+
applyAgentStreamFrame(editor, session, 'One.')
204+
expect(session.meta).not.toBeNull() // built and cached on the first frame
205+
206+
const deltas: Uint8Array[] = []
207+
const onUpdate = (u: Uint8Array, origin: unknown) => {
208+
if (origin === AGENT_STREAM_ORIGIN) deltas.push(u)
209+
}
210+
doc.on('update', onUpdate)
211+
applyAgentStreamFrame(editor, session, 'One.\n\nTwo.')
212+
applyAgentStreamFrame(editor, session, 'One.\n\nTwo.\n\nThree.')
213+
doc.off('update', onUpdate)
214+
215+
// Two later frames → two incremental deltas; the cached mapping kept diffs minimal and correct.
216+
expect(deltas.length).toBe(2)
217+
const text = editor.getText()
218+
expect(text).toContain('One')
219+
expect(text).toContain('Two')
220+
expect(text).toContain('Three')
221+
expect(text.match(/One/g)?.length).toBe(1)
222+
expect(text.match(/Two/g)?.length).toBe(1)
223+
endAgentStream(session)
224+
})
189225
})

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ export const AGENT_STREAM_ORIGIN = Symbol('agent-stream')
2323
export interface AgentStreamSession {
2424
shadow: Y.Doc
2525
fragment: Y.XmlFragment
26+
/**
27+
* The fragment↔PM binding metadata (`mapping`/`isOMark`) `updateYFragment` diffs against. Built ONCE
28+
* from the seeded fragment on the first frame, then maintained IN PLACE by `updateYFragment` on every
29+
* subsequent frame — the same persistent structure y-tiptap's own `ProsemirrorBinding` keeps for a
30+
* doc's whole life. Caching it avoids an O(doc) `initProseMirrorDoc` tree rebuild per streamed frame.
31+
* Safe ONLY because the shadow receives the agent's OWN reconciles and nothing else (never peer
32+
* updates), so the fragment never changes outside `updateYFragment`; it is torn down with the session
33+
* on leadership loss, so it can't outlive its fragment. A future change that ever applies an external
34+
* update to `shadow` MUST reset this to `null`.
35+
*/
36+
meta: ReturnType<typeof initProseMirrorDoc>['meta'] | null
2637
}
2738

2839
/**
@@ -34,7 +45,7 @@ export function beginAgentStream(editor: Editor): AgentStreamSession | null {
3445
if (!binding) return null
3546
const shadow = new Y.Doc()
3647
Y.applyUpdate(shadow, Y.encodeStateAsUpdate(binding.doc))
37-
return { shadow, fragment: shadow.getXmlFragment(COLLAB_DOC_FIELD) }
48+
return { shadow, fragment: shadow.getXmlFragment(COLLAB_DOC_FIELD), meta: null }
3849
}
3950

4051
/**
@@ -60,10 +71,10 @@ export function applyAgentStreamFrame(
6071
session.shadow.on('update', capture)
6172
try {
6273
session.shadow.transact(() => {
63-
// `updateYFragment` diffs against the fragment's CURRENT content, so it needs the fragment↔PM
64-
// binding metadata; `initProseMirrorDoc` reconstructs it from the fragment's present state.
65-
const { meta } = initProseMirrorDoc(session.fragment, editor.schema)
66-
updateYFragment(session.shadow, session.fragment, target, meta)
74+
// Build the binding metadata once, then reuse it; updateYFragment maintains it in place. See
75+
// AgentStreamSession.meta for why per-frame reuse is safe (and why a rebuild would be wasteful).
76+
session.meta ??= initProseMirrorDoc(session.fragment, editor.schema).meta
77+
updateYFragment(session.shadow, session.fragment, target, session.meta)
6778
}, AGENT_STREAM_ORIGIN)
6879
} finally {
6980
session.shadow.off('update', capture)

0 commit comments

Comments
 (0)