From 624b5890f10bc0f60c50a191cd7bc82fac9574d0 Mon Sep 17 00:00:00 2001 From: marius-kilocode Date: Thu, 30 Jul 2026 12:18:42 +0200 Subject: [PATCH] fix(cli): remove unset config keys from every layered config file --- .changeset/config-unset-propagation.md | 5 + .../pages/getting-started/settings/index.md | 2 + packages/opencode/src/config/config.ts | 23 ++- .../opencode/src/kilocode/config/config.ts | 118 ++++++++++++--- .../test/kilocode/config/config.test.ts | 139 ++++++++++++++++++ 5 files changed, 266 insertions(+), 21 deletions(-) create mode 100644 .changeset/config-unset-propagation.md diff --git a/.changeset/config-unset-propagation.md b/.changeset/config-unset-propagation.md new file mode 100644 index 00000000000..82418a56350 --- /dev/null +++ b/.changeset/config-unset-propagation.md @@ -0,0 +1,5 @@ +--- +"@kilocode/cli": patch +--- + +Fix settings snapping back to their previous value after being cleared to "Not set" when multiple config files exist (e.g. both `kilo.json` and `kilo.jsonc`) diff --git a/packages/kilo-docs/pages/getting-started/settings/index.md b/packages/kilo-docs/pages/getting-started/settings/index.md index 7a32691755b..ab807ac5972 100644 --- a/packages/kilo-docs/pages/getting-started/settings/index.md +++ b/packages/kilo-docs/pages/getting-started/settings/index.md @@ -29,6 +29,8 @@ This is especially useful for complex configuration like custom model definition Kilo reads JSONC config from a **global** location (`~/.config/kilo/kilo.jsonc`) and from your **project** (`kilo.jsonc`, or `.kilo/kilo.jsonc`). All clients — CLI, VS Code, and JetBrains — read the same files. +If `kilo.json` or the legacy `opencode.json`, `opencode.jsonc`, or `config.json` files exist in the same locations, Kilo reads and deep-merges them as well. Clearing a setting in the Settings UI (for example, setting a model back to "Not set") removes it from every config file that contains it. + {% callout type="warning" %} **Migrating from opencode?** Kilo no longer falls back to opencode configuration stored in `.opencode` directories (such as `~/.config/opencode` or a project `./.opencode/`). To keep using it, move your global config into `~/.config/kilo/` and any project config into `./.kilo/`. {% /callout %} diff --git a/packages/opencode/src/config/config.ts b/packages/opencode/src/config/config.ts index 8c396e638a2..5d08de3794c 100644 --- a/packages/opencode/src/config/config.ts +++ b/packages/opencode/src/config/config.ts @@ -207,6 +207,13 @@ function globalConfigFile() { function patchJsonc(input: string, patch: unknown, path: string[] = []): string { if (!isRecord(patch)) { + // kilocode_change start - jsonc-parser throws when deleting a path whose + // parent does not exist in the document; absent keys are already "unset" + if (patch === null) { + const tree = parseTree(input) + if (!tree || !findNodeAtLocation(tree, path)) return input + } + // kilocode_change end const edits = modify(input, path, patch === null ? undefined : patch, { // kilocode_change formattingOptions: { @@ -1015,20 +1022,28 @@ export const layer = Layer.effect( Effect.gen(function* () { const before = (yield* readConfigFile(file)) ?? "{}" const patch = writableGlobal(config) + // Reads merge every global config file, so delete sentinels must be + // removed from all of them, not just the primary write target. + const propagated = yield* KilocodeConfig.propagateUnset({ + fs, + files: KilocodeConfig.GLOBAL_CONFIG_FILES.map((name) => path.join(Global.Path.config, name)), + exclude: file, + patch, + }) if (!file.endsWith(".jsonc")) { const existing = ConfigParse.schema(ConfigV1.Info, ConfigParse.jsonc(before, file), file) const next = KilocodeConfig.mergeConfig(writable(existing), patch) const serialized = JSON.stringify(next, null, 2) - const changed = serialized !== before - if (changed) yield* fs.writeFileString(file, serialized).pipe(Effect.orDie) + const changed = serialized !== before || propagated + if (serialized !== before) yield* fs.writeFileString(file, serialized).pipe(Effect.orDie) return { next, changed } } const updated = patchJsonc(before, patch) const next = ConfigParse.schema(ConfigV1.Info, ConfigParse.jsonc(updated, file), file) - const changed = updated !== before - if (changed) yield* fs.writeFileString(file, updated).pipe(Effect.orDie) + const changed = updated !== before || propagated + if (updated !== before) yield* fs.writeFileString(file, updated).pipe(Effect.orDie) return { next, changed } }), `config:global:${path.resolve(Global.Path.config)}`, diff --git a/packages/opencode/src/kilocode/config/config.ts b/packages/opencode/src/kilocode/config/config.ts index a33445494b8..e84cfe19239 100644 --- a/packages/opencode/src/kilocode/config/config.ts +++ b/packages/opencode/src/kilocode/config/config.ts @@ -58,13 +58,11 @@ export namespace KilocodeConfig { ] as const /** - * Choose the project config file that Config.update should patch. - * - * This mirrors the Kilo project-config load chain: prefer existing config files - * in ancestor config directories, then existing root config files, and create - * `.kilo/kilo.jsonc` when no project config exists yet. + * List every project config file the read chain can merge: config files in + * ancestor config directories, then root config files, in update-target + * preference order. */ - export const projectConfigUpdateTarget = Effect.fn("KilocodeConfig.projectConfigUpdateTarget")(function* (input: { + export const projectConfigFiles = Effect.fn("KilocodeConfig.projectConfigFiles")(function* (input: { fs: FSUtil.Interface directory: string worktree?: string @@ -75,8 +73,7 @@ export namespace KilocodeConfig { const roots = yield* input.fs .up({ targets: [...ALL_CONFIG_FILES], start: input.directory, stop: input.worktree }) .pipe(Effect.orDie) - const files = [...dirs.flatMap((dir) => ALL_CONFIG_FILES.map((file) => path.join(dir, file))), ...roots] - return files.find((file) => existsSync(file)) ?? path.join(input.directory, ".kilo", "kilo.jsonc") + return [...dirs.flatMap((dir) => ALL_CONFIG_FILES.map((file) => path.join(dir, file))), ...roots] }) export const updateProjectConfig = Effect.fn("KilocodeConfig.updateProjectConfig")(function* (input: { @@ -89,22 +86,108 @@ export namespace KilocodeConfig { patch: (input: string, config: Config.Info) => string writable: (config: Config.Info) => Config.Info }) { - const file = yield* projectConfigUpdateTarget(input) + const files = yield* projectConfigFiles(input) + const file = files.find((item) => existsSync(item)) ?? path.join(input.directory, ".kilo", "kilo.jsonc") const source = yield* input.read(file) const before = source ?? "{}" const patch = input.writable(input.config) if (file.endsWith(".jsonc")) { - if (source === undefined && Object.keys(mergeConfig({}, patch)).length === 0) return - const updated = input.patch(before, patch) - yield* input.fs.writeWithDirs(file, updated).pipe(Effect.orDie) + if (!(source === undefined && Object.keys(mergeConfig({}, patch)).length === 0)) { + const updated = input.patch(before, patch) + yield* input.fs.writeWithDirs(file, updated).pipe(Effect.orDie) + } + } else { + const existing = input.parse(before, file) + const merged = mergeConfig(input.writable(existing), patch) + if (!(source === undefined && Object.keys(merged).length === 0)) { + yield* input.fs.writeWithDirs(file, JSON.stringify(merged, null, 2)).pipe(Effect.orDie) + } + } + + // Reads merge every project config file, so a delete sentinel applied only + // to the update target leaves lower-precedence copies of the key visible. + yield* propagateUnset({ fs: input.fs, files, exclude: file, patch }) + }) + + /** Collect the leaf paths of null delete sentinels in a config patch. */ + export function unsetPaths(patch: unknown, prefix: string[] = []): string[][] { + if (!isRecord(patch)) return [] + return Object.entries(patch).flatMap(([key, value]) => { + const parts = [...prefix, key] + if (value === null) return [parts] + return unsetPaths(value, parts) + }) + } + + const blocked = new Set(["__proto__", "constructor", "prototype"]) + + function sentinel(out: Record, parts: string[]) { + const [head, ...tail] = parts + if (!head || blocked.has(head)) return + if (tail.length === 0) { + out[head] = null return } + const next = isRecord(out[head]) ? out[head] : {} + out[head] = next + sentinel(next, tail) + } - const existing = input.parse(before, file) - const merged = mergeConfig(input.writable(existing), patch) - if (source === undefined && Object.keys(merged).length === 0) return - yield* input.fs.writeWithDirs(file, JSON.stringify(merged, null, 2)).pipe(Effect.orDie) + function has(input: unknown, parts: string[]) { + let cur = input + for (const part of parts) { + if (!isRecord(cur) || !(part in cur)) return false + cur = cur[part] + } + return true + } + + /** + * Remove null delete-sentinel keys from every layered config file that still + * contains them. Reads merge all candidate files, so deleting a key from only + * the primary write target leaves lower-precedence copies of it visible and + * the "unset" appears to have no effect. Returns true when a file changed. + */ + export const propagateUnset = Effect.fn("KilocodeConfig.propagateUnset")(function* (input: { + fs: FSUtil.Interface + files: readonly string[] + exclude: string + patch: Config.Info + }) { + const paths = unsetPaths(input.patch) + if (paths.length === 0) return false + let changed = false + for (const file of input.files) { + if (file === input.exclude || !existsSync(file)) continue + const text = yield* input.fs.readFileStringSafe(file).pipe(Effect.orDie) + if (!text) continue + const parsed = parseJsonc(text) + const hits = paths.filter((parts) => has(parsed, parts)) + if (hits.length === 0) continue + if (file.endsWith(".jsonc")) { + const updated = hits.reduce( + (acc, parts) => + applyEdits(acc, modify(acc, parts, undefined, { formattingOptions: { insertSpaces: true, tabSize: 2 } })), + text, + ) + if (updated === text) continue + yield* input.fs.writeFileString(file, updated).pipe(Effect.orDie) + changed = true + continue + } + const patch = hits.reduce( + (acc, parts) => { + sentinel(acc, parts) + return acc + }, + {} as Record, + ) + const next = mergeConfig(parsed as Config.Info, patch as Config.Info) + yield* input.fs.writeFileString(file, JSON.stringify(next, null, 2)).pipe(Effect.orDie) + changed = true + } + return changed }) export function scopeIndexing(info: Config.Info, scope: "global" | "local"): Config.Info { @@ -333,7 +416,8 @@ export namespace KilocodeConfig { // ── Bash permission migration ──────────────────────────────────────── - const GLOBAL_CONFIG_FILES = ["config.json", "kilo.json", "kilo.jsonc", "opencode.json", "opencode.jsonc"] + /** Global config file names in read-merge order (lowest-to-highest precedence). */ + export const GLOBAL_CONFIG_FILES = ["config.json", "kilo.json", "kilo.jsonc", "opencode.json", "opencode.jsonc"] /** * Migrate bash permission for existing users before config is consumed. diff --git a/packages/opencode/test/kilocode/config/config.test.ts b/packages/opencode/test/kilocode/config/config.test.ts index dde6a6d7078..d41d17976bb 100644 --- a/packages/opencode/test/kilocode/config/config.test.ts +++ b/packages/opencode/test/kilocode/config/config.test.ts @@ -468,6 +468,145 @@ describe("subagent variant overrides", () => { }) }) +describe("unset propagation across layered config files", () => { + const getGlobal = () => + Effect.runPromise(Config.Service.use((svc) => svc.getGlobal()).pipe(Effect.scoped, Effect.provide(layer))) + + test("removes subagent_model from every global config file when unset", async () => { + await using globalTmp = await tmpdir() + const json = path.join(globalTmp.path, "kilo.json") + const jsonc = path.join(globalTmp.path, "kilo.jsonc") + const jsoncText = ["{", " // Keep this comment.", ' "username": "marius"', "}", ""].join("\n") + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await writeConfig(globalTmp.path, { subagent_model: "kilo/openai/gpt-5" }, "kilo.json") + await Filesystem.write(jsonc, jsoncText) + + await saveGlobal(decode({ subagent_model: null })) + + // The key must be gone from the lower-precedence kilo.json as well, or + // the read chain keeps resolving it and the unset appears to do nothing. + expect(JSON.parse(await Bun.file(json).text())).not.toHaveProperty("subagent_model") + // The primary target had no key, so it must remain byte-identical. + expect(await Bun.file(jsonc).text()).toBe(jsoncText) + + await clear() + expect((await getGlobal()).subagent_model).toBeUndefined() + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("removes nested sentinels from jsonc siblings while preserving comments", async () => { + await using globalTmp = await tmpdir() + const opencode = path.join(globalTmp.path, "opencode.jsonc") + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await Filesystem.write(path.join(globalTmp.path, "kilo.jsonc"), '{ "username": "marius" }\n') + await Filesystem.write( + opencode, + [ + "{", + " // Preserve this comment while clearing overrides.", + ' "agent": {', + ' "explore": {', + ' "model": "kilo/anthropic/claude-sonnet-4-6",', + ' "description": "Keep me"', + " }", + " }", + "}", + ].join("\n"), + ) + + await saveGlobal(decode({ agent: { explore: { model: null } } })) + + const written = await Bun.file(opencode).text() + expect(written).toContain("// Preserve this comment while clearing overrides.") + expect(written).not.toContain('"model"') + expect(written).toContain('"description": "Keep me"') + + await clear() + expect((await getGlobal()).agent?.explore?.model).toBeUndefined() + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("does not rewrite sibling files on set or when the key is absent", async () => { + await using globalTmp = await tmpdir() + const json = path.join(globalTmp.path, "kilo.json") + const jsonText = JSON.stringify({ subagent_model: "kilo/old-model", username: "marius" }, null, 2) + const prev = Global.Path.config + ;(Global.Path as { config: string }).config = globalTmp.path + await clear() + await disposeAllInstances() + + try { + await Filesystem.write(path.join(globalTmp.path, "kilo.jsonc"), '{ "username": "marius" }\n') + await Filesystem.write(json, jsonText) + + // Sets only write to the primary target; lower-precedence copies stay + // untouched and are simply shadowed by the higher-precedence value. + await saveGlobal(decode({ subagent_model: "kilo/new-model" })) + expect(await Bun.file(json).text()).toBe(jsonText) + + // Unsetting an absent key must not rewrite the sibling either. + await saveGlobal(decode({ small_model: null })) + expect(await Bun.file(json).text()).toBe(jsonText) + } finally { + ;(Global.Path as { config: string }).config = prev + await clear() + await disposeAllInstances() + } + }) + + test("removes subagent_model from every project config file when unset", async () => { + await using tmp = await tmpdir({ git: true }) + await Filesystem.write( + path.join(tmp.path, ".kilo", "kilo.json"), + JSON.stringify({ subagent_model: "kilo/openai/gpt-5" }), + ) + await Filesystem.write(path.join(tmp.path, ".kilo", "kilo.jsonc"), '{\n "username": "keep"\n}\n') + + await provideTestInstance({ + directory: tmp.path, + fn: async () => { + await saveProject(decode({ subagent_model: null })) + expect((await load()).subagent_model).toBeUndefined() + }, + }) + + const json = JSON.parse(await Bun.file(path.join(tmp.path, ".kilo", "kilo.json")).text()) + expect(json).not.toHaveProperty("subagent_model") + // The primary target only gained nothing; the delete was a no-op there. + const jsonc = JSON.parse(await Bun.file(path.join(tmp.path, ".kilo", "kilo.jsonc")).text()) + expect(jsonc).not.toHaveProperty("subagent_model") + expect(jsonc.username).toBe("keep") + }) + + test("collects null sentinel paths from nested patches", () => { + expect( + KilocodeConfig.unsetPaths({ + subagent_model: null, + agent: { explore: { model: null, variant: "high" } }, + username: "marius", + }), + ).toEqual([["subagent_model"], ["agent", "explore", "model"]]) + }) +}) + describe("agent config", () => { test("accepts delete sentinels for agent model and variant overrides", () => { const patch = decode({ agent: { explore: { model: null, variant: null } } })