From 144c506ef46c263e93b78169e819d95b2136ab84 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 31 Jul 2026 19:24:18 -0700 Subject: [PATCH 1/2] improvement(files): cache stream binding meta, tighten file cache-control, prune dead persist path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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. --- apps/realtime/src/handlers/file-doc-app.ts | 6 +- apps/realtime/src/handlers/file-doc.test.ts | 1 - apps/sim/app/api/files/serve-inline-image.ts | 19 +++- .../app/api/files/serve/[...path]/route.ts | 5 + apps/sim/app/api/files/utils.test.ts | 20 ++++ apps/sim/app/api/files/utils.ts | 5 +- .../[id]/files/inline/route.test.ts | 8 +- .../api/workspaces/[id]/files/inline/route.ts | 5 +- .../apply-streamed-markdown.test.ts | 38 +++++++- .../collaboration/apply-streamed-markdown.ts | 21 +++- .../rich-markdown-editor.tsx | 8 +- .../workspace/[workspaceId]/files/files.tsx | 97 +++++++++++-------- apps/sim/lib/api/contracts/file-doc.ts | 8 +- apps/sim/lib/collab-doc/persist.ts | 27 ++---- 14 files changed, 179 insertions(+), 89 deletions(-) diff --git a/apps/realtime/src/handlers/file-doc-app.ts b/apps/realtime/src/handlers/file-doc-app.ts index abee966dd46..87e515954be 100644 --- a/apps/realtime/src/handlers/file-doc-app.ts +++ b/apps/realtime/src/handlers/file-doc-app.ts @@ -77,13 +77,13 @@ export async function fetchFileDocMerge( * Result of a persist attempt (mirrors the app's `persistFileDoc` contract): * - `persisted` — written; `version` is the new durable version the relay records as synced. * - `missing` — the file is gone. - * - `conflict` — the file changed out-of-band since `expectedVersion`; NOT written. `version` is the - * current durable version the relay adopts as its new If-Match to re-persist the current live stream. + * - `conflict` — the file changed out-of-band since `expectedVersion`; NOT written. The relay leaves the + * durable content authoritative and reads nothing off this result beyond the status. */ export type PersistResult = | { status: 'persisted'; version: number } | { status: 'missing' } - | { status: 'conflict'; version: number } + | { status: 'conflict' } | { status: 'deferred' } /** diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 153745e1d21..d56952bc619 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -338,7 +338,6 @@ describe('setupWorkspaceFileDocHandlers', () => { // authoritative; a later flush projects the converged stream once the merge lands. mockFetchFileDocPersist.mockResolvedValue({ status: 'conflict', - version: 999, }) const { io } = createIo() const { handlers } = setup('socket-1', io) diff --git a/apps/sim/app/api/files/serve-inline-image.ts b/apps/sim/app/api/files/serve-inline-image.ts index 88c3383d961..1017df45765 100644 --- a/apps/sim/app/api/files/serve-inline-image.ts +++ b/apps/sim/app/api/files/serve-inline-image.ts @@ -8,20 +8,29 @@ import { createFileResponse, FileNotFoundError } from '@/app/api/files/utils' const logger = createLogger('InlineImageServe') /** - * A shared/edited/deleted file must never serve stale bytes from its fixed inline URL, so every inline - * image revalidates on each request. + * A `fileId=` embed (or a shared/revocable audience) must never serve stale bytes from its fixed inline + * URL, so it revalidates on each request. See `immutable` below for the cacheable case. */ const INLINE_CACHE_CONTROL = 'private, no-cache, must-revalidate' +/** + * A `key=` embed addresses a CONTENT-ADDRESSED, immutable storage key (a re-upload mints a new key), so + * its bytes never change — safe to cache hard in the (private) browser cache, avoiding a re-download of + * every embedded image on each doc re-open/re-render. NEVER use this for the public-share route (a share + * can be revoked) or a `fileId=` embed (the underlying key can change under a stable fileId). + */ +const INLINE_IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable' + /** * Download and respond with an already-workspace-scoped inline image — the single serving tail for both * the in-app and public inline routes. When `sniff` is set (public shares, a less-trusted audience), the * served content type is derived from the bytes and non-raster content is refused with 404; otherwise the - * stored content type is served, matching the in-app serve route. + * stored content type is served, matching the in-app serve route. `immutable` opts a content-addressed + * (`key=`) in-app embed into a long private cache; leave it false for `fileId=` embeds and public shares. */ export async function serveInlineImage( image: ResolvedInlineImage, - { sniff }: { sniff: boolean } + { sniff, immutable = false }: { sniff: boolean; immutable?: boolean } ): Promise { const buffer = await downloadFile({ key: image.key, context: 'workspace' }) @@ -39,6 +48,6 @@ export async function serveInlineImage( buffer, contentType, filename: image.filename, - cacheControl: INLINE_CACHE_CONTROL, + cacheControl: immutable ? INLINE_IMMUTABLE_CACHE_CONTROL : INLINE_CACHE_CONTROL, }) } diff --git a/apps/sim/app/api/files/serve/[...path]/route.ts b/apps/sim/app/api/files/serve/[...path]/route.ts index aad77fe0393..4b29a568e78 100644 --- a/apps/sim/app/api/files/serve/[...path]/route.ts +++ b/apps/sim/app/api/files/serve/[...path]/route.ts @@ -60,6 +60,9 @@ function getWorkspaceIdForCompile(key: string): string | undefined { const IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable' const WORKSPACE_REVALIDATE_CACHE_CONTROL = 'private, no-cache, must-revalidate' +/** For the genuinely-public, pre-auth asset routes (avatars, OG images, workspace logos) — these are + * intentionally shared-cacheable. Passed EXPLICITLY so the default response cache stays `private`. */ +const PUBLIC_ASSET_CACHE_CONTROL = 'public, max-age=31536000' /** * Cache-Control for a served file. A versioned request (`?v=`) addresses @@ -314,6 +317,7 @@ async function handleCloudProxyPublic( buffer: fileBuffer, contentType, filename, + cacheControl: PUBLIC_ASSET_CACHE_CONTROL, }) } catch (error) { logger.error('Error serving public cloud file:', error) @@ -338,6 +342,7 @@ async function handleLocalFilePublic(filename: string): Promise { buffer: fileBuffer, contentType, filename, + cacheControl: PUBLIC_ASSET_CACHE_CONTROL, }) } catch (error) { logger.error('Error reading public local file:', error) diff --git a/apps/sim/app/api/files/utils.test.ts b/apps/sim/app/api/files/utils.test.ts index ff02212a808..3e4e2463cc3 100644 --- a/apps/sim/app/api/files/utils.test.ts +++ b/apps/sim/app/api/files/utils.test.ts @@ -173,6 +173,26 @@ describe('extractFilename', () => { expect(response.headers.get('Content-Security-Policy')).toBeNull() }) + it('defaults to a PRIVATE cache so access-verified content is never shared-cached', () => { + const response = createFileResponse({ + buffer: Buffer.from('fake-image-data'), + contentType: 'image/png', + filename: 'safe-image.png', + }) + // No explicit cacheControl → must NOT be `public` (a shared cache/CDN could re-serve authed bytes). + expect(response.headers.get('Cache-Control')).toBe('private, no-cache') + }) + + it('honors an explicit cacheControl (e.g. public assets opt in)', () => { + const response = createFileResponse({ + buffer: Buffer.from('fake-image-data'), + contentType: 'image/png', + filename: 'avatar.png', + cacheControl: 'public, max-age=31536000', + }) + expect(response.headers.get('Cache-Control')).toBe('public, max-age=31536000') + }) + it('should serve PDFs inline safely', () => { const response = createFileResponse({ buffer: Buffer.from('fake-pdf-data'), diff --git a/apps/sim/app/api/files/utils.ts b/apps/sim/app/api/files/utils.ts index 0eedc8211cc..a5f97e4b431 100644 --- a/apps/sim/app/api/files/utils.ts +++ b/apps/sim/app/api/files/utils.ts @@ -211,7 +211,10 @@ export function createFileResponse(file: FileResponse): NextResponse { const headers: Record = { 'Content-Type': contentType, 'Content-Disposition': `${disposition}; ${encodeFilenameForHeader(file.filename)}`, - 'Cache-Control': file.cacheControl || 'public, max-age=31536000', + // Default to PRIVATE: this response is served only after access verification, so it must never be + // stored by a shared cache/CDN and re-served cross-user. Genuinely public assets (avatars, OG images, + // workspace logos) pass an explicit `cacheControl` (see PUBLIC_ASSET_CACHE_CONTROL in the serve route). + 'Cache-Control': file.cacheControl || 'private, no-cache', 'X-Content-Type-Options': 'nosniff', } diff --git a/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts index c57ac919e38..9f19b421462 100644 --- a/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts @@ -38,15 +38,19 @@ describe('GET /api/workspaces/[id]/files/inline', () => { mockDownloadFile.mockResolvedValue(PNG) }) - it('serves a workspace-scoped image by fileId', async () => { + it('serves a workspace-scoped image by fileId (revalidated — the key can change on re-upload)', async () => { const res = await GET(req('fileId=wf_abc'), params) expect(res.status).toBe(200) expect(mockResolveImage).toHaveBeenCalledWith('ws-1', { fileId: 'wf_abc' }) + // A fileId points at whatever bytes are current, so it must NOT be cached immutably. + expect(res.headers.get('Cache-Control')).toBe('private, no-cache, must-revalidate') }) - it('serves a workspace-scoped image by key', async () => { + it('serves a workspace-scoped image by key with an immutable (content-addressed) cache', async () => { const res = await GET(req(`key=${encodeURIComponent('workspace/ws-1/x-photo.png')}`), params) expect(res.status).toBe(200) + // A `key=` embed addresses an immutable storage key → cache hard (privately) to avoid re-downloads. + expect(res.headers.get('Cache-Control')).toBe('private, max-age=31536000, immutable') }) it('404s when the reference does not resolve in the workspace (cross-workspace)', async () => { diff --git a/apps/sim/app/api/workspaces/[id]/files/inline/route.ts b/apps/sim/app/api/workspaces/[id]/files/inline/route.ts index 245fb5731d8..42720f927f8 100644 --- a/apps/sim/app/api/workspaces/[id]/files/inline/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/inline/route.ts @@ -47,7 +47,10 @@ export const GET = withRouteHandler( throw new FileNotFoundError('Not found') } - return await serveInlineImage(image, { sniff: false }) + // A `key=` embed addresses a content-addressed, immutable storage key → cache it hard (privately) + // so re-opening a doc doesn't re-download every embedded image. A `fileId=` embed can point at new + // bytes after a re-upload, so it must keep revalidating. + return await serveInlineImage(image, { sniff: false, immutable: 'key' in ref }) } catch (error) { if (error instanceof FileNotFoundError) { return createErrorResponse(error) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.test.ts index 8b4affc0843..d0019198c8b 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.test.ts @@ -7,7 +7,12 @@ import { afterEach, beforeAll, describe, expect, it } from 'vitest' import { Awareness } from 'y-protocols/awareness' import * as Y from 'yjs' import { createMarkdownEditorExtensions } from '../editor-extensions' -import { applyAgentStreamFrame, beginAgentStream, endAgentStream } from './apply-streamed-markdown' +import { + AGENT_STREAM_ORIGIN, + applyAgentStreamFrame, + beginAgentStream, + endAgentStream, +} from './apply-streamed-markdown' beforeAll(() => { // jsdom does not implement elementFromPoint; the Placeholder extension's viewport tracking calls it @@ -186,4 +191,35 @@ describe('agent-stream applier', () => { expect(live).toContain('EDITED') expect(live).toContain('Gamma paragraph') }) + + it('reuses cached binding metadata across frames, still emitting minimal per-frame deltas', () => { + // The binding `meta` is built ONCE (first frame) and reused — `updateYFragment` maintains it in place, + // so we skip an O(doc) `initProseMirrorDoc` rebuild per frame. This guards that caching preserves the + // minimal-delta + no-duplication behavior across frames (a stale/rebuilt mapping would re-emit existing + // paragraphs → duplication). + const { editor, doc } = track(makeCollabEditor()) + const session = beginAgentStream(editor)! + + applyAgentStreamFrame(editor, session, 'One.') + expect(session.meta).not.toBeNull() // built and cached on the first frame + + const deltas: Uint8Array[] = [] + const onUpdate = (u: Uint8Array, origin: unknown) => { + if (origin === AGENT_STREAM_ORIGIN) deltas.push(u) + } + doc.on('update', onUpdate) + applyAgentStreamFrame(editor, session, 'One.\n\nTwo.') + applyAgentStreamFrame(editor, session, 'One.\n\nTwo.\n\nThree.') + doc.off('update', onUpdate) + + // Two later frames → two incremental deltas; the cached mapping kept diffs minimal and correct. + expect(deltas.length).toBe(2) + const text = editor.getText() + expect(text).toContain('One') + expect(text).toContain('Two') + expect(text).toContain('Three') + expect(text.match(/One/g)?.length).toBe(1) + expect(text.match(/Two/g)?.length).toBe(1) + endAgentStream(session) + }) }) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.ts index f0a69769b2f..88f54d6c0b7 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/apply-streamed-markdown.ts @@ -23,6 +23,17 @@ export const AGENT_STREAM_ORIGIN = Symbol('agent-stream') export interface AgentStreamSession { shadow: Y.Doc fragment: Y.XmlFragment + /** + * The fragment↔PM binding metadata (`mapping`/`isOMark`) `updateYFragment` diffs against. Built ONCE + * from the seeded fragment on the first frame, then maintained IN PLACE by `updateYFragment` on every + * subsequent frame — the same persistent structure y-tiptap's own `ProsemirrorBinding` keeps for a + * doc's whole life. Caching it avoids an O(doc) `initProseMirrorDoc` tree rebuild per streamed frame. + * Safe ONLY because the shadow receives the agent's OWN reconciles and nothing else (never peer + * updates), so the fragment never changes outside `updateYFragment`; it is torn down with the session + * on leadership loss, so it can't outlive its fragment. A future change that ever applies an external + * update to `shadow` MUST reset this to `null`. + */ + meta: ReturnType['meta'] | null } /** @@ -34,7 +45,7 @@ export function beginAgentStream(editor: Editor): AgentStreamSession | null { if (!binding) return null const shadow = new Y.Doc() Y.applyUpdate(shadow, Y.encodeStateAsUpdate(binding.doc)) - return { shadow, fragment: shadow.getXmlFragment(COLLAB_DOC_FIELD) } + return { shadow, fragment: shadow.getXmlFragment(COLLAB_DOC_FIELD), meta: null } } /** @@ -60,10 +71,10 @@ export function applyAgentStreamFrame( session.shadow.on('update', capture) try { session.shadow.transact(() => { - // `updateYFragment` diffs against the fragment's CURRENT content, so it needs the fragment↔PM - // binding metadata; `initProseMirrorDoc` reconstructs it from the fragment's present state. - const { meta } = initProseMirrorDoc(session.fragment, editor.schema) - updateYFragment(session.shadow, session.fragment, target, meta) + // Build the binding metadata once, then reuse it; updateYFragment maintains it in place. See + // AgentStreamSession.meta for why per-frame reuse is safe (and why a rebuild would be wasteful). + session.meta ??= initProseMirrorDoc(session.fragment, editor.schema).meta + updateYFragment(session.shadow, session.fragment, target, session.meta) }, AGENT_STREAM_ORIGIN) } finally { session.shadow.off('update', capture) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx index 0b7da2229e4..5cd2f43dd3a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx @@ -810,10 +810,10 @@ export function LoadedRichMarkdownEditor({ if (editor.isEditable === isEditable) return // Defer out of the render/commit phase. `isEditable` flips from collab readiness (synced + seeded), // which is driven by a Yjs `config.observe` firing synchronously inside `Y.applyUpdate` — so this - // effect can run while React is mid-render. `setEditable` dispatches a TipTap transaction that the - // React binding commits with `flushSync`, which throws ("cannot flush while rendering") in that - // window. A microtask runs right after the current commit, before paint; re-check liveness/value - // since either can change before it fires. + // effect can run while React is mid-render. `setEditable` re-applies the view state and emits an + // `update` that the React binding commits with `flushSync`, which throws ("cannot flush while + // rendering") in that window. A microtask runs right after the current commit, before paint; re-check + // liveness/value since either can change before it fires. let cancelled = false queueMicrotask(() => { if (cancelled || editor.isDestroyed) return diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 82aad11ad2a..84c480a4c47 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -468,23 +468,28 @@ export function Files() { : siblings const col = activeSort?.column ?? 'name' const dir = activeSort?.direction ?? 'asc' - return [...searched].sort((a, b) => { + // Decorate-sort: compute each key + pinned flag once (O(N)) rather than parsing dates per comparison. + const decorated = searched.map((folder) => ({ + folder, + pinned: pinnedFolderIds.has(folder.id), + key: + col === 'updated' + ? new Date(folder.updatedAt).getTime() + : col === 'created' + ? new Date(folder.createdAt).getTime() + : folder.name, + })) + decorated.sort((a, b) => { // Pinned folders float to the top of every sort/direction — pinning is a // user-declared priority, not another sort key to be inverted by `desc`. - const aPinned = pinnedFolderIds.has(a.id) - const bPinned = pinnedFolderIds.has(b.id) - if (aPinned !== bPinned) return aPinned ? -1 : 1 - - let cmp = 0 - if (col === 'updated') { - cmp = new Date(a.updatedAt).getTime() - new Date(b.updatedAt).getTime() - } else if (col === 'created') { - cmp = new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime() - } else { - cmp = a.name.localeCompare(b.name) - } + if (a.pinned !== b.pinned) return a.pinned ? -1 : 1 + const cmp = + typeof a.key === 'number' && typeof b.key === 'number' + ? a.key - b.key + : String(a.key).localeCompare(String(b.key)) return dir === 'asc' ? cmp : -cmp }) + return decorated.map((d) => d.folder) }, [folders, currentFolderId, debouncedSearchTerm, activeSort, pinnedFolderIds]) const filteredFiles = useMemo(() => { @@ -522,38 +527,36 @@ export function Files() { const col = activeSort?.column ?? 'updated' const dir = activeSort?.direction ?? 'desc' - return [...result].sort((a, b) => { + // Decorate-sort: compute each row's sort key + pinned flag ONCE (O(N)), then compare precomputed + // keys — the comparator ran per-comparison work (Date parsing, `formatFileType`, member lookups) at + // O(N log N). Stable sort + pinned-primary ordering are preserved. + const decorated = result.map((f) => ({ + f, + pinned: pinnedFileIds.has(f.id), + key: + col === 'size' + ? f.size + : col === 'type' + ? formatFileType(f.type, f.name) + : col === 'created' + ? new Date(f.uploadedAt).getTime() + : col === 'updated' + ? new Date(f.updatedAt).getTime() + : col === 'owner' + ? (membersById.get(f.uploadedBy)?.name ?? '') + : f.name, + })) + decorated.sort((a, b) => { // Pinned files float to the top of every sort/direction — pinning is a // user-declared priority, not another sort key to be inverted by `desc`. - const aPinned = pinnedFileIds.has(a.id) - const bPinned = pinnedFileIds.has(b.id) - if (aPinned !== bPinned) return aPinned ? -1 : 1 - - let cmp = 0 - switch (col) { - case 'name': - cmp = a.name.localeCompare(b.name) - break - case 'size': - cmp = a.size - b.size - break - case 'type': - cmp = formatFileType(a.type, a.name).localeCompare(formatFileType(b.type, b.name)) - break - case 'created': - cmp = new Date(a.uploadedAt).getTime() - new Date(b.uploadedAt).getTime() - break - case 'updated': - cmp = new Date(a.updatedAt).getTime() - new Date(b.updatedAt).getTime() - break - case 'owner': - cmp = (membersById.get(a.uploadedBy)?.name ?? '').localeCompare( - membersById.get(b.uploadedBy)?.name ?? '' - ) - break - } + if (a.pinned !== b.pinned) return a.pinned ? -1 : 1 + const cmp = + typeof a.key === 'number' && typeof b.key === 'number' + ? a.key - b.key + : String(a.key).localeCompare(String(b.key)) return dir === 'asc' ? cmp : -cmp }) + return decorated.map((d) => d.f) }, [ files, currentFolderId, @@ -1818,10 +1821,18 @@ export function Files() { ) const contextMenuMoveOptions = useMemo((): MoveOptionNode[] => { + // Index children by parent ONCE (the same pattern used for folder sizes + descendant maps above), + // so building the tree is O(N) instead of a full `folders.filter` scan at every node (O(N²)). + const childrenByParent = new Map() + for (const f of folders) { + const key = f.parentId ?? null + const arr = childrenByParent.get(key) + if (arr) arr.push(f) + else childrenByParent.set(key, [f]) + } const buildSubtree = (parentId: string | null): MoveOptionNode[] => - folders + (childrenByParent.get(parentId) ?? []) .filter((f) => { - if ((f.parentId ?? null) !== parentId) return false if (selectedFolderIds.includes(f.id)) return false return selectedFolderIds.every( (sid) => !descendantFolderIdsByFolderId.get(sid)?.has(f.id) diff --git a/apps/sim/lib/api/contracts/file-doc.ts b/apps/sim/lib/api/contracts/file-doc.ts index 4284640570d..c3f5b1d610c 100644 --- a/apps/sim/lib/api/contracts/file-doc.ts +++ b/apps/sim/lib/api/contracts/file-doc.ts @@ -125,13 +125,11 @@ export const persistFileDocResponseSchema = z.discriminatedUnion('status', [ }), z.object({ /** - * The file changed out-of-band since `expectedVersion` — the write was refused to avoid clobbering - * it. `version` is the current durable version the relay adopts as its new If-Match to re-persist the - * current live stream (which already holds the out-of-band change via the write chokepoint). No - * markdown body: the relay never projects the durable body back over the live doc. + * The file changed out-of-band since `expectedVersion` — the write was refused to avoid clobbering it. + * The relay leaves the durable content authoritative and does not re-persist here (a later flush + * reconciles once the chokepoint merge lands), so it reads nothing off this result beyond the status. */ status: z.literal('conflict'), - version: z.number().int(), }), ]) export type PersistFileDocResponse = z.output diff --git a/apps/sim/lib/collab-doc/persist.ts b/apps/sim/lib/collab-doc/persist.ts index 24d4590ff50..e07b292d628 100644 --- a/apps/sim/lib/collab-doc/persist.ts +++ b/apps/sim/lib/collab-doc/persist.ts @@ -17,14 +17,14 @@ const logger = createLogger('FileDocPersist') * CONTENT version (`content_updated_at`, epoch ms) the relay records as what its live doc is synced to. * - `missing` — the file is gone (deleted); nothing to write. * - `conflict` — the file changed out-of-band since the relay's live doc last synced, so writing the - * projection would clobber that change (RFC 7232 `If-Match` failure). NOT written. `version` is the - * current durable version the relay adopts as its new If-Match to re-persist the current live stream - * (which already holds the out-of-band change via the write chokepoint). + * projection would clobber that change (RFC 7232 `If-Match` failure). NOT written; the relay leaves the + * durable content authoritative and does not advance its synced version (a later flush reconciles once + * the chokepoint merge lands). No `version` is returned — the relay never reads one on this path. */ export type PersistFileDocResult = | { status: 'persisted'; version: number } | { status: 'missing' } - | { status: 'conflict'; version: number } + | { status: 'conflict' } | { status: 'deferred' } /** @@ -113,22 +113,13 @@ export async function persistFileDoc( } } catch (error) { if (!(error instanceof ContentVersionConflictError)) throw error - // Out-of-band content change since the live doc last synced — DON'T clobber. Return the current - // durable version so the relay adopts it as its new If-Match and re-persists the current live stream - // (which already holds the out-of-band change, merged in via the write chokepoint). No markdown body - // is returned: the relay never projects the durable body back over the live doc (that would be a - // destructive "make it match" that could move the doc backward and wipe newer edits). - const current = await getWorkspaceFile(workspaceId, fileId, { throwOnError: true }) - if (!current) return { status: 'missing' } + // Out-of-band content change since the live doc last synced — DON'T clobber. The relay leaves the + // durable content authoritative and does NOT re-persist or advance its synced version here (a later + // flush reconciles once the chokepoint merge lands), so it reads nothing off this result beyond the + // `conflict` status — no durable version re-read is needed. logger.warn( `Persist conflict for file ${fileId}; durable content changed out-of-band since sync` ) - return { - status: 'conflict', - // The CONTENT version, not `updatedAt`: the relay adopts this as its If-Match. If a metadata write - // bumped `updatedAt` past `contentUpdatedAt`, returning `updatedAt` would make the re-persist's CAS - // (which checks `contentUpdatedAt`) never match → perpetual conflict. - version: (current.contentUpdatedAt ?? current.updatedAt).getTime(), - } + return { status: 'conflict' } } } From 04e8424fb57775fb57a7cc7f9b59e98a192f6c3a Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 31 Jul 2026 19:58:06 -0700 Subject: [PATCH 2/2] fix(files): serve inline images with no-cache so deletion/authorization is enforced per request Embedded images are authenticated content whose backing file can be deleted or have its access revoked at any time. Both key= and fileId= embeds serve private, no-cache, must-revalidate, so every request re-runs the server-side deletion/authorization check instead of serving a possibly-stale image from cache. --- apps/sim/app/api/files/serve-inline-image.ts | 21 +++++++------------ .../[id]/files/inline/route.test.ts | 11 +++++----- .../api/workspaces/[id]/files/inline/route.ts | 5 +---- 3 files changed, 14 insertions(+), 23 deletions(-) diff --git a/apps/sim/app/api/files/serve-inline-image.ts b/apps/sim/app/api/files/serve-inline-image.ts index 1017df45765..162685e8682 100644 --- a/apps/sim/app/api/files/serve-inline-image.ts +++ b/apps/sim/app/api/files/serve-inline-image.ts @@ -8,29 +8,22 @@ import { createFileResponse, FileNotFoundError } from '@/app/api/files/utils' const logger = createLogger('InlineImageServe') /** - * A `fileId=` embed (or a shared/revocable audience) must never serve stale bytes from its fixed inline - * URL, so it revalidates on each request. See `immutable` below for the cacheable case. + * An embedded image is authenticated content served from a fixed inline URL, and the file behind it can + * be DELETED or its access REVOKED at any time — so it always revalidates, letting each request re-run the + * server-side deletion/authorization check rather than serving a stale (possibly no-longer-authorized) + * image from cache. Private so no shared cache/CDN ever stores it. */ const INLINE_CACHE_CONTROL = 'private, no-cache, must-revalidate' -/** - * A `key=` embed addresses a CONTENT-ADDRESSED, immutable storage key (a re-upload mints a new key), so - * its bytes never change — safe to cache hard in the (private) browser cache, avoiding a re-download of - * every embedded image on each doc re-open/re-render. NEVER use this for the public-share route (a share - * can be revoked) or a `fileId=` embed (the underlying key can change under a stable fileId). - */ -const INLINE_IMMUTABLE_CACHE_CONTROL = 'private, max-age=31536000, immutable' - /** * Download and respond with an already-workspace-scoped inline image — the single serving tail for both * the in-app and public inline routes. When `sniff` is set (public shares, a less-trusted audience), the * served content type is derived from the bytes and non-raster content is refused with 404; otherwise the - * stored content type is served, matching the in-app serve route. `immutable` opts a content-addressed - * (`key=`) in-app embed into a long private cache; leave it false for `fileId=` embeds and public shares. + * stored content type is served, matching the in-app serve route. */ export async function serveInlineImage( image: ResolvedInlineImage, - { sniff, immutable = false }: { sniff: boolean; immutable?: boolean } + { sniff }: { sniff: boolean } ): Promise { const buffer = await downloadFile({ key: image.key, context: 'workspace' }) @@ -48,6 +41,6 @@ export async function serveInlineImage( buffer, contentType, filename: image.filename, - cacheControl: immutable ? INLINE_IMMUTABLE_CACHE_CONTROL : INLINE_CACHE_CONTROL, + cacheControl: INLINE_CACHE_CONTROL, }) } diff --git a/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts b/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts index 9f19b421462..11726424281 100644 --- a/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts +++ b/apps/sim/app/api/workspaces/[id]/files/inline/route.test.ts @@ -38,19 +38,20 @@ describe('GET /api/workspaces/[id]/files/inline', () => { mockDownloadFile.mockResolvedValue(PNG) }) - it('serves a workspace-scoped image by fileId (revalidated — the key can change on re-upload)', async () => { + it('serves a workspace-scoped image by fileId, always revalidating', async () => { const res = await GET(req('fileId=wf_abc'), params) expect(res.status).toBe(200) expect(mockResolveImage).toHaveBeenCalledWith('ws-1', { fileId: 'wf_abc' }) - // A fileId points at whatever bytes are current, so it must NOT be cached immutably. + // Authenticated content: always revalidate so a deletion/revocation is enforced on the next request. expect(res.headers.get('Cache-Control')).toBe('private, no-cache, must-revalidate') }) - it('serves a workspace-scoped image by key with an immutable (content-addressed) cache', async () => { + it('serves a workspace-scoped image by key, always revalidating', async () => { const res = await GET(req(`key=${encodeURIComponent('workspace/ws-1/x-photo.png')}`), params) expect(res.status).toBe(200) - // A `key=` embed addresses an immutable storage key → cache hard (privately) to avoid re-downloads. - expect(res.headers.get('Cache-Control')).toBe('private, max-age=31536000, immutable') + // Same policy as fileId: authenticated content never cached past a revalidation, so a deleted or + // access-revoked image drops out immediately rather than lingering in a private browser cache. + expect(res.headers.get('Cache-Control')).toBe('private, no-cache, must-revalidate') }) it('404s when the reference does not resolve in the workspace (cross-workspace)', async () => { diff --git a/apps/sim/app/api/workspaces/[id]/files/inline/route.ts b/apps/sim/app/api/workspaces/[id]/files/inline/route.ts index 42720f927f8..245fb5731d8 100644 --- a/apps/sim/app/api/workspaces/[id]/files/inline/route.ts +++ b/apps/sim/app/api/workspaces/[id]/files/inline/route.ts @@ -47,10 +47,7 @@ export const GET = withRouteHandler( throw new FileNotFoundError('Not found') } - // A `key=` embed addresses a content-addressed, immutable storage key → cache it hard (privately) - // so re-opening a doc doesn't re-download every embedded image. A `fileId=` embed can point at new - // bytes after a re-upload, so it must keep revalidating. - return await serveInlineImage(image, { sniff: false, immutable: 'key' in ref }) + return await serveInlineImage(image, { sniff: false }) } catch (error) { if (error instanceof FileNotFoundError) { return createErrorResponse(error)