After RestoreFromState/LoadStateAsync, a never-saved state at position -1 reports no unsaved changes again (#76 regresses on load)

[matt-edmondson]

What's wrong

The #76 fix added _initialStateIsClean to SaveBoundaryManager, but the flag is not part of the saved state. UndoRedoService.RestoreFromState (Services/UndoRedoService.cs, ~lines 345-373) calls _saveBoundaryManager.Clear(), which resets the flag to true (Services/SaveBoundaryManager.cs, ~lines 516 and 604-608). Save boundaries are then recreated from the state, but when there are none the flag stays true.

So a loaded stack whose oldest commands were trimmed, and which has no save boundary, is dirty at position -1 but reports clean. That is exactly #76, and it comes back after every LoadStateAsync or RestoreFromState, including restoring a service from its own GetCurrentState().

Repro

MaxStackSize = 2. Execute 3 commands and Undo twice (position -1, which is after the first, trimmed command). Then either RestoreFromState(GetCurrentState()) into a new service, or round-trip through JsonUndoRedoSerializer.

before: pos=-1 v=1 HasUnsavedChanges=True
after restore: pos=-1 HasUnsavedChanges=False
after self-restore: pos=-1 HasUnsavedChanges=False
JSON round trip: loaded=True before=True after=False

Expected: HasUnsavedChanges stays True.

Reproduced against the current main build.

Why it matters

The app can close without prompting to save and lose the user's work. This is the scenario #76 fixed, and an app that persists its undo history across sessions hits it on every reload.

Suggested fix

Carry the "initial state is clean" flag through the saved state:

• Add a field to UndoRedoStackState and to the serializer's SerializableStackState, defaulting to true so old data still loads.
• Add a way on ISaveBoundaryManager to set the flag after Clear(), and call it from RestoreFromState.

If a format change is unwanted, a simpler fallback is to restore the flag as false whenever the saved state was dirty at -1.

Acceptance criteria

• The repro reports HasUnsavedChanges == True after restore and after the JSON round trip.
• Data saved before the change still loads.

[matt-edmondson]

Triage

• Category: Bug (regression of HasUnsavedChanges reports false for never-saved states at position -1 (after trimming or branching), risking silent data loss on close #76)
• Priority: High. After any LoadStateAsync / RestoreFromState, a dirty document reports that it has no unsaved changes, so an app can close without prompting and lose the user's work. Any app that persists its undo history across sessions hits this on every reload once the stack has been trimmed.
• Area / suggested assignee: SaveBoundaryManager._initialStateIsClean persistence, UndoRedoStackState / SerializableStackState, and UndoRedoService.RestoreFromState. Maintainer: @matt-edmondson.
• Duplicates / in progress: No duplicate, and no open PR in this repo. It regresses the fix for HasUnsavedChanges reports false for never-saved states at position -1 (after trimming or branching), risking silent data loss on close #76 (closed). It touches the same RestoreFromState path as LoadStateAsync throws NullReferenceException/ArgumentNullException on malformed state, and can wipe the live history before throwing #81 (malformed-state handling), so coordinate the two fixes to avoid conflicting edits there.
• Next steps: Carry the flag through the saved state as a new field that defaults to true, so old data still loads, and give ISaveBoundaryManager a setter to call after Clear(). If a format change is unwanted, use the fallback of restoring the flag as false when the saved state was dirty at -1. Add regression tests for a direct restore, a self-restore and a JSON round trip, plus one that loads pre-change data.

---

Generated by Claude Code

[matt-edmondson]

Triage

An automated run picked this up and handed it back before writing code. Every complete fix changes a public contract, and that choice belongs to the maintainer.

What I found

• IUndoRedoSerializer.SerializeAsync(commands, currentPosition, saveBoundaries, ct) never receives the "initial state is clean" flag. JsonUndoRedoSerializer therefore has nothing to write, and the JSON round-trip criterion cannot be met without changing that interface or adding a new one.
• RestoreFromState can only reach the flag through ISaveBoundaryManager, which has no member to set it. Adding one is a breaking change for anyone who implements the interface. The library targets netstandard2.0, so a default interface method is not an option.
• The in-memory part is safe to change: an init property InitialStateIsClean { get; init; } = true on UndoRedoStackState, filled by GetCurrentState(). It adds to the API without breaking it, and old data defaults to clean.

Options

1. Accept a [major] bump. Add SetInitialStateClean(bool) (or similar) to ISaveBoundaryManager, add an initialStateIsClean parameter to IUndoRedoSerializer.SerializeAsync, and add the matching field to SerializableStackState, defaulting to true.
2. Keep the contracts. Add an opt-in IUndoRedoStateSerializer with SerializeAsync(UndoRedoStackState, ct) that JsonUndoRedoSerializer also implements. SaveStateAsync prefers it when present, and RestoreFromState sets the flag when the manager is the built-in SaveBoundaryManager.

Once you pick an option, the work is small and well defined. I've unassigned myself.

---

Generated by Claude Code

[matt-edmondson]

Decision (maintainer, 2026-09-28)

• Option 1, "Accept a [major] bump", is chosen. Make these three changes:
  • add a setter such as SetInitialStateClean(bool) to ISaveBoundaryManager
  • add an initialStateIsClean parameter to IUndoRedoSerializer.SerializeAsync
  • add the matching serialized field to SerializableStackState and UndoRedoStackState, defaulting to true so old data still loads
• Option 2, the opt-in IUndoRedoStateSerializer, is not taken. This follows the org-wide rule to take the correct fix and bump major.

Next reader: implement option 1 and add regression tests for a direct restore, a self-restore, a JSON round trip, and loading data saved before the change. Coordinate with #81 in RestoreFromState, and consider batching with other UndoRedo breaks into one major release.

---

Generated by Claude Code