diff --git a/CHANGELOG.md b/CHANGELOG.md index e16f9edf..7f709b8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **Settings preservation during evolve**: `jumbo evolve --yes` now additively inserts missing `.jumbo/settings.jsonc` defaults without overwriting customized known values or unknown user settings, and invalid JSONC settings files fail clearly instead of being partially rewritten. + ## [3.12.0] - 2026-07-04 ### Added diff --git a/docs/getting-started/what-jumbo-creates.md b/docs/getting-started/what-jumbo-creates.md index 145a977a..506535df 100644 --- a/docs/getting-started/what-jumbo-creates.md +++ b/docs/getting-started/what-jumbo-creates.md @@ -106,6 +106,7 @@ The `.jumbo/` directory is **local project memory**. It is not yet designed for ## Configuration Jumbo stores project-level settings in `.jumbo/settings.jsonc`. The file supports comments (JSONC format). +When `jumbo evolve --yes` or settings persistence updates this file, Jumbo preserves explicit values and unknown entries while adding any missing defaults. Default contents: diff --git a/docs/reference/commands/maintenance.md b/docs/reference/commands/maintenance.md index b927bbc0..286bf74d 100644 --- a/docs/reference/commands/maintenance.md +++ b/docs/reference/commands/maintenance.md @@ -72,11 +72,11 @@ Runs the complete installation update workflow in sequence: 3. Convert legacy component-coupling dependencies into relations 4. Refresh `AGENTS.md` and managed agent instruction files 5. Sync Jumbo-managed skills from `assets/skills/` -6. Ensure settings files exist +6. Ensure settings files exist and add missing default settings without overwriting user values 7. Refresh managed harness and hook configuration 8. Rebuild database projections from the event store -The command is designed to be idempotent. Re-running it after a successful evolve should not create duplicate events or duplicate relations. +The command is designed to be idempotent. Re-running it after a successful evolve should not create duplicate events or duplicate relations. Existing `.jumbo/settings.jsonc` files are upgraded additively: missing defaults are inserted, explicit values and unknown entries are preserved, and invalid JSONC fails instead of being partially rewritten. ### Examples diff --git a/src/infrastructure/settings/FsSettingsInitializer.ts b/src/infrastructure/settings/FsSettingsInitializer.ts index 9c589a34..3fd03fb1 100644 --- a/src/infrastructure/settings/FsSettingsInitializer.ts +++ b/src/infrastructure/settings/FsSettingsInitializer.ts @@ -2,12 +2,15 @@ import fs from "fs-extra"; import path from "path"; import { ISettingsInitializer } from "../../application/settings/ISettingsInitializer.js"; import { PlannedFileChange } from "../../application/context/project/init/PlannedFileChange.js"; +import { DEFAULT_SETTINGS } from "./DefaultSettings.js"; +import { JsonObject, applyMissingDefaults, assertValidJsonc } from "./JsoncSettingsEditor.js"; /** - * FsSettingsInitializer - Creates settings file with defaults if it doesn't exist. + * FsSettingsInitializer - Creates settings file with defaults if it doesn't exist, + * and additively fills in any missing default sections or fields if it does. * - * Called during bootstrap to ensure settings infrastructure is ready. - * Only creates the file if missing - doesn't overwrite existing settings. + * Called during bootstrap and evolve to ensure settings infrastructure is current. + * Existing explicit values and unknown entries are always preserved. */ export class FsSettingsInitializer implements ISettingsInitializer { private readonly settingsFilePath: string; @@ -17,8 +20,8 @@ export class FsSettingsInitializer implements ISettingsInitializer { } async ensureSettingsFileExists(): Promise { - // Only create if it doesn't exist if (await fs.pathExists(this.settingsFilePath)) { + await this.applyMissingDefaults(); return; } @@ -70,6 +73,20 @@ export class FsSettingsInitializer implements ISettingsInitializer { await fs.writeFile(this.settingsFilePath, content, "utf-8"); } + /** + * Fill in any missing default sections or fields on an existing settings + * file, leaving explicit values and unknown entries untouched. + */ + private async applyMissingDefaults(): Promise { + const content = await fs.readFile(this.settingsFilePath, "utf-8"); + assertValidJsonc(content, this.settingsFilePath); + + const updated = applyMissingDefaults(content, DEFAULT_SETTINGS as unknown as JsonObject); + if (updated !== content) { + await fs.writeFile(this.settingsFilePath, updated, "utf-8"); + } + } + async getPlannedFileChange(): Promise { if (await fs.pathExists(this.settingsFilePath)) { return null; // No change needed diff --git a/src/infrastructure/settings/FsSettingsReader.ts b/src/infrastructure/settings/FsSettingsReader.ts index 5965478a..48dc643d 100644 --- a/src/infrastructure/settings/FsSettingsReader.ts +++ b/src/infrastructure/settings/FsSettingsReader.ts @@ -4,6 +4,7 @@ import * as jsonc from "jsonc-parser"; import { ISettingsReader } from "../../application/settings/ISettingsReader.js"; import { Settings } from "../../application/settings/Settings.js"; import { DEFAULT_SETTINGS } from "./DefaultSettings.js"; +import { JsonValue, setJsoncValue } from "./JsoncSettingsEditor.js"; /** * JsoncSettingsReader - File system implementation for reading settings. @@ -55,8 +56,56 @@ export class FsSettingsReader implements ISettingsReader { async write(settings: Settings): Promise { const normalizedSettings = this.mergeWithDefaults(settings); - const content = this.buildSettingsFileContent(normalizedSettings); - await fs.writeFile(this.settingsFilePath, content, "utf-8"); + + if (!(await fs.pathExists(this.settingsFilePath))) { + const content = this.buildSettingsFileContent(normalizedSettings); + await fs.writeFile(this.settingsFilePath, content, "utf-8"); + return; + } + + const existingContent = await fs.readFile(this.settingsFilePath, "utf-8"); + const errors: jsonc.ParseError[] = []; + jsonc.parse(existingContent, errors, { allowTrailingComma: true }); + if (errors.length > 0) { + throw new Error( + `Invalid JSON in settings file: ${this.formatParseErrors(errors)}` + ); + } + + let updated = existingContent; + for (const { path: fieldPath, value } of this.knownValueEntries(normalizedSettings)) { + updated = setJsoncValue(updated, fieldPath, value); + } + await fs.writeFile(this.settingsFilePath, updated, "utf-8"); + } + + /** + * Known settings leaf paths and their values, used to persist changes onto + * an existing settings file without disturbing unknown entries elsewhere + * in the document. + */ + private knownValueEntries( + settings: Settings + ): Array<{ path: jsonc.JSONPath; value: JsonValue }> { + return [ + { path: ["project", "id"], value: settings.project!.id }, + { path: ["qa", "defaultTurnLimit"], value: settings.qa.defaultTurnLimit }, + { + path: ["claims", "claimDurationMinutes"], + value: settings.claims.claimDurationMinutes, + }, + { path: ["telemetry", "enabled"], value: settings.telemetry.enabled }, + { path: ["telemetry", "anonymousId"], value: settings.telemetry.anonymousId }, + { path: ["telemetry", "consentGiven"], value: settings.telemetry.consentGiven }, + { + path: ["tui", "showLaunchpadWelcome"], + value: settings.tui!.showLaunchpadWelcome, + }, + { + path: ["session", "backlogPreviewSize"], + value: settings.session!.backlogPreviewSize, + }, + ]; } async hasTelemetryConfiguration(): Promise { diff --git a/src/infrastructure/settings/JsoncSettingsEditor.ts b/src/infrastructure/settings/JsoncSettingsEditor.ts new file mode 100644 index 00000000..ae61f68d --- /dev/null +++ b/src/infrastructure/settings/JsoncSettingsEditor.ts @@ -0,0 +1,101 @@ +import * as jsonc from "jsonc-parser"; + +/** + * Recursive JSON value shape used to walk settings documents and defaults + * without resorting to `any` at the JSONC editing boundary. + */ +export type JsonValue = + | string + | number + | boolean + | null + | JsonValue[] + | { [key: string]: JsonValue }; + +export type JsonObject = { [key: string]: JsonValue }; + +const FORMATTING_OPTIONS: jsonc.FormattingOptions = { + insertSpaces: true, + tabSize: 2, + eol: "\n", +}; + +function isPlainObject(value: JsonValue): value is JsonObject { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** + * Parse settings JSONC content, throwing a descriptive error if it is invalid. + * Used at every write path so malformed files fail loudly instead of being + * partially rewritten. + */ +export function assertValidJsonc(content: string, filePath: string): void { + const errors: jsonc.ParseError[] = []; + jsonc.parse(content, errors, { allowTrailingComma: true }); + if (errors.length > 0) { + const formatted = errors + .map((err) => `Error at offset ${err.offset}: ${jsonc.printParseErrorCode(err.error)}`) + .join(", "); + throw new Error(`Invalid JSON in settings file at ${filePath}: ${formatted}`); + } +} + +/** + * Set a single value at `path` within JSONC `content`, preserving comments, + * formatting, and every other entry in the document. + */ +export function setJsoncValue(content: string, path: jsonc.JSONPath, value: JsonValue): string { + const edits = jsonc.modify(content, path, value, { formattingOptions: FORMATTING_OPTIONS }); + return jsonc.applyEdits(content, edits); +} + +function isMissing(content: string, path: jsonc.JSONPath): boolean { + const tree = jsonc.parseTree(content); + if (!tree) { + return true; + } + return jsonc.findNodeAtLocation(tree, path) === undefined; +} + +export interface MissingEntry { + path: jsonc.JSONPath; + value: JsonValue; +} + +/** + * Walk `defaults` against existing JSONC `content` and collect entries that + * are absent. Whole subtrees are reported as a single entry when their parent + * is entirely missing; otherwise only the missing nested fields are reported. + * Collected entries never nest one inside another, so they can be applied to + * `content` in any order without conflicting. + */ +export function collectMissingDefaults( + content: string, + defaults: JsonObject, + basePath: jsonc.JSONPath = [] +): MissingEntry[] { + const entries: MissingEntry[] = []; + for (const key of Object.keys(defaults)) { + const path = [...basePath, key]; + const value = defaults[key]; + if (isMissing(content, path)) { + entries.push({ path, value }); + } else if (isPlainObject(value)) { + entries.push(...collectMissingDefaults(content, value, path)); + } + } + return entries; +} + +/** + * Apply every missing default entry to `content`, returning the updated + * document. Returns `content` unchanged if there is nothing to add. + */ +export function applyMissingDefaults(content: string, defaults: JsonObject): string { + const entries = collectMissingDefaults(content, defaults); + let updated = content; + for (const entry of entries) { + updated = setJsoncValue(updated, entry.path, entry.value); + } + return updated; +} diff --git a/tests/infrastructure/settings/FsSettingsInitializer.test.ts b/tests/infrastructure/settings/FsSettingsInitializer.test.ts index a8802928..e21763bf 100644 --- a/tests/infrastructure/settings/FsSettingsInitializer.test.ts +++ b/tests/infrastructure/settings/FsSettingsInitializer.test.ts @@ -38,4 +38,94 @@ describe("FsSettingsInitializer", () => { const settings = await new FsSettingsReader(tempDir).read(); expect(settings.session).toEqual(DEFAULT_SETTINGS.session); }); + + it("adds missing default sections to an existing settings file", async () => { + const tempDir = await createTempDir(); + const settingsPath = path.join(tempDir, "settings.jsonc"); + await fs.writeFile( + settingsPath, + JSON.stringify({ + qa: { defaultTurnLimit: 7 }, + claims: { claimDurationMinutes: 45 }, + telemetry: { enabled: false, anonymousId: null, consentGiven: true }, + }), + "utf-8", + ); + const initializer = new FsSettingsInitializer(tempDir); + + await initializer.ensureSettingsFileExists(); + + const settings = await new FsSettingsReader(tempDir).read(); + expect(settings.project).toEqual(DEFAULT_SETTINGS.project); + expect(settings.tui).toEqual(DEFAULT_SETTINGS.tui); + expect(settings.session).toEqual(DEFAULT_SETTINGS.session); + }); + + it("preserves explicit known values already present in the settings file", async () => { + const tempDir = await createTempDir(); + const settingsPath = path.join(tempDir, "settings.jsonc"); + await fs.writeFile( + settingsPath, + JSON.stringify({ + project: { id: "11111111-1111-4111-8111-111111111111" }, + qa: { defaultTurnLimit: 7 }, + claims: { claimDurationMinutes: 45 }, + telemetry: { enabled: false, anonymousId: null, consentGiven: true }, + tui: { showLaunchpadWelcome: false }, + session: { backlogPreviewSize: 2 }, + }), + "utf-8", + ); + const initializer = new FsSettingsInitializer(tempDir); + + await initializer.ensureSettingsFileExists(); + + const settings = await new FsSettingsReader(tempDir).read(); + expect(settings.project).toEqual({ id: "11111111-1111-4111-8111-111111111111" }); + expect(settings.qa).toEqual({ defaultTurnLimit: 7 }); + expect(settings.claims).toEqual({ claimDurationMinutes: 45 }); + expect(settings.telemetry).toEqual({ enabled: false, anonymousId: null, consentGiven: true }); + expect(settings.tui).toEqual({ showLaunchpadWelcome: false }); + expect(settings.session).toEqual({ backlogPreviewSize: 2 }); + }); + + it("preserves unknown top-level and nested entries when filling in defaults", async () => { + const tempDir = await createTempDir(); + const settingsPath = path.join(tempDir, "settings.jsonc"); + await fs.writeFile( + settingsPath, + JSON.stringify({ + qa: { defaultTurnLimit: 7, customField: "keep-me" }, + claims: { claimDurationMinutes: 45 }, + telemetry: { enabled: false, anonymousId: null, consentGiven: true }, + experimental: { flagA: true }, + }), + "utf-8", + ); + const initializer = new FsSettingsInitializer(tempDir); + + await initializer.ensureSettingsFileExists(); + + const rawContent = await fs.readFile(settingsPath, "utf-8"); + const raw = JSON.parse( + rawContent + .split("\n") + .filter((line) => !line.trim().startsWith("//")) + .join("\n"), + ); + expect(raw.qa.customField).toBe("keep-me"); + expect(raw.experimental).toEqual({ flagA: true }); + }); + + it("throws a clear error instead of rewriting an invalid JSONC settings file", async () => { + const tempDir = await createTempDir(); + const settingsPath = path.join(tempDir, "settings.jsonc"); + await fs.writeFile(settingsPath, "{ invalid json ", "utf-8"); + const initializer = new FsSettingsInitializer(tempDir); + + await expect(initializer.ensureSettingsFileExists()).rejects.toThrow(); + + const contentAfterFailure = await fs.readFile(settingsPath, "utf-8"); + expect(contentAfterFailure).toBe("{ invalid json "); + }); }); diff --git a/tests/infrastructure/settings/FsSettingsReader.test.ts b/tests/infrastructure/settings/FsSettingsReader.test.ts index 257ac7ce..acdb0f04 100644 --- a/tests/infrastructure/settings/FsSettingsReader.test.ts +++ b/tests/infrastructure/settings/FsSettingsReader.test.ts @@ -117,4 +117,32 @@ describe("FsSettingsReader", () => { session: { backlogPreviewSize: 2 }, }); }); + + it("preserves unknown top-level and nested entries when persisting known settings changes", async () => { + const tempDir = await createTempDir(); + const settingsPath = path.join(tempDir, "settings.jsonc"); + await fs.writeFile( + settingsPath, + JSON.stringify({ + ...DEFAULT_SETTINGS, + qa: { defaultTurnLimit: 3, customField: "keep-me" }, + experimental: { flagA: true }, + }), + "utf-8", + ); + const reader = new FsSettingsReader(tempDir); + + await reader.write({ + ...DEFAULT_SETTINGS, + tui: { showLaunchpadWelcome: false }, + }); + + const rawContent = await fs.readFile(settingsPath, "utf-8"); + const raw = JSON.parse(rawContent); + expect(raw.qa.customField).toBe("keep-me"); + expect(raw.experimental).toEqual({ flagA: true }); + + const settings = await reader.read(); + expect(settings.tui).toEqual({ showLaunchpadWelcome: false }); + }); }); diff --git a/tests/infrastructure/settings/JsoncSettingsEditor.test.ts b/tests/infrastructure/settings/JsoncSettingsEditor.test.ts new file mode 100644 index 00000000..b8eda578 --- /dev/null +++ b/tests/infrastructure/settings/JsoncSettingsEditor.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it } from "@jest/globals"; +import { + JsonObject, + applyMissingDefaults, + assertValidJsonc, + collectMissingDefaults, + setJsoncValue, +} from "../../../src/infrastructure/settings/JsoncSettingsEditor.js"; + +describe("JsoncSettingsEditor", () => { + describe("assertValidJsonc", () => { + it("does not throw for valid JSONC content", () => { + expect(() => assertValidJsonc('{ "a": 1 }', "settings.jsonc")).not.toThrow(); + }); + + it("throws a descriptive error for invalid JSONC content", () => { + expect(() => assertValidJsonc("{ invalid", "settings.jsonc")).toThrow( + /Invalid JSON in settings file at settings\.jsonc/ + ); + }); + }); + + describe("setJsoncValue", () => { + it("sets a nested value while preserving comments and unrelated entries", () => { + const content = `{ + // keep this comment + "qa": { + "defaultTurnLimit": 3 + }, + "custom": { + "flag": true + } +} +`; + + const updated = setJsoncValue(content, ["qa", "defaultTurnLimit"], 9); + + expect(updated).toContain("// keep this comment"); + expect(JSON.parse(updated.replace(/\/\/.*$/gm, ""))).toEqual({ + qa: { defaultTurnLimit: 9 }, + custom: { flag: true }, + }); + }); + + it("inserts a value at a path that does not yet exist", () => { + const content = `{ + "qa": { "defaultTurnLimit": 3 } +} +`; + + const updated = setJsoncValue(content, ["tui", "showLaunchpadWelcome"], false); + + expect(JSON.parse(updated)).toEqual({ + qa: { defaultTurnLimit: 3 }, + tui: { showLaunchpadWelcome: false }, + }); + }); + }); + + describe("collectMissingDefaults", () => { + it("reports an entire section as missing when its parent key is absent", () => { + const content = `{ "qa": { "defaultTurnLimit": 3 } }`; + const defaults: JsonObject = { + qa: { defaultTurnLimit: 3 }, + tui: { showLaunchpadWelcome: true }, + }; + + const entries = collectMissingDefaults(content, defaults); + + expect(entries).toEqual([{ path: ["tui"], value: { showLaunchpadWelcome: true } }]); + }); + + it("reports only the missing nested field when the parent already exists", () => { + const content = `{ "telemetry": { "enabled": false } }`; + const defaults: JsonObject = { + telemetry: { enabled: true, anonymousId: null, consentGiven: false }, + }; + + const entries = collectMissingDefaults(content, defaults); + + expect(entries).toEqual( + expect.arrayContaining([ + { path: ["telemetry", "anonymousId"], value: null }, + { path: ["telemetry", "consentGiven"], value: false }, + ]) + ); + expect(entries).toHaveLength(2); + }); + + it("reports nothing missing when every default field is already present", () => { + const content = `{ "qa": { "defaultTurnLimit": 3 } }`; + const defaults: JsonObject = { qa: { defaultTurnLimit: 3 } }; + + expect(collectMissingDefaults(content, defaults)).toEqual([]); + }); + }); + + describe("applyMissingDefaults", () => { + it("adds missing sections and fields without overwriting explicit values", () => { + const content = `{ + "qa": { "defaultTurnLimit": 7 }, + "custom": { "keep": true } +} +`; + const defaults: JsonObject = { + qa: { defaultTurnLimit: 3 }, + tui: { showLaunchpadWelcome: true }, + session: { backlogPreviewSize: 5 }, + }; + + const updated = applyMissingDefaults(content, defaults); + const parsed = JSON.parse(updated); + + expect(parsed.qa).toEqual({ defaultTurnLimit: 7 }); + expect(parsed.custom).toEqual({ keep: true }); + expect(parsed.tui).toEqual({ showLaunchpadWelcome: true }); + expect(parsed.session).toEqual({ backlogPreviewSize: 5 }); + }); + + it("returns the content unchanged when nothing is missing", () => { + const content = `{ "qa": { "defaultTurnLimit": 3 } }`; + const defaults: JsonObject = { qa: { defaultTurnLimit: 3 } }; + + expect(applyMissingDefaults(content, defaults)).toBe(content); + }); + }); +});