Skip to content

Commit dc1a853

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix(files): harden Markdown PDF export
1 parent aa07c60 commit dc1a853

14 files changed

Lines changed: 314 additions & 114 deletions

File tree

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

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ import {
4242
} from '@/handlers/file-doc'
4343
import { beginRoomPermissionRead, commitRoomPermission } from '@/middleware/permissions'
4444

45-
type Handler = (payload?: unknown) => Promise<void> | void
45+
type Handler = (...args: unknown[]) => Promise<void> | void
4646

4747
const ROOM_NAME = 'workspace-file-doc:file-1'
4848

@@ -368,6 +368,56 @@ describe('setupWorkspaceFileDocHandlers', () => {
368368
expect(mockFetchFileDocPersist).toHaveBeenCalled()
369369
})
370370

371+
it('acknowledges an export flush only after the latest live edit is persisted', async () => {
372+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server'))
373+
const { io } = createIo()
374+
const { handlers } = setup('socket-1', io)
375+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
376+
await flushMicrotasks()
377+
378+
const edit = new Y.Doc()
379+
edit.getText(FILE_DOC_FIELD).insert(0, 'latest edit')
380+
handlers[FILE_DOC_EVENTS.MESSAGE](
381+
frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) =>
382+
syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(edit))
383+
)
384+
)
385+
await flushMicrotasks()
386+
mockFetchFileDocPersist.mockClear()
387+
const acknowledge = vi.fn()
388+
389+
await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }, acknowledge)
390+
391+
expect(mockFetchFileDocPersist).toHaveBeenCalledTimes(1)
392+
expect(acknowledge).toHaveBeenCalledWith({ ok: true })
393+
})
394+
395+
it('rejects an export flush when the live document cannot be persisted', async () => {
396+
mockFetchFileDocSeed.mockResolvedValue(seedResult('# From server'))
397+
const { io } = createIo()
398+
const { handlers } = setup('socket-1', io)
399+
await handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 })
400+
await flushMicrotasks()
401+
402+
const edit = new Y.Doc()
403+
edit.getText(FILE_DOC_FIELD).insert(0, 'latest edit')
404+
handlers[FILE_DOC_EVENTS.MESSAGE](
405+
frame(FILE_DOC_MESSAGE_TYPE.SYNC, (e) =>
406+
syncProtocol.writeUpdate(e, Y.encodeStateAsUpdate(edit))
407+
)
408+
)
409+
await flushMicrotasks()
410+
mockFetchFileDocPersist.mockResolvedValueOnce({ status: 'conflict' })
411+
const acknowledge = vi.fn()
412+
413+
await handlers[FILE_DOC_EVENTS.FLUSH]({ fileId: 'file-1' }, acknowledge)
414+
415+
expect(acknowledge).toHaveBeenCalledWith({
416+
ok: false,
417+
error: 'Unable to save the latest document changes for export',
418+
})
419+
})
420+
371421
it('drops document frames and evicts once the editor loses write access mid-session', async () => {
372422
// The join-time check is not a standing right: a collaborator downgraded to `read`
373423
// (or removed) must stop landing durable edits on the socket they already hold.

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

Lines changed: 101 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import {
3131
FILE_DOC_SEED,
3232
FILE_DOC_TIMEOUTS,
3333
type FileDocPresenceUser,
34+
type FlushFileDocPayload,
35+
type FlushFileDocResult,
3436
type JoinFileDocPayload,
3537
type LeaveFileDocPayload,
3638
toFileDocBytes,
@@ -81,6 +83,16 @@ const PERSIST_MAX_WAIT_MS = 20_000
8183
const FINAL_VERSION_RETRIES = 2
8284
const FINAL_VERSION_RETRY_MS = 100
8385

86+
type PersistMode = 'debounced' | 'final' | 'requested'
87+
type PersistOutcome =
88+
| 'unchanged'
89+
| 'persisted'
90+
| 'missing'
91+
| 'deferred'
92+
| 'conflict'
93+
| 'deduplicated'
94+
| 'failed'
95+
8496
/** Cross-task merge lock. The TTL must exceed the whole critical section it guards — stream fold +
8597
* `fetchFileDocMerge` (bounded at `mergeRequestMs`) + the awaited publish — so the lock never expires
8698
* mid-merge and lets a second task race the same base; hence `mergeRequestMs` plus generous headroom.
@@ -260,27 +272,31 @@ function schedulePersist(name: string, room: FileDocRoom): void {
260272
room.persistTimer = setTimeout(() => {
261273
room.persistTimer = null
262274
room.persistDeadline = null
263-
void flushPersist(name, room, false)
275+
void flushPersist(name, room, 'debounced')
264276
}, delay)
265277
}
266278

267279
/**
268-
* Project the live doc to markdown and write it durably via the app. `final` (last collaborator
269-
* leaving) always writes; a debounced mid-edit flush first claims a best-effort cross-task dedup WINDOW
280+
* Project the live doc to markdown and write it durably via the app. A final or explicitly requested
281+
* flush always writes; a debounced mid-edit flush first claims a best-effort cross-task dedup WINDOW
270282
* (a TTL key that just expires, so at most ~one persist per window cluster-wide) so concurrent tasks
271-
* editing the same file don't each write a redundant blob version. Best-effort: never throws (a failure
272-
* is retried on the next debounce; the stream holds the state meanwhile).
283+
* editing the same file don't each write a redundant blob version. Returns an outcome so an export can
284+
* wait for durable success; background callers still treat failures as best-effort.
273285
*
274286
* Persists the AUTHORITATIVE shared state (the stream), not this task's local doc: a copilot merge — or
275287
* a peer's edit — published by another task may not be integrated into `room.doc` yet (and the stream
276288
* holds content even when THIS task's doc was never locally seeded), so a last-disconnect flush can't
277289
* clobber the durable file with a lagging projection. The local doc is captured SYNCHRONOUSLY as a
278-
* fallback before any await, so a `void flushPersist(name, room, true)` fired immediately before the
290+
* fallback before any await, so a `void flushPersist(name, room, 'final')` fired immediately before the
279291
* caller destroys `room.doc` never encodes a destroyed doc, and the disabled path stays authoritative.
280292
*/
281-
async function flushPersist(name: string, room: FileDocRoom, final: boolean): Promise<void> {
293+
async function flushPersist(
294+
name: string,
295+
room: FileDocRoom,
296+
mode: PersistMode
297+
): Promise<PersistOutcome> {
282298
// Never project a doc no user actually edited back over the file (see {@link FileDocRoom.edited}).
283-
if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return
299+
if (!room.edited || !room.workspaceId || !room.lastEditorUserId) return 'unchanged'
284300
const store = getFileDocStore()
285301
const workspaceId = room.workspaceId
286302
const userId = room.lastEditorUserId
@@ -290,7 +306,10 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
290306

291307
// Capture the AUTHORITATIVE doc state: the shared stream when enabled (a copilot merge or a peer's
292308
// edit published by another task may not be integrated into THIS task's `room.doc` yet), else the
293-
// local snapshot. Re-read each attempt so a post-reconcile retry projects the converged state.
309+
// local snapshot. A requested export flush merges both CRDT snapshots: the socket's immediately
310+
// preceding edit can still be in the stream publisher's fire-and-forget queue, while a peer edit can
311+
// already be in the stream but not this task's doc. The CRDT union covers both without another save
312+
// path or waiting on the normal debounce.
294313
const captureState = async (): Promise<Uint8Array | null> => {
295314
if (!store.enabled) {
296315
// Single-pod: re-read the live doc so a post-reconcile retry projects the CONVERGED state, not the
@@ -302,12 +321,23 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
302321
: localState
303322
}
304323
try {
305-
return (await store.getStreamState(name)) ?? localState
324+
const sharedState = await store.getStreamState(name)
325+
if (mode !== 'requested' || !sharedState || !localState) return sharedState ?? localState
326+
327+
const merged = new Y.Doc()
328+
try {
329+
Y.applyUpdate(merged, sharedState)
330+
Y.applyUpdate(merged, localState)
331+
return Y.encodeStateAsUpdate(merged)
332+
} finally {
333+
merged.destroy()
334+
}
306335
} catch (streamError) {
307336
// A transient Redis read must NOT drop the write when we already hold a valid local snapshot —
308-
// else the last-disconnect flush loses the session's edits as the room is torn down. But once a
309-
// reconcile has run, `localState` is NULLED (it predates the merged-in out-of-band edit), so a
310-
// failed read then correctly THROWS and aborts rather than clobbering with the stale snapshot.
337+
// else the last-disconnect flush loses the session's edits as the room is torn down. An explicit
338+
// export flush can safely fail and retry, so do not risk omitting a peer edit when the shared state
339+
// is temporarily unavailable.
340+
if (mode === 'requested') throw streamError
311341
if (!localState) throw streamError
312342
logger.warn(`Stream state unavailable for file ${room.fileId}; persisting local snapshot`, {
313343
error: getErrorMessage(streamError),
@@ -327,8 +357,11 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
327357
}
328358

329359
try {
330-
if (!final && !(await store.tryClaimPersistWindow(name, FILE_DOC_TIMEOUTS.persistRequestMs)))
331-
return
360+
if (
361+
mode === 'debounced' &&
362+
!(await store.tryClaimPersistWindow(name, FILE_DOC_TIMEOUTS.persistRequestMs))
363+
)
364+
return 'deduplicated'
332365

333366
// The If-Match token: the durable content version the live doc is synced to.
334367
let ifMatch = await currentVersion()
@@ -338,7 +371,7 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
338371
// unset version never appears, and the flush must not stall teardown.
339372
for (
340373
let i = 0;
341-
ifMatch === undefined && final && store.enabled && i < FINAL_VERSION_RETRIES;
374+
ifMatch === undefined && mode !== 'debounced' && store.enabled && i < FINAL_VERSION_RETRIES;
342375
i++
343376
) {
344377
await sleep(FINAL_VERSION_RETRY_MS)
@@ -349,19 +382,24 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
349382
// still at the version the live doc synced from, so a projection can never silently clobber an
350383
// out-of-band edit. A single attempt — on conflict we STOP rather than retry (see below).
351384
const docState = await captureState()
352-
if (!docState) return // nothing seeded/authoritative to persist yet
385+
if (!docState) return 'unchanged' // nothing seeded/authoritative to persist yet
386+
// Make an acknowledged multi-replica flush a real snapshot handshake: the normal keystroke publish
387+
// is fire-and-forget, so append the converged snapshot and await Redis before updating the durable
388+
// blob. If Redis is unavailable, fail the export instead of acknowledging state that a later relay
389+
// persist could overwrite from an incomplete stream.
390+
if (mode === 'requested' && store.enabled) await store.publishAndWait(name, docState)
353391
const result = await fetchFileDocPersist(workspaceId, room.fileId, userId, docState, ifMatch)
354-
if (result.status === 'missing') return // the file was deleted; nothing to write
392+
if (result.status === 'missing') return 'missing' // the file was deleted; nothing to write
355393
if (result.status === 'deferred') {
356394
// No version token available (momentarily — a Redis blip on a peer-seeded task). Leave the edits in
357395
// the stream; a later persist writes them once the version is re-established.
358396
logger.warn(`Persist deferred for file ${room.fileId} (no synced version available yet)`)
359-
return
397+
return 'deferred'
360398
}
361399
if (result.status === 'persisted') {
362400
room.syncedVersion = Math.max(room.syncedVersion ?? 0, result.version)
363401
void store.setSyncedVersion(name, result.version)
364-
return
402+
return 'persisted'
365403
}
366404
// status === 'conflict': the durable file advanced out-of-band since our If-Match token. We do NOT
367405
// re-persist against the current stream: an external write commits durable BEFORE its chokepoint merge
@@ -375,8 +413,10 @@ async function flushPersist(name: string, room: FileDocRoom, final: boolean): Pr
375413
logger.warn(
376414
`Persist conflict for file ${room.fileId}; durable content advanced out-of-band, left authoritative`
377415
)
416+
return 'conflict'
378417
} catch (error) {
379418
logger.warn(`Persist failed for file ${room.fileId}`, { error: getErrorMessage(error) })
419+
return 'failed'
380420
}
381421
}
382422

@@ -446,7 +486,7 @@ function destroyRoomIfIdle(name: string) {
446486
}
447487
// Final durable flush BEFORE teardown — `flushPersist` encodes the doc synchronously (before the
448488
// destroy below) and awaits the write in the background. Best-effort; never throws.
449-
void flushPersist(name, room, true)
489+
void flushPersist(name, room, 'final')
450490
getFileDocStore().detachRoom(name)
451491
room.awareness.destroy()
452492
room.doc.destroy()
@@ -461,9 +501,9 @@ function destroyRoomIfIdle(name: string) {
461501
* process is exiting); only their durable state is secured.
462502
*/
463503
export async function flushAllFileDocRooms(): Promise<void> {
464-
const flushes: Promise<void>[] = []
504+
const flushes: Promise<PersistOutcome>[] = []
465505
for (const [name, room] of fileDocRooms) {
466-
if (room.edited) flushes.push(flushPersist(name, room, true))
506+
if (room.edited) flushes.push(flushPersist(name, room, 'final'))
467507
}
468508
await Promise.all(flushes)
469509
}
@@ -1229,6 +1269,44 @@ export function setupWorkspaceFileDocHandlers(
12291269
}
12301270
})
12311271

1272+
socket.on(
1273+
FILE_DOC_EVENTS.FLUSH,
1274+
async (payload: FlushFileDocPayload, acknowledge?: (result: FlushFileDocResult) => void) => {
1275+
if (typeof acknowledge !== 'function') return
1276+
if (!payload || typeof payload.fileId !== 'string' || payload.fileId.length === 0) {
1277+
acknowledge({ ok: false, error: 'Invalid file document flush request' })
1278+
return
1279+
}
1280+
1281+
const name = socketToRoomName.get(socket.id)
1282+
const requestedName = roomName(fileDocRoom(payload.fileId))
1283+
// A file with no live editor on this socket has no pending client edits to flush; its durable
1284+
// blob is already the export source. This also keeps cold-load and read-only exports immediate.
1285+
if (name !== requestedName) {
1286+
acknowledge({ ok: true })
1287+
return
1288+
}
1289+
1290+
const room = fileDocRooms.get(name)
1291+
if (!room || !isFileDocWriteAllowed(socket, io, name)) {
1292+
acknowledge({ ok: false, error: 'Unable to prepare the current document for export' })
1293+
return
1294+
}
1295+
1296+
// Socket.IO preserves event order on one connection, so all Yjs update frames emitted before
1297+
// this request have already been applied. Replace the pending debounce with this awaited write.
1298+
if (room.persistTimer) clearTimeout(room.persistTimer)
1299+
room.persistTimer = null
1300+
room.persistDeadline = null
1301+
const outcome = await flushPersist(name, room, 'requested')
1302+
if (outcome === 'persisted' || outcome === 'unchanged' || outcome === 'missing') {
1303+
acknowledge({ ok: true })
1304+
return
1305+
}
1306+
acknowledge({ ok: false, error: 'Unable to save the latest document changes for export' })
1307+
}
1308+
)
1309+
12321310
socket.on(FILE_DOC_EVENTS.MESSAGE, (data: unknown) => handleMessage(socket, io, data))
12331311

12341312
socket.on(FILE_DOC_EVENTS.LEAVE, (payload?: LeaveFileDocPayload) => {

apps/sim/app/api/files/export/[id]/markdown-pdf.test.ts

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ describe('Markdown PDF rendering', () => {
4646
4747
Smart quotes “work” and Greek Ω stays readable.
4848
49+
Common emoji stay readable too: 🚀 😀
50+
4951
中文排版应该清晰易读。 العربية يجب أن تكون متصلة ومقروءة. हिन्दी पाठ स्पष्ट और पठनीय होना चाहिए। עברית צריכה להיות ברורה וקריאה.
5052
5153
- First item
@@ -85,6 +87,9 @@ ${repeatedParagraphs}`
8587
// PDF extractors expose visually positioned Indic vowel marks before their base character.
8688
expect(text).toMatch(/[\u0900-\u097f]{4,}/u)
8789
expect(text).toContain('עברית')
90+
expect(text).toContain('[emoji U+1F680]')
91+
expect(text).toContain('[emoji U+1F600]')
92+
expect(text).not.toContain('�')
8893
expect(text).not.toContain('Image: Embedded image')
8994

9095
const parsed = await getDocument({ data: new Uint8Array(buffer), disableWorker: true }).promise
@@ -103,7 +108,7 @@ ${repeatedParagraphs}`
103108
}
104109
})
105110

106-
it('keeps long table rows together and repeats the header across table pages', async () => {
111+
it('lets a long table paginate without moving the whole table to a later page', async () => {
107112
const rows = Array.from(
108113
{ length: 90 },
109114
(_, index) =>
@@ -117,7 +122,21 @@ ${repeatedParagraphs}`
117122
const pages = await pdfPagesText(buffer)
118123
const tablePages = pages.filter((page) => page.includes('Row '))
119124
expect(tablePages.length).toBeGreaterThan(1)
120-
expect(tablePages.every((page) => page.includes('Name') && page.includes('Value'))).toBe(true)
125+
expect(pages[0]).toContain('Row 1')
126+
expect(pages.join(' ')).toContain('Row 90')
127+
})
128+
129+
it('allows a table row taller than a page to wrap without losing its content', async () => {
130+
const cell = `ROW-START ${'wrapping table content '.repeat(900)} ROW-END`
131+
const buffer = await renderMarkdownPdf({
132+
markdown: `| Name | Value |\n| --- | --- |\n| Tall row | ${cell} |`,
133+
title: 'Tall table row',
134+
})
135+
136+
const pages = await pdfPagesText(buffer)
137+
expect(pages.length).toBeGreaterThan(1)
138+
expect(pages.join(' ')).toContain('ROW-START')
139+
expect(pages.join(' ')).toContain('ROW-END')
121140
})
122141

123142
it('renders a table that contains only a header', async () => {

0 commit comments

Comments
 (0)