From 288caf49de4a78e35845187f736b9685f732c5cc Mon Sep 17 00:00:00 2001 From: Erik Kotsiuba Date: Sat, 2 May 2026 17:09:58 +0200 Subject: [PATCH] feat(config): add migration framework --- README.md | 2 +- docs/architecture.md | 10 +++-- docs/configuration.md | 10 ++++- src/config/migrate.ts | 49 ++++++++++------------ src/config/migrations/index.ts | 76 ++++++++++++++++++++++++++++++++++ src/config/migrations/types.ts | 10 +++++ src/config/path-migrations.ts | 56 +++++++++++++++++++++++++ src/config/store.ts | 48 +++++++-------------- tests/config-migrate.test.ts | 42 +++++++++++++++++++ tests/config-store.test.ts | 55 +++++++++++++++++++++--- 10 files changed, 288 insertions(+), 70 deletions(-) create mode 100644 src/config/migrations/index.ts create mode 100644 src/config/migrations/types.ts create mode 100644 src/config/path-migrations.ts create mode 100644 tests/config-migrate.test.ts diff --git a/README.md b/README.md index 14dca21..00666f6 100644 --- a/README.md +++ b/README.md @@ -123,7 +123,7 @@ Fixed bindings: Review-tab shortcuts can optionally require the same number key twice via `behaviour.doublePressReviewShortcuts`. -Config file: `~/.pi/.../eko24ive-pi-ask.json` +Config file: `~/.pi/agent/extensions/eko24ive-pi-ask.json` You can edit this file yourself, ask pi to edit it for you, or use `/ask-settings` to find the exact config path and toggle behaviour settings. diff --git a/docs/architecture.md b/docs/architecture.md index 112336b..c21ab91 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -32,7 +32,9 @@ The codebase is split so the implementation reads through file boundaries and na - `src/config/defaults.ts` — canonical runtime defaults - `src/config/schema.ts` — persisted config schema and runtime type -- `src/config/migrate.ts` — persisted-file validation/migration boundary, including keymap normalization/fallback +- `src/config/migrate.ts` — persisted-file validation boundary, including keymap normalization/fallback +- `src/config/migrations/` — ordered schema-version migration framework for persisted config shape changes +- `src/config/path-migrations.ts` — file-location migration before config load - `src/config/store.ts` — load/save/backup/runtime subscription store ### UI @@ -68,10 +70,12 @@ The codebase is split so the implementation reads through file boundaries and na - deselected option notes stay in UI state - only selected option notes are emitted in the final result - editor lifecycle stays in the controller, not in the reducers -- persisted ask settings are versioned and normalized before use +- persisted ask settings are migrated to the current schema version, validated, and normalized before use +- config schema migrations preserve user-provided values and add new defaults only when fields are absent - invalid persisted keymaps fall back to default keymaps for the current session without discarding valid behaviour settings - ask settings behaviour changes persist immediately from the settings list -- when the ask config file is missing, the first ask use writes a default persisted config snapshot +- when the ask config file is missing, the first ask use writes a default persisted config snapshot under `~/.pi/agent/extensions/` +- legacy root config files move into `~/.pi/agent/extensions/` only when the current config file is absent - live config updates can affect an in-progress ask flow immediately ## Documentation rule diff --git a/docs/configuration.md b/docs/configuration.md index f266ad9..68981d6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -12,10 +12,18 @@ When changing pi-ask settings: Default path: -`~/.pi/agent/eko24ive-pi-ask.json` +`~/.pi/agent/extensions/eko24ive-pi-ask.json` If the file does not exist yet, pi-ask creates it with the current default settings the first time the ask flow is used. +Older pi-ask versions wrote this file at `~/.pi/agent/eko24ive-pi-ask.json`. If that legacy file exists and the extensions config does not, pi-ask moves it into `~/.pi/agent/extensions/` on load. If both files exist, pi-ask uses the extensions config and leaves the legacy root file untouched. + +## Config versions and migrations + +`schemaVersion` identifies the persisted config shape. pi-ask migrates older supported schema versions forward before validation, then rewrites the file in the current shape. Migrations preserve existing user-provided values and add new fields from defaults when needed. + +Unsupported future versions or invalid files are backed up and defaults are loaded. + ## Config shape ```json diff --git a/src/config/migrate.ts b/src/config/migrate.ts index dbdada3..bad5ab6 100644 --- a/src/config/migrate.ts +++ b/src/config/migrate.ts @@ -3,6 +3,11 @@ import { normalizeConfiguredKeymaps, } from "../constants/keymaps.ts"; import { normalizeAskConfig } from "./defaults.ts"; +import { + AskConfigVersionMigrationError, + type AskConfigVersionMigrationResult, + migrateAskConfigFileToCurrent, +} from "./migrations/index.ts"; import type { AskConfig, AskConfigFileV1 } from "./schema.ts"; import { validateAskConfigFileV1 } from "./schema.ts"; @@ -25,37 +30,29 @@ export interface AskConfigMigrationResult { } export function migrateAskConfig(raw: unknown): AskConfigMigrationResult { - if (!(raw && typeof raw === "object")) { + let migratedFile: AskConfigVersionMigrationResult; + try { + migratedFile = migrateAskConfigFileToCurrent(raw); + } catch (error) { + throw new AskConfigMigrationError( + "Config was invalid or unsupported.", + error instanceof AskConfigVersionMigrationError + ? error.reason + : "migration_failed" + ); + } + + if (!validateAskConfigFileV1.Check(migratedFile.config)) { throw new AskConfigMigrationError( "Config was invalid or unsupported.", "invalid_or_unsupported" ); } - if (!("schemaVersion" in raw)) { - throw new AskConfigMigrationError( - "Config was invalid or unsupported.", - "invalid_or_unsupported" - ); - } - - if ((raw as { schemaVersion?: unknown }).schemaVersion !== 1) { - throw new AskConfigMigrationError( - "Config was invalid or unsupported.", - "invalid_or_unsupported" - ); - } - - if (!validateAskConfigFileV1.Check(raw)) { - throw new AskConfigMigrationError( - "Config was invalid or unsupported.", - "invalid_or_unsupported" - ); - } - - const config = normalizeAskConfig(raw as AskConfigFileV1); + const currentFile = migratedFile.config as AskConfigFileV1; + const config = normalizeAskConfig(currentFile); const keymapsResult = normalizeConfiguredKeymaps( - (raw as AskConfigFileV1).keymaps as + currentFile.keymaps as | Partial> | undefined ); @@ -65,7 +62,7 @@ export function migrateAskConfig(raw: unknown): AskConfigMigrationResult { ...config, keymaps: normalizeAskConfig().keymaps, }, - migrated: false, + migrated: migratedFile.migrated, notice: `${keymapsResult.error} Using default ask keymaps for this session. Edit the config and restart pi or run /reload.`, }; } @@ -75,6 +72,6 @@ export function migrateAskConfig(raw: unknown): AskConfigMigrationResult { ...config, keymaps: keymapsResult.keymaps, }, - migrated: false, + migrated: migratedFile.migrated, }; } diff --git a/src/config/migrations/index.ts b/src/config/migrations/index.ts new file mode 100644 index 0000000..2a40991 --- /dev/null +++ b/src/config/migrations/index.ts @@ -0,0 +1,76 @@ +import type { AskConfigMigration, VersionedAskConfigFile } from "./types.ts"; + +export const CURRENT_ASK_CONFIG_SCHEMA_VERSION = 1; + +const ASK_CONFIG_MIGRATIONS: AskConfigMigration[] = []; + +export class AskConfigVersionMigrationError extends Error { + readonly reason: "invalid_or_unsupported" | "migration_failed"; + + constructor( + message: string, + reason: "invalid_or_unsupported" | "migration_failed" + ) { + super(message); + this.reason = reason; + } +} + +export interface AskConfigVersionMigrationResult { + config: VersionedAskConfigFile; + migrated: boolean; +} + +export function migrateAskConfigFileToCurrent( + raw: unknown +): AskConfigVersionMigrationResult { + const start = getVersionedConfigFile(raw); + let config = start; + let migrated = false; + + while (config.schemaVersion < CURRENT_ASK_CONFIG_SCHEMA_VERSION) { + const migration = ASK_CONFIG_MIGRATIONS.find( + (candidate) => candidate.from === config.schemaVersion + ); + if (!migration) { + throw new AskConfigVersionMigrationError( + `No ask config migration from schemaVersion ${config.schemaVersion}`, + "migration_failed" + ); + } + config = migration.migrate(config); + if (config.schemaVersion !== migration.to) { + throw new AskConfigVersionMigrationError( + `Ask config migration from schemaVersion ${migration.from} did not produce schemaVersion ${migration.to}`, + "migration_failed" + ); + } + migrated = true; + } + + if (config.schemaVersion !== CURRENT_ASK_CONFIG_SCHEMA_VERSION) { + throw new AskConfigVersionMigrationError( + `Unsupported ask config schemaVersion ${config.schemaVersion}`, + "invalid_or_unsupported" + ); + } + + return { config, migrated }; +} + +function getVersionedConfigFile(raw: unknown): VersionedAskConfigFile { + if (!(raw && typeof raw === "object")) { + throw new AskConfigVersionMigrationError( + "Ask config must be an object", + "invalid_or_unsupported" + ); + } + const schemaVersion = (raw as { schemaVersion?: unknown }).schemaVersion; + if (!Number.isInteger(schemaVersion) || typeof schemaVersion !== "number") { + throw new AskConfigVersionMigrationError( + "Ask config must include an integer schemaVersion", + "invalid_or_unsupported" + ); + } + return raw as VersionedAskConfigFile; +} diff --git a/src/config/migrations/types.ts b/src/config/migrations/types.ts new file mode 100644 index 0000000..8efbbd6 --- /dev/null +++ b/src/config/migrations/types.ts @@ -0,0 +1,10 @@ +export interface VersionedAskConfigFile { + schemaVersion: number; + [key: string]: unknown; +} + +export interface AskConfigMigration { + from: number; + migrate: (config: VersionedAskConfigFile) => VersionedAskConfigFile; + to: number; +} diff --git a/src/config/path-migrations.ts b/src/config/path-migrations.ts new file mode 100644 index 0000000..798305f --- /dev/null +++ b/src/config/path-migrations.ts @@ -0,0 +1,56 @@ +import { mkdir, readFile, rename } from "node:fs/promises"; +import { dirname } from "node:path"; + +export interface AskConfigPathMigrationOptions { + currentPath: string; + legacyPaths: string[]; +} + +export async function migrateAskConfigPathIfNeeded({ + currentPath, + legacyPaths, +}: AskConfigPathMigrationOptions): Promise { + if (await pathExists(currentPath)) { + return; + } + + const legacyPath = await findFirstExistingPath(legacyPaths); + if (!legacyPath) { + return; + } + + await mkdir(dirname(currentPath), { recursive: true }); + await rename(legacyPath, currentPath); +} + +async function findFirstExistingPath( + paths: string[] +): Promise { + for (const path of paths) { + if (await pathExists(path)) { + return path; + } + } + return; +} + +async function pathExists(path: string): Promise { + try { + await readFile(path, "utf-8"); + return true; + } catch (error) { + if (isMissingFileError(error)) { + return false; + } + throw error; + } +} + +function isMissingFileError(error: unknown): boolean { + return ( + !!error && + typeof error === "object" && + "code" in error && + error.code === "ENOENT" + ); +} diff --git a/src/config/store.ts b/src/config/store.ts index 1997788..38fdf0c 100644 --- a/src/config/store.ts +++ b/src/config/store.ts @@ -10,6 +10,7 @@ import { toAskConfigFileV1, } from "./defaults.ts"; import { AskConfigMigrationError, migrateAskConfig } from "./migrate.ts"; +import { migrateAskConfigPathIfNeeded } from "./path-migrations.ts"; import type { AskConfig } from "./schema.ts"; export interface AskConfigNotice { @@ -28,15 +29,13 @@ export class AskConfigStore { private notice?: AskConfigNotice; private readonly listeners = new Set<(config: AskConfig) => void>(); private readonly configPath: string; - private readonly legacyConfigPath?: string; + private readonly legacyConfigPaths: string[]; - constructor( - configPath = getAskConfigPath(), - legacyConfigPath = getLegacyAskConfigPath() - ) { - this.configPath = configPath; - this.legacyConfigPath = - legacyConfigPath === configPath ? undefined : legacyConfigPath; + constructor(configPath?: string, legacyConfigPaths?: string[]) { + this.configPath = configPath ?? getAskConfigPath(); + this.legacyConfigPaths = ( + legacyConfigPaths ?? (configPath ? [] : getLegacyAskConfigPaths()) + ).filter((path) => path !== this.configPath); } subscribe(onChange: (config: AskConfig) => void): () => void { @@ -148,17 +147,10 @@ export class AskConfigStore { } private async migrateLegacyConfigIfNeeded(): Promise { - if (!this.legacyConfigPath) { - return; - } - if (await pathExists(this.configPath)) { - return; - } - if (!(await pathExists(this.legacyConfigPath))) { - return; - } - await mkdir(dirname(this.configPath), { recursive: true }); - await rename(this.legacyConfigPath, this.configPath); + await migrateAskConfigPathIfNeeded({ + currentPath: this.configPath, + legacyPaths: this.legacyConfigPaths, + }); } } @@ -170,11 +162,11 @@ export function getAskConfigStore(): AskConfigStore { } export function getAskConfigPath(): string { - return join(getAgentDir(), "eko24ive-pi-ask.json"); + return join(getAgentDir(), "extensions", "eko24ive-pi-ask.json"); } -export function getLegacyAskConfigPath(): string { - return join(getAgentDir(), "extensions", "eko24ive-pi-ask.json"); +export function getLegacyAskConfigPaths(): string[] { + return [join(getAgentDir(), "eko24ive-pi-ask.json")]; } function createBackupPath(path: string, date: Date): string { @@ -182,18 +174,6 @@ function createBackupPath(path: string, date: Date): string { return path.replace(JSON_EXTENSION_PATTERN, `.${timestamp}.bak.json`); } -async function pathExists(path: string): Promise { - try { - await readFile(path, "utf-8"); - return true; - } catch (error) { - if (isMissingFileError(error)) { - return false; - } - throw error; - } -} - function isMissingFileError(error: unknown): boolean { return ( !!error && diff --git a/tests/config-migrate.test.ts b/tests/config-migrate.test.ts new file mode 100644 index 0000000..6b3db96 --- /dev/null +++ b/tests/config-migrate.test.ts @@ -0,0 +1,42 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { DEFAULT_ASK_CONFIG } from "../src/config/defaults.ts"; +import { migrateAskConfig } from "../src/config/migrate.ts"; +import { CURRENT_ASK_CONFIG_SCHEMA_VERSION } from "../src/config/migrations/index.ts"; + +const INVALID_CONFIG_PATTERN = /Config was invalid or unsupported/; + +const currentConfigFile = { + schemaVersion: CURRENT_ASK_CONFIG_SCHEMA_VERSION, + behaviour: DEFAULT_ASK_CONFIG.behaviour, + keymaps: DEFAULT_ASK_CONFIG.keymaps, +}; + +test("config migration framework accepts the current schema version", () => { + const result = migrateAskConfig(currentConfigFile); + + assert.equal(result.migrated, false); + assert.deepEqual(result.config, DEFAULT_ASK_CONFIG); +}); + +test("config migration framework rejects unsupported future schema versions", () => { + assert.throws( + () => + migrateAskConfig({ + ...currentConfigFile, + schemaVersion: CURRENT_ASK_CONFIG_SCHEMA_VERSION + 1, + }), + INVALID_CONFIG_PATTERN + ); +}); + +test("config migration framework rejects unversioned config files", () => { + assert.throws( + () => + migrateAskConfig({ + behaviour: DEFAULT_ASK_CONFIG.behaviour, + keymaps: DEFAULT_ASK_CONFIG.keymaps, + }), + INVALID_CONFIG_PATTERN + ); +}); diff --git a/tests/config-store.test.ts b/tests/config-store.test.ts index 6481fc4..4ed131b 100644 --- a/tests/config-store.test.ts +++ b/tests/config-store.test.ts @@ -111,13 +111,12 @@ test("config store loads current config version without rewriting", async () => await rm(dirname(path), { force: true, recursive: true }); }); -test("config store migrates the legacy extensions config path", async () => { +test("config store migrates the legacy root config path into extensions", async () => { const root = await import("node:fs/promises").then(({ mkdtemp }) => mkdtemp(join(tmpdir(), "pi-ask-config-legacy-")) ); - const path = join(root, "eko24ive-pi-ask.json"); - const legacyPath = join(root, "extensions", "eko24ive-pi-ask.json"); - await mkdir(dirname(legacyPath), { recursive: true }); + const path = join(root, "extensions", "eko24ive-pi-ask.json"); + const legacyPath = join(root, "eko24ive-pi-ask.json"); await writeFile( legacyPath, JSON.stringify({ @@ -131,7 +130,7 @@ test("config store migrates the legacy extensions config path", async () => { keymaps: DEFAULT_ASK_CONFIG.keymaps, }) ); - const store = new AskConfigStore(path, legacyPath); + const store = new AskConfigStore(path, [legacyPath]); const result = await store.ensureLoaded(); @@ -156,6 +155,52 @@ test("config store migrates the legacy extensions config path", async () => { await rm(root, { force: true, recursive: true }); }); +test("config store leaves legacy root config when extensions config exists", async () => { + const root = await import("node:fs/promises").then(({ mkdtemp }) => + mkdtemp(join(tmpdir(), "pi-ask-config-conflict-")) + ); + const path = join(root, "extensions", "eko24ive-pi-ask.json"); + const legacyPath = join(root, "eko24ive-pi-ask.json"); + await mkdir(dirname(path), { recursive: true }); + await writeFile( + path, + JSON.stringify({ + schemaVersion: 1, + behaviour: { + autoSubmitWhenAnsweredWithoutNotes: false, + confirmDismissWhenDirty: true, + doublePressReviewShortcuts: true, + showFooterHints: true, + }, + keymaps: DEFAULT_ASK_CONFIG.keymaps, + }) + ); + await writeFile( + legacyPath, + JSON.stringify({ + schemaVersion: 1, + behaviour: { + autoSubmitWhenAnsweredWithoutNotes: true, + confirmDismissWhenDirty: true, + doublePressReviewShortcuts: true, + showFooterHints: false, + }, + keymaps: DEFAULT_ASK_CONFIG.keymaps, + }) + ); + const store = new AskConfigStore(path, [legacyPath]); + + const result = await store.ensureLoaded(); + + assert.equal( + result.config.behaviour.autoSubmitWhenAnsweredWithoutNotes, + false + ); + assert.equal(result.config.behaviour.showFooterHints, true); + assert.ok(await readFile(legacyPath, "utf-8")); + await rm(root, { force: true, recursive: true }); +}); + test("config store falls back only keymaps when configured keymaps are invalid", async () => { const path = await makeTempPath("pi-ask-config-invalid-keymaps-"); await mkdir(dirname(path), { recursive: true });