Transaction labels for undo redo bug fix

packages/dds/tree/src/shared-tree/treeCheckout.ts

@@ -134,6 +134,20 @@ function* collectTreeLabels(node: LabelTree): IterableIterator<unknown> {
 	}
 }
 
+/**
+ * Deep-clones a {@link LabelTree} so the result is independent of the source.
+ * @remarks
+ * Used when capturing the label tree on a revertible so that subsequent mutations
+ * (by the framework or by external listeners reading {@link TransactionLabels.tree})
+ * cannot affect the labels the revertible will emit.
+ */
+function cloneLabelTree(tree: LabelTree): LabelTree {
+	return {
+		label: tree.label,
+		sublabels: tree.sublabels.map(cloneLabelTree),
+	};
+}
+
 /**
  * Builds the labels set for a change event from the label tree.
  * If the tree exists and contains at least one defined label, returns a set of all
@@ -621,6 +635,16 @@ export class TreeCheckout implements ITreeCheckout {
 			// Only clear the label tree when the outermost transaction has completed.
 			// Inner transactions' commits don't fire the "changed" event, so the label tree
 			// must remain intact until the outermost commit reads it.
+			//
+			// TODO: clear `labelTreeNode` before the changed event is emitted rather than here
+			// in the finally. As-is, a `changed` listener that calls `runTransaction` re-entrantly
+			// will see `labelTreeNode` still set and `pushLabelFrame` will mutate the just-closed
+			// outer tree by pushing the new label as a sublabel — even though the two transactions
+			// are logically independent (the outer has already committed, `transaction.size === 0`
+			// when the inner starts). This conflates the inner transaction's labels with the
+			// outer's, breaking label-scoped undo/redo: e.g. an "auto-stamp" listener pattern
+			// would cause its commits to inherit the user's editor label. Moving the clear before
+			// emit lets re-entrant `pushLabelFrame` create a fresh root for the inner transaction.
 			if (this.transaction.size === 0) {
 				this.labelTreeNode = undefined;
 				this.mostRecentlyClosedLabelNode = undefined;
@@ -713,6 +737,13 @@ export class TreeCheckout implements ITreeCheckout {
 				const kind = event.type === "append" ? event.kind : CommitKind.Default;
 				const { change, revision } = commit;
 
+				// Snapshot the label tree for this commit before any listener runs, so the captured
+				// value is stable against in-place mutations to `this.labelTreeNode` (e.g. by a
+				// re-entrant `runTransaction` from a listener) or to `metadata.labels.tree`
+				// (which aliases the same object).
+				const commitLabelTree =
+					this.labelTreeNode === undefined ? undefined : cloneLabelTree(this.labelTreeNode);
+
 				const getRevertible = hasSchemaChange(change)
 					? undefined
 					: (onRevertibleDisposed?: (revertible: RevertibleAlpha) => void) => {
@@ -731,6 +762,7 @@ export class TreeCheckout implements ITreeCheckout {
 								kind,
 								this,
 								onRevertibleDisposed,
+								commitLabelTree,
 							);
 							this.revertibleCommitBranches.set(
 								revision,
@@ -1008,13 +1040,16 @@ export class TreeCheckout implements ITreeCheckout {
 	 * @param kind - The {@link CommitKind} that produced this revertible (e.g., Default, Undo, Redo).
 	 * @param checkout - The {@link TreeCheckout} instance this revertible belongs to.
 	 * @param onRevertibleDisposed - Callback function that will be called when the revertible is disposed.
+	 * @param labelTree - The {@link LabelTree} (if any) active when the original commit was produced.
+	 * The revert commit inherits these labels so that hosts can scope undo/redo by transaction label.
 	 * @returns A {@link RevertibleAlpha} object.
 	 */
 	private createRevertible(
 		revision: RevisionTag,
 		kind: CommitKind,
 		checkout: TreeCheckout,
 		onRevertibleDisposed: ((revertible: RevertibleAlpha) => void) | undefined,
+		labelTree: LabelTree | undefined,
 	): RevertibleAlpha {
 		const commitBranches = checkout.revertibleCommitBranches;
 
@@ -1030,7 +1065,7 @@ export class TreeCheckout implements ITreeCheckout {
 					throw new UsageError("Unable to revert a revertible that has been disposed.");
 				}
 
-				const revertMetrics = checkout.revertRevertible(revision, kind);
+				const revertMetrics = checkout.revertRevertible(revision, kind, labelTree);
 				checkout.logger?.sendTelemetryEvent({
 					eventName: TreeCheckout.revertTelemetryEventName,
 					...revertMetrics,
@@ -1060,7 +1095,13 @@ export class TreeCheckout implements ITreeCheckout {
 
 				targetCheckout.revertibleCommitBranches.set(revision, revertibleBranch.fork());
 
-				return this.createRevertible(revision, kind, targetCheckout, onRevertibleDisposed);
+				return this.createRevertible(
+					revision,
+					kind,
+					targetCheckout,
+					onRevertibleDisposed,
+					labelTree,
+				);
 			},
 			dispose: () => {
 				if (revertible.status === RevertibleStatus.Disposed) {
@@ -1321,7 +1362,11 @@ export class TreeCheckout implements ITreeCheckout {
 		this.revertibles.delete(revertible);
 	}
 
-	private revertRevertible(revision: RevisionTag, kind: CommitKind): RevertMetrics {
+	private revertRevertible(
+		revision: RevisionTag,
+		kind: CommitKind,
+		labelTree: LabelTree | undefined,
+	): RevertMetrics {
 		this.editLock.checkUnlocked("Reverting a commit");
 		if (this.transaction.size > 0) {
 			throw new UsageError("Undo is not yet supported during transactions.");
@@ -1364,12 +1409,23 @@ export class TreeCheckout implements ITreeCheckout {
 			);
 		}
 
-		this.#transaction.activeBranch.apply(
-			change,
-			kind === CommitKind.Default || kind === CommitKind.Redo
-				? CommitKind.Undo
-				: CommitKind.Redo,
-		);
+		// Install the original commit's label tree so the revert commit's metadata inherits
+		// the same labels — letting hosts scope undo/redo by label. Reusing the captured tree
+		// (rather than wrapping it) ensures revert-of-revert does not introduce new nesting.
+		const previousLabelTreeNode = this.labelTreeNode;
+		this.labelTreeNode = labelTree;
+		try {
+			this.#transaction.activeBranch.apply(
+				change,
+				kind === CommitKind.Default || kind === CommitKind.Redo
+					? CommitKind.Undo
+					: CommitKind.Redo,
+			);
+		} finally {
+			// Restore rather than clear: a revert triggered from inside another labeled
+			// transaction's changed event must not wipe that outer commit's label state.
+			this.labelTreeNode = previousLabelTreeNode;
+		}
 
 		// Derive some stats about the reversion to return to the caller.
 		let revertAge = 0;