fix: avoid destructive ask config writes
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
# @eko24ive/pi-ask
|
||||
|
||||
[](https://www.npmjs.com/package/@eko24ive/pi-ask)
|
||||
[](https://github.com/eko24ive/pi-ask/commits/main)
|
||||
[](https://github.com/eko24ive/pi-ask/commits/main)
|
||||
[](https://github.com/eko24ive/pi-ask/stargazers)
|
||||
|
||||
`@eko24ive/pi-ask` is an ask tool that cares about your answers.
|
||||
@@ -115,7 +115,7 @@ Fixed bindings:
|
||||
|
||||
Review-tab shortcuts can optionally require the same number key twice via `behaviour.doublePressReviewShortcuts`. `behaviour.presentSingleAsMulti` can render future single-select questions as multi-select while preserving the requested type in results; use `main.changeQuestionType` (`t` by default) to change the active question type live.
|
||||
|
||||
You can edit the config file yourself, ask pi to edit it for you, or use `/ask-settings` to find the exact config path, toggle behaviour/notification settings, or reset config to defaults with a guarded double press.
|
||||
You can edit the config file yourself, ask pi to edit it for you, or use `/ask-settings` to find the exact config path, toggle behaviour/notification settings, or reset config to defaults with a guarded double press. pi-ask treats the config file as user-owned: load-time migrations and invalid files are handled in memory without rewriting or backing up the file, and read-only/externally managed configs fail gracefully with a manual-edit message.
|
||||
|
||||
```json
|
||||
{
|
||||
@@ -186,7 +186,7 @@ Accepted notation follows pi-tui key ids. Common aliases are normalized, for exa
|
||||
|
||||
After installation, the extension registers the `ask_user` tool plus `/ask-settings`, `/answer`, `/answer:again`, and `/ask:replay` commands.
|
||||
|
||||
Agents can auto-discover and call `ask_user` when they need clarification instead of guessing. In interactive sessions, it opens a terminal UI flow for structured answers, supports native pi-style `@` file references while typing answers or notes, and returns normalized answers back to the agent. Ask settings are available both from `?` in the ask flow and from the `/ask-settings` command. Behaviour and notification settings are binary `on`/`off` toggles that save immediately, and the settings overlay includes a guarded double-press reset-to-defaults action; keymaps, notification channels, and extraction settings are changed by editing the shown config file path.
|
||||
Agents can auto-discover and call `ask_user` when they need clarification instead of guessing. In interactive sessions, it opens a terminal UI flow for structured answers, supports native pi-style `@` file references while typing answers or notes, and returns normalized answers back to the agent. Ask settings are available both from `?` in the ask flow and from the `/ask-settings` command. Behaviour and notification settings are binary `on`/`off` toggles that save immediately when the config file is writable; save failures revert the toggle and show a manual-edit message. The settings overlay includes a guarded double-press reset-to-defaults action; keymaps, notification channels, and extraction settings are changed by editing the shown config file path.
|
||||
|
||||
### Answer and replay commands
|
||||
|
||||
|
||||
@@ -38,8 +38,7 @@ The codebase is split so the implementation reads through file boundaries and na
|
||||
- `src/config/schema.ts` — persisted config schema and runtime type
|
||||
- `src/config/migrate.ts` — persisted-file validation boundary, including schema migration and context-aware 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
|
||||
- `src/config/store.ts` — non-destructive current/legacy config discovery, load, save, notice, and runtime subscription store
|
||||
|
||||
### UI
|
||||
|
||||
@@ -75,16 +74,17 @@ 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 migrated to the current schema version, validated, and normalized before use
|
||||
- persisted ask settings are migrated to the current schema version in memory, validated, and normalized before use without rewriting the config file on load
|
||||
- config schema migrations preserve user-provided values and add new defaults only when fields are absent
|
||||
- replay payload lookup scans only the current session branch and revalidates payloads before use
|
||||
- invalid persisted keymaps fall back to default keymaps for the current session without discarding valid behaviour, notification, or answer settings
|
||||
- invalid notification channels are skipped and fall back to the default bell channel if none are valid
|
||||
- ask settings behaviour and notification enabled changes persist immediately from the settings list; config reset is guarded by a short double-press confirmation
|
||||
- ask settings behaviour and notification enabled changes attempt to persist immediately from the settings list; save failures revert the change and show an error; config reset is guarded by a short double-press confirmation
|
||||
- `presentSingleAsMulti` is applied at ask-flow creation; toggling it does not rewrite already-normalized questions in an open flow
|
||||
- `main.changeQuestionType` changes the active question type live (non-preview: `single <-> multi`; preview: `preview <-> multi`) and may require confirmation before destructive multi-to-single conversion
|
||||
- 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
|
||||
- when the ask config file is missing, the first ask use attempts to write a default persisted config snapshot under `~/.pi/agent/extensions/`; if writing fails, built-in defaults are used for the session
|
||||
- legacy root config files are read as a fallback only when the current config file is absent; disk is left untouched
|
||||
- invalid config files are left untouched; defaults are loaded for the session with a notice
|
||||
- live config updates can affect an in-progress ask flow immediately
|
||||
|
||||
## Documentation rule
|
||||
|
||||
@@ -8,21 +8,23 @@ When changing pi-ask settings:
|
||||
2. Validate keymaps against the rules below.
|
||||
3. Run `/reload` or restart pi so the new config is picked up.
|
||||
|
||||
pi-ask treats this file as user-owned config. It does not rewrite or back up the file just because it was loaded, migrated, or found invalid. Settings toggled in `/ask-settings` are saved only when the file is writable; if saving fails, pi-ask reverts the toggle and shows the config path to edit manually.
|
||||
|
||||
## Config file path
|
||||
|
||||
Default path:
|
||||
|
||||
`~/.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.
|
||||
If the file does not exist yet, pi-ask attempts to create it with the current default settings the first time the ask flow is used. If the config location is read-only or managed outside pi-ask, pi-ask uses built-in defaults for the session and leaves disk unchanged.
|
||||
|
||||
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.
|
||||
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 reads the legacy file as a fallback and leaves disk unchanged. 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.
|
||||
`schemaVersion` identifies the persisted config shape. pi-ask migrates older supported schema versions forward in memory before validation. Migrations preserve existing user-provided values and add new fields from defaults when needed, but loading a config does not rewrite the file.
|
||||
|
||||
Unsupported future versions or invalid files are backed up and defaults are loaded.
|
||||
Unsupported future versions or invalid files are left unchanged and defaults are loaded for the current session. Fix the file, then run `/reload` or restart pi.
|
||||
|
||||
## Config shape
|
||||
|
||||
|
||||
+3
-3
@@ -213,7 +213,7 @@ This document defines the stable external behavior. It does not explain internal
|
||||
- `/ask:replay` command to replay the latest real `ask_user` form on the current branch
|
||||
- ask settings list with binary behaviour/notification toggles and a guarded reset-to-defaults action
|
||||
- `?` in the ask flow and `/ask-settings` in pi open the same lightweight ask settings overlay
|
||||
- settings persist immediately when changed: `Auto-submit when answered without notes`, `Confirm dismiss when dirty`, `Double-press review shortcuts`, `Notifications`, and `Show footer hints`; `Present single-select as multi-select` persists immediately but applies only to new/replayed ask flows; resetting config to defaults requires pressing the reset action twice within a short confirmation window
|
||||
- settings attempt to persist immediately when changed: `Auto-submit when answered without notes`, `Confirm dismiss when dirty`, `Double-press review shortcuts`, `Notifications`, and `Show footer hints`; `Present single-select as multi-select` persists immediately when saving succeeds but applies only to new/replayed ask flows; save failures revert the setting and show a manual-edit message; resetting config to defaults requires pressing the reset action twice within a short confirmation window
|
||||
- `Keymaps` is a persisted, context-aware config section for global, main-flow, editor, note-editor, and settings-modal actions
|
||||
- the settings list shows the absolute config file path for customizing keymaps, notifications, and extraction settings
|
||||
- if the flow is already on the review tab, all questions are answered, and no notes exist, enabling auto-submit can complete the current ask flow immediately
|
||||
@@ -248,7 +248,7 @@ Settings modal:
|
||||
|
||||
- `settingsModal.close` closes settings; defaults: `Esc`, `Ctrl+C`, `?`
|
||||
- `settingsModal.nextOption` / `settingsModal.previousOption` move between settings; defaults: `Down`, `Up`
|
||||
- `settingsModal.toggle` toggles the highlighted setting and saves immediately; on the reset action, the same binding must be pressed twice within a short confirmation window; defaults: `Enter`, `Space`
|
||||
- `settingsModal.toggle` toggles the highlighted setting and attempts to save immediately; if saving fails, the setting reverts and an error is shown; on the reset action, the same binding must be pressed twice within a short confirmation window; defaults: `Enter`, `Space`
|
||||
|
||||
Dirty dismiss:
|
||||
|
||||
@@ -261,7 +261,7 @@ If `ctx.hasUI === false`, the tool returns a `Needs user input` message in `cont
|
||||
|
||||
Validation is handled inside the tool so malformed calls produce the same structured error shape as other invalid payloads instead of relying on pre-execution schema failures.
|
||||
|
||||
The ask flow subscribes to runtime settings updates while open. In practice, this means changing `Auto-submit when answered without notes`, `Confirm dismiss when dirty`, `Double-press review shortcuts`, `Notifications`, `Show footer hints`, resetting config to defaults, or reloading config-backed keymaps can affect the in-progress ask flow immediately instead of only future asks. `Present single-select as multi-select` is applied when an ask flow is created and does not rewrite question semantics for an already-open flow; use `main.changeQuestionType` for live per-question changes.
|
||||
The ask flow subscribes to runtime settings updates while open. In practice, this means changing `Auto-submit when answered without notes`, `Confirm dismiss when dirty`, `Double-press review shortcuts`, `Notifications`, `Show footer hints`, resetting config to defaults, or reloading config-backed keymaps can affect the in-progress ask flow immediately instead of only future asks when the change is saved or otherwise applied in memory. Load-time migrations and invalid config handling do not rewrite, rename, or back up the config file; invalid files load defaults for the session and show a notice. `Present single-select as multi-select` is applied when an ask flow is created and does not rewrite question semantics for an already-open flow; use `main.changeQuestionType` for live per-question changes.
|
||||
|
||||
## Notifications
|
||||
|
||||
|
||||
@@ -1,56 +0,0 @@
|
||||
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<void> {
|
||||
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<string | undefined> {
|
||||
for (const path of paths) {
|
||||
if (await pathExists(path)) {
|
||||
return path;
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
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 {
|
||||
return (
|
||||
!!error &&
|
||||
typeof error === "object" &&
|
||||
"code" in error &&
|
||||
error.code === "ENOENT"
|
||||
);
|
||||
}
|
||||
+79
-40
@@ -1,8 +1,6 @@
|
||||
import { mkdir, readFile, rename, writeFile } from "node:fs/promises";
|
||||
import { mkdir, readFile, writeFile } from "node:fs/promises";
|
||||
import { dirname, join } from "node:path";
|
||||
|
||||
const JSON_EXTENSION_PATTERN = /\.json$/u;
|
||||
|
||||
import { getAgentDir } from "@earendil-works/pi-coding-agent";
|
||||
import {
|
||||
DEFAULT_ASK_CONFIG,
|
||||
@@ -10,9 +8,13 @@ import {
|
||||
toAskConfigFileV5,
|
||||
} from "./defaults.ts";
|
||||
import { AskConfigMigrationError, migrateAskConfig } from "./migrate.ts";
|
||||
import { migrateAskConfigPathIfNeeded } from "./path-migrations.ts";
|
||||
import type { AskConfig } from "./schema.ts";
|
||||
|
||||
const INVALID_CONFIG_NOTICE =
|
||||
"Config was invalid or unsupported. Loaded defaults for this session and left the config file unchanged. Edit the config file or run /reload after fixing it.";
|
||||
const MIGRATION_FAILED_NOTICE =
|
||||
"Config migration failed. Loaded defaults for this session and left the config file unchanged. Edit the config file or run /reload after fixing it.";
|
||||
|
||||
export interface AskConfigNotice {
|
||||
kind: "error" | "warning" | "success";
|
||||
text: string;
|
||||
@@ -70,8 +72,12 @@ export class AskConfigStore {
|
||||
null,
|
||||
2
|
||||
).concat("\n");
|
||||
try {
|
||||
await mkdir(dirname(this.configPath), { recursive: true });
|
||||
await writeFile(this.configPath, content, "utf-8");
|
||||
} catch (error) {
|
||||
throw createConfigSaveError(this.configPath, error);
|
||||
}
|
||||
this.setConfig(normalized);
|
||||
return normalized;
|
||||
}
|
||||
@@ -85,33 +91,47 @@ export class AskConfigStore {
|
||||
}
|
||||
|
||||
private async loadFromDisk(): Promise<AskConfigLoadResult> {
|
||||
await this.migrateLegacyConfigIfNeeded();
|
||||
let content: string;
|
||||
try {
|
||||
content = await readFile(this.configPath, "utf-8");
|
||||
} catch (error) {
|
||||
if (isMissingFileError(error)) {
|
||||
const content = await this.readDiskConfig();
|
||||
if (content === undefined) {
|
||||
return this.loadMissingConfig();
|
||||
}
|
||||
|
||||
const parsed = parseJson(content);
|
||||
if (!parsed.ok) {
|
||||
return this.loadDefaultsWithNotice(INVALID_CONFIG_NOTICE);
|
||||
}
|
||||
|
||||
return this.loadParsedConfig(parsed.value);
|
||||
}
|
||||
|
||||
private async readDiskConfig(): Promise<string | undefined> {
|
||||
for (const path of [this.configPath, ...this.legacyConfigPaths]) {
|
||||
const content = await readConfigFileIfPresent(path);
|
||||
if (content !== undefined) {
|
||||
return content;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private async loadMissingConfig(): Promise<AskConfigLoadResult> {
|
||||
const config = normalizeAskConfig(DEFAULT_ASK_CONFIG);
|
||||
try {
|
||||
await this.save(config);
|
||||
return { config };
|
||||
} catch (saveError) {
|
||||
return {
|
||||
config,
|
||||
notice: {
|
||||
kind: "warning",
|
||||
text: getErrorMessage(saveError),
|
||||
},
|
||||
};
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(content);
|
||||
} catch {
|
||||
return this.backupAndReset(
|
||||
"Config was invalid or unsupported. Backed it up and loaded defaults. Change any behaviour setting or edit the config file to save a fresh config."
|
||||
);
|
||||
}
|
||||
|
||||
private loadParsedConfig(parsed: unknown): AskConfigLoadResult {
|
||||
try {
|
||||
const migrated = migrateAskConfig(parsed);
|
||||
if (migrated.migrated) {
|
||||
await this.save(migrated.config);
|
||||
}
|
||||
return {
|
||||
config: migrated.config,
|
||||
notice: migrated.notice
|
||||
@@ -123,20 +143,17 @@ export class AskConfigStore {
|
||||
};
|
||||
} catch (error) {
|
||||
if (error instanceof AskConfigMigrationError) {
|
||||
return this.backupAndReset(
|
||||
return this.loadDefaultsWithNotice(
|
||||
error.reason === "migration_failed"
|
||||
? "Config migration failed. Backed up old config and loaded defaults. Change any behaviour setting or edit the config file to save a fresh config."
|
||||
: "Config was invalid or unsupported. Backed it up and loaded defaults. Change any behaviour setting or edit the config file to save a fresh config."
|
||||
? MIGRATION_FAILED_NOTICE
|
||||
: INVALID_CONFIG_NOTICE
|
||||
);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
private async backupAndReset(text: string): Promise<AskConfigLoadResult> {
|
||||
const backupPath = createBackupPath(this.configPath, new Date());
|
||||
await mkdir(dirname(this.configPath), { recursive: true });
|
||||
await rename(this.configPath, backupPath);
|
||||
private loadDefaultsWithNotice(text: string): AskConfigLoadResult {
|
||||
return {
|
||||
config: normalizeAskConfig(DEFAULT_ASK_CONFIG),
|
||||
notice: {
|
||||
@@ -145,13 +162,6 @@ export class AskConfigStore {
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
private async migrateLegacyConfigIfNeeded(): Promise<void> {
|
||||
await migrateAskConfigPathIfNeeded({
|
||||
currentPath: this.configPath,
|
||||
legacyPaths: this.legacyConfigPaths,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
let askConfigStore: AskConfigStore | undefined;
|
||||
@@ -169,9 +179,38 @@ export function getLegacyAskConfigPaths(): string[] {
|
||||
return [join(getAgentDir(), "eko24ive-pi-ask.json")];
|
||||
}
|
||||
|
||||
function createBackupPath(path: string, date: Date): string {
|
||||
const timestamp = date.toISOString().replaceAll(":", "-").replace(/\./g, "-");
|
||||
return path.replace(JSON_EXTENSION_PATTERN, `.${timestamp}.bak.json`);
|
||||
function createConfigSaveError(path: string, error: unknown): Error {
|
||||
const detail = getErrorMessage(error);
|
||||
return new Error(
|
||||
`Unable to save ask config at ${path}. The file may be read-only or managed outside pi-ask; edit it manually and run /reload. ${detail}`
|
||||
);
|
||||
}
|
||||
|
||||
function parseJson(
|
||||
content: string
|
||||
): { ok: true; value: unknown } | { ok: false } {
|
||||
try {
|
||||
return { ok: true, value: JSON.parse(content) };
|
||||
} catch {
|
||||
return { ok: false };
|
||||
}
|
||||
}
|
||||
|
||||
async function readConfigFileIfPresent(
|
||||
path: string
|
||||
): Promise<string | undefined> {
|
||||
try {
|
||||
return await readFile(path, "utf-8");
|
||||
} catch (error) {
|
||||
if (isMissingFileError(error)) {
|
||||
return;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
function getErrorMessage(error: unknown): string {
|
||||
return error instanceof Error ? error.message : String(error);
|
||||
}
|
||||
|
||||
function isMissingFileError(error: unknown): boolean {
|
||||
|
||||
+13
-7
@@ -65,12 +65,14 @@ interface AskFlowOptions {
|
||||
type AskFlowParams = AskParams &
|
||||
Pick<ExtensionContext, "cwd"> & {
|
||||
config: AskConfig;
|
||||
configNotice?: string;
|
||||
ctx: ExtensionContext;
|
||||
flowOptions: AskFlowOptions;
|
||||
};
|
||||
|
||||
interface AskFlowController {
|
||||
config: AskConfig;
|
||||
configNotice?: string;
|
||||
ctx: ExtensionContext;
|
||||
dismissNotice?: string;
|
||||
done: Done;
|
||||
@@ -91,7 +93,7 @@ export async function runAskFlow(
|
||||
options: AskFlowOptions = {}
|
||||
): Promise<AskResult> {
|
||||
const store = getAskConfigStore();
|
||||
const config = await store.getConfig();
|
||||
const { config, notice } = await store.ensureLoaded();
|
||||
const flowOptions = {
|
||||
...options,
|
||||
presentSingleAsMulti:
|
||||
@@ -101,6 +103,7 @@ export async function runAskFlow(
|
||||
createAskFlowController(args, {
|
||||
...params,
|
||||
config,
|
||||
configNotice: notice?.text,
|
||||
cwd: ctx.cwd,
|
||||
ctx,
|
||||
flowOptions,
|
||||
@@ -119,6 +122,7 @@ function createAskFlowController(
|
||||
) {
|
||||
const controller: AskFlowController = {
|
||||
config: params.config,
|
||||
configNotice: params.configNotice,
|
||||
ctx: params.ctx,
|
||||
dismissNotice: undefined,
|
||||
done,
|
||||
@@ -137,6 +141,7 @@ function createAskFlowController(
|
||||
|
||||
controller.unsubscribeConfig = getAskConfigStore().subscribe((config) => {
|
||||
controller.config = config;
|
||||
controller.configNotice = undefined;
|
||||
controller.state = maybeAutoSubmitState(
|
||||
controller.state,
|
||||
controller.config
|
||||
@@ -172,7 +177,7 @@ function renderController(
|
||||
return renderAskScreen({
|
||||
config: controller.config,
|
||||
editor: controller.editor,
|
||||
footerNotice: getDismissNotice(controller),
|
||||
footerNotice: getFooterNotice(controller),
|
||||
reviewShortcutHint: getActiveReviewShortcutHint(controller),
|
||||
state: controller.state,
|
||||
theme: controller.theme,
|
||||
@@ -378,7 +383,7 @@ function commitState(
|
||||
options: { finish?: boolean; syncSelection?: boolean } = {}
|
||||
) {
|
||||
if (nextState.activeTabIndex !== controller.state.activeTabIndex) {
|
||||
clearDismissNotice(controller);
|
||||
clearFooterNotices(controller);
|
||||
}
|
||||
controller.suppressAutoInputForSelection = false;
|
||||
controller.state = nextState;
|
||||
@@ -397,7 +402,7 @@ function submitEditor(controller: AskFlowController, value: string) {
|
||||
controller.suppressAutoInputForSelection = false;
|
||||
const nextState = submitEditorDraft(controller.state, value);
|
||||
if (nextState.activeTabIndex !== controller.state.activeTabIndex) {
|
||||
clearDismissNotice(controller);
|
||||
clearFooterNotices(controller);
|
||||
}
|
||||
controller.state = nextState;
|
||||
syncSelection(controller);
|
||||
@@ -447,7 +452,8 @@ function shouldRequestDismissConfirmation(
|
||||
});
|
||||
}
|
||||
|
||||
function clearDismissNotice(controller: AskFlowController) {
|
||||
function clearFooterNotices(controller: AskFlowController) {
|
||||
controller.configNotice = undefined;
|
||||
controller.dismissNotice = undefined;
|
||||
}
|
||||
|
||||
@@ -505,8 +511,8 @@ function handleReviewShortcutNumber(
|
||||
return true;
|
||||
}
|
||||
|
||||
function getDismissNotice(controller: AskFlowController): string | undefined {
|
||||
return controller.dismissNotice;
|
||||
function getFooterNotice(controller: AskFlowController): string | undefined {
|
||||
return controller.dismissNotice ?? controller.configNotice;
|
||||
}
|
||||
|
||||
function showSettingsModal(controller: AskFlowController) {
|
||||
|
||||
@@ -121,8 +121,8 @@ export class AskSettingsList {
|
||||
private error?: string;
|
||||
private focusIndex = 0;
|
||||
private resetConfirmUntil = 0;
|
||||
private notice?: AskConfigNotice;
|
||||
private readonly configPath: string;
|
||||
private readonly notice?: AskConfigNotice;
|
||||
private readonly onClose: () => void;
|
||||
private readonly onSave: (config: AskConfig) => Promise<AskConfig>;
|
||||
private readonly theme: Theme;
|
||||
@@ -264,11 +264,14 @@ export class AskSettingsList {
|
||||
}
|
||||
|
||||
private appendNotice(lines: string[], innerWidth: number): void {
|
||||
if (this.notice?.kind === "error") {
|
||||
if (this.notice) {
|
||||
lines.push(this.line("", innerWidth));
|
||||
this.appendWrapped(
|
||||
lines,
|
||||
this.theme.fg("warning", this.notice.text),
|
||||
this.theme.fg(
|
||||
this.notice.kind === "success" ? "accent" : "warning",
|
||||
this.notice.text
|
||||
),
|
||||
innerWidth
|
||||
);
|
||||
}
|
||||
@@ -407,6 +410,7 @@ export class AskSettingsList {
|
||||
this.onSave(nextConfig)
|
||||
.then((savedConfig) => {
|
||||
this.config = savedConfig;
|
||||
this.notice = undefined;
|
||||
this.tui.requestRender();
|
||||
})
|
||||
.catch((error: unknown) => {
|
||||
|
||||
+51
-26
@@ -1,13 +1,16 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { mkdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { mkdir, readdir, readFile, rm, writeFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
import test from "node:test";
|
||||
import { DEFAULT_ASK_CONFIG } from "../src/config/defaults.ts";
|
||||
import type { AskConfig } from "../src/config/schema.ts";
|
||||
import { AskConfigStore } from "../src/config/store.ts";
|
||||
|
||||
const DEFAULT_KEYMAPS_NOTICE_PATTERN =
|
||||
/Using default ask keymaps for this session/;
|
||||
const SAVE_FAILURE_PATTERN =
|
||||
/Unable to save ask config .* managed outside pi-ask/;
|
||||
|
||||
function expectedConfigFile(
|
||||
overrides: { behaviour?: typeof DEFAULT_ASK_CONFIG.behaviour } = {}
|
||||
@@ -42,6 +45,28 @@ test("config store writes defaults when file is missing", async () => {
|
||||
await rm(dirname(path), { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store uses defaults when initial config creation fails", async () => {
|
||||
class FailingInitialSaveStore extends AskConfigStore {
|
||||
override save(_config: AskConfig | Partial<AskConfig>): Promise<AskConfig> {
|
||||
return Promise.reject(
|
||||
new Error("Unable to save ask config; edit it manually.")
|
||||
);
|
||||
}
|
||||
}
|
||||
const path = await makeTempPath("pi-ask-config-initial-save-failure-");
|
||||
const store = new FailingInitialSaveStore(path);
|
||||
|
||||
const result = await store.ensureLoaded();
|
||||
|
||||
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
||||
assert.equal(result.notice?.kind, "warning");
|
||||
assert.equal(
|
||||
result.notice?.text,
|
||||
"Unable to save ask config; edit it manually."
|
||||
);
|
||||
await rm(dirname(path), { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store writes full normalized config on save", async () => {
|
||||
const path = await makeTempPath("pi-ask-config-save-");
|
||||
const store = new AskConfigStore(path);
|
||||
@@ -69,33 +94,41 @@ test("config store writes full normalized config on save", async () => {
|
||||
await rm(dirname(path), { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store backs up broken json and loads defaults", async () => {
|
||||
test("config store reports friendly save failures", async () => {
|
||||
const path = await makeTempPath("pi-ask-config-save-failure-");
|
||||
await mkdir(path, { recursive: true });
|
||||
const store = new AskConfigStore(path);
|
||||
|
||||
await assert.rejects(store.save(DEFAULT_ASK_CONFIG), SAVE_FAILURE_PATTERN);
|
||||
await rm(path, { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store leaves broken json unchanged and loads defaults", async () => {
|
||||
const path = await makeTempPath("pi-ask-config-broken-");
|
||||
await mkdir(dirname(path), { recursive: true });
|
||||
await writeFile(path, "{bad json", "utf-8");
|
||||
const store = new AskConfigStore(path);
|
||||
|
||||
const result = await store.ensureLoaded();
|
||||
const dirEntries = await import("node:fs/promises").then(({ readdir }) =>
|
||||
readdir(dirname(path))
|
||||
);
|
||||
const dirEntries = await readdir(dirname(path));
|
||||
|
||||
assert.deepEqual(result.config, DEFAULT_ASK_CONFIG);
|
||||
assert.equal(
|
||||
result.notice?.text,
|
||||
"Config was invalid or unsupported. Backed it up and loaded defaults. Change any behaviour setting or edit the config file to save a fresh config."
|
||||
"Config was invalid or unsupported. Loaded defaults for this session and left the config file unchanged. Edit the config file or run /reload after fixing it."
|
||||
);
|
||||
assert(dirEntries.some((entry) => entry.includes(".bak.json")));
|
||||
await assert.rejects(readFile(path, "utf-8"));
|
||||
assert.equal(
|
||||
dirEntries.some((entry) => entry.includes(".bak.json")),
|
||||
false
|
||||
);
|
||||
assert.equal(await readFile(path, "utf-8"), "{bad json");
|
||||
await rm(dirname(path), { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store loads current config version without rewriting", async () => {
|
||||
test("config store loads migrated config without rewriting", async () => {
|
||||
const path = await makeTempPath("pi-ask-config-current-");
|
||||
await mkdir(dirname(path), { recursive: true });
|
||||
await writeFile(
|
||||
path,
|
||||
JSON.stringify({
|
||||
const content = JSON.stringify({
|
||||
schemaVersion: 1,
|
||||
behaviour: {
|
||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||
@@ -104,8 +137,8 @@ test("config store loads current config version without rewriting", async () =>
|
||||
showFooterHints: false,
|
||||
},
|
||||
keymaps: DEFAULT_ASK_CONFIG.keymaps,
|
||||
})
|
||||
);
|
||||
});
|
||||
await writeFile(path, content);
|
||||
const store = new AskConfigStore(path);
|
||||
|
||||
const result = await store.ensureLoaded();
|
||||
@@ -118,10 +151,11 @@ test("config store loads current config version without rewriting", async () =>
|
||||
assert.equal(result.config.behaviour.doublePressReviewShortcuts, true);
|
||||
assert.equal(result.config.behaviour.showFooterHints, false);
|
||||
assert.deepEqual(result.config.keymaps, DEFAULT_ASK_CONFIG.keymaps);
|
||||
assert.equal(await readFile(path, "utf-8"), content);
|
||||
await rm(dirname(path), { force: true, recursive: true });
|
||||
});
|
||||
|
||||
test("config store migrates the legacy root config path into extensions", async () => {
|
||||
test("config store reads legacy root config without copying it", async () => {
|
||||
const root = await import("node:fs/promises").then(({ mkdtemp }) =>
|
||||
mkdtemp(join(tmpdir(), "pi-ask-config-legacy-"))
|
||||
);
|
||||
@@ -151,17 +185,8 @@ test("config store migrates the legacy root config path into extensions", async
|
||||
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")),
|
||||
expectedConfigFile({
|
||||
behaviour: {
|
||||
...DEFAULT_ASK_CONFIG.behaviour,
|
||||
autoSubmitWhenAnsweredWithoutNotes: true,
|
||||
showFooterHints: false,
|
||||
},
|
||||
})
|
||||
);
|
||||
assert.ok(await readFile(legacyPath, "utf-8"));
|
||||
await assert.rejects(readFile(path, "utf-8"));
|
||||
await rm(root, { force: true, recursive: true });
|
||||
});
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ import test from "node:test";
|
||||
import { visibleWidth } from "@earendil-works/pi-tui";
|
||||
import { DEFAULT_ASK_CONFIG } from "../src/config/defaults.ts";
|
||||
import type { AskConfig } from "../src/config/schema.ts";
|
||||
import type { AskConfigNotice } from "../src/config/store.ts";
|
||||
import { AskSettingsList } from "../src/ui/settings-list.ts";
|
||||
|
||||
const savedConfig: AskConfig = {
|
||||
@@ -36,6 +37,7 @@ function plainTheme() {
|
||||
|
||||
function createList(
|
||||
options: {
|
||||
notice?: AskConfigNotice;
|
||||
onClose?: () => void;
|
||||
onSave?: (config: AskConfig) => Promise<AskConfig>;
|
||||
savedConfig?: AskConfig;
|
||||
@@ -48,7 +50,7 @@ function createList(
|
||||
});
|
||||
return new AskSettingsList(plainTheme(), {
|
||||
configPath: "/tmp/eko24ive-pi-ask.json",
|
||||
notice: undefined,
|
||||
notice: options.notice,
|
||||
onClose,
|
||||
onSave: options.onSave ?? ((config) => Promise.resolve(config)),
|
||||
savedConfig: options.savedConfig ?? savedConfig,
|
||||
@@ -130,6 +132,38 @@ test("settings list shows save failures and reverts the toggle", async () => {
|
||||
assert(text.includes("[off]"));
|
||||
});
|
||||
|
||||
test("settings list renders load warnings", () => {
|
||||
const list = createList({
|
||||
notice: {
|
||||
kind: "warning",
|
||||
text: "Unable to save ask config; edit it manually.",
|
||||
},
|
||||
});
|
||||
|
||||
const text = list.render(72).join("\n");
|
||||
|
||||
assert(text.includes("Unable to save ask config; edit it manually."));
|
||||
});
|
||||
|
||||
test("settings list clears load warnings after successful save", async () => {
|
||||
const list = createList({
|
||||
notice: {
|
||||
kind: "warning",
|
||||
text: "Unable to save ask config; edit it manually.",
|
||||
},
|
||||
});
|
||||
|
||||
list.handleInput(" ");
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
const text = list.render(72).join("\n");
|
||||
|
||||
assert.equal(
|
||||
text.includes("Unable to save ask config; edit it manually."),
|
||||
false
|
||||
);
|
||||
});
|
||||
|
||||
test("settings list uses configured navigation and close keys", async () => {
|
||||
let saved: AskConfig | undefined;
|
||||
const customConfig: AskConfig = {
|
||||
|
||||
Reference in New Issue
Block a user