From e199c97e82e534a3dccab9c68507fb2a727f19ff Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Fri, 17 Jul 2026 16:34:00 -0700 Subject: [PATCH] fix(keybindings): preserve valid pre-swap overrides --- src/main/keybindings/keybinding-file.test.ts | 32 ++++++++++++++++++++ src/main/keybindings/keybinding-file.ts | 12 +++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/main/keybindings/keybinding-file.test.ts b/src/main/keybindings/keybinding-file.test.ts index 857b67ad91e..81af9ab31b2 100644 --- a/src/main/keybindings/keybinding-file.test.ts +++ b/src/main/keybindings/keybinding-file.test.ts @@ -263,6 +263,38 @@ describe('keybinding-file', () => { }) }) + it('preserves pre-swap customizations that conflict only with the new defaults', () => { + // This was valid before the swap: moving nextSameType to Mod+K freed its + // old Shift+] chord for previousSameType. The new registry temporarily + // assigns Shift+] to nextAllTypes, but the seed must not mistake the + // resulting conflict for an absent user override and replace it. + writeFileSync( + filePath, + JSON.stringify({ + version: 1, + platforms: { + darwin: { + 'tab.nextSameType': ['Mod+K'], + 'tab.previousSameType': ['Mod+Shift+BracketRight'] + } + } + }), + 'utf8' + ) + + seedLegacyTabSwitchBindings(filePath, 'darwin', LEGACY_TAB_SWITCH_BINDINGS) + + expect(readKeybindingFile(filePath, 'darwin')).toMatchObject({ + overrides: { + 'tab.nextSameType': ['Mod+K'], + 'tab.previousSameType': ['Mod+Shift+BracketRight'], + 'tab.nextAllTypes': ['Mod+Alt+BracketRight'], + 'tab.previousAllTypes': ['Mod+Alt+BracketLeft'] + }, + diagnostics: [] + }) + }) + it('is a no-op once every swapped action already resolves on this platform', () => { // Seeding writes the whole document at once, so it clears the per-write // conflict guard that a naive one-action-at-a-time write would trip. diff --git a/src/main/keybindings/keybinding-file.ts b/src/main/keybindings/keybinding-file.ts index c49343fc6d5..1da2f071c33 100644 --- a/src/main/keybindings/keybinding-file.ts +++ b/src/main/keybindings/keybinding-file.ts @@ -340,12 +340,14 @@ export function seedLegacyTabSwitchBindings( const keybindingPlatform = getKeybindingPlatform(platform) const actionIds = Object.keys(legacyBindings) as KeybindingActionId[] const current = readKeybindingFile(path, platform) - // `current.overrides` is the effective override map for this platform (common - // + this platform's section). Skip any action that already resolves to a - // user-set binding here; a foreign-platform-only override must not block the - // pin, or this platform would drift to the new default. + const activePlatformOverrides = current.platformOverrides[keybindingPlatform] ?? {} + // Why: the new defaults can temporarily make a valid pre-swap customization + // look conflicting and remove it from `current.overrides`. Inspect the parsed + // common + active-platform sections directly so the seed never replaces it. const toSeed = actionIds.filter( - (actionId) => !Object.prototype.hasOwnProperty.call(current.overrides, actionId) + (actionId) => + !Object.prototype.hasOwnProperty.call(current.commonOverrides, actionId) && + !Object.prototype.hasOwnProperty.call(activePlatformOverrides, actionId) ) if (toSeed.length === 0) { return { seeded: false, snapshot: current }