SemianiakaVY/openpencil
1
0
Fork 0
Code Issues Pull requests Actions 1 Packages Projects Releases Wiki Activity

fix(mcp): rollback re-syncs live canvas after restoring file

Browse source 
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.
This commit is contained in:
Fini 2026-04-19 18:33:12 +08:00
parent 36852d77ff
commit b191b35a3c
1 changed files with 23 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
packages/pen-mcp/src/tools/element-tool-helpers.ts
View file

@ -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<never> => {
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)'}`,
);
};