fix(coding-agent/config): support comments in JSON config migration
This commit is contained in:
@@ -11,6 +11,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed active `/goal` mode being paused by internal compaction and session-switch lifecycle aborts, and made those switches persist wall-clock goal usage without charging time spent in another session to a preserved goal.
|
||||
### Fixed
|
||||
|
||||
- Fixed legacy `settings.json`, `models.json`, and `keybindings.json` migration/loading to accept comments in JSON config files.
|
||||
|
||||
## [16.0.8] - 2026-06-18
|
||||
|
||||
|
||||
@@ -40,7 +40,7 @@ function migrateJsonToYml(jsonPath: string, ymlPath: string) {
|
||||
}
|
||||
|
||||
const content = fs.readFileSync(jsonPath, "utf-8");
|
||||
const parsed = JSON.parse(content);
|
||||
const parsed = JSONC.parse(content);
|
||||
if (!parsed) {
|
||||
logger.warn("migrateJsonToYml: invalid json structure", { path: jsonPath });
|
||||
migratedPaths.add(key);
|
||||
|
||||
@@ -10,7 +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";
|
||||
import { JSONC, YAML } from "bun";
|
||||
|
||||
/**
|
||||
* Application-level keybindings (coding agent specific).
|
||||
@@ -381,7 +381,7 @@ function loadRawConfig(filePath: string): unknown {
|
||||
try {
|
||||
const content = fs.readFileSync(filePath, "utf-8");
|
||||
if (filePath.endsWith(".json")) {
|
||||
return JSON.parse(content);
|
||||
return JSONC.parse(content);
|
||||
}
|
||||
if (filePath.endsWith(".yml") || filePath.endsWith(".yaml")) {
|
||||
return YAML.parse(content);
|
||||
|
||||
@@ -24,7 +24,7 @@ import {
|
||||
procmgr,
|
||||
setDefaultTabWidth,
|
||||
} from "@oh-my-pi/pi-utils";
|
||||
import { YAML } from "bun";
|
||||
import { JSONC, YAML } from "bun";
|
||||
import { type Settings as SettingsCapabilityItem, settingsCapability } from "../capability/settings";
|
||||
import type { ModelRole } from "../config/model-roles";
|
||||
import { loadCapability } from "../discovery";
|
||||
@@ -668,9 +668,9 @@ export class Settings {
|
||||
// 1. Migrate from settings.json
|
||||
const settingsJsonPath = path.join(this.#agentDir, "settings.json");
|
||||
try {
|
||||
const parsed = JSON.parse(await Bun.file(settingsJsonPath).text());
|
||||
const parsed: unknown = JSONC.parse(await Bun.file(settingsJsonPath).text());
|
||||
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) {
|
||||
settings = this.#deepMerge(settings, this.#migrateRawSettings(parsed));
|
||||
settings = this.#deepMerge(settings, this.#migrateRawSettings(parsed as RawSettings));
|
||||
migrated = true;
|
||||
try {
|
||||
fs.renameSync(settingsJsonPath, `${settingsJsonPath}.bak`);
|
||||
|
||||
@@ -55,6 +55,43 @@ describe("KeybindingsManager.create", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("migrates legacy keybinding JSON with comments to YAML during create", async () => {
|
||||
const agentDir = await fs.mkdtemp(path.join(os.tmpdir(), "pi-keybindings-"));
|
||||
const jsonPath = path.join(agentDir, "keybindings.json");
|
||||
const ymlPath = path.join(agentDir, "keybindings.yml");
|
||||
|
||||
await Bun.write(
|
||||
jsonPath,
|
||||
`{
|
||||
// Legacy config files may contain comments from hand-edited examples.
|
||||
"fork": "ctrl+f",
|
||||
"selectConfirm": "enter",
|
||||
"cursorUp": "ctrl+p",
|
||||
"app.clipboard.copyPrompt": ["alt+c", "ctrl+shift+c"]
|
||||
}
|
||||
`,
|
||||
);
|
||||
|
||||
try {
|
||||
const manager = KeybindingsManager.create(agentDir);
|
||||
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"]);
|
||||
expect(manager.getKeys("tui.editor.cursorUp")).toEqual(["ctrl+p"]);
|
||||
expect(manager.getKeys("app.clipboard.copyPrompt")).toEqual(["alt+c", "ctrl+shift+c"]);
|
||||
expect(writtenConfig).toEqual({
|
||||
"app.clipboard.copyPrompt": ["alt+c", "ctrl+shift+c"],
|
||||
"app.session.fork": "ctrl+f",
|
||||
"tui.editor.cursorUp": "ctrl+p",
|
||||
"tui.select.confirm": "enter",
|
||||
});
|
||||
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");
|
||||
|
||||
@@ -69,4 +69,23 @@ describe("ModelRegistry.create() factory (F6)", () => {
|
||||
const mtime2 = fs.statSync(yml).mtimeMs;
|
||||
expect(mtime2).toBe(mtime1);
|
||||
});
|
||||
|
||||
test("ConfigFile migrates legacy models.json containing comments", async () => {
|
||||
const yml = path.join(tempDir.path(), "models.yml");
|
||||
const json = path.join(tempDir.path(), "models.json");
|
||||
await Bun.write(
|
||||
json,
|
||||
`{
|
||||
// Custom models comment
|
||||
"providers": {
|
||||
/* Block comment */
|
||||
}
|
||||
}`,
|
||||
);
|
||||
|
||||
const cf = new ConfigFile("models", ModelsConfigSchema, yml);
|
||||
const result = cf.tryLoad();
|
||||
expect(result.status).toBe("ok");
|
||||
expect(fs.existsSync(yml)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -494,5 +494,24 @@ describe("Settings", () => {
|
||||
|
||||
expect(fs.readFileSync(path.join(agentDir, "last-changelog-version"), "utf8")).toBe("0.41.0");
|
||||
});
|
||||
|
||||
it("migrates from settings.json containing comments", async () => {
|
||||
const jsonPath = path.join(agentDir, "settings.json");
|
||||
await fs.promises.writeFile(
|
||||
jsonPath,
|
||||
`{
|
||||
// This is a comment
|
||||
"display": {
|
||||
/* Multiline comment */
|
||||
"showTokenUsage": true
|
||||
}
|
||||
}`,
|
||||
);
|
||||
|
||||
const settings = await Settings.init({ cwd: projectDir, agentDir });
|
||||
expect(settings.get("display.showTokenUsage")).toBe(true);
|
||||
expect(fs.existsSync(jsonPath)).toBe(false);
|
||||
expect(fs.existsSync(`${jsonPath}.bak`)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user