diff --git a/apps/memos-local-openclaw/index.ts b/apps/memos-local-openclaw/index.ts index 5e2245198..711d7022c 100644 --- a/apps/memos-local-openclaw/index.ts +++ b/apps/memos-local-openclaw/index.ts @@ -31,6 +31,7 @@ import { SkillInstaller } from "./src/skill/installer"; import { Summarizer } from "./src/ingest/providers"; import { MEMORY_GUIDE_SKILL_MD } from "./src/skill/bundled-memory-guide"; import { Telemetry } from "./src/telemetry"; +import { patchToolsAllow, parseJsonWithComments } from "./src/shared/json5-lite"; /** Remove near-duplicate hits based on summary word overlap (>70%). Keeps first (highest-scored) hit. */ @@ -356,14 +357,17 @@ const memosLocalPlugin = { const openclawJsonPath = path.join(stateDir, "openclaw.json"); if (fs.existsSync(openclawJsonPath)) { const raw = fs.readFileSync(openclawJsonPath, "utf-8"); - const cfg = JSON.parse(raw); + // openclaw.json is JSON5: comments and trailing commas are legal. + // Parse via the JSON5-tolerant helper (writeback below is a targeted + // regex replace on `raw`, so comments are preserved on round-trip). + const cfg = parseJsonWithComments<{ tools?: { allow?: string[] } }>(raw); const allow: string[] | undefined = cfg?.tools?.allow; if (Array.isArray(allow) && allow.length > 0 && !allow.includes("group:plugins") && !allow.includes("*")) { - const lastEntry = JSON.stringify(allow[allow.length - 1]); - const patched = raw.replace( - new RegExp(`(${lastEntry})(\\s*\\])`), - `$1,\n "group:plugins"$2`, - ); + const lastEntry = allow[allow.length - 1]; + // Anchored to the `tools.allow` array span (string-literal-aware + // bracket matching) and with regex metacharacters in `lastEntry` + // escaped — see #1377 for what happens when this is global. + const patched = patchToolsAllow(raw, lastEntry, "group:plugins"); if (patched !== raw && patched.includes("group:plugins")) { fs.writeFileSync(openclawJsonPath, patched, "utf-8"); ctx.log.info("memos-local: added 'group:plugins' to tools.allow in openclaw.json"); diff --git a/apps/memos-local-openclaw/src/shared/json5-lite.ts b/apps/memos-local-openclaw/src/shared/json5-lite.ts new file mode 100644 index 000000000..5ea73864c --- /dev/null +++ b/apps/memos-local-openclaw/src/shared/json5-lite.ts @@ -0,0 +1,222 @@ +/** + * Lightweight JSON5-tolerant parser. + * + * `openclaw.json` is JSON5: line/block comments and trailing commas are legal. + * The standard `JSON.parse` chokes on those, so anywhere we *read* `openclaw.json` + * we go through this helper first. + * + * This is a deliberately small shim, not a full JSON5 implementation: + * - strips `// …` line comments (string-literal aware) + * - strips `/* … *\/` block comments (string-literal aware, preserves newline count) + * - strips trailing commas before `]` and `}` (string-literal aware) + * - delegates to `JSON.parse` + * + * It does NOT support unquoted keys, single-quoted strings, hex literals, etc. + * Comments are by far the dominant JSON5 affordance users hit (issue #1543); + * the rest can be added if a real case shows up. + * + * NOTE: this helper is read-only. It cannot round-trip — any writeback path + * must operate on the original raw text (e.g. targeted regex replace) to + * preserve the user's comments and formatting. + */ +export function parseJsonWithComments(text: string): T { + return JSON.parse(stripJsonComments(text)) as T; +} + +/** + * Strip `//` line comments, `/* *\/` block comments, and trailing commas from + * a JSON-ish string. String literals (including escaped quotes) are left alone. + * + * Implemented as a single string-literal-aware character scan so that comma + * stripping can't accidentally rewrite content inside strings (e.g. a value + * like `",]"` must round-trip untouched), and so block comments preserve the + * newline count for accurate `JSON.parse` error line numbers. + * + * Exported for tests; prefer `parseJsonWithComments` for normal use. + */ +export function stripJsonComments(text: string): string { + let out = ""; + let i = 0; + const n = text.length; + + while (i < n) { + const ch = text[i]; + const next = i + 1 < n ? text[i + 1] : ""; + + // ─── String literal ───────────────────────────────────────────────── + if (ch === '"' || ch === "'") { + const quote = ch; + out += ch; + i += 1; + while (i < n) { + const sch = text[i]; + if (sch === "\\" && i + 1 < n) { + // Preserve escape sequences verbatim (e.g. \", \\, \n). + out += sch + text[i + 1]; + i += 2; + continue; + } + out += sch; + i += 1; + if (sch === quote) break; + } + continue; + } + + // ─── Line comment: `// …` to end-of-line ──────────────────────────── + if (ch === "/" && next === "/") { + i += 2; + while (i < n && text[i] !== "\n") i += 1; + // Leave the newline so line numbers stay aligned in error messages. + continue; + } + + // ─── Block comment: `/* … */` ─────────────────────────────────────── + // Preserve the newline count so JSON.parse error line numbers continue + // to align with the original source. + if (ch === "/" && next === "*") { + i += 2; + let newlines = 0; + while (i < n && !(text[i] === "*" && text[i + 1] === "/")) { + if (text[i] === "\n") newlines += 1; + i += 1; + } + i += 2; // skip closing `*/` + if (newlines > 0) out += "\n".repeat(newlines); + continue; + } + + // ─── Trailing comma: `,` followed by ws then `]` or `}` ───────────── + if (ch === ",") { + let j = i + 1; + while (j < n && (text[j] === " " || text[j] === "\t" || text[j] === "\n" || text[j] === "\r")) { + j += 1; + } + if (j < n && (text[j] === "]" || text[j] === "}")) { + // Drop the comma but keep the whitespace untouched so line numbers + // and indentation are preserved. + i += 1; + continue; + } + } + + out += ch; + i += 1; + } + + return out; +} + +/** + * Find the index of the closing brace/bracket that matches the opening one at + * `openIdx`. Respects string literals (including `\"` escapes) and skips + * line/block comments so it works on JSON5-ish text. Returns -1 if no match. + * + * `text[openIdx]` must equal `open`. + */ +export function findMatchingDelimiter( + text: string, + openIdx: number, + open: "{" | "[", + close: "}" | "]", +): number { + if (text[openIdx] !== open) return -1; + const n = text.length; + let depth = 0; + let i = openIdx; + while (i < n) { + const ch = text[i]; + const next = i + 1 < n ? text[i + 1] : ""; + + // String literal — skip past it without counting delimiters inside. + if (ch === '"' || ch === "'") { + const quote = ch; + i += 1; + while (i < n) { + const sch = text[i]; + if (sch === "\\" && i + 1 < n) { + i += 2; + continue; + } + i += 1; + if (sch === quote) break; + } + continue; + } + + // Line comment. + if (ch === "/" && next === "/") { + i += 2; + while (i < n && text[i] !== "\n") i += 1; + continue; + } + + // Block comment. + if (ch === "/" && next === "*") { + i += 2; + while (i < n && !(text[i] === "*" && text[i + 1] === "/")) i += 1; + i += 2; + continue; + } + + if (ch === open) { + depth += 1; + } + else if (ch === close) { + depth -= 1; + if (depth === 0) return i; + } + i += 1; + } + return -1; +} + +/** Escape regex metacharacters in `s` so it can be safely embedded in a `new RegExp(...)`. */ +export function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Add `newEntry` to the `tools.allow` array in a JSON5 openclaw config string, + * inserting it after the existing last entry. The patch is anchored to the + * `tools.allow` array span (located via brace/bracket matching that respects + * string literals and comments) so it cannot stray into other arrays + * elsewhere in the file — the root cause of #1377. + * + * `lastEntry` is regex-escaped before being interpolated, so tool names that + * contain regex metacharacters (`.`, `+`, `?`, `(`, `\\`, ...) match correctly. + * + * Returns the patched text, or the original unchanged if the structure can't + * be located or the last entry can't be matched. + */ +export function patchToolsAllow(raw: string, lastEntry: string, newEntry: string): string { + // 1. Locate `"tools"\s*:\s*{` + const toolsMatch = raw.match(/"tools"\s*:\s*\{/); + if (!toolsMatch || toolsMatch.index === undefined) return raw; + const toolsBraceIdx = toolsMatch.index + toolsMatch[0].length - 1; // index of `{` + // 2. Find balanced `}` for the tools object. + const toolsEnd = findMatchingDelimiter(raw, toolsBraceIdx, "{", "}"); + if (toolsEnd < 0) return raw; + + // 3. Locate `"allow"\s*:\s*[` *within* the tools block. + const toolsBlock = raw.slice(toolsBraceIdx, toolsEnd); + const allowMatch = toolsBlock.match(/"allow"\s*:\s*\[/); + if (!allowMatch || allowMatch.index === undefined) return raw; + const allowBracketIdx = toolsBraceIdx + allowMatch.index + allowMatch[0].length - 1; // index of `[` + // 4. Find balanced `]` for the allow array. + const allowEnd = findMatchingDelimiter(raw, allowBracketIdx, "[", "]"); + if (allowEnd < 0) return raw; + + // 5. Operate only on the array's contents (between the brackets). + const allowContentStart = allowBracketIdx + 1; + const arrayContent = raw.slice(allowContentStart, allowEnd); + + const escapedLast = escapeRegExp(JSON.stringify(lastEntry)); + // Match the last entry, then optional trailing comma + whitespace, anchored + // at the end of the array contents (just before the closing `]`). + const re = new RegExp(`(${escapedLast})\\s*,?(\\s*)$`); + if (!re.test(arrayContent)) return raw; + + const patched = arrayContent.replace(re, `$1,\n ${JSON.stringify(newEntry)}$2`); + return raw.slice(0, allowContentStart) + patched + raw.slice(allowEnd); +} diff --git a/apps/memos-local-openclaw/tests/json5-lite.test.ts b/apps/memos-local-openclaw/tests/json5-lite.test.ts new file mode 100644 index 000000000..a46852801 --- /dev/null +++ b/apps/memos-local-openclaw/tests/json5-lite.test.ts @@ -0,0 +1,220 @@ +import { describe, expect, it } from "vitest"; +import { + escapeRegExp, + findMatchingDelimiter, + parseJsonWithComments, + patchToolsAllow, + stripJsonComments, +} from "../src/shared/json5-lite"; + +describe("parseJsonWithComments", () => { + it("parses plain JSON unchanged (regression)", () => { + expect(parseJsonWithComments(`{"a":1,"b":[2,3]}`)).toEqual({ a: 1, b: [2, 3] }); + }); + + it("tolerates // line comments", () => { + const src = `{ + // a leading comment + "tools": { + "allow": ["task-cli"] // trailing inline note + } + }`; + expect(parseJsonWithComments(src)).toEqual({ tools: { allow: ["task-cli"] } }); + }); + + it("tolerates /* block */ comments", () => { + const src = `{ + /* multi-line + block comment */ + "tools": { "allow": ["a"] } + }`; + expect(parseJsonWithComments(src)).toEqual({ tools: { allow: ["a"] } }); + }); + + it("tolerates trailing commas in arrays and objects", () => { + const src = `{ + "tools": { + "allow": [ + "a", + "b", + ], + }, + }`; + expect(parseJsonWithComments(src)).toEqual({ tools: { allow: ["a", "b"] } }); + }); + + it("does not touch comment-like sequences inside string literals", () => { + const src = `{ "url": "https://example.com/a//b", "note": "/* not a comment */" }`; + expect(parseJsonWithComments(src)).toEqual({ + url: "https://example.com/a//b", + note: "/* not a comment */", + }); + }); + + it("respects escaped quotes inside strings", () => { + const src = `{ "q": "he said \\"hi\\" // not a comment" }`; + expect(parseJsonWithComments(src)).toEqual({ q: 'he said "hi" // not a comment' }); + }); + + it("does not strip ',]' or ',}' that appear inside string values", () => { + // Trailing-comma stripping must be string-literal aware. + expect(parseJsonWithComments(`{"a": ",]"}`)).toEqual({ a: ",]" }); + expect(parseJsonWithComments(`{"a": ",}"}`)).toEqual({ a: ",}" }); + expect(parseJsonWithComments(`{"a": "literal , ] inside"}`)).toEqual({ a: "literal , ] inside" }); + }); + + it("handles the openclaw.json shape from issue #1543", () => { + // Mirrors the structure in the bug report: comments scattered through a + // realistic config, including in/around tools.allow. + const src = `{ + // top-level openclaw config + "tools": { + "allow": [ + "task-cli", // first tool + "memos", + /* a block-comment listed mid-array */ + "summarizer", + ], + }, + "agents": { "defaults": { "model": "primary" } }, // trailing object comma too + }`; + const parsed = parseJsonWithComments<{ tools: { allow: string[] } }>(src); + expect(parsed.tools.allow).toEqual(["task-cli", "memos", "summarizer"]); + }); +}); + +describe("stripJsonComments", () => { + it("preserves newlines so line numbers stay aligned in error messages", () => { + const src = "{\n// foo\n\"a\":1\n}"; + const stripped = stripJsonComments(src); + // The `// foo` line becomes empty but the newline is retained. + expect(stripped.split("\n").length).toBe(src.split("\n").length); + }); + + it("preserves newline count when stripping multi-line block comments", () => { + // A block comment spanning N newlines should leave N newlines behind so + // JSON.parse error line numbers stay aligned with the original source. + const src = "{\n/* line1\nline2\nline3 */\n\"a\":1\n}"; + const stripped = stripJsonComments(src); + expect(stripped.split("\n").length).toBe(src.split("\n").length); + // And the parse should still succeed. + expect(JSON.parse(stripped)).toEqual({ a: 1 }); + }); +}); + +describe("escapeRegExp", () => { + it("escapes regex metacharacters", () => { + const s = "a.b+c?d(e)f[g]h{i}j|k^l$m*n\\o"; + const re = new RegExp(`^${escapeRegExp(s)}$`); + expect(re.test(s)).toBe(true); + // And it shouldn't match a different string of the same length. + expect(re.test("aXbYcZdWeVfUgThSiRjQkPlOmNnM\\o")).toBe(false); + }); +}); + +describe("findMatchingDelimiter", () => { + it("matches a simple object", () => { + const s = "x{a:1}y"; + expect(findMatchingDelimiter(s, 1, "{", "}")).toBe(5); + }); + + it("ignores delimiters inside string literals", () => { + const s = `{"v": "}"}`; + expect(findMatchingDelimiter(s, 0, "{", "}")).toBe(s.length - 1); + }); + + it("ignores delimiters inside line and block comments", () => { + const s = "{ // }\n /* } */ \"k\":1 }"; + expect(findMatchingDelimiter(s, 0, "{", "}")).toBe(s.length - 1); + }); + + it("matches nested arrays", () => { + const s = "[[1, [2, 3]], 4]"; + expect(findMatchingDelimiter(s, 0, "[", "]")).toBe(s.length - 1); + }); +}); + +describe("patchToolsAllow", () => { + it("appends a new entry after the last one", () => { + const raw = `{ + "tools": { + "allow": [ + "task-cli", + "memos" + ] + } +}`; + const out = patchToolsAllow(raw, "memos", "group:plugins"); + const parsed = parseJsonWithComments<{ tools: { allow: string[] } }>(out); + expect(parsed.tools.allow).toEqual(["task-cli", "memos", "group:plugins"]); + }); + + it("preserves comments and trailing commas in the surrounding file", () => { + const raw = `{ + // top of file + "tools": { + "allow": [ + "task-cli", // first + "memos", + ], + }, +}`; + const out = patchToolsAllow(raw, "memos", "group:plugins"); + expect(out).toContain("// top of file"); + expect(out).toContain("// first"); + const parsed = parseJsonWithComments<{ tools: { allow: string[] } }>(out); + expect(parsed.tools.allow).toEqual(["task-cli", "memos", "group:plugins"]); + }); + + it("regression #1377: does not corrupt other arrays whose last element matches", () => { + // An earlier array (`other.list`) ends with the same string ("memos") as + // `tools.allow`. The previous global `raw.replace(...)` would rewrite the + // FIRST match it found, corrupting `other.list` instead of `tools.allow`. + const raw = `{ + "other": { + "list": [ + "alpha", + "memos" + ] + }, + "tools": { + "allow": [ + "task-cli", + "memos" + ] + } +}`; + const out = patchToolsAllow(raw, "memos", "group:plugins"); + const parsed = parseJsonWithComments<{ + other: { list: string[] }; + tools: { allow: string[] }; + }>(out); + // `other.list` must be untouched. + expect(parsed.other.list).toEqual(["alpha", "memos"]); + // And `tools.allow` got the new entry. + expect(parsed.tools.allow).toEqual(["task-cli", "memos", "group:plugins"]); + }); + + it("escapes regex metacharacters in the last entry (e.g. tool names with dots/parens)", () => { + const raw = `{ + "tools": { + "allow": [ + "first.tool", + "weird(name).tool+v2" + ] + } +}`; + const out = patchToolsAllow(raw, "weird(name).tool+v2", "group:plugins"); + const parsed = parseJsonWithComments<{ tools: { allow: string[] } }>(out); + expect(parsed.tools.allow).toEqual([ + "first.tool", + "weird(name).tool+v2", + "group:plugins", + ]); + }); + + it("returns the input unchanged when tools.allow can't be located", () => { + const raw = `{ "agents": { "defaults": {} } }`; + expect(patchToolsAllow(raw, "anything", "group:plugins")).toBe(raw); + }); +});