ChangeTracker.reset() re-baselines initialState without recomputing isModified

[christian-byrne]

Summary

ChangeTracker.reset() is documented as "Save the current state as the initial state", but it re-baselines initialState only. workflow.isModified is derived from initialState by updateModified(), and reset() never calls it — so after a reset the workflow keeps a stale dirty flag until some unrelated graph change happens to recompute it.

The code

src/scripts/changeTracker.ts:

reset(state?: ComfyWorkflowJSON) {
  if (this._restoringState) return
  if (state) this.activeState = clone(state)
  this.initialState = clone(this.activeState)
}

and the only place isModified is derived:

updateModified(previousState?: ComfyWorkflowJSON) {
  const workflow = useWorkflowStore().getWorkflowByPath(this.workflow.path)
  if (workflow) {
    workflow.isModified = !ChangeTracker.graphEqual(
      this.initialState,
      this.activeState
    )
  }
  ...
}

After reset(), initialState and activeState are equal by construction, so isModified should be false — but nothing recomputes it, so it stays whatever it was.

How it surfaced

browser_tests/tests/assetDeleteClearsLoadImage.spec.ts re-baselines the tracker mid-test and then asserts the workflow is clean:

tracker?.reset?.()
await expect.poll(() => comfyPage.workflow.isCurrentWorkflowModified()).toBe(false)

That assertion only ever held because the upload preceding it never marked the workflow modified — which was itself the bug fixed in #15069. Once uploads correctly mark the workflow dirty, the reset() stopped clearing the flag and the test failed on the cloud project. It is worked around there by calling updateModified() after reset(); the underlying asymmetry is untouched.

So the test was silently resting on a product bug, and reset()'s incomplete semantics hid it.

Impact

Any caller that re-baselines a tracker and expects a clean workflow gets a stale dirty flag: a spurious unsaved-changes dot, and a spurious "unsaved changes" prompt on close/navigate. reset() is called from workflow load/activation paths, so this is not test-only.

Suggested fix

Have reset() recompute the flag it invalidates:

reset(state?: ComfyWorkflowJSON) {
  if (this._restoringState) return
  if (state) this.activeState = clone(state)
  this.initialState = clone(this.activeState)
  this.updateModified()
}

Note updateModified() also dispatches graphChanged, so this needs a check that the extra event is harmless on every reset() path (it drives the workflow-persistence debounce), or the flag update should be factored out of updateModified() so reset() can set it without emitting.

Deliberately not folded into #15069 — it changes shared change-tracker behaviour well outside that PR's intent.

[coderabbitai]

🔗 Related PRs

#14941 - fix: layer editor UX and review follow-ups [open]
#14971 - perf: reuse execution graph projections [open]
#14979 - test: harden run-on-change regression tests [merged]
#15053 - test: clear local storage before each test [merged]
#15072 - test: remove redundant mock cleanup [open]

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!