From 096233b23fe360ca8ddc0c4d35faa7114920709e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 9 Feb 2026 21:11:40 +0900 Subject: [PATCH] fix(config-manager): replace heuristic JSONC editing with jsonc-parser modify/applyEdits --- .../add-provider-config.test.ts | 205 ++++++++++++++++++ src/cli/config-manager/add-provider-config.ts | 69 ++---- .../config-manager/jsonc-provider-editor.ts | 11 + 3 files changed, 236 insertions(+), 49 deletions(-) create mode 100644 src/cli/config-manager/add-provider-config.test.ts create mode 100644 src/cli/config-manager/jsonc-provider-editor.ts diff --git a/src/cli/config-manager/add-provider-config.test.ts b/src/cli/config-manager/add-provider-config.test.ts new file mode 100644 index 000000000..53a7cc335 --- /dev/null +++ b/src/cli/config-manager/add-provider-config.test.ts @@ -0,0 +1,205 @@ +import { describe, expect, it } from "bun:test" +import { modifyProviderInJsonc } from "./jsonc-provider-editor" +import { parseJsonc } from "../../shared/jsonc-parser" + +describe("modifyProviderInJsonc", () => { + describe("Test 1: Basic JSONC with existing provider", () => { + it("replaces provider value, preserves comments and other keys", () => { + // given + const content = `{ + // my config + "provider": { "openai": {} }, + "plugin": ["foo"] +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"google"') + expect(result).toContain('"plugin": ["foo"]') + expect(result).toContain('// my config') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('plugin') + expect(parsed).toHaveProperty('provider') + }) + }) + + describe("Test 2: Comment containing '}' inside provider block", () => { + it("must NOT corrupt file", () => { + // given + const content = `{ + "provider": { + // } this brace should be ignored + "openai": {} + }, + "other": 1 +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"other"') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('other') + expect(parsed.other).toBe(1) + }) + }) + + describe("Test 3: Comment containing '\"provider\"' before real key", () => { + it("must NOT match wrong location", () => { + // given + const content = `{ + // "provider": { "example": true } + "provider": { "openai": {} }, + "other": 1 +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"other"') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('other') + expect(parsed.other).toBe(1) + expect(parsed.provider).toHaveProperty('google') + }) + }) + + describe("Test 4: Comment containing '{' inside provider", () => { + it("must NOT mess up depth", () => { + // given + const content = `{ + "provider": { + // { unmatched brace in comment + "openai": {} + }, + "other": 1 +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"other"') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('other') + expect(parsed.other).toBe(1) + }) + }) + + describe("Test 5: No existing provider key", () => { + it("inserts provider without corrupting", () => { + // given + const content = `{ + // config comment + "plugin": ["foo"] +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"provider"') + expect(result).toContain('"plugin"') + expect(result).toContain('foo') + expect(result).toContain('// config comment') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('provider') + expect(parsed).toHaveProperty('plugin') + expect(parsed.plugin).toEqual(['foo']) + }) + }) + + describe("Test 6: String value exactly 'provider' before real key", () => { + it("must NOT match wrong location", () => { + // given + const content = `{ + "note": "provider", + "provider": { "openai": {} }, + "other": 1 +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(result).toContain('"other"') + expect(result).toContain('"note": "provider"') + + // Post-write validation + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('other') + expect(parsed.other).toBe(1) + expect(parsed.note).toBe('provider') + }) + }) + + describe("Test 7: Post-write validation", () => { + it("result file must be valid JSONC for all cases", () => { + // Test Case 1 + const content1 = `{ + "provider": { "openai": {} }, + "plugin": ["foo"] +}` + const result1 = modifyProviderInJsonc(content1, { google: {} }) + expect(() => parseJsonc(result1)).not.toThrow() + + // Test Case 2 + const content2 = `{ + "provider": { + // } comment + "openai": {} + } +}` + const result2 = modifyProviderInJsonc(content2, { google: {} }) + expect(() => parseJsonc(result2)).not.toThrow() + + // Test Case 3 + const content3 = `{ + "plugin": ["foo"] +}` + const result3 = modifyProviderInJsonc(content3, { google: {} }) + expect(() => parseJsonc(result3)).not.toThrow() + }) + }) + + describe("Test 8: Trailing commas preserved", () => { + it("file is valid JSONC with trailing commas", () => { + // given + const content = `{ + "provider": { "openai": {}, }, + "plugin": ["foo",], +}` + const newProviderValue = { google: { name: "Google" } } + + // when + const result = modifyProviderInJsonc(content, newProviderValue) + + // then + expect(() => parseJsonc(result)).not.toThrow() + + const parsed = parseJsonc>(result) + expect(parsed).toHaveProperty('plugin') + expect(parsed.plugin).toEqual(['foo']) + }) + }) +}) diff --git a/src/cli/config-manager/add-provider-config.ts b/src/cli/config-manager/add-provider-config.ts index 1e49ab49f..daef30cc2 100644 --- a/src/cli/config-manager/add-provider-config.ts +++ b/src/cli/config-manager/add-provider-config.ts @@ -1,4 +1,4 @@ -import { readFileSync, writeFileSync } from "node:fs" +import { readFileSync, writeFileSync, copyFileSync } from "node:fs" import type { ConfigMergeResult, InstallConfig } from "../types" import { getConfigDir } from "./config-context" import { ensureConfigDirectoryExists } from "./ensure-config-directory-exists" @@ -6,6 +6,8 @@ import { formatErrorWithSuggestion } from "./format-error-with-suggestion" import { detectConfigFormat } from "./opencode-config-format" import { parseOpenCodeConfigFileWithError, type OpenCodeConfig } from "./parse-opencode-config-file" import { ANTIGRAVITY_PROVIDER_CONFIG } from "./antigravity-provider-configuration" +import { modifyProviderInJsonc } from "./jsonc-provider-editor" +import { parseJsonc } from "../../shared/jsonc-parser" export function addProviderConfig(config: InstallConfig): ConfigMergeResult { try { @@ -47,56 +49,25 @@ export function addProviderConfig(config: InstallConfig): ConfigMergeResult { if (format === "jsonc") { const content = readFileSync(path, "utf-8") - const providerJson = JSON.stringify(newConfig.provider, null, 2) - .split("\n") - .map((line, i) => (i === 0 ? line : ` ${line}`)) - .join("\n") - // Match "provider" key with any indentation and nested brace depth - const providerIdx = content.indexOf('"provider"') - if (providerIdx !== -1) { - const colonIdx = content.indexOf(":", providerIdx + '"provider"'.length) - const braceStart = colonIdx !== -1 ? content.indexOf("{", colonIdx) : -1 - if (braceStart === -1) { - writeFileSync(path, JSON.stringify(newConfig, null, 2) + "\n") - } else { - let depth = 0 - let braceEnd = braceStart - let inString = false - let escape = false - for (let i = braceStart; i < content.length; i++) { - const ch = content[i] - if (escape) { - escape = false - continue - } - if (ch === "\\") { - escape = true - continue - } - if (ch === '"') { - inString = !inString - continue - } - if (inString) continue - if (ch === "{") depth++ - else if (ch === "}") { - depth-- - if (depth === 0) { - braceEnd = i - break - } - } - } - const newContent = - content.slice(0, providerIdx) + - `"provider": ${providerJson}` + - content.slice(braceEnd + 1) - writeFileSync(path, newContent) + + // Backup original file + copyFileSync(path, `${path}.bak`) + + const providerValue = (newConfig.provider ?? {}) as Record + const newContent = modifyProviderInJsonc(content, providerValue) + + // Post-write validation + try { + parseJsonc(newContent) + } catch (error) { + return { + success: false, + configPath: path, + error: `Generated JSONC is invalid: ${error instanceof Error ? error.message : String(error)}`, } - } else { - const newContent = content.replace(/(\{)/, `$1\n "provider": ${providerJson},`) - writeFileSync(path, newContent) } + + writeFileSync(path, newContent) } else { writeFileSync(path, JSON.stringify(newConfig, null, 2) + "\n") } diff --git a/src/cli/config-manager/jsonc-provider-editor.ts b/src/cli/config-manager/jsonc-provider-editor.ts new file mode 100644 index 000000000..3f53c6d13 --- /dev/null +++ b/src/cli/config-manager/jsonc-provider-editor.ts @@ -0,0 +1,11 @@ +import { modify, applyEdits } from "jsonc-parser" + +export function modifyProviderInJsonc( + content: string, + newProviderValue: Record +): string { + const edits = modify(content, ["provider"], newProviderValue, { + formattingOptions: { tabSize: 2, insertSpaces: true }, + }) + return applyEdits(content, edits) +}