From 9785428930b1050e9a2394b6fff2b68d03f97a5d Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 14:18:57 -0700 Subject: [PATCH 1/4] fix(partials): keep substituted files faithful and never emit a live tag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Partial substitution decoded a file as UTF-8 and re-encoded it, so a file with a partial tag and any invalid byte (a Latin-1 "é", or a UTF-16 byte-order mark) came out with U+FFFD in place of those bytes, on a green build. The same file without a tag kept its bytes. A file that uses partials, and every partial, now has to survive a UTF-8 round trip, or the build fails and names the file. Extension matching was case-sensitive, so NOTES.MD or REF.TXT was treated as a file type substitution skips, and the build failed with an error saying substitution only runs on .md and .txt files. The extension is now compared in lower case, as the README already implied. Rendering could assemble a tag that no single piece contained: a partial ending in "{" before "{> name}}", a partial ending in "{{" before "> name}}", or "{{{> name}}}". The build guard skipped substituted file types on the reasoning that only the documented escape could leave a tag there, so these shipped as live tags. Escaped tags now render through their own placeholder, and substitute() fails on any tag left in its output. That makes the reasoning true, so validate no longer warns on every documented \{{> name}} escape, which was permanent noise on the one sanctioned way to write partial syntax. A partial's byte-order mark was inlined as U+FEFF mid-file, and a CRLF partial left mixed line endings in an LF file. Partials now drop a leading byte-order mark and take the line ending of the file that includes them. The README documents these rules and the backslash edge case. --- README.md | 14 +- src/partials.ts | 129 ++++++++++++---- src/targets/validation-shared.ts | 34 ++--- tests/core.test.ts | 11 +- tests/partials.test.ts | 242 +++++++++++++++++++++++++++++++ 5 files changed, 371 insertions(+), 59 deletions(-) create mode 100644 tests/partials.test.ts diff --git a/README.md b/README.md index 626eded..75466ca 100644 --- a/README.md +++ b/README.md @@ -523,9 +523,15 @@ export default defineConfig({ Partials are project-level (shared across every source plugin, not scoped to one), and may reference other partials — nested composition resolves in one pass, though a circular reference (A includes B includes A) is a build-time error. A tag alone on its own line — the common case — leaves no blank line behind. -Substitution runs on every `.md`/`.mdc`/`.markdown`/`.txt` file pluginpack emits — skills, agents, commands, rules, declared shipping files, and a target's `repositoryFiles` — via the real [`mustache`](https://github.com/janl/mustache.js) library. +Substitution runs on every `.md`/`.mdc`/`.markdown`/`.txt` file pluginpack emits, whatever the case of the extension (`NOTES.MD` counts) — skills, agents, commands, rules, declared shipping files, and a target's `repositoryFiles` — via the real [`mustache`](https://github.com/janl/mustache.js) library. -**A tag that cannot be resolved fails the build.** A `{{> name}}` reference naming a partial that does not exist is an error listing the available partials and the nearest match, rather than rendering as nothing — silently dropping a section out of a shipped skill file is worse than a red build. The same applies to a malformed reference (`{{> }}`), and to a tag inside a partial's own body. +**A tag that cannot be resolved fails the build.** A `{{> name}}` reference naming a partial that does not exist is an error listing the available partials and the nearest match, rather than rendering as nothing — silently dropping a section out of a shipped skill file is worse than a red build. The same applies to a malformed reference (`{{> }}`), to a tag inside a partial's own body, and to a tag that substitution itself would assemble from neighboring text, such as a partial that ends in `{` placed before `{> name}}`. + +Substitution keeps the bytes of the file intact: + +- A file that contains a partial tag, and every partial, must be valid UTF-8. Otherwise the build fails, instead of replacing the invalid bytes with U+FFFD. A file with no partial tag is copied byte for byte, whatever its encoding. +- A partial's leading byte-order mark is dropped, so it does not appear in the middle of the including file. +- A partial takes the line endings of the file that includes it. pluginpack reads them from that file's first line break, so a CRLF partial in an LF file emits LF, and an LF partial in a CRLF file emits CRLF. Only partial tags are substituted. Any other `{{...}}`-shaped text — documentation about Handlebars, Jinja, Go templates, or Mustache itself, or a curly-brace code sample — is emitted exactly as authored, including text Mustache could not parse as a template at all. To write a literal partial tag, escape it with a backslash: @@ -533,7 +539,9 @@ Only partial tags are substituted. Any other `{{...}}`-shaped text — documenta Reference a partial by writing \{{> auth}} in a skill file. ``` -That emits `{{> auth}}` verbatim. Because substitution does not run on other file types, a tag authored in (say) a `.yaml` reference file or a `.py` script would otherwise ship through untouched; `build` fails on any such tag left in emitted output, and `validate` reports it in an already-generated target repo. +That emits `{{> auth}}` verbatim. A backslash directly before a tag always escapes it, so `\\{{> auth}}` emits `\{{> auth}}`. To emit a literal backslash followed by the partial's content, put the backslash in a partial of its own (for example `partials/backslash.md`, holding only `\` with no trailing newline) and write `{{> backslash}}{{> auth}}`. + +Because substitution does not run on other file types, a tag authored in (say) a `.yaml` reference file or a `.py` script would otherwise ship through untouched; `build` fails on any such tag left in emitted output, and `validate` reports it in an already-generated target repo. `validate` does not report tags in `.md`/`.mdc`/`.markdown`/`.txt` files: there, the only tag a build leaves is an escaped one. ## Other Shapes diff --git a/src/partials.ts b/src/partials.ts index 8eb2f6e..42d7159 100644 --- a/src/partials.ts +++ b/src/partials.ts @@ -28,10 +28,21 @@ const ESCAPE = "\\"; /** * Stands in for a `{{` that is not a partial tag while Mustache renders, so * Mustache never treats it as a tag of its own. A private-use code point: - * never legitimately present in an authored skill file, and - * `assertNoSentinel` rejects the file outright if it somehow is. + * never legitimately present in an authored skill file, and `prepare` + * rejects the file outright if it somehow is. */ -const SENTINEL = ""; +const SENTINEL = "\uE000"; + +/** + * Stands in for the `{{` of an escaped `\{{> name}}` tag. It is kept apart + * from `SENTINEL` so that, after rendering, every `{{>` tag still in the text + * is known not to come from an escape (see `substitute`). + */ +const ESCAPED_SENTINEL = "\uE001"; + +const RESERVED_CODE_POINTS = [SENTINEL, ESCAPED_SENTINEL]; + +const BYTE_ORDER_MARK = "\uFEFF"; /** * Loads every file under `partialsDir` into a name -> content map, keyed by @@ -53,12 +64,37 @@ export async function loadPartials( if (partials.has(key)) { throw new Error(`Duplicate partial name "${key}" (from "${relative}").`); } - partials.set(key, await fs.readFile(file, "utf8")); + const bytes = await fs.readFile(file); + const text = decodeUtf8(bytes); + if (text === undefined) { + throw new Error( + `Partial "${key}" (${relative}) is not valid UTF-8, so inlining it would replace its invalid bytes with U+FFFD. Re-save it as UTF-8.`, + ); + } + partials.set(key, normalizePartial(text)); } assertNoPartialCycles(partials); return partials; } +/** + * A partial is inlined mid-file, so a leading byte-order mark would become a + * stray U+FEFF in the middle of the including file; it is dropped. Line + * endings are stored as LF and converted to the including file's line ending + * at substitution time (see `substitute`), so a CRLF partial does not leave + * mixed line endings in an LF file, or the reverse. + */ +function normalizePartial(text: string): string { + const body = text.startsWith(BYTE_ORDER_MARK) ? text.slice(1) : text; + return body.replaceAll("\r\n", "\n"); +} + +/** Decodes UTF-8, or returns `undefined` if the bytes do not survive a decode/encode round trip. */ +function decodeUtf8(bytes: Buffer): string | undefined { + const text = bytes.toString("utf8"); + return Buffer.from(text, "utf8").equals(bytes) ? text : undefined; +} + /** * Mustache.js has no protection against a circular partial reference (A * includes B includes A) — its own docs just say "avoid infinite loops." @@ -120,8 +156,10 @@ class UnknownPartialError extends Error { * * Non-textual paths, and files that never mention a partial tag at all, are * returned completely untouched (the original `FileValue`, byte-identical) — - * so a file that does not use partials cannot be changed by this at all, and - * pays no lossy UTF-8 round-trip. + * so a file that does not use partials cannot be changed by this at all. + * A file that does use partials must be valid UTF-8: substituting means + * decoding and re-encoding it, which would silently turn every invalid byte + * into U+FFFD, so such a file fails the build instead. */ export function resolvePartials( relativePath: string, @@ -135,6 +173,11 @@ export function resolvePartials( if (!text.includes(PARTIAL_OPENER)) { return value; } + if (typeof value !== "string" && decodeUtf8(value) === undefined) { + throw new Error( + `Partial substitution failed in "${relativePath}": the file is not valid UTF-8, so substituting its partial tags would replace the invalid bytes with U+FFFD. Re-save it as UTF-8, or remove the partial tags from it.`, + ); + } try { return substitute(text, partials); } catch (error) { @@ -145,9 +188,9 @@ export function resolvePartials( } } -/** Whether partial substitution runs on files at this path (by extension). */ +/** Whether partial substitution runs on files at this path (by extension, ignoring case). */ export function substitutionRunsOn(relativePath: string): boolean { - return TEXTUAL_EXTENSIONS.includes(path.extname(relativePath)); + return TEXTUAL_EXTENSIONS.includes(path.extname(relativePath).toLowerCase()); } /** The extensions substitution runs on, for error messages (`.md, .mdc, ...`). */ @@ -168,25 +211,48 @@ export function substitutedExtensions(): string { * * An empty view (`{}`) is always passed, so no config or environment data is * ever exposed to interpolation. + * + * Rendering can still assemble a tag out of pieces: a partial ending in `{` + * followed by `{> name}}`, a partial ending in `{{` followed by `> name}}`, + * or an extra `{` in front of a tag (`{{{> name}}}`). None of those pieces is + * a tag on its own, so a tag built from them would ship unsubstituted. After + * rendering, every non-partial `{{` is restored but escaped tags stay behind + * `ESCAPED_SENTINEL`, so any `{{> name}}` left in the text is such an + * assembled tag, and it fails the build. + * + * Partials are stored with LF line endings (`normalizePartial`) and written + * with the including file's line ending, taken from its first line break. */ function substitute(text: string, partials: Map): string { - const rendered = mustache.render(prepare(text), {}, (name: string) => { - const partial = partials.get(name); - if (partial === undefined) { - throw new UnknownPartialError(name); - } - // A partial's own body goes through Mustache too, so it needs the same - // treatment as the file including it — otherwise a `{{ var }}` written - // inside a partial would still be rendered away. - return prepare(partial); - }); - return rendered.replaceAll(SENTINEL, "{{"); + const lineEnding = /^[^\n]*\r\n/.test(text) ? "\r\n" : "\n"; + const rendered = mustache + .render(prepare(text), {}, (name: string) => { + const partial = partials.get(name); + if (partial === undefined) { + throw new UnknownPartialError(name); + } + // A partial's own body goes through Mustache too, so it needs the same + // treatment as the file including it — otherwise a `{{ var }}` written + // inside a partial would still be rendered away. + return prepare( + lineEnding === "\n" ? partial : partial.replaceAll("\n", lineEnding), + ); + }) + .replaceAll(SENTINEL, "{{"); + const assembled = PARTIAL_TAG.exec(rendered)?.[0]; + if (assembled) { + throw new Error( + `substituting partials produced the tag "${assembled}", which would ship unsubstituted. It was built from neighboring text, such as a partial ending in "{" or "{{" followed by the rest of a tag, or an extra "{" before a tag. Separate the pieces, or write \\${assembled} to emit the tag literally.`, + ); + } + return rendered.replaceAll(ESCAPED_SENTINEL, "{{"); } function prepare(text: string): string { - if (text.includes(SENTINEL)) { + const reserved = RESERVED_CODE_POINTS.find((point) => text.includes(point)); + if (reserved) { throw new Error( - "content contains the reserved code point U+E000, which pluginpack uses internally while substituting partials", + `content contains the reserved code point U+${reserved.codePointAt(0)?.toString(16).toUpperCase()}, which pluginpack uses internally while substituting partials`, ); } return neutralize(text); @@ -221,7 +287,7 @@ function neutralize(text: string): string { ); } out += text.slice(index, isPartialTag ? open - ESCAPE.length : open); - out += SENTINEL; + out += isPartialTag ? ESCAPED_SENTINEL : SENTINEL; index = open + 2; } return out; @@ -286,14 +352,17 @@ export type UnsubstitutedPartialTag = { }; /** - * Finds `{{> name}}` tags in emitted files that substitution never ran on. - * Substitution resolves — or rejects — every tag in the file types it covers - * (`TEXTUAL_EXTENSIONS`), so the tags left to find are ones authored in (say) - * a `.yaml` reference file or a `.py` script, which ship through verbatim and - * silently break whatever reads them. Files substitution *did* run on are - * excluded deliberately: a tag there can only be the documented `\{{>` - * escape, which is meant to reach output. Binary files are skipped — a NUL - * byte is the standard "not text" signal. + * Finds `{{> name}}` tags in emitted files that substitution never ran on: + * ones authored in (say) a `.yaml` reference file or a `.py` script, which + * ship through verbatim and silently break whatever reads them. + * + * Files substitution ran on are skipped. That is safe only because + * `substitute` checks its own output: it fails on any tag left after + * rendering that did not come from a `\{{>` escape, including one assembled + * from neighboring text. A file with a `.md`-style extension that reaches + * the artifact without passing through `resolvePartials` is not covered by + * either check. Binary files are skipped — a NUL byte is the standard "not + * text" signal. */ export function findUnsubstitutedPartialTags( files: Iterable, diff --git a/src/targets/validation-shared.ts b/src/targets/validation-shared.ts index 1a0eefb..6e3138d 100644 --- a/src/targets/validation-shared.ts +++ b/src/targets/validation-shared.ts @@ -70,16 +70,18 @@ export function warning(issues: ValidationIssue[], message: string): void { } /** - * Flags `{{> name}}` partial references left in generated output, where a tag - * should never survive — it means whatever reads the file gets a template - * marker instead of the text meant to be inlined. + * Flags `{{> name}}` partial references left in generated output in a file + * type substitution never runs on (a `.yaml` reference file, a `.py` + * script). Whatever reads such a file gets a template marker instead of the + * text meant to be inlined, so it's an error. * - * Where substitution never runs (a `.yaml` reference file, a `.py` script) - * this is unambiguously a mistake, so it's an error. In a file substitution - * *does* run on, the only way a tag reaches output is the documented `\{{>` - * escape — which is legitimate — so a build by a current pluginpack cannot - * produce one accidentally, and this is a warning covering output generated - * before unresolved tags were caught at build time. + * Files substitution does run on are not scanned. At build time + * `resolvePartials` resolves every tag in them, or fails the build on an + * unknown, malformed, or assembled one, so the only tag a current build + * leaves there is the documented `\{{>` escape, which is meant to reach + * output. Validate cannot tell an escape from a stale or hand-edited tag + * after the fact, and flagging every escape would make the documented + * syntax permanent noise. */ export async function validateNoSurvivingPartialTags( root: string, @@ -104,20 +106,14 @@ export async function validateNoSurvivingPartialTags( ).flat(); for (const file of [...new Set(files)]) { const relative = toPosix(path.relative(root, file)); - const tag = findPartialTag(await fs.readFile(file)); - if (!tag) { + if (substitutionRunsOn(relative)) { continue; } - const message = `${relative} contains an unsubstituted partial reference ${tag}.`; - if (substitutionRunsOn(relative)) { - warning( - issues, - `${message} If it is not an intentional \`\\{{>\` escape, rebuild with a current pluginpack.`, - ); - } else { + const tag = findPartialTag(await fs.readFile(file)); + if (tag) { error( issues, - `${message} Partial substitution does not run on this file type (${substitutedExtensions()}).`, + `${relative} contains an unsubstituted partial reference ${tag}. Partial substitution does not run on this file type (${substitutedExtensions()}).`, ); } } diff --git a/tests/core.test.ts b/tests/core.test.ts index c0a4ad9..81b9097 100644 --- a/tests/core.test.ts +++ b/tests/core.test.ts @@ -3633,14 +3633,11 @@ export default defineConfig({ }, ]), ); - // A substituted file type can only reach output with a tag via the - // documented escape, so it warns rather than failing. + // Build resolves or rejects every tag in a substituted file type, so the + // only tag left there is the documented escape, which validate ignores. expect( - result.issues.filter( - (issue) => - issue.level === "warning" && issue.message.includes("NOTES.md"), - ), - ).toHaveLength(1); + result.issues.filter((issue) => issue.message.includes("NOTES.md")), + ).toEqual([]); }); it("rejects a target that references an unknown source plugin in from:", async () => { diff --git a/tests/partials.test.ts b/tests/partials.test.ts new file mode 100644 index 0000000..5980095 --- /dev/null +++ b/tests/partials.test.ts @@ -0,0 +1,242 @@ +import { readFile, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { Project, type ProjectArgs } from "fixturify-project"; +import { afterEach, describe, expect, it } from "vitest"; +import { validateOutput } from "../src/adapters.js"; +import { build } from "../src/build.js"; +import { loadPartials, resolvePartials } from "../src/partials.js"; + +type DirJSON = NonNullable; + +let project: Project | undefined; + +afterEach(async () => { + await project?.dispose(); + project = undefined; +}); + +const partials = new Map([ + ["auth", "Authenticate first."], + ["lbrace", "{"], + ["open", "x{{"], +]); + +function skill(name: string): string { + return `---\nname: ${name}\ndescription: The ${name} skill.\n---\n\n# ${name}\n`; +} + +/** A project with one shared source `acme`, partials, and the given targets block. */ +async function setup(targets: string, files: DirJSON): Promise { + project = new Project("partials-fixture", "1.0.0", { + files: { + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "acme-plugins", + version: "1.0.0", + source: { partials: "partials" }, + metadata: { description: "Acme", author: { name: "Acme" }, license: "MIT" }, + targets: { +${targets} + } +}); +`, + partials: { "auth.md": "Authenticate first." }, + ...files, + }, + }); + await project.write(); + return project.baseDir; +} + +describe("partial substitution keeps file bytes intact", () => { + it("refuses to substitute in a file that is not valid UTF-8 instead of emitting U+FFFD", async () => { + const root = await setup( + ` cursor: { outDir: "dist", plugins: { acme: { source: "shared/acme" } } }`, + { shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } } }, + ); + const skillPath = path.join(root, "shared/acme/skills/a/SKILL.md"); + await writeFile( + skillPath, + Buffer.concat([ + Buffer.from(`${skill("a")}caf`), + Buffer.from([0xe9]), + Buffer.from(" a\n{{> auth}}\n"), + ]), + ); + + await expect(build({ cwd: root })).rejects.toThrow( + /Partial substitution failed in "skills\/a\/SKILL\.md": the file is not valid UTF-8/, + ); + await expect( + readFile(path.join(root, "dist/acme/skills/a/SKILL.md")), + ).rejects.toMatchObject({ code: "ENOENT" }); + }); + + it("refuses a file with a byte-order mark and NUL bytes that is not valid UTF-8", () => { + const value = Buffer.concat([ + Buffer.from([0xff, 0xfe, 0x00, 0x41]), + Buffer.from("\n{{> auth}}\n"), + ]); + expect(() => resolvePartials("rules/policy.txt", value, partials)).toThrow( + /not valid UTF-8/, + ); + }); + + it("still substitutes in valid UTF-8 that contains NUL bytes, keeping them", () => { + const value = Buffer.from("a\u0000b {{> auth}}"); + expect(resolvePartials("rules/policy.txt", value, partials)).toBe( + "a\u0000b Authenticate first.", + ); + }); + + it("refuses a partial file that is not valid UTF-8", async () => { + project = new Project("partials-latin1", "1.0.0", { + files: { partials: { "auth.md": "placeholder" } }, + }); + await project.write(); + const partialsDir = path.join(project.baseDir, "partials"); + await writeFile( + path.join(partialsDir, "auth.md"), + Buffer.from([0x63, 0x61, 0x66, 0xe9]), + ); + + await expect(loadPartials(partialsDir)).rejects.toThrow( + /Partial "auth" \(auth\.md\) is not valid UTF-8/, + ); + }); + + it("strips a partial's byte-order mark instead of inlining U+FEFF mid-file", async () => { + project = new Project("partials-bom", "1.0.0", { + files: { partials: { "auth.md": "\uFEFFAuthenticate first." } }, + }); + await project.write(); + const loaded = await loadPartials(path.join(project.baseDir, "partials")); + + expect(resolvePartials("a.md", "Before {{> auth}}", loaded)).toBe( + "Before Authenticate first.", + ); + }); + + it("writes a partial with the including file's line endings", async () => { + project = new Project("partials-eol", "1.0.0", { + files: { + partials: { + "crlf.md": "one\r\ntwo\r\n", + "lf.md": "three\nfour\n", + "outer.md": "outer\n{{> lf}}\n", + }, + }, + }); + await project.write(); + const loaded = await loadPartials(path.join(project.baseDir, "partials")); + + expect(resolvePartials("a.md", "start\n{{> crlf}}\nend\n", loaded)).toBe( + "start\none\ntwo\nend\n", + ); + expect( + resolvePartials("a.md", "start\r\n{{> outer}}\r\nend\r\n", loaded), + ).toBe("start\r\nouter\r\nthree\r\nfour\r\nend\r\n"); + }); +}); + +describe("partial substitution file types", () => { + it.each(["NOTES.MD", "Notes.Md", "REF.TXT", "guide.Markdown", "rule.MDC"])( + "substitutes in %s regardless of extension case", + (file) => { + expect(resolvePartials(`skills/a/${file}`, "{{> auth}}", partials)).toBe( + "Authenticate first.", + ); + }, + ); + + it("builds a NOTES.MD containing a partial tag", async () => { + const root = await setup( + ` cursor: { outDir: "dist", plugins: { acme: { source: "shared/acme" } } }`, + { + shared: { + acme: { + skills: { + a: { "SKILL.md": skill("a"), "NOTES.MD": "{{> auth}}\n" }, + }, + }, + }, + }, + ); + + await build({ cwd: root }); + + await expect( + readFile(path.join(root, "dist/acme/skills/a/NOTES.MD"), "utf8"), + ).resolves.toBe("Authenticate first."); + }); +}); + +describe("partial substitution never emits a live tag", () => { + it("fails when a partial ending in { joins the text after it into a tag", () => { + expect(() => + resolvePartials("a.md", "{{> lbrace}}{> auth}}", partials), + ).toThrow( + /Partial substitution failed in "a\.md": substituting partials produced the tag "\{\{> auth\}\}"/, + ); + }); + + it("fails when a partial ending in {{ joins the text after it into a tag", () => { + expect(() => + resolvePartials("a.md", "{{> open}}> auth}}", partials), + ).toThrow(/produced the tag "\{\{> auth\}\}"/); + }); + + it("fails when an extra { before a tag leaves a tag in the output", () => { + expect(() => resolvePartials("a.md", "{{{> auth}}}", partials)).toThrow( + /produced the tag "\{\{> auth\}\}"/, + ); + }); + + it("treats a doubled backslash as an escape, with a backslash partial as the workaround", () => { + const withBackslash = new Map([...partials, ["backslash", "\\"]]); + expect(resolvePartials("a.md", "\\\\{{> auth}}", withBackslash)).toBe( + "\\{{> auth}}", + ); + expect( + resolvePartials("a.md", "{{> backslash}}{{> auth}}", withBackslash), + ).toBe("\\Authenticate first."); + }); + + it("still emits an escaped tag literally, even next to a brace", () => { + expect( + resolvePartials( + "a.md", + "Write \\{{> auth}} or {\\{{> auth}}}.", + partials, + ), + ).toBe("Write {{> auth}} or {{{> auth}}}."); + }); +}); + +describe("validate and partial tags", () => { + it("reports nothing for an escaped tag in a substituted file type", async () => { + const root = await setup( + ` cursor: { outDir: "dist", plugins: { acme: { source: "shared/acme" } } }`, + { + shared: { + acme: { + skills: { + a: { + "SKILL.md": `${skill("a")}\nReference a partial by writing \\{{> auth}} in a skill file.\n`, + }, + }, + }, + }, + }, + ); + await build({ cwd: root }); + await expect( + readFile(path.join(root, "dist/acme/skills/a/SKILL.md"), "utf8"), + ).resolves.toContain("writing {{> auth}} in"); + + await expect( + validateOutput("cursor", path.join(root, "dist")), + ).resolves.toEqual({ ok: true, issues: [] }); + }); +}); From 853cd266214b3017a7b49dd089af78b8185120fc Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 14:19:40 -0700 Subject: [PATCH 2/4] fix(validate): stop scanning the whole repo for a plugin emitted at the root Each target's validator reports the plugin directories it owns, and validate scans those for surviving partial tags. A plugin configured with path "." lives at the output root, so with outDir "." its directory is the repository itself. Validate then walked node_modules, authored sources and unrelated files, and failed on tags in pluginpack's own dependency cache. For a plugin at the output root, validate now scans only its component directories and static files. The managed manifest still covers the rest of the files the build wrote. Plugins in their own directory are scanned as before. --- src/targets/agent-plugins.ts | 6 ++- src/targets/claude.ts | 3 +- src/targets/codex.ts | 3 +- src/targets/copilot.ts | 3 +- src/targets/cursor.ts | 3 +- src/targets/validation-shared.ts | 18 +++++++++ tests/partials.test.ts | 68 ++++++++++++++++++++++++++++++++ 7 files changed, 98 insertions(+), 6 deletions(-) diff --git a/src/targets/agent-plugins.ts b/src/targets/agent-plugins.ts index d4b00ab..b065d16 100644 --- a/src/targets/agent-plugins.ts +++ b/src/targets/agent-plugins.ts @@ -16,7 +16,7 @@ import { CLIENT_ONLY_CONTENT_KINDS, PACKAGE_CLIENT_CONTENT_KINDS, } from "../schema.js"; -import { error } from "./validation-shared.js"; +import { error, pluginScanPaths } from "./validation-shared.js"; import type { ValidationIssue } from "../types.js"; import type { PluginTargetDefinition } from "./types.js"; @@ -90,7 +90,9 @@ export const agentPlugins: PluginTargetDefinition = { await validatePackage(pluginDir, path.basename(pluginDir), issues); await rejectClientOnlyContent(pluginDir, issues); } - const owned = [...pluginDirs]; + const owned = pluginDirs.flatMap((pluginDir) => + pluginScanPaths(root, pluginDir), + ); for (const client of Object.values(PACKAGE_CLIENTS)) { owned.push(...(await client.validate(root, pluginDirs, issues))); } diff --git a/src/targets/claude.ts b/src/targets/claude.ts index 31c2cc1..e7a0ac9 100644 --- a/src/targets/claude.ts +++ b/src/targets/claude.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { stripUndefined } from "./shared.js"; import { error, + pluginScanPaths, readJson, validateBareStringSourceEntry, validateFrontmatter, @@ -133,7 +134,7 @@ export const claude: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(pluginDir); + ownedPaths.push(...pluginScanPaths(root, pluginDir)); const manifest = await readJson( path.join(pluginDir, ".claude-plugin", "plugin.json"), `${pluginName} plugin manifest`, diff --git a/src/targets/codex.ts b/src/targets/codex.ts index ddba33a..4e80798 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -11,6 +11,7 @@ import { stripUndefined } from "./shared.js"; import { error, pathExistsSync, + pluginScanPaths, readJson, validateFrontmatter, validateHooksShape, @@ -252,7 +253,7 @@ export const codexMarketplace = { if (!pluginDir) { continue; } - ownedPaths.push(pluginDir); + ownedPaths.push(...pluginScanPaths(root, pluginDir)); const manifest = await validateCodexPlugin(pluginDir, pluginName, issues); if (!manifest) { continue; diff --git a/src/targets/copilot.ts b/src/targets/copilot.ts index cb53387..1bee514 100644 --- a/src/targets/copilot.ts +++ b/src/targets/copilot.ts @@ -3,6 +3,7 @@ import { exists, toPosix, walkFiles } from "../fs.js"; import { stripUndefined } from "./shared.js"; import { error, + pluginScanPaths, readJson, validateBareStringSourceEntry, validateFrontmatter, @@ -184,7 +185,7 @@ export const copilot: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(pluginDir); + ownedPaths.push(...pluginScanPaths(root, pluginDir)); // .github/plugin/ is the authoritative copy; the root copy is only // checked for presence, not re-parsed. const manifest = await readJson( diff --git a/src/targets/cursor.ts b/src/targets/cursor.ts index 4038c46..3400604 100644 --- a/src/targets/cursor.ts +++ b/src/targets/cursor.ts @@ -2,6 +2,7 @@ import path from "node:path"; import { stripUndefined, titleCase } from "./shared.js"; import { error, + pluginScanPaths, readJson, validateBareStringSourceEntry, validateFrontmatter, @@ -199,7 +200,7 @@ export const cursor: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(pluginDir); + ownedPaths.push(...pluginScanPaths(root, pluginDir)); const manifest = await readJson( path.join(pluginDir, ".cursor-plugin", "plugin.json"), `${pluginName} plugin manifest`, diff --git a/src/targets/validation-shared.ts b/src/targets/validation-shared.ts index 6e3138d..291e698 100644 --- a/src/targets/validation-shared.ts +++ b/src/targets/validation-shared.ts @@ -1,6 +1,7 @@ import { promises as fs, statSync } from "node:fs"; import path from "node:path"; import yaml from "js-yaml"; +import { componentDirs, staticFiles } from "../components.js"; import { exists, isNotFoundError, @@ -119,6 +120,23 @@ export async function validateNoSurvivingPartialTags( } } +/** + * The paths `validateNoSurvivingPartialTags` should scan for one plugin + * directory a target's validator found. A plugin emitted at the output root + * (`path: "."`) shares that root with authored sources, dependencies, and + * other files pluginpack does not own, so only its component directories and + * static files are scanned; the managed manifest covers the rest of its + * output. + */ +export function pluginScanPaths(root: string, pluginDir: string): string[] { + if (path.resolve(pluginDir) !== path.resolve(root)) { + return [pluginDir]; + } + return [...componentDirs, ...staticFiles].map((entry) => + path.join(root, entry), + ); +} + /** Reads and parses a JSON file, pushing an issue and returning `null` on failure. */ export async function readJson( filePath: string, diff --git a/tests/partials.test.ts b/tests/partials.test.ts index 5980095..5b2ec9a 100644 --- a/tests/partials.test.ts +++ b/tests/partials.test.ts @@ -239,4 +239,72 @@ describe("validate and partial tags", () => { validateOutput("cursor", path.join(root, "dist")), ).resolves.toEqual({ ok: true, issues: [] }); }); + + // A plugin emitted at the output root (`path: "."`) shares that root with + // authored sources and dependencies, which validate must not scan. + it.each([ + [ + "cursor", + ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ], + [ + "claude", + ` claude: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ], + [ + "copilot", + ` copilot: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ], + [ + "codex", + ` codex: { outDir: ".", plugins: { acme: { source: "shared/acme", path: ".", entry: { policy: { installation: "AVAILABLE", authentication: "NOT_REQUIRED" }, category: "Developer Tools" } } } }`, + ], + [ + "agent-plugins", + ` "agent-plugins": { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ], + ] as const)( + "validates a %s plugin emitted at the output root without scanning unrelated files", + async (target, targets) => { + const root = await setup(targets, { + shared: { + acme: { + skills: { a: { "SKILL.md": `${skill("a")}\n{{> auth}}\n` } }, + }, + }, + node_modules: { + dep: { "index.js": 'const example = "{{> x}}";\n' }, + ".cache": { jiti: { "x.mjs": "// {{> x}}\n" } }, + }, + notes: { "unrelated.yaml": "a: '{{> x}}'\n" }, + }); + await build({ cwd: root }); + + await expect(validateOutput(target, root)).resolves.toEqual({ + ok: true, + issues: [], + }); + }, + ); + + it("still flags a tag in a root-level plugin's component directory", async () => { + const root = await setup( + ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + { shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } } }, + ); + await build({ cwd: root }); + await writeFile(path.join(root, "skills/a/data.yaml"), "a: {{> auth}}\n"); + + const result = await validateOutput("cursor", root); + + expect(result.ok).toBe(false); + expect(result.issues).toEqual([ + { + level: "error", + message: expect.stringContaining( + "skills/a/data.yaml contains an unsubstituted partial reference {{> auth}}", + ), + }, + ]); + }); }); From 78fee04e2d9fcb1b99f51cbc8f0eedbf877bfd48 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 15:27:11 -0700 Subject: [PATCH 3/4] fix(validate): scan a root plugin's native and managed files only The previous change stopped validate from walking the whole repository for a plugin emitted at the output root, but it went too far in one direction and not far enough in the other. It dropped the target's native files from the scan. Without a managed manifest, a partial tag in the plugin manifest or MCP config of a root plugin passed validation, although the app reads those files and the previous release reported the tag. Each target now names the files its app reads from a plugin root (manifest, MCP config, hooks file, and for Agent Plugins each client's extension directory), and validate always scans them. It still scanned every recognized component directory when a managed manifest was present, so an unrelated repository script under scripts/ failed validation for a plugin that only ships skills. Validate now tells each target whether the output has managed state. With it, the managed file list covers the plugin's output and the component directories are not scanned. Without it, they remain the fallback. --- README.md | 2 + src/adapters.ts | 2 +- src/targets/agent-plugins.ts | 16 +++- src/targets/claude.ts | 13 ++- src/targets/codex.ts | 19 ++++- src/targets/copilot.ts | 14 +++- src/targets/cursor.ts | 13 ++- src/targets/engine.ts | 10 ++- src/targets/types.ts | 11 +++ src/targets/validation-shared.ts | 34 ++++++-- tests/partials.test.ts | 133 ++++++++++++++++++++++++++++++- 11 files changed, 241 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 75466ca..41cbd21 100644 --- a/README.md +++ b/README.md @@ -543,6 +543,8 @@ That emits `{{> auth}}` verbatim. A backslash directly before a tag always escap Because substitution does not run on other file types, a tag authored in (say) a `.yaml` reference file or a `.py` script would otherwise ship through untouched; `build` fails on any such tag left in emitted output, and `validate` reports it in an already-generated target repo. `validate` does not report tags in `.md`/`.mdc`/`.markdown`/`.txt` files: there, the only tag a build leaves is an escaped one. +For a plugin emitted at the output root (`path: "."`), `validate` does not walk the whole root, because authored sources and dependencies live there too. It checks the files listed in the target's managed manifest (`.pluginpack/.json`) and the files the app reads from the plugin root, such as the plugin manifest and MCP config. Without a managed manifest, it checks the recognized component directories (`skills/`, `scripts/`, and so on) instead. + ## Other Shapes There are two reasonable alternatives when the single-repo shape is not enough: diff --git a/src/adapters.ts b/src/adapters.ts index 17d1acf..e764fcb 100644 --- a/src/adapters.ts +++ b/src/adapters.ts @@ -49,7 +49,7 @@ export async function validateOutput( const root = path.resolve(dir); const issues: ValidationIssue[] = []; const managed = await readManagedManifest(root, target); - const managedPaths = (managed?.files ?? []).map((file) => + const managedPaths = managed?.files.map((file) => path.resolve(root, normalizeManagedPath(file)), ); await validateFromDefinition(root, issues, registry[target], managedPaths); diff --git a/src/targets/agent-plugins.ts b/src/targets/agent-plugins.ts index b065d16..9799eca 100644 --- a/src/targets/agent-plugins.ts +++ b/src/targets/agent-plugins.ts @@ -28,6 +28,18 @@ import type { PluginTargetDefinition } from "./types.js"; */ const noProfile = {}; +/** + * Plugin-relative files and directories a client reads from a package, + * whoever wrote them: the manifest, the MCP config, the hooks file, and each + * client's extension directory. + */ +const NATIVE_FILES = [ + packageLayout.manifest, + packageLayout.mcp, + "hooks/hooks.json", + ...Object.values(PACKAGE_CLIENTS).map((client) => client.namespace), +]; + /** * Agent Plugins target: one portable package per plugin under * `plugins//`. With no `clients`, it writes packages only — no @@ -78,7 +90,7 @@ export const agentPlugins: PluginTargetDefinition = { // Validates every package, then each client's marketplace and extension // directory. Output carries no config, so a client is validated when its // marketplace or extension directory is present. - validateOutput: async (root, issues) => { + validateOutput: async (root, issues, context) => { const pluginDirs = await findPackages(root); if (pluginDirs.length === 0) { error( @@ -91,7 +103,7 @@ export const agentPlugins: PluginTargetDefinition = { await rejectClientOnlyContent(pluginDir, issues); } const owned = pluginDirs.flatMap((pluginDir) => - pluginScanPaths(root, pluginDir), + pluginScanPaths(root, pluginDir, NATIVE_FILES, context), ); for (const client of Object.values(PACKAGE_CLIENTS)) { owned.push(...(await client.validate(root, pluginDirs, issues))); diff --git a/src/targets/claude.ts b/src/targets/claude.ts index e7a0ac9..db64a6a 100644 --- a/src/targets/claude.ts +++ b/src/targets/claude.ts @@ -13,6 +13,13 @@ import type { PluginTargetDefinition } from "./types.js"; const pluginNamePattern = /^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$/; +/** Plugin-relative files Claude Code reads from a plugin, whoever wrote them. */ +const NATIVE_FILES = [ + ".claude-plugin/plugin.json", + ".mcp.json", + "hooks/hooks.json", +]; + /** * Claude Code agent-plugin target. `validateManifest`'s required fields * (`version`, `description`, `author.name`) were verified directly against @@ -100,7 +107,7 @@ export const claude: PluginTargetDefinition = { pluginNamePattern, ), - validateOutput: async (root, issues) => { + validateOutput: async (root, issues, context) => { const marketplacePath = path.join( root, ".claude-plugin", @@ -134,7 +141,9 @@ export const claude: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(...pluginScanPaths(root, pluginDir)); + ownedPaths.push( + ...pluginScanPaths(root, pluginDir, NATIVE_FILES, context), + ); const manifest = await readJson( path.join(pluginDir, ".claude-plugin", "plugin.json"), `${pluginName} plugin manifest`, diff --git a/src/targets/codex.ts b/src/targets/codex.ts index 4e80798..62dbac1 100644 --- a/src/targets/codex.ts +++ b/src/targets/codex.ts @@ -24,6 +24,19 @@ import type { PluginTargetDefinition } from "./types.js"; const pluginNamePattern = /^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$/; const sourceKinds = new Set(["local", "url", "git-subdir", "npm"]); +/** + * Plugin-relative files Codex reads from a plugin, whoever wrote them, in + * either layout: the legacy `.codex-plugin/` manifest and `.mcp.json`, or + * the Agent Plugins package's `plugin.json` and `mcp.json`. + */ +const NATIVE_FILES = [ + ".codex-plugin/plugin.json", + ".mcp.json", + packageLayout.manifest, + packageLayout.mcp, + "hooks/hooks.json", +]; + /** * Resolves a marketplace entry's local plugin directory, or `null` when the * entry points somewhere pluginpack's own output doesn't contain (a remote @@ -220,7 +233,7 @@ export const codexMarketplace = { validateManifest: requireName, validateMarketplaceEntry: validateCodexEntry, - validateOutput: async (root, issues) => { + validateOutput: async (root, issues, context) => { const marketplacePath = path.join( root, ".agents", @@ -253,7 +266,9 @@ export const codexMarketplace = { if (!pluginDir) { continue; } - ownedPaths.push(...pluginScanPaths(root, pluginDir)); + ownedPaths.push( + ...pluginScanPaths(root, pluginDir, NATIVE_FILES, context), + ); const manifest = await validateCodexPlugin(pluginDir, pluginName, issues); if (!manifest) { continue; diff --git a/src/targets/copilot.ts b/src/targets/copilot.ts index 1bee514..835ac44 100644 --- a/src/targets/copilot.ts +++ b/src/targets/copilot.ts @@ -15,6 +15,14 @@ import type { PluginTargetDefinition } from "./types.js"; const pluginNamePattern = /^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$/; +/** Plugin-relative files Copilot reads from a plugin, whoever wrote them. */ +const NATIVE_FILES = [ + ".github/plugin/plugin.json", + "plugin.json", + ".mcp.json", + "hooks/hooks.json", +]; + /** Kebab-case, letters/numbers/hyphens only, per the Copilot plugin.json field reference. */ const copilotNamePattern = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; @@ -139,7 +147,7 @@ export const copilot: PluginTargetDefinition = { pluginNamePattern, ), - validateOutput: async (root, issues) => { + validateOutput: async (root, issues, context) => { const marketplacePath = path.join( root, ".claude-plugin", @@ -185,7 +193,9 @@ export const copilot: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(...pluginScanPaths(root, pluginDir)); + ownedPaths.push( + ...pluginScanPaths(root, pluginDir, NATIVE_FILES, context), + ); // .github/plugin/ is the authoritative copy; the root copy is only // checked for presence, not re-parsed. const manifest = await readJson( diff --git a/src/targets/cursor.ts b/src/targets/cursor.ts index 3400604..82c7aad 100644 --- a/src/targets/cursor.ts +++ b/src/targets/cursor.ts @@ -14,6 +14,13 @@ import type { PluginTargetDefinition } from "./types.js"; const pluginNamePattern = /^[a-z0-9](?:[a-z0-9.-]*[a-z0-9])?$/; +/** Plugin-relative files Cursor reads from a plugin, whoever wrote them. */ +const NATIVE_FILES = [ + ".cursor-plugin/plugin.json", + ".mcp.json", + "hooks/hooks.json", +]; + /** * Top-level `plugin.json` keys per the vendored Cursor plugin schema * (`tests/fixtures/cursor/plugin.schema.json`, `additionalProperties: false`). @@ -166,7 +173,7 @@ export const cursor: PluginTargetDefinition = { pluginNamePattern, ), - validateOutput: async (root, issues) => { + validateOutput: async (root, issues, context) => { const marketplacePath = path.join( root, ".cursor-plugin", @@ -200,7 +207,9 @@ export const cursor: PluginTargetDefinition = { continue; } const pluginDir = path.join(root, entry.source); - ownedPaths.push(...pluginScanPaths(root, pluginDir)); + ownedPaths.push( + ...pluginScanPaths(root, pluginDir, NATIVE_FILES, context), + ); const manifest = await readJson( path.join(pluginDir, ".cursor-plugin", "plugin.json"), `${pluginName} plugin manifest`, diff --git a/src/targets/engine.ts b/src/targets/engine.ts index 93c2d5c..2f0778d 100644 --- a/src/targets/engine.ts +++ b/src/targets/engine.ts @@ -321,17 +321,21 @@ export async function emitFromDefinition( /** * Validates one target's output using its `PluginTargetDefinition`, plus the * checks that hold for every target regardless of its layout. + * `managedPaths` is the managed manifest's file list, or `undefined` when + * the output has no managed manifest for this target. */ export async function validateFromDefinition( root: string, issues: ValidationIssue[], definition: PluginTargetDefinition, - managedPaths: string[] = [], + managedPaths?: string[], ): Promise { - const ownedPaths = await definition.validateOutput(root, issues); + const ownedPaths = await definition.validateOutput(root, issues, { + hasManagedState: managedPaths !== undefined, + }); await validateNoSurvivingPartialTags( root, - [...ownedPaths, ...managedPaths], + [...ownedPaths, ...(managedPaths ?? [])], issues, ); } diff --git a/src/targets/types.ts b/src/targets/types.ts index 943c222..f2d849d 100644 --- a/src/targets/types.ts +++ b/src/targets/types.ts @@ -9,6 +9,16 @@ import type { ValidationIssue, } from "../types.js"; +/** What validation knows about the output beyond its files. */ +export type OutputValidationContext = { + /** + * Whether the output directory has a managed-file manifest for this + * target. When it does, validation scans the files it lists; without one, + * a target falls back to the paths its layout implies. + */ + hasManagedState: boolean; +}; + /** Inputs available when building a single plugin's own manifest file. */ export type ManifestBuildContext = { metadata: Metadata | undefined; @@ -183,6 +193,7 @@ export type PluginTargetDefinition = { validateOutput: ( root: string, issues: ValidationIssue[], + context: OutputValidationContext, ) => Promise; installSnippet: InstallSnippetDefinition; diff --git a/src/targets/validation-shared.ts b/src/targets/validation-shared.ts index 291e698..4647e21 100644 --- a/src/targets/validation-shared.ts +++ b/src/targets/validation-shared.ts @@ -16,6 +16,7 @@ import { substitutionRunsOn, } from "../partials.js"; import type { TargetName, ValidationIssue } from "../types.js"; +import type { OutputValidationContext } from "./types.js"; export const marketplaceNamePattern = /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/; @@ -122,18 +123,35 @@ export async function validateNoSurvivingPartialTags( /** * The paths `validateNoSurvivingPartialTags` should scan for one plugin - * directory a target's validator found. A plugin emitted at the output root - * (`path: "."`) shares that root with authored sources, dependencies, and - * other files pluginpack does not own, so only its component directories and - * static files are scanned; the managed manifest covers the rest of its - * output. + * directory a target's validator found. A plugin in its own directory is + * scanned whole. + * + * A plugin emitted at the output root (`path: "."`) shares that root with + * authored sources, dependencies, and other files pluginpack does not own, + * so the root is never walked. Instead: + * + * - `nativeFiles` (plugin-relative manifest, MCP, hooks, and extension + * paths) are always scanned, because the host reads them from a root + * plugin whoever wrote them. + * - With managed state, the managed manifest's file list covers the rest of + * the output (the engine adds it), so nothing else is scanned. + * - Without managed state, the recognized component directories and static + * files are scanned as the best available guess at the plugin's output. */ -export function pluginScanPaths(root: string, pluginDir: string): string[] { +export function pluginScanPaths( + root: string, + pluginDir: string, + nativeFiles: readonly string[], + context: OutputValidationContext, +): string[] { if (path.resolve(pluginDir) !== path.resolve(root)) { return [pluginDir]; } - return [...componentDirs, ...staticFiles].map((entry) => - path.join(root, entry), + const fallback = context.hasManagedState + ? [] + : [...componentDirs, ...staticFiles]; + return [...nativeFiles, ...fallback].map((entry) => + path.join(pluginDir, entry), ); } diff --git a/tests/partials.test.ts b/tests/partials.test.ts index 5b2ec9a..170dcdd 100644 --- a/tests/partials.test.ts +++ b/tests/partials.test.ts @@ -1,4 +1,4 @@ -import { readFile, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { Project, type ProjectArgs } from "fixturify-project"; import { afterEach, describe, expect, it } from "vitest"; @@ -242,28 +242,41 @@ describe("validate and partial tags", () => { // A plugin emitted at the output root (`path: "."`) shares that root with // authored sources and dependencies, which validate must not scan. - it.each([ + // Each row: target, targets block, native manifest, native MCP file. + const rootTargets = [ [ "cursor", ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ".cursor-plugin/plugin.json", + ".mcp.json", ], [ "claude", ` claude: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ".claude-plugin/plugin.json", + ".mcp.json", ], [ "copilot", ` copilot: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + ".github/plugin/plugin.json", + ".mcp.json", ], [ "codex", ` codex: { outDir: ".", plugins: { acme: { source: "shared/acme", path: ".", entry: { policy: { installation: "AVAILABLE", authentication: "NOT_REQUIRED" }, category: "Developer Tools" } } } }`, + "plugin.json", + "mcp.json", ], [ "agent-plugins", ` "agent-plugins": { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + "plugin.json", + "mcp.json", ], - ] as const)( + ] as const; + + it.each(rootTargets)( "validates a %s plugin emitted at the output root without scanning unrelated files", async (target, targets) => { const root = await setup(targets, { @@ -287,12 +300,124 @@ describe("validate and partial tags", () => { }, ); - it("still flags a tag in a root-level plugin's component directory", async () => { + // The host reads its native manifest and MCP file from a root plugin no + // matter which tool wrote them, so validate checks them with or without + // the managed manifest. + it.each(rootTargets)( + "flags a tag in a %s root plugin's native manifest and MCP file, with or without managed state", + async (target, targets, manifestPath, mcpPath) => { + const root = await setup(targets, { + shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } }, + }); + await build({ cwd: root }); + const manifestFile = path.join(root, manifestPath); + const manifest = JSON.parse(await readFile(manifestFile, "utf8")); + manifest.description = "{{> auth}}"; + await writeFile(manifestFile, JSON.stringify(manifest)); + await writeFile( + path.join(root, mcpPath), + JSON.stringify({ mcpServers: { s: { command: "{{> auth}}" } } }), + ); + const expected = { + ok: false, + issues: expect.arrayContaining([ + { + level: "error", + message: expect.stringContaining( + `${manifestPath} contains an unsubstituted partial reference {{> auth}}`, + ), + }, + { + level: "error", + message: expect.stringContaining( + `${mcpPath} contains an unsubstituted partial reference {{> auth}}`, + ), + }, + ]), + }; + + await expect(validateOutput(target, root)).resolves.toEqual(expected); + await rm(path.join(root, ".pluginpack"), { recursive: true }); + await expect(validateOutput(target, root)).resolves.toEqual(expected); + }, + ); + + it("flags a tag in a root Agent Plugins package's client extension directory without managed state", async () => { + const root = await setup( + ` "agent-plugins": { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + { shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } } }, + ); + await build({ cwd: root }); + await rm(path.join(root, ".pluginpack"), { recursive: true }); + await mkdir(path.join(root, "com.github.copilot")); + await writeFile( + path.join(root, "com.github.copilot/lsp.json"), + '{"a": "{{> auth}}"}', + ); + + await expect(validateOutput("agent-plugins", root)).resolves.toEqual({ + ok: false, + issues: [ + { + level: "error", + message: expect.stringContaining( + "com.github.copilot/lsp.json contains an unsubstituted partial reference {{> auth}}", + ), + }, + ], + }); + }); + + it("scans only managed files and native files for a root plugin with managed state", async () => { + const root = await setup( + ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: ".", include: ["skills"] } } }`, + { + shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } }, + scripts: { "check-template.mjs": 'const tag = "{{> auth}}";\n' }, + }, + ); + await build({ cwd: root }); + + await expect(validateOutput("cursor", root)).resolves.toEqual({ + ok: true, + issues: [], + }); + }); + + it("flags a tag in a managed file of a root-level plugin", async () => { + const root = await setup( + ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, + { + shared: { + acme: { + skills: { a: { "SKILL.md": skill("a"), "data.yaml": "a: 1\n" } }, + }, + }, + }, + ); + await build({ cwd: root }); + await writeFile(path.join(root, "skills/a/data.yaml"), "a: {{> auth}}\n"); + + const result = await validateOutput("cursor", root); + + expect(result.ok).toBe(false); + expect(result.issues).toEqual([ + { + level: "error", + message: expect.stringContaining( + "skills/a/data.yaml contains an unsubstituted partial reference {{> auth}}", + ), + }, + ]); + }); + + it("falls back to a root-level plugin's component directories without managed state", async () => { const root = await setup( ` cursor: { outDir: ".", plugins: { acme: { source: "shared/acme", path: "." } } }`, { shared: { acme: { skills: { a: { "SKILL.md": skill("a") } } } } }, ); await build({ cwd: root }); + await rm(path.join(root, ".pluginpack"), { recursive: true }); await writeFile(path.join(root, "skills/a/data.yaml"), "a: {{> auth}}\n"); const result = await validateOutput("cursor", root); From a8f97a88fafb8e610cb159ef5ef116777f2e3492 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 21 Oct 2026 15:52:01 -0700 Subject: [PATCH 4/4] fix(partials): accept any Uint8Array when checking UTF-8 File values may be Uint8Array rather than Buffer, so decode through Buffer.from instead of calling Buffer methods on the value. --- src/partials.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/partials.ts b/src/partials.ts index 42d7159..538238d 100644 --- a/src/partials.ts +++ b/src/partials.ts @@ -90,8 +90,8 @@ function normalizePartial(text: string): string { } /** Decodes UTF-8, or returns `undefined` if the bytes do not survive a decode/encode round trip. */ -function decodeUtf8(bytes: Buffer): string | undefined { - const text = bytes.toString("utf8"); +function decodeUtf8(bytes: Uint8Array): string | undefined { + const text = Buffer.from(bytes).toString("utf8"); return Buffer.from(text, "utf8").equals(bytes) ? text : undefined; }