feat: bootstrap ask config on first use
This commit is contained in:
@@ -71,7 +71,8 @@ The codebase is split so the implementation reads through file boundaries and na
|
|||||||
- editor lifecycle stays in the controller, not in the reducers
|
- editor lifecycle stays in the controller, not in the reducers
|
||||||
- persisted ask settings are versioned and normalized before use
|
- persisted ask settings are versioned and normalized before use
|
||||||
- invalid persisted keymaps fall back to default keymaps for the current session without discarding valid behaviour settings
|
- invalid persisted keymaps fall back to default keymaps for the current session without discarding valid behaviour settings
|
||||||
- settings modal edits a local draft and only persists on explicit save
|
- settings modal edits a local draft and only persists modal changes on explicit save
|
||||||
|
- when the ask config file is missing, the first ask use writes a default persisted config snapshot
|
||||||
- live config updates can affect an in-progress ask flow immediately
|
- live config updates can affect an in-progress ask flow immediately
|
||||||
|
|
||||||
## Documentation rule
|
## Documentation rule
|
||||||
|
|||||||
+15
-3
@@ -12,7 +12,9 @@ When changing pi-ask settings:
|
|||||||
|
|
||||||
Default path:
|
Default path:
|
||||||
|
|
||||||
`~/.pi/agent/extensions/eko24ive-pi-ask.json`
|
`~/.pi/agent/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.
|
||||||
|
|
||||||
## Config shape
|
## Config shape
|
||||||
|
|
||||||
@@ -21,7 +23,8 @@ Default path:
|
|||||||
"schemaVersion": 1,
|
"schemaVersion": 1,
|
||||||
"behaviour": {
|
"behaviour": {
|
||||||
"autoSubmitWhenAnsweredWithoutNotes": false,
|
"autoSubmitWhenAnsweredWithoutNotes": false,
|
||||||
"confirmDismissWhenDirty": false,
|
"confirmDismissWhenDirty": true,
|
||||||
|
"doublePressReviewShortcuts": true,
|
||||||
"showFooterHints": true
|
"showFooterHints": true
|
||||||
},
|
},
|
||||||
"keymaps": {
|
"keymaps": {
|
||||||
@@ -46,11 +49,18 @@ Default path:
|
|||||||
### `behaviour.confirmDismissWhenDirty`
|
### `behaviour.confirmDismissWhenDirty`
|
||||||
|
|
||||||
- type: boolean
|
- type: boolean
|
||||||
- default: `false`
|
- default: `true`
|
||||||
- effect: when enabled, discarding a dirty ask flow requires a second cancel/dismiss action
|
- effect: when enabled, discarding a dirty ask flow requires a second cancel/dismiss action
|
||||||
- the warning stays visible until the user changes tabs in the ask flow
|
- the warning stays visible until the user changes tabs in the ask flow
|
||||||
- dirty means there are saved answers/notes or unsaved editor draft text
|
- dirty means there are saved answers/notes or unsaved editor draft text
|
||||||
|
|
||||||
|
### `behaviour.doublePressReviewShortcuts`
|
||||||
|
|
||||||
|
- type: boolean
|
||||||
|
- default: `true`
|
||||||
|
- effect: when enabled, review-tab number shortcuts (`1`, `2`, `3`) only trigger after pressing the same key twice
|
||||||
|
- the review screen shows an inline hint and keeps the pending shortcut armed until another review shortcut is pressed or the user leaves the tab
|
||||||
|
|
||||||
### `behaviour.showFooterHints`
|
### `behaviour.showFooterHints`
|
||||||
|
|
||||||
- type: boolean
|
- type: boolean
|
||||||
@@ -142,6 +152,7 @@ These are intentionally not configurable:
|
|||||||
- `tab`, `shift+tab`, `left`, `right` move between tabs
|
- `tab`, `shift+tab`, `left`, `right` move between tabs
|
||||||
- `up`, `down` move between options/actions
|
- `up`, `down` move between options/actions
|
||||||
- `1..9` triggers option/review shortcuts
|
- `1..9` triggers option/review shortcuts
|
||||||
|
- when `behaviour.doublePressReviewShortcuts` is enabled, review-tab shortcuts `1`, `2`, and `3` require the same key twice
|
||||||
- `@` remains file-reference affordance in editors
|
- `@` remains file-reference affordance in editors
|
||||||
|
|
||||||
## Invalid keymaps behavior
|
## Invalid keymaps behavior
|
||||||
@@ -169,6 +180,7 @@ Invalid keymaps include:
|
|||||||
"behaviour": {
|
"behaviour": {
|
||||||
"autoSubmitWhenAnsweredWithoutNotes": true,
|
"autoSubmitWhenAnsweredWithoutNotes": true,
|
||||||
"confirmDismissWhenDirty": true,
|
"confirmDismissWhenDirty": true,
|
||||||
|
"doublePressReviewShortcuts": true,
|
||||||
"showFooterHints": false
|
"showFooterHints": false
|
||||||
},
|
},
|
||||||
"keymaps": {
|
"keymaps": {
|
||||||
|
|||||||
+41
-2
@@ -28,9 +28,15 @@ export class AskConfigStore {
|
|||||||
private notice?: AskConfigNotice;
|
private notice?: AskConfigNotice;
|
||||||
private readonly listeners = new Set<(config: AskConfig) => void>();
|
private readonly listeners = new Set<(config: AskConfig) => void>();
|
||||||
private readonly configPath: string;
|
private readonly configPath: string;
|
||||||
|
private readonly legacyConfigPath?: string;
|
||||||
|
|
||||||
constructor(configPath = getAskConfigPath()) {
|
constructor(
|
||||||
|
configPath = getAskConfigPath(),
|
||||||
|
legacyConfigPath = getLegacyAskConfigPath()
|
||||||
|
) {
|
||||||
this.configPath = configPath;
|
this.configPath = configPath;
|
||||||
|
this.legacyConfigPath =
|
||||||
|
legacyConfigPath === configPath ? undefined : legacyConfigPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
subscribe(onChange: (config: AskConfig) => void): () => void {
|
subscribe(onChange: (config: AskConfig) => void): () => void {
|
||||||
@@ -80,12 +86,15 @@ export class AskConfigStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private async loadFromDisk(): Promise<AskConfigLoadResult> {
|
private async loadFromDisk(): Promise<AskConfigLoadResult> {
|
||||||
|
await this.migrateLegacyConfigIfNeeded();
|
||||||
let content: string;
|
let content: string;
|
||||||
try {
|
try {
|
||||||
content = await readFile(this.configPath, "utf-8");
|
content = await readFile(this.configPath, "utf-8");
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (isMissingFileError(error)) {
|
if (isMissingFileError(error)) {
|
||||||
return { config: normalizeAskConfig(DEFAULT_ASK_CONFIG) };
|
const config = normalizeAskConfig(DEFAULT_ASK_CONFIG);
|
||||||
|
await this.save(config);
|
||||||
|
return { config };
|
||||||
}
|
}
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
@@ -137,6 +146,20 @@ export class AskConfigStore {
|
|||||||
},
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private async migrateLegacyConfigIfNeeded(): Promise<void> {
|
||||||
|
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);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let askConfigStore: AskConfigStore | undefined;
|
let askConfigStore: AskConfigStore | undefined;
|
||||||
@@ -147,6 +170,10 @@ export function getAskConfigStore(): AskConfigStore {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function getAskConfigPath(): string {
|
export function getAskConfigPath(): string {
|
||||||
|
return join(getAgentDir(), "eko24ive-pi-ask.json");
|
||||||
|
}
|
||||||
|
|
||||||
|
export function getLegacyAskConfigPath(): string {
|
||||||
return join(getAgentDir(), "extensions", "eko24ive-pi-ask.json");
|
return join(getAgentDir(), "extensions", "eko24ive-pi-ask.json");
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -155,6 +182,18 @@ function createBackupPath(path: string, date: Date): string {
|
|||||||
return path.replace(JSON_EXTENSION_PATTERN, `.${timestamp}.bak.json`);
|
return path.replace(JSON_EXTENSION_PATTERN, `.${timestamp}.bak.json`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async function pathExists(path: string): Promise<boolean> {
|
||||||
|
try {
|
||||||
|
await readFile(path, "utf-8");
|
||||||
|
return true;
|
||||||
|
} catch (error) {
|
||||||
|
if (isMissingFileError(error)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function isMissingFileError(error: unknown): boolean {
|
function isMissingFileError(error: unknown): boolean {
|
||||||
return (
|
return (
|
||||||
!!error &&
|
!!error &&
|
||||||
|
|||||||
+68
-13
@@ -1,7 +1,7 @@
|
|||||||
import assert from "node:assert/strict";
|
import assert from "node:assert/strict";
|
||||||
import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||||
import { tmpdir } from "node:os";
|
import { tmpdir } from "node:os";
|
||||||
import { join } from "node:path";
|
import { dirname, join } from "node:path";
|
||||||
import test from "node:test";
|
import test from "node:test";
|
||||||
import { DEFAULT_ASK_CONFIG } from "../src/config/defaults.ts";
|
import { DEFAULT_ASK_CONFIG } from "../src/config/defaults.ts";
|
||||||
import { AskConfigStore } from "../src/config/store.ts";
|
import { AskConfigStore } from "../src/config/store.ts";
|
||||||
@@ -13,18 +13,22 @@ async function makeTempPath(name: string): Promise<string> {
|
|||||||
const root = await import("node:fs/promises").then(({ mkdtemp }) =>
|
const root = await import("node:fs/promises").then(({ mkdtemp }) =>
|
||||||
mkdtemp(join(tmpdir(), name))
|
mkdtemp(join(tmpdir(), name))
|
||||||
);
|
);
|
||||||
return join(root, "extensions", "eko24ive-pi-ask.json");
|
return join(root, "eko24ive-pi-ask.json");
|
||||||
}
|
}
|
||||||
|
|
||||||
test("config store uses in-memory defaults when file is missing", async () => {
|
test("config store writes defaults when file is missing", async () => {
|
||||||
const path = await makeTempPath("pi-ask-config-missing-");
|
const path = await makeTempPath("pi-ask-config-missing-");
|
||||||
const store = new AskConfigStore(path);
|
const store = new AskConfigStore(path);
|
||||||
|
|
||||||
const result = await store.ensureLoaded();
|
const result = await store.ensureLoaded();
|
||||||
|
|
||||||
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
||||||
await assert.rejects(readFile(path, "utf-8"));
|
assert.deepEqual(JSON.parse(await readFile(path, "utf-8")), {
|
||||||
await rm(join(path, "..", ".."), { force: true, recursive: true });
|
schemaVersion: 1,
|
||||||
|
behaviour: DEFAULT_ASK_CONFIG.behaviour,
|
||||||
|
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||||
|
});
|
||||||
|
await rm(dirname(path), { force: true, recursive: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
test("config store writes full normalized config on save", async () => {
|
test("config store writes full normalized config on save", async () => {
|
||||||
@@ -35,6 +39,7 @@ test("config store writes full normalized config on save", async () => {
|
|||||||
behaviour: {
|
behaviour: {
|
||||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||||
confirmDismissWhenDirty: true,
|
confirmDismissWhenDirty: true,
|
||||||
|
doublePressReviewShortcuts: true,
|
||||||
showFooterHints: false,
|
showFooterHints: false,
|
||||||
},
|
},
|
||||||
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||||
@@ -46,22 +51,23 @@ test("config store writes full normalized config on save", async () => {
|
|||||||
behaviour: {
|
behaviour: {
|
||||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||||
confirmDismissWhenDirty: true,
|
confirmDismissWhenDirty: true,
|
||||||
|
doublePressReviewShortcuts: true,
|
||||||
showFooterHints: false,
|
showFooterHints: false,
|
||||||
},
|
},
|
||||||
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||||
});
|
});
|
||||||
await rm(join(path, "..", ".."), { force: true, recursive: true });
|
await rm(dirname(path), { force: true, recursive: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
test("config store backs up broken json and loads defaults", async () => {
|
test("config store backs up broken json and loads defaults", async () => {
|
||||||
const path = await makeTempPath("pi-ask-config-broken-");
|
const path = await makeTempPath("pi-ask-config-broken-");
|
||||||
await mkdir(join(path, ".."), { recursive: true });
|
await mkdir(dirname(path), { recursive: true });
|
||||||
await writeFile(path, "{bad json", "utf-8");
|
await writeFile(path, "{bad json", "utf-8");
|
||||||
const store = new AskConfigStore(path);
|
const store = new AskConfigStore(path);
|
||||||
|
|
||||||
const result = await store.ensureLoaded();
|
const result = await store.ensureLoaded();
|
||||||
const dirEntries = await import("node:fs/promises").then(({ readdir }) =>
|
const dirEntries = await import("node:fs/promises").then(({ readdir }) =>
|
||||||
readdir(join(path, ".."))
|
readdir(dirname(path))
|
||||||
);
|
);
|
||||||
|
|
||||||
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
||||||
@@ -71,12 +77,12 @@ test("config store backs up broken json and loads defaults", async () => {
|
|||||||
);
|
);
|
||||||
assert(dirEntries.some((entry) => entry.includes(".bak.json")));
|
assert(dirEntries.some((entry) => entry.includes(".bak.json")));
|
||||||
await assert.rejects(readFile(path, "utf-8"));
|
await assert.rejects(readFile(path, "utf-8"));
|
||||||
await rm(join(path, "..", ".."), { force: true, recursive: true });
|
await rm(dirname(path), { force: true, recursive: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
test("config store loads current config version without rewriting", async () => {
|
test("config store loads current config version without rewriting", async () => {
|
||||||
const path = await makeTempPath("pi-ask-config-current-");
|
const path = await makeTempPath("pi-ask-config-current-");
|
||||||
await mkdir(join(path, ".."), { recursive: true });
|
await mkdir(dirname(path), { recursive: true });
|
||||||
await writeFile(
|
await writeFile(
|
||||||
path,
|
path,
|
||||||
JSON.stringify({
|
JSON.stringify({
|
||||||
@@ -84,6 +90,7 @@ test("config store loads current config version without rewriting", async () =>
|
|||||||
behaviour: {
|
behaviour: {
|
||||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||||
confirmDismissWhenDirty: true,
|
confirmDismissWhenDirty: true,
|
||||||
|
doublePressReviewShortcuts: true,
|
||||||
showFooterHints: false,
|
showFooterHints: false,
|
||||||
},
|
},
|
||||||
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||||
@@ -98,14 +105,60 @@ test("config store loads current config version without rewriting", async () =>
|
|||||||
true
|
true
|
||||||
);
|
);
|
||||||
assert.equal(result.config.behaviour.confirmDismissWhenDirty, true);
|
assert.equal(result.config.behaviour.confirmDismissWhenDirty, true);
|
||||||
|
assert.equal(result.config.behaviour.doublePressReviewShortcuts, true);
|
||||||
assert.equal(result.config.behaviour.showFooterHints, false);
|
assert.equal(result.config.behaviour.showFooterHints, false);
|
||||||
assert.deepEqual(result.config.keymaps, DEFAULT_ASK_CONFIG.keymaps);
|
assert.deepEqual(result.config.keymaps, DEFAULT_ASK_CONFIG.keymaps);
|
||||||
await rm(join(path, "..", ".."), { force: true, recursive: true });
|
await rm(dirname(path), { force: true, recursive: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
test("config store migrates the legacy extensions config path", 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 });
|
||||||
|
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,
|
||||||
|
true
|
||||||
|
);
|
||||||
|
assert.equal(result.config.behaviour.confirmDismissWhenDirty, true);
|
||||||
|
assert.equal(result.config.behaviour.doublePressReviewShortcuts, true);
|
||||||
|
assert.equal(result.config.behaviour.showFooterHints, false);
|
||||||
|
await assert.rejects(readFile(legacyPath, "utf-8"));
|
||||||
|
assert.deepEqual(JSON.parse(await readFile(path, "utf-8")), {
|
||||||
|
schemaVersion: 1,
|
||||||
|
behaviour: {
|
||||||
|
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||||
|
confirmDismissWhenDirty: true,
|
||||||
|
doublePressReviewShortcuts: true,
|
||||||
|
showFooterHints: false,
|
||||||
|
},
|
||||||
|
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||||
|
});
|
||||||
|
await rm(root, { force: true, recursive: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
test("config store falls back only keymaps when configured keymaps are invalid", async () => {
|
test("config store falls back only keymaps when configured keymaps are invalid", async () => {
|
||||||
const path = await makeTempPath("pi-ask-config-invalid-keymaps-");
|
const path = await makeTempPath("pi-ask-config-invalid-keymaps-");
|
||||||
await mkdir(join(path, ".."), { recursive: true });
|
await mkdir(dirname(path), { recursive: true });
|
||||||
await writeFile(
|
await writeFile(
|
||||||
path,
|
path,
|
||||||
JSON.stringify({
|
JSON.stringify({
|
||||||
@@ -113,6 +166,7 @@ test("config store falls back only keymaps when configured keymaps are invalid",
|
|||||||
behaviour: {
|
behaviour: {
|
||||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||||
confirmDismissWhenDirty: true,
|
confirmDismissWhenDirty: true,
|
||||||
|
doublePressReviewShortcuts: true,
|
||||||
showFooterHints: false,
|
showFooterHints: false,
|
||||||
},
|
},
|
||||||
keymaps: {
|
keymaps: {
|
||||||
@@ -134,8 +188,9 @@ test("config store falls back only keymaps when configured keymaps are invalid",
|
|||||||
true
|
true
|
||||||
);
|
);
|
||||||
assert.equal(result.config.behaviour.confirmDismissWhenDirty, true);
|
assert.equal(result.config.behaviour.confirmDismissWhenDirty, true);
|
||||||
|
assert.equal(result.config.behaviour.doublePressReviewShortcuts, true);
|
||||||
assert.equal(result.config.behaviour.showFooterHints, false);
|
assert.equal(result.config.behaviour.showFooterHints, false);
|
||||||
assert.deepEqual(result.config.keymaps, DEFAULT_ASK_CONFIG.keymaps);
|
assert.deepEqual(result.config.keymaps, DEFAULT_ASK_CONFIG.keymaps);
|
||||||
assert.match(result.notice?.text ?? "", DEFAULT_KEYMAPS_NOTICE_PATTERN);
|
assert.match(result.notice?.text ?? "", DEFAULT_KEYMAPS_NOTICE_PATTERN);
|
||||||
await rm(join(path, "..", ".."), { force: true, recursive: true });
|
await rm(dirname(path), { force: true, recursive: true });
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user