From ece55b4682bdcec202b415f59d60387314b5d7c5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 14 Feb 2026 13:55:30 +0000 Subject: [PATCH] refactor(shared): dedupe frontmatter parsing --- src/agents/skills/frontmatter.ts | 116 ++++++++++-------------------- src/hooks/frontmatter.ts | 118 ++++++++++--------------------- src/shared/frontmatter.test.ts | 43 +++++++++++ src/shared/frontmatter.ts | 60 ++++++++++++++++ 4 files changed, 176 insertions(+), 161 deletions(-) create mode 100644 src/shared/frontmatter.test.ts create mode 100644 src/shared/frontmatter.ts diff --git a/src/agents/skills/frontmatter.ts b/src/agents/skills/frontmatter.ts index a2c2901696..857bed643e 100644 --- a/src/agents/skills/frontmatter.ts +++ b/src/agents/skills/frontmatter.ts @@ -1,5 +1,4 @@ import type { Skill } from "@mariozechner/pi-coding-agent"; -import JSON5 from "json5"; import type { OpenClawSkillMetadata, ParsedSkillFrontmatter, @@ -7,30 +6,18 @@ import type { SkillInstallSpec, SkillInvocationPolicy, } from "./types.js"; -import { LEGACY_MANIFEST_KEYS, MANIFEST_KEY } from "../../compat/legacy-names.js"; import { parseFrontmatterBlock } from "../../markdown/frontmatter.js"; -import { parseBooleanValue } from "../../utils/boolean.js"; +import { + getFrontmatterString, + normalizeStringList, + parseFrontmatterBool, + resolveOpenClawManifestBlock, +} from "../../shared/frontmatter.js"; export function parseFrontmatter(content: string): ParsedSkillFrontmatter { return parseFrontmatterBlock(content); } -function normalizeStringList(input: unknown): string[] { - if (!input) { - return []; - } - if (Array.isArray(input)) { - return input.map((value) => String(value).trim()).filter(Boolean); - } - if (typeof input === "string") { - return input - .split(",") - .map((value) => value.trim()) - .filter(Boolean); - } - return []; -} - function parseInstallSpec(input: unknown): SkillInstallSpec | undefined { if (!input || typeof input !== "object") { return undefined; @@ -89,79 +76,48 @@ function parseInstallSpec(input: unknown): SkillInstallSpec | undefined { return spec; } -function getFrontmatterValue(frontmatter: ParsedSkillFrontmatter, key: string): string | undefined { - const raw = frontmatter[key]; - return typeof raw === "string" ? raw : undefined; -} - -function parseFrontmatterBool(value: string | undefined, fallback: boolean): boolean { - const parsed = parseBooleanValue(value); - return parsed === undefined ? fallback : parsed; -} - export function resolveOpenClawMetadata( frontmatter: ParsedSkillFrontmatter, ): OpenClawSkillMetadata | undefined { - const raw = getFrontmatterValue(frontmatter, "metadata"); - if (!raw) { - return undefined; - } - try { - const parsed = JSON5.parse(raw); - if (!parsed || typeof parsed !== "object") { - return undefined; - } - const metadataRawCandidates = [MANIFEST_KEY, ...LEGACY_MANIFEST_KEYS]; - let metadataRaw: unknown; - for (const key of metadataRawCandidates) { - const candidate = parsed[key]; - if (candidate && typeof candidate === "object") { - metadataRaw = candidate; - break; - } - } - if (!metadataRaw || typeof metadataRaw !== "object") { - return undefined; - } - const metadataObj = metadataRaw as Record; - const requiresRaw = - typeof metadataObj.requires === "object" && metadataObj.requires !== null - ? (metadataObj.requires as Record) - : undefined; - const installRaw = Array.isArray(metadataObj.install) ? (metadataObj.install as unknown[]) : []; - const install = installRaw - .map((entry) => parseInstallSpec(entry)) - .filter((entry): entry is SkillInstallSpec => Boolean(entry)); - const osRaw = normalizeStringList(metadataObj.os); - return { - always: typeof metadataObj.always === "boolean" ? metadataObj.always : undefined, - emoji: typeof metadataObj.emoji === "string" ? metadataObj.emoji : undefined, - homepage: typeof metadataObj.homepage === "string" ? metadataObj.homepage : undefined, - skillKey: typeof metadataObj.skillKey === "string" ? metadataObj.skillKey : undefined, - primaryEnv: typeof metadataObj.primaryEnv === "string" ? metadataObj.primaryEnv : undefined, - os: osRaw.length > 0 ? osRaw : undefined, - requires: requiresRaw - ? { - bins: normalizeStringList(requiresRaw.bins), - anyBins: normalizeStringList(requiresRaw.anyBins), - env: normalizeStringList(requiresRaw.env), - config: normalizeStringList(requiresRaw.config), - } - : undefined, - install: install.length > 0 ? install : undefined, - }; - } catch { + const metadataObj = resolveOpenClawManifestBlock({ frontmatter }); + if (!metadataObj) { return undefined; } + const requiresRaw = + typeof metadataObj.requires === "object" && metadataObj.requires !== null + ? (metadataObj.requires as Record) + : undefined; + const installRaw = Array.isArray(metadataObj.install) ? (metadataObj.install as unknown[]) : []; + const install = installRaw + .map((entry) => parseInstallSpec(entry)) + .filter((entry): entry is SkillInstallSpec => Boolean(entry)); + const osRaw = normalizeStringList(metadataObj.os); + return { + always: typeof metadataObj.always === "boolean" ? metadataObj.always : undefined, + emoji: typeof metadataObj.emoji === "string" ? metadataObj.emoji : undefined, + homepage: typeof metadataObj.homepage === "string" ? metadataObj.homepage : undefined, + skillKey: typeof metadataObj.skillKey === "string" ? metadataObj.skillKey : undefined, + primaryEnv: typeof metadataObj.primaryEnv === "string" ? metadataObj.primaryEnv : undefined, + os: osRaw.length > 0 ? osRaw : undefined, + requires: requiresRaw + ? { + bins: normalizeStringList(requiresRaw.bins), + anyBins: normalizeStringList(requiresRaw.anyBins), + env: normalizeStringList(requiresRaw.env), + config: normalizeStringList(requiresRaw.config), + } + : undefined, + install: install.length > 0 ? install : undefined, + }; } export function resolveSkillInvocationPolicy( frontmatter: ParsedSkillFrontmatter, ): SkillInvocationPolicy { return { - userInvocable: parseFrontmatterBool(getFrontmatterValue(frontmatter, "user-invocable"), true), + userInvocable: parseFrontmatterBool(getFrontmatterString(frontmatter, "user-invocable"), true), disableModelInvocation: parseFrontmatterBool( - getFrontmatterValue(frontmatter, "disable-model-invocation"), + getFrontmatterString(frontmatter, "disable-model-invocation"), false, ), }; diff --git a/src/hooks/frontmatter.ts b/src/hooks/frontmatter.ts index a213d04870..801da9a1d5 100644 --- a/src/hooks/frontmatter.ts +++ b/src/hooks/frontmatter.ts @@ -1,4 +1,3 @@ -import JSON5 from "json5"; import type { OpenClawHookMetadata, HookEntry, @@ -6,30 +5,18 @@ import type { HookInvocationPolicy, ParsedHookFrontmatter, } from "./types.js"; -import { LEGACY_MANIFEST_KEYS, MANIFEST_KEY } from "../compat/legacy-names.js"; import { parseFrontmatterBlock } from "../markdown/frontmatter.js"; -import { parseBooleanValue } from "../utils/boolean.js"; +import { + getFrontmatterString, + normalizeStringList, + parseFrontmatterBool, + resolveOpenClawManifestBlock, +} from "../shared/frontmatter.js"; export function parseFrontmatter(content: string): ParsedHookFrontmatter { return parseFrontmatterBlock(content); } -function normalizeStringList(input: unknown): string[] { - if (!input) { - return []; - } - if (Array.isArray(input)) { - return input.map((value) => String(value).trim()).filter(Boolean); - } - if (typeof input === "string") { - return input - .split(",") - .map((value) => value.trim()) - .filter(Boolean); - } - return []; -} - function parseInstallSpec(input: unknown): HookInstallSpec | undefined { if (!input || typeof input !== "object") { return undefined; @@ -66,79 +53,48 @@ function parseInstallSpec(input: unknown): HookInstallSpec | undefined { return spec; } -function getFrontmatterValue(frontmatter: ParsedHookFrontmatter, key: string): string | undefined { - const raw = frontmatter[key]; - return typeof raw === "string" ? raw : undefined; -} - -function parseFrontmatterBool(value: string | undefined, fallback: boolean): boolean { - const parsed = parseBooleanValue(value); - return parsed === undefined ? fallback : parsed; -} - export function resolveOpenClawMetadata( frontmatter: ParsedHookFrontmatter, ): OpenClawHookMetadata | undefined { - const raw = getFrontmatterValue(frontmatter, "metadata"); - if (!raw) { - return undefined; - } - try { - const parsed = JSON5.parse(raw); - if (!parsed || typeof parsed !== "object") { - return undefined; - } - const metadataRawCandidates = [MANIFEST_KEY, ...LEGACY_MANIFEST_KEYS]; - let metadataRaw: unknown; - for (const key of metadataRawCandidates) { - const candidate = parsed[key]; - if (candidate && typeof candidate === "object") { - metadataRaw = candidate; - break; - } - } - if (!metadataRaw || typeof metadataRaw !== "object") { - return undefined; - } - const metadataObj = metadataRaw as Record; - const requiresRaw = - typeof metadataObj.requires === "object" && metadataObj.requires !== null - ? (metadataObj.requires as Record) - : undefined; - const installRaw = Array.isArray(metadataObj.install) ? (metadataObj.install as unknown[]) : []; - const install = installRaw - .map((entry) => parseInstallSpec(entry)) - .filter((entry): entry is HookInstallSpec => Boolean(entry)); - const osRaw = normalizeStringList(metadataObj.os); - const eventsRaw = normalizeStringList(metadataObj.events); - return { - always: typeof metadataObj.always === "boolean" ? metadataObj.always : undefined, - emoji: typeof metadataObj.emoji === "string" ? metadataObj.emoji : undefined, - homepage: typeof metadataObj.homepage === "string" ? metadataObj.homepage : undefined, - hookKey: typeof metadataObj.hookKey === "string" ? metadataObj.hookKey : undefined, - export: typeof metadataObj.export === "string" ? metadataObj.export : undefined, - os: osRaw.length > 0 ? osRaw : undefined, - events: eventsRaw.length > 0 ? eventsRaw : [], - requires: requiresRaw - ? { - bins: normalizeStringList(requiresRaw.bins), - anyBins: normalizeStringList(requiresRaw.anyBins), - env: normalizeStringList(requiresRaw.env), - config: normalizeStringList(requiresRaw.config), - } - : undefined, - install: install.length > 0 ? install : undefined, - }; - } catch { + const metadataObj = resolveOpenClawManifestBlock({ frontmatter }); + if (!metadataObj) { return undefined; } + const requiresRaw = + typeof metadataObj.requires === "object" && metadataObj.requires !== null + ? (metadataObj.requires as Record) + : undefined; + const installRaw = Array.isArray(metadataObj.install) ? (metadataObj.install as unknown[]) : []; + const install = installRaw + .map((entry) => parseInstallSpec(entry)) + .filter((entry): entry is HookInstallSpec => Boolean(entry)); + const osRaw = normalizeStringList(metadataObj.os); + const eventsRaw = normalizeStringList(metadataObj.events); + return { + always: typeof metadataObj.always === "boolean" ? metadataObj.always : undefined, + emoji: typeof metadataObj.emoji === "string" ? metadataObj.emoji : undefined, + homepage: typeof metadataObj.homepage === "string" ? metadataObj.homepage : undefined, + hookKey: typeof metadataObj.hookKey === "string" ? metadataObj.hookKey : undefined, + export: typeof metadataObj.export === "string" ? metadataObj.export : undefined, + os: osRaw.length > 0 ? osRaw : undefined, + events: eventsRaw.length > 0 ? eventsRaw : [], + requires: requiresRaw + ? { + bins: normalizeStringList(requiresRaw.bins), + anyBins: normalizeStringList(requiresRaw.anyBins), + env: normalizeStringList(requiresRaw.env), + config: normalizeStringList(requiresRaw.config), + } + : undefined, + install: install.length > 0 ? install : undefined, + }; } export function resolveHookInvocationPolicy( frontmatter: ParsedHookFrontmatter, ): HookInvocationPolicy { return { - enabled: parseFrontmatterBool(getFrontmatterValue(frontmatter, "enabled"), true), + enabled: parseFrontmatterBool(getFrontmatterString(frontmatter, "enabled"), true), }; } diff --git a/src/shared/frontmatter.test.ts b/src/shared/frontmatter.test.ts new file mode 100644 index 0000000000..78265a5857 --- /dev/null +++ b/src/shared/frontmatter.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test } from "vitest"; +import { + getFrontmatterString, + normalizeStringList, + parseFrontmatterBool, + resolveOpenClawManifestBlock, +} from "./frontmatter.js"; + +describe("shared/frontmatter", () => { + test("normalizeStringList handles strings and arrays", () => { + expect(normalizeStringList("a, b,,c")).toEqual(["a", "b", "c"]); + expect(normalizeStringList([" a ", "", "b"])).toEqual(["a", "b"]); + expect(normalizeStringList(null)).toEqual([]); + }); + + test("getFrontmatterString extracts strings only", () => { + expect(getFrontmatterString({ a: "b" }, "a")).toBe("b"); + expect(getFrontmatterString({ a: 1 }, "a")).toBeUndefined(); + }); + + test("parseFrontmatterBool respects fallback", () => { + expect(parseFrontmatterBool("true", false)).toBe(true); + expect(parseFrontmatterBool("false", true)).toBe(false); + expect(parseFrontmatterBool(undefined, true)).toBe(true); + }); + + test("resolveOpenClawManifestBlock parses JSON5 metadata and picks openclaw block", () => { + const frontmatter = { + metadata: "{ openclaw: { foo: 1, bar: 'baz' } }", + }; + expect(resolveOpenClawManifestBlock({ frontmatter })).toEqual({ foo: 1, bar: "baz" }); + }); + + test("resolveOpenClawManifestBlock returns undefined for invalid input", () => { + expect(resolveOpenClawManifestBlock({ frontmatter: {} })).toBeUndefined(); + expect( + resolveOpenClawManifestBlock({ frontmatter: { metadata: "not-json5" } }), + ).toBeUndefined(); + expect( + resolveOpenClawManifestBlock({ frontmatter: { metadata: "{ nope: { a: 1 } }" } }), + ).toBeUndefined(); + }); +}); diff --git a/src/shared/frontmatter.ts b/src/shared/frontmatter.ts new file mode 100644 index 0000000000..8611744d9b --- /dev/null +++ b/src/shared/frontmatter.ts @@ -0,0 +1,60 @@ +import JSON5 from "json5"; +import { LEGACY_MANIFEST_KEYS, MANIFEST_KEY } from "../compat/legacy-names.js"; +import { parseBooleanValue } from "../utils/boolean.js"; + +export function normalizeStringList(input: unknown): string[] { + if (!input) { + return []; + } + if (Array.isArray(input)) { + return input.map((value) => String(value).trim()).filter(Boolean); + } + if (typeof input === "string") { + return input + .split(",") + .map((value) => value.trim()) + .filter(Boolean); + } + return []; +} + +export function getFrontmatterString( + frontmatter: Record, + key: string, +): string | undefined { + const raw = frontmatter[key]; + return typeof raw === "string" ? raw : undefined; +} + +export function parseFrontmatterBool(value: string | undefined, fallback: boolean): boolean { + const parsed = parseBooleanValue(value); + return parsed === undefined ? fallback : parsed; +} + +export function resolveOpenClawManifestBlock(params: { + frontmatter: Record; + key?: string; +}): Record | undefined { + const raw = getFrontmatterString(params.frontmatter, params.key ?? "metadata"); + if (!raw) { + return undefined; + } + + try { + const parsed = JSON5.parse(raw); + if (!parsed || typeof parsed !== "object") { + return undefined; + } + + const manifestKeys = [MANIFEST_KEY, ...LEGACY_MANIFEST_KEYS]; + for (const key of manifestKeys) { + const candidate = (parsed as Record)[key]; + if (candidate && typeof candidate === "object") { + return candidate as Record; + } + } + return undefined; + } catch { + return undefined; + } +}