diff --git a/src/app/api/cli-tools/_lib/jsoncConfig.ts b/src/app/api/cli-tools/_lib/jsoncConfig.ts new file mode 100644 index 0000000000..6141801737 --- /dev/null +++ b/src/app/api/cli-tools/_lib/jsoncConfig.ts @@ -0,0 +1,51 @@ +/** + * Shared JSONC-tolerant config reader for CLI-tools settings routes. + * + * Background: several upstream CLI tools (opencode, kilo, droid, cline, etc.) + * ship config files that are JSON with the occasional trailing comma or a + * stray comment — valid JSONC, but `JSON.parse()` rejects them with a + * `SyntaxError`. Until this helper, every `readSettings`/`readConfig` helper + * only caught `ENOENT` and re-threw, surfacing as a 500 that the dashboard + * misread as "tool not installed". + * + * Behaviour: + * - strip trailing commas before parsing so JSONC files load cleanly; + * - on ANY read or parse failure, return the caller-supplied fallback + * (typically `null` or `{}`) instead of throwing, so the dashboard shows + * "installed but not configured" rather than "not installed". + * + * Ported from upstream `decolua/9router@6c10edf8`. Co-authored-by: Zireael. + */ +import { promises as fs } from "node:fs"; + +/** + * Parse a JSON/JSONC string, returning `null` on syntax errors instead of + * throwing. Trailing commas before `}` or `]` are stripped before parsing. + */ +export function parseJsoncOrNull(content: string): T | null { + try { + const stripped = content.replace(/,(\s*[}\]])/g, "$1"); + return JSON.parse(stripped) as T; + } catch { + return null; + } +} + +/** + * Read a JSON/JSONC config file. Returns `fallback` (default: `null`) on any + * filesystem or parse error so callers can render an "installed but not + * configured" state instead of crashing with a 500. + */ +export async function readJsoncConfig( + path: string, + fallback: T | null = null +): Promise { + let content: string; + try { + content = await fs.readFile(path, "utf-8"); + } catch { + return fallback; + } + const parsed = parseJsoncOrNull(content); + return parsed ?? fallback; +} diff --git a/src/app/api/cli-tools/claude-settings/route.ts b/src/app/api/cli-tools/claude-settings/route.ts index ab11635644..1a3d735f3f 100644 --- a/src/app/api/cli-tools/claude-settings/route.ts +++ b/src/app/api/cli-tools/claude-settings/route.ts @@ -15,22 +15,18 @@ import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db import { cliSettingsEnvSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { getApiKeyById } from "@/lib/localDb"; +import { readJsoncConfig } from "../_lib/jsoncConfig"; // Get claude settings path based on OS const getClaudeSettingsPath = () => getCliPrimaryConfigPath("claude"); -// Read current settings +// Read current settings. +// Ported from upstream decolua/9router@6c10edf8: tolerate JSONC (trailing +// commas) and return null on any parse error so the dashboard renders +// "installed but not configured" instead of a 500 misread as "not installed". const readSettings = async () => { - try { - const settingsPath = getClaudeSettingsPath(); - const content = await fs.readFile(settingsPath, "utf-8"); - return JSON.parse(content); - } catch (error: any) { - if (error.code === "ENOENT") { - return null; - } - throw error; - } + const settingsPath = getClaudeSettingsPath(); + return readJsoncConfig(settingsPath); }; // GET - Check claude CLI and read current settings diff --git a/src/app/api/cli-tools/cline-settings/route.ts b/src/app/api/cli-tools/cline-settings/route.ts index 090dcaf920..5b8dd37cd7 100644 --- a/src/app/api/cli-tools/cline-settings/route.ts +++ b/src/app/api/cli-tools/cline-settings/route.ts @@ -11,32 +11,20 @@ import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db import { cliModelConfigSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { resolveApiKey } from "@/shared/services/apiKeyResolver"; +import { readJsoncConfig } from "../_lib/jsoncConfig"; const CLINE_DATA_DIR = path.join(os.homedir(), ".cline", "data"); const GLOBAL_STATE_PATH = path.join(CLINE_DATA_DIR, "globalState.json"); const SECRETS_PATH = path.join(CLINE_DATA_DIR, "secrets.json"); -// Read globalState.json -const readGlobalState = async () => { - try { - const content = await fs.readFile(GLOBAL_STATE_PATH, "utf-8"); - return JSON.parse(content); - } catch (error: any) { - if (error.code === "ENOENT") return null; - throw error; - } -}; +// Read globalState.json. +// Ported from upstream decolua/9router@6c10edf8: tolerate JSONC (trailing +// commas) and return null on any parse error so the dashboard renders +// "installed but not configured" instead of a 500 misread as "not installed". +const readGlobalState = async () => readJsoncConfig(GLOBAL_STATE_PATH); -// Read secrets.json -const readSecrets = async () => { - try { - const content = await fs.readFile(SECRETS_PATH, "utf-8"); - return JSON.parse(content); - } catch (error: any) { - if (error.code === "ENOENT") return {}; - throw error; - } -}; +// Read secrets.json (same JSONC-tolerant behaviour; defaults to {} for compat). +const readSecrets = async () => readJsoncConfig>(SECRETS_PATH, {}); // Check if OmniRoute is configured as OpenAI-compatible provider const hasOmniRouteConfig = (globalState: any) => { diff --git a/src/app/api/cli-tools/droid-settings/route.ts b/src/app/api/cli-tools/droid-settings/route.ts index c7e15f42a0..7313faef81 100644 --- a/src/app/api/cli-tools/droid-settings/route.ts +++ b/src/app/api/cli-tools/droid-settings/route.ts @@ -14,21 +14,16 @@ import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db import { cliModelConfigSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { resolveApiKey } from "@/shared/services/apiKeyResolver"; +import { readJsoncConfig } from "../_lib/jsoncConfig"; const getDroidSettingsPath = () => getCliPrimaryConfigPath("droid"); const getDroidDir = () => path.dirname(getDroidSettingsPath()); -// Read current settings.json -const readSettings = async () => { - try { - const settingsPath = getDroidSettingsPath(); - const content = await fs.readFile(settingsPath, "utf-8"); - return JSON.parse(content); - } catch (error: any) { - if (error.code === "ENOENT") return null; - throw error; - } -}; +// Read current settings.json. +// Ported from upstream decolua/9router@6c10edf8: tolerate JSONC (trailing +// commas) and return null on any parse error so the dashboard renders +// "installed but not configured" instead of a 500 misread as "not installed". +const readSettings = async () => readJsoncConfig(getDroidSettingsPath()); // Check if settings has OmniRoute customModels const hasOmniRouteConfig = (settings: any) => { diff --git a/src/app/api/cli-tools/kilo-settings/route.ts b/src/app/api/cli-tools/kilo-settings/route.ts index aac82bebd7..965ac03bea 100644 --- a/src/app/api/cli-tools/kilo-settings/route.ts +++ b/src/app/api/cli-tools/kilo-settings/route.ts @@ -11,21 +11,17 @@ import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db import { cliModelConfigSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { resolveApiKey } from "@/shared/services/apiKeyResolver"; +import { readJsoncConfig } from "../_lib/jsoncConfig"; const KILO_DATA_DIR = path.join(os.homedir(), ".local", "share", "kilo"); const AUTH_PATH = path.join(KILO_DATA_DIR, "auth.json"); const KILO_CONFIG_DIR = path.join(os.homedir(), ".config", "kilo"); -// Read auth.json -const readAuth = async () => { - try { - const content = await fs.readFile(AUTH_PATH, "utf-8"); - return JSON.parse(content); - } catch (error) { - if (error.code === "ENOENT") return null; - throw error; - } -}; +// Read auth.json. +// Ported from upstream decolua/9router@6c10edf8: tolerate JSONC (trailing +// commas) and return null on any parse error so the dashboard renders +// "installed but not configured" instead of a 500 misread as "not installed". +const readAuth = async () => readJsoncConfig(AUTH_PATH); // Check if OmniRoute OpenAI-compatible provider is configured const hasOmniRouteConfig = (auth) => { diff --git a/src/app/api/cli-tools/openclaw-settings/route.ts b/src/app/api/cli-tools/openclaw-settings/route.ts index 3b6e37d0d4..19ca818804 100644 --- a/src/app/api/cli-tools/openclaw-settings/route.ts +++ b/src/app/api/cli-tools/openclaw-settings/route.ts @@ -14,21 +14,16 @@ import { saveCliToolLastConfigured, deleteCliToolLastConfigured } from "@/lib/db import { cliModelConfigSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; import { resolveApiKey } from "@/shared/services/apiKeyResolver"; +import { readJsoncConfig } from "../_lib/jsoncConfig"; const getOpenClawSettingsPath = () => getCliPrimaryConfigPath("openclaw"); const getOpenClawDir = () => path.dirname(getOpenClawSettingsPath()); -// Read current settings.json -const readSettings = async () => { - try { - const settingsPath = getOpenClawSettingsPath(); - const content = await fs.readFile(settingsPath, "utf-8"); - return JSON.parse(content); - } catch (error: any) { - if (error.code === "ENOENT") return null; - throw error; - } -}; +// Read current settings.json. +// Ported from upstream decolua/9router@6c10edf8: tolerate JSONC (trailing +// commas) and return null on any parse error so the dashboard renders +// "installed but not configured" instead of a 500 misread as "not installed". +const readSettings = async () => readJsoncConfig(getOpenClawSettingsPath()); // Check if settings has OmniRoute config const hasOmniRouteConfig = (settings: any) => { diff --git a/tests/unit/cli-tools-settings-jsonc.test.ts b/tests/unit/cli-tools-settings-jsonc.test.ts new file mode 100644 index 0000000000..c259397ec4 --- /dev/null +++ b/tests/unit/cli-tools-settings-jsonc.test.ts @@ -0,0 +1,127 @@ +/** + * Regression test for the JSONC-tolerant config reader used by every + * cli-tools settings route. + * + * Ported from upstream `decolua/9router@6c10edf8`: + * "fix(cli-tools): tolerate JSONC configs in CLI tool settings routes". + * + * Before the fix, `readSettings`/`readConfig` helpers only caught `ENOENT` and + * re-threw on any other error — so a config file with a single trailing + * comma (valid JSONC, emitted by tools like opencode) crashed the GET route + * with a 500 and the dashboard misread the response as "tool not installed". + * + * After the fix: + * - trailing commas are stripped before parsing (JSONC tolerated); + * - any other parse error returns `null` (or the caller's fallback) so the + * dashboard renders "installed but not configured". + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const { parseJsoncOrNull, readJsoncConfig } = await import( + "../../src/app/api/cli-tools/_lib/jsoncConfig.ts" +); + +test("parseJsoncOrNull tolerates trailing commas in objects", () => { + const jsonc = `{ + "model": "gpt-5", + "tools": ["a", "b",], + "nested": { "x": 1, }, + }`; + const parsed = parseJsoncOrNull<{ model: string; tools: string[] }>(jsonc); + assert.ok(parsed, "parser must accept JSONC with trailing commas"); + assert.equal(parsed.model, "gpt-5"); + assert.deepEqual(parsed.tools, ["a", "b"]); +}); + +test("parseJsoncOrNull returns null on truly malformed JSON", () => { + assert.equal(parseJsoncOrNull("{ not json at all"), null); +}); + +test("readJsoncConfig returns fallback when file is missing", async () => { + const missing = path.join(os.tmpdir(), `cli-tools-jsonc-missing-${Date.now()}.json`); + assert.equal(await readJsoncConfig(missing), null); + assert.deepEqual(await readJsoncConfig(missing, {}), {}); +}); + +test("readJsoncConfig parses a JSONC file with trailing commas (regression)", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "cli-tools-jsonc-")); + const file = path.join(dir, "settings.json"); + await fs.writeFile( + file, + `{ + "apiKey": "sk-test", + "model": "claude-sonnet-4-5", +} +`, + "utf-8" + ); + try { + const parsed = await readJsoncConfig<{ apiKey: string; model: string }>(file); + assert.ok(parsed, "JSONC file with trailing comma must NOT crash the reader"); + assert.equal(parsed.apiKey, "sk-test"); + assert.equal(parsed.model, "claude-sonnet-4-5"); + } finally { + await fs.rm(dir, { recursive: true, force: true }); + } +}); + +test("readJsoncConfig returns fallback on corrupted config instead of throwing", async () => { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), "cli-tools-jsonc-bad-")); + const file = path.join(dir, "settings.json"); + await fs.writeFile(file, "{ this is not valid json at all !!! ", "utf-8"); + try { + // Must NOT throw — the dashboard renders "installed but not configured" + // when this returns null, instead of "not installed" on a 500. + assert.equal(await readJsoncConfig(file), null); + assert.deepEqual(await readJsoncConfig(file, {}), {}); + } finally { + await fs.rm(dir, { recursive: true, force: true }); + } +}); + +/** + * Source-guard: every cli-tools settings route's read helper must go through + * the JSONC-tolerant reader. A regression to raw `JSON.parse(content)` inside + * a `readSettings` / `readConfig` helper would re-introduce the original + * 500-on-JSONC bug, so we assert the routes do not contain that pattern. + * + * Same source-guard pattern as `tests/unit/source-guard-*.test.ts`. + */ +test("cli-tools settings routes use the JSONC-tolerant reader (source-guard)", async () => { + // For each route, look for the GET-read helper region (top of file, up to + // the first `export async function GET`) and assert it imports + uses the + // JSONC-tolerant reader instead of raw `JSON.parse(content)`. + const routes = [ + "claude-settings", + "cline-settings", + "droid-settings", + "kilo-settings", + "openclaw-settings", + ]; + const repoRoot = path.resolve(import.meta.dirname ?? ".", "..", ".."); + for (const r of routes) { + const src = await fs.readFile( + path.join(repoRoot, "src", "app", "api", "cli-tools", r, "route.ts"), + "utf-8" + ); + const getIdx = src.indexOf("export async function GET"); + assert.ok(getIdx > 0, `${r}: expected an exported GET handler`); + const head = src.slice(0, getIdx); + assert.ok( + /from\s+["']\.\.\/_lib\/jsoncConfig["']/.test(src), + `${r}: must import readJsoncConfig from ../_lib/jsoncConfig` + ); + assert.ok( + !/JSON\.parse\(\s*content\s*\)/.test(head), + `${r}: read helper still calls raw JSON.parse(content) — port the JSONC fix` + ); + assert.ok( + /readJsoncConfig\s*[<(]/.test(head), + `${r}: read helper must invoke readJsoncConfig` + ); + } +});