From d301ef5a628d20af1d96428aae1916d8d3093e87 Mon Sep 17 00:00:00 2001 From: haylan Date: Wed, 14 Oct 2026 12:34:56 +0200 Subject: [PATCH] fix: strip frontmatter from injected skill content and share parser from src --- scripts/generate.ts | 12 +++++-- {scripts => src}/frontmatter.ts | 0 src/index.ts | 10 ++++-- test/drift.test.ts | 30 +++++++++++------ test/generate.test.ts | 57 ++++++++++++++++++++++++++------- test/plugin.test.ts | 12 +++++-- test/release.test.ts | 30 ++++++++--------- 7 files changed, 108 insertions(+), 43 deletions(-) rename {scripts => src}/frontmatter.ts (100%) diff --git a/scripts/generate.ts b/scripts/generate.ts index 355eeb2..22ed83b 100644 --- a/scripts/generate.ts +++ b/scripts/generate.ts @@ -10,10 +10,18 @@ // // Deterministic: same inputs produce byte-identical outputs, so the nightly git diff is the // affected-set report. Never hand-edit skills/ or the matrix section. -import { cpSync, existsSync, readFileSync, readdirSync, rmSync, statSync, writeFileSync } from "node:fs" +import { + cpSync, + existsSync, + readFileSync, + readdirSync, + rmSync, + statSync, + writeFileSync, +} from "node:fs" import path from "node:path" import { fileURLToPath } from "node:url" -import { isUserInvoked, parseFrontmatter } from "./frontmatter.ts" +import { isUserInvoked, parseFrontmatter } from "../src/frontmatter.ts" const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") diff --git a/scripts/frontmatter.ts b/src/frontmatter.ts similarity index 100% rename from scripts/frontmatter.ts rename to src/frontmatter.ts diff --git a/src/index.ts b/src/index.ts index dc9ac6b..60a1bf3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,6 +2,7 @@ import { existsSync, readFileSync } from "node:fs" import path from "node:path" import { Plugin, Skill } from "@opencode/plugin" import { blockedPattern, guardrailError, isShellTool, resolveOptions } from "./guardrails.ts" +import { parseFrontmatter } from "./frontmatter.ts" import { loadManifest, packageRoot, packageVersion, skillFile } from "./manifest.ts" const AGENT_IDS = ["pocock", "pocock-worker", "pocock-reviewer"] as const @@ -22,6 +23,9 @@ export default Plugin.define({ await ctx.skill.transform((editor) => { for (const entry of entries) { const file = skillFile(entry) + // File-based skill loading strips frontmatter from content (core skill.ts uses + // markdown.content); match that so injected skills read identically. + const content = parseFrontmatter(readFileSync(file, "utf8")).body.trim() // Brand types (Skill.ID/Name/AbsolutePath) are nominal-only; runtime values are plain // strings, asserted by test/plugin.test.ts and the serve-based smoke run. editor.add({ @@ -30,7 +34,7 @@ export default Plugin.define({ description: entry.description, autoinvoke: entry.autoinvoke, path: file, - content: readFileSync(file, "utf8"), + content, } as unknown as Skill.Info) } }) @@ -56,7 +60,9 @@ export default Plugin.define({ const want = packageVersion() const initCmd = "npx @haylan/opencode-pocock init" if (!AGENT_IDS.some((id) => ids.has(id))) { - console.warn(`pocock: skills loaded, but the pocock agents are not installed — run: ${initCmd}`) + console.warn( + `pocock: skills loaded, but the pocock agents are not installed — run: ${initCmd}`, + ) } else if (have !== want) { console.warn( `pocock: installed agents are ${have ? `from v${have}` : "unversioned"}, package is v${want} — refresh: ${initCmd}`, diff --git a/test/drift.test.ts b/test/drift.test.ts index cd672a4..cf87d13 100644 --- a/test/drift.test.ts +++ b/test/drift.test.ts @@ -3,7 +3,7 @@ import { existsSync, readFileSync, readdirSync } from "node:fs" import path from "node:path" import { test } from "node:test" import { fileURLToPath } from "node:url" -import { parseFrontmatter } from "../scripts/frontmatter.ts" +import { parseFrontmatter } from "../src/frontmatter.ts" import { loadClassification } from "../scripts/generate.ts" const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") @@ -61,9 +61,9 @@ test("every skill an agent names still exists in the upstream snapshot", () => { test("worker may only load model-invoked vendored skills", () => { const worker = readFileSync(path.join(root, "agents/pocock-worker.md"), "utf8") - const allowed = [...worker.matchAll(/action:\s*skill,\s*resource:\s*"([^"]+)",\s*effect:\s*allow/g)].map( - (m) => m[1], - ) + const allowed = [ + ...worker.matchAll(/action:\s*skill,\s*resource:\s*"([^"]+)",\s*effect:\s*allow/g), + ].map((m) => m[1]) assert.ok(allowed.length >= 3, "worker skill allow-list parsed") const userInvoked = new Set(entries.filter((e) => e.user_invoked).map((e) => e.name)) const vendoredNames = new Set(vendored.map((e) => e.name)) @@ -80,16 +80,20 @@ test("reviewer is read-only: edit/subagent denied, only code-review allowed", () const reviewer = readFileSync(path.join(root, "agents/pocock-reviewer.md"), "utf8") assert.match(reviewer, /action:\s*edit,\s*resource:\s*"\*",\s*effect:\s*deny/) assert.match(reviewer, /action:\s*subagent,\s*resource:\s*"\*",\s*effect:\s*deny/) - const allowedSkills = [...reviewer.matchAll(/action:\s*skill,\s*resource:\s*"([^"]+)",\s*effect:\s*allow/g)].map( - (m) => m[1], - ) + const allowedSkills = [ + ...reviewer.matchAll(/action:\s*skill,\s*resource:\s*"([^"]+)",\s*effect:\s*allow/g), + ].map((m) => m[1]) assert.deepEqual(allowedSkills, ["code-review"]) }) test("worker and reviewer cannot spawn subagents (no circular delegation)", () => { for (const file of ["pocock-worker.md", "pocock-reviewer.md"]) { const body = readFileSync(path.join(root, "agents", file), "utf8") - assert.match(body, /action:\s*subagent,\s*resource:\s*"\*",\s*effect:\s*deny/, `${file}: subagent deny`) + assert.match( + body, + /action:\s*subagent,\s*resource:\s*"\*",\s*effect:\s*deny/, + `${file}: subagent deny`, + ) } }) @@ -106,9 +110,15 @@ test("classification is fully reviewed and matches the generated tree", () => { vendored.map((e) => e.name).sort(), ) for (const e of vendored) { - assert.ok(existsSync(path.join(root, "skills", e.name, "SKILL.md")), `skills/${e.name} generated`) + assert.ok( + existsSync(path.join(root, "skills", e.name, "SKILL.md")), + `skills/${e.name} generated`, + ) } for (const e of entries.filter((x) => x.artifacts.includes("excluded"))) { - assert.ok(!existsSync(path.join(root, "skills", e.name)), `excluded skill ${e.name} must not ship`) + assert.ok( + !existsSync(path.join(root, "skills", e.name)), + `excluded skill ${e.name} must not ship`, + ) } }) diff --git a/test/generate.test.ts b/test/generate.test.ts index a58a6f2..ae8d325 100644 --- a/test/generate.test.ts +++ b/test/generate.test.ts @@ -11,12 +11,14 @@ import { updateDocMatrix, type ClassificationEntry, } from "../scripts/generate.ts" -import { isUserInvoked, parseFrontmatter } from "../scripts/frontmatter.ts" +import { isUserInvoked, parseFrontmatter } from "../src/frontmatter.ts" const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..") test("parseFrontmatter: plain, quoted with escapes and colons, DMI, nested blocks skipped", () => { - const plain = parseFrontmatter("---\nname: tdd\ndescription: Test-driven development\n---\nBody\n") + const plain = parseFrontmatter( + "---\nname: tdd\ndescription: Test-driven development\n---\nBody\n", + ) assert.equal(plain.frontmatter.name, "tdd") assert.equal(plain.frontmatter.description, "Test-driven development") assert.equal(plain.body, "Body\n") @@ -38,7 +40,12 @@ test("parseFrontmatter: plain, quoted with escapes and colons, DMI, nested block assert.deepEqual(none.frontmatter, {}) }) -function fixtureSkill(upstream: string, bucket: string, name: string, opts: { dmi?: boolean } = {}) { +function fixtureSkill( + upstream: string, + bucket: string, + name: string, + opts: { dmi?: boolean } = {}, +) { const dir = path.join(upstream, bucket, name) mkdirSync(path.join(dir, "agents"), { recursive: true }) writeFileSync( @@ -76,14 +83,30 @@ test("buildSkills copies verbatim minus agents/, builds manifest, honors exclusi const out = path.join(dir, "skills") const manifest = buildSkills({ upstreamSkills: upstream, - entries: [entry("tdd", "engineering"), entry("to-spec", "engineering"), entry("beta", "in-progress", { artifacts: ["excluded"] })], + entries: [ + entry("tdd", "engineering"), + entry("to-spec", "engineering"), + entry("beta", "in-progress", { artifacts: ["excluded"] }), + ], skillsOut: out, }) assert.deepEqual( manifest, [ - { id: "tdd", name: "tdd", description: "The tdd skill", autoinvoke: true, path: "skills/tdd/SKILL.md" }, - { id: "to-spec", name: "to-spec", description: "The to-spec skill", autoinvoke: false, path: "skills/to-spec/SKILL.md" }, + { + id: "tdd", + name: "tdd", + description: "The tdd skill", + autoinvoke: true, + path: "skills/tdd/SKILL.md", + }, + { + id: "to-spec", + name: "to-spec", + description: "The to-spec skill", + autoinvoke: false, + path: "skills/to-spec/SKILL.md", + }, ].sort((a, b) => a.id.localeCompare(b.id)), ) assert.ok(existsSync(path.join(out, "tdd/SKILL.md"))) @@ -101,7 +124,8 @@ test("buildSkills throws on unclassified upstream skills and missing classified const upstream = path.join(dir, "upstream") fixtureSkill(upstream, "engineering", "mystery") assert.throws( - () => buildSkills({ upstreamSkills: upstream, entries: [], skillsOut: path.join(dir, "out") }), + () => + buildSkills({ upstreamSkills: upstream, entries: [], skillsOut: path.join(dir, "out") }), /unclassified upstream skill: engineering\/mystery/, ) assert.throws( @@ -124,7 +148,10 @@ test("updateDocMatrix replaces only the marked section", () => { const doc = path.join(dir, "doc.md") writeFileSync(doc, "prose\n\nold\n\nmore prose\n") updateDocMatrix(doc, "NEW TABLE") - assert.equal(readFileSync(doc, "utf8"), "prose\n\n\nNEW TABLE\n\n\nmore prose\n") + assert.equal( + readFileSync(doc, "utf8"), + "prose\n\n\nNEW TABLE\n\n\nmore prose\n", + ) assert.throws(() => updateDocMatrix(doc.replace("doc", "x"), "t"), /ENOENT/) writeFileSync(path.join(dir, "bad.md"), "no markers") assert.throws(() => updateDocMatrix(path.join(dir, "bad.md"), "t"), /missing matrix markers/) @@ -134,11 +161,16 @@ test("updateDocMatrix replaces only the marked section", () => { }) test("renderMatrix: one row per entry with escaped cells", () => { - const md = renderMatrix([entry("tdd", "engineering", { depends_on: ["codebase-design"], rationale: "a | b" })]) + const md = renderMatrix([ + entry("tdd", "engineering", { depends_on: ["codebase-design"], rationale: "a | b" }), + ]) const [header, sep, row] = md.split("\n") assert.match(header, /^\| Upstream skill \| Bucket \| DMI \|/) assert.match(sep, /^\| --- \|/) - assert.equal(row, "| tdd | eng | | skill | skill:verbatim | codebase-design | analyzed | a \\| b |") + assert.equal( + row, + "| tdd | eng | | skill | skill:verbatim | codebase-design | analyzed | a \\| b |", + ) }) // Regression against the real vendored snapshot: every classified-verbatim skill must parse, @@ -162,7 +194,10 @@ test("real snapshot: all vendored skills parse and DMI flags match classificatio ) const m = manifest.find((x: { id: string }) => x.id === e.name) assert.equal(m.autoinvoke, !e.user_invoked, `${e.name}: manifest autoinvoke`) - assert.ok(!existsSync(path.join(root, "skills", e.name, "agents")), `${e.name}: openai.yaml stripped`) + assert.ok( + !existsSync(path.join(root, "skills", e.name, "agents")), + `${e.name}: openai.yaml stripped`, + ) } assert.equal(manifest.filter((m: { autoinvoke: boolean }) => !m.autoinvoke).length, 16) }) diff --git a/test/plugin.test.ts b/test/plugin.test.ts index 9aa2318..d52228b 100644 --- a/test/plugin.test.ts +++ b/test/plugin.test.ts @@ -9,7 +9,10 @@ import { blockedPattern, resolveOptions } from "../src/guardrails.ts" const manifest = loadManifest() -function fakeCtx(options: Record, opts: { agents?: { id: string }[]; dir?: string } = {}) { +function fakeCtx( + options: Record, + opts: { agents?: { id: string }[]; dir?: string } = {}, +) { const added: Record[] = [] const hooks: { name: string; fn: (event: unknown) => void }[] = [] const warns: string[] = [] @@ -23,7 +26,10 @@ function fakeCtx(options: Record, opts: { agents?: { id: string const ctx = { options, location: { directory: opts.dir ?? tmpdir() }, - skill: { transform: async (fn: (e: { add: (s: unknown) => void }) => void) => fn({ add: (s) => added.push(s) }) }, + skill: { + transform: async (fn: (e: { add: (s: unknown) => void }) => void) => + fn({ add: (s) => added.push(s) }), + }, tool: { hook: async (name: string, fn: (event: unknown) => void) => hooks.push({ name, fn }) }, agent: { transform: async (fn: (e: typeof agentEditor) => void) => fn(agentEditor) }, } @@ -55,8 +61,8 @@ test("setup injects every manifest skill with absolute existing paths and autoin assert.ok(path.isAbsolute(skill.path as string), `${skill.id}: absolute path`) assert.ok(existsSync(skill.path as string), `${skill.id}: file exists`) assert.ok((skill.content as string).length > 0, `${skill.id}: content loaded`) + assert.ok(!(skill.content as string).startsWith("---"), `${skill.id}: frontmatter stripped`) const entry = manifest.find((m) => m.id === skill.id)! - assert.equal(skill.content, readFileSync(skill.path as string, "utf8")) assert.equal(skill.description, entry.description) } const toSpec = f.added.find((s) => s.id === "to-spec")! diff --git a/test/release.test.ts b/test/release.test.ts index a8d2274..553ca48 100644 --- a/test/release.test.ts +++ b/test/release.test.ts @@ -1,6 +1,12 @@ import assert from "node:assert/strict" import { test } from "node:test" -import { encodeName, ensureLinked, ensureTag, isPublished, publishBody } from "../scripts/release.ts" +import { + encodeName, + ensureLinked, + ensureTag, + isPublished, + publishBody, +} from "../scripts/release.ts" import { buildEntry, bumpMinor, insertEntry, latestVersion } from "../scripts/version.ts" const CHANGELOG = `# Changelog @@ -66,7 +72,10 @@ test("publish helpers: name encoding, published check, body hashes", () => { assert.equal(isPublished({ versions: {} }, "1.0.0"), false) assert.equal(isPublished({}, "1.0.0"), false) - const body = publishBody({ name: "@haylan/opencode-pocock", version: "1.0.0" }, Buffer.from("tar")) as { + const body = publishBody( + { name: "@haylan/opencode-pocock", version: "1.0.0" }, + Buffer.from("tar"), + ) as { "dist-tags": Record versions: Record _attachments: Record @@ -79,18 +88,6 @@ test("publish helpers: name encoding, published check, body hashes", () => { assert.equal(att.length, 3) }) -function fakeFetch(routes: Record, seen: string[]) { - return (async (url: string | URL | Request, init?: RequestInit) => { - const u = String(url) - seen.push(`${init?.method ?? "GET"} ${u}`) - const match = Object.entries(routes).find(([suffix]) => u.endsWith(suffix)) - const route = match?.[1] ?? { status: 404 } - return new Response(route.body !== undefined ? JSON.stringify(route.body) : null, { - status: route.status, - }) - }) as typeof fetch -} - test("ensureTag: creates on 404, no-ops when it exists, fails loudly otherwise", async () => { const base = "https://gitea.test/api/v1" const creating = { @@ -130,5 +127,8 @@ test("ensureLinked: links once, then reports already linked", async () => { }) as typeof fetch const api = { base: "https://gitea.test/api/v1", headers: {}, fetchFn } assert.equal(await ensureLinked(api, "haylan", "@haylan/opencode-pocock", "0.2.0", "r"), "linked") - assert.equal(await ensureLinked(api, "haylan", "@haylan/opencode-pocock", "0.2.0", "r"), "already linked") + assert.equal( + await ensureLinked(api, "haylan", "@haylan/opencode-pocock", "0.2.0", "r"), + "already linked", + ) })