Skip to content

fix(cli): handle missing nested config unsets - #12727

Merged
marius-kilocode merged 2 commits into
mainfrom
sun-mule
Jul 31, 2026
Merged

fix(cli): handle missing nested config unsets#12727
marius-kilocode merged 2 commits into
mainfrom
sun-mule

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

The VS Code Settings overlay can represent clearing a nested setting as an unset path, such as agent.explore.model. When the project config target does not exist, the overlay writer patches an empty JSON document. jsonc-parser rejects deleting a nested path whose parent is absent with Can not delete in empty document, which surfaces in the extension as an opaque Unexpected server error.

Apply the same absent-path guard already used by the regular config writer to the overlay writer. Missing delete paths are treated as already unset, while existing paths continue to be removed normally. Add a route regression test for the missing nested project target and include a CLI patch changeset.

@marius-kilocode
marius-kilocode enabled auto-merge (squash) July 31, 2026 11:21
Comment thread packages/opencode/src/kilocode/config/writer.ts
Comment thread packages/opencode/test/kilocode/server/config-overlay.test.ts
@kilo-code-bot

kilo-code-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: 1 Issue Found | Recommendation: Optional follow-up

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1

Incremental review of 5ae91eb..899934e: the new commit adds the rationale comment above the guard and a regression test that an existing nested path is still deleted (with siblings preserved). Both address the prior review feedback; no new issues found in the changed lines.

Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilocode/config/writer.ts 88 Guard logic is still duplicated with patchJsonc in src/config/config.ts; consider extracting one shared Kilo-owned helper so the next fix only has to land once
Files Reviewed (2 files)
  • packages/opencode/src/kilocode/config/writer.ts - 1 issue (pre-existing comment, duplication remains)
  • packages/opencode/test/kilocode/server/config-overlay.test.ts - no issues (previous coverage gap resolved)

Fix these issues in Kilo Cloud

Previous Review Summary (commit 5ae91eb)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 5ae91eb)

Status: 2 Issues Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 2

The fix itself looks correct: the patch === null guard short-circuits before modify() for absent paths, and the recursive walk still applies sibling set values, so a mixed set/unset patch is unaffected. Nested unsets whose parent is created by a set in the same request are also fine, because the sibling insert is applied before the delete is evaluated.

Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilocode/config/writer.ts 87 patchJsonc is a near-verbatim duplicate of config.ts's copy — same bug had to be fixed twice; consider extracting one shared Kilo-owned helper and add the Can not delete in empty document rationale comment
packages/opencode/test/kilocode/server/config-overlay.test.ts 106 No coverage that an existing nested path is still deleted; the new guard could silently turn real deletes into 200-with-no-op
Files Reviewed (3 files)
  • packages/opencode/src/kilocode/config/writer.ts - 1 issue
  • packages/opencode/test/kilocode/server/config-overlay.test.ts - 1 issue
  • .changeset/quiet-config-unset.md - no issues (user-facing wording is appropriate)

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 32 · Output: 4.8K · Cached: 710.2K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode disabled auto-merge July 31, 2026 11:29
@marius-kilocode
marius-kilocode enabled auto-merge (squash) July 31, 2026 11:31
@marius-kilocode
marius-kilocode merged commit 1a34037 into main Jul 31, 2026
31 checks passed
@marius-kilocode
marius-kilocode deleted the sun-mule branch July 31, 2026 11:41
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
* fix(cli): handle missing nested config unsets

* test(cli): cover nested config unset behavior
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