refactor(shared): dedupe frontmatter parsing

2026-02-19 18:39:20 -05:00 · 2026-02-14 13:55:30 +00:00
parent 1b03eb71aa
commit ece55b4682
4 changed files with 176 additions and 161 deletions
--- 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<string, unknown>;
-    const requiresRaw =
-      typeof metadataObj.requires === "object" && metadataObj.requires !== null
-        ? (metadataObj.requires as Record<string, unknown>)
-        : 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<string, unknown>)
+      : 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,
    ),
  };
--- 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<string, unknown>;
-    const requiresRaw =
-      typeof metadataObj.requires === "object" && metadataObj.requires !== null
-        ? (metadataObj.requires as Record<string, unknown>)
-        : 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<string, unknown>)
+      : 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),
  };
 }

--- a/src/shared/frontmatter.test.ts
+++ 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();
+  });
+});
--- a/src/shared/frontmatter.ts
+++ 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<string, unknown>,
+  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<string, unknown>;
+  key?: string;
+}): Record<string, unknown> | 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<string, unknown>)[key];
+      if (candidate && typeof candidate === "object") {
+        return candidate as Record<string, unknown>;
+      }
+    }
+    return undefined;
+  } catch {
+    return undefined;
+  }
+}