After RestoreFromState, SaveBoundary objects from GetCurrentState() or SaveBoundaryCreated no longer resolve: UndoToSaveBoundaryAsync returns false and GetCommandsToUndo returns nothing

[matt-edmondson]

What's wrong

UndoToSaveBoundaryAsync and GetCommandsToUndo map the SaveBoundary they are given to a live boundary through FindLiveSaveBoundary (UndoRedo/Services/UndoRedoService.cs:278-279). The match is by the internal Identity object (SaveBoundary.IsSameSavePointAs, UndoRedo/Models/SaveBoundary.cs:62,67). #88 added this so a boundary a caller already holds still works after the stack is trimmed. AdjustPositions keeps Identity because it builds the copy with the internal SaveBoundary(original, position) constructor (SaveBoundary.cs:37-42).

RestoreFromState rebuilds every boundary without its identity:

• built-in manager: SaveBoundaryManager.RestoreSaveBoundary (UndoRedo/Services/SaveBoundaryManager.cs:46-50) calls new SaveBoundary(saveBoundary.Position, saveBoundary.Description, saveBoundary.Timestamp), which gets a fresh Identity
• custom manager: CreateSaveBoundary (UndoRedoService.cs:441), which also makes a new identity

So once a state is restored, no SaveBoundary object from before the restore resolves any more. That includes the objects inside the very UndoRedoStackState that was restored, and the ones handed out by SaveBoundaryCreated. UndoToSaveBoundaryAsync quietly returns false and GetCommandsToUndo returns an empty sequence, even though the same save point is still in service.SaveBoundaries at the same position.

Failure scenario (reproduced on net10.0 against HEAD)

var svc = new UndoRedoService(new StackManager(), new SaveBoundaryManager(), new CommandMerger());
SaveBoundary? fromEvent = null;
svc.SaveBoundaryCreated += (_, e) => fromEvent = e.SaveBoundary;
int v = 0;
svc.Execute(new DelegateCommand("a", () => v++, () => v--));
svc.MarkAsSaved("s");                                   // boundary at 0
svc.Execute(new DelegateCommand("b", () => v++, () => v--));
svc.Execute(new DelegateCommand("c", () => v++, () => v--));

var state = svc.GetCurrentState();
svc.GetCommandsToUndo(fromEvent!).Count();              // 2
svc.RestoreFromState(state);                            // true, same history

svc.GetCommandsToUndo(fromEvent!).Count();              // 0   (expected 2)
svc.GetCommandsToUndo(state.SaveBoundaries[0]).Count(); // 0   (expected 2)
await svc.UndoToSaveBoundaryAsync(state.SaveBoundaries[0]); // false, position stays 2 (expected true, position 0)
svc.GetCommandsToUndo(svc.SaveBoundaries[0]).Count();   // 2   (only a freshly read boundary works)

Output of the probe:

before restore: toUndo=2
restore=True
after restore event boundary: toUndo=0
after restore state boundary: toUndo=0
UndoToSaveBoundaryAsync(state boundary)=False pos=2 v=3
live boundary: toUndo=2

A real-world case is an editor that keeps one history per open document and switches between them with GetCurrentState() / RestoreFromState(), while its UI keeps a "revert to save" list filled from SaveBoundaryCreated. After switching back to a document, every entry in that list does nothing. The call does not throw, and it returns the same false as "that save point is gone".

This is separate from #118, which covers restoring from the live Commands/SaveBoundaries views, and from #91, where timestamps are now kept. PR #122 touches RestoreFromState but does not change how boundaries are rebuilt.

Suggested fix

In SaveBoundaryManager.RestoreSaveBoundary, keep the incoming boundary's identity (and timestamp) by using the existing internal copy constructor:

_saveBoundaries.Add(new SaveBoundary(saveBoundary, saveBoundary.Position));

Boundaries that come back from JSON already have a fresh identity from deserialization, so nothing changes on the LoadStateAsync path. Optionally, document on IUndoRedoService.RestoreFromState that a custom ISaveBoundaryManager (the CreateSaveBoundary fallback) does not keep boundary identity.

Acceptance criteria

• After RestoreFromState(service.GetCurrentState()), UndoToSaveBoundaryAsync and GetCommandsToUndo accept the boundaries in that state and the ones from earlier SaveBoundaryCreated events, and act as they did before the restore.
• Timestamps are still kept (existing Save-boundary timestamps are reset to "now" on load, restore and stack trim #91 tests pass).
• Regression test covering the scenario above; the existing suite still passes.

[matt-edmondson]

Triage

• Category: Bug
• Priority: Medium. UndoToSaveBoundaryAsync returns a silent false and GetCommandsToUndo returns an empty sequence for save points that still exist. Any editor that swaps histories through GetCurrentState() / RestoreFromState() loses its "revert to save" entries. The fix is a one-line change in SaveBoundaryManager.RestoreSaveBoundary: use the identity-preserving copy constructor.
• Area / suggested assignment: SaveBoundaryManager.RestoreSaveBoundary (UndoRedo/Services/SaveBoundaryManager.cs:46-50), plus the CreateSaveBoundary fallback in UndoRedoService.RestoreFromState.
• Duplicates: none. Related to UndoToSaveBoundaryAsync undoes to the wrong position when given a SaveBoundary obtained before the stack was trimmed #88 (identity-based lookup), RestoreFromState wipes the whole history and returns true when the state is built from service.Commands / service.SaveBoundaries #118 (restoring from the live views), Save-boundary timestamps are reset to "now" on load, restore and stack trim #91 (timestamps kept) and Calling MarkAsSaved repeatedly with no edits in between appends a duplicate save boundary each time, so SaveBoundaries and the saved state grow without bound #121 (duplicate boundaries on repeated MarkAsSaved).
• In progress: no PR fixes this. Open PR Keep a trimmed stack dirty at -1 through save, load, and restore [major] #122 also edits RestoreFromState and SaveBoundaryManager, so rebase on it or coordinate with it.

---

Generated by Claude Code