From 45be13df13dfe8d57582d722b6a8e3e416129232 Mon Sep 17 00:00:00 2001 From: Jason Qin Date: Thu, 29 Oct 2026 16:42:52 -0700 Subject: [PATCH 1/5] feat(desktop): add an Off option to the quit shortcut setting On Windows and Linux, Off lets Ctrl+Q through to the renderer instead of quitting, so mod+q can be bound in keybindings.json. macOS keeps Cmd+Q. The Linux File > Quit item no longer registers its role accelerator, which would otherwise quit when the shortcut is passed through. --- .../src/window/DesktopApplicationMenu.test.ts | 21 +++++++ .../src/window/DesktopApplicationMenu.ts | 3 +- apps/desktop/src/window/QuitHold.test.ts | 60 +++++++++++++++++-- apps/desktop/src/window/QuitHold.ts | 29 ++++++++- .../components/settings/SettingsPanels.tsx | 20 ++++++- .../src/components/settings/settingsSearch.ts | 2 +- docs/user/keybindings.md | 3 + packages/contracts/src/ipc.ts | 2 +- packages/contracts/src/settings.ts | 2 +- 9 files changed, 130 insertions(+), 12 deletions(-) diff --git a/apps/desktop/src/window/DesktopApplicationMenu.test.ts b/apps/desktop/src/window/DesktopApplicationMenu.test.ts index 9d8efd056665b..df34ec883068b 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.test.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.test.ts @@ -234,6 +234,27 @@ describe("DesktopApplicationMenu", () => { }), ); + it.effect("leaves Ctrl+Q to the quit shortcut setting on Windows and Linux", () => + Effect.gen(function* () { + for (const platform of ["linux", "win32"] as const) { + const applicationMenuTemplate = + yield* Deferred.make(); + yield* configureMenu(yield* Deferred.make(), applicationMenuTemplate, { + platform, + }); + + const fileMenu = (yield* Deferred.await(applicationMenuTemplate)).find( + (item) => item.label === "File", + ); + if (!Array.isArray(fileMenu?.submenu)) { + throw new Error("Expected File menu submenu to be an array."); + } + const quitItem = fileMenu.submenu.find((item) => item.role === "quit"); + assert.isFalse(quitItem?.registerAccelerator, platform); + } + }), + ); + it.effect("reloads the main window even while a browser page has focus", () => Effect.gen(function* () { const selectedAction = yield* Deferred.make(); diff --git a/apps/desktop/src/window/DesktopApplicationMenu.ts b/apps/desktop/src/window/DesktopApplicationMenu.ts index 07fd8654af6be..cdc25b12b7771 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.ts @@ -203,7 +203,8 @@ export const make = Effect.gen(function* () { // Cmd/Ctrl+W belongs to the renderer's tab close; it must never close the window. environment.platform === "darwin" ? { label: "Close Window", click: (_item, window) => window?.close() } - : { role: "quit" }, + : // Linux's role accelerator would bypass the quit shortcut setting; QuitHold owns Ctrl+Q. + { role: "quit", registerAccelerator: false }, ], }, { diff --git a/apps/desktop/src/window/QuitHold.test.ts b/apps/desktop/src/window/QuitHold.test.ts index bebcd66e61c6e..71157cbe536cb 100644 --- a/apps/desktop/src/window/QuitHold.test.ts +++ b/apps/desktop/src/window/QuitHold.test.ts @@ -121,7 +121,7 @@ describe("makeQuitShortcutHandler", () => { await harness.send(makeInput({ type: "keyUp" })); - resolvers[0]?.(mode); + resolvers[1]?.(mode); await Promise.resolve(); await Promise.resolve(); @@ -139,14 +139,14 @@ describe("makeQuitShortcutHandler", () => { await harness.send(makeInput({ type: "keyUp" })); vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); await harness.send(makeInput({})); - expect(resolvers).toHaveLength(2); + expect(resolvers).toHaveLength(3); - resolvers[0]?.("direct"); + resolvers[1]?.("direct"); await Promise.resolve(); await Promise.resolve(); expect(harness.quit).not.toHaveBeenCalled(); - resolvers[1]?.("hold"); + resolvers[2]?.("hold"); await harness.holdFor(QUIT_HOLD_DURATION_MS + 200); await harness.send(makeInput({ type: "keyUp" })); expect(harness.quit).toHaveBeenCalledTimes(1); @@ -160,4 +160,56 @@ describe("makeQuitShortcutHandler", () => { expect(harness.quit).not.toHaveBeenCalled(); expect(harness.notifications).toEqual([DOUBLE_CLICK_DOWN, UP, DOUBLE_CLICK_DOWN]); }); + + it.each(["win32", "linux"] as const)( + "passes Ctrl+Q through without quitting when Off on %s", + async (platform) => { + const harness = makeHarness({ mode: "off", platform }); + await Promise.resolve(); + const ctrlQ = { meta: false, control: true }; + await harness.send(makeInput(ctrlQ)); + await harness.holdFor(QUIT_HOLD_DURATION_MS + 200, ctrlQ); + await harness.send(makeInput({ ...ctrlQ, type: "keyUp" })); + await harness.send(makeInput(ctrlQ)); + + expect(harness.preventDefault).not.toHaveBeenCalled(); + expect(harness.quit).not.toHaveBeenCalled(); + expect(harness.notifications).toEqual([]); + }, + ); + + it("keeps Cmd+Q from reaching the app menu when Off on macOS", async () => { + const harness = makeHarness({ mode: "off", platform: "darwin" }); + await Promise.resolve(); + await harness.send(makeInput({})); + await harness.send(makeInput({})); + + expect(harness.preventDefault).toHaveBeenCalled(); + expect(harness.quit).not.toHaveBeenCalled(); + expect(harness.notifications).toEqual([]); + }); + + it("follows a mode change on the next press", async () => { + let mode: QuitConfirmationMode = "hold"; + const ctrlQ = { meta: false, control: true }; + const harness = makeHarness({ platform: "win32", getMode: () => Promise.resolve(mode) }); + await Promise.resolve(); + + mode = "off"; + await harness.send(makeInput(ctrlQ)); + await harness.send(makeInput({ ...ctrlQ, type: "keyUp" })); + expect(harness.preventDefault).toHaveBeenCalledTimes(1); + expect(harness.notifications).toEqual([]); + + mode = "direct"; + vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); + await harness.send(makeInput(ctrlQ)); + await harness.send(makeInput({ ...ctrlQ, type: "keyUp" })); + expect(harness.preventDefault).toHaveBeenCalledTimes(1); + expect(harness.quit).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); + await harness.send(makeInput(ctrlQ)); + expect(harness.quit).toHaveBeenCalledTimes(1); + }); }); diff --git a/apps/desktop/src/window/QuitHold.ts b/apps/desktop/src/window/QuitHold.ts index f7db33f17ed5e..f68f5e224fb4a 100644 --- a/apps/desktop/src/window/QuitHold.ts +++ b/apps/desktop/src/window/QuitHold.ts @@ -46,6 +46,18 @@ export function makeQuitShortcutHandler( let repeatCadenceMs = 0; // A key release must not invalidate a pending direct-mode read; only superseding/cancelled presses do. let generation = 0; + // preventDefault must be decided synchronously, so Off uses the last mode read. + let knownMode: QuitConfirmationMode | undefined; + let modeRead = 0; + + const readMode = () => { + const read = ++modeRead; + return options.getMode().then((resolvedMode) => { + if (read === modeRead) knownMode = resolvedMode; + return resolvedMode; + }); + }; + readMode().catch(() => undefined); const clearWatchdog = () => { if (watchdog !== undefined) { @@ -129,6 +141,17 @@ export function makeQuitShortcutHandler( return; } + if (knownMode === "off") { + // Passing Cmd+Q through would reach the macOS app menu's Quit accelerator. + if (options.platform === "darwin") event.preventDefault(); + if (!input.isAutoRepeat) { + lastPressAt = 0; + release(); + readMode().catch(() => undefined); + } + return; + } + event.preventDefault(); if (input.isAutoRepeat) { @@ -155,9 +178,13 @@ export function makeQuitShortcutHandler( const pressGeneration = generation; holding = true; heldSince = now; - void options.getMode().then( + void readMode().then( (resolvedMode) => { if (generation !== pressGeneration) return; + if (resolvedMode === "off") { + release(); + return; + } if (resolvedMode === "direct") { quitNow(); return; diff --git a/apps/web/src/components/settings/SettingsPanels.tsx b/apps/web/src/components/settings/SettingsPanels.tsx index c4e3d27a07fa7..2c2c7a09c0bf1 100644 --- a/apps/web/src/components/settings/SettingsPanels.tsx +++ b/apps/web/src/components/settings/SettingsPanels.tsx @@ -137,6 +137,7 @@ const QUIT_CONFIRMATION_MODE_LABELS: Record = { direct: "Direct", hold: "Hold", "double-click": "Double press", + off: "Off", }; const DEFAULT_DRIVER_KIND = ProviderDriverKind.make("codex"); @@ -766,6 +767,10 @@ export function GeneralSettingsPanel() { const lastEnabledProjectGroupingMode = useRef( readLastEnabledProjectGroupingMode(), ); + const isMac = isMacPlatform(navigator.platform); + const quitConfirmationModes = Object.entries(QUIT_CONFIRMATION_MODE_LABELS).filter( + ([value]) => !isMac || value !== "off", + ); return ( @@ -967,7 +972,11 @@ export function GeneralSettingsPanel() { {isElectron ? ( { - if (value === "direct" || value === "hold" || value === "double-click") { + if ( + value === "direct" || + value === "hold" || + value === "double-click" || + value === "off" + ) { updateSettings({ confirmQuit: value }); } }} @@ -995,7 +1009,7 @@ export function GeneralSettingsPanel() { {QUIT_CONFIRMATION_MODE_LABELS[settings.confirmQuit]} - {Object.entries(QUIT_CONFIRMATION_MODE_LABELS).map(([value, label]) => ( + {quitConfirmationModes.map(([value, label]) => ( {label} diff --git a/apps/web/src/components/settings/settingsSearch.ts b/apps/web/src/components/settings/settingsSearch.ts index dc494caf5344d..01073cb34b79f 100644 --- a/apps/web/src/components/settings/settingsSearch.ts +++ b/apps/web/src/components/settings/settingsSearch.ts @@ -665,7 +665,7 @@ export const SETTINGS_SEARCH_ITEMS = [ id: "quit-confirmation", title: "Quit shortcut", to: "/settings/general", - searchTerms: ["confirmation desktop app exit direct hold double click press twice"], + searchTerms: ["confirmation desktop app exit direct hold double click press twice off disable"], desktopOnly: true, }, { diff --git a/docs/user/keybindings.md b/docs/user/keybindings.md index 83989762c5b07..51addf1d2eef6 100644 --- a/docs/user/keybindings.md +++ b/docs/user/keybindings.md @@ -190,3 +190,6 @@ keyboard repeat; if repeat is disabled, use two presses or the application menu. Change **Settings → General → Confirmations → Quit shortcut** to **Direct** for a single press or **Double press** for two presses only. Choosing **Quit** from the application menu always quits immediately. + +On Windows and Linux, **Off** stops `Ctrl+Q` from quitting, so you can bind +`mod+q` to another command in `keybindings.json`. diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index f460619f11355..a35c02531e491 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -34,7 +34,7 @@ export interface ContextMenuItem { } export type QuitShortcutHintEvent = - | { readonly state: "down"; readonly mode: Exclude } + | { readonly state: "down"; readonly mode: Exclude } | { readonly state: "up" }; export interface ContextMenuItemSchemaType { diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 179e58b7a0b56..32a151d0da688 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -262,7 +262,7 @@ export const NotificationMode = Schema.Literals([ ]); export type NotificationMode = typeof NotificationMode.Type; -export const QuitConfirmationMode = Schema.Literals(["direct", "hold", "double-click"]); +export const QuitConfirmationMode = Schema.Literals(["direct", "hold", "double-click", "off"]); export type QuitConfirmationMode = typeof QuitConfirmationMode.Type; const DEFAULT_QUIT_CONFIRMATION_MODE: QuitConfirmationMode = "hold"; From dc366efa549b248736b7029bddadf648a14c9c24 Mon Sep 17 00:00:00 2001 From: Jason Qin Date: Thu, 29 Oct 2026 17:19:30 -0700 Subject: [PATCH 2/5] fix(desktop): read the quit mode synchronously so Off can't race a double press DesktopClientSettings keeps the latest settings it read or wrote, and the quit handler peeks that instead of caching its own last read. A quick second press can no longer quit while a stale cache disagrees with a just saved Off. --- apps/desktop/src/ipc/methods/preview.test.ts | 1 + .../settings/DesktopClientSettings.test.ts | 3 ++ .../src/settings/DesktopClientSettings.ts | 9 +++- .../src/snapShot/DesktopSnapShot.test.ts | 1 + apps/desktop/src/window/DesktopWindow.ts | 4 ++ apps/desktop/src/window/QuitHold.test.ts | 51 ++++++++++--------- apps/desktop/src/window/QuitHold.ts | 19 ++----- 7 files changed, 48 insertions(+), 40 deletions(-) diff --git a/apps/desktop/src/ipc/methods/preview.test.ts b/apps/desktop/src/ipc/methods/preview.test.ts index 802f2f3fb18b6..890f62a70ed6b 100644 --- a/apps/desktop/src/ipc/methods/preview.test.ts +++ b/apps/desktop/src/ipc/methods/preview.test.ts @@ -61,6 +61,7 @@ describe("preview IPC methods", () => { ), Effect.provideService(DesktopClientSettings.DesktopClientSettings, { get: Effect.succeedNone, + peek: Effect.succeedNone, set: () => Effect.void, }), ), diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index ec5062f9105bf..a7605889b4547 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -153,6 +153,7 @@ describe("DesktopClientSettings", () => { onboardingCompletedAt: "2026-09-05T12:00:00.000Z", }; yield* settings.set(savedSettings); + assert.deepEqual(yield* settings.peek, Option.some(savedSettings)); const savedContents = yield* fileSystem.readFileString(environment.clientSettingsPath); const cause = PlatformError.systemError({ _tag: failure.reason, @@ -179,6 +180,7 @@ describe("DesktopClientSettings", () => { assert.equal(error.operation, "read-file"); assert.equal(error.path, environment.clientSettingsPath); assert.strictEqual(error.cause, cause); + assert.deepEqual(yield* retryableSettings.peek, Option.none()); assert.equal( yield* fileSystem.readFileString(environment.clientSettingsPath), savedContents, @@ -186,6 +188,7 @@ describe("DesktopClientSettings", () => { failRead = false; assert.deepEqual(yield* retryableSettings.get, Option.some(savedSettings)); + assert.deepEqual(yield* retryableSettings.peek, Option.some(savedSettings)); }), ), ); diff --git a/apps/desktop/src/settings/DesktopClientSettings.ts b/apps/desktop/src/settings/DesktopClientSettings.ts index 0836db6678226..7dde6434b45ca 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.ts @@ -66,6 +66,8 @@ export class DesktopClientSettings extends Context.Service< DesktopClientSettings, { readonly get: Effect.Effect, DesktopClientSettingsReadError>; + /** Latest settings read or written, for callers that must decide synchronously. */ + readonly peek: Effect.Effect>; readonly set: ( settings: ClientSettings, ) => Effect.Effect; @@ -189,13 +191,17 @@ export const make = Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; const crypto = yield* Crypto.Crypto; + const latestRef = yield* Ref.make(Option.none()); return DesktopClientSettings.of({ get: readClientSettings(fileSystem, environment.clientSettingsPath).pipe( + Effect.tap((settings) => Ref.set(latestRef, settings)), Effect.withSpan("desktop.clientSettings.get"), ), + peek: Ref.get(latestRef), set: Effect.fn("desktop.clientSettings.set")(function (settings) { - return crypto.randomUUIDv4().pipe( + return Ref.set(latestRef, Option.some(settings)).pipe( + Effect.andThen(crypto.randomUUIDv4()), Effect.map((uuid) => uuid.replace(/-/g, "")), Effect.mapError( (cause) => @@ -228,6 +234,7 @@ export const layerTest = (initialSettings: Option.Option = Optio const settingsRef = yield* Ref.make(initialSettings); return DesktopClientSettings.of({ get: Ref.get(settingsRef), + peek: Ref.get(settingsRef), set: Effect.fn("DesktopClientSettings.set")(function (settings) { return Ref.set(settingsRef, Option.some(settings)); }), diff --git a/apps/desktop/src/snapShot/DesktopSnapShot.test.ts b/apps/desktop/src/snapShot/DesktopSnapShot.test.ts index 8532dee0038a0..8ec97094f90b8 100644 --- a/apps/desktop/src/snapShot/DesktopSnapShot.test.ts +++ b/apps/desktop/src/snapShot/DesktopSnapShot.test.ts @@ -503,6 +503,7 @@ const layerTest = ( DesktopClientSettings.DesktopClientSettings, DesktopClientSettings.DesktopClientSettings.of({ get: settingsGet, + peek: Effect.succeedNone, set: () => Effect.void, }), ), diff --git a/apps/desktop/src/window/DesktopWindow.ts b/apps/desktop/src/window/DesktopWindow.ts index ab5fc8be26a92..e195fb1ad776e 100644 --- a/apps/desktop/src/window/DesktopWindow.ts +++ b/apps/desktop/src/window/DesktopWindow.ts @@ -302,6 +302,7 @@ export const make = Effect.gen(function* () { const context = yield* Effect.context(); const runFork = Effect.runForkWith(context); const runPromise = Effect.runPromiseWith(context); + const runSync = Effect.runSyncWith(context); let flushMainWindowBounds: Effect.Effect = Effect.void; const dismissConnectingSplash = Effect.gen(function* () { @@ -619,6 +620,8 @@ export const make = Effect.gen(function* () { } }); + // Fills clientSettings.peek before the first key press. + runFork(Effect.ignore(clientSettings.get)); const quitShortcutHandler = makeQuitShortcutHandler({ platform: environment.platform, getMode: () => @@ -631,6 +634,7 @@ export const make = Effect.gen(function* () { }), ), ), + peekMode: () => Option.getOrUndefined(runSync(clientSettings.peek))?.confirmQuit, notify: (hint) => { if (!window.isDestroyed()) { window.webContents.send(QUIT_SHORTCUT_CHANNEL, hint); diff --git a/apps/desktop/src/window/QuitHold.test.ts b/apps/desktop/src/window/QuitHold.test.ts index 71157cbe536cb..3830602313794 100644 --- a/apps/desktop/src/window/QuitHold.test.ts +++ b/apps/desktop/src/window/QuitHold.test.ts @@ -28,6 +28,7 @@ function makeHarness(options?: { mode?: QuitConfirmationMode; platform?: NodeJS.Platform; getMode?: () => Promise; + peekMode?: () => QuitConfirmationMode | undefined; }) { const notifications: Array = []; const concealWindow = vi.fn(); @@ -35,6 +36,7 @@ function makeHarness(options?: { const handler = makeQuitShortcutHandler({ platform: options?.platform ?? "darwin", getMode: options?.getMode ?? (() => Promise.resolve(options?.mode ?? "hold")), + peekMode: options?.peekMode ?? (() => undefined), notify: (event) => notifications.push(event), concealWindow, quit, @@ -121,7 +123,7 @@ describe("makeQuitShortcutHandler", () => { await harness.send(makeInput({ type: "keyUp" })); - resolvers[1]?.(mode); + resolvers[0]?.(mode); await Promise.resolve(); await Promise.resolve(); @@ -139,14 +141,14 @@ describe("makeQuitShortcutHandler", () => { await harness.send(makeInput({ type: "keyUp" })); vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); await harness.send(makeInput({})); - expect(resolvers).toHaveLength(3); + expect(resolvers).toHaveLength(2); - resolvers[1]?.("direct"); + resolvers[0]?.("direct"); await Promise.resolve(); await Promise.resolve(); expect(harness.quit).not.toHaveBeenCalled(); - resolvers[2]?.("hold"); + resolvers[1]?.("hold"); await harness.holdFor(QUIT_HOLD_DURATION_MS + 200); await harness.send(makeInput({ type: "keyUp" })); expect(harness.quit).toHaveBeenCalledTimes(1); @@ -164,8 +166,7 @@ describe("makeQuitShortcutHandler", () => { it.each(["win32", "linux"] as const)( "passes Ctrl+Q through without quitting when Off on %s", async (platform) => { - const harness = makeHarness({ mode: "off", platform }); - await Promise.resolve(); + const harness = makeHarness({ mode: "off", peekMode: () => "off", platform }); const ctrlQ = { meta: false, control: true }; await harness.send(makeInput(ctrlQ)); await harness.holdFor(QUIT_HOLD_DURATION_MS + 200, ctrlQ); @@ -179,8 +180,7 @@ describe("makeQuitShortcutHandler", () => { ); it("keeps Cmd+Q from reaching the app menu when Off on macOS", async () => { - const harness = makeHarness({ mode: "off", platform: "darwin" }); - await Promise.resolve(); + const harness = makeHarness({ mode: "off", peekMode: () => "off", platform: "darwin" }); await harness.send(makeInput({})); await harness.send(makeInput({})); @@ -189,27 +189,30 @@ describe("makeQuitShortcutHandler", () => { expect(harness.notifications).toEqual([]); }); - it("follows a mode change on the next press", async () => { - let mode: QuitConfirmationMode = "hold"; + it("does not quit on a quick second press once Off is known, even with a read pending", async () => { + let peeked: QuitConfirmationMode = "hold"; const ctrlQ = { meta: false, control: true }; - const harness = makeHarness({ platform: "win32", getMode: () => Promise.resolve(mode) }); - await Promise.resolve(); - - mode = "off"; + const harness = makeHarness({ + platform: "win32", + getMode: () => new Promise(() => undefined), + peekMode: () => peeked, + }); await harness.send(makeInput(ctrlQ)); await harness.send(makeInput({ ...ctrlQ, type: "keyUp" })); - expect(harness.preventDefault).toHaveBeenCalledTimes(1); - expect(harness.notifications).toEqual([]); - - mode = "direct"; - vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); + peeked = "off"; + vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS - 100); await harness.send(makeInput(ctrlQ)); - await harness.send(makeInput({ ...ctrlQ, type: "keyUp" })); - expect(harness.preventDefault).toHaveBeenCalledTimes(1); + expect(harness.quit).not.toHaveBeenCalled(); + expect(harness.preventDefault).toHaveBeenCalledTimes(1); + }); - vi.advanceTimersByTime(QUIT_DOUBLE_PRESS_MS + 100); - await harness.send(makeInput(ctrlQ)); - expect(harness.quit).toHaveBeenCalledTimes(1); + it("does not quit when the mode read settles as Off", async () => { + const harness = makeHarness({ mode: "off", platform: "win32" }); + await harness.send(makeInput({ meta: false, control: true })); + await harness.holdFor(QUIT_HOLD_DURATION_MS + 200, { meta: false, control: true }); + + expect(harness.quit).not.toHaveBeenCalled(); + expect(harness.notifications).toEqual([]); }); }); diff --git a/apps/desktop/src/window/QuitHold.ts b/apps/desktop/src/window/QuitHold.ts index f68f5e224fb4a..15d990dae2224 100644 --- a/apps/desktop/src/window/QuitHold.ts +++ b/apps/desktop/src/window/QuitHold.ts @@ -25,6 +25,8 @@ export interface QuitHoldKeyInput { export interface QuitShortcutOptions { readonly platform: NodeJS.Platform; readonly getMode: () => Promise; + /** Latest known mode; preventDefault must be decided before getMode settles. */ + readonly peekMode: () => QuitConfirmationMode | undefined; readonly notify: (event: QuitShortcutHintEvent) => void; readonly concealWindow: () => void; readonly quit: () => void; @@ -46,18 +48,6 @@ export function makeQuitShortcutHandler( let repeatCadenceMs = 0; // A key release must not invalidate a pending direct-mode read; only superseding/cancelled presses do. let generation = 0; - // preventDefault must be decided synchronously, so Off uses the last mode read. - let knownMode: QuitConfirmationMode | undefined; - let modeRead = 0; - - const readMode = () => { - const read = ++modeRead; - return options.getMode().then((resolvedMode) => { - if (read === modeRead) knownMode = resolvedMode; - return resolvedMode; - }); - }; - readMode().catch(() => undefined); const clearWatchdog = () => { if (watchdog !== undefined) { @@ -141,13 +131,12 @@ export function makeQuitShortcutHandler( return; } - if (knownMode === "off") { + if (options.peekMode() === "off") { // Passing Cmd+Q through would reach the macOS app menu's Quit accelerator. if (options.platform === "darwin") event.preventDefault(); if (!input.isAutoRepeat) { lastPressAt = 0; release(); - readMode().catch(() => undefined); } return; } @@ -178,7 +167,7 @@ export function makeQuitShortcutHandler( const pressGeneration = generation; holding = true; heldSince = now; - void readMode().then( + void options.getMode().then( (resolvedMode) => { if (generation !== pressGeneration) return; if (resolvedMode === "off") { From b6e72b2adc2fb457f98247f2f27b2be801265720 Mon Sep 17 00:00:00 2001 From: Jason Qin Date: Thu, 29 Oct 2026 17:37:20 -0700 Subject: [PATCH 3/5] fix(desktop): settings peek ignores stale reads and failed writes A read that started before a newer write no longer overwrites that write in the cache, and set publishes only after the file write succeeds. --- .../settings/DesktopClientSettings.test.ts | 78 +++++++++++++++++++ .../src/settings/DesktopClientSettings.ts | 58 +++++++++----- 2 files changed, 115 insertions(+), 21 deletions(-) diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index a7605889b4547..48c4527c573fd 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -5,7 +5,9 @@ import { DEFAULT_CLIENT_SETTINGS, type ClientSettings, } from "@t3tools/contracts"; +import * as Deferred from "effect/Deferred"; import * as Effect from "effect/Effect"; +import * as Fiber from "effect/Fiber"; import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as Option from "effect/Option"; @@ -193,6 +195,82 @@ describe("DesktopClientSettings", () => { ), ); + it.effect("keeps a newer write when an older read finishes after it", () => + withClientSettings( + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const settings = yield* DesktopClientSettings.DesktopClientSettings; + const holdSettings = { ...clientSettings, confirmQuit: "hold" as const }; + const offSettings = { ...clientSettings, confirmQuit: "off" as const }; + yield* settings.set(holdSettings); + const readStarted = yield* Deferred.make(); + const releaseRead = yield* Deferred.make(); + let delayRead = true; + const raceSettings = yield* DesktopClientSettings.make.pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.FileSystem.of({ + ...fileSystem, + readFileString: (path) => + fileSystem + .readFileString(path) + .pipe( + Effect.tap(() => + delayRead + ? Deferred.succeed(readStarted, undefined).pipe( + Effect.andThen(Deferred.await(releaseRead)), + ) + : Effect.void, + ), + ), + }), + ), + ); + + const staleRead = yield* Effect.forkChild(raceSettings.get); + yield* Deferred.await(readStarted); + delayRead = false; + yield* raceSettings.set(offSettings); + yield* Deferred.succeed(releaseRead, undefined); + + assert.deepEqual(yield* Fiber.join(staleRead), Option.some(holdSettings)); + assert.deepEqual(yield* raceSettings.peek, Option.some(offSettings)); + }), + ), + ); + + it.effect("keeps the cached settings when a write fails", () => + withClientSettings( + Effect.gen(function* () { + const fileSystem = yield* FileSystem.FileSystem; + const settings = yield* DesktopClientSettings.DesktopClientSettings; + yield* settings.set(clientSettings); + const failingSettings = yield* DesktopClientSettings.make.pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.FileSystem.of({ + ...fileSystem, + writeFileString: (path) => + Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "writeFileString", + pathOrDescriptor: path, + }), + ), + }), + ), + ); + yield* failingSettings.get; + + yield* failingSettings.set({ ...clientSettings, confirmQuit: "off" }).pipe(Effect.flip); + + assert.deepEqual(yield* failingSettings.peek, Option.some(clientSettings)); + }), + ), + ); + it.effect("loads lenient direct client settings documents", () => withClientSettings( Effect.gen(function* () { diff --git a/apps/desktop/src/settings/DesktopClientSettings.ts b/apps/desktop/src/settings/DesktopClientSettings.ts index 7dde6434b45ca..92abfaae8efd7 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.ts @@ -192,33 +192,49 @@ export const make = Effect.gen(function* () { const path = yield* Path.Path; const crypto = yield* Crypto.Crypto; const latestRef = yield* Ref.make(Option.none()); + // Each write starts a generation; a read or write publishes only if no newer write began. + const generationRef = yield* Ref.make(0); + const publish = (generation: number, settings: Option.Option) => + Ref.get(generationRef).pipe( + Effect.flatMap((current) => + current === generation ? Ref.set(latestRef, settings) : Effect.void, + ), + ); return DesktopClientSettings.of({ - get: readClientSettings(fileSystem, environment.clientSettingsPath).pipe( - Effect.tap((settings) => Ref.set(latestRef, settings)), + get: Ref.get(generationRef).pipe( + Effect.flatMap((generation) => + readClientSettings(fileSystem, environment.clientSettingsPath).pipe( + Effect.tap((settings) => publish(generation, settings)), + ), + ), Effect.withSpan("desktop.clientSettings.get"), ), peek: Ref.get(latestRef), set: Effect.fn("desktop.clientSettings.set")(function (settings) { - return Ref.set(latestRef, Option.some(settings)).pipe( - Effect.andThen(crypto.randomUUIDv4()), - Effect.map((uuid) => uuid.replace(/-/g, "")), - Effect.mapError( - (cause) => - new DesktopClientSettingsWriteError({ - operation: "create-temporary-file-name", - path: environment.clientSettingsPath, - cause, - }), - ), - Effect.flatMap((suffix) => - writeClientSettings({ - fileSystem, - path, - settingsPath: environment.clientSettingsPath, - settings, - suffix, - }), + return Ref.updateAndGet(generationRef, (generation) => generation + 1).pipe( + Effect.flatMap((generation) => + crypto.randomUUIDv4().pipe( + Effect.map((uuid) => uuid.replace(/-/g, "")), + Effect.mapError( + (cause) => + new DesktopClientSettingsWriteError({ + operation: "create-temporary-file-name", + path: environment.clientSettingsPath, + cause, + }), + ), + Effect.flatMap((suffix) => + writeClientSettings({ + fileSystem, + path, + settingsPath: environment.clientSettingsPath, + settings, + suffix, + }), + ), + Effect.andThen(publish(generation, Option.some(settings))), + ), ), ); }), From d49f9734abed37ce81cb894f4f288b8d106a5e51 Mon Sep 17 00:00:00 2001 From: Jason Qin Date: Thu, 29 Oct 2026 17:58:09 -0700 Subject: [PATCH 4/5] fix(desktop): serialize client settings writes Overlapping set calls could rename out of order and leave older settings on disk while peek held the newer ones. --- .../settings/DesktopClientSettings.test.ts | 65 +++++++++++++++++++ .../src/settings/DesktopClientSettings.ts | 4 ++ 2 files changed, 69 insertions(+) diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index 48c4527c573fd..daf10a25fe9f6 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -239,6 +239,71 @@ describe("DesktopClientSettings", () => { ), ); + it.effect("lands overlapping writes in order on disk and in peek", () => + withClientSettings( + Effect.gen(function* () { + const environment = yield* DesktopEnvironment.DesktopEnvironment; + const fileSystem = yield* FileSystem.FileSystem; + const firstRenameStarted = yield* Deferred.make(); + const releaseFirstRename = yield* Deferred.make(); + // Synchronous in-memory files let an unblocked write finish inside its fork. + const files = new Map(); + let renames = 0; + const settings = yield* DesktopClientSettings.make.pipe( + Effect.provideService( + FileSystem.FileSystem, + FileSystem.FileSystem.of({ + ...fileSystem, + readLink: (path) => + Effect.fail( + PlatformError.systemError({ + _tag: "NotFound", + module: "FileSystem", + method: "readLink", + pathOrDescriptor: path, + }), + ), + makeDirectory: () => Effect.void, + writeFileString: (path, contents) => + Effect.sync(() => void files.set(path, contents)), + rename: (from, to) => + (renames++ === 0 + ? Deferred.succeed(firstRenameStarted, undefined).pipe( + Effect.andThen(Deferred.await(releaseFirstRename)), + ) + : Effect.void + ).pipe( + Effect.andThen( + Effect.sync(() => { + files.set(to, files.get(from) ?? ""); + files.delete(from); + }), + ), + ), + }), + ), + ); + const olderSettings = { ...clientSettings, confirmQuit: "hold" as const }; + const newerSettings = { ...clientSettings, confirmQuit: "off" as const }; + + const olderWrite = yield* Effect.forkChild(settings.set(olderSettings)); + yield* Deferred.await(firstRenameStarted); + const newerWrite = yield* Effect.forkChild(settings.set(newerSettings), { + startImmediately: true, + }); + yield* Deferred.succeed(releaseFirstRename, undefined); + yield* Fiber.join(olderWrite); + yield* Fiber.join(newerWrite); + + assert.deepEqual( + yield* decodeClientSettingsJson(files.get(environment.clientSettingsPath) ?? ""), + newerSettings, + ); + assert.deepEqual(yield* settings.peek, Option.some(newerSettings)); + }), + ), + ); + it.effect("keeps the cached settings when a write fails", () => withClientSettings( Effect.gen(function* () { diff --git a/apps/desktop/src/settings/DesktopClientSettings.ts b/apps/desktop/src/settings/DesktopClientSettings.ts index 92abfaae8efd7..99891413d2169 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.ts @@ -10,6 +10,7 @@ import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Ref from "effect/Ref"; +import * as Semaphore from "effect/Semaphore"; import * as DesktopEnvironment from "../app/DesktopEnvironment.ts"; @@ -191,6 +192,7 @@ export const make = Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; const crypto = yield* Crypto.Crypto; + const writeLock = yield* Semaphore.make(1); const latestRef = yield* Ref.make(Option.none()); // Each write starts a generation; a read or write publishes only if no newer write began. const generationRef = yield* Ref.make(0); @@ -236,6 +238,8 @@ export const make = Effect.gen(function* () { Effect.andThen(publish(generation, Option.some(settings))), ), ), + // Renames must land in generation order, or disk ends up older than peek. + Semaphore.withPermit(writeLock), ); }), }); From 93ac86b9df8228b578a80c35a073e53b270ead10 Mon Sep 17 00:00:00 2001 From: Jason Qin Date: Thu, 29 Oct 2026 18:03:06 -0700 Subject: [PATCH 5/5] Revert "fix(desktop): serialize client settings writes" This reverts commit d49f9734abed37ce81cb894f4f288b8d106a5e51. --- .../settings/DesktopClientSettings.test.ts | 65 ------------------- .../src/settings/DesktopClientSettings.ts | 4 -- 2 files changed, 69 deletions(-) diff --git a/apps/desktop/src/settings/DesktopClientSettings.test.ts b/apps/desktop/src/settings/DesktopClientSettings.test.ts index daf10a25fe9f6..48c4527c573fd 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.test.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.test.ts @@ -239,71 +239,6 @@ describe("DesktopClientSettings", () => { ), ); - it.effect("lands overlapping writes in order on disk and in peek", () => - withClientSettings( - Effect.gen(function* () { - const environment = yield* DesktopEnvironment.DesktopEnvironment; - const fileSystem = yield* FileSystem.FileSystem; - const firstRenameStarted = yield* Deferred.make(); - const releaseFirstRename = yield* Deferred.make(); - // Synchronous in-memory files let an unblocked write finish inside its fork. - const files = new Map(); - let renames = 0; - const settings = yield* DesktopClientSettings.make.pipe( - Effect.provideService( - FileSystem.FileSystem, - FileSystem.FileSystem.of({ - ...fileSystem, - readLink: (path) => - Effect.fail( - PlatformError.systemError({ - _tag: "NotFound", - module: "FileSystem", - method: "readLink", - pathOrDescriptor: path, - }), - ), - makeDirectory: () => Effect.void, - writeFileString: (path, contents) => - Effect.sync(() => void files.set(path, contents)), - rename: (from, to) => - (renames++ === 0 - ? Deferred.succeed(firstRenameStarted, undefined).pipe( - Effect.andThen(Deferred.await(releaseFirstRename)), - ) - : Effect.void - ).pipe( - Effect.andThen( - Effect.sync(() => { - files.set(to, files.get(from) ?? ""); - files.delete(from); - }), - ), - ), - }), - ), - ); - const olderSettings = { ...clientSettings, confirmQuit: "hold" as const }; - const newerSettings = { ...clientSettings, confirmQuit: "off" as const }; - - const olderWrite = yield* Effect.forkChild(settings.set(olderSettings)); - yield* Deferred.await(firstRenameStarted); - const newerWrite = yield* Effect.forkChild(settings.set(newerSettings), { - startImmediately: true, - }); - yield* Deferred.succeed(releaseFirstRename, undefined); - yield* Fiber.join(olderWrite); - yield* Fiber.join(newerWrite); - - assert.deepEqual( - yield* decodeClientSettingsJson(files.get(environment.clientSettingsPath) ?? ""), - newerSettings, - ); - assert.deepEqual(yield* settings.peek, Option.some(newerSettings)); - }), - ), - ); - it.effect("keeps the cached settings when a write fails", () => withClientSettings( Effect.gen(function* () { diff --git a/apps/desktop/src/settings/DesktopClientSettings.ts b/apps/desktop/src/settings/DesktopClientSettings.ts index 99891413d2169..92abfaae8efd7 100644 --- a/apps/desktop/src/settings/DesktopClientSettings.ts +++ b/apps/desktop/src/settings/DesktopClientSettings.ts @@ -10,7 +10,6 @@ import * as Option from "effect/Option"; import * as Path from "effect/Path"; import * as Schema from "effect/Schema"; import * as Ref from "effect/Ref"; -import * as Semaphore from "effect/Semaphore"; import * as DesktopEnvironment from "../app/DesktopEnvironment.ts"; @@ -192,7 +191,6 @@ export const make = Effect.gen(function* () { const fileSystem = yield* FileSystem.FileSystem; const path = yield* Path.Path; const crypto = yield* Crypto.Crypto; - const writeLock = yield* Semaphore.make(1); const latestRef = yield* Ref.make(Option.none()); // Each write starts a generation; a read or write publishes only if no newer write began. const generationRef = yield* Ref.make(0); @@ -238,8 +236,6 @@ export const make = Effect.gen(function* () { Effect.andThen(publish(generation, Option.some(settings))), ), ), - // Renames must land in generation order, or disk ends up older than peek. - Semaphore.withPermit(writeLock), ); }), });