From ecf72319ed2a1f189e58ea159d9acecc6d2c7852 Mon Sep 17 00:00:00 2001 From: jacky <820758481@qq.com> Date: Thu, 2 Apr 2026 22:02:04 +0800 Subject: [PATCH] fix: use JSON5 parser for plugin manifest loading (#57734) [AI-assisted] (#59084) Merged via squash. Prepared head SHA: 58a4d537fc7c4d2ca0ed4ad4bdfa8395674f628d Co-authored-by: singleGanghood <179357632+singleGanghood@users.noreply.github.com> Co-authored-by: hxy91819 <8814856+hxy91819@users.noreply.github.com> Reviewed-by: @hxy91819 --- CHANGELOG.md | 1 + src/plugins/bundle-manifest.test.ts | 114 +++++++++++++++++++ src/plugins/bundle-manifest.ts | 3 +- src/plugins/manifest.json5-tolerance.test.ts | 103 +++++++++++++++++ src/plugins/manifest.ts | 3 +- 5 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 src/plugins/manifest.json5-tolerance.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index ad729394151..8ada3d735ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ Docs: https://docs.openclaw.ai - Exec/env: block additional host environment override pivots for package roots, language runtimes, compiler include paths, and credential/config locations so request-scoped exec cannot redirect trusted toolchains or config lookups. (#59233) Thanks @drobison00. - OpenShell/mirror sync: constrain mirror sync to managed roots only so user-added shell roots are no longer overwritten or removed during config synchronization. (#58515) Thanks @eleqtrizit. - Dotenv/workspace overrides: block workspace `.env` files from overriding `OPENCLAW_PINNED_PYTHON` and `OPENCLAW_PINNED_WRITE_PYTHON` so trusted helper interpreters cannot be redirected by repo-local env injection. (#58473) Thanks @eleqtrizit. +- Plugins/install: accept JSON5 syntax in `openclaw.plugin.json` and bundle `plugin.json` manifests during install/validation, so third-party plugins with trailing commas, comments, or unquoted keys no longer fail to install. (#59084) Thanks @singleGanghood. ## 2026.4.1-beta.1 diff --git a/src/plugins/bundle-manifest.test.ts b/src/plugins/bundle-manifest.test.ts index 542161138cb..3bb1b0503ef 100644 --- a/src/plugins/bundle-manifest.test.ts +++ b/src/plugins/bundle-manifest.test.ts @@ -292,6 +292,120 @@ describe("bundle manifest parsing", () => { }); }); + it.each([ + { + name: "accepts JSON5 Codex bundle manifests", + bundleFormat: "codex" as const, + manifestRelativePath: CODEX_BUNDLE_MANIFEST_RELATIVE_PATH, + json5Manifest: `{ + // Bundle name can include comments and trailing commas. + name: "Codex JSON5 Bundle", + skills: "skills", + hooks: "hooks", +}`, + dirs: ["skills", "hooks"], + expected: { + id: "codex-json5-bundle", + name: "Codex JSON5 Bundle", + bundleFormat: "codex", + skills: ["skills"], + hooks: ["hooks"], + }, + }, + { + name: "accepts JSON5 Claude bundle manifests", + bundleFormat: "claude" as const, + manifestRelativePath: CLAUDE_BUNDLE_MANIFEST_RELATIVE_PATH, + json5Manifest: `{ + name: "Claude JSON5 Bundle", + commands: "commands-pack", + hooks: "hooks-pack", + outputStyles: "styles", +}`, + dirs: [".claude-plugin", "commands-pack", "hooks-pack", "styles"], + expected: { + id: "claude-json5-bundle", + name: "Claude JSON5 Bundle", + bundleFormat: "claude", + skills: ["commands-pack", "styles"], + hooks: ["hooks-pack"], + }, + }, + { + name: "accepts JSON5 Cursor bundle manifests", + bundleFormat: "cursor" as const, + manifestRelativePath: CURSOR_BUNDLE_MANIFEST_RELATIVE_PATH, + json5Manifest: `{ + name: "Cursor JSON5 Bundle", + commands: ".cursor/commands", + mcpServers: "./.mcp.json", +}`, + dirs: [".cursor-plugin", "skills", ".cursor/commands"], + textFiles: { + ".mcp.json": "{ servers: {}, }", + }, + expected: { + id: "cursor-json5-bundle", + name: "Cursor JSON5 Bundle", + bundleFormat: "cursor", + skills: ["skills", ".cursor/commands"], + hooks: [], + }, + }, + ] as const)( + "$name", + ({ bundleFormat, manifestRelativePath, json5Manifest, dirs, textFiles, expected }) => { + const rootDir = makeTempDir(); + setupBundleFixture({ + rootDir, + dirs: [path.dirname(manifestRelativePath), ...dirs], + textFiles: { + [manifestRelativePath]: json5Manifest, + ...textFiles, + }, + }); + + expectBundleManifest({ + rootDir, + bundleFormat, + expected, + }); + }, + ); + + it.each([ + { + name: "rejects JSON5 Codex bundle manifests that parse to non-objects", + bundleFormat: "codex" as const, + manifestRelativePath: CODEX_BUNDLE_MANIFEST_RELATIVE_PATH, + }, + { + name: "rejects JSON5 Claude bundle manifests that parse to non-objects", + bundleFormat: "claude" as const, + manifestRelativePath: CLAUDE_BUNDLE_MANIFEST_RELATIVE_PATH, + }, + { + name: "rejects JSON5 Cursor bundle manifests that parse to non-objects", + bundleFormat: "cursor" as const, + manifestRelativePath: CURSOR_BUNDLE_MANIFEST_RELATIVE_PATH, + }, + ] as const)("$name", ({ bundleFormat, manifestRelativePath }) => { + const rootDir = makeTempDir(); + setupBundleFixture({ + rootDir, + dirs: [path.dirname(manifestRelativePath)], + textFiles: { + [manifestRelativePath]: "'still not an object'", + }, + }); + + const result = loadBundleManifest({ rootDir, bundleFormat }); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("plugin manifest must be an object"); + } + }); + it.each([ { name: "resolves Claude bundle hooks from default and declared paths", diff --git a/src/plugins/bundle-manifest.ts b/src/plugins/bundle-manifest.ts index 8bf8b67bb45..c77791db793 100644 --- a/src/plugins/bundle-manifest.ts +++ b/src/plugins/bundle-manifest.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import JSON5 from "json5"; import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; import { DEFAULT_PLUGIN_ENTRY_CANDIDATES, PLUGIN_MANIFEST_FILENAME } from "./manifest.js"; @@ -117,7 +118,7 @@ function loadBundleManifestFile(params: { }); } try { - const raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; + const raw = JSON5.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; if (!isRecord(raw)) { return { ok: false, error: "plugin manifest must be an object", manifestPath }; } diff --git a/src/plugins/manifest.json5-tolerance.test.ts b/src/plugins/manifest.json5-tolerance.test.ts new file mode 100644 index 00000000000..815026c13ca --- /dev/null +++ b/src/plugins/manifest.json5-tolerance.test.ts @@ -0,0 +1,103 @@ +import fs from "node:fs"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { loadPluginManifest } from "./manifest.js"; +import { cleanupTrackedTempDirs, makeTrackedTempDir } from "./test-helpers/fs-fixtures.js"; + +const tempDirs: string[] = []; + +function makeTempDir() { + return makeTrackedTempDir("openclaw-manifest-json5", tempDirs); +} + +afterEach(() => { + cleanupTrackedTempDirs(tempDirs); +}); + +describe("loadPluginManifest JSON5 tolerance", () => { + it("parses a standard JSON manifest without issues", () => { + const dir = makeTempDir(); + const manifest = { + id: "demo", + configSchema: { type: "object" }, + }; + fs.writeFileSync( + path.join(dir, "openclaw.plugin.json"), + JSON.stringify(manifest, null, 2), + "utf-8", + ); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.manifest.id).toBe("demo"); + } + }); + + it("parses a manifest with trailing commas", () => { + const dir = makeTempDir(); + const json5Content = `{ + "id": "hindsight", + "configSchema": { + "type": "object", + "properties": { + "apiKey": { "type": "string" }, + }, + }, +}`; + fs.writeFileSync(path.join(dir, "openclaw.plugin.json"), json5Content, "utf-8"); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.manifest.id).toBe("hindsight"); + } + }); + + it("parses a manifest with single-line comments", () => { + const dir = makeTempDir(); + const json5Content = `{ + // Plugin identifier + "id": "commented-plugin", + "configSchema": { "type": "object" } +}`; + fs.writeFileSync(path.join(dir, "openclaw.plugin.json"), json5Content, "utf-8"); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.manifest.id).toBe("commented-plugin"); + } + }); + + it("parses a manifest with unquoted property names", () => { + const dir = makeTempDir(); + const json5Content = `{ + id: "unquoted-keys", + configSchema: { type: "object" } +}`; + fs.writeFileSync(path.join(dir, "openclaw.plugin.json"), json5Content, "utf-8"); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(true); + if (result.ok) { + expect(result.manifest.id).toBe("unquoted-keys"); + } + }); + + it("still rejects completely invalid syntax", () => { + const dir = makeTempDir(); + fs.writeFileSync(path.join(dir, "openclaw.plugin.json"), "not json at all {{{}}", "utf-8"); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("failed to parse plugin manifest"); + } + }); + + it("rejects JSON5 values that parse but are not objects", () => { + const dir = makeTempDir(); + fs.writeFileSync(path.join(dir, "openclaw.plugin.json"), "'just a string'", "utf-8"); + const result = loadPluginManifest(dir, false); + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.error).toContain("plugin manifest must be an object"); + } + }); +}); diff --git a/src/plugins/manifest.ts b/src/plugins/manifest.ts index 1341924bc6c..51c59b0e9a4 100644 --- a/src/plugins/manifest.ts +++ b/src/plugins/manifest.ts @@ -1,5 +1,6 @@ import fs from "node:fs"; import path from "node:path"; +import JSON5 from "json5"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; import { matchBoundaryFileOpenFailure, openBoundaryFileSync } from "../infra/boundary-file-read.js"; import { isRecord } from "../utils.js"; @@ -273,7 +274,7 @@ export function loadPluginManifest( } let raw: unknown; try { - raw = JSON.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; + raw = JSON5.parse(fs.readFileSync(opened.fd, "utf-8")) as unknown; } catch (err) { return { ok: false,