From 27d566a96090a4b8a2003165ed72025f4c700ddd Mon Sep 17 00:00:00 2001 From: 4gray Date: Sun, 9 Aug 2026 14:48:50 +0200 Subject: [PATCH] fix(settings): serialize Stay's cancellation against a racing close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renderer sent WINDOW:CANCEL_CLOSE fire-and-forget, so a close clicked immediately after choosing Stay on an intercepted quit could reach the main guard before the cancellation and be answered against the stale quit intent — Save would then quit the whole app instead of closing the window. Stay now awaits the cancellation acknowledgment, and a close request arriving inside that window is queued and re-asked once the cancel has landed, so the fresh dialog can only ever complete the fresh intent. Co-Authored-By: Claude Fable 5 --- .../settings-unload-guard.service.spec.ts | 34 +++++++++++++ .../settings/settings-unload-guard.service.ts | 48 +++++++++++++++---- 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts index 3cfa6d3bb..28bc0e1ae 100644 --- a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts +++ b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts @@ -282,6 +282,40 @@ describe('SettingsUnloadGuardService', () => { expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); }); + it('re-asks a close that raced the Stay cancellation', async () => { + // Stay's cancelWindowClose is awaited; a close arriving while + // that acknowledgment is in flight must be re-asked afterwards + // — never answered against the stale intent the cancel clears, + // and never silently swallowed. + let resolveCancel: (() => void) | null = null; + electronStub.cancelWindowClose.mockImplementation( + () => + new Promise((resolve) => { + resolveCancel = resolve; + }) + ); + host.confirmClose + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(); + await flushAsyncWork(); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + // Second close while the cancel acknowledgment is pending. + closeRequestCallback?.(); + await flushAsyncWork(); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + resolveCancel?.(); + await flushAsyncWork(); + + expect(host.confirmClose).toHaveBeenCalledTimes(2); + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + it('ignores repeated close requests while a dialog is open', async () => { let resolveConfirm: ((value: boolean) => void) | null = null; host.confirmClose.mockImplementation( diff --git a/apps/web/src/app/settings/settings-unload-guard.service.ts b/apps/web/src/app/settings/settings-unload-guard.service.ts index b90da2919..f9f681022 100644 --- a/apps/web/src/app/settings/settings-unload-guard.service.ts +++ b/apps/web/src/app/settings/settings-unload-guard.service.ts @@ -52,6 +52,10 @@ export class SettingsUnloadGuardService implements OnDestroy { * the user's most recent ask must be the one that completes. */ private activeIntent: 'close' | 'reload' | null = null; + /** True while a Stay's cancelWindowClose acknowledgment is in flight. */ + private cancelling = false; + /** A close requested during that window, re-asked once the cancel lands. */ + private queuedCloseRequest = false; /** Last guard state mirrored to the main process. */ private guardArmed = false; /** True while an updater-driven app quit must pass unchallenged. */ @@ -126,6 +130,7 @@ export class SettingsUnloadGuardService implements OnDestroy { this.unregisterFromInstallService?.(); this.unregisterFromInstallService = null; this.suspended = false; + this.queuedCloseRequest = false; this.syncCloseGuard(false); this.host = null; } @@ -164,17 +169,28 @@ export class SettingsUnloadGuardService implements OnDestroy { } if (this.confirmationPending) { - // A close outranks a reload, never the other way around: the - // open dialog stays, but Save/Discard must complete the close - // the user just asked for, not the earlier reload. if (intent === 'close') { - this.activeIntent = 'close'; + if (this.cancelling) { + // The dialog already resolved and Stay's cancellation is + // still in flight. This is a fresh user action — queue + // it and re-ask once the cancel has landed, so the new + // dialog can never answer against the stale intent the + // cancel is about to clear. + this.queuedCloseRequest = true; + } else { + // A close outranks a reload, never the other way + // around: the open dialog stays, but Save/Discard must + // complete the close the user just asked for, not the + // earlier reload. + this.activeIntent = 'close'; + } } return; } this.confirmationPending = true; this.activeIntent = intent; + let reAskClose = false; try { const proceed = await host.confirmClose(); @@ -186,13 +202,21 @@ export class SettingsUnloadGuardService implements OnDestroy { if (finalIntent === 'close') { // Staying must clear the intent the main process // remembered, or a later close attempt would replay a - // stale quit. - void window.electron?.cancelWindowClose?.(); - } - return; - } + // stale quit — and the clearing must be AWAITED, or a + // close racing this cancellation could still consume + // the stale intent. + this.cancelling = true; - if (finalIntent === 'close') { + try { + await window.electron?.cancelWindowClose?.(); + } finally { + this.cancelling = false; + } + + reAskClose = this.queuedCloseRequest; + this.queuedCloseRequest = false; + } + } else if (finalIntent === 'close') { await window.electron?.confirmWindowClose?.(); } else { this.reloadPage(); @@ -201,6 +225,10 @@ export class SettingsUnloadGuardService implements OnDestroy { this.confirmationPending = false; this.activeIntent = null; } + + if (reAskClose) { + void this.handleCloseRequest('close'); + } } private syncCloseGuard(active: boolean): void {