fix(cli-tools): tolerate JSONC (comments, trailing commas) in tool settings

The `readSettings`/`readConfig`/`readAuth` helpers in `claude`, `cline`,
`droid`, `kilo` and `openclaw` settings routes were re-throwing on any
non-`ENOENT` error, so a config file with a trailing comma (valid JSONC,
emitted by some upstream CLIs) crashed the GET route with a 500 and the
dashboard misread it as "tool not installed". A new shared
`readJsoncConfig` helper strips trailing commas before parsing and
returns `null` on any parse failure, so the dashboard correctly renders
"installed but not configured" instead.

Co-authored-by: Zireael <3856578+Zireael@users.noreply.github.com>
Inspired-by: https://github.com/decolua/9router/commit/6c10edf8
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-22 12:39:13 -03:00
parent d2aa67a11c
commit f218f965e3
7 changed files with 211 additions and 63 deletions

View File

@@ -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<T = unknown>(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<T = unknown>(
path: string,
fallback: T | null = null
): Promise<T | null> {
let content: string;
try {
content = await fs.readFile(path, "utf-8");
} catch {
return fallback;
}
const parsed = parseJsoncOrNull<T>(content);
return parsed ?? fallback;
}

View File

@@ -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

View File

@@ -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<Record<string, unknown>>(SECRETS_PATH, {});
// Check if OmniRoute is configured as OpenAI-compatible provider
const hasOmniRouteConfig = (globalState: any) => {

View File

@@ -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) => {

View File

@@ -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) => {

View File

@@ -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) => {

View File

@@ -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`
);
}
});