Skip to content

Commit fd34853

Browse files
authored
fix(collab-doc): stream every external file write into open editors, not just edit_content (#6070)
* fix(collab-doc): stream every external file write into open editors, not just edit_content A copilot/mothership edit to an open markdown file did not appear live in another user's editor: the live-doc merge bridge (mergeEditIntoLiveFileDoc) was wired into the edit_content tool ONLY. Every other server-side write — the file tool (/api/tools/file/manage), function_execute (/api/function/execute via writeWorkspaceFileByPath), create_file overwrite, and the PUT /content route — went straight to updateWorkspaceFileContent and skipped the merge, so the durable file changed but the open editor never updated. Confirmed from live logs (the mothership 'Prepend sentence' ran read + file + function_execute — zero apply-edit calls) and Redis (the prepended text was absent from the doc stream). Centralize the merge at the one chokepoint every external writer shares: - updateWorkspaceFileContent gains an opt-out "syncLiveDoc" (default on) and, after the durable write, merges markdown writes into any open collaborative doc (best-effort; no-op when nobody has it open). Any current OR future writer is covered automatically. - persist.ts opts out (syncLiveDoc:false) — it IS the doc→markdown projection, so merging it back would be a persist→merge→persist self-loop. - create_file opts its empty shell out (real content arrives via a later write) so an open editor never flickers to empty on overwrite; threaded through writeWorkspaceFileByPath. - edit_content drops its now-redundant explicit merge call (the chokepoint handles it). - binary writers (image/video/audio/ffmpeg/download) are naturally excluded — the merge is gated to markdown, the only format the collaborative editor renders. Also bump the api-validation route baseline 994→996 to match the true route count already on this branch (pre-existing ratchet drift from an earlier merge; NOT added by this PR). * fix(collab-doc): defer setEditable out of the render phase (flushSync warning) The collab editability-reapply effect called editor.setEditable synchronously. In collab mode isEditable flips from readiness (synced + seeded), which is driven by a Yjs config.observe firing synchronously inside Y.applyUpdate — so the effect can run while React is mid-render. TipTap's React binding commits setEditable's transaction with flushSync, which throws "flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering." Defer the setEditable to a microtask (runs right after the current commit, before paint), guarding against a destroyed editor or a stale value before it fires. Only the collab path (this effect) hit the warning; the streaming/settle effect's setEditable calls run on the non-collab path where isEditable isn't driven by a mid-render Yjs observer.
1 parent de595f4 commit fd34853

8 files changed

Lines changed: 127 additions & 19 deletions

File tree

apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -745,7 +745,21 @@ export function LoadedRichMarkdownEditor({
745745
*/
746746
useEffect(() => {
747747
if (!editor || !collaborationEnabled) return
748-
if (editor.isEditable !== isEditable) editor.setEditable(isEditable)
748+
if (editor.isEditable === isEditable) return
749+
// Defer out of the render/commit phase. `isEditable` flips from collab readiness (synced + seeded),
750+
// which is driven by a Yjs `config.observe` firing synchronously inside `Y.applyUpdate` — so this
751+
// effect can run while React is mid-render. `setEditable` dispatches a TipTap transaction that the
752+
// React binding commits with `flushSync`, which throws ("cannot flush while rendering") in that
753+
// window. A microtask runs right after the current commit, before paint; re-check liveness/value
754+
// since either can change before it fires.
755+
let cancelled = false
756+
queueMicrotask(() => {
757+
if (cancelled || editor.isDestroyed) return
758+
if (editor.isEditable !== isEditable) editor.setEditable(isEditable)
759+
})
760+
return () => {
761+
cancelled = true
762+
}
749763
}, [editor, collaborationEnabled, isEditable])
750764

751765
/**

apps/sim/lib/collab-doc/persist.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,12 @@ export async function persistFileDoc(
3939
ydoc.destroy()
4040
}
4141

42-
await updateWorkspaceFileContent(workspaceId, fileId, userId, markdownBuffer)
42+
// `syncLiveDoc: false`: this write IS the projection of the live doc back to markdown, so merging it
43+
// back into that same doc would be a persist → merge → persist self-loop. The doc already holds this
44+
// exact content; peers are already converged through the relay.
45+
await updateWorkspaceFileContent(workspaceId, fileId, userId, markdownBuffer, undefined, {
46+
syncLiveDoc: false,
47+
})
4348

4449
// Cache the Yjs binary (tagged with the exact markdown just written) so a later cold room open loads
4550
// it directly instead of re-converting markdown → Yjs. Best-effort: the markdown IS the durable file,

apps/sim/lib/copilot/tools/server/files/create-file.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,9 @@ export const createFileServerTool: BaseServerTool<CreateFileArgs, CreateFileResu
6464
},
6565
buffer: emptyBuffer,
6666
inferredMimeType: contentType,
67+
// This writes only an empty shell; the real content arrives via a later edit_content write (which
68+
// does merge). Merging this empty write would briefly blank an already-open editor on overwrite.
69+
syncLiveDoc: false,
6770
})
6871

6972
logger.info('File created via create_file', {

apps/sim/lib/copilot/tools/server/files/edit-content.ts

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ import {
66
type ServerToolContext,
77
} from '@/lib/copilot/tools/server/base-tool'
88
import { isDocSandboxEnabled } from '@/lib/core/config/env-flags'
9-
import { mergeEditIntoLiveFileDoc } from '@/lib/realtime/notify'
109
import { updateWorkspaceFileContent } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
11-
import { isMarkdownFile } from '@/lib/uploads/utils/file-utils'
1210
import { getE2BDocFormat } from './doc-compile'
1311
import { buildEmbeddedImageRefWarning } from './embedded-image-refs'
1412
import { consumeLatestFileIntent } from './file-intent-store'
@@ -235,6 +233,9 @@ export const editContentServerTool: BaseServerTool<EditContentArgs, EditContentR
235233

236234
const fileBuffer = Buffer.from(finalContent, 'utf-8')
237235
assertServerToolNotAborted(context)
236+
// `updateWorkspaceFileContent` also streams this edit into any open collaborative editor as a live
237+
// CRDT merge (gated to markdown, best-effort) — the shared chokepoint every external write path
238+
// goes through — so a copilot edit shows up live instead of the file changing under the reader.
238239
await updateWorkspaceFileContent(
239240
workspaceId,
240241
intent.fileId,
@@ -243,16 +244,6 @@ export const editContentServerTool: BaseServerTool<EditContentArgs, EditContentR
243244
compiled.sourceMime
244245
)
245246

246-
// If a collaborator has this markdown file open, merge the edit into their live document so it
247-
// streams into the editor as a CRDT merge instead of the file changing under them. Best-effort
248-
// and gated to markdown, the only format the collaborative editor renders — so code/text/doc
249-
// edits don't pay the realtime round-trip. The durable write above is the source of truth; this
250-
// merges the edit into any live collaborative doc so open editors see it stream in (the relay
251-
// persists the doc server-side — the collaborative editor's own client autosave is disabled).
252-
if (isMarkdownFile(fileRecord)) {
253-
await mergeEditIntoLiveFileDoc(intent.fileId, finalContent)
254-
}
255-
256247
const verb =
257248
operation === 'append' ? 'appended to' : operation === 'update' ? 'updated' : 'patched'
258249
logger.info(`Workspace file ${verb} via copilot (edit_content)`, {

apps/sim/lib/copilot/vfs/resource-writer.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,12 @@ export async function writeWorkspaceFileByPath(args: {
164164
target: WorkspaceFileWriteTarget
165165
buffer: Buffer
166166
inferredMimeType: string
167+
/**
168+
* Forwarded to {@link updateWorkspaceFileContent} on an overwrite. Defaults to `true` (stream a
169+
* markdown overwrite into any open collaborative editor). Pass `false` for a write whose content is
170+
* only a placeholder — e.g. `create_file`'s empty shell, whose real content lands via a later write.
171+
*/
172+
syncLiveDoc?: boolean
167173
}): Promise<WorkspaceFileWriteResult> {
168174
const contentType = args.target.mimeType || args.inferredMimeType
169175
if (args.target.mode === 'overwrite') {
@@ -177,7 +183,8 @@ export async function writeWorkspaceFileByPath(args: {
177183
existing.id,
178184
args.userId,
179185
args.buffer,
180-
contentType || existing.type
186+
contentType || existing.type,
187+
{ syncLiveDoc: args.syncLiveDoc }
181188
)
182189

183190
return {

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import { canonicalWorkspaceFilePath, decodeVfsPathSegments } from '@/lib/copilot
2222
import { generateRequestId } from '@/lib/core/utils/request'
2323
import { generateRestoreName } from '@/lib/core/utils/restore-name'
2424
import type { DbOrTx } from '@/lib/db/types'
25-
import { notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
25+
import { mergeEditIntoLiveFileDoc, notifyWorkspaceFilesChanged } from '@/lib/realtime/notify'
2626
import { getServePathPrefix } from '@/lib/uploads'
2727
import {
2828
deleteFile,
@@ -32,6 +32,7 @@ import {
3232
uploadFile,
3333
} from '@/lib/uploads/core/storage-service'
3434
import { MAX_WORKSPACE_FILE_SIZE } from '@/lib/uploads/shared/types'
35+
import { isMarkdownFile } from '@/lib/uploads/utils/file-utils'
3536
import { getWorkspaceWithOwner } from '@/lib/workspaces/permissions/utils'
3637
import { isUuid, sanitizeFileName } from '@/executor/constants'
3738
import type { UserFile } from '@/executor/types'
@@ -1024,7 +1025,18 @@ export async function updateWorkspaceFileContent(
10241025
fileId: string,
10251026
userId: string,
10261027
content: Buffer,
1027-
contentType?: string
1028+
contentType?: string,
1029+
options?: {
1030+
/**
1031+
* Whether to stream this write into any open collaborative editor as a live CRDT merge. Defaults
1032+
* to `true`, so EVERY external write path (copilot tools, the file tool, the content route) reaches
1033+
* an open editor through this one chokepoint — no per-writer wiring to forget. Pass `false` only
1034+
* for a write that must NOT touch the live doc: the relay's own project-to-markdown persist (which
1035+
* would otherwise merge the doc back into itself in a loop), and empty-shell creates whose real
1036+
* content arrives via a subsequent write.
1037+
*/
1038+
syncLiveDoc?: boolean
1039+
}
10281040
): Promise<WorkspaceFileRecord> {
10291041
logger.info(`Updating workspace file content: ${fileId} for workspace ${workspaceId}`)
10301042

@@ -1143,6 +1155,18 @@ export async function updateWorkspaceFileContent(
11431155
await cleanupWorkspaceStorageObject(finalized.oldKey, 'version replacement')
11441156
}
11451157

1158+
// Stream this write into any open collaborative editor as a CRDT merge, so a copilot/tool edit
1159+
// shows up live instead of the file silently changing underneath the reader. Gated to markdown (the
1160+
// only format the collaborative editor renders) and best-effort (a no-op when nobody has the file
1161+
// open; never throws). This is the single chokepoint every external writer shares — the relay's own
1162+
// persist and empty-shell creates pass `syncLiveDoc: false` to stay out of it.
1163+
if (
1164+
options?.syncLiveDoc !== false &&
1165+
isMarkdownFile({ type: nextContentType, name: finalized.file.originalName })
1166+
) {
1167+
await mergeEditIntoLiveFileDoc(fileId, content.toString('utf-8'))
1168+
}
1169+
11461170
const pathPrefix = getServePathPrefix()
11471171
const currentFolderPath =
11481172
finalized.file.folderId === fileRecord.folderId ? fileRecord.folderPath : null

apps/sim/lib/uploads/contexts/workspace/workspace-file-storage-accounting.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const {
1212
mockHeadObject,
1313
mockIncrementStorageUsageForBillingContextInTx,
1414
mockMaybeNotifyStorageLimitForBillingContext,
15+
mockMergeEditIntoLiveFileDoc,
16+
mockNotifyWorkspaceFilesChanged,
1517
mockResolveStorageBillingContext,
1618
mockUploadFile,
1719
} = vi.hoisted(() => ({
@@ -22,10 +24,17 @@ const {
2224
mockHeadObject: vi.fn(),
2325
mockIncrementStorageUsageForBillingContextInTx: vi.fn(),
2426
mockMaybeNotifyStorageLimitForBillingContext: vi.fn(),
27+
mockMergeEditIntoLiveFileDoc: vi.fn(),
28+
mockNotifyWorkspaceFilesChanged: vi.fn(),
2529
mockResolveStorageBillingContext: vi.fn(),
2630
mockUploadFile: vi.fn(),
2731
}))
2832

33+
vi.mock('@/lib/realtime/notify', () => ({
34+
mergeEditIntoLiveFileDoc: mockMergeEditIntoLiveFileDoc,
35+
notifyWorkspaceFilesChanged: mockNotifyWorkspaceFilesChanged,
36+
}))
37+
2938
vi.mock('@/lib/billing/storage', () => ({
3039
decrementStorageUsageForBillingContextInTx: mockDecrementStorageUsageForBillingContextInTx,
3140
incrementStorageUsageForBillingContextInTx: mockIncrementStorageUsageForBillingContextInTx,
@@ -105,6 +114,8 @@ describe('workspace file metadata and storage accounting', () => {
105114
mockDecrementStorageUsageForBillingContextInTx.mockResolvedValue(undefined)
106115
mockMaybeNotifyStorageLimitForBillingContext.mockResolvedValue(undefined)
107116
mockDeleteFile.mockResolvedValue(undefined)
117+
mockMergeEditIntoLiveFileDoc.mockResolvedValue(undefined)
118+
mockNotifyWorkspaceFilesChanged.mockResolvedValue(undefined)
108119
})
109120

110121
it('cleans up a newly uploaded object when atomic metadata finalization rolls back', async () => {
@@ -328,4 +339,57 @@ describe('workspace file metadata and storage accounting', () => {
328339
expect(mockDeleteFile).toHaveBeenCalledTimes(1)
329340
expect(mockDeleteFile).toHaveBeenCalledWith({ key: replacementKey, context: 'workspace' })
330341
})
342+
343+
const MD_ROW = { ...FILE_ROW, originalName: 'note.md', contentType: 'text/markdown' }
344+
345+
it('streams a markdown overwrite into any open collaborative editor (the shared merge chokepoint)', async () => {
346+
const updatedFile = { ...MD_ROW, size: 12 }
347+
dbChainMockFns.limit.mockResolvedValueOnce([MD_ROW]).mockResolvedValueOnce([MD_ROW])
348+
dbChainMockFns.returning.mockResolvedValueOnce([updatedFile])
349+
mockUploadFile.mockResolvedValueOnce({ key: MD_ROW.key })
350+
351+
await updateWorkspaceFileContent(
352+
MD_ROW.workspaceId,
353+
MD_ROW.id,
354+
MD_ROW.userId,
355+
Buffer.from('# new content', 'utf-8')
356+
)
357+
358+
expect(mockMergeEditIntoLiveFileDoc).toHaveBeenCalledWith(MD_ROW.id, '# new content')
359+
})
360+
361+
it('does NOT merge when syncLiveDoc is false (the relay persist / empty-shell opt-out)', async () => {
362+
const updatedFile = { ...MD_ROW, size: 12 }
363+
dbChainMockFns.limit.mockResolvedValueOnce([MD_ROW]).mockResolvedValueOnce([MD_ROW])
364+
dbChainMockFns.returning.mockResolvedValueOnce([updatedFile])
365+
mockUploadFile.mockResolvedValueOnce({ key: MD_ROW.key })
366+
367+
await updateWorkspaceFileContent(
368+
MD_ROW.workspaceId,
369+
MD_ROW.id,
370+
MD_ROW.userId,
371+
Buffer.from('# new content', 'utf-8'),
372+
undefined,
373+
{ syncLiveDoc: false }
374+
)
375+
376+
expect(mockMergeEditIntoLiveFileDoc).not.toHaveBeenCalled()
377+
})
378+
379+
it('does NOT merge a non-markdown write (the collaborative editor only renders markdown)', async () => {
380+
const updatedFile = { ...FILE_ROW, size: 10 }
381+
dbChainMockFns.limit.mockResolvedValueOnce([FILE_ROW]).mockResolvedValueOnce([FILE_ROW])
382+
dbChainMockFns.returning.mockResolvedValueOnce([updatedFile])
383+
mockUploadFile.mockResolvedValueOnce({ key: FILE_ROW.key })
384+
385+
await updateWorkspaceFileContent(
386+
FILE_ROW.workspaceId,
387+
FILE_ROW.id,
388+
FILE_ROW.userId,
389+
Buffer.alloc(10),
390+
'application/octet-stream'
391+
)
392+
393+
expect(mockMergeEditIntoLiveFileDoc).not.toHaveBeenCalled()
394+
})
331395
})

scripts/check-api-validation-contracts.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@ const QUERY_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/queries')
99
const SELECTOR_HOOKS_DIR = path.join(ROOT, 'apps/sim/hooks/selectors')
1010

1111
const BASELINE = {
12-
totalRoutes: 994,
13-
zodRoutes: 994,
12+
totalRoutes: 996,
13+
zodRoutes: 996,
1414
nonZodRoutes: 0,
1515
} as const
1616

0 commit comments

Comments
 (0)