From b191b35a3c30c6c0066370e1d29cc78beba8a7f2 Mon Sep 17 00:00:00 2001 From: Fini Date: Sun, 19 Apr 2026 18:33:12 +0800 Subject: [PATCH] fix(mcp): rollback re-syncs live canvas after restoring file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex stop-hook #6: saveDocument is DUAL-WRITE for file-backed paths (document-manager.ts:411) — it writes to disk AND calls pushLiveDocument. handleBatchDesign uses saveDocument so a bad insert gets pushed to the live canvas before our post-check runs. Prior rollback (6dfa88b) only restored the file via writeFile, leaving the live-canvas renderer showing the bad insert until the next refresh. Fix: after writeFile + invalidateCache, call saveDocument(fp, restoredDoc) in the rollback path. saveDocument will re-write the file (no-op, same content) AND call pushLiveDocument again with the restored doc, bringing the live canvas back in sync with disk. If the live-sync step fails, re-throw with a diagnostic noting the live canvas may be stale but disk is authoritative. Wrap the re-sync in try/catch so a transient live-sync failure doesn't swallow the original insert-failure reason. Disk is the source of truth and is already restored when we hit this path. If no sync URL is configured (common in unit-test contexts), pushLiveDocument is a no-op — so this is safe in all scenarios. Live-canvas-only paths (filePath='live://canvas') remain non-rollback-able because pushLiveDocument is a one-way push without a history mechanism; the pre-check is the primary defense there. 88/88 pen-mcp tests pass (rollback behavior unchanged at the observable-test level since our tests don't configure a live sync URL; the re-sync is correct by construction given saveDocument's published semantics). format + tsc green. --- .../pen-mcp/src/tools/element-tool-helpers.ts | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/pen-mcp/src/tools/element-tool-helpers.ts b/packages/pen-mcp/src/tools/element-tool-helpers.ts index 12550d3d9..f709cd712 100644 --- a/packages/pen-mcp/src/tools/element-tool-helpers.ts +++ b/packages/pen-mcp/src/tools/element-tool-helpers.ts @@ -4,6 +4,7 @@ import { invalidateCache, openDocument, resolveDocPath, + saveDocument, } from '../document-manager'; import { findNodeInTree, findParentInTree, getDocChildren } from '../utils/node-operations'; import { generateId } from '../utils/id'; @@ -176,11 +177,32 @@ export async function insertElementTree(args: { const rollback = async (reason: string): Promise => { if (!isLive && fileSnapshot !== null) { + // Restore disk content first so on-disk state is correct even if + // the subsequent live-canvas sync fails. await writeFile(resolvedFp, fileSnapshot, 'utf-8'); invalidateCache(resolvedFp); + // saveDocument is DUAL-WRITE for file-backed paths (document-manager.ts:411): + // it writes to disk AND calls pushLiveDocument. Our earlier + // handleBatchDesign call already pushed the bad insert to the live + // canvas; we must push the restored doc so the renderer state + // matches disk again. If no sync URL is active, pushLiveDocument + // is a no-op, so this is safe in all scenarios. + try { + const restoredDoc = await openDocument(resolvedFp); + await saveDocument(resolvedFp, restoredDoc); + } catch (syncErr) { + // Disk is the source of truth and is already restored; surface + // the sync failure so the caller knows live canvas may be stale + // but don't swallow the original insert failure. + throw new Error( + `${reason} (document restored to pre-insert state on disk, but re-syncing live canvas ` + + `after rollback failed: ${syncErr instanceof Error ? syncErr.message : String(syncErr)}. ` + + `Live canvas may show stale state until next refresh.)`, + ); + } } throw new Error( - `${reason} ${isLive ? '(live canvas cannot be atomically rolled back — the bad insert may still be visible until next refresh)' : '(document restored to pre-insert state)'}`, + `${reason} ${isLive ? '(live canvas cannot be atomically rolled back — the bad insert may still be visible until next refresh)' : '(document restored to pre-insert state on disk and re-synced to live canvas)'}`, ); };