From 24d755dc3525bbacd036970a08d007db05d02d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E1=84=8B=E1=85=B2=E1=84=8B=E1=85=AD=E1=86=BC=E1=84=90?= =?UTF-8?q?=E1=85=A2?= Date: Mon, 13 Jul 2026 01:59:56 +0900 Subject: [PATCH] fix(canvas): isolate remote changes from local history --- ...6-canonical-design-document-causal-host.md | 20 ++- src/canvas/design-document/DesignDocument.ts | 9 +- .../DesignDocumentPatchPort.test.ts | 126 +++++++++++++++--- .../DesignDocumentPatchPort.ts | 4 +- .../EditorEngineCausalIntegration.test.ts | 32 ++++- 5 files changed, 160 insertions(+), 31 deletions(-) diff --git a/docs/adr/0006-canonical-design-document-causal-host.md b/docs/adr/0006-canonical-design-document-causal-host.md index efa20920..84c6318a 100644 --- a/docs/adr/0006-canonical-design-document-causal-host.md +++ b/docs/adr/0006-canonical-design-document-causal-host.md @@ -36,21 +36,26 @@ validation and widen the public mutation surface. listeners before notifying patch observers and returning. A successful test-only or net-no-op batch returns without creating history or a publication. -4. DesignDocument commands, history restores, and patch-port commits reserve a +4. A concrete patch-port publication advances a conservative local history + barrier after the internal patch is recorded but before canonical + synchronization. It clears prior local undo and redo, and the remote change + itself is not locally undoable. Local commands authored afterward start a + fresh history epoch and undo back to the remote state. +5. DesignDocument commands, history restores, and patch-port commits reserve a monotonic ownership sequence before publication. Ownership remains active through snapshot synchronization and observer delivery so synchronous reentry fails closed. A subscriber added while synchronization is already handling a publication starts at the next publication. The sequence exists only in this synchronous scope and is not a transport clock. -5. `getEditorEngineDocumentHost(engine)` defers a ready change while a text or +6. `getEditorEngineDocumentHost(engine)` defers a ready change while a text or transform preview is active, while another document mutation is running, or while a ready change is already executing. The caller owns retry policy. The headless tracer schedules one microtask retry after an engine change; that is not a browser render-settle contract. -6. The first tracer uses a stable design-node id and one text-field +7. The first tracer uses a stable design-node id and one text-field replacement. It does not use positional rebase because current local DesignDocument commands publish a root replacement. -7. Unpublished causal/rebase packages remain SHA-pinned test dependencies. +8. Unpublished causal/rebase packages remain SHA-pinned test dependencies. Production `src/canvas/**` code does not import them and the published Canvas package keeps registry-only runtime dependencies. @@ -65,8 +70,8 @@ validation and widen the public mutation surface. selection restoration, or layout has completed before retrying. - A conflicting stable-id replacement fails on its authored `expected` value; it never overwrites a newer local field automatically. -- Patch-port commits currently enter the existing DesignDocument undo stack. - Canonical remote-history policy must be decided before production sync. +- The history barrier is deliberately fail-closed. It is not selective undo, + inverse rebase, replicated undo, or a way to recover pre-remote local history. ## Consequences and Follow-up @@ -75,6 +80,9 @@ validation and widen the public mutation surface. private labs. A FigJam browser test remains follow-up evidence. - Structural and positional delayed edits need granular DesignDocument command patches instead of a root replacement. +- A concrete remote publication discards pre-remote local undo and redo. A + future selective-undo design must rebase owned inverses explicitly before it + can preserve that history safely. - DOM caret handoff needs a separate selection adapter plus a render-settle signal; it must not be inferred from the authored graph. - FigJam composition/blur behavior and a real ReactDesignRenderer commit must diff --git a/src/canvas/design-document/DesignDocument.ts b/src/canvas/design-document/DesignDocument.ts index e6b8d258..4aa9a11d 100644 --- a/src/canvas/design-document/DesignDocument.ts +++ b/src/canvas/design-document/DesignDocument.ts @@ -41,14 +41,17 @@ export function createDesignDocument( ) as JSONDocument const publications = createDesignDocumentPublicationCoordinator({ - onExternalPublication: () => { - previousHistoryGroup = null - }, + onExternalPublication: advanceExternalHistoryBarrier, readSnapshot: () => snapshot, store, synchronize: synchronizeSnapshot, }) + function advanceExternalHistoryBarrier() { + previousHistoryGroup = null + store.history.clear() + } + function synchronizeSnapshot() { const nextSnapshot = freezeSnapshot( parseDesignDocumentSnapshot(store.value), diff --git a/src/canvas/design-document/DesignDocumentPatchPort.test.ts b/src/canvas/design-document/DesignDocumentPatchPort.test.ts index dc03c7a0..a4baf7be 100644 --- a/src/canvas/design-document/DesignDocumentPatchPort.test.ts +++ b/src/canvas/design-document/DesignDocumentPatchPort.test.ts @@ -8,10 +8,10 @@ import { } from './index' describe('DesignDocumentPatchPort', () => { - it('publishes a validated external patch through current document history', () => { + it('publishes a validated external patch outside local document history', () => { const document = createDesignDocument(createSnapshot()) const port = getDesignDocumentPatchPort(document) - const listener = vi.fn() + const listener = vi.fn(() => document.historyStatus()) document.subscribe(listener) @@ -28,17 +28,29 @@ describe('DesignDocumentPatchPort', () => { expect(document.read.node('leaf')?.text).toBe('Reviewed') expect(document.historyStatus()).toEqual({ canRedo: false, - canUndo: true, + canUndo: false, }) expect(listener).toHaveBeenCalledTimes(1) + expect(listener.mock.results[0]?.value).toEqual({ + canRedo: false, + canUndo: false, + }) }) it('rejects graph-invalid patches before publication', () => { const document = createDesignDocument(createSnapshot()) const port = getDesignDocumentPatchPort(document) const publication = vi.fn() - const snapshot = document.snapshot + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) + const snapshot = document.snapshot port.subscribe(publication) const invalidPatch = [ @@ -63,7 +75,10 @@ describe('DesignDocumentPatchPort', () => { reason: 'DesignDocument authored content does not own text selection', }) expect(document.snapshot).toBe(snapshot) + expect(document.historyStatus().canUndo).toBe(true) expect(publication).not.toHaveBeenCalled() + expect(document.undo()).toBe(true) + expect(document.read.node('leaf')?.text).toBe('Draft') }) it('rejects patches that would normalize differently in canonical state', () => { @@ -91,23 +106,33 @@ describe('DesignDocumentPatchPort', () => { expect(publication).not.toHaveBeenCalled() }) - it('does not create history or a publication for a successful no-op patch', () => { + it('preserves local history and emits no publication for a successful no-op patch', () => { const document = createDesignDocument(createSnapshot()) const port = getDesignDocumentPatchPort(document) const publication = vi.fn() + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) port.subscribe(publication) expect(port.commit([ - { op: 'test', path: '/nodes/1/text', value: 'Draft' }, - { op: 'replace', path: '/nodes/1/text', value: 'Draft' }, + { op: 'test', path: '/nodes/1/text', value: 'Local' }, + { op: 'replace', path: '/nodes/1/text', value: 'Local' }, ])).toEqual({ ok: true }) expect(document.historyStatus()).toEqual({ canRedo: false, - canUndo: false, + canUndo: true, }) - expect(document.read.node('leaf')?.text).toBe('Draft') + expect(document.read.node('leaf')?.text).toBe('Local') expect(publication).not.toHaveBeenCalled() + expect(document.undo()).toBe(true) + expect(document.read.node('leaf')?.text).toBe('Draft') }) it('keeps ordinary document commands atomic and undoable', () => { @@ -196,9 +221,19 @@ describe('DesignDocumentPatchPort', () => { const port = getDesignDocumentPatchPort(document) const observed: unknown[] = [] + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) + port.subscribe(() => { observed.push({ document: document.snapshot.nodes[1]?.text, + history: document.historyStatus(), pointer: port.at('/nodes/1/text'), query: port.query('$.nodes[?@.id == "leaf"].text'), value: port.value.nodes[1]?.text, @@ -210,6 +245,10 @@ describe('DesignDocumentPatchPort', () => { ])).toEqual({ ok: true }) expect(observed).toEqual([{ document: 'Remote', + history: { + canRedo: false, + canUndo: false, + }, pointer: { ok: true, path: '/nodes/1/text', @@ -273,7 +312,7 @@ describe('DesignDocumentPatchPort', () => { expect(document.read.node('leaf')?.text).toBe('Remote') }) - it('owns immutable patch inputs and observer payloads across undo and redo', () => { + it('owns immutable patch data across later local undo and redo', () => { const document = createDesignDocument(createSnapshot()) const port = getDesignDocumentPatchPort(document) const operations = [ @@ -301,14 +340,22 @@ describe('DesignDocumentPatchPort', () => { ]) expect(healthy.mock.calls[0]?.[1]).toBeUndefined() expect(Object.isFrozen(healthy.mock.calls[0]?.[0])).toBe(true) + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) expect(document.undo()).toBe(true) - expect(document.read.node('leaf')?.text).toBe('Draft') + expect(document.read.node('leaf')?.text).toBe('Remote') expect(document.redo()).toBe(true) + expect(document.read.node('leaf')?.text).toBe('Local') expect(document.snapshot.roots).toEqual(['root']) - expect(document.read.node('leaf')?.text).toBe('Remote') }) - it('uses an external publication as a local history-group boundary', () => { + it('uses an external publication as a local history barrier', () => { const document = createDesignDocument(createSnapshot()) const port = getDesignDocumentPatchPort(document) @@ -324,6 +371,10 @@ describe('DesignDocumentPatchPort', () => { expect(port.commit([ { op: 'replace', path: '/nodes/1/text', value: 'Remote' }, ])).toEqual({ ok: true }) + expect(document.historyStatus()).toEqual({ + canRedo: false, + canUndo: false, + }) expect(document.execute({ changes: [{ nodeId: 'leaf', @@ -336,8 +387,43 @@ describe('DesignDocumentPatchPort', () => { expect(document.undo()).toBe(true) expect(document.read.node('leaf')?.text).toBe('Remote') + expect(document.historyStatus()).toEqual({ + canRedo: true, + canUndo: false, + }) + expect(document.undo()).toBe(false) + expect(document.read.node('leaf')?.text).toBe('Remote') + expect(document.redo()).toBe(true) + expect(document.read.node('leaf')?.text).toBe('Local B') + }) + + it('discards stale local redo when an external publication advances the barrier', () => { + const document = createDesignDocument(createSnapshot()) + const port = getDesignDocumentPatchPort(document) + + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) expect(document.undo()).toBe(true) - expect(document.read.node('leaf')?.text).toBe('Local A') + expect(document.historyStatus()).toEqual({ + canRedo: true, + canUndo: false, + }) + + expect(port.commit([ + { op: 'replace', path: '/nodes/1/text', value: 'Remote' }, + ])).toEqual({ ok: true }) + expect(document.historyStatus()).toEqual({ + canRedo: false, + canUndo: false, + }) + expect(document.redo()).toBe(false) + expect(document.read.node('leaf')?.text).toBe('Remote') }) it('rejects non-string metadata before mutating the internal store', () => { @@ -359,8 +445,18 @@ describe('DesignDocumentPatchPort', () => { expect(port.commit([ { op: 'replace', path: '/nodes/1/text', value: 'Next' }, ])).toEqual({ ok: true }) + expect(document.undo()).toBe(false) + expect(document.read.node('leaf')?.text).toBe('Next') + expect(document.execute({ + changes: [{ + nodeId: 'leaf', + type: 'update', + values: { text: 'Local' }, + }], + label: 'Local text', + })).toEqual({ changed: true, ok: true }) expect(document.undo()).toBe(true) - expect(document.read.node('leaf')?.text).toBe('Draft') + expect(document.read.node('leaf')?.text).toBe('Next') }) it('never lets runtime-invalid command metadata split canonical state', () => { diff --git a/src/canvas/design-document/DesignDocumentPatchPort.ts b/src/canvas/design-document/DesignDocumentPatchPort.ts index 092680f8..89b6eeb0 100644 --- a/src/canvas/design-document/DesignDocumentPatchPort.ts +++ b/src/canvas/design-document/DesignDocumentPatchPort.ts @@ -20,8 +20,8 @@ import { validateAndIndexDesignDocument } from './DesignDocumentValidation' /** * Guarded JSON projection for integrations that need patch publication. * Commits pass both the persisted schema and DesignDocument graph invariants, - * then synchronize the immutable snapshot before returning. They currently - * use the DesignDocument history stack and do not accept text selection. + * then synchronize the immutable snapshot before returning. Concrete external + * commits advance the local history barrier and do not accept text selection. */ export type DesignDocumentPatchPort = Pick< JSONDocument, diff --git a/src/canvas/editor-engine/EditorEngineCausalIntegration.test.ts b/src/canvas/editor-engine/EditorEngineCausalIntegration.test.ts index f76cc926..5ea41b2e 100644 --- a/src/canvas/editor-engine/EditorEngineCausalIntegration.test.ts +++ b/src/canvas/editor-engine/EditorEngineCausalIntegration.test.ts @@ -96,7 +96,14 @@ describe('EditorEngine causal integration tracer', () => { }) expect(document.read.node('note-a')?.text).toBe('Composing A') expect(document.read.node('note-b')?.text).toBe('Remote B') - expect(engine.snapshot().preview).toBeNull() + expect(document.historyStatus()).toEqual({ + canRedo: false, + canUndo: false, + }) + expect(engine.snapshot()).toMatchObject({ + history: { canRedo: false, canUndo: false }, + preview: null, + }) expect(projection.element('note-a')).toBe(noteAElement) expect(projection.element('note-b')).toBe(noteBElement) expect(inbox.current()).toMatchObject({ @@ -163,6 +170,10 @@ describe('EditorEngine causal integration tracer', () => { policy: 'stable-id-replace', }) expect(document.read.node('note-b')?.text).toBe('Local B') + expect(document.historyStatus()).toEqual({ + canRedo: false, + canUndo: true, + }) expect(publications).toEqual(['design-document']) expect(inbox.current()).toMatchObject({ failure: { @@ -171,6 +182,8 @@ describe('EditorEngine causal integration tracer', () => { }, status: 'blocked', }) + expect(document.undo()).toBe(true) + expect(document.read.node('note-b')?.text).toBe('Draft B') stopPublication() inbox.dispose() @@ -186,13 +199,19 @@ describe('EditorEngine causal integration tracer', () => { getDesignDocumentPatchPort(document), { host: getEditorEngineDocumentHost(engine) }, ) + expect(engine.commands.execute({ + edits: [{ target: 'text', value: 'Local B' }], + label: 'Edit note B locally', + nodeId: 'note-b', + type: 'node.edit', + })).toEqual({ changed: true, ok: true }) const before = engine.snapshot() expect(inbox.ingest({ id: 'only-test', dependsOn: [], operations: [ - { op: 'test', path: '/nodes/2/text', value: 'Draft B' }, + { op: 'test', path: '/nodes/2/text', value: 'Local B' }, ], })).toMatchObject({ applied: ['only-test'], @@ -200,17 +219,20 @@ describe('EditorEngine causal integration tracer', () => { }) expect(document.historyStatus()).toEqual({ canRedo: false, - canUndo: false, + canUndo: true, }) expect(engine.snapshot()).toMatchObject({ - history: { canRedo: false, canUndo: false }, + history: { canRedo: false, canUndo: true }, revision: before.revision, }) expect(inbox.current()).toMatchObject({ frontier: ['only-test'], - journalRevision: 1, + journalRevision: 2, status: 'active', }) + expect(engine.commands.execute({ type: 'history.undo' })) + .toEqual({ changed: true, ok: true }) + expect(document.read.node('note-b')?.text).toBe('Draft B') inbox.dispose() engine.dispose()