fix: address Christian's review findings

- Guard deactivate() against _restoringState to prevent store() from
  overwriting viewport data during undo/redo
- Extract isActiveTracker() helper to DRY the active-tracker check
- Emit one-time deprecation warning in production (not just DEV)
- Add @internal JSDoc to deactivate/prepareForSave
- Add transaction lifecycle test (beforeChange/afterChange -> single undo)
- Add updateModified/graphChanged assertion to state capture test
- Add deactivate _restoringState guard test
- Fix docs: "above" -> "below" for Lifecycle Methods reference

docs/architecture/change-tracker.md

@@ -73,7 +73,7 @@ These locations call `captureCanvasState()` directly:
 - `useCoreCommands.ts` — After metadata/subgraph commands
 
 `workflowService.ts` calls `captureCanvasState()` indirectly via
-`deactivate()` and `prepareForSave()` (see Lifecycle Methods above).
+`deactivate()` and `prepareForSave()` (see Lifecycle Methods below).
 
 ## Lifecycle Methods
 

src/scripts/changeTracker.test.ts

@@ -60,6 +60,7 @@ vi.mock('@/platform/workflow/management/stores/workflowStore', () => ({
 }))
 
 import { app } from '@/scripts/app'
+import { api } from '@/scripts/api'
 import { ChangeTracker } from '@/scripts/changeTracker'
 
 let nodeIdCounter = 0
@@ -161,7 +162,7 @@ describe('ChangeTracker', () => {
     })
 
     describe('state capture', () => {
-      it('pushes to undoQueue and updates activeState when state differs', () => {
+      it('pushes to undoQueue, updates activeState, and calls updateModified', () => {
         const initial = createState(1)
         const tracker = createTracker(initial)
         const changed = createState(2)
@@ -172,6 +173,10 @@ describe('ChangeTracker', () => {
         expect(tracker.undoQueue).toHaveLength(1)
         expect(tracker.undoQueue[0]).toEqual(initial)
         expect(tracker.activeState).toEqual(changed)
+        expect(api.dispatchCustomEvent).toHaveBeenCalledWith(
+          'graphChanged',
+          changed
+        )
       })
 
       it('does not push when state is identical', () => {
@@ -194,6 +199,23 @@ describe('ChangeTracker', () => {
         expect(tracker.redoQueue).toHaveLength(0)
       })
 
+      it('produces a single undo entry for a beforeChange/afterChange transaction', () => {
+        const tracker = createTracker(createState(1))
+        const intermediate = createState(2)
+        const final = createState(3)
+
+        tracker.beforeChange()
+        mockCanvasState(intermediate)
+        tracker.captureCanvasState()
+        expect(tracker.undoQueue).toHaveLength(0)
+
+        mockCanvasState(final)
+        tracker.afterChange()
+
+        expect(tracker.undoQueue).toHaveLength(1)
+        expect(tracker.activeState).toEqual(final)
+      })
+
       it('caps undoQueue at MAX_HISTORY', () => {
         const tracker = createTracker(createState(1))
         for (let i = 0; i < ChangeTracker.MAX_HISTORY; i++) {
@@ -222,6 +244,16 @@ describe('ChangeTracker', () => {
       expect(mockSubgraphNavigationStore.exportState).toHaveBeenCalled()
     })
 
+    it('is a no-op during undo/redo to avoid overwriting viewport', () => {
+      const tracker = createTracker(createState(1))
+      tracker._restoringState = true
+
+      tracker.deactivate()
+
+      expect(app.rootGraph.serialize).not.toHaveBeenCalled()
+      expect(mockNodeOutputStore.snapshotOutputs).not.toHaveBeenCalled()
+    })
+
     it('is a full no-op when called on inactive tracker', () => {
       const tracker = createTracker()
       mockWorkflowStore.activeWorkflow = { changeTracker: {} }

src/scripts/changeTracker.ts

@@ -26,6 +26,10 @@ const logger = log.getLogger('ChangeTracker')
 // Change to debug for more verbose logging
 logger.setLevel('info')
 
+function isActiveTracker(tracker: ChangeTracker): boolean {
+  return useWorkflowStore().activeWorkflow?.changeTracker === tracker
+}
+
 export class ChangeTracker {
   static MAX_HISTORY = 50
   /**
@@ -99,11 +103,14 @@ export class ChangeTracker {
    * PRECONDITION: must be called while this workflow is still the active one
    * (before the activeWorkflow pointer is moved). If called after the pointer
    * has already moved, this is a no-op to avoid freezing wrong viewport data.
+   * Also skipped during undo/redo to avoid overwriting viewport with
+   * intermediate canvas state.
+   *
+   * @internal Not part of the public extension API.
    */
   deactivate() {
-    const isActive = useWorkflowStore().activeWorkflow?.changeTracker === this
-    if (!isActive) {
-      if (import.meta.env.DEV) {
+    if (!isActiveTracker(this) || this._restoringState) {
+      if (import.meta.env.DEV && !this._restoringState) {
         logger.error(
           'deactivate() called on inactive tracker for:',
           this.workflow.path
@@ -119,10 +126,11 @@ export class ChangeTracker {
    * Ensure activeState is up-to-date for persistence.
    * Active workflow: flushes canvas → activeState.
    * Inactive workflow: no-op (activeState was frozen by deactivate()).
+   *
+   * @internal Not part of the public extension API.
    */
   prepareForSave() {
-    const isActive = useWorkflowStore().activeWorkflow?.changeTracker === this
-    if (isActive) this.captureCanvasState()
+    if (isActiveTracker(this)) this.captureCanvasState()
   }
 
   restore() {
@@ -186,8 +194,7 @@ export class ChangeTracker {
     )
       return
 
-    const activeTracker = useWorkflowStore().activeWorkflow?.changeTracker
-    if (activeTracker !== this) {
+    if (!isActiveTracker(this)) {
       if (import.meta.env.DEV) {
         logger.error(
           'captureCanvasState called on inactive tracker for:',
@@ -217,7 +224,8 @@ export class ChangeTracker {
 
   /** @deprecated Use {@link captureCanvasState} instead. */
   checkState() {
-    if (import.meta.env.DEV) {
+    if (!ChangeTracker._checkStateWarned) {
+      ChangeTracker._checkStateWarned = true
       logger.warn(
         'checkState() is deprecated — use captureCanvasState() instead.'
       )
@@ -225,6 +233,8 @@ export class ChangeTracker {
     this.captureCanvasState()
   }
 
+  private static _checkStateWarned = false
+
   async updateState(source: ComfyWorkflowJSON[], target: ComfyWorkflowJSON[]) {
     const prevState = source.pop()
     if (prevState) {