From c1f5432c2b009a47c7a7ec974622d98aa3a17413 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 14:27:11 -0700 Subject: [PATCH 1/7] fix(markdown): secure browser document bindings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/markdown/src/agent/ipcTypes.ts | 34 +- .../src/view/route/collaborationManager.ts | 7 + .../agents/markdown/src/view/route/service.ts | 780 ++++++++++++------ .../agents/markdown/src/view/route/urlPath.ts | 48 ++ .../agents/markdown/test/urlPath.spec.ts | 42 + .../agents/markdown/test/viewService.spec.ts | 467 +++++++++++ 6 files changed, 1117 insertions(+), 261 deletions(-) create mode 100644 ts/packages/agents/markdown/src/view/route/urlPath.ts create mode 100644 ts/packages/agents/markdown/test/urlPath.spec.ts create mode 100644 ts/packages/agents/markdown/test/viewService.spec.ts diff --git a/ts/packages/agents/markdown/src/agent/ipcTypes.ts b/ts/packages/agents/markdown/src/agent/ipcTypes.ts index fd3778960a..8311d18262 100644 --- a/ts/packages/agents/markdown/src/agent/ipcTypes.ts +++ b/ts/packages/agents/markdown/src/agent/ipcTypes.ts @@ -3,6 +3,20 @@ // IPC Message Types for TypeAgent Communication +export interface SetFileMessage { + type: "setFile"; + workspaceRoot?: string; + relativePath?: string; +} + +export interface BindingUpdatedMessage { + type: "bindingUpdated"; + bindingToken: string | null; + boundFilePath: string | null; + boundRoot: string | null; + boundRelativePath: string | null; +} + // Agent ← View: UI command requests export interface UICommandMessage { type: "uiCommand"; @@ -46,7 +60,7 @@ export interface DocumentContentMessage { type: "documentContent"; requestId: string; content: string; - source?: "file" | "error"; + source?: "client-serializer" | "yjs-fallback" | "file-fallback" | "error"; error?: string; timestamp: number; bindingToken: string | null; @@ -100,6 +114,24 @@ export interface OperationsAppliedEvent { operationCount: number; } +export interface DocumentSnapshotEvent { + type: "documentSnapshot"; + bindingToken: string; + markdown: string; + revision: string; + timestamp: number; +} + +export interface BindingBootstrapEvent { + type: "bindingBootstrap"; + bindingToken: string | null; + documentId: string | null; + documentName: string | null; + boundRelativePath: string | null; + revision: string | null; + timestamp: number; +} + // Client ← View: Markdown content requests export interface RequestMarkdownMessage { type: "requestMarkdown"; diff --git a/ts/packages/agents/markdown/src/view/route/collaborationManager.ts b/ts/packages/agents/markdown/src/view/route/collaborationManager.ts index 42e68d463c..c68a146fb5 100644 --- a/ts/packages/agents/markdown/src/view/route/collaborationManager.ts +++ b/ts/packages/agents/markdown/src/view/route/collaborationManager.ts @@ -44,6 +44,13 @@ export class CollaborationManager { `Using existing Y.js document: ${documentId} ${filePath ? `(${filePath})` : "(memory-only)"}`, ); } + + forgetDocument(documentId: string): void { + this.documents.delete(documentId); + this.documentPaths.delete(documentId); + debug(`Forgot document: ${documentId}`); + } + getStats(): any { return { documents: this.documents.size, diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index 5eead72f02..b6ac9223ae 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -20,7 +20,6 @@ import registerDebug from "debug"; import sanitizeFilename from "sanitize-filename"; import { randomUUID } from "node:crypto"; import { isAllowedViewOrigin } from "./originAllowlist.js"; -import { resolvePathWithinRoot } from "./pathPolicy.js"; import { normalizeRelativeDocumentPath, resolveExistingFileWithinRoot, @@ -28,20 +27,25 @@ import { resolveWritableFileWithinRoot, } from "../../agent/documentPathPolicy.js"; import { + computeContentRevision, persistDocumentOperations, readBoundDocument, type DocumentBinding, } from "../../agent/documentUpdatePersistence.js"; +import { applyDocumentOperations } from "../../agent/documentOperations.js"; import type { DocumentOperation } from "../../agent/markdownOperationSchema.js"; const debug = registerDebug("typeagent:markdown:service"); +class ClientBindingMismatchError extends Error {} + const app: Express = express(); const LOOPBACK_HOST = "127.0.0.1"; const port = parseInt(process.argv[2]); if (isNaN(port)) { throw new Error("Port must be a number"); } +let boundPort = port; // Origin allowlist — runs before everything else so non-loopback // requests get HTTP 403 without consuming rate-limit budget or hitting @@ -76,8 +80,8 @@ app.get("/", (req: Request, res: Response) => { res.sendFile(path.join(staticPath, "index.html")); }); -// Document-specific route -app.get("/document/:documentName", (req: Request, res: Response) => { +// Document-specific route, including nested relative paths. +app.get(/^\/document\/.+/, (req: Request, res: Response) => { res.sendFile(path.join(staticPath, "index.html")); }); @@ -85,7 +89,8 @@ app.get("/document/:documentName", (req: Request, res: Response) => { app.get("/api/current-document", (req: Request, res: Response) => { res.json({ currentDocument: filePath ? path.basename(filePath, ".md") : null, - fullPath: filePath || null, + relativePath: boundRelativePath, + boundRelativePath, }); }); @@ -95,43 +100,23 @@ app.post( express.json(), (req: Request, res: Response) => { try { - const { documentName } = req.body; - - if (!documentName || !/^[a-zA-Z0-9_\- ]+$/.test(documentName)) { + const rawPath = + typeof req.body?.documentPath === "string" + ? req.body.documentPath + : req.body?.documentName; + const normalized = normalizeRelativeDocumentPath(rawPath); + if (normalized === undefined) { res.status(400).json({ - error: "Invalid document name. Only alphanumeric characters and underscores are allowed.", + error: "Invalid document path", }); return; } - - debug("Switch document called with parameter ", documentName); - - // Construct file path - const sanitizedDocumentName = sanitizeFilename(documentName); - - if (!sanitizedDocumentName) { - res.status(400).json({ error: "Invalid document name" }); - return; - } - - // Construct and normalize file path - const documentPath = resolvePathWithinRoot( - getValidatedCurrentRoot(), - `${sanitizedDocumentName}.md`, - ); - - debug("Sanitized document path ", documentPath); - // Verify that the file path is within the safe root directory - if (documentPath === undefined) { - res.status(403).json({ - error: "Access to the specified path is forbidden.", - }); - return; - } - + const relativePath = normalized.toLowerCase().endsWith(".md") + ? normalized + : `${normalized}.md`; let safeDocumentPath = resolveWritableFileWithinRoot( getValidatedCurrentRoot(), - documentPath, + relativePath, ); if (safeDocumentPath === undefined) { res.status(403).json({ @@ -141,43 +126,70 @@ app.post( } if (!fs.existsSync(safeDocumentPath)) { - // Create new document if it doesn't exist + const leafName = relativePath + .slice(0, -".md".length) + .split("/") + .pop() as string; + const displayName = sanitizeFilename(leafName) || leafName; fs.writeFileSync( safeDocumentPath, - `# ${documentName}\n\nThis is a new document.\n`, + `# ${displayName}\n\nThis is a new document.\n`, { flag: "wx" }, ); safeDocumentPath = fs.realpathSync(safeDocumentPath); } + const previousDocumentId = getCurrentDocumentId(); + const sameBinding = + filePath === safeDocumentPath && + boundRelativePath === relativePath && + bindingToken !== null; + const oldFilePath = filePath; filePath = safeDocumentPath; - boundRelativePath = `${sanitizedDocumentName}.md`; - bindingToken = randomUUID(); + boundRelativePath = relativePath; + if (!sameBinding) { + bindingToken = randomUUID(); + } notifyBindingToParent(); - // Initialize collaboration for new document - const documentId = sanitizedDocumentName; - collaborationManager.initializeDocument( - sanitizedDocumentName, - safeDocumentPath, - ); - - // Load content into collaboration manager - const content = fs.readFileSync(safeDocumentPath, "utf-8"); - debug("Raw content: ", content); - // collaborationManager.setDocumentContent(documentId, content); - + const documentId = getCurrentDocumentId(); + if (!sameBinding && previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } const ydoc = getAuthoritativeDocument(documentId); const ytext = ydoc.getText("content"); + const content = sameBinding + ? ytext.toString() + : fs.readFileSync(safeDocumentPath, "utf-8"); + if (!sameBinding) { + ytext.delete(0, ytext.length); + ytext.insert(0, content); + } + const revision = computeContentRevision( + fs.readFileSync(safeDocumentPath, "utf-8"), + ); - // Update document content - ytext.delete(0, ytext.length); - ytext.insert(0, content); + if (oldFilePath !== filePath) { + broadcastEvent({ + type: "documentChanged", + newDocumentId: documentId, + newDocumentName: path.basename(relativePath, ".md"), + bindingToken, + boundRelativePath, + revision, + timestamp: Date.now(), + }); + } res.json({ success: true, - documentName: documentName, - content: content, + documentName: path.basename(relativePath, ".md"), + documentId, + relativePath, + boundRelativePath, + bindingToken, + content, + revision, documentPath: safeDocumentPath, }); } catch (error) { @@ -196,6 +208,10 @@ app.post( (req: Request, res: Response) => { try { const { requestId, markdown, positionInfo, error } = req.body; + const responseToken = + typeof req.body?.bindingToken === "string" + ? req.body.bindingToken + : null; if (!requestId) { res.status(400).json({ error: "Request ID is required" }); @@ -214,6 +230,15 @@ app.post( if (error) { pendingRequest.reject(new Error(error)); + } else if ( + pendingRequest.expectedBindingToken !== null && + responseToken !== pendingRequest.expectedBindingToken + ) { + pendingRequest.reject( + new ClientBindingMismatchError( + "Client markdown response binding token changed", + ), + ); } else { pendingRequest.resolve({ markdown: markdown || "", @@ -254,7 +279,20 @@ const pendingCommands = new Map(); // Markdown request state let markdownRequestCounter = 0; -const pendingMarkdownRequests = new Map(); +type PendingMarkdownRequest = { + resolve: (value: { + markdown: string; + positionInfo: { + position: number; + selection?: { from: number; to: number }; + }; + }) => void; + reject: (error: Error) => void; + timeout: NodeJS.Timeout; + expectedBindingToken: string | null; +}; +const pendingMarkdownRequests = new Map(); +const activeApplyBindings = new Set(); const userHomeDir = os.homedir(); const INITIAL_ROOT_DIR = process.env.TYPEAGENT_MARKDOWN_ROOT || path.join(userHomeDir, "Documents"); @@ -287,6 +325,15 @@ function captureBindingSnapshot(): BindingSnapshot { return { bindingToken, currentRoot, filePath, boundRelativePath }; } +function bindingsDiffer(a: BindingSnapshot, b: BindingSnapshot): boolean { + return ( + a.bindingToken !== b.bindingToken || + a.currentRoot !== b.currentRoot || + a.filePath !== b.filePath || + a.boundRelativePath !== b.boundRelativePath + ); +} + function bindingError( message: Record, snapshot: BindingSnapshot, @@ -312,6 +359,99 @@ function bindingError( return undefined; } +function getCurrentDocumentId( + snapshot: BindingSnapshot = captureBindingSnapshot(), +): string { + return snapshot.filePath && snapshot.bindingToken + ? snapshot.bindingToken + : "default"; +} + +type BoundWriteValidation = + | { + ok: true; + snapshot: BindingSnapshot; + targetFilePath: string; + targetDocumentId: string; + } + | { + ok: false; + status: number; + error: string; + revision?: string; + content?: string; + }; + +function validateBoundWriteRequest(body: { + bindingToken?: unknown; + expectedRevision?: unknown; +}): BoundWriteValidation { + const snapshot = captureBindingSnapshot(); + if (!snapshot.filePath || !snapshot.boundRelativePath) { + return { + ok: false, + status: 409, + error: "No file is bound", + }; + } + if ( + snapshot.bindingToken === null || + body.bindingToken !== snapshot.bindingToken + ) { + return { + ok: false, + status: 409, + error: "bindingToken is missing or stale", + }; + } + + try { + const document = readBoundDocument({ + token: snapshot.bindingToken, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + if ( + typeof body.expectedRevision !== "string" || + body.expectedRevision !== document.revision + ) { + return { + ok: false, + status: 409, + error: + typeof body.expectedRevision === "string" + ? "Document content changed since it was loaded" + : "expectedRevision is required", + revision: document.revision, + content: document.content, + }; + } + return { + ok: true, + snapshot, + targetFilePath: document.filePath, + targetDocumentId: getCurrentDocumentId(snapshot), + }; + } catch (error) { + return { + ok: false, + status: 403, + error: error instanceof Error ? error.message : "Invalid binding", + }; + } +} + +function broadcastEvent(event: Record): void { + for (const client of clients) { + try { + client.write(`data: ${JSON.stringify(event)}\n\n`); + } catch (error) { + console.error("[SSE] Failed to send event:", error); + } + } +} + function notifyBindingToParent(): void { process.send?.({ type: "bindingUpdated", @@ -445,7 +585,10 @@ async function sendUICommandToAgentWithStreaming( /** * Request markdown content from connected client with retry logic */ -async function requestMarkdownFromClient(retryCount: number = 0): Promise<{ +async function requestMarkdownFromClient( + retryCount: number = 0, + snapshot: BindingSnapshot = captureBindingSnapshot(), +): Promise<{ markdown: string; positionInfo: { position: number; @@ -466,7 +609,7 @@ async function requestMarkdownFromClient(retryCount: number = 0): Promise<{ // Retry after a longer delay for better reliability setTimeout( () => { - requestMarkdownFromClient(retryCount + 1) + requestMarkdownFromClient(retryCount + 1, snapshot) .then(resolve) .catch(reject); }, @@ -478,7 +621,12 @@ async function requestMarkdownFromClient(retryCount: number = 0): Promise<{ }, 8000); // 8 second timeout (increased from 5s) // Store resolver for this request - pendingMarkdownRequests.set(requestId, { resolve, reject, timeout }); + pendingMarkdownRequests.set(requestId, { + resolve, + reject, + timeout, + expectedBindingToken: snapshot.bindingToken, + }); // Send request to clients via SSE debug( @@ -499,6 +647,8 @@ async function requestMarkdownFromClient(retryCount: number = 0): Promise<{ `data: ${JSON.stringify({ type: "requestMarkdown", requestId: requestId, + expectedBindingToken: snapshot.bindingToken, + expectedRelativePath: snapshot.boundRelativePath, timestamp: Date.now(), })}\n\n`, ); @@ -597,6 +747,7 @@ app.get("/document", (req: Request, res: Response) => { const ydoc = getAuthoritativeDocument(documentId); const ytext = ydoc.getText("content"); const content = ytext.toString(); + res.setHeader("X-Content-Revision", computeContentRevision(content)); debug( `Retrieved content from authoritative Y.js doc: ${documentId}, ${content.length} chars`, @@ -611,11 +762,17 @@ app.get("/document", (req: Request, res: Response) => { filePath, ); - // File mode: get content from authoritative document (which should be synced with file) - const documentId = path.basename(filePath, ".md"); + const documentId = getCurrentDocumentId(); const ydoc = getAuthoritativeDocument(documentId); const ytext = ydoc.getText("content"); const content = ytext.toString(); + const persistedContent = fs.existsSync(filePath) + ? fs.readFileSync(filePath, "utf-8") + : ""; + res.setHeader( + "X-Content-Revision", + computeContentRevision(persistedContent), + ); debug( `Retrieved content from authoritative Y.js doc: ${documentId}, ${content.length} chars`, @@ -630,24 +787,17 @@ app.get("/document", (req: Request, res: Response) => { } }); -// Save document from markdown text +// Save document from markdown text. app.post("/document", express.json(), (req: Request, res: Response) => { - const markdownContent = req.body.content || ""; + const markdownContent = + typeof req.body?.content === "string" ? req.body.content : ""; if (!filePath) { - // Memory-only mode: save to authoritative Y.js document - const documentId = "default"; // Use consistent document ID - + const documentId = "default"; const ydoc = getAuthoritativeDocument(documentId); const ytext = ydoc.getText("content"); - - // Replace entire content atomically ytext.delete(0, ytext.length); ytext.insert(0, markdownContent); - - debug( - `Saved content to authoritative Y.js doc: ${markdownContent.length} chars`, - ); res.json({ success: true, message: "Content saved to memory (no file mode)", @@ -657,32 +807,32 @@ app.post("/document", express.json(), (req: Request, res: Response) => { } try { - const writableFilePath = resolveWritableFileWithinRoot( - getValidatedCurrentRoot(), - filePath, - ); - if (writableFilePath === undefined) { - res.status(403).json({ error: "Access to the file is forbidden" }); + const validation = validateBoundWriteRequest(req.body ?? {}); + if (!validation.ok) { + res.status(validation.status).json({ + error: validation.error, + revision: validation.revision, + content: validation.content, + }); + return; + } + if (bindingsDiffer(captureBindingSnapshot(), validation.snapshot)) { + res.status(409).json({ error: "Binding rotated during request" }); return; } - // File mode: save to both authoritative document and file - const documentId = path.basename(writableFilePath, ".md"); - const ydoc = getAuthoritativeDocument(documentId); + const ydoc = getAuthoritativeDocument(validation.targetDocumentId); const ytext = ydoc.getText("content"); - - // Update authoritative document first ytext.delete(0, ytext.length); ytext.insert(0, markdownContent); - - // Then save to file - fs.writeFileSync(writableFilePath, markdownContent, "utf-8"); - filePath = writableFilePath; - - debug( - `Saved content to both Y.js doc and file: ${writableFilePath}, ${markdownContent.length} chars`, - ); - res.json({ success: true }); + fs.writeFileSync(validation.targetFilePath, markdownContent, "utf-8"); + filePath = validation.targetFilePath; + res.json({ + success: true, + filePath: validation.targetFilePath, + documentId: validation.targetDocumentId, + revision: computeContentRevision(markdownContent), + }); } catch (error) { res.status(500).json({ error: "Failed to save document", @@ -749,125 +899,52 @@ app.post("/api/ai-awareness", express.json(), (req: Request, res: Response) => { } }); -// Add auto-save endpoint +// Save browser content only when it still belongs to the active binding. app.post("/autosave", express.json(), (req: Request, res: Response) => { try { - const { content, filePath: requestFilePath, documentId } = req.body; - - if (!content && content !== "") { + const content = + typeof req.body?.content === "string" ? req.body.content : null; + if (content === null) { res.status(400).json({ error: "Content is required" }); return; } - debug( - `Auto-save request received for document: ${documentId}, path: ${requestFilePath}, content: ${content.length} chars`, - ); - - // Use the provided file path or fall back to current filePath - let sanitizedFilePath = sanitizeFilename(documentId || filePath); - if (!sanitizedFilePath.endsWith(".md")) { - sanitizedFilePath += ".md"; - } - - const resolvedFilePath = resolvePathWithinRoot( - getValidatedCurrentRoot(), - sanitizedFilePath, - ); - if (resolvedFilePath === undefined) { - res.status(403).json({ error: "Invalid file path" }); + const validation = validateBoundWriteRequest(req.body ?? {}); + if (!validation.ok) { + res.status(validation.status).json({ + error: validation.error, + revision: validation.revision, + content: validation.content, + }); return; } - const targetFilePath = resolvedFilePath; - const targetDocumentId = - documentId || - (sanitizedFilePath - ? path.basename(sanitizedFilePath, ".md") - : "default"); - - if (!targetFilePath) { - // Memory-only mode: save to authoritative Y.js document - debug( - `Memory-only mode auto-save to Y.js document: ${targetDocumentId}`, - ); - - const ydoc = getAuthoritativeDocument(targetDocumentId); - const ytext = ydoc.getText("content"); - - // Replace entire content atomically - ytext.delete(0, ytext.length); - ytext.insert(0, content); - - debug( - `Auto-save completed to Y.js document: ${targetDocumentId}, ${content.length} chars`, - ); - - // Notify clients via SSE - clients.forEach((client) => { - try { - client.write( - `data: ${JSON.stringify({ - type: "autoSave", - documentId: targetDocumentId, - contentLength: content.length, - timestamp: Date.now(), - })}\n\n`, - ); - } catch (error) { - console.error( - "[SSE] Failed to send auto-save event to client:", - error, - ); - } - }); - - res.json({ - success: true, - message: "Auto-saved to memory", - documentId: targetDocumentId, - }); + if (bindingsDiffer(captureBindingSnapshot(), validation.snapshot)) { + res.status(409).json({ error: "Binding rotated during request" }); return; } - // File mode: save to both authoritative document and file - const ydoc = getAuthoritativeDocument(targetDocumentId); + const ydoc = getAuthoritativeDocument(validation.targetDocumentId); const ytext = ydoc.getText("content"); - - // Update authoritative document first ytext.delete(0, ytext.length); ytext.insert(0, content); - - // Then save to file - // fs.writeFileSync(targetFilePath, content, "utf-8"); - - debug( - `Auto-save completed to both Y.js document and file: ${targetFilePath}, ${content.length} chars`, - ); - - // Notify clients via SSE - clients.forEach((client) => { - try { - client.write( - `data: ${JSON.stringify({ - type: "autoSave", - filePath: targetFilePath, - documentId: targetDocumentId, - contentLength: content.length, - timestamp: Date.now(), - })}\n\n`, - ); - } catch (error) { - console.error( - "[SSE] Failed to send auto-save event to client:", - error, - ); - } + fs.writeFileSync(validation.targetFilePath, content, "utf-8"); + const revision = computeContentRevision(content); + broadcastEvent({ + type: "autoSave", + filePath: validation.targetFilePath, + documentId: validation.targetDocumentId, + bindingToken: validation.snapshot.bindingToken, + revision, + contentLength: content.length, + timestamp: Date.now(), }); res.json({ success: true, message: "Auto-saved successfully", - filePath: targetFilePath, - documentId: targetDocumentId, + filePath: validation.targetFilePath, + documentId: validation.targetDocumentId, + revision, }); } catch (error) { console.error("[AUTO-SAVE] Auto-save failed:", error); @@ -903,18 +980,23 @@ app.post("/autosave", express.json(), (req: Request, res: Response) => { // Add collaboration info endpoint app.get("/collaboration/info", (req: Request, res: Response) => { const stats = collaborationManager.getStats(); - const currentDocument = filePath + const currentDocumentName = filePath ? path.basename(filePath, ".md") : "default"; + const currentDocumentId = getCurrentDocumentId(); debug( - `[COLLAB-INFO] Returning collaboration info - currentDocument: "${currentDocument}", filePath: ${filePath}`, + `[COLLAB-INFO] Returning collaboration info - currentDocument: "${currentDocumentName}", filePath: ${filePath}`, ); res.json({ ...stats, - websocketServerUrl: `ws://${LOOPBACK_HOST}:${port}`, - currentDocument: currentDocument, + websocketServerUrl: `ws://${LOOPBACK_HOST}:${boundPort}`, + currentDocument: currentDocumentName, + currentDocumentName, + currentDocumentId, + boundRelativePath, + bindingToken, }); }); @@ -939,7 +1021,7 @@ app.post("/file/load", express.json(), (req: Request, res: Response) => { return; } - // Set new file path + const previousDocumentId = getCurrentDocumentId(); filePath = resolvedPath; boundRelativePath = path .relative(currentRoot, resolvedPath) @@ -948,18 +1030,23 @@ app.post("/file/load", express.json(), (req: Request, res: Response) => { bindingToken = randomUUID(); notifyBindingToParent(); - // Initialize collaboration for new document - const documentId = path.basename(resolvedPath, ".md"); + const documentId = getCurrentDocumentId(); + if (previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } collaborationManager.initializeDocument(documentId, resolvedPath); - // Load content into collaboration manager const content = fs.readFileSync(resolvedPath, "utf-8"); collaborationManager.setDocumentContent(documentId, content); res.json({ success: true, fileName: path.basename(newFilePath), - content: content, + documentId, + boundRelativePath, + bindingToken, + content, + revision: computeContentRevision(content), }); } catch (error) { res.status(500).json({ @@ -1571,6 +1658,30 @@ 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}`); + } + } + res.write( + `data: ${JSON.stringify({ + type: "bindingBootstrap", + bindingToken, + documentId: filePath ? getCurrentDocumentId() : null, + documentName: filePath ? path.basename(filePath, ".md") : null, + boundRelativePath, + revision, + timestamp: Date.now(), + })}\n\n`, + ); req.on("close", () => { clients = clients.filter((client) => client !== res); @@ -1608,15 +1719,28 @@ process.on("message", async (message: any) => { return; } + if ( + currentRoot === nextRoot && + filePath === resolvedFilePath && + boundRelativePath === relativePath && + bindingToken !== null + ) { + notifyBindingToParent(); + return; + } + const oldFilePath = filePath; + const previousDocumentId = getCurrentDocumentId(); currentRoot = nextRoot; filePath = resolvedFilePath; boundRelativePath = relativePath; bindingToken = randomUUID(); notifyBindingToParent(); - // Initialize collaboration for this document using authoritative document - const documentId = path.basename(relativePath, ".md"); + const documentId = getCurrentDocumentId(); + if (previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } // Get or create the authoritative Y.js document const ydoc = getAuthoritativeDocument(documentId); @@ -1641,20 +1765,20 @@ process.on("message", async (message: any) => { // Notify frontend clients if the document has changed if (oldFilePath !== filePath) { - // Send SSE notification to all clients to switch rooms - clients.forEach((client) => { - client.write( - `data: ${JSON.stringify({ - type: "documentChanged", - newDocumentId: documentId, - newDocumentName: path.basename(relativePath, ".md"), - bindingToken, - timestamp: Date.now(), - })}\n\n`, - ); + broadcastEvent({ + type: "documentChanged", + newDocumentId: documentId, + newDocumentName: path.basename(relativePath, ".md"), + bindingToken, + boundRelativePath, + revision: computeContentRevision( + ydoc.getText("content").toString(), + ), + timestamp: Date.now(), }); } } else { + const previousDocumentId = getCurrentDocumentId(); // No file mode - initialize with default content using authoritative document filePath = null; boundRelativePath = null; @@ -1663,6 +1787,9 @@ process.on("message", async (message: any) => { debug("Running in memory-only mode (no file)"); const documentId = "default"; + if (previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } // Get or create authoritative Y.js document for memory-only mode const ydoc = getAuthoritativeDocument(documentId); @@ -1739,12 +1866,12 @@ Start typing to see the editor in action! const requestId = typeof message.requestId === "string" ? message.requestId : ""; const snapshot = captureBindingSnapshot(); + let activeApplyKey: string | undefined; try { if ( !Array.isArray(message.operations) || !snapshot.filePath || - !snapshot.boundRelativePath || - typeof message.expectedRevision !== "string" + !snapshot.boundRelativePath ) { throw new Error("Invalid document update request"); } @@ -1760,6 +1887,16 @@ Start typing to see the editor in action! }); return; } + if (typeof message.expectedRevision !== "string") { + throw new Error("Invalid document update request"); + } + activeApplyKey = snapshot.bindingToken ?? "memory"; + if (activeApplyBindings.has(activeApplyKey)) { + throw new Error( + "Another document update is already in progress for this binding", + ); + } + activeApplyBindings.add(activeApplyKey); const binding: DocumentBinding = { token: snapshot.bindingToken ?? undefined, @@ -1767,35 +1904,107 @@ Start typing to see the editor in action! relativePath: snapshot.boundRelativePath, filePath: snapshot.filePath, }; - const persisted = persistDocumentOperations( - binding, - message.operations as DocumentOperation[], - { - bindingToken: - typeof message.expectedBindingToken === "string" - ? message.expectedBindingToken - : undefined, - root: - typeof message.expectedRoot === "string" - ? message.expectedRoot - : undefined, - relativePath: - typeof message.expectedRelativePath === "string" - ? message.expectedRelativePath - : undefined, - revision: message.expectedRevision, - updatedRevision: - typeof message.expectedUpdatedRevision === "string" - ? message.expectedUpdatedRevision - : undefined, - }, - ); + const expected = { + bindingToken: + typeof message.expectedBindingToken === "string" + ? message.expectedBindingToken + : undefined, + root: + typeof message.expectedRoot === "string" + ? message.expectedRoot + : undefined, + relativePath: + typeof message.expectedRelativePath === "string" + ? message.expectedRelativePath + : undefined, + revision: message.expectedRevision, + updatedRevision: + typeof message.expectedUpdatedRevision === "string" + ? message.expectedUpdatedRevision + : undefined, + }; + let persisted; + if (clients.length === 0) { + persisted = persistDocumentOperations( + binding, + message.operations as DocumentOperation[], + expected, + ); + } else { + const persistedRevisionBeforeRead = + readBoundDocument(binding).revision; + const response = await requestMarkdownFromClient(0, snapshot); + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error("Document binding changed during read"); + } + const baseRevision = computeContentRevision(response.markdown); + const alreadyApplied = + expected.updatedRevision !== undefined && + expected.updatedRevision === baseRevision; + if (!alreadyApplied && expected.revision !== baseRevision) { + throw new Error( + "Document changed between read and apply (revision mismatch)", + ); + } + const content = alreadyApplied + ? response.markdown + : applyDocumentOperations( + response.markdown, + message.operations as DocumentOperation[], + ); + const revision = computeContentRevision(content); + if ( + expected.updatedRevision !== undefined && + expected.updatedRevision !== revision + ) { + throw new Error( + "Updated document revision does not match operations", + ); + } + const writableFilePath = resolveWritableFileWithinRoot( + snapshot.currentRoot, + snapshot.boundRelativePath, + ); + if ( + writableFilePath === undefined || + path.relative(writableFilePath, snapshot.filePath) !== "" + ) { + throw new Error("Document binding path changed"); + } + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error("Document binding changed before write"); + } + if ( + readBoundDocument(binding).revision !== + persistedRevisionBeforeRead + ) { + throw new Error( + "Document changed during browser read (revision mismatch)", + ); + } + fs.writeFileSync(writableFilePath, content, "utf-8"); + persisted = { + content, + revision, + alreadyApplied, + filePath: writableFilePath, + }; + } - const documentId = path.basename(snapshot.boundRelativePath, ".md"); + const documentId = getCurrentDocumentId(snapshot); collaborationManager.setDocumentContent( documentId, persisted.content, ); + if (snapshot.bindingToken) { + broadcastEvent({ + type: "documentSnapshot", + bindingToken: snapshot.bindingToken, + markdown: persisted.content, + revision: persisted.revision, + timestamp: Date.now(), + }); + } process.send?.({ type: "operationsApplied", requestId, @@ -1811,11 +2020,17 @@ Start typing to see the editor in action! type: "operationsApplied", requestId, success: false, - identityMismatch: /binding|workspace root/.test(errorMessage), + identityMismatch: + error instanceof ClientBindingMismatchError || + /binding|workspace root/.test(errorMessage), revisionMismatch: /revision mismatch/.test(errorMessage), error: errorMessage, bindingToken: snapshot.bindingToken, }); + } finally { + if (activeApplyKey !== undefined) { + activeApplyBindings.delete(activeApplyKey); + } } } else if (message.type === "getDocumentContent") { const requestId = @@ -1843,30 +2058,49 @@ Start typing to see the editor in action! if (!snapshot.filePath || !snapshot.boundRelativePath) { throw new Error("No markdown document is bound"); } - const document = await readBoundDocument({ - token: snapshot.bindingToken ?? undefined, - root: snapshot.currentRoot, - relativePath: snapshot.boundRelativePath, - filePath: snapshot.filePath, - }); - if ( - bindingToken !== snapshot.bindingToken || - currentRoot !== snapshot.currentRoot || - filePath !== snapshot.filePath || - boundRelativePath !== snapshot.boundRelativePath - ) { - throw new Error("Document binding changed while reading"); + let content: string; + let source: "client-serializer" | "file-fallback"; + if (clients.length > 0) { + try { + content = (await requestMarkdownFromClient(0, snapshot)) + .markdown; + source = "client-serializer"; + } catch (error) { + if (error instanceof ClientBindingMismatchError) { + throw error; + } + const document = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + content = document.content; + source = "file-fallback"; + } + } else { + const document = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + content = document.content; + source = "file-fallback"; + } + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error("Document binding changed during read"); } process.send?.({ type: "documentContent", requestId, - content: document.content, - source: "file", + content, + source, bindingToken: snapshot.bindingToken, boundFilePath: snapshot.filePath, boundRoot: snapshot.currentRoot, boundRelativePath: snapshot.boundRelativePath, - revision: document.revision, + revision: computeContentRevision(content), timestamp: Date.now(), }); } catch (error) { @@ -1878,7 +2112,9 @@ Start typing to see the editor in action! content: "", source: "error", error: errorMessage, - identityMismatch: /binding|workspace root/.test(errorMessage), + identityMismatch: + error instanceof ClientBindingMismatchError || + /binding|workspace root/.test(errorMessage), bindingToken: snapshot.bindingToken, boundFilePath: snapshot.filePath, boundRoot: snapshot.filePath ? snapshot.currentRoot : null, @@ -2000,6 +2236,27 @@ function getAuthoritativeDocument(documentId: string): Y.Doc { return ydoc; } +function evictRoomIfIdle(documentId: string | null): void { + if ( + documentId === null || + documentId === "default" || + !docs.has(documentId) + ) { + return; + } + const attached = roomConnections.get(documentId); + if (attached && attached.size > 0) { + return; + } + + docs.get(documentId)?.destroy(); + docs.delete(documentId); + awarenessStates.delete(documentId); + roomConnections.delete(documentId); + roomAwarenessConnections.delete(documentId); + collaborationManager.forgetDocument(documentId); +} + // Helper function to setup a Yjs connection (compatible with y-websocket) function setupWSConnection(conn: any, req: any, roomName: string): void { debug(`Setting up WebSocket connection for room: ${roomName}`); @@ -2067,6 +2324,9 @@ function setupWSConnection(conn: any, req: any, roomName: string): void { debug( `Client disconnected from room: ${roomName}, ${connections.size} clients remaining`, ); + if (connections.size === 0 && roomName !== getCurrentDocumentId()) { + evictRoomIfIdle(roomName); + } } }; @@ -2374,7 +2634,7 @@ debug(`[SIGNAL] Y.js WebSocket server integrated`); // Bind only to loopback. Origin checks are not authentication and requests // from non-browser clients may legitimately omit the Origin header. server.listen(port, LOOPBACK_HOST, () => { - const boundPort = (server.address() as { port: number }).port; + boundPort = (server.address() as { port: number }).port; debug( `Express server with WebSocket support listening at http://${LOOPBACK_HOST}:${boundPort}`, ); diff --git a/ts/packages/agents/markdown/src/view/route/urlPath.ts b/ts/packages/agents/markdown/src/view/route/urlPath.ts new file mode 100644 index 0000000000..b072245363 --- /dev/null +++ b/ts/packages/agents/markdown/src/view/route/urlPath.ts @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +export function parseDocumentPathFromUrl(pathname: string): string | null { + const match = pathname.match(/^\/document\/(.+)$/); + if (match === null) { + return null; + } + const encoded = match[1].replace(/\/+$/, ""); + if (encoded.length === 0) { + return null; + } + + const segments: string[] = []; + for (const segment of encoded.split("/")) { + if (segment.length === 0) { + return null; + } + try { + const decoded = decodeURIComponent(segment); + if ( + decoded.length === 0 || + decoded.includes("/") || + decoded.includes("\\") + ) { + return null; + } + segments.push(decoded); + } catch { + return null; + } + } + return segments.join("/"); +} + +export function ensureMarkdownExtension(relativePath: string): string { + return relativePath.toLowerCase().endsWith(".md") + ? relativePath + : `${relativePath}.md`; +} + +export function encodeDocumentPathForUrl(relativePath: string): string { + const withoutExtension = relativePath.replace(/\.md$/i, ""); + return withoutExtension + .split("/") + .map((segment) => encodeURIComponent(segment)) + .join("/"); +} diff --git a/ts/packages/agents/markdown/test/urlPath.spec.ts b/ts/packages/agents/markdown/test/urlPath.spec.ts new file mode 100644 index 0000000000..0de06e5242 --- /dev/null +++ b/ts/packages/agents/markdown/test/urlPath.spec.ts @@ -0,0 +1,42 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { + encodeDocumentPathForUrl, + ensureMarkdownExtension, + parseDocumentPathFromUrl, +} from "../src/view/route/urlPath.js"; + +describe("markdown document URL paths", () => { + test("round-trips nested paths and independently encoded segments", () => { + expect(parseDocumentPathFromUrl("/document/team/2025/plan.md")).toBe( + "team/2025/plan.md", + ); + expect( + parseDocumentPathFromUrl("/document/team%20a/plan%20v2.md"), + ).toBe("team a/plan v2.md"); + expect(encodeDocumentPathForUrl("team a/plan v2.md")).toBe( + "team%20a/plan%20v2", + ); + }); + + test("rejects empty, malformed, and encoded-separator paths", () => { + for (const url of [ + "/", + "/document/", + "/document//foo", + "/document/foo//bar", + "/document/broken%GZ", + "/document/team%2Fplan.md", + "/document/team%5Cplan.md", + ]) { + expect(parseDocumentPathFromUrl(url)).toBeNull(); + } + }); + + test("normalizes the extension without flattening the path", () => { + expect(ensureMarkdownExtension("team/plan")).toBe("team/plan.md"); + expect(ensureMarkdownExtension("team/plan.md")).toBe("team/plan.md"); + expect(ensureMarkdownExtension("team/plan.MD")).toBe("team/plan.MD"); + }); +}); diff --git a/ts/packages/agents/markdown/test/viewService.spec.ts b/ts/packages/agents/markdown/test/viewService.spec.ts new file mode 100644 index 0000000000..04ac554ed7 --- /dev/null +++ b/ts/packages/agents/markdown/test/viewService.spec.ts @@ -0,0 +1,467 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { ChildProcess, fork } from "node:child_process"; +import fs from "node:fs"; +import { createConnection } from "node:net"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +type ServiceMessage = Record; + +const servicePath = fileURLToPath( + new URL("../view/route/service.js", import.meta.url), +); +const insertOperation = { + type: "insert", + position: 0, + content: [{ type: "text", text: "must-not-write" }], +}; + +describe("markdown view service binding isolation", () => { + let viewProcess: ChildProcess | undefined; + let root: string | undefined; + + afterEach(() => { + viewProcess?.kill(); + viewProcess = undefined; + if (root) { + fs.rmSync(root, { recursive: true, force: true }); + root = undefined; + } + }); + + async function start(files: Record): Promise { + root = fs.mkdtempSync(path.join(os.tmpdir(), "markdown-view-test-")); + for (const [relativePath, content] of Object.entries(files)) { + const filePath = path.join(root, relativePath); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync(filePath, content, "utf-8"); + } + viewProcess = fork(servicePath, ["0"], { + env: { ...process.env, TYPEAGENT_MARKDOWN_ROOT: root }, + stdio: ["ignore", "ignore", "ignore", "ipc"], + }); + const ready = await waitForMessage( + viewProcess, + (message) => message.type === "Success", + ); + return ready.port as number; + } + + async function bind( + relativePath: string, + requestId: string, + ): Promise { + viewProcess!.send({ + type: "setFile", + workspaceRoot: root, + relativePath, + }); + return sendAndWait( + viewProcess!, + { type: "getDocumentContent", requestId }, + (message) => + message.type === "documentContent" && + message.requestId === requestId, + ); + } + + test("rejects stale token, root, path, and revision writes", async () => { + await start({ "safe.md": "seed" }); + const bound = await bind("safe.md", "initial-read"); + expect(typeof bound.bindingToken).toBe("string"); + expect(typeof bound.revision).toBe("string"); + + const cases: Array<{ + name: string; + expectation: Record; + flag: string; + }> = [ + { + name: "token", + expectation: { expectedBindingToken: "stale-token" }, + flag: "identityMismatch", + }, + { + name: "root", + expectation: { + expectedRoot: path.join(root!, "different-root"), + }, + flag: "identityMismatch", + }, + { + name: "path", + expectation: { expectedRelativePath: "other.md" }, + flag: "identityMismatch", + }, + { + name: "revision", + expectation: { + expectedBindingToken: bound.bindingToken as string, + expectedRevision: "stale-revision", + }, + flag: "revisionMismatch", + }, + ]; + + for (const testCase of cases) { + const requestId = `reject-${testCase.name}`; + const response = await sendAndWait( + viewProcess!, + { + type: "applyLLMOperations", + requestId, + operations: [insertOperation], + ...testCase.expectation, + }, + (message) => + message.type === "operationsApplied" && + message.requestId === requestId, + ); + expect(response.success).toBe(false); + expect(response[testCase.flag]).toBe(true); + } + expect(fs.readFileSync(path.join(root!, "safe.md"), "utf-8")).toBe( + "seed", + ); + }); + + test("preserves the token when rebinding the same relative file", async () => { + await start({ "same.md": "seed" }); + const updates: string[] = []; + viewProcess!.on("message", (message: unknown) => { + if ( + isServiceMessage(message) && + message.type === "bindingUpdated" && + typeof message.bindingToken === "string" + ) { + updates.push(message.bindingToken); + } + }); + + const first = await bind("same.md", "same-1"); + const second = await bind("same.md", "same-2"); + await waitUntil(() => updates.length >= 2); + + expect(second.bindingToken).toBe(first.bindingToken); + expect(updates).toEqual([ + first.bindingToken as string, + first.bindingToken as string, + ]); + }); + + test("keeps nested same-basename documents in distinct rooms", async () => { + const port = await start({ + "a/note.md": "seed-a", + "b/note.md": "seed-b", + }); + const switchDocument = async (documentPath: string) => { + const response = await fetch( + `http://127.0.0.1:${port}/api/switch-document`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ documentPath }), + }, + ); + expect(response.ok).toBe(true); + return (await response.json()) as ServiceMessage; + }; + + const first = await switchDocument("a/note"); + const second = await switchDocument("b/note.md"); + expect(first.boundRelativePath).toBe("a/note.md"); + expect(second.boundRelativePath).toBe("b/note.md"); + expect(first.documentId).toBe(first.bindingToken); + expect(second.documentId).toBe(second.bindingToken); + expect(second.documentId).not.toBe(first.documentId); + + const rejected = await sendAndWait( + viewProcess!, + { + type: "applyLLMOperations", + requestId: "stale-sibling", + operations: [insertOperation], + expectedBindingToken: first.bindingToken, + expectedRevision: second.revision, + }, + (message) => + message.type === "operationsApplied" && + message.requestId === "stale-sibling", + ); + expect(rejected).toMatchObject({ + success: false, + identityMismatch: true, + }); + expect(fs.readFileSync(path.join(root!, "a", "note.md"), "utf-8")).toBe( + "seed-a", + ); + expect(fs.readFileSync(path.join(root!, "b", "note.md"), "utf-8")).toBe( + "seed-b", + ); + }); + + test("correlates concurrent reads by requestId", async () => { + await start({ "concurrent.md": "hello" }); + await bind("concurrent.md", "bind-concurrent"); + + const readA = sendAndWait( + viewProcess!, + { type: "getDocumentContent", requestId: "read-a" }, + (message) => message.requestId === "read-a", + ); + const readB = sendAndWait( + viewProcess!, + { type: "getDocumentContent", requestId: "read-b" }, + (message) => message.requestId === "read-b", + ); + const [a, b] = await Promise.all([readA, readB]); + expect(a.content).toBe("hello"); + expect(b.content).toBe("hello"); + expect(a.revision).toBe(b.revision); + }); + + test("emits bootstrap, binding-change, and snapshot SSE events", async () => { + const port = await start({ + "first.md": "first", + "nested/second.md": "second", + }); + const first = await bind("first.md", "bind-first"); + const controller = new AbortController(); + const events = await openSseEvents( + `http://127.0.0.1:${port}/events`, + controller.signal, + ); + try { + const bootstrap = await waitForEvent(events, "bindingBootstrap"); + expect(bootstrap).toMatchObject({ + bindingToken: first.bindingToken, + documentId: first.bindingToken, + boundRelativePath: "first.md", + }); + + const switchedResponse = await fetch( + `http://127.0.0.1:${port}/api/switch-document`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + documentPath: "nested/second.md", + }), + }, + ); + const switched = (await switchedResponse.json()) as ServiceMessage; + expect(switchedResponse.ok).toBe(true); + const changed = await waitForEvent(events, "documentChanged"); + expect(changed).toMatchObject({ + bindingToken: switched.bindingToken, + boundRelativePath: "nested/second.md", + }); + + const apply = sendAndWait( + viewProcess!, + { + type: "applyLLMOperations", + requestId: "snapshot-apply", + operations: [ + { + type: "insert", + position: 0, + content: [{ type: "text", text: "updated-" }], + }, + ], + expectedBindingToken: switched.bindingToken, + expectedRevision: switched.revision, + }, + (message) => message.requestId === "snapshot-apply", + ); + const markdownRequest = await waitForEvent( + events, + "requestMarkdown", + ); + const browserResponse = await fetch( + `http://127.0.0.1:${port}/api/markdown-response`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + requestId: markdownRequest.requestId, + markdown: "second", + positionInfo: { position: 0 }, + bindingToken: switched.bindingToken, + }), + }, + ); + expect(browserResponse.ok).toBe(true); + expect(await apply).toMatchObject({ success: true }); + expect( + await waitForEvent(events, "documentSnapshot"), + ).toMatchObject({ + bindingToken: switched.bindingToken, + markdown: "updated-second", + }); + } finally { + controller.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"); + const before = (await ( + await fetch(`http://127.0.0.1:${port}/collaboration/info`) + ).json()) as ServiceMessage; + await bind("b.md", "room-b"); + const after = (await ( + await fetch(`http://127.0.0.1:${port}/collaboration/info`) + ).json()) as ServiceMessage; + expect([before.documents, after.documents]).toEqual([1, 1]); + }); + + test("listens only on the IPv4 loopback interface", async () => { + const port = await start({}); + const info = (await ( + await fetch(`http://127.0.0.1:${port}/collaboration/info`) + ).json()) as ServiceMessage; + expect(info.websocketServerUrl).toBe(`ws://127.0.0.1:${port}`); + + const externalAddress = Object.values(os.networkInterfaces()) + .flat() + .find( + (entry) => + entry !== undefined && + !entry.internal && + entry.family === "IPv4", + )?.address; + if (externalAddress) { + await expect(canConnect(externalAddress, port)).resolves.toBe( + false, + ); + } + }); +}); + +function isServiceMessage(value: unknown): value is ServiceMessage { + return typeof value === "object" && value !== null; +} + +function waitForMessage( + child: ChildProcess, + predicate: (message: ServiceMessage) => boolean, +): Promise { + return new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + cleanup(); + reject(new Error("Timed out waiting for view service response")); + }, 10_000); + const onMessage = (value: unknown) => { + if (isServiceMessage(value) && predicate(value)) { + cleanup(); + resolve(value); + } + }; + const onExit = (code: number | null) => { + cleanup(); + reject(new Error(`View service exited with code ${code}`)); + }; + const cleanup = () => { + clearTimeout(timeout); + child.off("message", onMessage); + child.off("exit", onExit); + }; + child.on("message", onMessage); + child.on("exit", onExit); + }); +} + +function sendAndWait( + child: ChildProcess, + message: ServiceMessage, + predicate: (response: ServiceMessage) => boolean, +): Promise { + const response = waitForMessage(child, predicate); + child.send(message); + return response; +} + +async function openSseEvents( + url: string, + signal: AbortSignal, +): Promise { + const response = await fetch(url, { signal }); + if (!response.body) { + throw new Error("SSE response body missing"); + } + const events: ServiceMessage[] = []; + const reader = response.body.getReader(); + const decoder = new TextDecoder(); + void (async () => { + let buffered = ""; + while (!signal.aborted) { + const { value, done } = await reader.read(); + if (done) { + return; + } + buffered += decoder.decode(value, { stream: true }); + const chunks = buffered.split(/\r?\n\r?\n/); + buffered = chunks.pop() ?? ""; + for (const chunk of chunks) { + const data = chunk + .split(/\r?\n/) + .find((line) => line.startsWith("data: ")); + if (data) { + const event: unknown = JSON.parse(data.slice(6)); + if (isServiceMessage(event)) { + events.push(event); + } + } + } + } + })().catch((error: unknown) => { + if (!signal.aborted) { + throw error; + } + }); + return events; +} + +async function waitForEvent( + events: ServiceMessage[], + type: string, +): Promise { + let found: ServiceMessage | undefined; + await waitUntil(() => { + found = events.find((event) => event.type === type); + return found !== undefined; + }); + return found!; +} + +async function waitUntil( + predicate: () => boolean, + timeoutMs = 5000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) { + return; + } + await new Promise((resolve) => setTimeout(resolve, 20)); + } + throw new Error("Timed out waiting for condition"); +} + +function canConnect(host: string, port: number): Promise { + return new Promise((resolve) => { + const socket = createConnection({ host, port }); + const finish = (connected: boolean) => { + socket.destroy(); + resolve(connected); + }; + socket.setTimeout(500, () => finish(false)); + socket.once("connect", () => finish(true)); + socket.once("error", () => finish(false)); + }); +} From f07f9d5604b79c1cf18e725febf19bb969173986 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 3 Sep 2026 14:32:34 -0700 Subject: [PATCH 2/7] style(markdown): satisfy binding lint Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ts/packages/agents/markdown/src/view/route/service.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index b6ac9223ae..2b91905d12 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -271,7 +271,7 @@ let clients: any[] = []; let filePath: string | null = null; let boundRelativePath: string | null = null; let bindingToken: string | null = null; -let collaborationManager: CollaborationManager; +const collaborationManager = new CollaborationManager(); // UI Command routing state let commandCounter = 0; @@ -732,9 +732,6 @@ function handleStreamingChunkFromAgent( } } -// Initialize collaboration manager -collaborationManager = new CollaborationManager(); - // Get document as markdown text app.get("/document", (req: Request, res: Response) => { if (!filePath) { From 970bb9db487956d068fa4fa66686f839b8204ce0 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 10 Sep 2026 17:57:02 -0700 Subject: [PATCH 3/7] fix(markdown): harden browser binding synchronization Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/markdown/src/agent/ipcTypes.ts | 2 + .../src/agent/markdownActionHandler.ts | 18 +- .../agents/markdown/src/view/route/service.ts | 308 +++++++++++--- .../view/site/core/collaboration-manager.ts | 6 +- .../src/view/site/core/document-manager.ts | 380 +++++++++++++----- .../agents/markdown/src/view/site/index.ts | 24 +- .../markdown/src/view/site/tsconfig.json | 4 +- .../agents/markdown/src/view/site/types.ts | 1 + .../markdown/test/browserBinding.spec.ts | 221 ++++++++++ .../markdown/test/markdownService.spec.ts | 13 +- .../agents/markdown/test/viewService.spec.ts | 27 +- 11 files changed, 818 insertions(+), 186 deletions(-) create mode 100644 ts/packages/agents/markdown/test/browserBinding.spec.ts diff --git a/ts/packages/agents/markdown/src/agent/ipcTypes.ts b/ts/packages/agents/markdown/src/agent/ipcTypes.ts index 8311d18262..290d5a594c 100644 --- a/ts/packages/agents/markdown/src/agent/ipcTypes.ts +++ b/ts/packages/agents/markdown/src/agent/ipcTypes.ts @@ -68,6 +68,7 @@ export interface DocumentContentMessage { boundRoot: string | null; boundRelativePath: string | null; revision: string | null; + readToken?: string; identityMismatch?: boolean; } @@ -82,6 +83,7 @@ export interface LLMOperationsMessage { expectedRelativePath?: string; expectedRevision: string; expectedUpdatedRevision?: string; + expectedReadToken?: string; } export interface OperationsAppliedMessage { diff --git a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts index 39e013b870..2b9e818b44 100644 --- a/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts +++ b/ts/packages/agents/markdown/src/agent/markdownActionHandler.ts @@ -448,6 +448,7 @@ async function handleStreamingMarkdownAction( content: markdownContent, binding, revision, + readToken, } = await readCurrentDocumentContent(actionContext); try { @@ -510,6 +511,7 @@ async function handleStreamingMarkdownAction( binding, revision, computeContentRevision(updatedContent), + readToken, ); } @@ -912,6 +914,7 @@ async function readCurrentDocumentContent( content: string; binding: CurrentDocumentBinding; revision: string; + readToken?: string; }> { const agentContext = actionContext.sessionContext.agentContext; const storage = actionContext.sessionContext.sessionStorage; @@ -953,6 +956,7 @@ async function readCurrentDocumentContent( token: agentContext.currentBindingToken, }, revision: response.revision ?? computeContentRevision(response.content), + ...(response.readToken ? { readToken: response.readToken } : {}), }; } @@ -962,6 +966,7 @@ async function applyOperationsForCurrentDocument( binding: CurrentDocumentBinding, revision: string, expectedUpdatedRevision?: string, + expectedReadToken?: string, ): Promise { const agentContext = actionContext.sessionContext.agentContext; const storage = actionContext.sessionContext.sessionStorage; @@ -1016,6 +1021,7 @@ async function applyOperationsForCurrentDocument( expectedRelativePath: binding.relativePath, expectedRevision: revision, expectedUpdatedRevision, + ...(expectedReadToken ? { expectedReadToken } : {}), }; const viewProcess = getCurrentDocumentViewProcess(agentContext); if (!viewProcess) { @@ -1073,7 +1079,7 @@ async function updateCurrentDocument( actionContext: ActionContext, agent: Awaited>, ): Promise { - const { content, binding, revision } = + const { content, binding, revision, readToken } = await readCurrentDocumentContent(actionContext); const response = await agent.updateDocument( content, @@ -1094,6 +1100,8 @@ async function updateCurrentDocument( response.data.operations, binding, revision, + undefined, + readToken, ); } return createActionResult( @@ -1155,6 +1163,7 @@ type ApplyExpectations = { expectedRelativePath: string; expectedRevision: string; expectedUpdatedRevision: string | undefined; + expectedReadToken?: string; }; type ApplyResult = { @@ -1189,7 +1198,7 @@ export async function sendOperationsToView( revisionMismatch: false, error: "View process operation timeout", }); - }, 15000); + }, 60_000); const responseHandler = (message: Record) => { if ( @@ -1226,6 +1235,7 @@ type ViewDocumentContentResponse = { content: string; bindingToken: string | null; revision: string | null; + readToken: string | undefined; identityMismatch: boolean; error: string | undefined; }; @@ -1269,6 +1279,10 @@ export async function getDocumentContentFromView( typeof message.revision === "string" ? message.revision : null, + readToken: + typeof message.readToken === "string" + ? message.readToken + : undefined, identityMismatch: message.identityMismatch === true, error: typeof message.error === "string" diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index 2b91905d12..b263483e06 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -150,6 +150,8 @@ app.post( if (!sameBinding) { bindingToken = randomUUID(); } + bindingGeneration++; + bindingSource = "managed"; notifyBindingToParent(); const documentId = getCurrentDocumentId(); @@ -271,6 +273,8 @@ let clients: any[] = []; let filePath: string | null = null; let boundRelativePath: string | null = null; let bindingToken: string | null = null; +let bindingGeneration = 0; +let bindingSource: "http-load" | "managed" | null = null; const collaborationManager = new CollaborationManager(); // UI Command routing state @@ -293,6 +297,14 @@ type PendingMarkdownRequest = { }; const pendingMarkdownRequests = new Map(); const activeApplyBindings = new Set(); +const browserReadDiskRevisions = new Map< + string, + { + bindingToken: string | null; + contentRevision: string; + diskRevision: string; + } +>(); const userHomeDir = os.homedir(); const INITIAL_ROOT_DIR = process.env.TYPEAGENT_MARKDOWN_ROOT || path.join(userHomeDir, "Documents"); @@ -316,18 +328,26 @@ function getValidatedCurrentRoot(): string { type BindingSnapshot = { bindingToken: string | null; + bindingGeneration: number; currentRoot: string; filePath: string | null; boundRelativePath: string | null; }; function captureBindingSnapshot(): BindingSnapshot { - return { bindingToken, currentRoot, filePath, boundRelativePath }; + return { + bindingToken, + bindingGeneration, + currentRoot, + filePath, + boundRelativePath, + }; } function bindingsDiffer(a: BindingSnapshot, b: BindingSnapshot): boolean { return ( a.bindingToken !== b.bindingToken || + a.bindingGeneration !== b.bindingGeneration || a.currentRoot !== b.currentRoot || a.filePath !== b.filePath || a.boundRelativePath !== b.boundRelativePath @@ -382,10 +402,10 @@ type BoundWriteValidation = content?: string; }; -function validateBoundWriteRequest(body: { +async function validateBoundWriteRequest(body: { bindingToken?: unknown; expectedRevision?: unknown; -}): BoundWriteValidation { +}): Promise { const snapshot = captureBindingSnapshot(); if (!snapshot.filePath || !snapshot.boundRelativePath) { return { @@ -406,7 +426,7 @@ function validateBoundWriteRequest(body: { } try { - const document = readBoundDocument({ + const document = await readBoundDocument({ token: snapshot.bindingToken, root: snapshot.currentRoot, relativePath: snapshot.boundRelativePath, @@ -754,6 +774,14 @@ app.get("/document", (req: Request, res: Response) => { } try { + const expectedBindingToken = req.get("X-Binding-Token"); + if ( + expectedBindingToken !== undefined && + expectedBindingToken !== bindingToken + ) { + res.status(409).json({ error: "Document binding changed" }); + return; + } debug( "[FILE_MODE] File provided when resolving the /document call " + filePath, @@ -763,13 +791,10 @@ app.get("/document", (req: Request, res: Response) => { const ydoc = getAuthoritativeDocument(documentId); const ytext = ydoc.getText("content"); const content = ytext.toString(); - const persistedContent = fs.existsSync(filePath) - ? fs.readFileSync(filePath, "utf-8") - : ""; - res.setHeader( - "X-Content-Revision", - computeContentRevision(persistedContent), - ); + res.setHeader("X-Content-Revision", computeContentRevision(content)); + if (bindingToken) { + res.setHeader("X-Binding-Token", bindingToken); + } debug( `Retrieved content from authoritative Y.js doc: ${documentId}, ${content.length} chars`, @@ -785,7 +810,7 @@ app.get("/document", (req: Request, res: Response) => { }); // Save document from markdown text. -app.post("/document", express.json(), (req: Request, res: Response) => { +app.post("/document", express.json(), async (req: Request, res: Response) => { const markdownContent = typeof req.body?.content === "string" ? req.body.content : ""; @@ -803,8 +828,26 @@ app.post("/document", express.json(), (req: Request, res: Response) => { return; } + let acquiredWriteKey: string | undefined; try { - const validation = validateBoundWriteRequest(req.body ?? {}); + const activeWriteKey = + typeof req.body?.bindingToken === "string" + ? req.body.bindingToken + : undefined; + if ( + activeWriteKey !== undefined && + activeApplyBindings.has(activeWriteKey) + ) { + res.status(409).json({ + error: "Another document update is already in progress for this binding", + }); + return; + } + if (activeWriteKey !== undefined) { + activeApplyBindings.add(activeWriteKey); + acquiredWriteKey = activeWriteKey; + } + const validation = await validateBoundWriteRequest(req.body ?? {}); if (!validation.ok) { res.status(validation.status).json({ error: validation.error, @@ -818,16 +861,17 @@ app.post("/document", express.json(), (req: Request, res: Response) => { return; } + fs.writeFileSync(validation.targetFilePath, markdownContent, "utf-8"); const ydoc = getAuthoritativeDocument(validation.targetDocumentId); const ytext = ydoc.getText("content"); ytext.delete(0, ytext.length); ytext.insert(0, markdownContent); - fs.writeFileSync(validation.targetFilePath, markdownContent, "utf-8"); filePath = validation.targetFilePath; res.json({ success: true, filePath: validation.targetFilePath, documentId: validation.targetDocumentId, + bindingToken: validation.snapshot.bindingToken, revision: computeContentRevision(markdownContent), }); } catch (error) { @@ -835,6 +879,10 @@ app.post("/document", express.json(), (req: Request, res: Response) => { error: "Failed to save document", details: error, }); + } finally { + if (acquiredWriteKey !== undefined) { + activeApplyBindings.delete(acquiredWriteKey); + } } }); @@ -897,7 +945,8 @@ app.post("/api/ai-awareness", express.json(), (req: Request, res: Response) => { }); // Save browser content only when it still belongs to the active binding. -app.post("/autosave", express.json(), (req: Request, res: Response) => { +app.post("/autosave", express.json(), async (req: Request, res: Response) => { + let acquiredWriteKey: string | undefined; try { const content = typeof req.body?.content === "string" ? req.body.content : null; @@ -906,7 +955,24 @@ app.post("/autosave", express.json(), (req: Request, res: Response) => { return; } - const validation = validateBoundWriteRequest(req.body ?? {}); + const activeWriteKey = + typeof req.body?.bindingToken === "string" + ? req.body.bindingToken + : undefined; + if ( + activeWriteKey !== undefined && + activeApplyBindings.has(activeWriteKey) + ) { + res.status(409).json({ + error: "Another document update is already in progress for this binding", + }); + return; + } + if (activeWriteKey !== undefined) { + activeApplyBindings.add(activeWriteKey); + acquiredWriteKey = activeWriteKey; + } + const validation = await validateBoundWriteRequest(req.body ?? {}); if (!validation.ok) { res.status(validation.status).json({ error: validation.error, @@ -920,11 +986,11 @@ app.post("/autosave", express.json(), (req: Request, res: Response) => { return; } + fs.writeFileSync(validation.targetFilePath, content, "utf-8"); const ydoc = getAuthoritativeDocument(validation.targetDocumentId); const ytext = ydoc.getText("content"); ytext.delete(0, ytext.length); ytext.insert(0, content); - fs.writeFileSync(validation.targetFilePath, content, "utf-8"); const revision = computeContentRevision(content); broadcastEvent({ type: "autoSave", @@ -941,6 +1007,7 @@ app.post("/autosave", express.json(), (req: Request, res: Response) => { message: "Auto-saved successfully", filePath: validation.targetFilePath, documentId: validation.targetDocumentId, + bindingToken: validation.snapshot.bindingToken, revision, }); } catch (error) { @@ -971,6 +1038,10 @@ app.post("/autosave", express.json(), (req: Request, res: Response) => { error: "Auto-save failed", details: error instanceof Error ? error.message : error, }); + } finally { + if (acquiredWriteKey !== undefined) { + activeApplyBindings.delete(acquiredWriteKey); + } } }); @@ -1025,16 +1096,18 @@ app.post("/file/load", express.json(), (req: Request, res: Response) => { .split(path.sep) .join("/"); bindingToken = randomUUID(); + bindingGeneration++; + bindingSource = "http-load"; notifyBindingToParent(); const documentId = getCurrentDocumentId(); if (previousDocumentId !== documentId) { evictRoomIfIdle(previousDocumentId); } - collaborationManager.initializeDocument(documentId, resolvedPath); - const content = fs.readFileSync(resolvedPath, "utf-8"); - collaborationManager.setDocumentContent(documentId, content); + const ytext = getAuthoritativeDocument(documentId).getText("content"); + ytext.delete(0, ytext.length); + ytext.insert(0, content); res.json({ success: true, @@ -1648,41 +1721,66 @@ erDiagram }; } -app.get("/events", (req: Request, res: Response) => { +app.get("/events", async (req: Request, res: Response) => { res.setHeader("Content-Type", "text/event-stream"); res.setHeader("Cache-Control", "no-cache"); res.setHeader("Connection", "keep-alive"); res.flushHeaders(); - clients.push(res); + let closed = false; + req.on("close", () => { + closed = true; + clients = clients.filter((client) => client !== res); + }); + + let bootstrap = captureBindingSnapshot(); 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}`); + for (let attempt = 0; attempt < 3; attempt++) { + bootstrap = captureBindingSnapshot(); + revision = null; + if (bootstrap.filePath && bootstrap.boundRelativePath) { + try { + revision = ( + await readBoundDocument({ + token: bootstrap.bindingToken ?? undefined, + root: bootstrap.currentRoot, + relativePath: bootstrap.boundRelativePath, + filePath: bootstrap.filePath, + }) + ).revision; + } catch (error) { + debug(`Unable to read binding bootstrap revision: ${error}`); + res.end(); + return; + } + } + if (closed) { + return; + } + if (!bindingsDiffer(captureBindingSnapshot(), bootstrap)) { + break; + } + if (attempt === 2) { + res.end(); + return; } } res.write( `data: ${JSON.stringify({ type: "bindingBootstrap", - bindingToken, - documentId: filePath ? getCurrentDocumentId() : null, - documentName: filePath ? path.basename(filePath, ".md") : null, - boundRelativePath, + bindingToken: bootstrap.bindingToken, + documentId: bootstrap.filePath + ? getCurrentDocumentId(bootstrap) + : null, + documentName: bootstrap.filePath + ? path.basename(bootstrap.filePath, ".md") + : null, + boundRelativePath: bootstrap.boundRelativePath, revision, timestamp: Date.now(), })}\n\n`, ); - - req.on("close", () => { - clients = clients.filter((client) => client !== res); - }); + clients.push(res); }); // Serve static files AFTER API routes to avoid conflicts @@ -1720,8 +1818,10 @@ process.on("message", async (message: any) => { currentRoot === nextRoot && filePath === resolvedFilePath && boundRelativePath === relativePath && - bindingToken !== null + bindingToken !== null && + bindingSource === "managed" ) { + bindingGeneration++; notifyBindingToParent(); return; } @@ -1732,6 +1832,8 @@ process.on("message", async (message: any) => { filePath = resolvedFilePath; boundRelativePath = relativePath; bindingToken = randomUUID(); + bindingGeneration++; + bindingSource = "managed"; notifyBindingToParent(); const documentId = getCurrentDocumentId(); @@ -1780,6 +1882,8 @@ process.on("message", async (message: any) => { filePath = null; boundRelativePath = null; bindingToken = null; + bindingGeneration++; + bindingSource = null; notifyBindingToParent(); debug("Running in memory-only mode (no file)"); @@ -1863,7 +1967,7 @@ Start typing to see the editor in action! const requestId = typeof message.requestId === "string" ? message.requestId : ""; const snapshot = captureBindingSnapshot(); - let activeApplyKey: string | undefined; + let acquiredApplyKey: string | undefined; try { if ( !Array.isArray(message.operations) || @@ -1887,13 +1991,14 @@ Start typing to see the editor in action! if (typeof message.expectedRevision !== "string") { throw new Error("Invalid document update request"); } - activeApplyKey = snapshot.bindingToken ?? "memory"; + const activeApplyKey = snapshot.bindingToken ?? "memory"; if (activeApplyBindings.has(activeApplyKey)) { throw new Error( "Another document update is already in progress for this binding", ); } activeApplyBindings.add(activeApplyKey); + acquiredApplyKey = activeApplyKey; const binding: DocumentBinding = { token: snapshot.bindingToken ?? undefined, @@ -1921,18 +2026,46 @@ Start typing to see the editor in action! : undefined, }; let persisted; - if (clients.length === 0) { + let browserBaseMarkdown: string | undefined; + if ( + clients.length === 0 || + typeof message.expectedReadToken !== "string" + ) { persisted = persistDocumentOperations( binding, message.operations as DocumentOperation[], expected, ); } else { - const persistedRevisionBeforeRead = - readBoundDocument(binding).revision; + const persistedRevisionBeforeRead = ( + await readBoundDocument(binding) + ).revision; + const readToken = + typeof message.expectedReadToken === "string" + ? message.expectedReadToken + : ""; + const readSnapshot = browserReadDiskRevisions.get(readToken); + browserReadDiskRevisions.delete(readToken); + if ( + readSnapshot === undefined || + readSnapshot.bindingToken !== snapshot.bindingToken || + readSnapshot.contentRevision !== expected.revision || + readSnapshot.diskRevision !== persistedRevisionBeforeRead + ) { + throw new Error( + "Document changed since the browser snapshot was read (revision mismatch)", + ); + } const response = await requestMarkdownFromClient(0, snapshot); + browserBaseMarkdown = response.markdown; + const authoritativeDocument = getAuthoritativeDocument( + getCurrentDocumentId(snapshot), + ); + const browserStateVector = Y.encodeStateVector( + authoritativeDocument, + ); if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { - throw new Error("Document binding changed during read"); + throw new Error("Document binding changed while reading"); } const baseRevision = computeContentRevision(response.markdown); const alreadyApplied = @@ -1971,20 +2104,43 @@ Start typing to see the editor in action! if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { throw new Error("Document binding changed before write"); } + const persistedRevisionAfterRead = ( + await readBoundDocument(binding) + ).revision; if ( - readBoundDocument(binding).revision !== - persistedRevisionBeforeRead + bindingsDiffer(captureBindingSnapshot(), snapshot) || + persistedRevisionAfterRead !== persistedRevisionBeforeRead ) { throw new Error( "Document changed during browser read (revision mismatch)", ); } - fs.writeFileSync(writableFilePath, content, "utf-8"); + const verifiedFilePath = resolveWritableFileWithinRoot( + snapshot.currentRoot, + snapshot.boundRelativePath, + ); + if ( + verifiedFilePath === undefined || + verifiedFilePath !== writableFilePath || + path.relative(verifiedFilePath, snapshot.filePath) !== "" + ) { + throw new Error("Document binding path changed"); + } + if ( + !Buffer.from( + Y.encodeStateVector(authoritativeDocument), + ).equals(Buffer.from(browserStateVector)) + ) { + throw new Error( + "Document changed after browser serialization (revision mismatch)", + ); + } + fs.writeFileSync(verifiedFilePath, content, "utf-8"); persisted = { content, revision, alreadyApplied, - filePath: writableFilePath, + filePath: verifiedFilePath, }; } @@ -1993,10 +2149,11 @@ Start typing to see the editor in action! documentId, persisted.content, ); - if (snapshot.bindingToken) { + if (snapshot.bindingToken && browserBaseMarkdown !== undefined) { broadcastEvent({ type: "documentSnapshot", bindingToken: snapshot.bindingToken, + baseMarkdown: browserBaseMarkdown, markdown: persisted.content, revision: persisted.revision, timestamp: Date.now(), @@ -2025,8 +2182,8 @@ Start typing to see the editor in action! bindingToken: snapshot.bindingToken, }); } finally { - if (activeApplyKey !== undefined) { - activeApplyBindings.delete(activeApplyKey); + if (acquiredApplyKey !== undefined) { + activeApplyBindings.delete(acquiredApplyKey); } } } else if (message.type === "getDocumentContent") { @@ -2057,10 +2214,29 @@ Start typing to see the editor in action! } let content: string; let source: "client-serializer" | "file-fallback"; + let browserDiskRevision: string | undefined; if (clients.length > 0) { try { + const diskBefore = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); content = (await requestMarkdownFromClient(0, snapshot)) .markdown; + const diskAfter = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + if (diskBefore.revision !== diskAfter.revision) { + throw new Error( + "Document changed during browser serialization (revision mismatch)", + ); + } + browserDiskRevision = diskBefore.revision; source = "client-serializer"; } catch (error) { if (error instanceof ClientBindingMismatchError) { @@ -2086,7 +2262,26 @@ Start typing to see the editor in action! source = "file-fallback"; } if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { - throw new Error("Document binding changed during read"); + throw new Error("Document binding changed while reading"); + } + const revision = computeContentRevision(content); + let readToken: string | undefined; + if (source === "client-serializer") { + if (browserDiskRevision === undefined) { + throw new Error( + "Browser document read is missing its disk revision", + ); + } + readToken = randomUUID(); + browserReadDiskRevisions.set(readToken, { + bindingToken: snapshot.bindingToken, + contentRevision: revision, + diskRevision: browserDiskRevision, + }); + const tokenToExpire = readToken; + setTimeout(() => { + browserReadDiskRevisions.delete(tokenToExpire); + }, 300_000).unref(); } process.send?.({ type: "documentContent", @@ -2097,7 +2292,8 @@ Start typing to see the editor in action! boundFilePath: snapshot.filePath, boundRoot: snapshot.currentRoot, boundRelativePath: snapshot.boundRelativePath, - revision: computeContentRevision(content), + revision, + readToken, timestamp: Date.now(), }); } catch (error) { 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..308c9819a0 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 @@ -184,9 +184,9 @@ export class CollaborationManager { websocketServerUrl: collabInfo.websocketServerUrl || COLLABORATION_CONFIG.DEFAULT_WEBSOCKET_URL, - documentId: collabInfo.currentDocument - ? collabInfo.currentDocument.replace(".md", "") - : COLLABORATION_CONFIG.DEFAULT_DOCUMENT_ID, + documentId: + collabInfo.currentDocumentId || + COLLABORATION_CONFIG.DEFAULT_DOCUMENT_ID, 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..4a7abd8a1c 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 @@ -5,15 +5,22 @@ import type { Editor } from "@milkdown/core"; import { editorViewCtx, parserCtx } from "@milkdown/core"; import { AI_CONFIG, DEFAULT_MARKDOWN_CONTENT, EDITOR_CONFIG } from "../config"; import { getMarkdownFromEditor, getEditorPositionInfo } from "../utils"; +import { encodeDocumentPathForUrl } from "../../route/urlPath"; export class DocumentManager { private notificationManager: any = null; private editorManager: any = null; private eventSource: EventSource | null = null; + private sseEventQueue: Promise = Promise.resolve(); + private bindingTransitionQueue: Promise = Promise.resolve(); private autoSaveTimer: NodeJS.Timeout | null = null; private isPrimaryClient = false; + private isBindingTransitionInProgress = false; private lastAutoSaveContent = ""; private currentDocumentId = "default"; + private bindingToken: string | null = null; + private revision: string | null = null; + private bindingVersion = 0; public setNotificationManager(notificationManager: any): void { this.notificationManager = notificationManager; @@ -67,6 +74,13 @@ export class DocumentManager { console.log("[AUTO-SAVE] Skipping - no editor manager"); return; } + if ( + this.isBindingTransitionInProgress || + this.bindingToken === null + ) { + console.log("[AUTO-SAVE] Skipping - binding is not ready"); + return; + } const editor = this.editorManager.getEditor(); if (!editor) { @@ -85,22 +99,30 @@ export class DocumentManager { console.log(`[AUTO-SAVE] Content changed, auto-saving...`); - // Get current document path from server - const docInfo = await this.getCurrentDocumentInfo(); - // Send auto-save request + const bindingToken = this.bindingToken; + const bindingVersion = this.bindingVersion; 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, + expectedRevision: this.revision, }), }); if (response.ok) { - this.lastAutoSaveContent = currentContent; + const result = await response.json(); + if ( + bindingVersion === this.bindingVersion && + bindingToken === this.bindingToken && + result.bindingToken === bindingToken + ) { + this.adoptRevision(result); + this.lastAutoSaveContent = currentContent; + } console.log("[AUTO-SAVE] Successfully saved document"); } else { console.error( @@ -113,29 +135,6 @@ export class DocumentManager { } } - /** - * 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(); - } - } catch (error) { - console.warn("Failed to get current document info:", error); - } - - // Fallback - return { - currentDocument: this.currentDocumentId, - fullPath: null, - }; - } - private setupSSEConnection(): void { try { this.eventSource = new EventSource("/events"); @@ -148,7 +147,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( @@ -178,10 +184,31 @@ export class DocumentManager { console.log("[SSE] Received event:", data.type, data); switch (data.type) { + case "bindingBootstrap": + if ( + typeof data.bindingToken === "string" && + typeof data.documentId === "string" && + typeof data.revision === "string" + ) { + await this.transitionToBinding( + { + documentId: data.documentId, + bindingToken: data.bindingToken, + revision: data.revision, + }, + data.documentName, + data.boundRelativePath, + ); + } else if ( + data.bindingToken === null && + data.documentId === null + ) { + this.adoptBinding(data); + } + break; + case "documentChanged": console.log(`[SSE] Document changed to: ${data.newDocumentId}`); - this.currentDocumentId = data.newDocumentId; - // Reset sync notification state for new document if (this.notificationManager) { this.notificationManager.resetDocumentSyncState( @@ -189,9 +216,14 @@ export class DocumentManager { ); } - await this.handleDocumentChangeFromBackend( - data.newDocumentId, + await this.transitionToBinding( + { + documentId: data.newDocumentId, + bindingToken: data.bindingToken, + revision: data.revision, + }, data.newDocumentName, + data.boundRelativePath, ); break; @@ -201,10 +233,48 @@ export class DocumentManager { break; case "autoSave": + this.adoptRevision(data); console.log(`[SSE] Auto-save completed for: ${data.filePath}`); // Auto-save notification removed per user request break; + case "documentSnapshot": + if ( + data.bindingToken === this.bindingToken && + typeof data.markdown === "string" && + typeof data.baseMarkdown === "string" && + this.editorManager + ) { + await this.bindingTransitionQueue; + if ( + data.bindingToken !== this.bindingToken || + this.isBindingTransitionInProgress + ) { + break; + } + const editor = this.editorManager.getEditor(); + if (!editor) { + break; + } + const currentMarkdown = + await this.getMarkdownContent(editor); + if (currentMarkdown === data.markdown) { + this.lastAutoSaveContent = data.markdown; + this.adoptRevision(data); + break; + } + if (currentMarkdown !== data.baseMarkdown) { + console.warn( + "[SSE] Ignoring document snapshot because the editor changed after the agent read it", + ); + break; + } + await this.editorManager.setContent(data.markdown); + this.adoptRevision(data); + this.lastAutoSaveContent = data.markdown; + } + break; + case "autoSaveError": console.error(`[SSE] Auto-save error: ${data.error}`); // Auto-save error notification removed per user request @@ -295,37 +365,78 @@ export class DocumentManager { private async handleDocumentChangeFromBackend( documentId: string, documentName: string, + relativePath?: string, + expectedBindingToken?: string, ): Promise { - try { - console.log( - `[DOCUMENT] Backend switched to: ${documentName}, reconnecting frontend...`, - ); + console.log( + `[DOCUMENT] Backend switched to: ${documentName}, reconnecting frontend...`, + ); - // Get content from server with URL logging - const documentUrl = AI_CONFIG.ENDPOINTS.DOCUMENT; + const response = await fetch( + AI_CONFIG.ENDPOINTS.DOCUMENT, + expectedBindingToken + ? { headers: { "X-Binding-Token": expectedBindingToken } } + : undefined, + ); + if (!response.ok) { + throw new Error( + `Failed to load switched document: ${response.status}`, + ); + } + const content = await response.text(); - const response = await fetch(documentUrl); + console.log( + ` [DOCUMENT] Frontend switched to document: "${documentId}"`, + ); - const content = response.ok ? await response.text() : ""; - console.log( - ` [DOCUMENT] Frontend switched to document: "${documentId}"`, - ); + if (!this.editorManager) { + throw new Error("Editor is not ready for a binding transition"); + } + await this.editorManager.switchToDocument(documentId, content); + + document.title = `${documentName} - AI-Enhanced Markdown Editor`; + const documentPath = relativePath || documentName; + const newUrl = `/document/${encodeDocumentPathForUrl(documentPath)}`; + window.history.pushState( + { documentName: documentPath }, + document.title, + newUrl, + ); + } - // Switch editor collaboration to new document room - if (this.editorManager) { - await this.editorManager.switchToDocument(documentId, content); + private async transitionToBinding( + binding: { + documentId: string; + bindingToken: string; + revision: unknown; + }, + documentName: string, + relativePath?: string, + ): Promise { + const transition = this.bindingTransitionQueue.then(async () => { + if ( + binding.bindingToken === this.bindingToken && + binding.documentId === this.currentDocumentId + ) { + this.adoptRevision(binding); + return; } - // Update page title and URL - document.title = `${documentName} - AI-Enhanced Markdown Editor`; - const newUrl = `/document/${encodeURIComponent(documentName)}`; - window.history.pushState({ documentName }, document.title, newUrl); - } catch (error) { - console.error( - "[DOCUMENT] Failed to handle backend document change:", - error, - ); - } + this.isBindingTransitionInProgress = true; + try { + await this.handleDocumentChangeFromBackend( + binding.documentId, + documentName, + relativePath, + binding.bindingToken, + ); + this.adoptBinding(binding); + } finally { + this.isBindingTransitionInProgress = false; + } + }); + this.bindingTransitionQueue = transition.catch(() => undefined); + await transition; } public destroy(): void { @@ -351,6 +462,13 @@ export class DocumentManager { ); try { + if ( + this.isBindingTransitionInProgress || + !this.editorManager || + !this.editorManager.getEditor() + ) { + throw new Error("Editor is not ready to serialize Markdown"); + } let markdown = ""; let positionInfo = { position: 0 }; @@ -389,6 +507,7 @@ export class DocumentManager { requestId: requestId, markdown: markdown, positionInfo: positionInfo, + bindingToken: this.bindingToken, timestamp: Date.now(), }), }); @@ -417,6 +536,7 @@ export class DocumentManager { error instanceof Error ? error.message : "Unknown error", + bindingToken: this.bindingToken, timestamp: Date.now(), }), }); @@ -431,6 +551,12 @@ export class DocumentManager { public async saveDocument(editor?: Editor): Promise { try { + if ( + this.isBindingTransitionInProgress || + this.bindingToken === null + ) { + throw new Error("Cannot save while the binding is not ready"); + } // Get markdown content from editor or server const content = editor ? await this.getMarkdownContent(editor) @@ -438,16 +564,31 @@ export class DocumentManager { const saveUrl = AI_CONFIG.ENDPOINTS.DOCUMENT; + const bindingToken = this.bindingToken; + const bindingVersion = this.bindingVersion; const response = await fetch(saveUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ content }), + body: JSON.stringify({ + content, + bindingToken, + expectedRevision: this.revision, + }), }); if (!response.ok) { throw new Error(`Save failed: ${response.status}`); } + const result = await response.json(); + if ( + bindingVersion !== this.bindingVersion || + bindingToken !== this.bindingToken || + result.bindingToken !== bindingToken + ) { + throw new Error("Binding changed while saving"); + } + this.adoptRevision(result); console.log(` [DOCUMENT] Document saved successfully`); } catch (error) { console.error("[DOCUMENT] Failed to save document:", error); @@ -460,35 +601,7 @@ export class DocumentManager { 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 +612,7 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionHeader(response); return content; } else { return this.getDefaultContent(); @@ -516,6 +630,7 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionHeader(response); return content; } throw new Error( @@ -536,6 +651,7 @@ export class DocumentManager { if (response.ok) { const content = await response.text(); + this.adoptRevisionHeader(response); return content; } throw new Error( @@ -549,12 +665,25 @@ export class DocumentManager { public async setDocumentContent(content: string): Promise { try { + if ( + this.isBindingTransitionInProgress || + this.bindingToken === null + ) { + throw new Error( + "Cannot update content while the binding is not ready", + ); + } const saveUrl = AI_CONFIG.ENDPOINTS.DOCUMENT; - + const bindingToken = this.bindingToken; + const bindingVersion = this.bindingVersion; const response = await fetch(saveUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ content }), + body: JSON.stringify({ + content, + bindingToken, + expectedRevision: this.revision, + }), }); if (!response.ok) { @@ -563,6 +692,15 @@ export class DocumentManager { ); } + const result = await response.json(); + if ( + bindingVersion !== this.bindingVersion || + bindingToken !== this.bindingToken || + result.bindingToken !== bindingToken + ) { + throw new Error("Binding changed while updating content"); + } + this.adoptRevision(result); console.log(` [DOCUMENT] Document content updated successfully`); // Don't reload the whole page, just notify the editor will update via collaboration console.log( @@ -621,12 +759,13 @@ export class DocumentManager { public async switchToDocument(documentName: string): Promise { try { const switchUrl = "/api/switch-document"; + const startingBindingVersion = this.bindingVersion; // Call server to switch document const response = await fetch(switchUrl, { method: "POST", headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ documentName }), + body: JSON.stringify({ documentPath: documentName }), }); if (!response.ok) { @@ -636,30 +775,59 @@ export class DocumentManager { } const result = await response.json(); - console.log(`[DOCUMENT] Server switched to: ${documentName}`); - - // 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, - ); - console.log( - ` [DOCUMENT] Editor switched to document: "${documentId}"`, - ); + if ( + this.bindingVersion !== startingBindingVersion && + this.bindingToken !== result.bindingToken + ) { + return; } + console.log(`[DOCUMENT] Server switched to: ${documentName}`); - // Update page title and URL - document.title = `${documentName} - AI-Enhanced Markdown Editor`; - const newUrl = `/document/${encodeURIComponent(documentName)}`; - window.history.pushState({ documentName }, document.title, newUrl); + await this.transitionToBinding( + { + documentId: result.documentId, + bindingToken: result.bindingToken, + revision: result.revision, + }, + documentName, + result.boundRelativePath, + ); } catch (error) { console.error("[DOCUMENT] Failed to switch document:", error); throw error; } } + private adoptBinding(data: any): void { + this.bindingVersion++; + if (typeof data.documentId === "string") { + this.currentDocumentId = data.documentId; + } + this.bindingToken = + typeof data.bindingToken === "string" ? data.bindingToken : null; + this.revision = + typeof data.revision === "string" ? data.revision : null; + } + + private adoptRevision(data: any): void { + if ( + typeof data.bindingToken === "string" && + data.bindingToken !== this.bindingToken + ) { + return; + } + if (typeof data.revision === "string") { + this.revision = data.revision; + } + } + + private adoptRevisionHeader(response: Response): void { + const revision = response.headers.get("X-Content-Revision"); + if (revision) { + this.revision = revision; + } + } + private async hasUnsavedChanges(): Promise { try { if (!this.editorManager) return false; diff --git a/ts/packages/agents/markdown/src/view/site/index.ts b/ts/packages/agents/markdown/src/view/site/index.ts index 7c7af70e3d..848dd6a315 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"; // Global state for the application let editorManager: EditorManager | null = null; @@ -36,9 +37,7 @@ 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; + const documentName = parseDocumentPathFromUrl(window.location.pathname); // Initialize managers editorManager = new EditorManager(); @@ -48,17 +47,9 @@ async function initializeApplication(): Promise { // Initialize UI first await uiManager.initialize(); - // Initialize document manager (sets up SSE connection) - await documentManager.initialize(); - // 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); - } - // Get required DOM elements const editorElement = getRequiredElement("editor"); @@ -68,6 +59,13 @@ async function initializeApplication(): Promise { // Setup cross-manager dependencies setupManagerDependencies(editor); + // Bind the initialized editor before opening SSE. Otherwise an early + // serializer request can return empty content with a valid binding token. + if (documentName) { + await switchToDocument(documentName); + } + await documentManager.initialize(); + // Setup event handlers eventHandlers.setEditor(editor); eventHandlers.setupKeyboardShortcuts(); @@ -100,9 +98,7 @@ async function switchToDocument(documentName: string): Promise { 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; + const documentName = parseDocumentPathFromUrl(window.location.pathname); if (documentName && event.state?.documentName !== documentName) { await switchToDocument(documentName); diff --git a/ts/packages/agents/markdown/src/view/site/tsconfig.json b/ts/packages/agents/markdown/src/view/site/tsconfig.json index b9d1c6fce3..e086db9815 100644 --- a/ts/packages/agents/markdown/src/view/site/tsconfig.json +++ b/ts/packages/agents/markdown/src/view/site/tsconfig.json @@ -4,7 +4,7 @@ "noEmit": true, "composite": true, "incremental": true, - "rootDir": ".", + "rootDir": "..", "outDir": "../../../dist/view/site-typecheck", "moduleResolution": "bundler", "module": "ESNext", @@ -13,7 +13,7 @@ "noUnusedLocals": true, "noUnusedParameters": true }, - "include": ["./**/*"], + "include": ["./**/*", "../route/urlPath.ts"], "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..5022bf3914 100644 --- a/ts/packages/agents/markdown/src/view/site/types.ts +++ b/ts/packages/agents/markdown/src/view/site/types.ts @@ -82,6 +82,7 @@ export type NotificationType = "success" | "error" | "info"; export interface CollaborationInfo { websocketServerUrl: string; currentDocument: string; + currentDocumentId?: string; documents: number; totalClients: number; } diff --git a/ts/packages/agents/markdown/test/browserBinding.spec.ts b/ts/packages/agents/markdown/test/browserBinding.spec.ts new file mode 100644 index 0000000000..da10e50110 --- /dev/null +++ b/ts/packages/agents/markdown/test/browserBinding.spec.ts @@ -0,0 +1,221 @@ +// 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 binding", () => { + 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("echoes the active binding in serializer responses", async () => { + const requests: Array> = []; + globalThis.fetch = (async (_input, init) => { + requests.push(JSON.parse(init?.body as string)); + return Response.json({ success: true }); + }) as typeof fetch; + + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + manager.editorManager = { + getEditor: () => createEditor(() => "# Current\n"), + }; + + await manager.handleMarkdownRequest("request-1"); + + expect(requests).toEqual([ + expect.objectContaining({ + requestId: "request-1", + markdown: "# Current\n", + bindingToken: "binding-1", + }), + ]); + }); + + test("rejects serializer requests until the editor is ready", async () => { + const requests: Array> = []; + globalThis.fetch = (async (_input, init) => { + requests.push(JSON.parse(init?.body as string)); + return Response.json({ success: true }); + }) as typeof fetch; + + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + + await manager.handleMarkdownRequest("request-1"); + + expect(requests).toEqual([ + expect.objectContaining({ + requestId: "request-1", + error: "Editor is not ready to serialize Markdown", + bindingToken: "binding-1", + }), + ]); + }); + + test("sends and advances binding revisions on document saves", async () => { + const requests: Array> = []; + globalThis.fetch = (async (_input, init) => { + requests.push(JSON.parse(init?.body as string)); + return Response.json({ + bindingToken: "binding-1", + revision: "revision-2", + }); + }) as typeof fetch; + + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + manager.revision = "revision-1"; + + await manager.saveDocument(createEditor(() => "# Saved\n")); + + expect(requests).toEqual([ + { + content: "# Saved\n", + bindingToken: "binding-1", + expectedRevision: "revision-1", + }, + ]); + expect(manager.revision).toBe("revision-2"); + }); + + test("applies matching server snapshots to the editor", async () => { + const setContent = jest.fn(async () => {}); + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + manager.editorManager = { + getEditor: () => createEditor(() => "# Current\n"), + setContent, + }; + + await manager.handleSSEEvent({ + type: "documentSnapshot", + bindingToken: "binding-1", + baseMarkdown: "# Current\n", + markdown: "# Updated\n", + revision: "revision-2", + }); + + expect(setContent).toHaveBeenCalledWith("# Updated\n"); + expect(manager.revision).toBe("revision-2"); + }); + + test("does not overwrite edits made after an agent read", async () => { + const setContent = jest.fn(async () => {}); + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + manager.editorManager = { + getEditor: () => createEditor(() => "# User edit\n"), + setContent, + }; + + await manager.handleSSEEvent({ + type: "documentSnapshot", + bindingToken: "binding-1", + baseMarkdown: "# Earlier\n", + markdown: "# AI update\n", + revision: "revision-2", + }); + + expect(setContent).not.toHaveBeenCalled(); + expect(manager.revision).toBeNull(); + }); + + test("adopts a snapshot revision already synchronized by Yjs", async () => { + const setContent = jest.fn(async () => {}); + const manager = new DocumentManager(); + manager.bindingToken = "binding-1"; + manager.editorManager = { + getEditor: () => createEditor(() => "# AI update\n"), + setContent, + }; + + await manager.handleSSEEvent({ + type: "documentSnapshot", + bindingToken: "binding-1", + baseMarkdown: "# Earlier\n", + markdown: "# AI update\n", + revision: "revision-2", + }); + + expect(setContent).not.toHaveBeenCalled(); + expect(manager.revision).toBe("revision-2"); + }); + + test("uses the server room id instead of the display 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): { + action(callback: (ctx: { get: () => unknown }) => void): void; +} { + return { + action(callback): void { + let getCount = 0; + callback({ + get: () => + getCount++ === 0 + ? { + state: { + doc: { textContent: "Current" }, + selection: { + head: 0, + empty: true, + from: 0, + to: 0, + }, + }, + } + : () => getMarkdown(), + }); + }, + }; +} diff --git a/ts/packages/agents/markdown/test/markdownService.spec.ts b/ts/packages/agents/markdown/test/markdownService.spec.ts index 2eb11b8524..1f5615214b 100644 --- a/ts/packages/agents/markdown/test/markdownService.spec.ts +++ b/ts/packages/agents/markdown/test/markdownService.spec.ts @@ -132,6 +132,15 @@ describe("markdown service document reads", () => { expect((await load("escape/plan.md")).status).toBe(403); }); + test("hydrates the authoritative collaboration document after HTTP load", async () => { + const response = await load("plan.md"); + expect(response.status).toBe(200); + + const document = await fetch(`http://127.0.0.1:${port}/document`); + expect(document.status).toBe(200); + expect(await document.text()).toBe("original"); + }); + test("returns the durable snapshot after an asynchronous read", async () => { await bind("plan.md"); const paused = nextMessage(child, "readPaused"); @@ -142,7 +151,7 @@ describe("markdown service document reads", () => { expect(await response).toMatchObject({ requestId: "read-1", content: "original", - source: "file", + source: "file-fallback", }); }); @@ -184,7 +193,7 @@ describe("markdown service document reads", () => { expect(await response).toMatchObject({ requestId: "external-read", content: "externally updated 😀", - source: "file", + source: "file-fallback", }); }); diff --git a/ts/packages/agents/markdown/test/viewService.spec.ts b/ts/packages/agents/markdown/test/viewService.spec.ts index 04ac554ed7..4500f2bb2b 100644 --- a/ts/packages/agents/markdown/test/viewService.spec.ts +++ b/ts/packages/agents/markdown/test/viewService.spec.ts @@ -260,6 +260,30 @@ describe("markdown view service binding isolation", () => { boundRelativePath: "nested/second.md", }); + const read = sendAndWait( + viewProcess!, + { + type: "getDocumentContent", + requestId: "snapshot-read", + }, + (message) => message.requestId === "snapshot-read", + ); + const readMarkdownRequest = await waitForEvent( + events, + "requestMarkdown", + ); + await fetch(`http://127.0.0.1:${port}/api/markdown-response`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + requestId: readMarkdownRequest.requestId, + markdown: "second", + positionInfo: { position: 0 }, + bindingToken: switched.bindingToken, + }), + }); + const readResult = await read; + events.splice(events.indexOf(readMarkdownRequest), 1); const apply = sendAndWait( viewProcess!, { @@ -273,7 +297,8 @@ describe("markdown view service binding isolation", () => { }, ], expectedBindingToken: switched.bindingToken, - expectedRevision: switched.revision, + expectedRevision: readResult.revision, + expectedReadToken: readResult.readToken, }, (message) => message.requestId === "snapshot-apply", ); From 256e472ae4081646a95376dda6ffce7089ab6f4a Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 10 Sep 2026 18:26:37 -0700 Subject: [PATCH 4/7] refactor(markdown): reduce browser sync complexity Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/markdown/src/view/route/service.ts | 890 +++++++++--------- .../src/view/site/core/document-manager.ts | 274 +++--- 2 files changed, 616 insertions(+), 548 deletions(-) diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index b263483e06..c2f70d433a 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -1792,111 +1792,112 @@ process.on("message", async (message: any) => { ); if (message.type == "setFile") { - if (message.relativePath) { - const nextRoot = - typeof message.workspaceRoot === "string" && - resolveCanonicalRoot(message.workspaceRoot) !== undefined - ? resolveCanonicalRoot(message.workspaceRoot) - : undefined; - const relativePath = normalizeRelativeDocumentPath( - message.relativePath, - ); - if (nextRoot === undefined || relativePath === undefined) { - debug("Invalid document binding provided in message"); - return; - } - const resolvedFilePath = resolveWritableFileWithinRoot( - nextRoot, - relativePath, - ); - if (resolvedFilePath === undefined) { - debug("Invalid file path provided in message"); - return; - } + const handleSetFileMessage = (): void => { + if (message.relativePath) { + const nextRoot = + typeof message.workspaceRoot === "string" && + resolveCanonicalRoot(message.workspaceRoot) !== undefined + ? resolveCanonicalRoot(message.workspaceRoot) + : undefined; + const relativePath = normalizeRelativeDocumentPath( + message.relativePath, + ); + if (nextRoot === undefined || relativePath === undefined) { + debug("Invalid document binding provided in message"); + return; + } + const resolvedFilePath = resolveWritableFileWithinRoot( + nextRoot, + relativePath, + ); + if (resolvedFilePath === undefined) { + debug("Invalid file path provided in message"); + return; + } - if ( - currentRoot === nextRoot && - filePath === resolvedFilePath && - boundRelativePath === relativePath && - bindingToken !== null && - bindingSource === "managed" - ) { + if ( + currentRoot === nextRoot && + filePath === resolvedFilePath && + boundRelativePath === relativePath && + bindingToken !== null && + bindingSource === "managed" + ) { + bindingGeneration++; + notifyBindingToParent(); + return; + } + + const oldFilePath = filePath; + const previousDocumentId = getCurrentDocumentId(); + currentRoot = nextRoot; + filePath = resolvedFilePath; + boundRelativePath = relativePath; + bindingToken = randomUUID(); bindingGeneration++; + bindingSource = "managed"; notifyBindingToParent(); - return; - } - const oldFilePath = filePath; - const previousDocumentId = getCurrentDocumentId(); - currentRoot = nextRoot; - filePath = resolvedFilePath; - boundRelativePath = relativePath; - bindingToken = randomUUID(); - bindingGeneration++; - bindingSource = "managed"; - notifyBindingToParent(); + const documentId = getCurrentDocumentId(); + if (previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } - const documentId = getCurrentDocumentId(); - if (previousDocumentId !== documentId) { - evictRoomIfIdle(previousDocumentId); - } + // Get or create the authoritative Y.js document + const ydoc = getAuthoritativeDocument(documentId); - // Get or create the authoritative Y.js document - const ydoc = getAuthoritativeDocument(documentId); + // Load existing content into the authoritative document + if (fs.existsSync(resolvedFilePath)) { + const content = fs.readFileSync(resolvedFilePath, "utf-8"); - // Load existing content into the authoritative document - if (fs.existsSync(resolvedFilePath)) { - const content = fs.readFileSync(resolvedFilePath, "utf-8"); + // Set content directly in the authoritative Y.js document + const ytext = ydoc.getText("content"); + ytext.delete(0, ytext.length); // Clear existing content + ytext.insert(0, content); // Insert file content - // Set content directly in the authoritative Y.js document - const ytext = ydoc.getText("content"); - ytext.delete(0, ytext.length); // Clear existing content - ytext.insert(0, content); // Insert file content + debug( + `File loaded into authoritative document: ${documentId}, ${content.length} chars from ${relativePath}`, + ); + } else { + debug( + `File doesn't exist, authoritative document ${documentId} remains empty`, + ); + } - debug( - `File loaded into authoritative document: ${documentId}, ${content.length} chars from ${relativePath}`, - ); + // Notify frontend clients if the document has changed + if (oldFilePath !== filePath) { + broadcastEvent({ + type: "documentChanged", + newDocumentId: documentId, + newDocumentName: path.basename(relativePath, ".md"), + bindingToken, + boundRelativePath, + revision: computeContentRevision( + ydoc.getText("content").toString(), + ), + timestamp: Date.now(), + }); + } } else { - debug( - `File doesn't exist, authoritative document ${documentId} remains empty`, - ); - } - - // Notify frontend clients if the document has changed - if (oldFilePath !== filePath) { - broadcastEvent({ - type: "documentChanged", - newDocumentId: documentId, - newDocumentName: path.basename(relativePath, ".md"), - bindingToken, - boundRelativePath, - revision: computeContentRevision( - ydoc.getText("content").toString(), - ), - timestamp: Date.now(), - }); - } - } else { - const previousDocumentId = getCurrentDocumentId(); - // No file mode - initialize with default content using authoritative document - filePath = null; - boundRelativePath = null; - bindingToken = null; - bindingGeneration++; - bindingSource = null; - notifyBindingToParent(); - debug("Running in memory-only mode (no file)"); + const previousDocumentId = getCurrentDocumentId(); + // No file mode - initialize with default content using authoritative document + filePath = null; + boundRelativePath = null; + bindingToken = null; + bindingGeneration++; + bindingSource = null; + notifyBindingToParent(); + debug("Running in memory-only mode (no file)"); - const documentId = "default"; - if (previousDocumentId !== documentId) { - evictRoomIfIdle(previousDocumentId); - } + const documentId = "default"; + if (previousDocumentId !== documentId) { + evictRoomIfIdle(previousDocumentId); + } - // Get or create authoritative Y.js document for memory-only mode - const ydoc = getAuthoritativeDocument(documentId); + // Get or create authoritative Y.js document for memory-only mode + const ydoc = getAuthoritativeDocument(documentId); - // Set default content in the authoritative Y.js document - const defaultContent = `# Welcome to AI-Enhanced Markdown Editor + // Set default content in the authoritative Y.js document + const defaultContent = `# Welcome to AI-Enhanced Markdown Editor Start editing your markdown document with AI assistance! @@ -1935,20 +1936,22 @@ graph TD Start typing to see the editor in action! `; - const ytext = ydoc.getText("content"); + const ytext = ydoc.getText("content"); - // Only set content if document is empty to avoid overwriting existing content - if (ytext.length === 0) { - ytext.insert(0, defaultContent); - debug( - `Initialized authoritative Y.js document ${documentId} with default content: ${defaultContent.length} chars`, - ); - } else { - debug( - `Authoritative Y.js document ${documentId} already has content: ${ytext.length} chars`, - ); + // Only set content if document is empty to avoid overwriting existing content + if (ytext.length === 0) { + ytext.insert(0, defaultContent); + debug( + `Initialized authoritative Y.js document ${documentId} with default content: ${defaultContent.length} chars`, + ); + } else { + debug( + `Authoritative Y.js document ${documentId} already has content: ${ytext.length} chars`, + ); + } } - } + }; + handleSetFileMessage(); } else if (message.type == "applyOperations") { // Send operations to frontend debug( @@ -1964,284 +1967,335 @@ Start typing to see the editor in action! ); }); } else if (message.type === "applyLLMOperations") { - const requestId = - typeof message.requestId === "string" ? message.requestId : ""; - const snapshot = captureBindingSnapshot(); - let acquiredApplyKey: string | undefined; - try { - if ( - !Array.isArray(message.operations) || - !snapshot.filePath || - !snapshot.boundRelativePath - ) { - throw new Error("Invalid document update request"); - } - const identityError = bindingError(message, snapshot); - if (identityError) { - process.send?.({ - type: "operationsApplied", - requestId, - success: false, - identityMismatch: true, - error: identityError, - bindingToken: snapshot.bindingToken, - }); - return; - } - if (typeof message.expectedRevision !== "string") { - throw new Error("Invalid document update request"); - } - const activeApplyKey = snapshot.bindingToken ?? "memory"; - if (activeApplyBindings.has(activeApplyKey)) { - throw new Error( - "Another document update is already in progress for this binding", - ); - } - activeApplyBindings.add(activeApplyKey); - acquiredApplyKey = activeApplyKey; - - const binding: DocumentBinding = { - token: snapshot.bindingToken ?? undefined, - root: snapshot.currentRoot, - relativePath: snapshot.boundRelativePath, - filePath: snapshot.filePath, - }; - const expected = { - bindingToken: - typeof message.expectedBindingToken === "string" - ? message.expectedBindingToken - : undefined, - root: - typeof message.expectedRoot === "string" - ? message.expectedRoot - : undefined, - relativePath: - typeof message.expectedRelativePath === "string" - ? message.expectedRelativePath - : undefined, - revision: message.expectedRevision, - updatedRevision: - typeof message.expectedUpdatedRevision === "string" - ? message.expectedUpdatedRevision - : undefined, - }; - let persisted; - let browserBaseMarkdown: string | undefined; - if ( - clients.length === 0 || - typeof message.expectedReadToken !== "string" - ) { - persisted = persistDocumentOperations( - binding, - message.operations as DocumentOperation[], - expected, - ); - } else { - const persistedRevisionBeforeRead = ( - await readBoundDocument(binding) - ).revision; - const readToken = - typeof message.expectedReadToken === "string" - ? message.expectedReadToken - : ""; - const readSnapshot = browserReadDiskRevisions.get(readToken); - browserReadDiskRevisions.delete(readToken); + const handleApplyLLMOperationsMessage = async (): Promise => { + const requestId = + typeof message.requestId === "string" ? message.requestId : ""; + const snapshot = captureBindingSnapshot(); + let acquiredApplyKey: string | undefined; + try { + const prepareApply = (): + | { + binding: DocumentBinding; + expected: { + bindingToken: string | undefined; + root: string | undefined; + relativePath: string | undefined; + revision: string; + updatedRevision: string | undefined; + }; + } + | undefined => { + if ( + !Array.isArray(message.operations) || + !snapshot.filePath || + !snapshot.boundRelativePath + ) { + throw new Error("Invalid document update request"); + } + const identityError = bindingError(message, snapshot); + if (identityError) { + process.send?.({ + type: "operationsApplied", + requestId, + success: false, + identityMismatch: true, + error: identityError, + bindingToken: snapshot.bindingToken, + }); + return undefined; + } + if (typeof message.expectedRevision !== "string") { + throw new Error("Invalid document update request"); + } + const activeApplyKey = snapshot.bindingToken ?? "memory"; + if (activeApplyBindings.has(activeApplyKey)) { + throw new Error( + "Another document update is already in progress for this binding", + ); + } + activeApplyBindings.add(activeApplyKey); + acquiredApplyKey = activeApplyKey; + return { + binding: { + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }, + expected: { + bindingToken: + typeof message.expectedBindingToken === "string" + ? message.expectedBindingToken + : undefined, + root: + typeof message.expectedRoot === "string" + ? message.expectedRoot + : undefined, + relativePath: + typeof message.expectedRelativePath === "string" + ? message.expectedRelativePath + : undefined, + revision: message.expectedRevision, + updatedRevision: + typeof message.expectedUpdatedRevision === + "string" + ? message.expectedUpdatedRevision + : undefined, + }, + }; + }; + const prepared = prepareApply(); + if (!prepared) { + return; + } + const { binding, expected } = prepared; + let persisted; + let browserBaseMarkdown: string | undefined; if ( - readSnapshot === undefined || - readSnapshot.bindingToken !== snapshot.bindingToken || - readSnapshot.contentRevision !== expected.revision || - readSnapshot.diskRevision !== persistedRevisionBeforeRead + clients.length === 0 || + typeof message.expectedReadToken !== "string" ) { - throw new Error( - "Document changed since the browser snapshot was read (revision mismatch)", + persisted = persistDocumentOperations( + binding, + message.operations as DocumentOperation[], + expected, ); - } - const response = await requestMarkdownFromClient(0, snapshot); - browserBaseMarkdown = response.markdown; - const authoritativeDocument = getAuthoritativeDocument( - getCurrentDocumentId(snapshot), - ); - const browserStateVector = Y.encodeStateVector( - authoritativeDocument, - ); - if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { - throw new Error("Document binding changed while reading"); - } - const baseRevision = computeContentRevision(response.markdown); - const alreadyApplied = - expected.updatedRevision !== undefined && - expected.updatedRevision === baseRevision; - if (!alreadyApplied && expected.revision !== baseRevision) { - throw new Error( - "Document changed between read and apply (revision mismatch)", + } else { + const persistedRevisionBeforeRead = ( + await readBoundDocument(binding) + ).revision; + const readToken = + typeof message.expectedReadToken === "string" + ? message.expectedReadToken + : ""; + const readSnapshot = + browserReadDiskRevisions.get(readToken); + browserReadDiskRevisions.delete(readToken); + if ( + readSnapshot === undefined || + readSnapshot.bindingToken !== snapshot.bindingToken || + readSnapshot.contentRevision !== expected.revision || + readSnapshot.diskRevision !== + persistedRevisionBeforeRead + ) { + throw new Error( + "Document changed since the browser snapshot was read (revision mismatch)", + ); + } + const response = await requestMarkdownFromClient( + 0, + snapshot, ); - } - const content = alreadyApplied - ? response.markdown - : applyDocumentOperations( - response.markdown, - message.operations as DocumentOperation[], - ); - const revision = computeContentRevision(content); - if ( - expected.updatedRevision !== undefined && - expected.updatedRevision !== revision - ) { - throw new Error( - "Updated document revision does not match operations", + browserBaseMarkdown = response.markdown; + const authoritativeDocument = getAuthoritativeDocument( + getCurrentDocumentId(snapshot), ); - } - const writableFilePath = resolveWritableFileWithinRoot( - snapshot.currentRoot, - snapshot.boundRelativePath, - ); - if ( - writableFilePath === undefined || - path.relative(writableFilePath, snapshot.filePath) !== "" - ) { - throw new Error("Document binding path changed"); - } - if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { - throw new Error("Document binding changed before write"); - } - const persistedRevisionAfterRead = ( - await readBoundDocument(binding) - ).revision; - if ( - bindingsDiffer(captureBindingSnapshot(), snapshot) || - persistedRevisionAfterRead !== persistedRevisionBeforeRead - ) { - throw new Error( - "Document changed during browser read (revision mismatch)", + const browserStateVector = Y.encodeStateVector( + authoritativeDocument, + ); + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error( + "Document binding changed while reading", + ); + } + const baseRevision = computeContentRevision( + response.markdown, + ); + const alreadyApplied = + expected.updatedRevision !== undefined && + expected.updatedRevision === baseRevision; + if (!alreadyApplied && expected.revision !== baseRevision) { + throw new Error( + "Document changed between read and apply (revision mismatch)", + ); + } + const content = alreadyApplied + ? response.markdown + : applyDocumentOperations( + response.markdown, + message.operations as DocumentOperation[], + ); + const revision = computeContentRevision(content); + if ( + expected.updatedRevision !== undefined && + expected.updatedRevision !== revision + ) { + throw new Error( + "Updated document revision does not match operations", + ); + } + const writableFilePath = resolveWritableFileWithinRoot( + binding.root, + binding.relativePath, ); + if ( + writableFilePath === undefined || + path.relative(writableFilePath, binding.filePath) !== "" + ) { + throw new Error("Document binding path changed"); + } + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error( + "Document binding changed before write", + ); + } + const persistedRevisionAfterRead = ( + await readBoundDocument(binding) + ).revision; + if ( + bindingsDiffer(captureBindingSnapshot(), snapshot) || + persistedRevisionAfterRead !== + persistedRevisionBeforeRead + ) { + throw new Error( + "Document changed during browser read (revision mismatch)", + ); + } + const verifiedFilePath = resolveWritableFileWithinRoot( + binding.root, + binding.relativePath, + ); + if ( + verifiedFilePath === undefined || + verifiedFilePath !== writableFilePath || + path.relative(verifiedFilePath, binding.filePath) !== "" + ) { + throw new Error("Document binding path changed"); + } + if ( + !Buffer.from( + Y.encodeStateVector(authoritativeDocument), + ).equals(Buffer.from(browserStateVector)) + ) { + throw new Error( + "Document changed after browser serialization (revision mismatch)", + ); + } + fs.writeFileSync(verifiedFilePath, content, "utf-8"); + persisted = { + content, + revision, + alreadyApplied, + filePath: verifiedFilePath, + }; } - const verifiedFilePath = resolveWritableFileWithinRoot( - snapshot.currentRoot, - snapshot.boundRelativePath, + + const documentId = getCurrentDocumentId(snapshot); + collaborationManager.setDocumentContent( + documentId, + persisted.content, ); if ( - verifiedFilePath === undefined || - verifiedFilePath !== writableFilePath || - path.relative(verifiedFilePath, snapshot.filePath) !== "" + snapshot.bindingToken && + browserBaseMarkdown !== undefined ) { - throw new Error("Document binding path changed"); - } - if ( - !Buffer.from( - Y.encodeStateVector(authoritativeDocument), - ).equals(Buffer.from(browserStateVector)) - ) { - throw new Error( - "Document changed after browser serialization (revision mismatch)", - ); + broadcastEvent({ + type: "documentSnapshot", + bindingToken: snapshot.bindingToken, + baseMarkdown: browserBaseMarkdown, + markdown: persisted.content, + revision: persisted.revision, + timestamp: Date.now(), + }); } - fs.writeFileSync(verifiedFilePath, content, "utf-8"); - persisted = { - content, - revision, - alreadyApplied, - filePath: verifiedFilePath, - }; - } - - const documentId = getCurrentDocumentId(snapshot); - collaborationManager.setDocumentContent( - documentId, - persisted.content, - ); - if (snapshot.bindingToken && browserBaseMarkdown !== undefined) { - broadcastEvent({ - type: "documentSnapshot", + process.send?.({ + type: "operationsApplied", + requestId, + success: true, + operationCount: message.operations.length, bindingToken: snapshot.bindingToken, - baseMarkdown: browserBaseMarkdown, - markdown: persisted.content, revision: persisted.revision, - timestamp: Date.now(), }); - } - process.send?.({ - type: "operationsApplied", - requestId, - success: true, - operationCount: message.operations.length, - bindingToken: snapshot.bindingToken, - revision: persisted.revision, - }); - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : "Unknown error"; - process.send?.({ - type: "operationsApplied", - requestId, - success: false, - identityMismatch: - error instanceof ClientBindingMismatchError || - /binding|workspace root/.test(errorMessage), - revisionMismatch: /revision mismatch/.test(errorMessage), - error: errorMessage, - bindingToken: snapshot.bindingToken, - }); - } finally { - if (acquiredApplyKey !== undefined) { - activeApplyBindings.delete(acquiredApplyKey); - } - } - } else if (message.type === "getDocumentContent") { - const requestId = - typeof message.requestId === "string" ? message.requestId : ""; - const snapshot = captureBindingSnapshot(); - try { - const identityError = bindingError(message, snapshot); - if (identityError) { + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : "Unknown error"; process.send?.({ - type: "documentContent", + type: "operationsApplied", requestId, - content: "", - source: "error", - error: identityError, - identityMismatch: true, + success: false, + identityMismatch: + error instanceof ClientBindingMismatchError || + /binding|workspace root/.test(errorMessage), + revisionMismatch: /revision mismatch/.test(errorMessage), + error: errorMessage, bindingToken: snapshot.bindingToken, - boundFilePath: snapshot.filePath, - boundRoot: snapshot.filePath ? snapshot.currentRoot : null, - boundRelativePath: snapshot.boundRelativePath, - revision: null, - timestamp: Date.now(), }); - return; - } - if (!snapshot.filePath || !snapshot.boundRelativePath) { - throw new Error("No markdown document is bound"); + } finally { + if (acquiredApplyKey !== undefined) { + activeApplyBindings.delete(acquiredApplyKey); + } } - let content: string; - let source: "client-serializer" | "file-fallback"; - let browserDiskRevision: string | undefined; - if (clients.length > 0) { - try { - const diskBefore = await readBoundDocument({ - token: snapshot.bindingToken ?? undefined, - root: snapshot.currentRoot, - relativePath: snapshot.boundRelativePath, - filePath: snapshot.filePath, - }); - content = (await requestMarkdownFromClient(0, snapshot)) - .markdown; - const diskAfter = await readBoundDocument({ - token: snapshot.bindingToken ?? undefined, - root: snapshot.currentRoot, - relativePath: snapshot.boundRelativePath, - filePath: snapshot.filePath, + }; + await handleApplyLLMOperationsMessage(); + } else if (message.type === "getDocumentContent") { + const handleGetDocumentContentMessage = async (): Promise => { + const requestId = + typeof message.requestId === "string" ? message.requestId : ""; + const snapshot = captureBindingSnapshot(); + try { + const identityError = bindingError(message, snapshot); + if (identityError) { + process.send?.({ + type: "documentContent", + requestId, + content: "", + source: "error", + error: identityError, + identityMismatch: true, + bindingToken: snapshot.bindingToken, + boundFilePath: snapshot.filePath, + boundRoot: snapshot.filePath + ? snapshot.currentRoot + : null, + boundRelativePath: snapshot.boundRelativePath, + revision: null, + timestamp: Date.now(), }); - if (diskBefore.revision !== diskAfter.revision) { - throw new Error( - "Document changed during browser serialization (revision mismatch)", - ); - } - browserDiskRevision = diskBefore.revision; - source = "client-serializer"; - } catch (error) { - if (error instanceof ClientBindingMismatchError) { - throw error; + return; + } + if (!snapshot.filePath || !snapshot.boundRelativePath) { + throw new Error("No markdown document is bound"); + } + let content: string; + let source: "client-serializer" | "file-fallback"; + let browserDiskRevision: string | undefined; + if (clients.length > 0) { + try { + const diskBefore = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + content = (await requestMarkdownFromClient(0, snapshot)) + .markdown; + const diskAfter = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + if (diskBefore.revision !== diskAfter.revision) { + throw new Error( + "Document changed during browser serialization (revision mismatch)", + ); + } + browserDiskRevision = diskBefore.revision; + source = "client-serializer"; + } catch (error) { + if (error instanceof ClientBindingMismatchError) { + throw error; + } + const document = await readBoundDocument({ + token: snapshot.bindingToken ?? undefined, + root: snapshot.currentRoot, + relativePath: snapshot.boundRelativePath, + filePath: snapshot.filePath, + }); + content = document.content; + source = "file-fallback"; } + } else { const document = await readBoundDocument({ token: snapshot.bindingToken ?? undefined, root: snapshot.currentRoot, @@ -2251,71 +2305,63 @@ Start typing to see the editor in action! content = document.content; source = "file-fallback"; } - } else { - const document = await readBoundDocument({ - token: snapshot.bindingToken ?? undefined, - root: snapshot.currentRoot, - relativePath: snapshot.boundRelativePath, - filePath: snapshot.filePath, - }); - content = document.content; - source = "file-fallback"; - } - if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { - throw new Error("Document binding changed while reading"); - } - const revision = computeContentRevision(content); - let readToken: string | undefined; - if (source === "client-serializer") { - if (browserDiskRevision === undefined) { - throw new Error( - "Browser document read is missing its disk revision", - ); + if (bindingsDiffer(captureBindingSnapshot(), snapshot)) { + throw new Error("Document binding changed while reading"); } - readToken = randomUUID(); - browserReadDiskRevisions.set(readToken, { + const revision = computeContentRevision(content); + let readToken: string | undefined; + if (source === "client-serializer") { + if (browserDiskRevision === undefined) { + throw new Error( + "Browser document read is missing its disk revision", + ); + } + readToken = randomUUID(); + browserReadDiskRevisions.set(readToken, { + bindingToken: snapshot.bindingToken, + contentRevision: revision, + diskRevision: browserDiskRevision, + }); + const tokenToExpire = readToken; + setTimeout(() => { + browserReadDiskRevisions.delete(tokenToExpire); + }, 300_000).unref(); + } + process.send?.({ + type: "documentContent", + requestId, + content, + source, bindingToken: snapshot.bindingToken, - contentRevision: revision, - diskRevision: browserDiskRevision, + boundFilePath: snapshot.filePath, + boundRoot: snapshot.currentRoot, + boundRelativePath: snapshot.boundRelativePath, + revision, + readToken, + timestamp: Date.now(), + }); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : "Unknown error"; + process.send?.({ + type: "documentContent", + requestId, + content: "", + source: "error", + error: errorMessage, + identityMismatch: + error instanceof ClientBindingMismatchError || + /binding|workspace root/.test(errorMessage), + bindingToken: snapshot.bindingToken, + boundFilePath: snapshot.filePath, + boundRoot: snapshot.filePath ? snapshot.currentRoot : null, + boundRelativePath: snapshot.boundRelativePath, + revision: null, + timestamp: Date.now(), }); - const tokenToExpire = readToken; - setTimeout(() => { - browserReadDiskRevisions.delete(tokenToExpire); - }, 300_000).unref(); } - process.send?.({ - type: "documentContent", - requestId, - content, - source, - bindingToken: snapshot.bindingToken, - boundFilePath: snapshot.filePath, - boundRoot: snapshot.currentRoot, - boundRelativePath: snapshot.boundRelativePath, - revision, - readToken, - timestamp: Date.now(), - }); - } catch (error) { - const errorMessage = - error instanceof Error ? error.message : "Unknown error"; - process.send?.({ - type: "documentContent", - requestId, - content: "", - source: "error", - error: errorMessage, - identityMismatch: - error instanceof ClientBindingMismatchError || - /binding|workspace root/.test(errorMessage), - bindingToken: snapshot.bindingToken, - boundFilePath: snapshot.filePath, - boundRoot: snapshot.filePath ? snapshot.currentRoot : null, - boundRelativePath: snapshot.boundRelativePath, - revision: null, - timestamp: Date.now(), - }); - } + }; + await handleGetDocumentContentMessage(); } else if (message.type === "uiCommandResult") { // Handle UI command results from agent debug( 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 4a7abd8a1c..b03f076f1d 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 @@ -7,6 +7,31 @@ import { AI_CONFIG, DEFAULT_MARKDOWN_CONTENT, EDITOR_CONFIG } from "../config"; import { getMarkdownFromEditor, getEditorPositionInfo } from "../utils"; import { encodeDocumentPathForUrl } from "../../route/urlPath"; +interface SSEEventData { + type: string; + bindingToken: string | null; + documentId: string | null; + revision: string; + documentName: string; + boundRelativePath: string; + newDocumentId: string; + newDocumentName: string; + markdown: string; + baseMarkdown: string; + clientRole: string; + operations: Array>; + filePath: string; + error: unknown; + operationCount: number; + requestId: string; +} + +interface BindingStateData { + documentId?: unknown; + bindingToken?: unknown; + revision?: unknown; +} + export class DocumentManager { private notificationManager: any = null; private editorManager: any = null; @@ -180,51 +205,16 @@ export class DocumentManager { } } - private async handleSSEEvent(data: any): Promise { + private async handleSSEEvent(data: SSEEventData): Promise { console.log("[SSE] Received event:", data.type, data); switch (data.type) { case "bindingBootstrap": - if ( - typeof data.bindingToken === "string" && - typeof data.documentId === "string" && - typeof data.revision === "string" - ) { - await this.transitionToBinding( - { - documentId: data.documentId, - bindingToken: data.bindingToken, - revision: data.revision, - }, - data.documentName, - data.boundRelativePath, - ); - } else if ( - data.bindingToken === null && - data.documentId === null - ) { - this.adoptBinding(data); - } + await this.handleBindingBootstrap(data); break; case "documentChanged": - console.log(`[SSE] Document changed to: ${data.newDocumentId}`); - // Reset sync notification state for new document - if (this.notificationManager) { - this.notificationManager.resetDocumentSyncState( - data.newDocumentId, - ); - } - - await this.transitionToBinding( - { - documentId: data.newDocumentId, - bindingToken: data.bindingToken, - revision: data.revision, - }, - data.newDocumentName, - data.boundRelativePath, - ); + await this.handleDocumentChanged(data); break; case "documentSynced": @@ -239,40 +229,7 @@ export class DocumentManager { break; case "documentSnapshot": - if ( - data.bindingToken === this.bindingToken && - typeof data.markdown === "string" && - typeof data.baseMarkdown === "string" && - this.editorManager - ) { - await this.bindingTransitionQueue; - if ( - data.bindingToken !== this.bindingToken || - this.isBindingTransitionInProgress - ) { - break; - } - const editor = this.editorManager.getEditor(); - if (!editor) { - break; - } - const currentMarkdown = - await this.getMarkdownContent(editor); - if (currentMarkdown === data.markdown) { - this.lastAutoSaveContent = data.markdown; - this.adoptRevision(data); - break; - } - if (currentMarkdown !== data.baseMarkdown) { - console.warn( - "[SSE] Ignoring document snapshot because the editor changed after the agent read it", - ); - break; - } - await this.editorManager.setContent(data.markdown); - this.adoptRevision(data); - this.lastAutoSaveContent = data.markdown; - } + await this.handleDocumentSnapshot(data); break; case "autoSaveError": @@ -281,58 +238,7 @@ export class DocumentManager { break; case "llmOperations": - // PRODUCTION: Handle LLM operations sent to PRIMARY client only via SSE - // Apply operations through editor API for proper markdown parsing - if ( - data.clientRole === "primary" && - data.operations && - Array.isArray(data.operations) && - this.editorManager - ) { - try { - // Mark this client as primary for auto-save - this.isPrimaryClient = true; - console.log( - "[SSE] Marked as PRIMARY CLIENT for auto-save", - ); - - // 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, - ); - 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, - ); - } + await this.handleLLMOperations(data); break; case "operationsBeingApplied": @@ -362,6 +268,122 @@ export class DocumentManager { } } + private async handleBindingBootstrap(data: SSEEventData): Promise { + if ( + typeof data.bindingToken === "string" && + typeof data.documentId === "string" && + typeof data.revision === "string" + ) { + await this.transitionToBinding( + { + documentId: data.documentId, + bindingToken: data.bindingToken, + revision: data.revision, + }, + data.documentName, + data.boundRelativePath, + ); + } else if (data.bindingToken === null && data.documentId === null) { + this.adoptBinding(data); + } + } + + private async handleDocumentChanged(data: SSEEventData): Promise { + console.log(`[SSE] Document changed to: ${data.newDocumentId}`); + if (typeof data.bindingToken !== "string") { + return; + } + if (this.notificationManager) { + this.notificationManager.resetDocumentSyncState(data.newDocumentId); + } + await this.transitionToBinding( + { + documentId: data.newDocumentId, + bindingToken: data.bindingToken, + revision: data.revision, + }, + data.newDocumentName, + data.boundRelativePath, + ); + } + + private async handleDocumentSnapshot(data: SSEEventData): Promise { + if ( + data.bindingToken !== this.bindingToken || + typeof data.markdown !== "string" || + typeof data.baseMarkdown !== "string" || + !this.editorManager + ) { + return; + } + await this.bindingTransitionQueue; + if ( + data.bindingToken !== this.bindingToken || + this.isBindingTransitionInProgress + ) { + return; + } + const editor = this.editorManager.getEditor(); + if (!editor) { + return; + } + const currentMarkdown = await this.getMarkdownContent(editor); + if (currentMarkdown === data.markdown) { + this.lastAutoSaveContent = data.markdown; + this.adoptRevision(data); + return; + } + if (currentMarkdown !== data.baseMarkdown) { + console.warn( + "[SSE] Ignoring document snapshot because the editor changed after the agent read it", + ); + return; + } + await this.editorManager.setContent(data.markdown); + this.adoptRevision(data); + this.lastAutoSaveContent = data.markdown; + } + + private async handleLLMOperations(data: SSEEventData): Promise { + if (data.clientRole !== "primary") { + this.isPrimaryClient = false; + console.log(`[SSE] Marked as SECONDARY CLIENT`); + return; + } + if ( + !data.operations || + !Array.isArray(data.operations) || + !this.editorManager + ) { + console.warn(`[SSE] Invalid LLM operations received:`, data); + return; + } + try { + this.isPrimaryClient = true; + console.log("[SSE] Marked as PRIMARY CLIENT for auto-save"); + const editor = this.editorManager.getEditor(); + if (!editor) { + console.warn(`[SSE] No editor available to apply operations`); + return; + } + await this.applyOperationsThroughEditor(editor, data.operations); + console.log( + ` [SSE] Applied ${data.operations.length} operations via editor API`, + ); + } catch (error) { + console.error( + `[ERROR] [SSE] Failed to apply LLM operations:`, + error, + ); + if (this.notificationManager) { + this.notificationManager.showNotification( + `❌ Failed to apply AI changes`, + "error", + ); + } + } + } + private async handleDocumentChangeFromBackend( documentId: string, documentName: string, @@ -798,7 +820,7 @@ export class DocumentManager { } } - private adoptBinding(data: any): void { + private adoptBinding(data: BindingStateData): void { this.bindingVersion++; if (typeof data.documentId === "string") { this.currentDocumentId = data.documentId; @@ -809,7 +831,7 @@ export class DocumentManager { typeof data.revision === "string" ? data.revision : null; } - private adoptRevision(data: any): void { + private adoptRevision(data: BindingStateData): void { if ( typeof data.bindingToken === "string" && data.bindingToken !== this.bindingToken From d134c85752d2bc4115b9fd8c3bc0af2ced52d6ad Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 10 Sep 2026 20:59:22 -0700 Subject: [PATCH 5/7] fix(markdown): address CI portability and path checks Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/markdown/src/agent/documentPathPolicy.ts | 13 +++++++++++-- .../markdown/src/agent/documentUpdatePersistence.ts | 11 +++++++---- .../agents/markdown/src/view/route/service.ts | 6 +----- .../agents/markdown/test/boundDocumentRead.spec.ts | 8 ++------ 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts index 3a17a87f17..a336ac9208 100644 --- a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts +++ b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts @@ -84,8 +84,13 @@ export function resolveExistingFileWithinRoot( root: string, requestedPath: string, ): string | undefined { + const relativePath = normalizeRelativeDocumentPath(requestedPath); + if (relativePath === undefined) { + return undefined; + } const rootPaths = resolveRootPaths(root); - const candidate = path.resolve(rootPaths.resolvedRoot, requestedPath); + // Both inputs are validated and the canonical file is checked below. + const candidate = path.resolve(rootPaths.resolvedRoot, relativePath); // lgtm[js/path-injection] if (!isPathWithinRoot(rootPaths.resolvedRoot, candidate)) { return undefined; } @@ -147,8 +152,12 @@ export function resolveWritableFileWithinRoot( root: string, requestedPath: string, ): string | undefined { + const relativePath = normalizeRelativeDocumentPath(requestedPath); + if (relativePath === undefined) { + return undefined; + } const rootPaths = resolveRootPaths(root); - const candidate = path.resolve(rootPaths.resolvedRoot, requestedPath); + const candidate = path.resolve(rootPaths.resolvedRoot, relativePath); if (!isPathWithinRoot(rootPaths.resolvedRoot, candidate)) { return undefined; } diff --git a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts index 818f72cf09..816ec146b5 100644 --- a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts +++ b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts @@ -131,7 +131,9 @@ export function persistDocumentOperations( ) { validateIdentity(binding, expected); let filePath = resolveBoundFile(binding); - const currentContent = fs.readFileSync(filePath, "utf-8"); + // resolveBoundFile verifies the canonical path and binding immediately + // before every use. + const currentContent = fs.readFileSync(filePath, "utf-8"); // lgtm[js/path-injection] const currentRevision = computeContentRevision(currentContent); if (expected.updatedRevision === currentRevision) { return { @@ -159,13 +161,14 @@ export function persistDocumentOperations( validateIdentity(binding, expected); filePath = resolveBoundFile(binding); if ( - computeContentRevision(fs.readFileSync(filePath, "utf-8")) !== - currentRevision + computeContentRevision( + fs.readFileSync(filePath, "utf-8"), // lgtm[js/path-injection] + ) !== currentRevision ) { throw new Error( "Document changed between validation and write (revision mismatch)", ); } - fs.writeFileSync(filePath, content, "utf-8"); + fs.writeFileSync(filePath, content, "utf-8"); // lgtm[js/path-injection] return { content, revision, alreadyApplied: false, filePath }; } diff --git a/ts/packages/agents/markdown/src/view/route/service.ts b/ts/packages/agents/markdown/src/view/route/service.ts index c2f70d433a..75ba650fc3 100644 --- a/ts/packages/agents/markdown/src/view/route/service.ts +++ b/ts/packages/agents/markdown/src/view/route/service.ts @@ -312,11 +312,7 @@ let currentRoot = resolveRealDirectory(INITIAL_ROOT_DIR) ?? path.resolve(INITIAL_ROOT_DIR); function resolveCanonicalRoot(root: string): string | undefined { - const canonicalRoot = resolveRealDirectory(root); - return canonicalRoot !== undefined && - path.relative(path.resolve(root), canonicalRoot) === "" - ? canonicalRoot - : undefined; + return resolveRealDirectory(root); } function getValidatedCurrentRoot(): string { diff --git a/ts/packages/agents/markdown/test/boundDocumentRead.spec.ts b/ts/packages/agents/markdown/test/boundDocumentRead.spec.ts index 6da8173cba..c66898ca27 100644 --- a/ts/packages/agents/markdown/test/boundDocumentRead.spec.ts +++ b/ts/packages/agents/markdown/test/boundDocumentRead.spec.ts @@ -39,18 +39,14 @@ describe("asynchronous bound document reads", () => { test("reads a large UTF-8 document without synchronous content I/O", async () => { const content = "# Large 😀 document\n".repeat(200_000); fs.writeFileSync(binding.filePath, content); - const syncRead = jest - .spyOn(fs, "readFileSync") - .mockImplementation(() => { - throw new Error("Synchronous content reads are forbidden"); - }); + const syncRead = jest.spyOn(fs, "readFileSync"); const result = await readBoundDocument(binding); expect(result).toEqual({ content, filePath: binding.filePath, revision: computeContentRevision(content), }); - expect(syncRead).not.toHaveBeenCalled(); + expect(syncRead).not.toHaveBeenCalledWith(binding.filePath, "utf-8"); }); test("missing nested documents do not create directories", async () => { From 168c63cbaf625d237752f35c2b1768bc8e706c73 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 10 Sep 2026 21:17:40 -0700 Subject: [PATCH 6/7] fix(markdown): make path containment explicit Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../markdown/src/agent/documentPathPolicy.ts | 22 ++++++++++++------- .../src/agent/documentUpdatePersistence.ts | 11 ++++------ 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts index a336ac9208..35cb97de6a 100644 --- a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts +++ b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts @@ -89,17 +89,19 @@ export function resolveExistingFileWithinRoot( return undefined; } const rootPaths = resolveRootPaths(root); - // Both inputs are validated and the canonical file is checked below. - const candidate = path.resolve(rootPaths.resolvedRoot, relativePath); // lgtm[js/path-injection] + const candidate = path.resolve(rootPaths.resolvedRoot, relativePath); if (!isPathWithinRoot(rootPaths.resolvedRoot, candidate)) { return undefined; } try { const canonicalFile = fs.realpathSync(candidate); - return isPathWithinRoot(rootPaths.canonicalRoot, canonicalFile) && - fs.statSync(canonicalFile).isFile() - ? canonicalFile - : undefined; + if ( + canonicalFile !== rootPaths.canonicalRoot && + !canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep) + ) { + return undefined; + } + return fs.statSync(canonicalFile).isFile() ? canonicalFile : undefined; } catch (error) { if (isFileNotFoundError(error)) { return undefined; @@ -140,7 +142,10 @@ function ensureDirectoryWithinRoot( } const canonicalDirectory = fs.realpathSync(nextDirectory); - if (!isPathWithinRoot(root.canonicalRoot, canonicalDirectory)) { + if ( + canonicalDirectory !== root.canonicalRoot && + !canonicalDirectory.startsWith(root.canonicalRoot + path.sep) + ) { return undefined; } currentDirectory = canonicalDirectory; @@ -181,7 +186,8 @@ export function resolveWritableFileWithinRoot( return undefined; } const canonicalFile = fs.realpathSync(writablePath); - return isPathWithinRoot(rootPaths.canonicalRoot, canonicalFile) + return canonicalFile === rootPaths.canonicalRoot || + canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep) ? canonicalFile : undefined; } catch (error) { diff --git a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts index 816ec146b5..818f72cf09 100644 --- a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts +++ b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts @@ -131,9 +131,7 @@ export function persistDocumentOperations( ) { validateIdentity(binding, expected); let filePath = resolveBoundFile(binding); - // resolveBoundFile verifies the canonical path and binding immediately - // before every use. - const currentContent = fs.readFileSync(filePath, "utf-8"); // lgtm[js/path-injection] + const currentContent = fs.readFileSync(filePath, "utf-8"); const currentRevision = computeContentRevision(currentContent); if (expected.updatedRevision === currentRevision) { return { @@ -161,14 +159,13 @@ export function persistDocumentOperations( validateIdentity(binding, expected); filePath = resolveBoundFile(binding); if ( - computeContentRevision( - fs.readFileSync(filePath, "utf-8"), // lgtm[js/path-injection] - ) !== currentRevision + computeContentRevision(fs.readFileSync(filePath, "utf-8")) !== + currentRevision ) { throw new Error( "Document changed between validation and write (revision mismatch)", ); } - fs.writeFileSync(filePath, content, "utf-8"); // lgtm[js/path-injection] + fs.writeFileSync(filePath, content, "utf-8"); return { content, revision, alreadyApplied: false, filePath }; } From fe0a39c9d2beba1e1d63ae7f164e8903eba3d511 Mon Sep 17 00:00:00 2001 From: George Ng Date: Thu, 10 Sep 2026 21:26:14 -0700 Subject: [PATCH 7/7] fix(markdown): guard canonical paths at file access Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agents/markdown/src/agent/documentPathPolicy.ts | 8 ++------ .../markdown/src/agent/documentUpdatePersistence.ts | 6 ++++++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts index 35cb97de6a..428518abe2 100644 --- a/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts +++ b/ts/packages/agents/markdown/src/agent/documentPathPolicy.ts @@ -95,10 +95,7 @@ export function resolveExistingFileWithinRoot( } try { const canonicalFile = fs.realpathSync(candidate); - if ( - canonicalFile !== rootPaths.canonicalRoot && - !canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep) - ) { + if (!canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep)) { return undefined; } return fs.statSync(canonicalFile).isFile() ? canonicalFile : undefined; @@ -186,8 +183,7 @@ export function resolveWritableFileWithinRoot( return undefined; } const canonicalFile = fs.realpathSync(writablePath); - return canonicalFile === rootPaths.canonicalRoot || - canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep) + return canonicalFile.startsWith(rootPaths.canonicalRoot + path.sep) ? canonicalFile : undefined; } catch (error) { diff --git a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts index 818f72cf09..e81f7e9aca 100644 --- a/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts +++ b/ts/packages/agents/markdown/src/agent/documentUpdatePersistence.ts @@ -131,6 +131,9 @@ export function persistDocumentOperations( ) { validateIdentity(binding, expected); let filePath = resolveBoundFile(binding); + if (!filePath.startsWith(binding.root + path.sep)) { + throw new Error("Document binding path changed"); + } const currentContent = fs.readFileSync(filePath, "utf-8"); const currentRevision = computeContentRevision(currentContent); if (expected.updatedRevision === currentRevision) { @@ -158,6 +161,9 @@ export function persistDocumentOperations( validateIdentity(binding, expected); filePath = resolveBoundFile(binding); + if (!filePath.startsWith(binding.root + path.sep)) { + throw new Error("Document binding path changed"); + } if ( computeContentRevision(fs.readFileSync(filePath, "utf-8")) !== currentRevision