From 9b5256718c2117511f0253a656bb8cff7410b92a Mon Sep 17 00:00:00 2001 From: haoyuren <13851610112@163.com> Date: Wed, 18 Mar 2026 08:06:32 +0000 Subject: Fix OT sync corruption: match Overleaf ShareJS ack/echo handling The server broadcasts otUpdateApplied (with ops) to ALL clients including the sender. Our bridge was treating its own echoed ops as remote ops and re-applying them, causing text duplication (e.g. "simulatorimulator"). Rewrite OT handling to match Overleaf's ShareJS _onMessage pattern: - ACK = no ops OR meta.source matches our publicId (own echo) - REMOTE = ops from a different source - ACK path calls onAck() without re-applying ops - OtClient silently drops duplicate acks in synchronized state - OtClient drops stale remote ops (version < current) - Remove pendingEchos counter in favor of meta.source detection Also: refresh MCP comment contexts on new-comment/delete-thread events, add Overleaf reference repo to .gitignore. Co-Authored-By: Claude Opus 4.6 --- .gitignore | 1 + src/main/fileSyncBridge.ts | 79 +++++++++++++++++++++++++++++++++++----------- src/main/index.ts | 32 +++++++++++++++++-- src/main/otClient.ts | 36 +++++++++++++++++++-- 4 files changed, 125 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index 606d0fa..eee0821 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ dist/ *.exe *.AppImage .DS_Store +reference/ diff --git a/src/main/fileSyncBridge.ts b/src/main/fileSyncBridge.ts index c9397f0..707d3d8 100644 --- a/src/main/fileSyncBridge.ts +++ b/src/main/fileSyncBridge.ts @@ -268,30 +268,60 @@ export class FileSyncBridge { } // ── OT update handler ───────────────────────────────────── + // + // Modeled after Overleaf's ShareJS _onMessage (vendor/libs/sharejs.js): + // + // ACK = msg.op === undefined (explicit ack, no ops) + // | msg.meta.source === ourId (echo of our own ops) + // + // REMOTE = msg.op && msg.meta.source !== ourId + // + // The ACK path does NOT re-apply ops (they were applied optimistically). + // The REMOTE path transforms against inflight/buffered ops before applying. + // Stale messages (version < ours) are silently dropped by OtClient. private handleOtUpdate(args: unknown[]): void { - const update = args[0] as { doc?: string; op?: OtOp[]; v?: number } | undefined + const update = args[0] as { doc?: string; op?: OtOp[]; v?: number; meta?: { source?: string } } | undefined if (!update?.doc) return const docId = update.doc - - // For non-editor docs, process remote ops through bridge's OtClient - if (!this.editorDocs.has(docId) && update.op && update.v !== undefined) { - const otClient = this.otClients.get(docId) - if (otClient) { - otClient.onRemoteOps(update.op, update.v) - } + const relPath = this.docPathMap[docId] || docId + + // Skip editor docs — renderer handles their OT + if (this.editorDocs.has(docId)) return + + const otClient = this.otClients.get(docId) + if (!otClient) return + + const isOwnSource = this.isOwnSource(update.meta?.source) + + bridgeLog(`[FileSyncBridge] handleOtUpdate: ${relPath} v=${update.v} hasOp=${!!update.op} own=${isOwnSource} state=${otClient.stateName} ver=${otClient.version}`) + + if (!update.op || isOwnSource) { + // ACK — either: + // 1. No ops (explicit ack from server to sender) + // 2. Echoed ops from our own source (server broadcast to all clients) + // In both cases: acknowledge the inflight op. Do NOT re-apply ops. + // OtClient.onAck() silently drops duplicate acks (synchronized state). + bridgeLog(`[FileSyncBridge] handleOtUpdate: ACK for ${relPath} (${!update.op ? 'no-op' : 'own-echo'}), state=${otClient.stateName}`) + otClient.onAck() + bridgeLog(`[FileSyncBridge] handleOtUpdate: ACK done → state=${otClient.stateName} ver=${otClient.version}`) + } else if (update.op && update.v !== undefined) { + // REMOTE — genuine ops from another client + bridgeLog(`[FileSyncBridge] handleOtUpdate: REMOTE ops for ${relPath} from ${update.meta?.source || 'unknown'}, state=${otClient.stateName}`) + otClient.onRemoteOps(update.op, update.v) } + } - // Handle ack — process even for editor docs so the bridge's OtClient - // can finish pending ops (e.g. ops sent just before addEditorDoc was called). - // Without this, the OtClient stays stuck in awaitingConfirm and the ops - // appear lost until removeEditorDoc re-discovers the discrepancy. - if (!update.op) { - const otClient = this.otClients.get(docId) - if (otClient) { - otClient.onAck() - } + /** Check if an otUpdateApplied event originated from our own socket */ + private isOwnSource(metaSource?: string): boolean { + // Primary: match meta.source against our publicId (same as Overleaf's inflightSubmittedIds check) + if (metaSource && this.socket.publicId) { + return metaSource === this.socket.publicId } + // If meta.source is absent, we can't determine origin. + // The open-source server sends ack without meta to the sender (no ops, no meta.source). + // If we get ops WITHOUT meta.source, treat conservatively as remote. + return false } // ── OT error handler ──────────────────────────────────────── @@ -311,7 +341,7 @@ export class FileSyncBridge { const relPath = this.docPathMap[docId] if (!relPath) return - bridgeLog(`[FileSyncBridge] otUpdateError for ${relPath}: ${error.message || 'unknown'}`) + bridgeLog(`[FileSyncBridge] OT_ERROR for ${relPath}: ${error.message || JSON.stringify(error)}`) // Re-join the doc to get fresh version and content, then re-apply disk content if different this.socket.joinDoc(docId).then(async (result) => { @@ -796,7 +826,18 @@ export class FileSyncBridge { const relPath = this.docPathMap[docId] const content = relPath ? this.lastKnownContent.get(relPath) ?? '' : '' const hash = createHash('sha1').update(content).digest('hex') - this.socket.applyOtUpdate(docId, ops, version, hash) + bridgeLog(`[FileSyncBridge] sendOps: ${relPath} v${version} ${ops.length} ops, hash=${hash.slice(0, 8)}`) + this.socket.applyOtUpdate(docId, ops, version, hash).then(() => { + bridgeLog(`[FileSyncBridge] sendOps: server accepted ${relPath}`) + // Do NOT call onAck() here. + // ACK comes via otUpdateApplied events processed in handleOtUpdate: + // - Echo (with ops, meta.source === ours) → treated as ACK + // - Explicit ack (without ops) → treated as ACK + // OtClient.onAck() silently deduplicates if both arrive. + }).catch((e) => { + bridgeLog(`[FileSyncBridge] sendOps: REJECTED for ${relPath}: ${e?.message || e}`) + this.handleOtError([{ doc: docId, message: e?.message || 'applyOtUpdate rejected' }]) + }) } // ── Apply remote ops (for non-editor docs) ────────────────── diff --git a/src/main/index.ts b/src/main/index.ts index 5efdae5..d2c3d64 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -47,6 +47,31 @@ async function writeMcpState(): Promise { } catch { /* ignore */ } } +let commentContextRefreshTimer: ReturnType | null = null +function scheduleCommentContextRefresh(): void { + if (commentContextRefreshTimer) clearTimeout(commentContextRefreshTimer) + commentContextRefreshTimer = setTimeout(async () => { + commentContextRefreshTimer = null + if (!overleafSock?.projectData) return + const { docPathMap: dp } = walkRootFolder(overleafSock.projectData.project.rootFolder) + const contexts: Record = {} + for (const [did, rp] of Object.entries(dp)) { + try { + const result = await overleafSock.joinDoc(did) + if (result.ranges?.comments) { + for (const c of result.ranges.comments) { + if (c.op?.t) contexts[c.op.t] = { file: rp, text: c.op.c || '', pos: c.op.p || 0 } + } + } + // Don't leaveDoc — bridge keeps all docs joined + } catch { /* ignore */ } + } + mcpCommentContexts = contexts + writeMcpState() + sendToRenderer('comments:initContexts', { contexts }) + }, 2000) // 2s debounce +} + function writeMcpOnlineUsers(): void { if (!mcpStateDir) return if (mcpOnlineUsersWriteTimer) clearTimeout(mcpOnlineUsersWriteTimer) @@ -776,6 +801,10 @@ ipcMain.handle('ot:connect', async (_e, projectId: string) => { name === 'delete-message' ) { sendToRenderer('comments:event', { type: name, args }) + // Re-fetch comment contexts for MCP when comments change + if (name === 'new-comment' || name === 'delete-thread') { + scheduleCommentContextRefresh() + } } }) @@ -968,14 +997,13 @@ The \`claude-workspace/\` directory is your private scratch space. It is **not s const contexts: Record = {} for (const [did, rp] of Object.entries(dp)) { try { - const alreadyJoined = docEventHandlers.has(did) const result = await overleafSock.joinDoc(did) if (result.ranges?.comments) { for (const c of result.ranges.comments) { if (c.op?.t) contexts[c.op.t] = { file: rp, text: c.op.c || '', pos: c.op.p || 0 } } } - if (!alreadyJoined) await overleafSock.leaveDoc(did) + // Don't leaveDoc — bridge keeps all docs joined } catch { /* ignore */ } } mcpCommentContexts = contexts diff --git a/src/main/otClient.ts b/src/main/otClient.ts index dc6bb4c..2917bcc 100644 --- a/src/main/otClient.ts +++ b/src/main/otClient.ts @@ -1,7 +1,17 @@ // Copyright (c) 2026 Yuren Hao // Licensed under AGPL-3.0 - see LICENSE file -// OT state machine for main process (mirror of renderer otClient) +// OT state machine for main process +// Modeled after Overleaf's ShareJS client (vendor/libs/sharejs.js) +// +// States: +// synchronized — no pending ops, version matches server +// awaitingConfirm — one inflight op awaiting server ack +// awaitingWithBuffer — inflight + buffered local ops +// +// Key invariant: at most ONE inflight op at a time. +// Version increments by 1 on each ack or remote op. + import type { OtOp } from './otTypes' import { transformOps } from './otTransform' @@ -66,6 +76,15 @@ export class OtClient { } } + /** + * Server acknowledged our inflight op. + * Matches Overleaf's ShareJS: both "ack without ops" and "echoed ops from + * our own source" are treated as acks. The echoed ops are NOT re-applied + * because they were already applied optimistically when submitted. + * + * In synchronized state, silently drops (duplicate ack — common when server + * sends both an echo and a separate ack event). + */ onAck() { switch (this.state.name) { case 'awaitingConfirm': @@ -90,12 +109,25 @@ export class OtClient { } case 'synchronized': - console.warn('[OtClient:main] unexpected ack in synchronized state') + // Duplicate ack — silently drop. + // This is expected: server may send both an echoed op (with meta.source) + // and a separate ack event (without ops). The first one transitions us + // to synchronized, the second arrives when we're already there. break } } + /** + * Server sent a remote op from another client. + * Transform against inflight/buffered ops before applying. + */ onRemoteOps(ops: OtOp[], newVersion: number) { + // Stale message detection (matching Overleaf's ShareJS): + // if the server version is behind our version, we already processed this. + if (newVersion < this.state.version) { + return + } + switch (this.state.name) { case 'synchronized': this.state = { ...this.state, version: newVersion } -- cgit v1.2.3