fix: address code review findings for ChangeTracker refactor

- Guard captureCanvasState against _restoringState to prevent undo
  history corruption when deactivate() is called during undo/redo
- Add deprecated checkState() wrapper for extension backward compat
- Use optional chaining for deactivate() call in beforeLoadNewGraph
- Add deactivate/prepareForSave to mock utility
- Document lifecycle methods and preconditions in JSDoc and arch docs

docs/architecture/change-tracker.md

@@ -73,6 +73,17 @@ These locations already call `captureCanvasState()` explicitly:
 - `useCoreCommands.ts` — After metadata/subgraph commands
 - `workflowService.ts` — After workflow service operations
 
+## Lifecycle Methods
+
+| Method                 | Caller                          | Purpose                                                                                                                    |
+| ---------------------- | ------------------------------- | -------------------------------------------------------------------------------------------------------------------------- |
+| `captureCanvasState()` | Event handlers, UI interactions | Snapshots canvas into activeState, pushes undo. Asserts active tracker.                                                    |
+| `deactivate()`         | `beforeLoadNewGraph` only       | `captureCanvasState()` + `store()`. Freezes everything for tab switch. Must be called while this workflow is still active. |
+| `prepareForSave()`     | Save paths only                 | Active: calls `captureCanvasState()`. Inactive: no-op (state was frozen by `deactivate()`).                                |
+| `store()`              | Internal to `deactivate()`      | Saves viewport scale/offset, node outputs, subgraph navigation.                                                            |
+| `restore()`            | `afterLoadNewGraph`             | Restores viewport, outputs, subgraph navigation.                                                                           |
+| `reset()`              | `afterLoadNewGraph`, save       | Resets initial state (marks workflow as "clean").                                                                          |
+
 ## Transaction Guards
 
 For operations that make multiple changes that should be a single undo entry:
@@ -92,6 +103,8 @@ The `litegraph:canvas` custom event also supports this with `before-change` /
   inactive trackers get an early return (and a DEV error log)
 - `captureCanvasState()` is a no-op during `loadGraphData` (guarded by
   `isLoadingGraph`) to prevent cross-workflow corruption
+- `captureCanvasState()` is a no-op during undo/redo (guarded by
+  `_restoringState`) to prevent undo history corruption
 - `captureCanvasState()` is a no-op when `changeCount > 0` (inside a transaction)
 - `undoQueue` is capped at 50 entries (`MAX_HISTORY`)
 - `graphEqual` ignores node order and `ds` (pan/zoom) when comparing

src/platform/workflow/core/services/workflowService.ts

@@ -366,7 +366,7 @@ export const useWorkflowService = () => {
     const workflowStore = useWorkspaceStore().workflow
     const activeWorkflow = workflowStore.activeWorkflow
     if (activeWorkflow) {
-      activeWorkflow.changeTracker.deactivate()
+      activeWorkflow.changeTracker?.deactivate()
       if (settingStore.get('Comfy.Workflow.Persist') && activeWorkflow.path) {
         const activeState = activeWorkflow.activeState
         if (activeState) {

src/scripts/changeTracker.ts

@@ -95,6 +95,10 @@ export class ChangeTracker {
    * Freeze this tracker's state before the workflow goes inactive.
    * Captures the final canvas state + viewport/outputs.
    * Called exactly once when switching away from this workflow.
+   *
+   * PRECONDITION: must be called while this workflow is still the active one
+   * (before the activeWorkflow pointer is moved). captureCanvasState() will
+   * no-op if called after the pointer has already moved.
    */
   deactivate() {
     this.captureCanvasState()
@@ -164,7 +168,13 @@ export class ChangeTracker {
    * Calling this on an inactive tracker would capture the wrong graph.
    */
   captureCanvasState() {
-    if (!app.graph || this.changeCount || ChangeTracker.isLoadingGraph) return
+    if (
+      !app.graph ||
+      this.changeCount ||
+      this._restoringState ||
+      ChangeTracker.isLoadingGraph
+    )
+      return
 
     const activeTracker = useWorkflowStore().activeWorkflow?.changeTracker
     if (activeTracker !== this) {
@@ -195,6 +205,16 @@ export class ChangeTracker {
     }
   }
 
+  /** @deprecated Use {@link captureCanvasState} instead. */
+  checkState() {
+    if (import.meta.env.DEV) {
+      logger.warn(
+        'checkState() is deprecated — use captureCanvasState() instead.'
+      )
+    }
+    this.captureCanvasState()
+  }
+
   async updateState(source: ComfyWorkflowJSON[], target: ComfyWorkflowJSON[]) {
     const prevState = source.pop()
     if (prevState) {

src/utils/__tests__/litegraphTestUtils.ts

@@ -257,6 +257,8 @@ export function createMockChangeTracker(
     redoQueue: [],
     changeCount: 0,
     captureCanvasState: vi.fn(),
+    deactivate: vi.fn(),
+    prepareForSave: vi.fn(),
     reset: vi.fn(),
     restore: vi.fn(),
     store: vi.fn(),