From eb230837ccff15760bfcd26d0dc2c444c9ceb5ed Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Mon, 15 Jun 2026 23:14:32 -0700 Subject: [PATCH 1/3] fix(updater): don't auto-install Desktop updates when the setting is off (#1104) The "Automatically install Desktop updates" setting (autoInstallUpdates) gated nothing on the install side: a downloaded update still installed via the Windows startup-install path or electron-updater's install-on-quit, regardless of the setting. (ToDesktop's checkForUpdates always downloads, so the download itself stays - only the install is now gated.) - evaluateStartupInstall(): skip with new `auto_install_disabled` reason when auto-install is off, so a staged update is never applied at startup. - syncInstallOnQuitPolicy(): single source of truth for autoInstallOnAppQuit - off when the startup-install path owns the install OR auto-install is disabled; armed only on non-Windows with auto-install on. Used by register() and re-applied on the autoInstallUpdates toggle so it takes effect without a restart. With the setting off, a downloaded update now waits for an explicit "Desktop Update Ready" pill click (installUpdate) instead of installing automatically on next launch or on close. Amp-Thread-ID: https://ampcode.com/threads/T-019ece99-2c11-75ca-bcdf-3ad07bde982b Co-authored-by: Amp --- src/main/lib/updater.test.ts | 52 ++++++++++++++++++++++++ src/main/lib/updater.ts | 77 +++++++++++++++++++++++++----------- 2 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/main/lib/updater.test.ts b/src/main/lib/updater.test.ts index 8e946c3c..4cb33201 100644 --- a/src/main/lib/updater.test.ts +++ b/src/main/lib/updater.test.ts @@ -421,6 +421,58 @@ describe('startup update install + session-end guard (issue #1065)', () => { expect(fakeUpdater.restartAndInstall).not.toHaveBeenCalled() }) + // Issue #1104 — auto-install off: the update still downloads, but it must + // never install without an explicit pill click (no startup install, no + // install-on-quit). + it('register() disables install-on-quit when auto-install is off (opted out of startup install)', async () => { + settingsStore['installUpdatesOnStartup'] = false + settingsStore['autoInstallUpdates'] = false + const updater = await import('./updater') + updater.register() + // Without the #1104 gate this would stay armed (the opt-out path) and a + // staged update would install on the next quit. + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + }) + + it('startup install is inert when auto-install is off, even with a staged update on Windows', async () => { + settingsStore['autoInstallUpdates'] = false + settingsStore['pendingDownloadedUpdateVersion'] = '1.0.1' + readyVersion = '1.0.1' + const updater = await import('./updater') + updater.register() + expect(updater.hasPendingStartupUpdate()).toBe(false) + expect(await updater.applyPendingUpdateOnStartup()).toBe(false) + expect(fakeUpdater.restartAndInstall).not.toHaveBeenCalled() + // Intentional user choice, not an anomaly — no canary telemetry. + expect(findEmitCalls('comfy.desktop.app_update.startup_install_skipped')).toHaveLength(0) + }) + + it('installUpdate() (pill-confirm path) still installs when auto-install is off', async () => { + settingsStore['autoInstallUpdates'] = false + const updater = await import('./updater') + updater.register() + updater.installUpdate() + // The manual path is the whole point of auto-install off — it must work. + expect(fakeUpdater.restartAndInstall).toHaveBeenCalled() + }) + + it('toggling auto-install re-arms / disarms install-on-quit without a restart', async () => { + // Opt out of startup install so install-on-quit is the live gate. + settingsStore['installUpdatesOnStartup'] = false + const updater = await import('./updater') + updater.register() + // Auto-install defaults on → install-on-quit armed. + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) + + settingsStore['autoInstallUpdates'] = false + updater.notifyAutoUpdateChanged() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + + settingsStore['autoInstallUpdates'] = true + updater.notifyAutoUpdateChanged() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) + }) + it('installUpdate() is a no-op while the OS session is ending', async () => { sessionEnding = true const updater = await import('./updater') diff --git a/src/main/lib/updater.ts b/src/main/lib/updater.ts index f0ca3188..3919e810 100644 --- a/src/main/lib/updater.ts +++ b/src/main/lib/updater.ts @@ -103,14 +103,18 @@ function isAutoInstallEnabled(): boolean { } /** - * Re-broadcast the cached `_appUpdateState` with a refreshed - * `autoUpdate` flag. Settings handler calls this when the user toggles - * the autoUpdate preference so a pending `'ready'` state immediately - * starts reading as auto-on / auto-off (drives the title-bar pill copy - * and the click-modal flow without having to wait for the next - * update-check broadcast). No-op when there's no cached state. + * Settings handler calls this when the user toggles the auto-install + * preference. Two effects: + * 1. Re-applies the install-on-quit policy (Issue #1104) so flipping the + * setting arms/disarms install-on-quit immediately, without a restart. + * This runs regardless of whether an update is cached. + * 2. Re-broadcasts the cached `_appUpdateState` with a refreshed `autoUpdate` + * flag so a pending `'ready'` state immediately reads as auto-on / auto-off + * (drives the title-bar pill copy and the click-modal flow without waiting + * for the next update-check broadcast). No-op when there's no cached state. */ export function notifyAutoUpdateChanged(): void { + syncInstallOnQuitPolicy() if (_appUpdateState.kind === null) return const refreshed = isAutoInstallEnabled() if (_appUpdateState.autoUpdate === refreshed) return @@ -192,6 +196,26 @@ export function suppressInstallOnQuit(): void { } catch {} } +/** + * Reconcile electron-updater's install-on-quit flag with current settings. + * Install-on-quit is disabled when either: + * - the startup-install path owns the install (Windows default — the staged + * update applies on the next launch, not on quit), or + * - the user disabled auto-install (Issue #1104 — a staged update must wait + * for an explicit "Desktop Update Ready" pill click rather than installing + * on the next quit/close). + * It stays enabled only when neither holds (non-Windows with auto-install on), + * where a normal quit still applies a staged update. Re-applied on the + * `autoInstallUpdates` toggle so flipping the setting takes effect without a + * restart. The download itself is unaffected — updates still download in the + * background; only the install is gated. + */ +export function syncInstallOnQuitPolicy(): void { + try { + electronAutoUpdater.autoInstallOnAppQuit = !isStartupInstallEnabled() && isAutoInstallEnabled() + } catch {} +} + function asRecord(value: unknown): Record | null { return typeof value === 'object' && value !== null ? (value as Record) : null } @@ -584,7 +608,14 @@ type StartupInstallDecision = | { attempt: true; version: string } | { attempt: false - reason: 'disabled' | 'e2e' | 'system_managed' | 'session_ending' | 'no_pending' | 'loop_breaker' + reason: + | 'disabled' + | 'auto_install_disabled' + | 'e2e' + | 'system_managed' + | 'session_ending' + | 'no_pending' + | 'loop_breaker' } /** @@ -592,14 +623,18 @@ type StartupInstallDecision = * synchronous (reads only persisted markers + environment). * * Returns a skip for: the startup-install gate being off (non-Windows, or the - * `installUpdatesOnStartup` opt-out — installs still happen on quit), E2E runs, - * system-package-managed installs (apt/dnf own + * `installUpdatesOnStartup` opt-out — installs still happen on quit), auto-install + * being disabled (Issue #1104 — staged update waits for an explicit pill click), + * E2E runs, system-package-managed installs (apt/dnf own * the update), an OS session that's already ending, no staged download (or one * that's already the running version), and the loop-breaker case (we already * auto-attempted this exact version and are still on the old one). */ function evaluateStartupInstall(): StartupInstallDecision { if (!isStartupInstallEnabled()) return { attempt: false, reason: 'disabled' } + // Issue #1104 — with auto-install off, a staged update must wait for an + // explicit pill click; never apply it automatically at startup. + if (!isAutoInstallEnabled()) return { attempt: false, reason: 'auto_install_disabled' } if (process.env['E2E'] === '1') return { attempt: false, reason: 'e2e' } if (isSystemPackageInstall()) return { attempt: false, reason: 'system_managed' } if (isSessionEnding()) return { attempt: false, reason: 'session_ending' } @@ -736,20 +771,16 @@ export async function applyPendingUpdateOnStartup(splashShownAt?: number): Promi export function register(): void { bindUpdaterEvents() - // Startup install (the Windows default): disable electron-updater's - // install-on-quit entirely up front — the staged update applies on the next - // launch (`applyPendingUpdateOnStartup`) instead of on quit, which is what a - // Windows shutdown can kill mid-write (the "reinstall on every shutdown" - // corruption loop). `electronAutoUpdater` is the same singleton the ToDesktop - // runtime drives, so this affects the real updater. - // - // Opted out (non-Windows, or `installUpdatesOnStartup` set to false): keep - // install-on-quit armed. A normal quit still installs a staged update; the - // `session-end` guard (`suppressInstallOnQuit`) flips `autoInstallOnAppQuit` - // off only when the OS is shutting down. - if (isStartupInstallEnabled()) { - suppressInstallOnQuit() - } + // Reconcile install-on-quit with current settings (see + // `syncInstallOnQuitPolicy`). Disabled when the startup-install path owns the + // install (the Windows default — the staged update applies on the next launch + // instead of on quit, avoiding the Windows-shutdown mid-write corruption + // loop) or when auto-install is off (Issue #1104 — wait for an explicit pill + // click). Otherwise (non-Windows with auto-install on) install-on-quit stays + // armed; the `session-end` guard (`suppressInstallOnQuit`) still flips it off + // only while the OS is shutting down. `electronAutoUpdater` is the same + // singleton the ToDesktop runtime drives, so this affects the real updater. + syncInstallOnQuitPolicy() ipcMain.handle('check-for-update', async () => { try { From 15d63e0d49d0a1bf1615477961e2940d6b80839e Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Mon, 15 Jun 2026 23:25:40 -0700 Subject: [PATCH 2/3] fix(updater): latch session-end install-on-quit suppression (#1104 review) Code review caught a race: syncInstallOnQuitPolicy() (now re-run on the autoInstallUpdates toggle) could re-arm autoInstallOnAppQuit after the OS session-end guard suppressInstallOnQuit() had disabled it, reintroducing the mid-write install corruption during shutdown. Latch the suppression for the life of the process: suppressInstallOnQuit() sets a flag that syncInstallOnQuitPolicy() honors, so a settings toggle mid-shutdown can never re-arm install-on-quit. Tests: never re-arms after session-end suppression; macOS disables install-on-quit when auto-install is off. Amp-Thread-ID: https://ampcode.com/threads/T-019ece99-2c11-75ca-bcdf-3ad07bde982b Co-authored-by: Amp --- src/main/lib/updater.test.ts | 25 +++++++++++++++++++++++++ src/main/lib/updater.ts | 20 +++++++++++++++----- 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/src/main/lib/updater.test.ts b/src/main/lib/updater.test.ts index 4cb33201..00e5faaf 100644 --- a/src/main/lib/updater.test.ts +++ b/src/main/lib/updater.test.ts @@ -473,6 +473,31 @@ describe('startup update install + session-end guard (issue #1065)', () => { expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) }) + it('notifyAutoUpdateChanged() never re-arms install-on-quit after session-end suppression', async () => { + // Opt out of startup install so install-on-quit would otherwise be armed. + settingsStore['installUpdatesOnStartup'] = false + const updater = await import('./updater') + updater.register() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(true) + + // OS session ends → guard suppresses install-on-quit for the session. + updater.suppressInstallOnQuit() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + + // A settings toggle mid-shutdown must NOT re-arm it (mid-write corruption guard). + settingsStore['autoInstallUpdates'] = true + updater.notifyAutoUpdateChanged() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + }) + + it('register() disables install-on-quit on macOS when auto-install is off', async () => { + Object.defineProperty(process, 'platform', { value: 'darwin', configurable: true }) + settingsStore['autoInstallUpdates'] = false + const updater = await import('./updater') + updater.register() + expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) + }) + it('installUpdate() is a no-op while the OS session is ending', async () => { sessionEnding = true const updater = await import('./updater') diff --git a/src/main/lib/updater.ts b/src/main/lib/updater.ts index 3919e810..8dca486c 100644 --- a/src/main/lib/updater.ts +++ b/src/main/lib/updater.ts @@ -181,16 +181,23 @@ function isInstallerUIEnabled(): boolean { return isWindowsOptOutGate('showInstallerUI') } +/** Set once the OS session-end guard suppresses install-on-quit; never cleared + * for the life of the process. Latches the suppression so a later settings + * toggle (which re-runs `syncInstallOnQuitPolicy`) can't re-arm install-on-quit + * mid-shutdown and reintroduce the mid-write corruption the guard prevents. */ +let _installOnQuitSuppressedForSession = false + /** * Disable electron-updater's install-on-quit. Called when the OS signals the * session is ending (Windows shutdown / restart / logoff) so the quit handler * electron-updater registers after a download won't spawn the installer while * the OS tears everything down — that mid-write kill is the corruption mode * behind the "reinstall on every shutdown" loop. The quit handler re-reads this - * flag at quit time, so flipping it here is enough. Safe to call in any mode (a - * no-op when the startup-install path already disabled it at register time). + * flag at quit time, so flipping it here is enough. Latches for the session so + * `syncInstallOnQuitPolicy` can't undo it. Safe to call in any mode. */ export function suppressInstallOnQuit(): void { + _installOnQuitSuppressedForSession = true try { electronAutoUpdater.autoInstallOnAppQuit = false } catch {} @@ -198,13 +205,15 @@ export function suppressInstallOnQuit(): void { /** * Reconcile electron-updater's install-on-quit flag with current settings. - * Install-on-quit is disabled when either: + * Install-on-quit is disabled when any of: + * - the OS session-end guard already suppressed it for this session + * (`suppressInstallOnQuit` — never re-arm mid-shutdown), or * - the startup-install path owns the install (Windows default — the staged * update applies on the next launch, not on quit), or * - the user disabled auto-install (Issue #1104 — a staged update must wait * for an explicit "Desktop Update Ready" pill click rather than installing * on the next quit/close). - * It stays enabled only when neither holds (non-Windows with auto-install on), + * It stays enabled only when none hold (non-Windows with auto-install on), * where a normal quit still applies a staged update. Re-applied on the * `autoInstallUpdates` toggle so flipping the setting takes effect without a * restart. The download itself is unaffected — updates still download in the @@ -212,7 +221,8 @@ export function suppressInstallOnQuit(): void { */ export function syncInstallOnQuitPolicy(): void { try { - electronAutoUpdater.autoInstallOnAppQuit = !isStartupInstallEnabled() && isAutoInstallEnabled() + electronAutoUpdater.autoInstallOnAppQuit = + !_installOnQuitSuppressedForSession && !isStartupInstallEnabled() && isAutoInstallEnabled() } catch {} } From 9c79df5dbc8def8a1ca3938eca31c62c1b12ef5c Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Mon, 15 Jun 2026 23:43:02 -0700 Subject: [PATCH 3/3] test(updater): exercise a real false->true flip in the session-end latch test (#1104 review) Amp-Thread-ID: https://ampcode.com/threads/T-019ece99-2c11-75ca-bcdf-3ad07bde982b Co-authored-by: Amp --- src/main/lib/updater.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/main/lib/updater.test.ts b/src/main/lib/updater.test.ts index 00e5faaf..73340723 100644 --- a/src/main/lib/updater.test.ts +++ b/src/main/lib/updater.test.ts @@ -484,7 +484,9 @@ describe('startup update install + session-end guard (issue #1065)', () => { updater.suppressInstallOnQuit() expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false) - // A settings toggle mid-shutdown must NOT re-arm it (mid-write corruption guard). + // Mid-shutdown setting flips must never re-arm install-on-quit. + settingsStore['autoInstallUpdates'] = false + updater.notifyAutoUpdateChanged() settingsStore['autoInstallUpdates'] = true updater.notifyAutoUpdateChanged() expect(electronUpdaterMock.autoInstallOnAppQuit).toBe(false)