diff --git a/ts/packages/agents/markdown/package.json b/ts/packages/agents/markdown/package.json index cf19f1fb47..923dae5357 100644 --- a/ts/packages/agents/markdown/package.json +++ b/ts/packages/agents/markdown/package.json @@ -70,6 +70,7 @@ "yjs": "^13.6.8" }, "devDependencies": { + "@jest/globals": "^29.7.0", "@milkdown/ctx": "^7.3.6", "@types/debug": "^4.1.12", "@types/express": "^4.17.17", diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index 1fd6c212b4..56c830bea4 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -451,6 +451,23 @@ function broadcastEvent(event: Record): void { } } +function getBoundRevision(): string | null { + if (!filePath || !boundRelativePath) { + return null; + } + try { + return readBoundDocument({ + token: bindingToken ?? undefined, + root: currentRoot, + relativePath: boundRelativePath, + filePath, + }).revision; + } catch (error) { + debug(`Unable to read current binding revision: ${error}`); + return null; + } +} + function notifyBindingToParent(): void { process.send?.({ type: "bindingUpdated", @@ -1598,19 +1615,8 @@ app.get("/events", (req: Request, res: Response) => { res.flushHeaders(); clients.push(res); - let revision: string | null = null; - if (filePath && boundRelativePath) { - try { - revision = readBoundDocument({ - token: bindingToken ?? undefined, - root: currentRoot, - relativePath: boundRelativePath, - filePath, - }).revision; - } catch (error) { - debug(`Unable to read binding bootstrap revision: ${error}`); - } - } + const clientRole = clients.length === 1 ? "primary" : "secondary"; + const revision = getBoundRevision(); res.write( `data: ${JSON.stringify({ type: "bindingBootstrap", @@ -1619,12 +1625,25 @@ app.get("/events", (req: Request, res: Response) => { documentName: filePath ? path.basename(filePath, ".md") : null, boundRelativePath, revision, + clientRole, timestamp: Date.now(), })}\n\n`, ); req.on("close", () => { + const wasPrimary = clients[0] === res; clients = clients.filter((client) => client !== res); + if (wasPrimary && clients[0]) { + safeWriteToResponse( + clients[0], + `data: ${JSON.stringify({ + type: "primaryElected", + bindingToken, + revision: getBoundRevision(), + timestamp: Date.now(), + })}\n\n`, + ); + } }); }); diff --git a/ts/packages/agents/markdown/src/view/site/core/collaboration-manager.ts b/ts/packages/agents/markdown/src/view/site/core/collaboration-manager.ts index e3c7cb0d53..78aa810a9f 100644 --- a/ts/packages/agents/markdown/src/view/site/core/collaboration-manager.ts +++ b/ts/packages/agents/markdown/src/view/site/core/collaboration-manager.ts @@ -180,13 +180,18 @@ export class CollaborationManager { `[COLLAB] Retrieved collaboration info for document: ${collabInfo.currentDocument}`, ); + // Never derive a room key from the display basename: + // same-basename files in different folders would collide. + const authoritativeDocumentId = + typeof collabInfo.currentDocumentId === "string" && + collabInfo.currentDocumentId.length > 0 + ? collabInfo.currentDocumentId + : COLLABORATION_CONFIG.DEFAULT_DOCUMENT_ID; const config = { websocketServerUrl: collabInfo.websocketServerUrl || COLLABORATION_CONFIG.DEFAULT_WEBSOCKET_URL, - documentId: collabInfo.currentDocument - ? collabInfo.currentDocument.replace(".md", "") - : COLLABORATION_CONFIG.DEFAULT_DOCUMENT_ID, + documentId: authoritativeDocumentId, fallbackToLocal: true, }; diff --git a/ts/packages/agents/markdown/src/view/site/core/document-manager.ts b/ts/packages/agents/markdown/src/view/site/core/document-manager.ts index 67e46b1bc9..5aeedf473f 100644 --- a/ts/packages/agents/markdown/src/view/site/core/document-manager.ts +++ b/ts/packages/agents/markdown/src/view/site/core/document-manager.ts @@ -1,19 +1,65 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -import type { Editor } from "@milkdown/core"; -import { editorViewCtx, parserCtx } from "@milkdown/core"; +import { editorViewCtx, type Editor } from "@milkdown/core"; import { AI_CONFIG, DEFAULT_MARKDOWN_CONTENT, EDITOR_CONFIG } from "../config"; import { getMarkdownFromEditor, getEditorPositionInfo } from "../utils"; +import { + encodeDocumentPathForUrl, + ensureMarkdownExtension, +} from "../../route/urlPath.js"; + +class DocumentWriteConflictError extends Error { + public constructor(message: string) { + super(message); + this.name = "DocumentWriteConflictError"; + } +} + +interface DocumentWriteResponse { + content?: unknown; + error?: unknown; + revision?: unknown; +} export class DocumentManager { private notificationManager: any = null; private editorManager: any = null; private eventSource: EventSource | null = null; + private sseEventQueue: Promise = Promise.resolve(); private autoSaveTimer: NodeJS.Timeout | null = null; private isPrimaryClient = false; + private isBindingTransitionInProgress = false; private lastAutoSaveContent = ""; + // A 409 with different on-disk content blocks repeated autosaves of + // exactly the same editor state. A subsequent edit may try again, but + // the conflicted payload is never retried indefinitely. + private lastConflictedAutoSaveContent: string | null = null; private currentDocumentId = "default"; + private currentRevision: string | null = null; + // Token rotated by the view service on every trusted rebinding + // (setFile from the agent or /api/switch-document from another + // browser). Snapshots we accept must carry this exact value or + // we discard them - a stale snapshot for a previous binding must + // never overwrite the current editor content. + private currentBindingToken: string | null = null; + private bindingGeneration = 0; + private switchRequestGeneration = 0; + // Full user-relative path (POSIX form) of the currently-bound + // document. Nested paths (docs/team/roadmap.md) are preserved end + // to end; the browser never derives this from an absolute path + // because the service does not expose absolute paths to callers. + private currentBoundRelativePath: string | null = null; + + /** + * Expose the current bound relative path (POSIX form, includes .md) + * for callers that need to render or route with it. Read-only from + * outside; the value is only mutated from bindingBootstrap / + * documentChanged / switchToDocument. + */ + public getCurrentBoundRelativePath(): string | null { + return this.currentBoundRelativePath; + } public setNotificationManager(notificationManager: any): void { this.notificationManager = notificationManager; @@ -34,6 +80,7 @@ export class DocumentManager { public async initialize(): Promise { // Set up SSE connection for document change notifications this.setupSSEConnection(); + await this.loadCurrentBindingPath(); // Initialize auto-save if enabled if (EDITOR_CONFIG.FEATURES.AUTO_SAVE) { @@ -41,6 +88,26 @@ export class DocumentManager { } } + private async loadCurrentBindingPath(): Promise { + try { + const response = await fetch("/api/current-document"); + if (!response.ok) { + return; + } + const current = (await response.json()) as { + boundRelativePath?: unknown; + }; + if (typeof current.boundRelativePath === "string") { + this.currentBoundRelativePath = current.boundRelativePath; + } + } catch (error) { + console.warn( + "[DOCUMENT] Failed to load current binding path:", + error, + ); + } + } + /** * Start auto-save timer for primary client */ @@ -59,7 +126,11 @@ export class DocumentManager { } /** - * Perform auto-save if content has changed + * Perform auto-save if content has changed. Autosave requires a + * live binding token learned from bindingBootstrap / + * documentChanged; without one the server would (and does) reject + * the request, so we skip locally rather than firing a doomed + * write. */ private async performAutoSave(): Promise { try { @@ -67,6 +138,12 @@ export class DocumentManager { console.log("[AUTO-SAVE] Skipping - no editor manager"); return; } + if (this.isBindingTransitionInProgress) { + console.log( + "[AUTO-SAVE] Skipping - document binding is changing", + ); + return; + } const editor = this.editorManager.getEditor(); if (!editor) { @@ -74,66 +151,68 @@ export class DocumentManager { return; } - // Get current content using editor API - const currentContent = await this.getMarkdownContent(editor); + if (this.currentBindingToken === null) { + console.log( + "[AUTO-SAVE] Skipping - no bindingToken yet (unbootstrapped)", + ); + return; + } - // Only save if content has changed + const currentContent = await this.getMarkdownContent(editor); if (currentContent === this.lastAutoSaveContent) { console.log("[AUTO-SAVE] Skipping - content unchanged"); return; } + if (currentContent === this.lastConflictedAutoSaveContent) { + console.warn( + "[AUTO-SAVE] Skipping unchanged content after a write conflict", + ); + return; + } console.log(`[AUTO-SAVE] Content changed, auto-saving...`); - // Get current document path from server - const docInfo = await this.getCurrentDocumentInfo(); - - // Send auto-save request + // Send the binding identity and base revision with the save. + // They prevent a stale tab from confusing one binding or + // revision for another; they are not authorization credentials. + // We do NOT + // send any absolute path - the server writes only to its + // snapshotted trusted file/root. const response = await fetch(AI_CONFIG.ENDPOINTS.AUTOSAVE, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ content: currentContent, - filePath: docInfo.fullPath, documentId: this.currentDocumentId, + bindingToken: this.currentBindingToken, + expectedRevision: this.currentRevision, }), }); + await this.reconcileDocumentWriteResponse( + response, + currentContent, + "Auto-save", + ); if (response.ok) { - this.lastAutoSaveContent = currentContent; console.log("[AUTO-SAVE] Successfully saved document"); } else { - console.error( - "[AUTO-SAVE] Failed to save:", - response.statusText, + console.log( + "[AUTO-SAVE] Reconciled with content already persisted by another client", ); } } catch (error) { console.error("[AUTO-SAVE] Error during auto-save:", error); - } - } - - /** - * Get current document info from server - */ - private async getCurrentDocumentInfo(): Promise<{ - currentDocument: string; - fullPath: string | null; - }> { - try { - const response = await fetch("/api/current-document"); - if (response.ok) { - return await response.json(); + if ( + error instanceof DocumentWriteConflictError && + this.notificationManager + ) { + this.notificationManager.showNotification( + error.message, + "error", + ); } - } catch (error) { - console.warn("Failed to get current document info:", error); } - - // Fallback - return { - currentDocument: this.currentDocumentId, - fullPath: null, - }; } private setupSSEConnection(): void { @@ -148,7 +227,14 @@ export class DocumentManager { try { const data = JSON.parse(event.data); console.log(`[SSE] Received event: ${data.type}`, data); - this.handleSSEEvent(data); + this.sseEventQueue = this.sseEventQueue + .then(() => this.handleSSEEvent(data)) + .catch((error: unknown) => { + console.error( + "[SSE] Failed to process event:", + error, + ); + }); } catch (error) { console.error("[SSE] Failed to parse event data:", error); console.error( @@ -180,19 +266,42 @@ export class DocumentManager { switch (data.type) { case "documentChanged": console.log(`[SSE] Document changed to: ${data.newDocumentId}`); + // Adopt the new identity atomically before we touch the + // editor or fire autosave: pairing the new token with the + // new documentId prevents a same-tick autosave from + // carrying a stale token that would then be rejected. + if ( + typeof data.newDocumentId !== "string" || + typeof data.bindingToken !== "string" || + typeof data.boundRelativePath !== "string" + ) { + console.warn( + "[SSE] Ignoring incomplete documentChanged binding", + ); + break; + } + this.bindingGeneration++; this.currentDocumentId = data.newDocumentId; + this.currentBindingToken = data.bindingToken; + this.currentBoundRelativePath = data.boundRelativePath; + this.adoptRevision(data.revision); + this.isBindingTransitionInProgress = true; - // Reset sync notification state for new document - if (this.notificationManager) { - this.notificationManager.resetDocumentSyncState( + try { + // Reset sync notification state for new document + if (this.notificationManager) { + this.notificationManager.resetDocumentSyncState( + data.newDocumentId, + ); + } + + await this.handleDocumentChangeFromBackend( data.newDocumentId, + data.newDocumentName, ); + } finally { + this.isBindingTransitionInProgress = false; } - - await this.handleDocumentChangeFromBackend( - data.newDocumentId, - data.newDocumentName, - ); break; case "documentSynced": @@ -202,6 +311,16 @@ export class DocumentManager { case "autoSave": console.log(`[SSE] Auto-save completed for: ${data.filePath}`); + if ( + typeof data.bindingToken === "string" && + data.bindingToken === this.currentBindingToken + ) { + this.adoptRevision(data.revision); + } else { + console.warn( + "[SSE] Ignoring autoSave revision for a stale binding", + ); + } // Auto-save notification removed per user request break; @@ -210,60 +329,128 @@ export class DocumentManager { // Auto-save error notification removed per user request break; - case "llmOperations": - // PRODUCTION: Handle LLM operations sent to PRIMARY client only via SSE - // Apply operations through editor API for proper markdown parsing + case "bindingBootstrap": { + // Trusted bootstrap from the same-origin service. Its + // view of the current binding is authoritative on every + // reconnect: a stale in-memory token here would just + // paper over a real rebinding (e.g. another tab or the + // agent switched files while this tab was offline). + // Adopt the identity atomically so a subsequent + // autosave and any pending snapshots compare against + // a consistent token/documentId pair. + if ( + typeof data.bindingToken === "string" && + typeof data.documentId === "string" && + typeof data.boundRelativePath === "string" + ) { + this.bindingGeneration++; + this.currentBindingToken = data.bindingToken; + this.currentDocumentId = data.documentId; + this.currentBoundRelativePath = data.boundRelativePath; + this.adoptRevision(data.revision); + console.log( + `[SSE] Adopted bindingBootstrap token for ${data.documentId ?? ""}`, + ); + } else if ( + data.bindingToken === null && + data.documentId === null && + data.boundRelativePath === null + ) { + // View is in memory-only mode. Clear our token so a + // stale value cannot survive across an unbind / + // rebind cycle and get re-associated with a + // different file. + this.bindingGeneration++; + this.currentBindingToken = null; + this.currentBoundRelativePath = null; + this.currentRevision = null; + } else { + console.warn( + "[SSE] Ignoring incomplete bindingBootstrap event", + ); + } + // clientRole is assigned by SSE connection ordering (see + // service /events). The first-connected browser is the + // primary autosave writer; secondary tabs skip autosave. + if (data.clientRole === "primary") { + this.isPrimaryClient = true; + console.log("[SSE] bindingBootstrap assigned PRIMARY role"); + } else if (data.clientRole === "secondary") { + this.isPrimaryClient = false; + console.log( + "[SSE] bindingBootstrap assigned SECONDARY role", + ); + } + break; + } + + case "primaryElected": { + // Sent to the next-connected browser when the previous + // primary tab closed. Flip the autosave flag on so this + // tab starts writing on the next timer tick. Promotion must + // not seat a new binding token by itself: without the matching + // documentChanged/bootstrap content that could pair stale + // editor content with a new file identity. + if (data.bindingToken !== this.currentBindingToken) { + console.warn( + "[SSE] Ignoring primaryElected for a stale binding", + ); + break; + } + this.isPrimaryClient = true; + this.adoptRevision(data.revision); + console.log("[SSE] Promoted to PRIMARY for autosave"); + break; + } + + case "documentSnapshot": { + // Post-commit snapshot from the server after it applied + // LLM operations to raw Markdown. Only adopt it when the + // binding token matches the one we last recorded from + // documentChanged / bindingBootstrap. Fail closed when + // we have no token yet: an untrusted snapshot on an + // unbootstrapped browser must never seat a token from + // arbitrary content. + const snapshotToken = data.bindingToken; + if (typeof snapshotToken !== "string") { + console.warn( + "[SSE] Ignoring documentSnapshot with no bindingToken", + ); + break; + } + if (this.currentBindingToken === null) { + console.warn( + `[SSE] Ignoring documentSnapshot: no established binding token to compare against (snapshot ${snapshotToken})`, + ); + break; + } + if (snapshotToken !== this.currentBindingToken) { + console.warn( + `[SSE] Ignoring documentSnapshot for stale binding (snapshot ${snapshotToken}, current ${this.currentBindingToken})`, + ); + break; + } if ( - data.clientRole === "primary" && - data.operations && - Array.isArray(data.operations) && - this.editorManager + this.editorManager && + typeof data.markdown === "string" && + typeof this.editorManager.setContent === "function" ) { try { - // Mark this client as primary for auto-save - this.isPrimaryClient = true; + await this.editorManager.setContent(data.markdown); + this.lastAutoSaveContent = data.markdown; + this.adoptRevision(data.revision); console.log( - "[SSE] Marked as PRIMARY CLIENT for auto-save", + `[SSE] Adopted documentSnapshot (${data.markdown.length} chars, revision ${data.revision ?? "-"})`, ); - - // Apply operations through editor API for proper markdown parsing - const editor = this.editorManager.getEditor(); - if (editor) { - await this.applyOperationsThroughEditor( - editor, - data.operations, - ); - console.log( - ` [SSE] Applied ${data.operations.length} operations via editor API`, - ); - } else { - console.warn( - ` [SSE] No editor available to apply operations`, - ); - } } catch (error) { console.error( - `[ERROR] [SSE] Failed to apply LLM operations:`, + "[ERROR] [SSE] Failed to apply documentSnapshot:", error, ); - if (this.notificationManager) { - this.notificationManager.showNotification( - `❌ Failed to apply AI changes`, - "error", - ); - } } - } else if (data.clientRole !== "primary") { - // Mark as secondary client - this.isPrimaryClient = false; - console.log(`[SSE] Marked as SECONDARY CLIENT`); - } else { - console.warn( - `[SSE] Invalid LLM operations received:`, - data, - ); } break; + } case "operationsBeingApplied": // Handle notification that operations are being applied by primary client @@ -307,6 +494,7 @@ export class DocumentManager { const response = await fetch(documentUrl); const content = response.ok ? await response.text() : ""; + this.adoptRevisionFromResponse(response); console.log( ` [DOCUMENT] Frontend switched to document: "${documentId}"`, ); @@ -317,9 +505,16 @@ export class DocumentManager { } // Update page title and URL - document.title = `${documentName} - AI-Enhanced Markdown Editor`; - const newUrl = `/document/${encodeURIComponent(documentName)}`; - window.history.pushState({ documentName }, document.title, newUrl); + const relativePath = + this.currentBoundRelativePath ?? `${documentName}.md`; + const displayPath = relativePath.replace(/\.md$/i, ""); + document.title = `${displayPath} - AI-Enhanced Markdown Editor`; + const newUrl = `/document/${encodeDocumentPathForUrl(relativePath)}`; + window.history.pushState( + { documentPath: displayPath }, + document.title, + newUrl, + ); } catch (error) { console.error( "[DOCUMENT] Failed to handle backend document change:", @@ -381,7 +576,13 @@ export class DocumentManager { } console.log(`[CLIENT] Sending markdown response to server...`); - // Send markdown content back to view process + // Send markdown content back to view process. Echo our + // currentBindingToken so the service can reject the + // response when we rebound mid-flight (the pending server + // request is pinned to the token that was live when the + // SSE was sent). A null echo is fine and expected during + // the pre-bootstrap window; the service simply skips the + // check when both sides carry null. const response = await fetch("/api/markdown-response", { method: "POST", headers: { "Content-Type": "application/json" }, @@ -389,6 +590,7 @@ export class DocumentManager { requestId: requestId, markdown: markdown, positionInfo: positionInfo, + bindingToken: this.currentBindingToken, timestamp: Date.now(), }), }); @@ -417,6 +619,7 @@ export class DocumentManager { error instanceof Error ? error.message : "Unknown error", + bindingToken: this.currentBindingToken, timestamp: Date.now(), }), }); @@ -429,9 +632,25 @@ export class DocumentManager { } } + /** + * Persist the current editor state to the bound file. Payload is + * the serialized Markdown from getMarkdownContent - never plain + * text - so headings/bold/code/links survive a manual save. When + * an editor is provided, we carry the current bindingToken as an + * identity proof: the service applies the same trust check the + * autosave endpoint does and rejects a missing/stale token + * without touching disk. + */ public async saveDocument(editor?: Editor): Promise { try { - // Get markdown content from editor or server + if (this.isBindingTransitionInProgress) { + throw new Error( + "Cannot save while the document binding is changing", + ); + } + // Get markdown content from editor (via serializer) or, + // when there is no editor to serialize from, from the + // service's current view of the bound file. const content = editor ? await this.getMarkdownContent(editor) : await this.loadContentFromServer(); @@ -441,12 +660,19 @@ export class DocumentManager { const response = await fetch(saveUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ content }), + body: JSON.stringify({ + content, + documentId: this.currentDocumentId, + bindingToken: this.currentBindingToken, + expectedRevision: this.currentRevision, + }), }); - if (!response.ok) { - throw new Error(`Save failed: ${response.status}`); - } + await this.reconcileDocumentWriteResponse( + response, + content, + "Save", + ); console.log(` [DOCUMENT] Document saved successfully`); } catch (error) { @@ -458,37 +684,17 @@ export class DocumentManager { } } + /** + * Get the full serialized Markdown for the editor. This is the + * ONLY content that autosave / saveDocument may persist: it runs + * the ProseMirror doc through Milkdown's serializerCtx so + * headings, bold, code fences, links, etc. round-trip. The + * ProseMirror `textContent` shortcut is intentionally not used + * here - it strips formatting and would silently overwrite the + * bound file with plain text. + */ public async getMarkdownContent(editor: Editor): Promise { - if (!editor) return ""; - - try { - // Get content directly from editor first (most current state) - const editorContent = await new Promise((resolve) => { - editor.action((ctx) => { - const view = ctx.get(editorViewCtx); - resolve(view.state.doc.textContent || ""); - }); - }); - - if (editorContent) { - return editorContent; - } - } catch (error) { - console.warn("Failed to get content from editor:", error); - } - - try { - // Fallback to server content if editor content is empty - const response = await fetch(AI_CONFIG.ENDPOINTS.DOCUMENT); - if (response.ok) { - const serverContent = await response.text(); - return serverContent; - } - } catch (error) { - console.warn("Failed to fetch document from server:", error); - } - - return ""; + return getMarkdownFromEditor(editor); } public async loadInitialContent(): Promise { @@ -499,6 +705,8 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionFromResponse(response); + this.lastAutoSaveContent = content; return content; } else { return this.getDefaultContent(); @@ -516,6 +724,7 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionFromResponse(response); return content; } throw new Error( @@ -536,6 +745,7 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionFromResponse(response); return content; } throw new Error( @@ -554,14 +764,19 @@ export class DocumentManager { const response = await fetch(saveUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ content }), + body: JSON.stringify({ + content, + documentId: this.currentDocumentId, + bindingToken: this.currentBindingToken, + expectedRevision: this.currentRevision, + }), }); - if (!response.ok) { - throw new Error( - `Failed to set document content: ${response.status} ${response.statusText}`, - ); - } + await this.reconcileDocumentWriteResponse( + response, + content, + "Set document content", + ); console.log(` [DOCUMENT] Document content updated successfully`); // Don't reload the whole page, just notify the editor will update via collaboration @@ -618,15 +833,45 @@ export class DocumentManager { } } - public async switchToDocument(documentName: string): Promise { + public async switchToDocument(documentPath: string): Promise { try { + // Short-circuit when the SSE bootstrap already bound this + // browser to the requested document. Without this guard + // the initial `/document/team/2025/plan.md` load would + // trigger a redundant /api/switch-document call, which + // rotates the binding token, might race the + // bootstrap adoption path, and - if the raw path is not + // normalized identically - could persuade the service to + // create a new empty file. + if (this.currentBoundRelativePath !== null) { + const targetRelative = ensureMarkdownExtension(documentPath); + if (this.currentBoundRelativePath === targetRelative) { + console.log( + `[DOCUMENT] Already bound to ${this.currentBoundRelativePath}; skipping /api/switch-document`, + ); + return; + } + } + + const requestGeneration = ++this.switchRequestGeneration; + const bindingGeneration = this.bindingGeneration; const switchUrl = "/api/switch-document"; - // Call server to switch document + // Send the raw user-relative path (possibly nested, e.g. + // "docs/team/roadmap"). The service re-validates via + // pathPolicy, appends .md if needed, and returns the full + // normalized relative path plus the freshly-rotated + // bindingToken and documentId (Yjs room) it assigned. const response = await fetch(switchUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ documentName }), + body: JSON.stringify({ + documentPath, + // Back-compat for older callers/tests that read + // this on the server side; the server prefers + // documentPath. + documentName: documentPath, + }), }); if (!response.ok) { @@ -636,11 +881,50 @@ export class DocumentManager { } const result = await response.json(); - console.log(`[DOCUMENT] Server switched to: ${documentName}`); + console.log(`[DOCUMENT] Server switched to: ${documentPath}`); + + // Adopt the new identity atomically BEFORE the editor + // room switch and any autosave. Otherwise the next + // autosave tick would carry the previous token/documentId + // and be rejected by the service - and worse, if the + // service had already rotated to a different binding + // (concurrent switch), we would pair new content with + // the old identity. + const documentId = + typeof result.documentId === "string" + ? result.documentId + : documentPath; + const relative = + typeof result.boundRelativePath === "string" + ? result.boundRelativePath + : typeof result.relativePath === "string" + ? result.relativePath + : null; + if (requestGeneration !== this.switchRequestGeneration) { + console.warn( + `[DOCUMENT] Ignoring superseded switch response for ${documentPath}`, + ); + return; + } + if ( + this.bindingGeneration !== bindingGeneration && + (typeof result.bindingToken !== "string" || + result.bindingToken !== this.currentBindingToken || + relative !== this.currentBoundRelativePath) + ) { + console.warn( + `[DOCUMENT] Ignoring stale switch response for ${documentPath}`, + ); + return; + } + this.currentDocumentId = documentId; + this.currentBoundRelativePath = relative; + if (typeof result.bindingToken === "string") { + this.currentBindingToken = result.bindingToken; + } + this.adoptRevision(result.revision); - // Switch editor collaboration to new document room if (this.editorManager) { - const documentId = documentName; // Document ID is same as document name (without .md) await this.editorManager.switchToDocument( documentId, result.content, @@ -650,16 +934,105 @@ export class DocumentManager { ); } - // Update page title and URL - document.title = `${documentName} - AI-Enhanced Markdown Editor`; - const newUrl = `/document/${encodeURIComponent(documentName)}`; - window.history.pushState({ documentName }, document.title, newUrl); + // Update page title and URL. For nested paths, encode each + // segment so slashes are preserved. + const displayPath = relative + ? relative.replace(/\.md$/i, "") + : documentPath; + document.title = `${displayPath} - AI-Enhanced Markdown Editor`; + const encodedPath = encodeDocumentPathForUrl( + relative ?? ensureMarkdownExtension(documentPath), + ); + const newUrl = `/document/${encodedPath}`; + window.history.pushState( + { documentPath: displayPath }, + document.title, + newUrl, + ); } catch (error) { console.error("[DOCUMENT] Failed to switch document:", error); throw error; } } + private adoptRevision(revision: unknown): void { + if (typeof revision === "string" && revision.length > 0) { + this.currentRevision = revision; + } + } + + private adoptRevisionFromResponse(response: Response): void { + this.adoptRevision(response.headers.get("X-Content-Revision")); + } + + private async parseDocumentWriteResponse( + response: Response, + ): Promise { + try { + const result: unknown = await response.json(); + if ( + typeof result === "object" && + result !== null && + !Array.isArray(result) + ) { + return result as DocumentWriteResponse; + } + } catch (error) { + console.warn( + `[DOCUMENT] Could not parse ${response.status} response body:`, + error, + ); + } + return undefined; + } + + /** + * Reconcile an optimistic document write. A 409 is considered resolved + * only when the server proves that the exact attempted content is already + * on disk. Otherwise we retain the old base revision and surface a + * conflict, preventing a follow-up save from overwriting newer disk data. + */ + private async reconcileDocumentWriteResponse( + response: Response, + attemptedContent: string, + operation: string, + ): Promise { + const result = await this.parseDocumentWriteResponse(response); + + if (response.ok) { + this.adoptRevision(result?.revision); + this.lastAutoSaveContent = attemptedContent; + this.lastConflictedAutoSaveContent = null; + return; + } + + if ( + response.status === 409 && + typeof result?.revision === "string" && + result.content === attemptedContent + ) { + this.adoptRevision(result.revision); + this.lastAutoSaveContent = attemptedContent; + this.lastConflictedAutoSaveContent = null; + return; + } + + if (response.status === 409) { + this.lastConflictedAutoSaveContent = attemptedContent; + const detail = + typeof result?.error === "string" ? ` ${result.error}` : ""; + throw new DocumentWriteConflictError( + `${operation} conflict: the document changed on disk and was not overwritten.${detail}`, + ); + } + + const detail = + typeof result?.error === "string" ? ` ${result.error}` : ""; + throw new Error( + `${operation} failed: ${response.status} ${response.statusText}.${detail}`, + ); + } + private async hasUnsavedChanges(): Promise { try { if (!this.editorManager) return false; @@ -686,205 +1059,6 @@ export class DocumentManager { } } - /** - * Apply operations through the editor API for proper markdown parsing and DOM updates - */ - private async applyOperationsThroughEditor( - editor: any, - operations: any[], - ): Promise { - console.log( - `[WRITE] [EDITOR-API] Applying ${operations.length} operations through editor`, - ); - - await editor.action((ctx: any) => { - const view = ctx.get(editorViewCtx); - const parser = ctx.get(parserCtx); - let tr = view.state.tr; - - for (const operation of operations) { - console.log( - `[EDITOR-API] Applying operation: ${operation.type} at position ${operation.position || 0}`, - ); - - try { - switch (operation.type) { - case "insert": { - // Convert operation content to markdown text - const markdownText = - this.operationContentToMarkdown( - operation.content, - ); - - const position = Math.min( - operation.position || 0, - view.state.doc.content.size, - ); - - // Parse markdown to ProseMirror nodes - const doc = parser(markdownText); - if (doc && doc.content) { - tr = tr.insert(position, doc.content); - console.log( - ` [EDITOR-API] Inserted "${markdownText}" at position ${position}`, - ); - } else { - console.warn( - ` [EDITOR-API] Failed to parse markdown: "${markdownText}"`, - ); - } - break; - } - case "replace": { - const markdownText = - this.operationContentToMarkdown( - operation.content, - ); - - const fromPos = Math.min( - operation.from || 0, - view.state.doc.content.size, - ); - const toPos = Math.min( - operation.to || fromPos + 1, - view.state.doc.content.size, - ); - - // Parse markdown to ProseMirror nodes - const doc = parser(markdownText); - if (doc && doc.content) { - tr = tr.replaceWith( - fromPos, - toPos, - doc.content, - ); - console.log( - ` [EDITOR-API] Replaced content from ${fromPos} to ${toPos} with "${markdownText}"`, - ); - } - break; - } - case "delete": { - const fromPos = Math.min( - operation.from || 0, - view.state.doc.content.size, - ); - const toPos = Math.min( - operation.to || fromPos + 1, - view.state.doc.content.size, - ); - - tr = tr.delete(fromPos, toPos); - console.log( - ` [EDITOR-API] Deleted content from ${fromPos} to ${toPos}`, - ); - break; - } - default: - console.warn( - `[ERROR] [EDITOR-API] Unknown operation type: ${operation.type}`, - ); - break; - } - } catch (operationError) { - console.error( - `[ERROR] [EDITOR-API] Failed to apply operation ${operation.type}:`, - operationError, - ); - } - } - - // Dispatch all changes in a single transaction - if (tr.docChanged) { - view.dispatch(tr); - console.log( - ` [EDITOR-API] Applied ${operations.length} operations successfully`, - ); - } else { - console.log(` [EDITOR-API] No document changes to apply`); - } - }); - } - - /** - * Convert operation content array to markdown text - */ - private operationContentToMarkdown(content: any[]): string { - if (!Array.isArray(content)) { - const result = String(content || ""); - return result; - } - - const result = content - .map((item: any) => { - if (typeof item === "string") { - return item; - } - - if (item && typeof item === "object") { - // Handle different content types - switch (item.type) { - case "heading": - const headingText = this.extractTextFromContent( - item.content || item.text, - ); - return headingText; - - case "paragraph": - const paragraphText = this.extractTextFromContent( - item.content || item.text, - ); - return paragraphText; - - case "text": - const textResult = item.text || ""; - return textResult; - - default: - // Fallback: extract any text content - const fallbackResult = - this.extractTextFromContent(item.content) || - item.text || - ""; - return fallbackResult; - } - } - - const stringResult = String(item || ""); - return stringResult; - }) - .join("\n"); - - return result; - } - - /** - * Extract plain text from nested content structures - */ - private extractTextFromContent(content: any): string { - if (!content) return ""; - - if (typeof content === "string") { - return content; - } - - if (Array.isArray(content)) { - return content - .map((item) => this.extractTextFromContent(item)) - .join(""); - } - - if (content.text) { - return content.text; - } - - if (content.content) { - return this.extractTextFromContent(content.content); - } - - return ""; - } - /** * Handle AI awareness events from SSE */ diff --git a/ts/packages/agents/markdown/src/view/site/index.ts b/ts/packages/agents/markdown/src/view/site/index.ts index 7c7af70e3d..73b3a67a01 100644 --- a/ts/packages/agents/markdown/src/view/site/index.ts +++ b/ts/packages/agents/markdown/src/view/site/index.ts @@ -18,6 +18,7 @@ import { UIManager } from "./ui/ui-manager"; // Import utilities import { getRequiredElement, eventHandlers } from "./utils"; +import { parseDocumentPathFromUrl } from "../route/urlPath.js"; // Global state for the application let editorManager: EditorManager | null = null; @@ -35,10 +36,12 @@ document.addEventListener("DOMContentLoaded", async () => { }); async function initializeApplication(): Promise { - // Check if we have a document name in the URL - const urlPath = window.location.pathname; - const documentNameMatch = urlPath.match(/\/document\/([^\/]+)/); - const documentName = documentNameMatch ? documentNameMatch[1] : null; + // Parse the target document path from the URL. Nested paths + // (`/document/team/2025/plan`) and per-segment percent-encoded + // spaces (`/document/my%20notes`) round-trip end-to-end; the + // service also matches on the full path so no dirname/basename + // reduction happens on either side. + const documentPath = parseDocumentPathFromUrl(window.location.pathname); // Initialize managers editorManager = new EditorManager(); @@ -54,9 +57,13 @@ async function initializeApplication(): Promise { // Connect DocumentManager to UI components uiManager.setDocumentManager(documentManager); - // If we have a document name in URL, switch to that document - if (documentName) { - await switchToDocument(documentName); + // If the URL asked for a specific document, ask the DocumentManager + // to align to it. The DocumentManager compares against the binding + // the service just bootstrapped over SSE, so a redundant switch + // (URL already matches the bound file) becomes a no-op instead of + // creating a stray file or rotating the trusted binding token. + if (documentPath) { + await switchToDocument(documentPath); } // Get required DOM elements @@ -81,31 +88,30 @@ async function initializeApplication(): Promise { console.log("[APP] Application initialized successfully"); } -async function switchToDocument(documentName: string): Promise { +async function switchToDocument(documentPath: string): Promise { try { if (documentManager) { - await documentManager.switchToDocument(documentName); + await documentManager.switchToDocument(documentPath); console.log( - `[APP] Successfully switched to document: ${documentName}`, + `[APP] Successfully switched to document: ${documentPath}`, ); } else { throw new Error("DocumentManager not initialized"); } } catch (error) { console.error("[APP] Failed to switch document:", error); - showError(`Failed to load document: ${documentName}`); + showError(`Failed to load document: ${documentPath}`); } } function setupBrowserHistoryHandling(): void { - // Handle browser back/forward navigation - window.addEventListener("popstate", async (event) => { - const urlPath = window.location.pathname; - const documentNameMatch = urlPath.match(/\/document\/([^\/]+)/); - const documentName = documentNameMatch ? documentNameMatch[1] : null; - - if (documentName && event.state?.documentName !== documentName) { - await switchToDocument(documentName); + // Handle browser back/forward navigation. Reuse the same nested + // URL parser as initial load so `/document/team/2025/plan` and + // percent-encoded segments navigate correctly. + window.addEventListener("popstate", async () => { + const target = parseDocumentPathFromUrl(window.location.pathname); + if (target) { + await switchToDocument(target); } }); } diff --git a/ts/packages/agents/markdown/src/view/site/tsconfig.json b/ts/packages/agents/markdown/src/view/site/tsconfig.json index b9d1c6fce3..90ca7915ce 100644 --- a/ts/packages/agents/markdown/src/view/site/tsconfig.json +++ b/ts/packages/agents/markdown/src/view/site/tsconfig.json @@ -14,6 +14,7 @@ "noUnusedParameters": true }, "include": ["./**/*"], + "references": [{ "path": "../route" }], "ts-node": { "esm": true } diff --git a/ts/packages/agents/markdown/src/view/site/types.ts b/ts/packages/agents/markdown/src/view/site/types.ts index 168359c331..ab678a3358 100644 --- a/ts/packages/agents/markdown/src/view/site/types.ts +++ b/ts/packages/agents/markdown/src/view/site/types.ts @@ -81,6 +81,13 @@ export type NotificationType = "success" | "error" | "info"; export interface CollaborationInfo { websocketServerUrl: string; + // Opaque server-authoritative identifier for the Yjs collaboration + // room. Always prefer this over any file-name derived key, because + // two files with the same basename in different folders map to + // different rooms. + currentDocumentId: string; + // Human-readable display name (typically the file basename without + // the .md extension). Not safe to use as a room key. currentDocument: string; documents: number; totalClients: number; diff --git a/ts/packages/agents/markdown/src/view/site/ui/toolbar-manager.ts b/ts/packages/agents/markdown/src/view/site/ui/toolbar-manager.ts index 5430d94d36..05b55aa57e 100644 --- a/ts/packages/agents/markdown/src/view/site/ui/toolbar-manager.ts +++ b/ts/packages/agents/markdown/src/view/site/ui/toolbar-manager.ts @@ -3,6 +3,7 @@ import { DocumentManager } from "../core/document-manager"; import { getElementById } from "../utils"; +import { encodeDocumentPathForUrl } from "../../route/urlPath.js"; //import { editorViewCtx } from "@milkdown/core"; export class ToolbarManager { @@ -169,17 +170,19 @@ export class ToolbarManager { } const docInfo = await response.json(); - const documentName = docInfo.currentDocument || "live"; + const documentPath = + docInfo.boundRelativePath || docInfo.currentDocument || "live"; // Create shareable URL const baseUrl = window.location.origin; - const shareUrl = `${baseUrl}/document/${documentName}`; + const encodedPath = encodeDocumentPathForUrl(documentPath); + const shareUrl = `${baseUrl}/document/${encodedPath}`; // Copy to clipboard await navigator.clipboard.writeText(shareUrl); this.showNotification( - `🔗 Link copied: /document/${documentName}`, + `🔗 Link copied: /document/${encodedPath}`, "success", ); } catch (error) { @@ -189,8 +192,11 @@ export class ToolbarManager { try { const response = await fetch("/api/current-document"); const docInfo = await response.json(); - const documentName = docInfo.currentDocument || "live"; - const shareUrl = `${window.location.origin}/document/${documentName}`; + const documentPath = + docInfo.boundRelativePath || + docInfo.currentDocument || + "live"; + const shareUrl = `${window.location.origin}/document/${encodeDocumentPathForUrl(documentPath)}`; // Show URL in prompt as fallback prompt("Copy this shareable URL:", shareUrl); diff --git a/ts/packages/agents/markdown/src/view/site/utils.ts b/ts/packages/agents/markdown/src/view/site/utils.ts index 1998b738b5..53215f5939 100644 --- a/ts/packages/agents/markdown/src/view/site/utils.ts +++ b/ts/packages/agents/markdown/src/view/site/utils.ts @@ -93,9 +93,11 @@ export function hasClass(element: HTMLElement, className: string): boolean { * This ensures we get accurate markdown formatting and position information */ export async function getMarkdownFromEditor(editor: Editor): Promise { - if (!editor) return ""; + if (!editor) { + throw new Error("Cannot serialize Markdown without an editor"); + } - return new Promise((resolve) => { + return new Promise((resolve, reject) => { try { editor.action((ctx) => { const view = ctx.get(editorViewCtx); @@ -104,12 +106,11 @@ export async function getMarkdownFromEditor(editor: Editor): Promise { resolve(markdown); }); } catch (error) { - console.warn("Failed to serialize markdown from editor:", error); - // Fallback to text content if serializer fails - editor.action((ctx) => { - const view = ctx.get(editorViewCtx); - resolve(view.state.doc.textContent || ""); - }); + reject( + error instanceof Error + ? error + : new Error("Failed to serialize Markdown"), + ); } }); } diff --git a/ts/packages/agents/markdown/test/browserPersistence.spec.ts b/ts/packages/agents/markdown/test/browserPersistence.spec.ts new file mode 100644 index 0000000000..125ea23536 --- /dev/null +++ b/ts/packages/agents/markdown/test/browserPersistence.spec.ts @@ -0,0 +1,340 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { fileURLToPath } from "node:url"; +import { jest } from "@jest/globals"; +import { createServer, type ViteDevServer } from "vite"; + +const packageRoot = fileURLToPath(new URL("../../", import.meta.url)); + +describe("browser document persistence", () => { + let vite: ViteDevServer; + let DocumentManager: new () => any; + let CollaborationManager: new () => any; + const originalFetch = globalThis.fetch; + + beforeAll(async () => { + vite = await createServer({ + root: packageRoot, + appType: "custom", + logLevel: "silent", + server: { middlewareMode: true }, + }); + ({ DocumentManager } = await vite.ssrLoadModule( + "/src/view/site/core/document-manager.ts", + )); + ({ CollaborationManager } = await vite.ssrLoadModule( + "/src/view/site/core/collaboration-manager.ts", + )); + }); + + beforeEach(() => { + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + jest.restoreAllMocks(); + }); + + afterAll(async () => { + await vite.close(); + }); + + test("autosave and saveDocument persist serializer Markdown, including formatting-only edits", async () => { + let markdown = + "# Heading\n\nParagraph with **bold** text.\n\n```ts\nconst x = 1;\n```\n"; + const editor = createEditor( + () => markdown, + "HeadingParagraph with bold text.const x = 1;", + ); + const requests: Array> = []; + globalThis.fetch = (async (_input, init) => { + requests.push(JSON.parse(init?.body as string)); + return Response.json({ revision: `revision-${requests.length}` }); + }) as typeof fetch; + + const manager = new DocumentManager(); + manager.editorManager = { getEditor: () => editor }; + manager.isPrimaryClient = true; + manager.currentBindingToken = "binding-1"; + manager.currentDocumentId = "binding-1"; + manager.currentRevision = "revision-0"; + + await manager.performAutoSave(); + markdown = + "# Heading\n\nParagraph with *bold* text.\n\n```ts\nconst x = 1;\n```\n"; + await manager.performAutoSave(); + await manager.saveDocument(editor); + + expect(requests).toHaveLength(3); + expect(requests[0]).toMatchObject({ + content: + "# Heading\n\nParagraph with **bold** text.\n\n```ts\nconst x = 1;\n```\n", + bindingToken: "binding-1", + expectedRevision: "revision-0", + }); + expect(requests[1]).toMatchObject({ + content: + "# Heading\n\nParagraph with *bold* text.\n\n```ts\nconst x = 1;\n```\n", + expectedRevision: "revision-1", + }); + expect(requests[2]).toMatchObject({ + content: + "# Heading\n\nParagraph with *bold* text.\n\n```ts\nconst x = 1;\n```\n", + expectedRevision: "revision-2", + }); + }); + + test("serializer failure aborts persistence instead of falling back to textContent", async () => { + const editor = { + action(callback: (ctx: { get: () => unknown }) => void): void { + let getCount = 0; + callback({ + get: () => { + if (getCount++ === 0) { + return { + state: { + doc: { textContent: "formatting was lost" }, + }, + }; + } + throw new Error("serializer unavailable"); + }, + }); + }, + }; + const fetchMock = jest.fn(); + globalThis.fetch = fetchMock as typeof fetch; + + const manager = new DocumentManager(); + manager.currentBindingToken = "binding-1"; + manager.currentRevision = "revision-0"; + + await expect(manager.saveDocument(editor)).rejects.toThrow( + "serializer unavailable", + ); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + test("adopts current revisions from autosave and primary promotion events", async () => { + const manager = new DocumentManager(); + manager.currentBindingToken = "binding-1"; + manager.currentRevision = "revision-0"; + + await manager.handleSSEEvent({ + type: "autoSave", + bindingToken: "binding-1", + revision: "revision-1", + }); + expect(manager.currentRevision).toBe("revision-1"); + + await manager.handleSSEEvent({ + type: "autoSave", + bindingToken: "stale-binding", + revision: "wrong-revision", + }); + expect(manager.currentRevision).toBe("revision-1"); + + await manager.handleSSEEvent({ + type: "primaryElected", + bindingToken: "stale-binding", + revision: "wrong-revision", + }); + expect(manager.isPrimaryClient).toBe(false); + expect(manager.currentRevision).toBe("revision-1"); + + await manager.handleSSEEvent({ + type: "primaryElected", + bindingToken: "binding-1", + revision: "revision-2", + }); + expect(manager.isPrimaryClient).toBe(true); + expect(manager.currentRevision).toBe("revision-2"); + }); + + test("reconciles a 409 only when the same content is already on disk", async () => { + const markdown = "# Shared edit\n"; + const editor = createEditor(() => markdown, "Shared edit"); + const fetchMock = jest.fn(async () => + Response.json( + { + error: "Document content changed since it was loaded.", + content: markdown, + revision: "revision-from-primary", + }, + { status: 409 }, + ), + ); + globalThis.fetch = fetchMock as typeof fetch; + + const manager = new DocumentManager(); + manager.editorManager = { getEditor: () => editor }; + manager.currentBindingToken = "binding-1"; + manager.currentDocumentId = "binding-1"; + manager.currentRevision = "stale-revision"; + + await manager.performAutoSave(); + await manager.performAutoSave(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(manager.currentRevision).toBe("revision-from-primary"); + expect(manager.lastAutoSaveContent).toBe(markdown); + }); + + test("surfaces a divergent or malformed 409 without retrying or adopting its revision", async () => { + const markdown = "# Local edit\n"; + const editor = createEditor(() => markdown, "Local edit"); + const fetchMock = jest + .fn<() => Promise>() + .mockResolvedValueOnce( + Response.json( + { + error: "Document content changed since it was loaded.", + content: "# Newer disk edit\n", + revision: "newer-disk-revision", + }, + { status: 409 }, + ), + ) + .mockResolvedValue( + new Response("not-json", { + status: 409, + statusText: "Conflict", + }), + ); + globalThis.fetch = fetchMock as typeof fetch; + + const manager = new DocumentManager(); + manager.editorManager = { getEditor: () => editor }; + manager.currentBindingToken = "binding-1"; + manager.currentDocumentId = "binding-1"; + manager.currentRevision = "local-base-revision"; + + await manager.performAutoSave(); + await manager.performAutoSave(); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(manager.currentRevision).toBe("local-base-revision"); + expect(console.error).toHaveBeenCalledWith( + "[AUTO-SAVE] Error during auto-save:", + expect.objectContaining({ name: "DocumentWriteConflictError" }), + ); + + manager.lastConflictedAutoSaveContent = null; + await expect(manager.saveDocument(editor)).rejects.toThrow( + "changed on disk and was not overwritten", + ); + expect(manager.currentRevision).toBe("local-base-revision"); + }); + + test("matching bootstrap path skips a redundant switch request", async () => { + const fetchMock = jest.fn(async () => + Response.json({ + boundRelativePath: "team/2025/plan.md", + }), + ); + globalThis.fetch = fetchMock as typeof fetch; + + const manager = new DocumentManager(); + try { + await manager.initialize(); + await manager.switchToDocument("team/2025/plan.md"); + } finally { + manager.destroy(); + } + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledWith("/api/current-document"); + }); + + test("ignores switch responses superseded by a newer binding", async () => { + let resolveSwitch!: (response: Response) => void; + globalThis.fetch = jest.fn( + () => + new Promise((resolve) => { + resolveSwitch = resolve; + }), + ) as typeof fetch; + + const manager = new DocumentManager(); + const switching = manager.switchToDocument("old.md"); + await Promise.resolve(); + await manager.handleSSEEvent({ + type: "bindingBootstrap", + bindingToken: "new-binding", + documentId: "new-room", + boundRelativePath: "new.md", + revision: "new-revision", + }); + resolveSwitch( + Response.json({ + bindingToken: "old-binding", + documentId: "old-room", + boundRelativePath: "old.md", + revision: "old-revision", + content: "old content", + }), + ); + await switching; + + expect(manager.currentBindingToken).toBe("new-binding"); + expect(manager.currentDocumentId).toBe("new-room"); + expect(manager.currentBoundRelativePath).toBe("new.md"); + expect(manager.currentRevision).toBe("new-revision"); + }); + + test("ignores incomplete binding bootstrap data", async () => { + const manager = new DocumentManager(); + manager.currentBindingToken = "binding-1"; + manager.currentDocumentId = "room-1"; + manager.currentBoundRelativePath = "one.md"; + + await manager.handleSSEEvent({ + type: "bindingBootstrap", + bindingToken: "binding-2", + documentId: "room-2", + }); + + expect(manager.currentBindingToken).toBe("binding-1"); + expect(manager.currentDocumentId).toBe("room-1"); + expect(manager.currentBoundRelativePath).toBe("one.md"); + }); + + test("collaboration config uses the server room id, not the basename", async () => { + globalThis.fetch = (async () => + Response.json({ + websocketServerUrl: "ws://127.0.0.1:4321", + currentDocumentId: "opaque-binding-token", + currentDocument: "note", + documents: 1, + totalClients: 0, + })) as typeof fetch; + + const manager = new CollaborationManager(); + const config = await manager.getCollaborationConfig(); + + expect(config.documentId).toBe("opaque-binding-token"); + }); +}); + +function createEditor( + getMarkdown: () => string, + textContent: string, +): { + action(callback: (ctx: { get: () => unknown }) => void): void; +} { + return { + action(callback): void { + let getCount = 0; + callback({ + get: () => + getCount++ === 0 + ? { state: { doc: { textContent } } } + : () => getMarkdown(), + }); + }, + }; +} diff --git a/ts/packages/agents/markdown/test/viewService.spec.ts b/ts/packages/agents/markdown/test/viewService.spec.ts index 04ac554ed7..ead7bd30da 100644 --- a/ts/packages/agents/markdown/test/viewService.spec.ts +++ b/ts/packages/agents/markdown/test/viewService.spec.ts @@ -240,6 +240,7 @@ describe("markdown view service binding isolation", () => { bindingToken: first.bindingToken, documentId: first.bindingToken, boundRelativePath: "first.md", + clientRole: "primary", }); const switchedResponse = await fetch( @@ -307,6 +308,39 @@ describe("markdown view service binding isolation", () => { } }); + test("elects the next SSE client when the primary disconnects", async () => { + const port = await start({ "shared.md": "shared" }); + const bound = await bind("shared.md", "bind-shared"); + const primaryController = new AbortController(); + const secondaryController = new AbortController(); + const primaryEvents = await openSseEvents( + `http://127.0.0.1:${port}/events`, + primaryController.signal, + ); + const secondaryEvents = await openSseEvents( + `http://127.0.0.1:${port}/events`, + secondaryController.signal, + ); + try { + expect( + await waitForEvent(primaryEvents, "bindingBootstrap"), + ).toMatchObject({ clientRole: "primary" }); + expect( + await waitForEvent(secondaryEvents, "bindingBootstrap"), + ).toMatchObject({ clientRole: "secondary" }); + + primaryController.abort(); + expect( + await waitForEvent(secondaryEvents, "primaryElected"), + ).toMatchObject({ + bindingToken: bound.bindingToken, + }); + } finally { + primaryController.abort(); + secondaryController.abort(); + } + }); + test("evicts idle rooms after binding rotation", async () => { const port = await start({ "a.md": "A", "b.md": "B" }); await bind("a.md", "room-a"); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 23f2e1807f..0487530af1 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -2908,6 +2908,9 @@ importers: specifier: ^13.6.8 version: 13.6.27 devDependencies: + '@jest/globals': + specifier: ^29.7.0 + version: 29.7.0 '@milkdown/ctx': specifier: ^7.3.6 version: 7.13.1