fix: strip frontmatter from injected skill content and share parser from src
This commit is contained in:
1 parent
023e012eda
commit
d301ef5a62
7 files changed
+108
-43
No files matched your search
+10
-2
@@ -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)), "..")
|
||||
|
||||
|
||||
File renamed without changes.
+8
-2
@@ -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}`,
|
||||
|
||||
+20
-10
@@ -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`,
|
||||
)
|
||||
}
|
||||
})
|
||||
+46
-11
@@ -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<!-- matrix:start -->\nold\n<!-- matrix:end -->\nmore prose\n")
|
||||
updateDocMatrix(doc, "NEW TABLE")
|
||||
assert.equal(readFileSync(doc, "utf8"), "prose\n<!-- matrix:start -->\n\nNEW TABLE\n\n<!-- matrix:end -->\nmore prose\n")
|
||||
assert.equal(
|
||||
readFileSync(doc, "utf8"),
|
||||
"prose\n<!-- matrix:start -->\n\nNEW TABLE\n\n<!-- matrix:end -->\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)
|
||||
})
|
||||
+9
-3
@@ -9,7 +9,10 @@ import { blockedPattern, resolveOptions } from "../src/guardrails.ts"
|
||||
|
||||
const manifest = loadManifest()
|
||||
|
||||
function fakeCtx(options: Record<string, unknown>, opts: { agents?: { id: string }[]; dir?: string } = {}) {
|
||||
function fakeCtx(
|
||||
options: Record<string, unknown>,
|
||||
opts: { agents?: { id: string }[]; dir?: string } = {},
|
||||
) {
|
||||
const added: Record<string, unknown>[] = []
|
||||
const hooks: { name: string; fn: (event: unknown) => void }[] = []
|
||||
const warns: string[] = []
|
||||
@@ -23,7 +26,10 @@ function fakeCtx(options: Record<string, unknown>, 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")!
|
||||
|
||||
+15
-15
@@ -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<string, string>
|
||||
versions: Record<string, { dist: { shasum: string; integrity: string } }>
|
||||
_attachments: Record<string, { data: string; length: number }>
|
||||
@@ -79,18 +88,6 @@ test("publish helpers: name encoding, published check, body hashes", () => {
|
||||
assert.equal(att.length, 3)
|
||||
})
|
||||
|
||||
function fakeFetch(routes: Record<string, { status: number; body?: unknown }>, 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",
|
||||
)
|
||||
})
|
||||
Reference in new issue
Block a user