Skip to content

fix(cli): remove unset config keys from every layered config file - #12687

Merged
marius-kilocode merged 1 commit into
mainfrom
yielding-newsboy
Jul 30, 2026
Merged

fix(cli): remove unset config keys from every layered config file#12687
marius-kilocode merged 1 commit into
mainfrom
yielding-newsboy

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Clearing a setting back to "not set" in the Settings UI (or via the config API) did not stick when more than one config file existed. Kilo reads and deep-merges every config file in a scope (global: config.json, kilo.json, kilo.jsonc, opencode.json, opencode.jsonc; project: the same names in the project root and config directories), but writes only patched the single primary target file. An unset deleted the key from kilo.jsonc, and when the value actually lived in kilo.json or a legacy file, the delete was a no-op there, the merged read kept resolving the old value, and the setting snapped back after save. Setting a value worked, which made this look intermittent: sets shadow lower-precedence files, unsets did not.

With this change, delete sentinels are removed from every layered config file that contains the key, in both the global and project scopes. Files that do not contain the key are not touched (no byte-level rewrites), comments in .jsonc files are preserved, and sets keep their existing single-target behavior. The propagation runs inside the global config file lock and counts toward the change flag, so cache invalidation and instance disposal still fire when only a sibling file changed.

The same patch fixes a second latent crash in this path: deleting a nested key whose parent object is absent from the target file (for example clearing agent.explore.model in a file without an agent section) threw Can not delete in empty document from jsonc-parser. Absent paths are now skipped.

The settings docs now also state that layered config files are merged and that clearing a setting removes it from every file that contains it.

Related: #11484 documents the opposite direction of the same read/write mismatch (sets can be silently overridden because read precedence favors legacy opencode.* files). This PR does not change set behavior or read precedence; it makes unsets robust regardless of that ordering.

@kilo-code-bot

kilo-code-bot Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (5 files)
  • .changeset/config-unset-propagation.md
  • packages/kilo-docs/pages/getting-started/settings/index.md
  • packages/opencode/src/config/config.ts
  • packages/opencode/src/kilocode/config/config.ts
  • packages/opencode/test/kilocode/config/config.test.ts

Traced the propagation logic end-to-end (unsetPathssentinel/haspropagateUnset for both global and project scopes, plus the new patchJsonc guard against deleting a missing parent path). The global-scope propagation correctly runs inside the existing flock lock, changed is set from either the primary write or a sibling propagation, and the .jsonc vs plain-JSON branches use the right delete mechanism (modify() with undefined vs. null-sentinel + mergeConfig + stripNulls). New tests exercise real file I/O (no mocks) and cover the previously-broken cases (multi-file unset, jsonc comment preservation, no-op on sets/absent keys, project scope). Shared packages/opencode/src/config/config.ts changes are minimal and properly marked with kilocode_change. Changeset is concise and user-facing. No new bugs, security issues, or performance concerns found in the changed lines.


Reviewed by claude-sonnet-5 · Input: 30 · Output: 9.9K · Cached: 825.7K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode merged commit 4c0fcb0 into main Jul 30, 2026
43 of 47 checks passed
@marius-kilocode
marius-kilocode deleted the yielding-newsboy branch July 30, 2026 11:07
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
fix(cli): remove unset config keys from every layered config file
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants