refactor(coding-agent): improve settings storage semantics and error handling

This commit is contained in:
Mario Zechner 2026-02-17 00:08:32 +01:00
parent 5133697bc4
commit de2736bad0
7 changed files with 386 additions and 152 deletions

View file

@ -2,6 +2,22 @@
## [Unreleased] ## [Unreleased]
### Breaking Changes
- `SettingsManager` persistence semantics changed for SDK consumers. Setters now update in-memory state immediately and queue disk writes. Code that requires durable on-disk settings must call `await settingsManager.flush()`.
### Added
- Added `SettingsManager.drainErrors()` for caller-controlled settings I/O error handling without manager-side console output.
### Changed
- `SettingsManager` now uses scoped storage abstraction with per-scope locked read/merge/write persistence for global and project settings.
### Fixed
- Fixed project settings persistence to preserve unrelated external edits via merge-on-write, while still applying in-memory changes for modified keys.
## [0.52.12] - 2026-02-13 ## [0.52.12] - 2026-02-13
### Added ### Added

View file

@ -700,6 +700,13 @@ Settings load from two locations and merge:
Project overrides global. Nested objects merge keys. Setters modify global settings by default. Project overrides global. Nested objects merge keys. Setters modify global settings by default.
**Persistence and error handling semantics:**
- Settings getters/setters are synchronous for in-memory state.
- Setters enqueue persistence writes asynchronously.
- Call `await settingsManager.flush()` when you need a durability boundary (for example, before process exit or before asserting file contents in tests).
- `SettingsManager` does not print settings I/O errors. Use `settingsManager.drainErrors()` and report them in your app layer.
> See [examples/sdk/10-settings.ts](../examples/sdk/10-settings.ts) > See [examples/sdk/10-settings.ts](../examples/sdk/10-settings.ts)
## ResourceLoader ## ResourceLoader

View file

@ -24,6 +24,19 @@ await createAgentSession({
console.log("Session created with custom settings"); console.log("Session created with custom settings");
// Setters update memory immediately and queue persistence writes.
// Call flush() when you need a durability boundary.
settingsManager.setDefaultThinkingLevel("low");
await settingsManager.flush();
// Surface settings I/O errors at the app layer.
const settingsErrors = settingsManager.drainErrors();
if (settingsErrors.length > 0) {
for (const { scope, error } of settingsErrors) {
console.warn(`Warning (${scope} settings): ${error.message}`);
}
}
// For testing without file I/O: // For testing without file I/O:
const inMemorySettings = SettingsManager.inMemory({ const inMemorySettings = SettingsManager.inMemory({
compaction: { enabled: false }, compaction: { enabled: false },

View file

@ -1,6 +1,7 @@
import type { Transport } from "@mariozechner/pi-ai"; import type { Transport } from "@mariozechner/pi-ai";
import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "fs";
import { dirname, join } from "path"; import { dirname, join } from "path";
import lockfile from "proper-lockfile";
import { CONFIG_DIR_NAME, getAgentDir } from "../config.js"; import { CONFIG_DIR_NAME, getAgentDir } from "../config.js";
export interface CompactionSettings { export interface CompactionSettings {
@ -123,67 +124,156 @@ function deepMergeSettings(base: Settings, overrides: Settings): Settings {
return result; return result;
} }
export type SettingsScope = "global" | "project";
export interface SettingsStorage {
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void;
}
export interface SettingsError {
scope: SettingsScope;
error: Error;
}
export class FileSettingsStorage implements SettingsStorage {
private globalSettingsPath: string;
private projectSettingsPath: string;
constructor(cwd: string = process.cwd(), agentDir: string = getAgentDir()) {
this.globalSettingsPath = join(agentDir, "settings.json");
this.projectSettingsPath = join(cwd, CONFIG_DIR_NAME, "settings.json");
}
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void {
const path = scope === "global" ? this.globalSettingsPath : this.projectSettingsPath;
const dir = dirname(path);
if (!existsSync(dir)) {
mkdirSync(dir, { recursive: true });
}
let release: (() => void) | undefined;
try {
release = lockfile.lockSync(path, { realpath: false });
const current = existsSync(path) ? readFileSync(path, "utf-8") : undefined;
const next = fn(current);
if (next !== undefined) {
writeFileSync(path, next, "utf-8");
}
} finally {
if (release) {
release();
}
}
}
}
export class InMemorySettingsStorage implements SettingsStorage {
private global: string | undefined;
private project: string | undefined;
withLock(scope: SettingsScope, fn: (current: string | undefined) => string | undefined): void {
const current = scope === "global" ? this.global : this.project;
const next = fn(current);
if (next !== undefined) {
if (scope === "global") {
this.global = next;
} else {
this.project = next;
}
}
}
}
export class SettingsManager { export class SettingsManager {
private settingsPath: string | null; private storage: SettingsStorage;
private projectSettingsPath: string | null;
private globalSettings: Settings; private globalSettings: Settings;
private inMemoryProjectSettings: Settings; // For in-memory mode private projectSettings: Settings;
private settings: Settings; private settings: Settings;
private persist: boolean; private modifiedFields = new Set<keyof Settings>(); // Track global fields modified during session
private modifiedFields = new Set<keyof Settings>(); // Track fields modified during session private modifiedNestedFields = new Map<keyof Settings, Set<string>>(); // Track global nested field modifications
private modifiedNestedFields = new Map<keyof Settings, Set<string>>(); // Track nested field modifications private modifiedProjectFields = new Set<keyof Settings>(); // Track project fields modified during session
private globalSettingsLoadError: Error | null = null; // Track if settings file had parse errors private modifiedProjectNestedFields = new Map<keyof Settings, Set<string>>(); // Track project nested field modifications
private globalSettingsLoadError: Error | null = null; // Track if global settings file had parse errors
private projectSettingsLoadError: Error | null = null; // Track if project settings file had parse errors
private writeQueue: Promise<void> = Promise.resolve();
private errors: SettingsError[];
private constructor( private constructor(
settingsPath: string | null, storage: SettingsStorage,
projectSettingsPath: string | null, initialGlobal: Settings,
initialSettings: Settings, initialProject: Settings,
persist: boolean, globalLoadError: Error | null = null,
loadError: Error | null = null, projectLoadError: Error | null = null,
initialErrors: SettingsError[] = [],
) { ) {
this.settingsPath = settingsPath; this.storage = storage;
this.projectSettingsPath = projectSettingsPath; this.globalSettings = initialGlobal;
this.persist = persist; this.projectSettings = initialProject;
this.globalSettings = initialSettings; this.globalSettingsLoadError = globalLoadError;
this.inMemoryProjectSettings = {}; this.projectSettingsLoadError = projectLoadError;
this.globalSettingsLoadError = loadError; this.errors = [...initialErrors];
const projectSettings = this.loadProjectSettings(); this.settings = deepMergeSettings(this.globalSettings, this.projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
/** Create a SettingsManager that loads from files */ /** Create a SettingsManager that loads from files */
static create(cwd: string = process.cwd(), agentDir: string = getAgentDir()): SettingsManager { static create(cwd: string = process.cwd(), agentDir: string = getAgentDir()): SettingsManager {
const settingsPath = join(agentDir, "settings.json"); const storage = new FileSettingsStorage(cwd, agentDir);
const projectSettingsPath = join(cwd, CONFIG_DIR_NAME, "settings.json"); return SettingsManager.fromStorage(storage);
}
let globalSettings: Settings = {}; /** Create a SettingsManager from an arbitrary storage backend */
let loadError: Error | null = null; static fromStorage(storage: SettingsStorage): SettingsManager {
const globalLoad = SettingsManager.tryLoadFromStorage(storage, "global");
try { const projectLoad = SettingsManager.tryLoadFromStorage(storage, "project");
globalSettings = SettingsManager.loadFromFile(settingsPath); const initialErrors: SettingsError[] = [];
} catch (error) { if (globalLoad.error) {
loadError = error as Error; initialErrors.push({ scope: "global", error: globalLoad.error });
console.error(`Warning: Invalid JSON in ${settingsPath}: ${error}`); }
console.error(`Fix the syntax error to enable settings persistence.`); if (projectLoad.error) {
initialErrors.push({ scope: "project", error: projectLoad.error });
} }
return new SettingsManager(settingsPath, projectSettingsPath, globalSettings, true, loadError); return new SettingsManager(
storage,
globalLoad.settings,
projectLoad.settings,
globalLoad.error,
projectLoad.error,
initialErrors,
);
} }
/** Create an in-memory SettingsManager (no file I/O) */ /** Create an in-memory SettingsManager (no file I/O) */
static inMemory(settings: Partial<Settings> = {}): SettingsManager { static inMemory(settings: Partial<Settings> = {}): SettingsManager {
return new SettingsManager(null, null, settings, false); const storage = new InMemorySettingsStorage();
return new SettingsManager(storage, settings, {});
} }
private static loadFromFile(path: string): Settings { private static loadFromStorage(storage: SettingsStorage, scope: SettingsScope): Settings {
if (!existsSync(path)) { let content: string | undefined;
storage.withLock(scope, (current) => {
content = current;
return undefined;
});
if (!content) {
return {}; return {};
} }
const content = readFileSync(path, "utf-8");
const settings = JSON.parse(content); const settings = JSON.parse(content);
return SettingsManager.migrateSettings(settings); return SettingsManager.migrateSettings(settings);
} }
private static tryLoadFromStorage(
storage: SettingsStorage,
scope: SettingsScope,
): { settings: Settings; error: Error | null } {
try {
return { settings: SettingsManager.loadFromStorage(storage, scope), error: null };
} catch (error) {
return { settings: {}, error: error as Error };
}
}
/** Migrate old settings format to new format */ /** Migrate old settings format to new format */
private static migrateSettings(settings: Record<string, unknown>): Settings { private static migrateSettings(settings: Record<string, unknown>): Settings {
// Migrate queueMode -> steeringMode // Migrate queueMode -> steeringMode
@ -222,55 +312,39 @@ export class SettingsManager {
return settings as Settings; return settings as Settings;
} }
private loadProjectSettings(): Settings {
// In-memory mode: return stored in-memory project settings
if (!this.persist) {
return structuredClone(this.inMemoryProjectSettings);
}
if (!this.projectSettingsPath || !existsSync(this.projectSettingsPath)) {
return {};
}
try {
const content = readFileSync(this.projectSettingsPath, "utf-8");
const settings = JSON.parse(content);
return SettingsManager.migrateSettings(settings);
} catch (error) {
console.error(`Warning: Could not read project settings file: ${error}`);
return {};
}
}
getGlobalSettings(): Settings { getGlobalSettings(): Settings {
return structuredClone(this.globalSettings); return structuredClone(this.globalSettings);
} }
getProjectSettings(): Settings { getProjectSettings(): Settings {
return this.loadProjectSettings(); return structuredClone(this.projectSettings);
} }
reload(): void { reload(): void {
let nextGlobalSettings: Settings | null = null; const globalLoad = SettingsManager.tryLoadFromStorage(this.storage, "global");
if (!globalLoad.error) {
if (this.persist && this.settingsPath) { this.globalSettings = globalLoad.settings;
try { this.globalSettingsLoadError = null;
nextGlobalSettings = SettingsManager.loadFromFile(this.settingsPath); } else {
this.globalSettingsLoadError = null; this.globalSettingsLoadError = globalLoad.error;
} catch (error) { this.recordError("global", globalLoad.error);
this.globalSettingsLoadError = error as Error;
}
}
if (nextGlobalSettings) {
this.globalSettings = nextGlobalSettings;
} }
this.modifiedFields.clear(); this.modifiedFields.clear();
this.modifiedNestedFields.clear(); this.modifiedNestedFields.clear();
this.modifiedProjectFields.clear();
this.modifiedProjectNestedFields.clear();
const projectSettings = this.loadProjectSettings(); const projectLoad = SettingsManager.tryLoadFromStorage(this.storage, "project");
this.settings = deepMergeSettings(this.globalSettings, projectSettings); if (!projectLoad.error) {
this.projectSettings = projectLoad.settings;
this.projectSettingsLoadError = null;
} else {
this.projectSettingsLoadError = projectLoad.error;
this.recordError("project", projectLoad.error);
}
this.settings = deepMergeSettings(this.globalSettings, this.projectSettings);
} }
/** Apply additional overrides on top of current settings */ /** Apply additional overrides on top of current settings */
@ -278,7 +352,7 @@ export class SettingsManager {
this.settings = deepMergeSettings(this.settings, overrides); this.settings = deepMergeSettings(this.settings, overrides);
} }
/** Mark a field as modified during this session */ /** Mark a global field as modified during this session */
private markModified(field: keyof Settings, nestedKey?: string): void { private markModified(field: keyof Settings, nestedKey?: string): void {
this.modifiedFields.add(field); this.modifiedFields.add(field);
if (nestedKey) { if (nestedKey) {
@ -289,80 +363,123 @@ export class SettingsManager {
} }
} }
private save(): void { /** Mark a project field as modified during this session */
if (this.persist && this.settingsPath) { private markProjectModified(field: keyof Settings, nestedKey?: string): void {
// Don't overwrite if the file had parse errors on initial load this.modifiedProjectFields.add(field);
if (this.globalSettingsLoadError) { if (nestedKey) {
// Re-merge to update active settings even though we can't persist if (!this.modifiedProjectNestedFields.has(field)) {
const projectSettings = this.loadProjectSettings(); this.modifiedProjectNestedFields.set(field, new Set());
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
return;
} }
this.modifiedProjectNestedFields.get(field)!.add(nestedKey);
}
}
try { private recordError(scope: SettingsScope, error: unknown): void {
const dir = dirname(this.settingsPath); const normalizedError = error instanceof Error ? error : new Error(String(error));
if (!existsSync(dir)) { this.errors.push({ scope, error: normalizedError });
mkdirSync(dir, { recursive: true }); }
}
// Re-read current file to get latest external changes private clearModifiedScope(scope: SettingsScope): void {
const currentFileSettings = SettingsManager.loadFromFile(this.settingsPath); if (scope === "global") {
this.modifiedFields.clear();
// Start with file settings as base - preserves external edits this.modifiedNestedFields.clear();
const mergedSettings: Settings = { ...currentFileSettings }; return;
// Only override with in-memory values for fields that were explicitly modified during this session
for (const field of this.modifiedFields) {
const value = this.globalSettings[field];
// Handle nested objects specially - merge at nested level to preserve unmodified nested keys
if (this.modifiedNestedFields.has(field) && typeof value === "object" && value !== null) {
const nestedModified = this.modifiedNestedFields.get(field)!;
const baseNested = (currentFileSettings[field] as Record<string, unknown>) ?? {};
const inMemoryNested = value as Record<string, unknown>;
const mergedNested = { ...baseNested };
for (const nestedKey of nestedModified) {
mergedNested[nestedKey] = inMemoryNested[nestedKey];
}
(mergedSettings as Record<string, unknown>)[field] = mergedNested;
} else {
// For top-level primitives and arrays, use the modified value directly
(mergedSettings as Record<string, unknown>)[field] = value;
}
}
this.globalSettings = mergedSettings;
writeFileSync(this.settingsPath, JSON.stringify(this.globalSettings, null, 2), "utf-8");
} catch (error) {
// File may have been externally modified with invalid JSON - don't overwrite
console.error(`Warning: Could not save settings file: ${error}`);
}
} }
// Always re-merge to update active settings (needed for both file and inMemory modes) this.modifiedProjectFields.clear();
const projectSettings = this.loadProjectSettings(); this.modifiedProjectNestedFields.clear();
this.settings = deepMergeSettings(this.globalSettings, projectSettings); }
private enqueueWrite(scope: SettingsScope, task: () => void): void {
this.writeQueue = this.writeQueue
.then(() => {
task();
this.clearModifiedScope(scope);
})
.catch((error) => {
this.recordError(scope, error);
});
}
private cloneModifiedNestedFields(source: Map<keyof Settings, Set<string>>): Map<keyof Settings, Set<string>> {
const snapshot = new Map<keyof Settings, Set<string>>();
for (const [key, value] of source.entries()) {
snapshot.set(key, new Set(value));
}
return snapshot;
}
private persistScopedSettings(
scope: SettingsScope,
snapshotSettings: Settings,
modifiedFields: Set<keyof Settings>,
modifiedNestedFields: Map<keyof Settings, Set<string>>,
): void {
this.storage.withLock(scope, (current) => {
const currentFileSettings = current
? SettingsManager.migrateSettings(JSON.parse(current) as Record<string, unknown>)
: {};
const mergedSettings: Settings = { ...currentFileSettings };
for (const field of modifiedFields) {
const value = snapshotSettings[field];
if (modifiedNestedFields.has(field) && typeof value === "object" && value !== null) {
const nestedModified = modifiedNestedFields.get(field)!;
const baseNested = (currentFileSettings[field] as Record<string, unknown>) ?? {};
const inMemoryNested = value as Record<string, unknown>;
const mergedNested = { ...baseNested };
for (const nestedKey of nestedModified) {
mergedNested[nestedKey] = inMemoryNested[nestedKey];
}
(mergedSettings as Record<string, unknown>)[field] = mergedNested;
} else {
(mergedSettings as Record<string, unknown>)[field] = value;
}
}
return JSON.stringify(mergedSettings, null, 2);
});
}
private save(): void {
this.settings = deepMergeSettings(this.globalSettings, this.projectSettings);
if (this.globalSettingsLoadError) {
return;
}
const snapshotGlobalSettings = structuredClone(this.globalSettings);
const modifiedFields = new Set(this.modifiedFields);
const modifiedNestedFields = this.cloneModifiedNestedFields(this.modifiedNestedFields);
this.enqueueWrite("global", () => {
this.persistScopedSettings("global", snapshotGlobalSettings, modifiedFields, modifiedNestedFields);
});
} }
private saveProjectSettings(settings: Settings): void { private saveProjectSettings(settings: Settings): void {
// In-memory mode: store in memory this.projectSettings = structuredClone(settings);
if (!this.persist) { this.settings = deepMergeSettings(this.globalSettings, this.projectSettings);
this.inMemoryProjectSettings = structuredClone(settings);
if (this.projectSettingsLoadError) {
return; return;
} }
if (!this.projectSettingsPath) { const snapshotProjectSettings = structuredClone(this.projectSettings);
return; const modifiedFields = new Set(this.modifiedProjectFields);
} const modifiedNestedFields = this.cloneModifiedNestedFields(this.modifiedProjectNestedFields);
try { this.enqueueWrite("project", () => {
const dir = dirname(this.projectSettingsPath); this.persistScopedSettings("project", snapshotProjectSettings, modifiedFields, modifiedNestedFields);
if (!existsSync(dir)) { });
mkdirSync(dir, { recursive: true }); }
}
writeFileSync(this.projectSettingsPath, JSON.stringify(settings, null, 2), "utf-8"); async flush(): Promise<void> {
} catch (error) { await this.writeQueue;
console.error(`Warning: Could not save project settings file: ${error}`); }
}
drainErrors(): SettingsError[] {
const drained = [...this.errors];
this.errors = [];
return drained;
} }
getLastChangelogVersion(): string | undefined { getLastChangelogVersion(): string | undefined {
@ -571,10 +688,10 @@ export class SettingsManager {
} }
setProjectPackages(packages: PackageSource[]): void { setProjectPackages(packages: PackageSource[]): void {
const projectSettings = this.loadProjectSettings(); const projectSettings = structuredClone(this.projectSettings);
projectSettings.packages = packages; projectSettings.packages = packages;
this.markProjectModified("packages");
this.saveProjectSettings(projectSettings); this.saveProjectSettings(projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
getExtensionPaths(): string[] { getExtensionPaths(): string[] {
@ -588,10 +705,10 @@ export class SettingsManager {
} }
setProjectExtensionPaths(paths: string[]): void { setProjectExtensionPaths(paths: string[]): void {
const projectSettings = this.loadProjectSettings(); const projectSettings = structuredClone(this.projectSettings);
projectSettings.extensions = paths; projectSettings.extensions = paths;
this.markProjectModified("extensions");
this.saveProjectSettings(projectSettings); this.saveProjectSettings(projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
getSkillPaths(): string[] { getSkillPaths(): string[] {
@ -605,10 +722,10 @@ export class SettingsManager {
} }
setProjectSkillPaths(paths: string[]): void { setProjectSkillPaths(paths: string[]): void {
const projectSettings = this.loadProjectSettings(); const projectSettings = structuredClone(this.projectSettings);
projectSettings.skills = paths; projectSettings.skills = paths;
this.markProjectModified("skills");
this.saveProjectSettings(projectSettings); this.saveProjectSettings(projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
getPromptTemplatePaths(): string[] { getPromptTemplatePaths(): string[] {
@ -622,10 +739,10 @@ export class SettingsManager {
} }
setProjectPromptTemplatePaths(paths: string[]): void { setProjectPromptTemplatePaths(paths: string[]): void {
const projectSettings = this.loadProjectSettings(); const projectSettings = structuredClone(this.projectSettings);
projectSettings.prompts = paths; projectSettings.prompts = paths;
this.markProjectModified("prompts");
this.saveProjectSettings(projectSettings); this.saveProjectSettings(projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
getThemePaths(): string[] { getThemePaths(): string[] {
@ -639,10 +756,10 @@ export class SettingsManager {
} }
setProjectThemePaths(paths: string[]): void { setProjectThemePaths(paths: string[]): void {
const projectSettings = this.loadProjectSettings(); const projectSettings = structuredClone(this.projectSettings);
projectSettings.themes = paths; projectSettings.themes = paths;
this.markProjectModified("themes");
this.saveProjectSettings(projectSettings); this.saveProjectSettings(projectSettings);
this.settings = deepMergeSettings(this.globalSettings, projectSettings);
} }
getEnableSkillCommands(): boolean { getEnableSkillCommands(): boolean {

View file

@ -55,6 +55,16 @@ async function readPipedStdin(): Promise<string | undefined> {
}); });
} }
function reportSettingsErrors(settingsManager: SettingsManager, context: string): void {
const errors = settingsManager.drainErrors();
for (const { scope, error } of errors) {
console.error(chalk.yellow(`Warning (${context}, ${scope} settings): ${error.message}`));
if (error.stack) {
console.error(chalk.dim(error.stack));
}
}
}
type PackageCommand = "install" | "remove" | "update" | "list"; type PackageCommand = "install" | "remove" | "update" | "list";
interface PackageCommandOptions { interface PackageCommandOptions {
@ -200,6 +210,7 @@ async function handlePackageCommand(args: string[]): Promise<boolean> {
const cwd = process.cwd(); const cwd = process.cwd();
const agentDir = getAgentDir(); const agentDir = getAgentDir();
const settingsManager = SettingsManager.create(cwd, agentDir); const settingsManager = SettingsManager.create(cwd, agentDir);
reportSettingsErrors(settingsManager, "package command");
const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager }); const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager });
packageManager.setProgressCallback((event) => { packageManager.setProgressCallback((event) => {
@ -508,6 +519,7 @@ async function handleConfigCommand(args: string[]): Promise<boolean> {
const cwd = process.cwd(); const cwd = process.cwd();
const agentDir = getAgentDir(); const agentDir = getAgentDir();
const settingsManager = SettingsManager.create(cwd, agentDir); const settingsManager = SettingsManager.create(cwd, agentDir);
reportSettingsErrors(settingsManager, "config command");
const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager }); const packageManager = new DefaultPackageManager({ cwd, agentDir, settingsManager });
const resolvedPaths = await packageManager.resolve(); const resolvedPaths = await packageManager.resolve();
@ -541,6 +553,7 @@ export async function main(args: string[]) {
const cwd = process.cwd(); const cwd = process.cwd();
const agentDir = getAgentDir(); const agentDir = getAgentDir();
const settingsManager = SettingsManager.create(cwd, agentDir); const settingsManager = SettingsManager.create(cwd, agentDir);
reportSettingsErrors(settingsManager, "startup");
const authStorage = new AuthStorage(); const authStorage = new AuthStorage();
const modelRegistry = new ModelRegistry(authStorage, getModelsPath()); const modelRegistry = new ModelRegistry(authStorage, getModelsPath());

View file

@ -34,7 +34,7 @@ describe("SettingsManager - External Edit Preservation", () => {
} }
}); });
it("should preserve file changes to packages array when changing unrelated setting", () => { it("should preserve file changes to packages array when changing unrelated setting", async () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
// Initial state: packages has one item // Initial state: packages has one item
@ -62,6 +62,7 @@ describe("SettingsManager - External Edit Preservation", () => {
// User changes an UNRELATED setting via UI (this triggers save) // User changes an UNRELATED setting via UI (this triggers save)
manager.setTheme("light"); manager.setTheme("light");
await manager.flush();
// With the fix, packages should be preserved as [] (not reverted to startup value) // With the fix, packages should be preserved as [] (not reverted to startup value)
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
@ -70,7 +71,7 @@ describe("SettingsManager - External Edit Preservation", () => {
expect(savedSettings.theme).toBe("light"); expect(savedSettings.theme).toBe("light");
}); });
it("should preserve file changes to extensions array when changing unrelated setting", () => { it("should preserve file changes to extensions array when changing unrelated setting", async () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
writeFileSync( writeFileSync(
@ -90,10 +91,57 @@ describe("SettingsManager - External Edit Preservation", () => {
// Change unrelated setting // Change unrelated setting
manager.setDefaultThinkingLevel("high"); manager.setDefaultThinkingLevel("high");
await manager.flush();
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
// With the fix, extensions should be preserved (not reverted to startup value) // With the fix, extensions should be preserved (not reverted to startup value)
expect(savedSettings.extensions).toEqual(["/new/extension.ts"]); expect(savedSettings.extensions).toEqual(["/new/extension.ts"]);
}); });
it("should preserve external project settings changes when updating unrelated project field", async () => {
const projectSettingsPath = join(projectDir, ".pi", "settings.json");
writeFileSync(
projectSettingsPath,
JSON.stringify({
extensions: ["./old-extension.ts"],
prompts: ["./old-prompt.md"],
}),
);
const manager = SettingsManager.create(projectDir, agentDir);
const currentProjectSettings = JSON.parse(readFileSync(projectSettingsPath, "utf-8"));
currentProjectSettings.prompts = ["./new-prompt.md"];
writeFileSync(projectSettingsPath, JSON.stringify(currentProjectSettings, null, 2));
manager.setProjectExtensionPaths(["./updated-extension.ts"]);
await manager.flush();
const savedProjectSettings = JSON.parse(readFileSync(projectSettingsPath, "utf-8"));
expect(savedProjectSettings.prompts).toEqual(["./new-prompt.md"]);
expect(savedProjectSettings.extensions).toEqual(["./updated-extension.ts"]);
});
it("should let in-memory project changes override external changes for the same project field", async () => {
const projectSettingsPath = join(projectDir, ".pi", "settings.json");
writeFileSync(
projectSettingsPath,
JSON.stringify({
extensions: ["./initial-extension.ts"],
}),
);
const manager = SettingsManager.create(projectDir, agentDir);
const currentProjectSettings = JSON.parse(readFileSync(projectSettingsPath, "utf-8"));
currentProjectSettings.extensions = ["./external-extension.ts"];
writeFileSync(projectSettingsPath, JSON.stringify(currentProjectSettings, null, 2));
manager.setProjectExtensionPaths(["./in-memory-extension.ts"]);
await manager.flush();
const savedProjectSettings = JSON.parse(readFileSync(projectSettingsPath, "utf-8"));
expect(savedProjectSettings.extensions).toEqual(["./in-memory-extension.ts"]);
});
}); });

View file

@ -24,7 +24,7 @@ describe("SettingsManager", () => {
}); });
describe("preserves externally added settings", () => { describe("preserves externally added settings", () => {
it("should preserve enabledModels when changing thinking level", () => { it("should preserve enabledModels when changing thinking level", async () => {
// Create initial settings file // Create initial settings file
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
writeFileSync( writeFileSync(
@ -45,6 +45,7 @@ describe("SettingsManager", () => {
// User changes thinking level via Shift+Tab // User changes thinking level via Shift+Tab
manager.setDefaultThinkingLevel("high"); manager.setDefaultThinkingLevel("high");
await manager.flush();
// Verify enabledModels is preserved // Verify enabledModels is preserved
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
@ -54,7 +55,7 @@ describe("SettingsManager", () => {
expect(savedSettings.defaultModel).toBe("claude-sonnet"); expect(savedSettings.defaultModel).toBe("claude-sonnet");
}); });
it("should preserve custom settings when changing theme", () => { it("should preserve custom settings when changing theme", async () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
writeFileSync( writeFileSync(
settingsPath, settingsPath,
@ -73,6 +74,7 @@ describe("SettingsManager", () => {
// User changes theme // User changes theme
manager.setTheme("light"); manager.setTheme("light");
await manager.flush();
// Verify all settings preserved // Verify all settings preserved
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
@ -81,7 +83,7 @@ describe("SettingsManager", () => {
expect(savedSettings.theme).toBe("light"); expect(savedSettings.theme).toBe("light");
}); });
it("should let in-memory changes override file changes for same key", () => { it("should let in-memory changes override file changes for same key", async () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
writeFileSync( writeFileSync(
settingsPath, settingsPath,
@ -99,6 +101,7 @@ describe("SettingsManager", () => {
// But then changes it via UI to "high" // But then changes it via UI to "high"
manager.setDefaultThinkingLevel("high"); manager.setDefaultThinkingLevel("high");
await manager.flush();
// In-memory change should win // In-memory change should win
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
@ -193,6 +196,22 @@ describe("SettingsManager", () => {
}); });
}); });
describe("error tracking", () => {
it("should collect and clear load errors via drainErrors", () => {
const globalSettingsPath = join(agentDir, "settings.json");
const projectSettingsPath = join(projectDir, ".pi", "settings.json");
writeFileSync(globalSettingsPath, "{ invalid global json");
writeFileSync(projectSettingsPath, "{ invalid project json");
const manager = SettingsManager.create(projectDir, agentDir);
const errors = manager.drainErrors();
expect(errors).toHaveLength(2);
expect(errors.map((e) => e.scope).sort()).toEqual(["global", "project"]);
expect(manager.drainErrors()).toEqual([]);
});
});
describe("shellCommandPrefix", () => { describe("shellCommandPrefix", () => {
it("should load shellCommandPrefix from settings", () => { it("should load shellCommandPrefix from settings", () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
@ -212,12 +231,13 @@ describe("SettingsManager", () => {
expect(manager.getShellCommandPrefix()).toBeUndefined(); expect(manager.getShellCommandPrefix()).toBeUndefined();
}); });
it("should preserve shellCommandPrefix when saving unrelated settings", () => { it("should preserve shellCommandPrefix when saving unrelated settings", async () => {
const settingsPath = join(agentDir, "settings.json"); const settingsPath = join(agentDir, "settings.json");
writeFileSync(settingsPath, JSON.stringify({ shellCommandPrefix: "shopt -s expand_aliases" })); writeFileSync(settingsPath, JSON.stringify({ shellCommandPrefix: "shopt -s expand_aliases" }));
const manager = SettingsManager.create(projectDir, agentDir); const manager = SettingsManager.create(projectDir, agentDir);
manager.setTheme("light"); manager.setTheme("light");
await manager.flush();
const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8")); const savedSettings = JSON.parse(readFileSync(settingsPath, "utf-8"));
expect(savedSettings.shellCommandPrefix).toBe("shopt -s expand_aliases"); expect(savedSettings.shellCommandPrefix).toBe("shopt -s expand_aliases");