Skip to content

Commit 9674e20

Browse files
icecrasher321claude
andcommitted
fix(files): stop the collaborative editor rewriting and reflowing a document on open
Opening a file rewrote it. Binding an editor to a seeded document emits a Yjs update of its own — ProseMirror appends an empty paragraph to any doc that does not end in one — which the relay saw as a real edit and persisted. Every open therefore uploaded the file under a FRESH storage key and deleted the old one, 404ing the page's own in-flight content read, bumping "Last Updated" just from viewing, and churning a blob per open. Worse, a trailing blank line cannot serialize, so the file never recorded that paragraph and nothing reconciled the two: each client that seeded without seeing another's contribution stacked one more. A real document reached 18 against the placeholder's 1 — measured as the pane growing several hundred pixels the instant the live editor took over. - Seed and merge through the editor's own normal form (`editorNormalForm`), so binding is a no-op and `canonicalizeYDoc` collapses an accumulated run back to one. Placed at the collab boundary, not in `parseMarkdownToDoc`: only the CRDT has to agree with the editor — every other consumer of the parse renders through a real editor that normalizes itself. - Skip a persist whose projection already matches the durable bytes. Byte length is the free reject, so the compare read only happens when a no-op write is actually on the table. - Revoke collaborative readiness on a fatal join. The sticky `syncedOnce` latch outlived the document: after a readiness timeout the provider drops `synced` so the gate closes, but the latch re-opened it on the offline fallback's seed flag — handing back an EDITABLE editor on a document the provider had abandoned, with client autosave gated off because collaboration is nominally on. Keystrokes went nowhere and vanished on reload, with no error shown. - Recover from a superseded storage key instead of stranding the reader: a 404 re-resolves the file record, so the read re-keys onto the current object. And do not focus-refetch durable bytes while the relay owns durability. - Prefetch the workspace file list in the layout, where the sidebar already reads it. `HydrationBoundary` defers an already-seen query to an effect that SSR never runs, so a page-level prefetch of that key could not reach the server render — the file route rendered a spinner and disagreed with the client about the header's markup (a hydration mismatch). - Load the document font with `display: block`. A swap repaints prose in metric-adjusted Arial first, so paragraphs re-wrap when the real face lands. Also: a detail-route `loading.tsx` (the segment was inheriting the list chrome), `normalize.ts` renamed to `field.ts` now that it holds only the field constant, and the duplicate `COLLAB_DOC_FIELD` in the streaming path folded into it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2da8015 commit 9674e20

25 files changed

Lines changed: 1205 additions & 361 deletions

apps/sim/app/_styles/fonts/season/season.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,26 @@ import localFont from 'next/font/local'
33
/**
44
* Season Sans variable font configuration
55
* Uses variable font file to support any weight from 300-800
6+
*
7+
* `display: 'block'`, not `swap`: this is the document font, so a swap is not a cosmetic change of
8+
* typeface — the fallback's glyph advances differ, so paragraphs re-wrap and everything below them
9+
* moves. In long-form prose (the Files editor) that reads as the line and paragraph spacing visibly
10+
* correcting itself a beat after the text appears, on every hard refresh (a normal reload serves the
11+
* font from cache and never swaps). `swap` is the setting that says "painting the wrong font first is
12+
* fine"; for a brand face it is not.
13+
*
14+
* The block period costs nothing here because delivery is already optimal: `preload` emits a
15+
* `Link: rel=preload` RESPONSE header, so the fetch starts before the HTML is parsed, and the file is
16+
* one same-origin, immutably-cached 87KB woff2. The metric-adjusted Arial below stays as the safety
17+
* net for the >3s tail, where the browser gives up blocking and swaps — i.e. the worst case is
18+
* today's behavior, not a regression.
619
*/
720
export const season = localFont({
821
src: [
922
// Variable font - supports all weights from 300 to 800
1023
{ path: './SeasonSansUprightsVF.woff2', weight: '300 800', style: 'normal' },
1124
],
12-
display: 'swap',
25+
display: 'block',
1326
preload: true,
1427
variable: '--font-season',
1528
fallback: ['system-ui', 'Segoe UI', 'Roboto', 'Helvetica Neue', 'Arial', 'Noto Sans'],
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
'use client'
2+
3+
import { File as FileIcon } from '@sim/emcn/icons'
4+
import { noop } from '@sim/utils/helpers'
5+
import {
6+
type BreadcrumbItem,
7+
ResourceChromeFallback,
8+
} from '@/app/workspace/[workspaceId]/components'
9+
import { FOLDERED_RESOURCE_HEADERS } from '@/app/workspace/[workspaceId]/components/folders/foldered-resources'
10+
11+
const FILES_HEADER = FOLDERED_RESOURCE_HEADERS.file
12+
13+
/**
14+
* Transcribes the trail the loaded page shows while its record resolves (`loadingBreadcrumbs` in
15+
* `files.tsx`): the root crumb plus a terminal `…`, with no icon on the leaf — so the fallback and
16+
* the page paint the same two crumbs and only the label changes.
17+
*/
18+
const BREADCRUMBS: BreadcrumbItem[] = [
19+
{ label: FILES_HEADER.rootLabel, icon: FileIcon, onClick: noop },
20+
{ label: '…', terminal: true },
21+
]
22+
23+
/**
24+
* Fallback for the file DETAIL route. Without it the segment inherits the Files list fallback, which
25+
* paints an options bar and a table header row that a document page does not have — chrome that has
26+
* to be torn down a frame later. A detail page is header + body, so this is the header alone.
27+
*
28+
* Header actions are deliberately omitted: they are a function of the open file (a previewable
29+
* non-markdown file gets a mode toggle, an editable one gets Share/Delete), which is exactly what is
30+
* not yet known here. Chips appearing beside the title reads as content arriving; chips appearing
31+
* and then changing reads as a glitch.
32+
*/
33+
export default function FilesFileLoading() {
34+
return <ResourceChromeFallback icon={FileIcon} breadcrumbs={BREADCRUMBS} />
35+
}
Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,48 @@
11
import { Suspense } from 'react'
2+
import { dehydrate, HydrationBoundary } from '@tanstack/react-query'
23
import type { Metadata } from 'next'
4+
import { getSession } from '@/lib/auth'
5+
import { getQueryClient } from '@/app/_shell/providers/get-query-client'
6+
import FilesFileLoading from '@/app/workspace/[workspaceId]/files/[fileId]/loading'
37
import { Files } from '@/app/workspace/[workspaceId]/files/files'
4-
import FilesLoading from '@/app/workspace/[workspaceId]/files/loading'
8+
import { prefetchFilesBrowser } from '@/app/workspace/[workspaceId]/files/prefetch'
59

610
export const metadata: Metadata = {
711
title: 'Files',
812
robots: { index: false },
913
}
1014

11-
export default function FilesFilePage() {
15+
/**
16+
* File detail entry. `Files` resolves the open file out of the workspace file LIST, so this route
17+
* needs the same prefetch its sibling list page does — without it the server can only ever render
18+
* the "resolving the record" spinner, and the real header (breadcrumbs, actions) has to pop in a
19+
* frame later on the client.
20+
*
21+
* It also removes a whole class of hydration mismatch: which branch `Files` renders is decided by
22+
* whether that list is in the cache, so a server render without it and a client render with it
23+
* disagree on the header's markup (a static `…` crumb vs. the file's dropdown crumb). Prefetching
24+
* here makes both sides read the same cache and pick the same branch by construction.
25+
*
26+
* `Files` reads URL query params via nuqs (`useSearchParams` internally), so it must sit under a
27+
* Suspense boundary; the fallback is the detail chrome, matching the route's own `loading.tsx`.
28+
*/
29+
export default async function FilesFilePage({
30+
params,
31+
}: {
32+
params: Promise<{ workspaceId: string; fileId: string }>
33+
}) {
34+
const [{ workspaceId }, session] = await Promise.all([params, getSession()])
35+
36+
const queryClient = getQueryClient()
37+
if (session?.user?.id) {
38+
await prefetchFilesBrowser(queryClient, workspaceId, session.user.id)
39+
}
40+
1241
return (
13-
<Suspense fallback={<FilesLoading />}>
14-
<Files />
15-
</Suspense>
42+
<HydrationBoundary state={dehydrate(queryClient)}>
43+
<Suspense fallback={<FilesFileLoading />}>
44+
<Files />
45+
</Suspense>
46+
</HydrationBoundary>
1647
)
1748
}

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,9 @@ import type { Editor } from '@tiptap/core'
22
import { Node as PMNode } from '@tiptap/pm/model'
33
import { initProseMirrorDoc, updateYFragment, ySyncPluginKey } from '@tiptap/y-tiptap'
44
import * as Y from 'yjs'
5+
import { COLLAB_DOC_FIELD } from '@/lib/collab-doc/field'
56
import { parseMarkdownToDoc } from '../markdown-parse'
67

7-
/** The Yjs fragment name TipTap's Collaboration extension binds to (its default `field`). */
8-
const COLLAB_DOC_FIELD = 'default'
9-
108
/**
119
* Transaction origin for agent-streamed writes into a live collaborative doc. It is deliberately NOT
1210
* the `ySyncPluginKey` origin that local user edits use, so the Collaboration UndoManager — which

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ describe('collab streaming integration — moving pieces', () => {
164164
reopened.destroy()
165165
})
166166

167-
it('EMPTY-COLLAPSE ON THE STREAM PATH: an agent body with a huge blank run does not strand empties', () => {
167+
it('EMPTY-BOUND ON THE STREAM PATH: an agent body with a huge blank run does not strand empties', () => {
168168
const A = makeCollabEditor()
169169
A.editor.commands.setContent(parseMarkdownToDoc('# Title\n\nintro'), { contentType: 'json' })
170170
const session = beginAgentStream(A.editor)!
@@ -173,9 +173,11 @@ describe('collab streaming integration — moving pieces', () => {
173173
endAgentStream(session)
174174

175175
console.log(
176-
`\n[STREAM-COLLAPSE] text=${JSON.stringify(A.editor.state.doc.textContent)} empty=${emptyParas(A.editor)}`
176+
`\n[STREAM-BOUND] text=${JSON.stringify(A.editor.state.doc.textContent)} empty=${emptyParas(A.editor)}`
177177
)
178-
expect(emptyParas(A.editor)).toBe(0) // collapse protects the live streaming path, not just static open
178+
// ~200 blank paragraphs' worth of run arrives; the parse bound caps what reaches the live doc, so the
179+
// streaming path is protected exactly like a static open — no unbounded node explosion in the CRDT.
180+
expect(emptyParas(A.editor)).toBe(20)
179181
expect(A.editor.state.doc.textContent).toContain('tail paragraph')
180182
})
181183

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

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,33 +4,48 @@
44
import { describe, expect, it } from 'vitest'
55
import { type CollabReadinessInputs, nextCollabReadiness } from './readiness'
66

7+
/** An observation, with the healthy defaults filled in so each case states only what it exercises. */
8+
const at = (input: Partial<CollabReadinessInputs>): CollabReadinessInputs => ({
9+
synced: false,
10+
seeded: false,
11+
offlineSeed: false,
12+
fatal: false,
13+
...input,
14+
})
15+
716
/** Drive a sequence of observations through the latch, returning the readiness at each step. */
8-
function run(steps: CollabReadinessInputs[]): boolean[] {
17+
function run(steps: Partial<CollabReadinessInputs>[]): boolean[] {
918
let syncedOnce = false
10-
return steps.map((input) => {
11-
const next = nextCollabReadiness(syncedOnce, input)
19+
return steps.map((step) => {
20+
const next = nextCollabReadiness(syncedOnce, at(step))
1221
syncedOnce = next.syncedOnce
1322
return next.ready
1423
})
1524
}
1625

1726
describe('nextCollabReadiness', () => {
1827
it('is not ready before syncing or seeding', () => {
19-
const { syncedOnce, ready } = nextCollabReadiness(false, {
20-
synced: false,
21-
seeded: false,
22-
offlineSeed: false,
23-
})
28+
const { syncedOnce, ready } = nextCollabReadiness(
29+
false,
30+
at({
31+
synced: false,
32+
seeded: false,
33+
offlineSeed: false,
34+
})
35+
)
2436
expect(syncedOnce).toBe(false)
2537
expect(ready).toBe(false)
2638
})
2739

2840
it('is not ready when synced but not yet seeded', () => {
29-
const { syncedOnce, ready } = nextCollabReadiness(false, {
30-
synced: true,
31-
seeded: false,
32-
offlineSeed: false,
33-
})
41+
const { syncedOnce, ready } = nextCollabReadiness(
42+
false,
43+
at({
44+
synced: true,
45+
seeded: false,
46+
offlineSeed: false,
47+
})
48+
)
3449
expect(syncedOnce).toBe(true) // latched
3550
expect(ready).toBe(false) // waits for the seed
3651
})
@@ -50,11 +65,14 @@ describe('nextCollabReadiness', () => {
5065
it('opens even if the seed lands before we ever observed synced (server seed proves a sync)', () => {
5166
// If the flap beat our first observation, the seed flag alone (not the offline fallback) proves a
5267
// completed sync happened.
53-
const { syncedOnce, ready } = nextCollabReadiness(false, {
54-
synced: false,
55-
seeded: true,
56-
offlineSeed: false,
57-
})
68+
const { syncedOnce, ready } = nextCollabReadiness(
69+
false,
70+
at({
71+
synced: false,
72+
seeded: true,
73+
offlineSeed: false,
74+
})
75+
)
5876
expect(syncedOnce).toBe(true)
5977
expect(ready).toBe(true)
6078
})
@@ -74,4 +92,33 @@ describe('nextCollabReadiness', () => {
7492
])
7593
expect(readiness).toEqual([true, true])
7694
})
95+
/**
96+
* The reported bug. A brand-new file syncs EMPTY (latching `syncedOnce`), its server seed never
97+
* lands, and the readiness deadline fires: the provider goes fatal and drops `synced` precisely so
98+
* this gate closes. The offline fallback then seeds locally — and the sticky latch used to re-open
99+
* the gate on that, handing back an editable editor bound to a document the provider had abandoned.
100+
* Every keystroke was dropped (the provider ignores frames and never rejoins) and client autosave
101+
* stayed off (collaboration is nominally on), so the edits vanished on reload with no error shown.
102+
*/
103+
it('stays read-only after the readiness deadline goes fatal, even though a sync was latched', () => {
104+
const readiness = run([
105+
{ synced: false },
106+
{ synced: true }, // initial EMPTY sync — latches syncedOnce
107+
{ synced: false, fatal: true }, // deadline: provider drops synced and gives up
108+
{ seeded: true, offlineSeed: true, fatal: true }, // fallback seeds locally
109+
])
110+
expect(readiness).toEqual([false, false, false, false])
111+
})
112+
113+
/**
114+
* The same revocation on an ALREADY-ready doc: access is withdrawn mid-session, the provider goes
115+
* fatal, and readiness must be taken back rather than left latched open.
116+
*/
117+
it('revokes readiness when a live document turns fatal', () => {
118+
const readiness = run([
119+
{ synced: true, seeded: true }, // ready
120+
{ synced: false, seeded: true, fatal: true }, // access revoked mid-session
121+
])
122+
expect(readiness).toEqual([true, false])
123+
})
77124
})

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,32 @@ export interface CollabReadinessInputs {
2222
seeded: boolean
2323
/** Whether the seed flag was set by the offline fallback (no server sync) rather than the server. */
2424
offlineSeed: boolean
25+
/**
26+
* Whether the provider has GIVEN UP on this document — a non-retryable rejection, an access
27+
* revocation, or the readiness deadline lapsing. A fatal provider ignores every inbound frame and
28+
* never rejoins, so nothing typed after this point reaches the server.
29+
*/
30+
fatal: boolean
2531
}
2632

2733
/**
2834
* Pure transition for the readiness latch. `syncedOnce` is the sticky prior state — pass the returned
2935
* `syncedOnce` back in on the next call. `ready` is whether the doc is synced-and-seeded.
36+
*
37+
* `fatal` overrides the latch, and that override is the whole reason it is an input. The latch is
38+
* sticky on purpose, but stickiness must not outlive the document: a doc that syncs empty and never
39+
* receives its server seed trips the readiness deadline, and the provider answers by dropping `synced`
40+
* so this gate closes. The latch ignored that — `syncedOnce` was already set by the empty sync — so the
41+
* offline fallback's seed flag re-opened the gate and handed back an EDITABLE editor on a document the
42+
* provider had already abandoned. Nothing typed into it could persist: the provider drops every frame
43+
* and never rejoins, and the client's own autosave stays gated off because collaboration is nominally
44+
* on. The user types, sees no error, and loses the edits on reload. Revoking readiness on `fatal` is
45+
* what makes the fallback what it is documented to be — a READ-ONLY view of the stored content.
3046
*/
3147
export function nextCollabReadiness(
3248
syncedOnce: boolean,
3349
input: CollabReadinessInputs
3450
): { syncedOnce: boolean; ready: boolean } {
3551
const next = syncedOnce || input.synced || (input.seeded && !input.offlineSeed)
36-
return { syncedOnce: next, ready: next && input.seeded }
52+
return { syncedOnce: next, ready: next && input.seeded && !input.fatal }
3753
}

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

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,10 +173,11 @@ function stripEmptyListItemLines(markdown: string): string {
173173
* round-trip ({@link stripEmptyListItemLines}), restores callout markers the serializer
174174
* backslash-escapes (`> \[!NOTE\]` → `> [!NOTE]`), and collapses trailing blank lines to a single
175175
* newline. Interior blank runs are NOT collapsed here — blank lines inside a fenced code block (or a
176-
* verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. Spurious
177-
* interior blank runs between top-level blocks are removed upstream instead, by
178-
* {@link parseMarkdownToDoc} stripping empty paragraphs, so a doc that has been through the editor
179-
* never serializes with an interior blank run outside code in the first place. The table serializer's
176+
* verbatim raw-markdown-snippet) are significant, and a global collapse would corrupt them. An interior
177+
* run between top-level blocks is significant too: it is how an empty paragraph is written, and
178+
* {@link parseMarkdownToDoc} reads exactly the count back out, so collapsing it here would delete the
179+
* document's spacing. Only the TRAILING run is collapsed — it can carry no paragraph (see
180+
* `clampEmptyParagraphs`) and would otherwise churn the file on every save. The table serializer's
180181
* spurious surrounding blank lines are trimmed at the source (PipeSafeTable), so no global
181182
* leading-newline strip is needed here — avoiding clobbering content that legitimately begins with
182183
* whitespace.

0 commit comments

Comments
 (0)