Resolve a held save boundary to its live position before undoing to it

UndoRedo.Test/UndoRedoStackTests.cs

@@ -548,7 +548,7 @@
 		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
 
 		// Act
 		bool result = await stack.UndoAsync().ConfigureAwait(false);
 
 		// Assert
 		Assert.IsTrue(result, "UndoAsync should report the undo, which was applied before navigation failed");
@@ -564,10 +564,10 @@
 		stack.SetNavigationProvider(new ThrowingNavigationProvider());
 		int value = 0;
 		stack.Execute(new DelegateCommand("Increment", () => value++, () => value--, navigationContext: "editor"));
 		await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false);
 
 		// Act
 		bool result = await stack.RedoAsync().ConfigureAwait(false);
 
 		// Assert
 		Assert.IsTrue(result, "RedoAsync should report the redo, which was applied before navigation failed");
@@ -589,7 +589,7 @@
 		SaveBoundary boundary = stack.SaveBoundaries[0];
 
 		// Act
 		bool result = await stack.UndoToSaveBoundaryAsync(boundary).ConfigureAwait(false);
 
 		// Assert
 		Assert.IsTrue(result, "UndoToSaveBoundaryAsync should report the undo, which was applied before navigation failed");
@@ -812,6 +812,59 @@
 		Assert.IsTrue(stack.CanRedo, "C was undone, so it must be redoable");
 	}
 
+	[TestMethod]
+	public async Task UndoToSaveBoundary_BoundaryHeldAcrossTrim_UndoesToSavedState()
+	{
+		// Arrange
+		UndoRedoService stack = new(new StackManager(), new SaveBoundaryManager(), new CommandMerger(), UndoRedoOptions.Create(maxStackSize: 3));
+		int value = 0;
+		SaveBoundary? heldBoundary = null;
+		stack.SaveBoundaryCreated += (_, e) => heldBoundary = e.SaveBoundary;
+
+		stack.Execute(new DelegateCommand("A", () => value++, () => value--));
+		stack.MarkAsSaved();
+		stack.Execute(new DelegateCommand("B", () => value++, () => value--));
+		stack.Execute(new DelegateCommand("C", () => value++, () => value--));
+		stack.Execute(new DelegateCommand("D", () => value++, () => value--)); // Trims A, moving the save point to -1
+		Assert.IsNotNull(heldBoundary);
+		Assert.AreEqual(3, stack.GetCommandsToUndo(heldBoundary).Count(), "The held boundary should resolve to the save point's current position");
+
+		// Act
+		bool result = await stack.UndoToSaveBoundaryAsync(heldBoundary, navigateToLastChange: false).ConfigureAwait(false);
+
+		// Assert
+		Assert.IsTrue(result, "UndoToSaveBoundary should resolve a boundary held across a trim");
+		Assert.AreEqual(1, value, "The value should be back at the saved state");
+		Assert.AreEqual(-1, stack.CurrentPosition);
+		Assert.IsFalse(stack.HasUnsavedChanges, "The stack should be at the save point");
+	}
+
+	[TestMethod]
+	public async Task UndoToSaveBoundary_BoundaryRemovedByBranching_ReturnsFalse()
+	{
+		// Arrange
+		UndoRedoService stack = CreateService();
+		int value = 0;
+		stack.Execute(new DelegateCommand("A", () => value++, () => value--));
+		stack.Execute(new DelegateCommand("B", () => value++, () => value--));
+		stack.MarkAsSaved();
+		SaveBoundary removedBoundary = stack.SaveBoundaries[0];
+		await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false);
+		await stack.UndoAsync(navigateToChange: false).ConfigureAwait(false);
+		stack.Execute(new DelegateCommand("C", () => value++, () => value--)); // Branches, discarding the save point
+		stack.Execute(new DelegateCommand("D", () => value++, () => value--));
+		stack.Execute(new DelegateCommand("E", () => value++, () => value--));
+
+		// Act
+		bool result = await stack.UndoToSaveBoundaryAsync(removedBoundary, navigateToLastChange: false).ConfigureAwait(false);
+
+		// Assert
+		Assert.IsFalse(result, "UndoToSaveBoundary should reject a boundary that no longer exists");
+		Assert.AreEqual(3, value, "Nothing should have been undone");
+		Assert.AreEqual(2, stack.CurrentPosition);
+		Assert.IsEmpty(stack.GetCommandsToUndo(removedBoundary));
+	}
+
 	[TestMethod]
 	public async Task UndoToSaveBoundary_WhenAlreadyAtPosition_ReturnsFalse()
 	{

UndoRedo/Models/SaveBoundary.cs

@@ -9,6 +9,17 @@ namespace ktsu.UndoRedo;
 /// <param name="description">Optional description</param>
 public sealed class SaveBoundary(int position, string? description = null)
 {
+	/// <summary>
+	/// Creates a copy of <paramref name="original"/> at a new position that is still the same save point,
+	/// so a caller holding the original can have it resolved to where the save point is now
+	/// </summary>
+	internal SaveBoundary(SaveBoundary original, int position)
+		: this(position, original.Description)
+	{
+		Identity = original.Identity;
+		Timestamp = original.Timestamp;
+	}
+
 	/// <summary>
 	/// The position in the stack where this save boundary was created
 	/// </summary>
@@ -23,4 +34,14 @@ public sealed class SaveBoundary(int position, string? description = null)
 	/// Optional description of what was saved
 	/// </summary>
 	public string? Description { get; } = description;
+
+	/// <summary>
+	/// Shared by every copy of one save point as its position is adjusted
+	/// </summary>
+	internal object Identity { get; } = new();
+
+	/// <summary>
+	/// Whether this boundary and <paramref name="other"/> describe the same save point
+	/// </summary>
+	internal bool IsSameSavePointAs(SaveBoundary other) => ReferenceEquals(Identity, other.Identity);
 }

UndoRedo/Services/SaveBoundaryManager.cs

@@ -81,8 +81,9 @@ public void AdjustPositions(int adjustment)
 			}
 			else
 			{
-				// Create a new boundary with adjusted position
-				_saveBoundaries[i] = new SaveBoundary(newPosition, boundary.Description);
+				// Create a new boundary with adjusted position that is still the same save point, so a
+				// boundary a caller already holds can be resolved to it
+				_saveBoundaries[i] = new SaveBoundary(boundary, newPosition);
 			}
 		}
 	}

UndoRedo/Services/UndoRedoService.cs

@@ -217,14 +217,31 @@
 	}
 
 	/// <inheritdoc />
-	public IEnumerable<ICommand> GetCommandsToUndo(SaveBoundary saveBoundary) =>
-		_saveBoundaryManager.GetCommandsToUndo(saveBoundary, _stackManager.CurrentPosition, _stackManager.Commands);
+	public IEnumerable<ICommand> GetCommandsToUndo(SaveBoundary saveBoundary)
+	{
+		Ensure.NotNull(saveBoundary);
+
+		SaveBoundary? liveBoundary = FindLiveSaveBoundary(saveBoundary);
+		return liveBoundary == null
+			? []
+			: _saveBoundaryManager.GetCommandsToUndo(liveBoundary, _stackManager.CurrentPosition, _stackManager.Commands);
+	}
 
 	/// <inheritdoc />
 	public async Task<bool> UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool navigateToLastChange = true, CancellationToken cancellationToken = default)
 	{
 		Ensure.NotNull(saveBoundary);
 
+		// A boundary the caller has held since before the stack was trimmed carries a stale position,
+		// and one removed by branching or clearing no longer marks a reachable saved state
+		SaveBoundary? liveBoundary = FindLiveSaveBoundary(saveBoundary);
+		if (liveBoundary == null)
+		{
+			return false;
+		}
+
+		saveBoundary = liveBoundary;
+
 		if (_stackManager.CurrentPosition <= saveBoundary.Position)
 		{
 			return false;
@@ -254,6 +271,13 @@
 		return true;
 	}
 
+	/// <summary>
+	/// Resolves a save boundary, which may have been obtained before the stack was trimmed, to the live
+	/// boundary for the same save point, or null if that save point no longer exists
+	/// </summary>
+	private SaveBoundary? FindLiveSaveBoundary(SaveBoundary saveBoundary) =>
+		_saveBoundaryManager.SaveBoundaries.FirstOrDefault(boundary => boundary.IsSameSavePointAs(saveBoundary));
+
 	/// <summary>
 	/// Navigates to where a change was made, after the undo or redo has already been applied.
 	/// Navigation is best effort: any failure is swallowed, because an exception here would tell the
@@ -271,7 +295,7 @@
 
 		try
 		{
 			await _navigationProvider.NavigateToAsync(navigationContext!, cts.Token).ConfigureAwait(false);
 		}
 #pragma warning disable CA1031 // Do not catch general exception types
 		catch (Exception)

docs/api-reference.md

@@ -278,7 +278,7 @@ LoadNewDocument();
 ```csharp
 IEnumerable<ICommand> GetCommandsToUndo(SaveBoundary saveBoundary);
 ```
-Gets commands that would be undone to reach the specified save boundary.
+Gets commands that would be undone to reach the specified save boundary. Returns nothing for a boundary whose save point no longer exists.
 
 **Parameters:**
 - `saveBoundary`: The target save boundary
@@ -292,7 +292,7 @@ Gets commands that would be undone to reach the specified save boundary.
 ```csharp
 Task<bool> UndoToSaveBoundaryAsync(SaveBoundary saveBoundary, bool navigateToLastChange = true, CancellationToken cancellationToken = default);
 ```
-Undoes commands until reaching the specified save boundary.
+Undoes commands until reaching the specified save boundary. A boundary obtained earlier, for example from `SaveBoundaryCreated`, still resolves to its save point after `MaxStackSize` trims the stack. A boundary whose save point no longer exists, because a new branch or `Clear()` removed it, is rejected and nothing is undone.
 
 **Parameters:**
 - `saveBoundary`: The target save boundary