fix(skills): preserve frontmatter fence variants

lib/skill-frontmatter.test.mjs

@@ -1,6 +1,9 @@
 import assert from "node:assert/strict";
+import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
 import { describe, it } from "node:test";
-import { parseFrontmatter } from "@earendil-works/pi-coding-agent";
+import { loadSkillsFromDir, parseFrontmatter } from "@earendil-works/pi-coding-agent";
 
 import { setDisableModelInvocation } from "./skill-frontmatter.ts";
 
@@ -66,6 +69,91 @@ describe("setDisableModelInvocation", () => {
     );
   });
 
+  it("preserves SDK-supported frontmatter fence variants", () => {
+    const cases = [
+      {
+        name: "UTF-8 BOM",
+        content: "\uFEFF---\nname: my-skill\ndescription: Does things\n---\nBody text.\n",
+        enabled:
+          "\uFEFF---\ndisable-model-invocation: true\nname: my-skill\ndescription: Does things\n---\nBody text.\n",
+      },
+      {
+        name: "opening fence trailing whitespace",
+        content: "---  \t\nname: my-skill\ndescription: Does things\n---\nBody text.\n",
+        enabled:
+          "---  \t\ndisable-model-invocation: true\nname: my-skill\ndescription: Does things\n---\nBody text.\n",
+      },
+      {
+        name: "CR-only line endings",
+        content: "---\rname: my-skill\rdescription: Does things\r---\rBody text.\r",
+        enabled:
+          "---\rdisable-model-invocation: true\rname: my-skill\rdescription: Does things\r---\rBody text.\r",
+      },
+    ];
+
+    for (const fixture of cases) {
+      const updated = setDisableModelInvocation(fixture.content, true);
+      assert.equal(updated, fixture.enabled, fixture.name);
+      assert.equal(parseFrontmatter(updated).frontmatter["disable-model-invocation"], true);
+      assert.equal(setDisableModelInvocation(updated, false), fixture.content, `${fixture.name} round trip`);
+    }
+  });
+
+  it("preserves closing fence suffixes accepted by the SDK when toggling", () => {
+    for (const newline of ["\n", "\r\n", "\r"]) {
+      for (const closing of ["--- # end frontmatter", "----"]) {
+        const opening = `---${newline}`;
+        const tail = [
+          "name: my-skill",
+          "description: Does things",
+          closing,
+          "Body text.",
+          "disable-model-invocation: false",
+          "",
+        ].join(newline);
+        const original = opening + tail;
+        const disabled = `${opening}disable-model-invocation: true${newline}${tail}`;
+        const explicitFalse = `${opening}disable-model-invocation: false${newline}${tail}`;
+
+        assert.deepEqual(parseFrontmatter(original).frontmatter, {
+          name: "my-skill",
+          description: "Does things",
+        });
+        assert.equal(setDisableModelInvocation(original, true), disabled);
+        assert.equal(setDisableModelInvocation(explicitFalse, true), disabled);
+        assert.equal(parseFrontmatter(disabled).frontmatter["disable-model-invocation"], true);
+        assert.equal(setDisableModelInvocation(disabled, false), original);
+      }
+    }
+  });
+
+  it("keeps a BOM-prefixed skill loadable after toggling", async (t) => {
+    const root = await mkdtemp(join(tmpdir(), "pi-web-skill-frontmatter-"));
+    t.after(() => rm(root, { recursive: true, force: true }));
+    const skillDir = join(root, "demo");
+    const skillPath = join(skillDir, "SKILL.md");
+    const original = "\uFEFF---\nname: demo\ndescription: Demo skill\n---\nBody stays here.\n";
+    await mkdir(skillDir);
+    await writeFile(skillPath, original);
+
+    const before = loadSkillsFromDir({ dir: root, source: "path" });
+    assert.equal(before.diagnostics.length, 0);
+    assert.deepEqual(before.skills.map((skill) => skill.name), ["demo"]);
+
+    await writeFile(skillPath, setDisableModelInvocation(await readFile(skillPath, "utf8"), true));
+    const enabled = loadSkillsFromDir({ dir: root, source: "path" });
+    assert.equal(enabled.diagnostics.length, 0);
+    assert.equal(enabled.skills[0]?.name, "demo");
+    assert.equal(enabled.skills[0]?.description, "Demo skill");
+    assert.equal(enabled.skills[0]?.disableModelInvocation, true);
+
+    await writeFile(skillPath, setDisableModelInvocation(await readFile(skillPath, "utf8"), false));
+    assert.equal(await readFile(skillPath, "utf8"), original);
+    const restored = loadSkillsFromDir({ dir: root, source: "path" });
+    assert.equal(restored.diagnostics.length, 0);
+    assert.equal(restored.skills[0]?.disableModelInvocation, false);
+  });
+
   it("keeps a single key when disabling an already-true skill", () => {
     const content = "---\nname: my-skill\ndisable-model-invocation: true\ndescription: Does things\n---\n\nBody text.\n";
     const updated = setDisableModelInvocation(content, true);

lib/skill-frontmatter.ts

@@ -2,6 +2,34 @@ import { parseFrontmatter } from "@earendil-works/pi-coding-agent";
 
 const KEY = "disable-model-invocation";
 const KEY_LINE = `[ \\t]*(?:${KEY}|"${KEY}"|'${KEY}')[ \\t]*:`;
+const NEWLINE = "\\r\\n|\\n|\\r";
+
+interface FrontmatterBlock {
+  openingEnd: number;
+  closingStart: number;
+  newline: string;
+}
+
+function findFrontmatterBlock(content: string): FrontmatterBlock | undefined {
+  const opening = new RegExp(`^\\uFEFF?---[ \\t]*(${NEWLINE})`).exec(content);
+  if (!opening) return undefined;
+
+  const rest = content.slice(opening[0].length);
+  // Pi SDK closes frontmatter at the first line starting with ---.
+  const closing = new RegExp(`(^|${NEWLINE})---`).exec(rest);
+  if (!closing) return undefined;
+
+  return {
+    openingEnd: opening[0].length,
+    closingStart: opening[0].length + closing.index + closing[1].length,
+    newline: opening[1],
+  };
+}
+
+function startsWithFrontmatterFence(content: string): boolean {
+  const start = content.startsWith("\uFEFF") ? 1 : 0;
+  return content.startsWith("---", start);
+}
 
 /**
  * Toggle the `disable-model-invocation` frontmatter key with a surgical line
@@ -19,27 +47,39 @@ export function setDisableModelInvocation(content: string, disable: boolean): st
 
   // Only edit inside the frontmatter block, so a body line that happens to
   // document the key is never touched.
-  const closing = content.startsWith("---") ? content.indexOf("\n---", 3) : -1;
-  const head = closing === -1 ? content : content.slice(0, closing);
-  const tail = closing === -1 ? "" : content.slice(closing);
+  const block = findFrontmatterBlock(content);
 
   if (disable) {
     if (hasKey) {
-      const keyLine = new RegExp(`^(${KEY_LINE})[^\\r\\n]*(\\r?)$`, "m");
+      if (!block) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`);
+      const head = content.slice(block.openingEnd, block.closingStart);
+      const keyLine = new RegExp(`(^|${NEWLINE})(${KEY_LINE})[^\\r\\n]*`);
       if (!keyLine.test(head)) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`);
-      return head.replace(keyLine, "$1 true$2") + tail;
+      const updated = head.replace(keyLine, "$1$2 true");
+      return content.slice(0, block.openingEnd) + updated + content.slice(block.closingStart);
     }
-    const withKey = head.replace(/^---(\r?\n)/, `---$1${KEY}: true$1`);
-    if (withKey === head) {
+    if (!block) {
+      if (startsWithFrontmatterFence(content)) {
+        throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`);
+      }
       // No frontmatter block at all — create one.
-      return `---\n${KEY}: true\n---\n${content}`;
+      const bom = content.startsWith("\uFEFF") ? "\uFEFF" : "";
+      const body = bom ? content.slice(1) : content;
+      return `${bom}---\n${KEY}: true\n---\n${body}`;
     }
-    return withKey + tail;
+    return (
+      content.slice(0, block.openingEnd) +
+      `${KEY}: true${block.newline}` +
+      content.slice(block.openingEnd)
+    );
   }
 
-  // Drop the line together with its preceding newline so no blank line is
-  // left behind; the key is never the first line of the frontmatter block.
-  const keyLine = new RegExp(`\\n${KEY_LINE}[^\\n]*`);
+  if (!block) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`);
+  const head = content.slice(block.openingEnd, block.closingStart);
+  // Keep the preceding newline, when present, and consume the key line's own
+  // newline so the surrounding frontmatter retains exactly one line break.
+  const keyLine = new RegExp(`(^|${NEWLINE})${KEY_LINE}[^\\r\\n]*(?:${NEWLINE}|$)`);
   if (!keyLine.test(head)) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`);
-  return head.replace(keyLine, "") + tail;
+  const updated = head.replace(keyLine, "$1");
+  return content.slice(0, block.openingEnd) + updated + content.slice(block.closingStart);
 }