fix: stop Canvas Navigation mode resetting to Custom on reload (#14716)

*PR Created by the Glary-Bot Agent*

---

Fixes
[FE-1503](https://linear.app/comfyorg/issue/FE-1503/bug-canvas-navigation-setting-resets-to-custom-after-page-refresh).

> Stacked on #14714, which repairs `typecheck:browser` on `main`. Review
that first; this PR targets its branch so the diff stays clean.

## Problem

Pick a Navigation Mode, refresh, and the dropdown reads **Custom** —
while the Left Mouse Click Behavior / Mouse Wheel Scroll radios still
show the preset you chose.

`Comfy.Canvas.NavigationMode` is stored independently of the two
overrides it implies, and the three were kept in sync by cross-writes in
`onChange`. Two separate paths destroyed the stored mode.

**1. The preset cascade read a stale mode.** `applySettingLocally` fired
`onChange` *before* committing the new value:

```ts
const oldValue = get(key)
if (newValue === oldValue) return undefined
onChange(settingsById.value[key], newValue, oldValue)   // fired first
settingValues.value[key] = typedNewValue                // committed second
```

So selecting `standard` cascaded into `setMany`, and
`LeftMouseClickBehavior.onChange` read `NavigationMode` as the value it
was replacing (`legacy`), concluded `select` no longer matched it, and
wrote `NavigationMode = 'custom'`. Captured from one dropdown click:

```
POST /settings/Comfy.Canvas.NavigationMode   "custom"     ← spurious
POST /settings   {LeftMouseClickBehavior:"select", MouseWheelScroll:"panning"}
POST /settings/Comfy.Canvas.NavigationMode   "standard"
```

Three concurrent writes, two to the same key. ComfyUI's
`app_settings.py` has an `await request.json()` between its file read
and write, so these whole-file read-modify-writes are not atomic and the
last one to land wins.

**2. A mode stored before 1.27.4 was overruled by the override
defaults.** `addSetting` replays `onChange` for every setting at
registration. The overrides shipped in 1.27.4; `NavigationMode` shipped
in 1.25.0. Anyone who chose a mode in between has only the mode on
record, so the overrides load as their defaults — `panning`/`zoom`,
which describe `legacy`. The override handlers saw the mismatch and
rewrote the mode to `custom` on the first load after upgrading, with no
user interaction. Deterministic, and it explains "has been there for
quite some time" on both Windows and Mac.

Once `custom` is stored the handlers no-op, so it never recovers. This
also isn't purely cosmetic: `useCanvasInteractions` gates on
`NavigationMode === 'standard'` exactly, so the standard-mode wheel path
silently turns off.

## Fix

- **Commit the value before firing `onChange`** so a cascade observes
the mode it is applying. This removes the spurious write entirely,
leaving one write per key.
- **Treat a stored preset as authoritative.** On the registration
replay, a stored preset now supplies the overrides that were never
stored, instead of being overruled by their defaults. The preset pairs
move into one `CANVAS_NAVIGATION_PRESETS` map rather than being restated
per branch.

Muting the override handlers during registration was the smaller change
and I started there, but review caught that it only fixes the label: the
dropdown would read Standard while the canvas still panned and zoomed
like Legacy. Making the preset supply its missing overrides fixes both,
and affected profiles self-heal on next load.

## Tests

Written first, and each fails on `main`:

| Test | On `main` |
|---|---|
| picking a preset never persists custom | `Received array: ["custom",
"standard"]` |
| keeps the stored preset through load | `Expected "standard"`,
`Received "custom"` |
| applies the stored preset to the overrides | `Expected "select"`,
`Received "panning"` |

The third is the one that would have caught the label-only fix. The unit
test in `settingStore.test.ts` pins the ordering contract directly
(`['default','default']` vs `['default','newvalue']` without the fix),
since it governs every setting, not just this cluster.

## Verification

Reproduced and confirmed end to end against a local ComfyUI backend.
Seeded a pre-1.27.4 profile (`NavigationMode: 'standard'`, overrides
absent) and loaded the page with no user interaction:

**Before** — `comfy.settings.json` silently rewritten to `custom`;
dropdown reads Custom while the radios still show the Standard pair:

![before](https://placeholder-before)

**After** — mode holds, and the overrides materialise to
`select`/`panning` so label and behaviour agree:

![after](https://placeholder-after)

- 3 Playwright tests + `settingStore` unit test: fail on `main`, pass
here
- `canvasSettings.spec.ts`: 5 pre-existing failures in this sandbox
(headless canvas drag + screenshot baselines) — identical set on clean
`main`; baseline 8 passed → 10 passed here
- `pnpm test:unit`: 3 pre-existing failing files (`previewAny`,
`onboardingCloudRoutes`, `GraphView`), confirmed failing on clean
`main`, none in the settings domain
- `pnpm typecheck`, `typecheck:browser`, `lint`, `format:check`, `knip`
— all clean

## Follow-ups (not in scope here)

- `Comfy.Canvas.NavigationMode` would be better derived from the two
overrides than stored as a third key — one source of truth, no
cross-setting cascade. That removes this bug class rather than this
instance.
- `POST /settings` and `POST /settings/{id}` in ComfyUI's
`app/app_settings.py` do non-atomic read-modify-write of one JSON file
with an `await` between read and write, so any concurrent setting writes
can lose updates.
-
[FE-1507](https://linear.app/comfyorg/issue/FE-1507/ops-run-typecheckbrowser-unconditionally-so-browser-tests-type-errors)
— make `typecheck:browser` unconditional in CI.

## Screenshots

![Before: after a reload Navigation Mode reads Custom while Left Mouse
Click Behavior is Select and Mouse Wheel Scroll is Panning - the
Standard
preset](https://pub-0c501c1cf38f477fa8efc67068523b33.r2.dev/sessions/b4bf4aad7065f79965deac7b000dd8c53f46c9c4ed7f6af6c7e44d714af85512/pr-images/1785882191788-d210acf4-ad52-4822-8a25-a4fe69526fc1.png)

![After: Navigation Mode retains Standard (New) across a reload, with
Select and Panning consistent with
it](https://pub-0c501c1cf38f477fa8efc67068523b33.r2.dev/sessions/b4bf4aad7065f79965deac7b000dd8c53f46c9c4ed7f6af6c7e44d714af85512/pr-images/1785882192021-8b769799-9112-48bb-a216-51e6d923faaf.png)

---------

Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com>

Author: DrJKL

browser_tests/fixtures/helpers/SettingsHelper.ts

@@ -20,4 +20,18 @@ export class SettingsHelper {
       return await window.app!.extensionManager.setting.get(id)
     }, settingId)) as T
   }
+
+  /**
+   * Reads the value the server holds, which {@link getSetting} cannot: that
+   * reports the in-memory store, where a value may have been derived at load
+   * rather than persisted. Returns `undefined` for a setting never written.
+   */
+  async getPersistedSetting<T = unknown>(
+    settingId: string
+  ): Promise<T | undefined> {
+    return (await this.page.evaluate(async (id) => {
+      const persisted = await window.app!.api.getSettings()
+      return persisted[id as keyof typeof persisted]
+    }, settingId)) as T | undefined
+  }
 }

browser_tests/tests/canvasSettings.spec.ts

@@ -188,6 +188,138 @@ test.describe('Canvas settings', { tag: '@canvas' }, () => {
     })
   })
 
+  test.describe('Comfy.Canvas.NavigationMode', () => {
+    test('picking a preset never persists custom', async ({ comfyPage }) => {
+      const modeWrites: unknown[] = []
+      comfyPage.page.on('request', (request) => {
+        if (
+          request.method() === 'POST' &&
+          request.url().endsWith('/api/settings/Comfy.Canvas.NavigationMode')
+        ) {
+          modeWrites.push(request.postDataJSON())
+        }
+      })
+
+      await comfyPage.settings.setSetting(
+        'Comfy.Canvas.NavigationMode',
+        'standard'
+      )
+      await comfyPage.workflow.reloadAndWaitForApp()
+
+      // A preset also writes the two overrides it implies; those writes must
+      // not read back as the user hand-picking an override.
+      expect(modeWrites).toContain('standard')
+      expect(modeWrites).not.toContain('custom')
+      expect(
+        await comfyPage.settings.getSetting('Comfy.Canvas.NavigationMode')
+      ).toBe('standard')
+    })
+
+    test('picking custom leaves the overrides untouched', async ({
+      comfyPage
+    }) => {
+      // Arrive on the standard pair first, so both overrides differ from their
+      // defaults and a handler that rewrote them would be caught.
+      await comfyPage.settings.setSetting(
+        'Comfy.Canvas.NavigationMode',
+        'standard'
+      )
+
+      await comfyPage.settings.setSetting(
+        'Comfy.Canvas.NavigationMode',
+        'custom'
+      )
+      await comfyPage.workflow.reloadAndWaitForApp()
+
+      expect(
+        await comfyPage.settings.getSetting('Comfy.Canvas.NavigationMode')
+      ).toBe('custom')
+      expect(
+        await comfyPage.settings.getSetting(
+          'Comfy.Canvas.LeftMouseClickBehavior'
+        )
+      ).toBe('select')
+      expect(
+        await comfyPage.settings.getSetting('Comfy.Canvas.MouseWheelScroll')
+      ).toBe('panning')
+    })
+
+    // A mode stored before the overrides shipped in 1.27.4 is the only value on
+    // record, so they load as their defaults — which describe a different mode.
+    test.describe('stored without the override settings', () => {
+      test.use({
+        initialSettings: { 'Comfy.Canvas.NavigationMode': 'standard' }
+      })
+
+      test('keeps the stored preset through load', async ({ comfyPage }) => {
+        expect(
+          await comfyPage.settings.getSetting('Comfy.Canvas.NavigationMode')
+        ).toBe('standard')
+      })
+
+      // Reads the server, not the store: the migration is idempotent, so an
+      // in-memory read passes whether or not the write ever landed.
+      test('persists the stored preset to the overrides', async ({
+        comfyPage
+      }) => {
+        expect(
+          await comfyPage.settings.getPersistedSetting(
+            'Comfy.Canvas.LeftMouseClickBehavior'
+          )
+        ).toBe('select')
+        expect(
+          await comfyPage.settings.getPersistedSetting(
+            'Comfy.Canvas.MouseWheelScroll'
+          )
+        ).toBe('panning')
+      })
+    })
+
+    // Every profile that already loaded a 1.27.4+ build has the mode demoted to
+    // 'custom' with the overrides never written. The original choice is
+    // unrecoverable, so this pins the no-op as deliberate.
+    test.describe('already demoted to custom', () => {
+      test.use({
+        initialSettings: { 'Comfy.Canvas.NavigationMode': 'custom' }
+      })
+
+      test('is left as custom', async ({ comfyPage }) => {
+        expect(
+          await comfyPage.settings.getSetting('Comfy.Canvas.NavigationMode')
+        ).toBe('custom')
+      })
+    })
+
+    test.describe('stored with only one override', () => {
+      test.use({
+        initialSettings: {
+          'Comfy.Canvas.NavigationMode': 'standard',
+          'Comfy.Canvas.MouseWheelScroll': 'zoom'
+        }
+      })
+
+      test('fills the gap without overwriting the stored override', async ({
+        comfyPage
+      }) => {
+        expect(
+          await comfyPage.settings.getSetting(
+            'Comfy.Canvas.LeftMouseClickBehavior'
+          )
+        ).toBe('select')
+        expect(
+          await comfyPage.settings.getSetting('Comfy.Canvas.MouseWheelScroll')
+        ).toBe('zoom')
+
+        // select + zoom is no preset, so demoting the mode is correct here.
+        // Overwriting the stored 'zoom' to match the mode instead would
+        // discard a real preference, which is the bug this all started as.
+        expect(
+          await comfyPage.settings.getSetting('Comfy.Canvas.NavigationMode')
+        ).toBe('custom')
+      })
+    })
+  })
+
   test.describe('Comfy.Canvas.LeftMouseClickBehavior', () => {
     test('override to panning makes empty left-drag pan the canvas', async ({
       comfyPage

src/platform/settings/constants/canvasNavigation.test.ts

@@ -0,0 +1,49 @@
+import { describe, expect, it } from 'vitest'
+
+import { CANVAS_NAVIGATION_PRESETS } from '@/platform/settings/constants/canvasNavigation'
+import { CORE_SETTINGS } from '@/platform/settings/constants/coreSettings'
+import type { SettingParams } from '@/platform/settings/types'
+
+const NAV = 'Comfy.Canvas.NavigationMode'
+const LEFT = 'Comfy.Canvas.LeftMouseClickBehavior'
+const WHEEL = 'Comfy.Canvas.MouseWheelScroll'
+
+const settingById = (id: string) => CORE_SETTINGS.find((s) => s.id === id)
+
+const resolveDefaultValue = (setting: SettingParams | undefined): unknown => {
+  const { defaultValue } = setting ?? {}
+  return typeof defaultValue === 'function'
+    ? (defaultValue as () => unknown)()
+    : defaultValue
+}
+
+const presetForMode = (mode: unknown) =>
+  typeof mode === 'string' ? CANVAS_NAVIGATION_PRESETS[mode] : undefined
+
+const overrideDefaults = () => ({
+  [LEFT]: resolveDefaultValue(settingById(LEFT)),
+  [WHEEL]: resolveDefaultValue(settingById(WHEEL))
+})
+
+describe('CANVAS_NAVIGATION_PRESETS', () => {
+  /**
+   * The override defaults have to describe whichever Navigation Mode a fresh
+   * profile resolves to. If they disagree, that profile loads with a mode no
+   * preset matches and the override handlers demote it to 'custom' on first
+   * load — the bug this pairing exists to prevent. Asserted as a relationship
+   * rather than against fixed values so changing a default is what trips it.
+   */
+  it('agrees with the default Navigation Mode', () => {
+    const defaultMode = resolveDefaultValue(settingById(NAV))
+
+    expect(presetForMode(defaultMode)).toEqual(overrideDefaults())
+  })
+
+  it('agrees with every install-versioned Navigation Mode default', () => {
+    const versioned = settingById(NAV)?.defaultsByInstallVersion ?? {}
+
+    for (const mode of Object.values(versioned)) {
+      expect(presetForMode(mode)).toEqual(overrideDefaults())
+    }
+  })
+})

src/platform/settings/constants/canvasNavigation.ts

@@ -0,0 +1,22 @@
+import type { Settings } from '@/schemas/apiSchema'
+
+/**
+ * The Left Mouse Click Behavior and Mouse Wheel Scroll values each Navigation
+ * Mode preset stands for.
+ *
+ * `custom` is deliberately absent: it means "the overrides are whatever the
+ * user set", so there is no pair to apply or to restore from it.
+ */
+export const CANVAS_NAVIGATION_PRESETS: Record<
+  string,
+  Partial<Settings> | undefined
+> = {
+  standard: {
+    'Comfy.Canvas.LeftMouseClickBehavior': 'select',
+    'Comfy.Canvas.MouseWheelScroll': 'panning'
+  },
+  legacy: {
+    'Comfy.Canvas.LeftMouseClickBehavior': 'panning',
+    'Comfy.Canvas.MouseWheelScroll': 'zoom'
+  }
+}

src/platform/settings/constants/coreSettings.ts

@@ -5,6 +5,7 @@ import {
 } from '@/locales/localeConfig'
 import { isCloud, isDesktop, isNightly } from '@/platform/distribution/types'
 import { TOUR_SEEN_SETTING } from '@/platform/onboarding/onboardingTours'
+import { CANVAS_NAVIGATION_PRESETS } from '@/platform/settings/constants/canvasNavigation'
 import { useSettingStore } from '@/platform/settings/settingStore'
 import type { SettingParams } from '@/platform/settings/types'
 import type { ColorPalettes } from '@/schemas/colorPaletteSchema'
@@ -188,22 +189,11 @@ export const CORE_SETTINGS: SettingParams[] = [
       '1.25.0': 'legacy'
     },
     onChange: async (val: unknown, old?: unknown) => {
-      const newValue = val as string
-      const oldValue = old as string | undefined
-      if (!oldValue) return
-      const settingStore = useSettingStore()
+      if (!old || typeof val !== 'string') return
+      const preset = CANVAS_NAVIGATION_PRESETS[val]
+      if (!preset) return
 
-      if (newValue === 'standard') {
-        await settingStore.setMany({
-          'Comfy.Canvas.LeftMouseClickBehavior': 'select',
-          'Comfy.Canvas.MouseWheelScroll': 'panning'
-        })
-      } else if (newValue === 'legacy') {
-        await settingStore.setMany({
-          'Comfy.Canvas.LeftMouseClickBehavior': 'panning',
-          'Comfy.Canvas.MouseWheelScroll': 'zoom'
-        })
-      }
+      await useSettingStore().setMany(preset)
     }
   },
   {