test(keybindings): prove existing-user parity + make the seed strictly per-action

seedLegacyTabSwitchBindings now pins each un-customized action individually
instead of skipping all four when any one is customized. A partially-customized
existing user keeps their rebound action AND the pre-swap default on the rest;
no existing user's behavior changes. The skip check keys on this platform's
effective overrides so a foreign-platform-only override can't leave the active
platform on a new default.

Adds keybinding-service.test.ts: constructs a real KeybindingService and asserts
effective bindings + real keystroke matching for both cohorts across darwin/
linux/win32, plus partial-customization, idempotency, and seed-failure retry.
This commit is contained in:
Brennan Benson
2026-07-17 15:52:03 -07:00
parent 30e2fad672
commit 1141c56a6c
3 changed files with 279 additions and 15 deletions
+35 -4
View File
@@ -247,15 +247,46 @@ describe('keybinding-file', () => {
expect(snapshot.diagnostics).toEqual([])
})
it('skips the seed when the user has already customized any swapped action', () => {
it('preserves a customized action while still pinning the un-customized ones', () => {
// A partially-customized existing user: they rebound one action. The other
// three must still land on their pre-swap defaults, not the new ones.
writeKeybindingOverride(filePath, 'darwin', 'tab.nextSameType', ['Mod+Alt+K'])
const result = seedLegacyTabSwitchBindings(filePath, 'darwin', LEGACY_TAB_SWITCH_BINDINGS)
expect(result.seeded).toBe(false)
// The user's choice is untouched and the other actions stay on their defaults.
expect(result.seeded).toBe(true)
expect(readKeybindingFile(filePath, 'darwin').overrides).toEqual({
'tab.nextSameType': ['Mod+Alt+K']
'tab.nextSameType': ['Mod+Alt+K'],
'tab.previousSameType': ['Mod+Shift+BracketLeft'],
'tab.nextAllTypes': ['Mod+Alt+BracketRight'],
'tab.previousAllTypes': ['Mod+Alt+BracketLeft']
})
})
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.
const first = seedLegacyTabSwitchBindings(filePath, 'darwin', LEGACY_TAB_SWITCH_BINDINGS)
expect(first.seeded).toBe(true)
expect(first.snapshot.diagnostics).toEqual([])
const second = seedLegacyTabSwitchBindings(filePath, 'darwin', LEGACY_TAB_SWITCH_BINDINGS)
expect(second.seeded).toBe(false)
})
it('does not let a foreign-platform override block the active-platform pin', () => {
// Only linux is customized; a darwin launch must still pin darwin so the
// active platform keeps the pre-swap behavior.
writeKeybindingOverride(filePath, 'linux', 'tab.nextAllTypes', ['Mod+Alt+K'])
const result = seedLegacyTabSwitchBindings(filePath, 'darwin', LEGACY_TAB_SWITCH_BINDINGS)
expect(result.seeded).toBe(true)
expect(readKeybindingFile(filePath, 'darwin').platformOverrides.darwin).toMatchObject({
'tab.nextAllTypes': ['Mod+Alt+BracketRight']
})
expect(readKeybindingFile(filePath, 'linux').platformOverrides.linux).toEqual({
'tab.nextAllTypes': ['Mod+Alt+K']
})
})
+17 -11
View File
@@ -322,9 +322,15 @@ export function migrateLegacyKeybindings(
* Pin the pre-swap tab-switch chords for a pre-existing install so upgrading
* users keep the shortcuts they learned. Writes into the active-platform
* section (mirroring `writeKeybindingOverride`) so the seeded values stay
* resettable from Settings. No-op when the user has already customized any of
* the swapped actions — that is custom territory, and a half-swapped pin would
* be worse than leaving their choices alone.
* resettable from Settings.
*
* Pins per action, not all-or-nothing: an action is seeded only when this
* platform has no effective override for it yet. That way a user who rebound
* just one of the swapped actions keeps that choice AND keeps the pre-swap
* default on the other three — an existing user's behavior is never altered,
* whether they customized none, some, or all of them. Because every pin equals
* the action's old default, the seeded set reproduces exactly today's effective
* config and introduces no new conflicts.
*/
export function seedLegacyTabSwitchBindings(
path: string,
@@ -334,14 +340,14 @@ export function seedLegacyTabSwitchBindings(
const keybindingPlatform = getKeybindingPlatform(platform)
const actionIds = Object.keys(legacyBindings) as KeybindingActionId[]
const current = readKeybindingFile(path, platform)
const alreadyCustomized = actionIds.some(
(actionId) =>
Object.prototype.hasOwnProperty.call(current.commonOverrides, actionId) ||
PLATFORM_KEYS.some((os) =>
Object.prototype.hasOwnProperty.call(current.platformOverrides[os] ?? {}, actionId)
)
// `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 toSeed = actionIds.filter(
(actionId) => !Object.prototype.hasOwnProperty.call(current.overrides, actionId)
)
if (alreadyCustomized) {
if (toSeed.length === 0) {
return { seeded: false, snapshot: current }
}
@@ -359,7 +365,7 @@ export function seedLegacyTabSwitchBindings(
const activePlatform = isJsonObject(platforms[keybindingPlatform])
? { ...(platforms[keybindingPlatform] as JsonObject) }
: {}
for (const actionId of actionIds) {
for (const actionId of toSeed) {
const normalized = normalizeKeybindingArrayForAction(actionId, legacyBindings[actionId] ?? [])
if (Array.isArray(normalized)) {
activePlatform[actionId] = normalized
@@ -0,0 +1,227 @@
import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
import {
getEffectiveKeybindingsForAction,
keybindingMatchesAction,
LEGACY_TAB_SWITCH_BINDINGS,
type KeybindingActionId,
type KeybindingInput
} from '../../shared/keybindings'
import { getUserKeybindingsPath, writeKeybindingOverride } from './keybinding-file'
import { KeybindingService } from './keybinding-service'
// End-to-end coverage of the tab-switch convention swap: constructs a real
// KeybindingService (which runs the seed) and asserts the effective bindings AND
// real keystroke matching for both cohorts. The load-bearing claim under test is
// that a pre-existing install's behavior is byte-identical to what it was before
// the swap, on every platform and every customization state.
const PLATFORMS: NodeJS.Platform[] = ['darwin', 'linux', 'win32']
const SWAPPED_ACTIONS = Object.keys(LEGACY_TAB_SWITCH_BINDINGS) as KeybindingActionId[]
function makeCohort(pending: boolean): {
controller: { isPending: () => boolean; markSeeded: () => void }
seeded: () => boolean
} {
let stillPending = pending
let didSeed = false
return {
controller: {
isPending: () => stillPending,
markSeeded: () => {
didSeed = true
stillPending = false
}
},
seeded: () => didSeed
}
}
// A physical bracket press. Mod is Cmd on macOS and Ctrl elsewhere; the matcher
// resolves brackets by `code`, so this stays layout-independent.
function bracketPress(opts: {
code: 'BracketLeft' | 'BracketRight'
alt: boolean
shift: boolean
platform: NodeJS.Platform
}): KeybindingInput {
const isMac = opts.platform === 'darwin'
return {
code: opts.code,
key: opts.code === 'BracketRight' ? ']' : '[',
meta: isMac,
control: !isMac,
alt: opts.alt,
shift: opts.shift
}
}
describe('KeybindingService tab-switch cohort seeding', () => {
let home: string
beforeEach(() => {
home = mkdtempSync(join(tmpdir(), 'orca-kb-service-'))
})
afterEach(() => {
rmSync(home, { recursive: true, force: true })
})
it.each(PLATFORMS)(
'fresh install (%s) adopts the swapped defaults and writes no file',
(platform) => {
const cohort = makeCohort(false)
const service = new KeybindingService({
homePath: home,
platform,
legacyTabSwitchSeed: cohort.controller
})
expect(cohort.seeded()).toBe(false)
// Nothing written: a fresh install rides on the registry defaults.
expect(existsSync(getUserKeybindingsPath(home))).toBe(false)
const overrides = service.getOverrides()
expect(getEffectiveKeybindingsForAction('tab.nextAllTypes', platform, overrides)).toEqual([
'Mod+Shift+BracketRight'
])
expect(getEffectiveKeybindingsForAction('tab.nextSameType', platform, overrides)).toEqual([
'Mod+Alt+BracketRight'
])
// Shift+bracket now drives the broad all-types cycle.
expect(
keybindingMatchesAction(
'tab.nextAllTypes',
bracketPress({ code: 'BracketRight', alt: false, shift: true, platform }),
platform,
overrides
)
).toBe(true)
expect(
keybindingMatchesAction(
'tab.nextSameType',
bracketPress({ code: 'BracketRight', alt: false, shift: true, platform }),
platform,
overrides
)
).toBe(false)
}
)
it.each(PLATFORMS)(
'existing install (%s) keeps the pre-swap chords exactly, on a clean profile',
(platform) => {
const cohort = makeCohort(true)
const service = new KeybindingService({
homePath: home,
platform,
legacyTabSwitchSeed: cohort.controller
})
expect(cohort.seeded()).toBe(true)
const overrides = service.getOverrides()
// Effective bindings for all four actions equal the pre-swap defaults.
for (const actionId of SWAPPED_ACTIONS) {
expect(getEffectiveKeybindingsForAction(actionId, platform, overrides)).toEqual(
LEGACY_TAB_SWITCH_BINDINGS[actionId]
)
}
// Behavior is unchanged: Mod+Shift+bracket still cycles same-type, and
// Mod+Alt+bracket still cycles all-types — the inverse of a fresh install.
const shiftRight = bracketPress({ code: 'BracketRight', alt: false, shift: true, platform })
expect(keybindingMatchesAction('tab.nextSameType', shiftRight, platform, overrides)).toBe(
true
)
expect(keybindingMatchesAction('tab.nextAllTypes', shiftRight, platform, overrides)).toBe(
false
)
const altLeft = bracketPress({ code: 'BracketLeft', alt: true, shift: false, platform })
expect(keybindingMatchesAction('tab.previousAllTypes', altLeft, platform, overrides)).toBe(
true
)
expect(keybindingMatchesAction('tab.previousSameType', altLeft, platform, overrides)).toBe(
false
)
}
)
it('existing install keeps a rebound action AND pins the pre-swap default on the rest', () => {
const platform: NodeJS.Platform = 'darwin'
// The user had rebound just one action before upgrading.
writeKeybindingOverride(getUserKeybindingsPath(home), platform, 'tab.nextSameType', ['Mod+K'])
const cohort = makeCohort(true)
const service = new KeybindingService({
homePath: home,
platform,
legacyTabSwitchSeed: cohort.controller
})
const overrides = service.getOverrides()
// Their custom binding is untouched...
expect(getEffectiveKeybindingsForAction('tab.nextSameType', platform, overrides)).toEqual([
'Mod+K'
])
// ...and the other three still resolve to their pre-swap defaults, not the
// new ones — no silent behavior change for a partially-customized user.
expect(getEffectiveKeybindingsForAction('tab.previousSameType', platform, overrides)).toEqual([
'Mod+Shift+BracketLeft'
])
expect(getEffectiveKeybindingsForAction('tab.nextAllTypes', platform, overrides)).toEqual([
'Mod+Alt+BracketRight'
])
expect(getEffectiveKeybindingsForAction('tab.previousAllTypes', platform, overrides)).toEqual([
'Mod+Alt+BracketLeft'
])
})
it('is idempotent: a second launch after the seed changes nothing', () => {
const platform: NodeJS.Platform = 'linux'
const cohort = makeCohort(true)
const first = new KeybindingService({
homePath: home,
platform,
legacyTabSwitchSeed: cohort.controller
})
const afterFirst = first.getOverrides()
// Same cohort controller: markSeeded flipped it to not-pending, mirroring the
// persisted flag flipping to 'done'.
const second = new KeybindingService({
homePath: home,
platform,
legacyTabSwitchSeed: cohort.controller
})
expect(second.getOverrides()).toEqual(afterFirst)
for (const actionId of SWAPPED_ACTIONS) {
expect(getEffectiveKeybindingsForAction(actionId, platform, second.getOverrides())).toEqual(
LEGACY_TAB_SWITCH_BINDINGS[actionId]
)
}
})
it('leaves the cohort pending when the seed write fails (retries next launch)', () => {
// homePath is a regular file, so creating `<home>/.orca/` throws. The service
// must swallow the error and NOT mark the one-shot done.
const fileHome = join(home, 'not-a-dir')
writeFileSync(fileHome, 'x', 'utf8')
const cohort = makeCohort(true)
expect(
() =>
new KeybindingService({
homePath: fileHome,
platform: 'darwin',
legacyTabSwitchSeed: cohort.controller
})
).not.toThrow()
expect(cohort.seeded()).toBe(false)
expect(cohort.controller.isPending()).toBe(true)
})
})