From 531e9f8e8f5f20555342824f0071672620d8d035 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Tue, 9 Jun 2026 00:35:43 +0000 Subject: [PATCH 1/2] fix(tree): prevent revert of commits that were rolled back --- .../dds/tree/src/shared-tree/treeCheckout.ts | 19 +++++++++------ .../src/test/shared-tree/treeCheckout.spec.ts | 23 +++++++++++++++++++ packages/dds/tree/src/test/utils.ts | 9 ++++++-- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/dds/tree/src/shared-tree/treeCheckout.ts b/packages/dds/tree/src/shared-tree/treeCheckout.ts index c5c48d7e4b29..88bfc915d9c2 100644 --- a/packages/dds/tree/src/shared-tree/treeCheckout.ts +++ b/packages/dds/tree/src/shared-tree/treeCheckout.ts @@ -486,9 +486,9 @@ export class TreeCheckout implements ITreeCheckout { private readonly views = new Set>(); /** - * Set of revertibles maintained for automatic disposal + * Revertibles maintained for automatic disposal */ - private readonly revertibles = new Set(); + private readonly revertibles = new Map(); /** * Each branch's head commit corresponds to a revertible commit. @@ -754,7 +754,7 @@ export class TreeCheckout implements ITreeCheckout { revision, this.#transaction.activeBranch.fork(commit), ); - this.revertibles.add(revertible); + this.revertibles.set(revision, revertible); return revertible; }; @@ -791,6 +791,11 @@ export class TreeCheckout implements ITreeCheckout { this.#events.emit("changed", metadata, getRevertible); withinEventContext = false; } + } else if (event.type === "remove") { + // Commits that are rolled back should no longer be revertible + for (const commit of event.removedCommits) { + this.revertibleCommitBranches.delete(commit.revision); + } } else if (this.isRemoteChangeEvent(event)) { // TODO: figure out how to plumb through commit kind info for remote changes this.#events.emit("changed", { @@ -1097,7 +1102,7 @@ export class TreeCheckout implements ITreeCheckout { "Unable to dispose a revertible that has already been disposed.", ); } - checkout.disposeRevertible(revertible, revision); + checkout.disposeRevertible(revision); onRevertibleDisposed?.(revertible); }, }; @@ -1339,15 +1344,15 @@ export class TreeCheckout implements ITreeCheckout { } private purgeRevertibles(): void { - for (const revertible of this.revertibles) { + for (const revertible of this.revertibles.values()) { revertible.dispose(); } } - private disposeRevertible(revertible: RevertibleAlpha, revision: RevisionTag): void { + private disposeRevertible(revision: RevisionTag): void { this.revertibleCommitBranches.get(revision)?.dispose(); this.revertibleCommitBranches.delete(revision); - this.revertibles.delete(revertible); + this.revertibles.delete(revision); } private revertRevertible( diff --git a/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts b/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts index 09ca13834efb..3b82d4b437eb 100644 --- a/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts @@ -1848,6 +1848,29 @@ describe("sharedTreeView", () => { fork1.dispose(); }); }); + + it("Rolled back commits cannot be reverted", () => { + const sf = new SchemaFactory("Enrichment Schema"); + class Node extends sf.object("Node", { id: sf.string }) {} + const NodeArray = sf.array(Node); + const provider = new TestTreeProviderLite(1); + const config = new TreeViewConfiguration({ schema: NodeArray, enableSchemaValidation }); + const view = provider.trees[0].kernel.viewWith(config); + view.initialize([]); + const init = view.checkout.mainBranch.getHead(); + + const { undoStack, unsubscribe } = createTestUndoRedoStacks(view.events); + view.root.insertAtEnd({ id: "A" }); + view.root[0].id = "B"; + assert.equal(undoStack.length, 2); + + view.checkout.mainBranch.removeAfter(init); + assert.equal(view.root.length, 0); + assert.equal(undoStack.length, 2); + assert.equal(undoStack[0].status, RevertibleStatus.Disposed); + assert.equal(undoStack[1].status, RevertibleStatus.Disposed); + unsubscribe(); + }); }); const defaultSf = new SchemaFactory("Checkout and view test schema"); diff --git a/packages/dds/tree/src/test/utils.ts b/packages/dds/tree/src/test/utils.ts index a995eec7e190..b42ff12325d4 100644 --- a/packages/dds/tree/src/test/utils.ts +++ b/packages/dds/tree/src/test/utils.ts @@ -119,6 +119,7 @@ import { type FieldKindIdentifier, type TreeNodeSchemaIdentifier, type TreeFieldStoredSchema, + RevertibleStatus, } from "../core/index.js"; import { FormatValidatorBasic } from "../external-utilities/index.js"; import { @@ -1347,10 +1348,14 @@ export function createTestUndoRedoStacks( const unsubscribe = (): void => { unsubscribeFromChangedEvent(); for (const revertible of undoStack) { - revertible.dispose(); + if (revertible.status !== RevertibleStatus.Disposed) { + revertible.dispose(); + } } for (const revertible of redoStack) { - revertible.dispose(); + if (revertible.status !== RevertibleStatus.Disposed) { + revertible.dispose(); + } } }; return { undoStack, redoStack, unsubscribe }; From 5b478aa321dd8693909366af49b0fc025f490114 Mon Sep 17 00:00:00 2001 From: yann-achard-MS <97201204+yann-achard-MS@users.noreply.github.com> Date: Wed, 10 Jun 2026 16:19:10 +0000 Subject: [PATCH 2/2] Fix --- packages/dds/tree/src/shared-tree/treeCheckout.ts | 2 +- packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/dds/tree/src/shared-tree/treeCheckout.ts b/packages/dds/tree/src/shared-tree/treeCheckout.ts index 88bfc915d9c2..668723d56138 100644 --- a/packages/dds/tree/src/shared-tree/treeCheckout.ts +++ b/packages/dds/tree/src/shared-tree/treeCheckout.ts @@ -794,7 +794,7 @@ export class TreeCheckout implements ITreeCheckout { } else if (event.type === "remove") { // Commits that are rolled back should no longer be revertible for (const commit of event.removedCommits) { - this.revertibleCommitBranches.delete(commit.revision); + this.revertibles.get(commit.revision)?.dispose(); } } else if (this.isRemoteChangeEvent(event)) { // TODO: figure out how to plumb through commit kind info for remote changes diff --git a/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts b/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts index 3b82d4b437eb..6343a7e95462 100644 --- a/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts +++ b/packages/dds/tree/src/test/shared-tree/treeCheckout.spec.ts @@ -1866,9 +1866,7 @@ describe("sharedTreeView", () => { view.checkout.mainBranch.removeAfter(init); assert.equal(view.root.length, 0); - assert.equal(undoStack.length, 2); - assert.equal(undoStack[0].status, RevertibleStatus.Disposed); - assert.equal(undoStack[1].status, RevertibleStatus.Disposed); + assert.equal(undoStack.length, 0); unsubscribe(); }); });