diff --git a/lib/skill-frontmatter.test.mjs b/lib/skill-frontmatter.test.mjs index ce5f27d99..12232cdfe 100644 --- a/lib/skill-frontmatter.test.mjs +++ b/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); diff --git a/lib/skill-frontmatter.ts b/lib/skill-frontmatter.ts index 7eb025cb4..e5a7bcb7b 100644 --- a/lib/skill-frontmatter.ts +++ b/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); }