diff --git a/app/api/skills/route.ts b/app/api/skills/route.ts index d71fd103d..291be3ba6 100644 --- a/app/api/skills/route.ts +++ b/app/api/skills/route.ts @@ -2,8 +2,9 @@ import { NextResponse } from "next/server"; import { existsSync, readFileSync, writeFileSync } from "fs"; import { homedir } from "os"; import path from "path"; -import { getAgentDir, parseFrontmatter } from "@earendil-works/pi-coding-agent"; +import { getAgentDir } from "@earendil-works/pi-coding-agent"; import { loadSkillsWithInstallInfo } from "@/lib/skills-service"; +import { setDisableModelInvocation } from "@/lib/skill-frontmatter"; import { getAllowedFileRoots, isExistingFilePathAllowed } from "@/lib/file-access"; export const dynamic = "force-dynamic"; @@ -47,24 +48,7 @@ export async function PATCH(req: Request) { } const content = readFileSync(filePath, "utf8"); - const key = "disable-model-invocation"; - - // Use parseFrontmatter to check current value, then do a surgical line edit - // to preserve the original YAML formatting of all other fields. - const { frontmatter } = parseFrontmatter>(content); - const alreadySet = Boolean(frontmatter[key]); - - let updated = content; - if (disableModelInvocation && !alreadySet) { - // Add key after the opening --- line - updated = content.replace(/^---\r?\n/, `---\n${key}: true\n`); - // If no frontmatter exists, create one - if (updated === content) updated = `---\n${key}: true\n---\n${content}`; - } else if (!disableModelInvocation && alreadySet) { - // Remove the key line entirely - updated = content.replace(new RegExp(`^${key}\\s*:.*\\r?\\n`, "m"), ""); - } - + const updated = setDisableModelInvocation(content, disableModelInvocation); writeFileSync(filePath, updated, "utf8"); return NextResponse.json({ success: true }); } catch (e) { diff --git a/lib/skill-frontmatter.test.mjs b/lib/skill-frontmatter.test.mjs new file mode 100644 index 000000000..ce5f27d99 --- /dev/null +++ b/lib/skill-frontmatter.test.mjs @@ -0,0 +1,104 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { parseFrontmatter } from "@earendil-works/pi-coding-agent"; + +import { setDisableModelInvocation } from "./skill-frontmatter.ts"; + +describe("setDisableModelInvocation", () => { + const withFrontmatter = "---\nname: my-skill\ndescription: Does things\n---\n\nBody text.\n"; + + it("adds the key after the opening fence when absent", () => { + const updated = setDisableModelInvocation(withFrontmatter, true); + assert.equal( + updated, + "---\ndisable-model-invocation: true\nname: my-skill\ndescription: Does things\n---\n\nBody text.\n", + ); + assert.equal(parseFrontmatter(updated).frontmatter["disable-model-invocation"], true); + }); + + it("creates a frontmatter block when the file has none", () => { + const updated = setDisableModelInvocation("Just a body.\n", true); + const { frontmatter, body } = parseFrontmatter(updated); + assert.equal(frontmatter["disable-model-invocation"], true); + assert.equal(body, "Just a body."); + }); + + it("replaces an explicit false value instead of adding a duplicate key", () => { + const content = "---\nname: my-skill\ndisable-model-invocation: false\ndescription: Does things\n---\n\nBody text.\n"; + const updated = setDisableModelInvocation(content, true); + // A duplicate key would make the whole file unparseable and the loader + // would drop the skill, so this must parse to a single true value. + const { frontmatter } = parseFrontmatter(updated); + assert.equal(frontmatter["disable-model-invocation"], true); + assert.equal( + updated.match(/^disable-model-invocation[^\n]*/gm)?.length, + 1, + "must not emit a duplicate key", + ); + }); + + it("updates and removes indented quoted keys", () => { + for (const quote of ['"', "'"]) { + const content = `---\n name: my-skill\n ${quote}disable-model-invocation${quote}: false\n---\nBody text.\n`; + const updated = setDisableModelInvocation(content, true); + assert.equal(parseFrontmatter(updated).frontmatter["disable-model-invocation"], true); + assert.equal( + parseFrontmatter(setDisableModelInvocation(updated, false)).frontmatter["disable-model-invocation"], + undefined, + ); + } + }); + + it("rejects unsupported key formatting instead of silently succeeding", () => { + const content = "---\n{ disable-model-invocation: false, name: my-skill }\n---\nBody text.\n"; + assert.throws(() => setDisableModelInvocation(content, true), /unsupported frontmatter formatting/); + }); + + it("preserves CRLF line endings when updating or adding the key", () => { + const content = "---\r\nname: my-skill\r\ndisable-model-invocation: false\r\n---\r\nBody text.\r\n"; + assert.equal( + setDisableModelInvocation(content, true), + "---\r\nname: my-skill\r\ndisable-model-invocation: true\r\n---\r\nBody text.\r\n", + ); + assert.equal( + setDisableModelInvocation(content.replace("disable-model-invocation: false\r\n", ""), true), + "---\r\ndisable-model-invocation: true\r\nname: my-skill\r\n---\r\nBody text.\r\n", + ); + }); + + 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); + assert.equal(parseFrontmatter(updated).frontmatter["disable-model-invocation"], true); + assert.equal(updated.match(/^disable-model-invocation[^\n]*/gm)?.length, 1); + }); + + it("removes the key when disabling is turned off", () => { + const content = "---\nname: my-skill\ndisable-model-invocation: true\ndescription: Does things\n---\n\nBody text.\n"; + const updated = setDisableModelInvocation(content, false); + assert.deepEqual(parseFrontmatter(updated).frontmatter, { + name: "my-skill", + description: "Does things", + }); + }); + + it("removes the key when it is the last frontmatter line before the closing fence", () => { + const content = "---\nname: my-skill\ndescription: Does things\ndisable-model-invocation: true\n---\n\nBody text.\n"; + const updated = setDisableModelInvocation(content, false); + assert.equal(updated, withFrontmatter); + }); + + it("is a no-op when disabling is off and the key is absent", () => { + assert.equal(setDisableModelInvocation(withFrontmatter, false), withFrontmatter); + }); + + it("preserves unrelated frontmatter formatting and body lines that mention the key", () => { + const content = "---\nname: my-skill\n# comment\nallowed-tools: [ read, write ]\ndisable-model-invocation: false\n---\nUse `disable-model-invocation: true` to hide me.\n"; + const updated = setDisableModelInvocation(content, true); + const { frontmatter, body } = parseFrontmatter(updated); + assert.equal(frontmatter["disable-model-invocation"], true); + assert.deepEqual(frontmatter["allowed-tools"], ["read", "write"]); + assert.match(updated, /# comment/); + assert.equal(body, "Use `disable-model-invocation: true` to hide me."); + }); +}); diff --git a/lib/skill-frontmatter.ts b/lib/skill-frontmatter.ts new file mode 100644 index 000000000..7eb025cb4 --- /dev/null +++ b/lib/skill-frontmatter.ts @@ -0,0 +1,45 @@ +import { parseFrontmatter } from "@earendil-works/pi-coding-agent"; + +const KEY = "disable-model-invocation"; +const KEY_LINE = `[ \\t]*(?:${KEY}|"${KEY}"|'${KEY}')[ \\t]*:`; + +/** + * Toggle the `disable-model-invocation` frontmatter key with a surgical line + * edit that preserves the original YAML formatting of every other field. + * + * The key is detected by presence rather than truthiness: an explicit + * `disable-model-invocation: false` must be updated in place. Prepending a + * second key (as a truthiness check would) creates a duplicate YAML key that + * makes the whole file unparseable, and the skill loader then drops the skill. + */ +export function setDisableModelInvocation(content: string, disable: boolean): string { + const { frontmatter } = parseFrontmatter>(content); + const hasKey = Object.prototype.hasOwnProperty.call(frontmatter, KEY); + if (!disable && !hasKey) return content; + + // 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); + + if (disable) { + if (hasKey) { + const keyLine = new RegExp(`^(${KEY_LINE})[^\\r\\n]*(\\r?)$`, "m"); + if (!keyLine.test(head)) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`); + return head.replace(keyLine, "$1 true$2") + tail; + } + const withKey = head.replace(/^---(\r?\n)/, `---$1${KEY}: true$1`); + if (withKey === head) { + // No frontmatter block at all — create one. + return `---\n${KEY}: true\n---\n${content}`; + } + return withKey + tail; + } + + // 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 (!keyLine.test(head)) throw new Error(`Cannot edit ${KEY}: unsupported frontmatter formatting`); + return head.replace(keyLine, "") + tail; +}