From c1a70f7261750b370286a719b0489336316c3d32 Mon Sep 17 00:00:00 2001 From: can1357 Date: Sun, 31 May 2026 04:34:45 +0200 Subject: [PATCH] feat(coding-agent): migrated keybindings config to YAML with JSON migration - Changed canonical config path from `keybindings.json` to `keybindings.yml`. - Added automatic migration of legacy JSON to YAML on first load. - Retained read support for `keybindings.yaml` without promoting it to canonical. - Added tests for yml, yaml, and JSON migration scenarios. --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/config/keybindings.ts | 105 +++++++++++++----- .../test/keybindings-migration.test.ts | 63 ++++++++++- 3 files changed, 138 insertions(+), 31 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c2807d80c..4b19a5d0e 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -12,6 +12,7 @@ - Changed `search` output to preserve full virtual and internal URL paths in grouped results and `details.files` instead of collapsing them to file basenames - Changed `/omfg` to run up to three generation attempts with validation feedback and only prompt saving when no draft matches assistant history - Changed `/omfg` to show a live draft panel with generation/validation/saving status and allow canceling an active rule request with `Esc` +- Changed keybindings config to use `~/.omp/agent/keybindings.yml`, with automatic migration from legacy `keybindings.json` and continued support for `keybindings.yaml`. ### Fixed diff --git a/packages/coding-agent/src/config/keybindings.ts b/packages/coding-agent/src/config/keybindings.ts index 3b80149c4..b8a89c908 100644 --- a/packages/coding-agent/src/config/keybindings.ts +++ b/packages/coding-agent/src/config/keybindings.ts @@ -1,4 +1,4 @@ -import { existsSync, readFileSync, writeFileSync } from "node:fs"; +import * as fs from "node:fs"; import * as path from "node:path"; import { type Keybinding, @@ -10,6 +10,7 @@ import { KeybindingsManager as TuiKeybindingsManager, } from "@oh-my-pi/pi-tui"; import { getAgentDir, isEnoent, logger } from "@oh-my-pi/pi-utils"; +import { YAML } from "bun"; /** * Application-level keybindings (coding agent specific). @@ -330,17 +331,29 @@ function orderKeybindingsConfig(config: KeybindingsConfig): KeybindingsConfig { return ordered; } +const KEYBINDINGS_YML = "keybindings.yml"; +const KEYBINDINGS_YAML = "keybindings.yaml"; +const LEGACY_KEYBINDINGS_JSON = "keybindings.json"; + +interface KeybindingsConfigPaths { + readPath: string; + writeBackPath: string; +} + /** * Load raw config from a file synchronously. - * Returns parsed JSON or null if file doesn't exist or is invalid. + * Returns parsed JSON/YAML or null if file doesn't exist or is invalid. */ function loadRawConfig(filePath: string): unknown { try { - if (!existsSync(filePath)) { - return null; + const content = fs.readFileSync(filePath, "utf-8"); + if (filePath.endsWith(".json")) { + return JSON.parse(content); } - const content = readFileSync(filePath, "utf-8"); - return JSON.parse(content); + if (filePath.endsWith(".yml") || filePath.endsWith(".yaml")) { + return YAML.parse(content); + } + throw new Error(`Unsupported keybindings config extension: ${filePath}`); } catch (error) { if (isEnoent(error)) { return null; @@ -350,34 +363,67 @@ function loadRawConfig(filePath: string): unknown { } } +function writeKeybindingsConfig(filePath: string, config: KeybindingsConfig): boolean { + try { + fs.writeFileSync(filePath, YAML.stringify(config, null, 2), "utf-8"); + logger.debug("Migrated keybindings config", { path: filePath }); + return true; + } catch (error) { + logger.warn("Failed to write migrated keybindings config", { path: filePath, error: String(error) }); + return false; + } +} + +function resolveKeybindingsConfigPaths(agentDir: string): KeybindingsConfigPaths { + const ymlPath = path.join(agentDir, KEYBINDINGS_YML); + if (fs.existsSync(ymlPath)) { + return { readPath: ymlPath, writeBackPath: ymlPath }; + } + + const yamlPath = path.join(agentDir, KEYBINDINGS_YAML); + if (fs.existsSync(yamlPath)) { + return { readPath: yamlPath, writeBackPath: yamlPath }; + } + + const jsonPath = path.join(agentDir, LEGACY_KEYBINDINGS_JSON); + if (fs.existsSync(jsonPath)) { + return { readPath: jsonPath, writeBackPath: ymlPath }; + } + + return { readPath: ymlPath, writeBackPath: ymlPath }; +} + /** - * Migrate keybindings config file from old format to new. - * Reads from agentDir/keybindings.json, migrates old names, and writes back. + * Load and migrate keybindings config. + * Legacy JSON is read for compatibility, but successful write-back goes to YAML. */ -function loadKeybindingsConfig(filePath: string, writeBack: boolean): KeybindingsConfig { +function loadKeybindingsConfig( + filePath: string, + writeBackPath: string | undefined, +): { + config: KeybindingsConfig; + persistedPath: string; +} { const rawConfig = loadRawConfig(filePath); if (rawConfig === null) { - return {}; + return { config: {}, persistedPath: filePath }; } const { config: migratedConfig, migrated } = migrateKeybindingNames(rawConfig); - if (writeBack && migrated) { + const shouldWriteBack = writeBackPath !== undefined && (migrated || writeBackPath !== filePath); + if (shouldWriteBack) { const ordered = orderKeybindingsConfig(migratedConfig); - try { - writeFileSync(filePath, `${JSON.stringify(ordered, null, 2)}\n`, "utf-8"); - logger.debug("Migrated keybindings config", { path: filePath }); - } catch (error) { - logger.warn("Failed to write migrated keybindings config", { path: filePath, error: String(error) }); - } + const persistedPath = writeKeybindingsConfig(writeBackPath, ordered) ? writeBackPath : filePath; + return { config: migratedConfig, persistedPath }; } - return migratedConfig; + return { config: migratedConfig, persistedPath: filePath }; } function migrateKeybindingsConfigFile(agentDir: string): void { - const configPath = path.join(agentDir, "keybindings.json"); - loadKeybindingsConfig(configPath, true); + const { readPath, writeBackPath } = resolveKeybindingsConfigPaths(agentDir); + loadKeybindingsConfig(readPath, writeBackPath); } /** @@ -393,12 +439,13 @@ export class KeybindingsManager extends TuiKeybindingsManager { } /** - * Create from config file at agentDir/keybindings.json. + * Create from config file at agentDir/keybindings.yml. + * Legacy keybindings.json is migrated to keybindings.yml on load. */ static create(agentDir: string = getAgentDir()): KeybindingsManager { - const configPath = path.join(agentDir, "keybindings.json"); - const userBindings = KeybindingsManager.#loadFromFile(configPath); - const manager = new KeybindingsManager(userBindings, configPath); + const { readPath, writeBackPath } = resolveKeybindingsConfigPaths(agentDir); + const { config: userBindings, persistedPath } = KeybindingsManager.#loadFromFile(readPath, writeBackPath); + const manager = new KeybindingsManager(userBindings, persistedPath); // Set globally so getKeybindings() returns this manager setKeybindings(manager); return manager; @@ -416,7 +463,8 @@ export class KeybindingsManager extends TuiKeybindingsManager { */ reload(): void { if (!this.#configPath) return; - this.setUserBindings(KeybindingsManager.#loadFromFile(this.#configPath)); + const { config } = KeybindingsManager.#loadFromFile(this.#configPath); + this.setUserBindings(config); } /** @@ -437,8 +485,11 @@ export class KeybindingsManager extends TuiKeybindingsManager { /** * Load user bindings from a file, migrating old names if needed. */ - static #loadFromFile(filePath: string): KeybindingsConfig { - return loadKeybindingsConfig(filePath, true); + static #loadFromFile( + filePath: string, + writeBackPath?: string, + ): { config: KeybindingsConfig; persistedPath: string } { + return loadKeybindingsConfig(filePath, writeBackPath); } } diff --git a/packages/coding-agent/test/keybindings-migration.test.ts b/packages/coding-agent/test/keybindings-migration.test.ts index cf21a1d8c..2dc8e0614 100644 --- a/packages/coding-agent/test/keybindings-migration.test.ts +++ b/packages/coding-agent/test/keybindings-migration.test.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import { setKeybindings } from "@oh-my-pi/pi-tui"; +import { YAML } from "bun"; import { KeybindingsManager } from "../src/config/keybindings"; describe("KeybindingsManager.create", () => { @@ -14,12 +15,13 @@ describe("KeybindingsManager.create", () => { setKeybindings(KeybindingsManager.inMemory()); }); - it("migrates legacy keybinding names on disk during create", async () => { + it("migrates legacy keybinding JSON to YAML during create", async () => { const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-")); - const configPath = path.join(agentDir, "keybindings.json"); + const jsonPath = path.join(agentDir, "keybindings.json"); + const ymlPath = path.join(agentDir, "keybindings.yml"); await Bun.write( - configPath, + jsonPath, `${JSON.stringify( { fork: "ctrl+f", @@ -34,7 +36,7 @@ describe("KeybindingsManager.create", () => { try { const manager = KeybindingsManager.create(agentDir); - const writtenConfig = await Bun.file(configPath).json(); + const writtenConfig = YAML.parse(await Bun.file(ymlPath).text()); expect(manager.getKeys("app.session.fork")).toEqual(["ctrl+f"]); expect(manager.getKeys("tui.select.confirm")).toEqual(["enter"]); @@ -47,6 +49,59 @@ describe("KeybindingsManager.create", () => { "tui.select.confirm": "enter", }); expect(writtenConfig).not.toHaveProperty("selectModelTemporary"); + expect(await Bun.file(jsonPath).exists()).toBe(true); + } finally { + await fs.rm(agentDir, { recursive: true, force: true }); + } + }); + + it("loads keybindings.yml directly", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-")); + const configPath = path.join(agentDir, "keybindings.yml"); + + await Bun.write( + configPath, + YAML.stringify( + { + "app.session.fork": "ctrl+f", + "app.clipboard.copyPrompt": ["alt+c", "ctrl+shift+c"], + }, + null, + 2, + ), + ); + + try { + const manager = KeybindingsManager.create(agentDir); + + expect(manager.getKeys("app.session.fork")).toEqual(["ctrl+f"]); + expect(manager.getKeys("app.clipboard.copyPrompt")).toEqual(["alt+c", "ctrl+shift+c"]); + } finally { + await fs.rm(agentDir, { recursive: true, force: true }); + } + }); + + it("accepts keybindings.yaml when present", async () => { + const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-")); + const yamlPath = path.join(agentDir, "keybindings.yaml"); + const canonicalPath = path.join(agentDir, "keybindings.yml"); + + await Bun.write( + yamlPath, + YAML.stringify( + { + "app.plan.toggle": "alt+shift+p", + }, + null, + 2, + ), + ); + + try { + const manager = KeybindingsManager.create(agentDir); + + expect(manager.getKeys("app.plan.toggle")).toEqual(["alt+shift+p"]); + expect(await Bun.file(canonicalPath).exists()).toBe(false); } finally { await fs.rm(agentDir, { recursive: true, force: true }); }