From 7b2f787810c4c93472d10091407984dc85adcd4f Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 5 Aug 2026 15:14:27 -0700 Subject: [PATCH 1/3] internal: Re-roll lockfile --- package-lock.json | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/package-lock.json b/package-lock.json index 487b7db..7c109de 100644 --- a/package-lock.json +++ b/package-lock.json @@ -218,7 +218,6 @@ "os": [ "aix" ], - "peer": true, "engines": { "node": ">=18" } @@ -236,7 +235,6 @@ "os": [ "android" ], - "peer": true, "engines": { "node": ">=18" } @@ -254,7 +252,6 @@ "os": [ "android" ], - "peer": true, "engines": { "node": ">=18" } @@ -272,7 +269,6 @@ "os": [ "android" ], - "peer": true, "engines": { "node": ">=18" } @@ -290,7 +286,6 @@ "os": [ "darwin" ], - "peer": true, "engines": { "node": ">=18" } @@ -308,7 +303,6 @@ "os": [ "darwin" ], - "peer": true, "engines": { "node": ">=18" } @@ -326,7 +320,6 @@ "os": [ "freebsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -344,7 +337,6 @@ "os": [ "freebsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -362,7 +354,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -380,7 +371,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -398,7 +388,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -416,7 +405,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -434,7 +422,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -452,7 +439,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -470,7 +456,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -488,7 +473,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -506,7 +490,6 @@ "os": [ "linux" ], - "peer": true, "engines": { "node": ">=18" } @@ -524,7 +507,6 @@ "os": [ "netbsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -542,7 +524,6 @@ "os": [ "netbsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -560,7 +541,6 @@ "os": [ "openbsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -578,7 +558,6 @@ "os": [ "openbsd" ], - "peer": true, "engines": { "node": ">=18" } @@ -596,7 +575,6 @@ "os": [ "openharmony" ], - "peer": true, "engines": { "node": ">=18" } @@ -614,7 +592,6 @@ "os": [ "sunos" ], - "peer": true, "engines": { "node": ">=18" } @@ -632,7 +609,6 @@ "os": [ "win32" ], - "peer": true, "engines": { "node": ">=18" } @@ -650,7 +626,6 @@ "os": [ "win32" ], - "peer": true, "engines": { "node": ">=18" } @@ -668,7 +643,6 @@ "os": [ "win32" ], - "peer": true, "engines": { "node": ">=18" } @@ -5841,7 +5815,6 @@ "dev": true, "hasInstallScript": true, "license": "MIT", - "peer": true, "bin": { "esbuild": "bin/esbuild" }, From 783b79848f4e0d5c68971c583a20bf0ddd9d9143 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 5 Aug 2026 16:51:29 -0700 Subject: [PATCH 2/3] fix: fail the build on unresolvable partial references A {{> name}} tag naming a partial that does not exist rendered as an empty string, so a typo or a renamed partial silently deleted a section from every skill that included it. Unrelated {{...}} text in the same file was rendered away as well, and a tag in a file type substitution skips (.yaml, .py) shipped through into output verbatim. - unknown, malformed, and nested-unknown references now fail the build, naming the emitted file, the nearest match, and the available partials - only partial tags are substituted now: mustache gets a function for its partials argument, and every other {{ is swapped for a sentinel before it parses, so other brace text survives byte for byte, including text mustache cannot parse as a template at all - \{{> name}} emits a literal tag, for files that document partial syntax - build fails, and validate reports, when a tag survives in a file type substitution does not run on BREAKING CHANGE: an unresolvable partial reference is now a build error rather than rendering as an empty string. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 13 +- README.md | 12 +- src/build.ts | 25 +++ src/partials.ts | 242 ++++++++++++++++++++++++++-- src/targets/engine.ts | 7 +- src/targets/validation-shared.ts | 47 ++++++ tests/core.test.ts | 266 ++++++++++++++++++++++++++++--- 7 files changed, 568 insertions(+), 44 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 42b7f74..ebbca56 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,10 +46,15 @@ diff, prune, and validate all derive from it. `targets//` override resolution) and `resolveMcpServers`. - `src/partials.ts` — `loadPartials`/`resolvePartials`: project-level `{{> name}}` text-reuse, wired into `collectPluginFiles` and - `withRootFiles`. Thin wrapper around the real `mustache` library (view is - always `{}` — no config/env data is ever exposed; this is not a general - templating hook), plus one custom check `mustache` doesn't provide - (circular partial reference detection at load time). + `withRootFiles`. Substitution is real `mustache` rendering (view is always + `{}` — no config/env data is ever exposed; this is not a general templating + hook), but only ever sees partial tags: every other `{{` is swapped for a + sentinel first and restored after, so text that merely looks like a template + survives byte-for-byte. An unresolvable tag is a build error, not an empty + string. Also holds two checks `mustache` doesn't provide — circular partial + reference detection at load time, and `findUnsubstitutedPartialTags`, the + guard for tags in file types substitution skips (called from `build()` and, + reading from disk, from `validate`). - `src/targets/registry.ts` — `targets: Record`, one file per target (`src/targets/.ts`). Everything that varies by target — default components, manifest/marketplace builders, output paths, diff --git a/README.md b/README.md index be71dbf..51dbd40 100644 --- a/README.md +++ b/README.md @@ -383,11 +383,19 @@ 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 `{{> name}}` tag with no matching partial renders as nothing rather than failing the build (see Known limitation below), and a tag alone on its own line — the common case — leaves no blank line behind when it resolves to nothing. +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, `additionalFiles`, and a target's `rootFiles` — via the real [`mustache`](https://github.com/janl/mustache.js) library. -**Known limitation:** because substitution is real Mustache rendering, any _other_ `{{...}}`-looking text in the same file — documentation about Handlebars, Angular, Go templates, Jinja, or Mustache itself, or a curly-brace code sample — is also processed against an empty context and typically disappears. A skill that needs to show literal double-curly-brace syntax has to work around it, e.g. by splitting the braces across adjacent inline-code spans (`{{` + `}}`) rather than writing them as one contiguous run. +**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. + +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: + +```md +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. ## Other Shapes diff --git a/src/build.ts b/src/build.ts index d2bb1e6..f03edc8 100644 --- a/src/build.ts +++ b/src/build.ts @@ -9,6 +9,10 @@ import { writeManagedManifest, } from "./managed.js"; import { emitTarget, targetNames } from "./adapters.js"; +import { + findUnsubstitutedPartialTags, + substitutedExtensions, +} from "./partials.js"; import type { Artifact, BuildOptions, @@ -33,6 +37,7 @@ export async function build(options: BuildOptions = {}): Promise { } const owner = assertNoCrossTargetCollisions(artifacts); await assertNoCollisionsWithBuiltTargets(project, targets, owner); + assertNoUnsubstitutedPartialTags(artifacts); if (!options.dryRun) { // Write every target's new files before pruning any target's stale ones. // If a later target's write throws, no target has had files pruned yet — @@ -51,6 +56,26 @@ export async function build(options: BuildOptions = {}): Promise { return artifacts; } +// A `{{> name}}` tag that reaches output is broken content: whatever reads the +// file gets a template marker instead of the text it was meant to inline. +// Substitution resolves (or rejects) every tag in the file types it runs on, so +// what is left is a tag authored somewhere it never ran — a reference file, a +// script, a data file. Fail rather than ship it. +function assertNoUnsubstitutedPartialTags(artifacts: Artifact[]): void { + const found = artifacts.flatMap((artifact) => + findUnsubstitutedPartialTags(artifact.files).map( + (tag) => ` ${artifact.target}: ${tag.path} contains ${tag.tag}`, + ), + ); + if (found.length > 0) { + throw new Error( + `Unsubstituted partial references in emitted output:\n${found.join("\n")}\n` + + `Partial substitution only runs on ${substitutedExtensions()} files. ` + + `Move the shared text into one of those, or inline it here instead.`, + ); + } +} + // Two targets pointed at overlapping output paths would silently overwrite each // other (and one target's prune could delete the other's files). Catch it. // Returns the absolute-path -> owning-target map so diff --git a/src/partials.ts b/src/partials.ts index 15cdf42..8eb2f6e 100644 --- a/src/partials.ts +++ b/src/partials.ts @@ -4,10 +4,34 @@ import mustache from "mustache"; import { toPosix, walkFiles } from "./fs.js"; import type { FileValue } from "./types.js"; +/** File extensions partial substitution runs on. */ const TEXTUAL_EXTENSIONS = [".md", ".mdc", ".markdown", ".txt"]; -/** A `{{> name}}` reference, used only to build the cycle-detection graph below — never to render. */ -const PARTIAL_REFERENCE = /\{\{>\s*([\w./-]+)\s*\}\}/g; +/** + * The one construct pluginpack substitutes: a `{{> name}}` partial tag. Any + * other `{{...}}`-shaped text is left exactly as authored (see `substitute`). + */ +const PARTIAL_TAG = /\{\{>\s*([\w./-]+)\s*\}\}/; +/** Sticky, to test for a tag at one exact offset without slicing the rest of the file. */ +const PARTIAL_TAG_STICKY = new RegExp(PARTIAL_TAG.source, "y"); +const PARTIAL_TAG_GLOBAL = new RegExp(PARTIAL_TAG.source, "g"); + +/** A `{{>` opener, well-formed or not — the trigger for doing any partial work at all. */ +const PARTIAL_OPENER = "{{>"; + +/** + * Writing `\{{> name}}` emits a literal `{{> name}}` instead of substituting, + * so a file can document partial syntax itself. + */ +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. + */ +const SENTINEL = ""; /** * Loads every file under `partialsDir` into a name -> content map, keyed by @@ -46,7 +70,7 @@ export async function loadPartials( function assertNoPartialCycles(partials: Map): void { const references = (content: string): string[] => { const names: string[] = []; - for (const match of content.matchAll(PARTIAL_REFERENCE)) { + for (const match of content.matchAll(PARTIAL_TAG_GLOBAL)) { names.push(match[1]); } return names; @@ -81,29 +105,217 @@ function assertNoPartialCycles(partials: Map): void { } } +/** Thrown through `Mustache.render` when it asks for a partial that does not exist. */ +class UnknownPartialError extends Error { + constructor(readonly partialName: string) { + super(`unknown partial "${partialName}"`); + } +} + /** - * Substitutes `{{> name}}` partial references in a textual file's content, - * via the real `mustache` library — an empty view (`{}`) is always passed, - * so no config/environment data is ever exposed to interpolation; only - * partials resolve to real content, everything else Mustache-shaped resolves - * to `""` per its own documented missing-key behavior (a known, documented - * limitation, not a bug — see README). + * Substitutes `{{> name}}` partial references in a textual file's content. + * A tag naming a partial that does not exist is an error, not an empty + * string — silently dropping a section from a shipped skill file is worse + * than a failed build. * - * Non-textual paths, and textual files with no `{{` at all, are returned - * completely untouched (the original `FileValue`, byte-identical) — avoids a - * lossy UTF-8 round-trip for files that don't use partials at all. + * 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. */ export function resolvePartials( relativePath: string, value: FileValue, partials: Map, ): FileValue { - if (!TEXTUAL_EXTENSIONS.includes(path.extname(relativePath))) { + if (!substitutionRunsOn(relativePath)) { return value; } const text = typeof value === "string" ? value : value.toString("utf8"); - if (!text.includes("{{")) { + if (!text.includes(PARTIAL_OPENER)) { return value; } - return mustache.render(text, {}, Object.fromEntries(partials)); + try { + return substitute(text, partials); + } catch (error) { + throw new Error( + `Partial substitution failed in "${relativePath}": ${describe(error, partials)}`, + { cause: error }, + ); + } +} + +/** Whether partial substitution runs on files at this path (by extension). */ +export function substitutionRunsOn(relativePath: string): boolean { + return TEXTUAL_EXTENSIONS.includes(path.extname(relativePath)); +} + +/** The extensions substitution runs on, for error messages (`.md, .mdc, ...`). */ +export function substitutedExtensions(): string { + return TEXTUAL_EXTENSIONS.join(", "); +} + +/** + * Renders via the real `mustache` library, but only ever hands it partial + * tags: every other `{{` is swapped for a sentinel first (`neutralize`) and + * restored afterwards. Mustache therefore still owns the substitution + * semantics that are genuinely subtle — standalone-line handling and partial + * indentation — while text that merely looks like a template (Handlebars, + * Jinja, Go, or Mustache documentation; a curly-brace code sample) survives + * byte-for-byte instead of being rendered against an empty view and + * vanishing. It also cannot fail to parse on such a file, since none of it + * reaches Mustache's parser as a tag. + * + * An empty view (`{}`) is always passed, so no config or environment data is + * ever exposed to interpolation. + */ +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, "{{"); +} + +function prepare(text: string): string { + if (text.includes(SENTINEL)) { + throw new Error( + "content contains the reserved code point U+E000, which pluginpack uses internally while substituting partials", + ); + } + return neutralize(text); +} + +/** + * Replaces the opening `{{` of every non-partial construct with `SENTINEL`, + * leaving real partial tags as the only tags Mustache can see. An escaped + * tag (`\{{> name}}`) is neutralized too, and its backslash consumed, so it + * renders as the literal tag it documents. + */ +function neutralize(text: string): string { + let out = ""; + let index = 0; + while (index < text.length) { + const open = text.indexOf("{{", index); + if (open === -1) { + out += text.slice(index); + break; + } + const escaped = text[open - 1] === ESCAPE; + PARTIAL_TAG_STICKY.lastIndex = open; + const isPartialTag = PARTIAL_TAG_STICKY.test(text); + if (isPartialTag && !escaped) { + out += text.slice(index, open + 2); + index = open + 2; + continue; + } + if (text.startsWith(PARTIAL_OPENER, open) && !escaped) { + throw new Error( + `malformed partial reference "${firstLine(text.slice(open))}" — expected \`{{> name}}\``, + ); + } + out += text.slice(index, isPartialTag ? open - ESCAPE.length : open); + out += SENTINEL; + index = open + 2; + } + return out; +} + +/** Turns a thrown error into a message, expanding an unknown partial into actionable advice. */ +function describe(error: unknown, partials: Map): string { + if (!(error instanceof UnknownPartialError)) { + return error instanceof Error ? error.message : String(error); + } + const names = [...partials.keys()]; + if (names.length === 0) { + return `${error.message}, and no partials are configured — set \`source.partials\` in pluginpack.config.ts`; + } + const suggestion = closestName(error.partialName, names); + return [ + error.message, + suggestion ? ` (did you mean "${suggestion}"?)` : "", + `. Available partials: ${names.sort().join(", ")}`, + ].join(""); +} + +/** The candidate within a small edit distance of `name`, if one is close enough to suggest. */ +function closestName(name: string, candidates: string[]): string | undefined { + const limit = Math.max(2, Math.floor(name.length / 4)); + let best: string | undefined; + let bestDistance = Number.POSITIVE_INFINITY; + for (const candidate of candidates) { + const distance = editDistance(name.toLowerCase(), candidate.toLowerCase()); + if (distance < bestDistance) { + best = candidate; + bestDistance = distance; + } + } + return bestDistance <= limit ? best : undefined; +} + +function editDistance(a: string, b: string): number { + let previous = Array.from({ length: b.length + 1 }, (_, i) => i); + for (let i = 1; i <= a.length; i += 1) { + const current = [i]; + for (let j = 1; j <= b.length; j += 1) { + current[j] = Math.min( + previous[j] + 1, + current[j - 1] + 1, + previous[j - 1] + (a[i - 1] === b[j - 1] ? 0 : 1), + ); + } + previous = current; + } + return previous[b.length]; +} + +function firstLine(value: string): string { + return value.split("\n", 1)[0].slice(0, 40); +} + +/** A partial tag found in a file partial substitution never ran on. */ +export type UnsubstitutedPartialTag = { + path: string; + tag: string; +}; + +/** + * 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. + */ +export function findUnsubstitutedPartialTags( + files: Iterable, +): UnsubstitutedPartialTag[] { + const found: UnsubstitutedPartialTag[] = []; + for (const [relativePath, value] of files) { + if (substitutionRunsOn(relativePath)) { + continue; + } + const tag = findPartialTag(value); + if (tag) { + found.push({ path: relativePath, tag }); + } + } + return found; +} + +/** The first `{{> name}}` tag in a file's contents, or `undefined` for binary/tag-free files. */ +export function findPartialTag(value: FileValue): string | undefined { + if (typeof value !== "string" && value.includes(0)) { + return undefined; + } + const text = typeof value === "string" ? value : value.toString("utf8"); + return PARTIAL_TAG.exec(text)?.[0]; } diff --git a/src/targets/engine.ts b/src/targets/engine.ts index 03f25cd..d47caa0 100644 --- a/src/targets/engine.ts +++ b/src/targets/engine.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { collectPluginFiles, resolveMcpServers } from "../render.js"; import { isSafeRelativePath, json, toPosix } from "../fs.js"; import { resolvePartials } from "../partials.js"; +import { validateNoSurvivingPartialTags } from "./validation-shared.js"; import { deepMerge, stripUndefined } from "./shared.js"; import { applyUpdateCheck, pluginAllowsUpdateCheck } from "../update-check.js"; import type { UpdateCheckFormat } from "../update-check.js"; @@ -197,13 +198,17 @@ export async function emitFromDefinition( return artifact(target, outDir, files); } -/** Validates one target's output using its `PluginTargetDefinition`. */ +/** + * Validates one target's output using its `PluginTargetDefinition`, plus the + * checks that hold for every target regardless of its layout. + */ export async function validateFromDefinition( root: string, issues: ValidationIssue[], definition: PluginTargetDefinition, ): Promise { await definition.validateOutput(root, issues); + await validateNoSurvivingPartialTags(root, issues); } function emittedPluginMetadata( diff --git a/src/targets/validation-shared.ts b/src/targets/validation-shared.ts index 75b6754..abb6518 100644 --- a/src/targets/validation-shared.ts +++ b/src/targets/validation-shared.ts @@ -9,6 +9,11 @@ import { walkFiles, } from "../fs.js"; import { UPDATE_CHECK_SCRIPT_PATH } from "../update-check.js"; +import { + findPartialTag, + substitutedExtensions, + substitutionRunsOn, +} from "../partials.js"; import type { TargetName, ValidationIssue } from "../types.js"; export const marketplaceNamePattern = /^[a-z0-9](?:[a-z0-9-]*[a-z0-9])?$/; @@ -59,6 +64,48 @@ export function error(issues: ValidationIssue[], message: string): void { issues.push({ level: "error", message }); } +/** Pushes a warning-level issue onto `issues` (does not fail validation). */ +export function warning(issues: ValidationIssue[], message: string): void { + issues.push({ level: "warning", message }); +} + +/** + * 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. + * + * 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. + */ +export async function validateNoSurvivingPartialTags( + root: string, + issues: ValidationIssue[], +): Promise { + for (const file of await walkFiles(root)) { + const relative = toPosix(path.relative(root, file)); + const tag = findPartialTag(await fs.readFile(file)); + if (!tag) { + 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 { + error( + issues, + `${message} Partial substitution does not run on this file type (${substitutedExtensions()}).`, + ); + } + } +} + /** Reads and parses a JSON file, pushing an issue and returning `null` on failure. */ export async function readJson( filePath: string, diff --git a/tests/core.test.ts b/tests/core.test.ts index 8930d6c..a0106ee 100644 --- a/tests/core.test.ts +++ b/tests/core.test.ts @@ -2643,7 +2643,7 @@ More instructions. `); }); - it("renders a missing partial as empty, cleanly (no orphaned blank line)", async () => { + it("fails the build on a partial reference no partial satisfies", async () => { const project = await fixtureProject({ "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; @@ -2651,6 +2651,7 @@ export default defineConfig({ name: "partials-missing-plugins", version: "1.0.0", metadata: { description: "Partials", author: { name: "X" }, license: "MIT" }, + source: { partials: "partials" }, targets: { claude: { outDir: "dist/claude", @@ -2659,6 +2660,7 @@ export default defineConfig({ } }); `, + partials: { "auth.md": "Authenticate first.\n" }, plugins: { demo: { skills: { @@ -2669,11 +2671,41 @@ description: Demo skill. --- Before -{{> nonexistent}} +{{> autb}} After +`, + }, + }, + }, + }, + }); + const root = project.baseDir; + + await expect(build({ cwd: root, target: "claude" })).rejects.toThrow( + /Partial substitution failed in "skills\/demo\/SKILL\.md": unknown partial "autb" \(did you mean "auth"\?\)\. Available partials: auth/, + ); + }); -See: {{> nonexistent}} above. + it("names the config key when a partial is referenced with no partials configured", async () => { + const project = await fixtureProject({ + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "partials-unconfigured", + version: "1.0.0", + targets: { + claude: { + outDir: "dist/claude", + plugins: { demo: { from: ["demo"] } } + } + } +}); `, + plugins: { + demo: { + skills: { + demo: { + "SKILL.md": skill("demo", "Demo skill.") + "\n{{> auth}}\n", }, }, }, @@ -2681,22 +2713,75 @@ See: {{> nonexistent}} above. }); const root = project.baseDir; - await build({ cwd: root, target: "claude" }); + await expect(build({ cwd: root, target: "claude" })).rejects.toThrow( + /no partials are configured — set `source\.partials`/, + ); + }); - const content = await readFile( - path.join(root, "dist/claude/plugins/demo/skills/demo/SKILL.md"), - "utf8", + it("fails the build on an unknown partial referenced from inside another partial", async () => { + const project = await fixtureProject({ + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "partials-nested-missing", + version: "1.0.0", + source: { partials: "partials" }, + targets: { + claude: { + outDir: "dist/claude", + plugins: { demo: { from: ["demo"] } } + } + } +}); +`, + partials: { "auth.md": "Authenticate first.\n{{> footnote}}\n" }, + plugins: { + demo: { + skills: { + demo: { + "SKILL.md": skill("demo", "Demo skill.") + "\n{{> auth}}\n", + }, + }, + }, + }, + }); + const root = project.baseDir; + + await expect(build({ cwd: root, target: "claude" })).rejects.toThrow( + /unknown partial "footnote"/, ); - expect(content).toBe(`--- -name: demo -description: Demo skill. ---- + }); -Before -After + it("fails the build on a malformed partial reference", async () => { + const project = await fixtureProject({ + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; -See: above. -`); +export default defineConfig({ + name: "partials-malformed", + version: "1.0.0", + source: { partials: "partials" }, + targets: { + claude: { + outDir: "dist/claude", + plugins: { demo: { from: ["demo"] } } + } + } +}); +`, + partials: { "auth.md": "Authenticate first.\n" }, + plugins: { + demo: { + skills: { + demo: { "SKILL.md": skill("demo", "Demo skill.") + "\n{{> }}\n" }, + }, + }, + }, + }); + const root = project.baseDir; + + await expect(build({ cwd: root, target: "claude" })).rejects.toThrow( + /malformed partial reference/, + ); }); it("resolves nested partial composition end to end", async () => { @@ -3033,13 +3118,26 @@ export default defineConfig({ expect(readme).not.toContain("{{>"); }); - it("renders unrelated {{...}}-looking text as empty (documented Mustache behavior, not a bug)", async () => { + it("leaves unrelated {{...}}-looking text exactly as authored", async () => { + const documentation = [ + "Jinja: {{ user.name }}", + "Mustache section:", + "", + "```handlebars", + "{{#each items}}", + " {{this}}", + "{{/each}}", + "```", + "", + "Triple-stash {{{raw}}} and a dangling {{ opener.", + ].join("\n"); const project = await fixtureProject({ "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; export default defineConfig({ - name: "partials-collision-doc", + name: "partials-passthrough", version: "1.0.0", + source: { partials: "partials" }, targets: { claude: { outDir: "dist/claude", @@ -3048,13 +3146,62 @@ export default defineConfig({ } }); `, + partials: { "auth.md": "Authenticate first." }, plugins: { demo: { skills: { demo: { - "SKILL.md": - skill("demo", "Demo skill.") + - "\nExample Handlebars tag: {{unrelated}}\n", + "SKILL.md": `${skill("demo", "Demo skill.")} +${documentation} + +{{> auth}} +`, + }, + }, + }, + }, + }); + const root = project.baseDir; + + await build({ cwd: root, target: "claude" }); + + const content = await readFile( + path.join(root, "dist/claude/plugins/demo/skills/demo/SKILL.md"), + "utf8", + ); + // Every non-partial construct survives verbatim, including ones Mustache + // would otherwise fail to parse, while the real partial still resolves. + expect(content).toContain(documentation); + expect(content).toContain("Authenticate first."); + expect(content).not.toContain("{{> auth}}"); + }); + + it("emits a literal partial tag for an escaped \\{{> name}} reference", async () => { + const project = await fixtureProject({ + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "partials-escape", + version: "1.0.0", + source: { partials: "partials" }, + targets: { + claude: { + outDir: "dist/claude", + plugins: { demo: { from: ["demo"] } } + } + } +}); +`, + partials: { "auth.md": "Authenticate first." }, + plugins: { + demo: { + skills: { + demo: { + "SKILL.md": `${skill("demo", "Demo skill.")} +Reference a partial by writing \\{{> auth}} in a file. + +{{> auth}} +`, }, }, }, @@ -3068,8 +3215,83 @@ export default defineConfig({ path.join(root, "dist/claude/plugins/demo/skills/demo/SKILL.md"), "utf8", ); - expect(content).toContain("Example Handlebars tag: \n"); - expect(content).not.toContain("{{unrelated}}"); + expect(content).toContain("by writing {{> auth}} in a file."); + expect(content).toContain("Authenticate first."); + }); + + it("fails the build when a partial tag survives in a file type substitution skips", async () => { + const project = await fixtureProject({ + "pluginpack.config.ts": `import { defineConfig } from "${path.resolve("src/index.ts")}"; + +export default defineConfig({ + name: "partials-survivor", + version: "1.0.0", + source: { partials: "partials" }, + targets: { + claude: { + outDir: "dist/claude", + plugins: { demo: { from: ["demo"] } } + } + } +}); +`, + partials: { "auth.md": "Authenticate first." }, + plugins: { + demo: { + skills: { + demo: { + "SKILL.md": skill("demo", "Demo skill."), + "config.yaml": "auth: |\n {{> auth}}\n", + }, + }, + }, + }, + }); + const root = project.baseDir; + + await expect(build({ cwd: root, target: "claude" })).rejects.toThrow( + /Unsubstituted partial references in emitted output:\n {2}claude: plugins\/demo\/skills\/demo\/config\.yaml contains \{\{> auth\}\}/, + ); + }); + + it("flags a partial tag surviving in already-generated output during validate", async () => { + const project = await fixture(); + const root = project.baseDir; + await build({ cwd: root, target: "claude" }); + const pluginDir = path.join(root, "dist/claude/plugins/demo"); + await writeFile( + path.join(pluginDir, "skills/demo/data.yaml"), + "auth: {{> auth}}\n", + ); + await writeFile( + path.join(pluginDir, "skills/demo/NOTES.md"), + "Stale: {{> auth}}\n", + ); + + const result = await validateOutput( + "claude", + path.join(root, "dist/claude"), + ); + + expect(result.ok).toBe(false); + expect(result.issues.filter((issue) => issue.level === "error")).toEqual( + expect.arrayContaining([ + { + level: "error", + message: expect.stringContaining( + "plugins/demo/skills/demo/data.yaml contains an unsubstituted partial reference {{> auth}}", + ), + }, + ]), + ); + // A substituted file type can only reach output with a tag via the + // documented escape, so it warns rather than failing. + expect( + result.issues.filter( + (issue) => + issue.level === "warning" && issue.message.includes("NOTES.md"), + ), + ).toHaveLength(1); }); it("rejects a target that references an unknown source plugin in from:", async () => { From ede0db4a060470581d6c34acad87e39622d6f5a5 Mon Sep 17 00:00:00 2001 From: Steve Calvert Date: Wed, 5 Aug 2026 17:03:49 -0700 Subject: [PATCH 3/3] style: use JSDoc for declaration comments in src MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A run of // lines above a top-level function, type, or const was doing the job of a doc comment without being one: not picked up by editors, TSDoc, or generated docs. Converted all 11 remaining sites in src/ to JSDoc, including the one added earlier on this branch. Inline // comments inside function bodies are left alone — they annotate statements, which JSDoc does not cover. Co-Authored-By: Claude Opus 5 (1M context) --- src/build.ts | 40 ++++++++++++++++++++++---------------- src/config.ts | 8 +++++--- src/source.ts | 14 +++++++------ src/targets/antigravity.ts | 8 +++++--- src/update-check.ts | 38 +++++++++++++++++++++++------------- 5 files changed, 65 insertions(+), 43 deletions(-) diff --git a/src/build.ts b/src/build.ts index f03edc8..e3ba7fe 100644 --- a/src/build.ts +++ b/src/build.ts @@ -56,11 +56,13 @@ export async function build(options: BuildOptions = {}): Promise { return artifacts; } -// A `{{> name}}` tag that reaches output is broken content: whatever reads the -// file gets a template marker instead of the text it was meant to inline. -// Substitution resolves (or rejects) every tag in the file types it runs on, so -// what is left is a tag authored somewhere it never ran — a reference file, a -// script, a data file. Fail rather than ship it. +/** + * A `{{> name}}` tag that reaches output is broken content: whatever reads the + * file gets a template marker instead of the text it was meant to inline. + * Substitution resolves (or rejects) every tag in the file types it runs on, so + * what is left is a tag authored somewhere it never ran — a reference file, a + * script, a data file. Fail rather than ship it. + */ function assertNoUnsubstitutedPartialTags(artifacts: Artifact[]): void { const found = artifacts.flatMap((artifact) => findUnsubstitutedPartialTags(artifact.files).map( @@ -76,10 +78,12 @@ function assertNoUnsubstitutedPartialTags(artifacts: Artifact[]): void { } } -// Two targets pointed at overlapping output paths would silently overwrite each -// other (and one target's prune could delete the other's files). Catch it. -// Returns the absolute-path -> owning-target map so -// `assertNoCollisionsWithBuiltTargets` can reuse it. +/** + * Two targets pointed at overlapping output paths would silently overwrite each + * other (and one target's prune could delete the other's files). Catch it. + * Returns the absolute-path -> owning-target map so + * `assertNoCollisionsWithBuiltTargets` can reuse it. + */ function assertNoCrossTargetCollisions( artifacts: Artifact[], ): Map { @@ -104,14 +108,16 @@ function assertNoCrossTargetCollisions( return owner; } -// assertNoCrossTargetCollisions only sees artifacts built in *this* -// invocation. Running `pluginpack build --target X` after an earlier -// `pluginpack build --target Y` wrote overlapping paths would otherwise slip -// through — X's build would silently overwrite Y's files, and a later -// `clean --target Y` would then delete what are now X's live files. Guard -// against that by also checking incoming paths against every other -// *configured* target's on-disk managed manifest, not just artifacts present -// in the current invocation. +/** + * `assertNoCrossTargetCollisions` only sees artifacts built in *this* + * invocation. Running `pluginpack build --target X` after an earlier + * `pluginpack build --target Y` wrote overlapping paths would otherwise slip + * through — X's build would silently overwrite Y's files, and a later + * `clean --target Y` would then delete what are now X's live files. Guard + * against that by also checking incoming paths against every other + * *configured* target's on-disk managed manifest, not just artifacts present + * in the current invocation. + */ async function assertNoCollisionsWithBuiltTargets( project: ResolvedProject, targets: TargetName[], diff --git a/src/config.ts b/src/config.ts index 3f74ebd..15f3d37 100644 --- a/src/config.ts +++ b/src/config.ts @@ -161,9 +161,11 @@ async function discoverSourcePlugins( return plugins; } -// A source plugin dir declares a manifest or has at least one component dir. -// This keeps generated target output (e.g. plugins/cursor/ in a single-repo -// layout) from being misread as source on rebuild. +/** + * A source plugin dir declares a manifest or has at least one component dir. + * This keeps generated target output (e.g. plugins/cursor/ in a single-repo + * layout) from being misread as source on rebuild. + */ async function isSourcePluginDir(dir: string): Promise { if (await exists(path.join(dir, "plugin.pluginpack.json"))) { return true; diff --git a/src/source.ts b/src/source.ts index fbbedf7..861298c 100644 --- a/src/source.ts +++ b/src/source.ts @@ -132,12 +132,14 @@ async function resolveTargetOverride( return file; } -// A source plugin declares MCP servers via a .mcp.json file (standard -// { mcpServers: {...} } shape) or an mcpServers key in plugin.pluginpack.json. -// The file takes precedence when both are present. The file form supports -// per-target overrides: targets//.mcp.json wins for that host. The -// manifest form has no per-file override; authors who need per-target MCP -// config should use the .mcp.json file form. +/** + * A source plugin declares MCP servers via a .mcp.json file (standard + * { mcpServers: {...} } shape) or an mcpServers key in plugin.pluginpack.json. + * The file takes precedence when both are present. The file form supports + * per-target overrides: targets//.mcp.json wins for that host. The + * manifest form has no per-file override; authors who need per-target MCP + * config should use the .mcp.json file form. + */ async function readMcpServers( plugin: SourcePlugin, target: TargetName, diff --git a/src/targets/antigravity.ts b/src/targets/antigravity.ts index 8d7e2ee..d64e379 100644 --- a/src/targets/antigravity.ts +++ b/src/targets/antigravity.ts @@ -13,9 +13,11 @@ import type { PluginTargetDefinition } from "./types.js"; const antigravityNamePattern = /^[a-zA-Z0-9-_]+$/; -// plugin.json's schema is { name (required), description (optional) }, -// additionalProperties: false — a strict validator rejects the whole -// manifest for an extra key, not just the key itself. See citations below. +/** + * plugin.json's schema is { name (required), description (optional) }, + * additionalProperties: false — a strict validator rejects the whole + * manifest for an extra key, not just the key itself. See citations below. + */ const ALLOWED_MANIFEST_KEYS = new Set(["name", "description"]); /** Antigravity plugin target — see `citations` for source facts. */ diff --git a/src/update-check.ts b/src/update-check.ts index 50e98ca..bc39b64 100644 --- a/src/update-check.ts +++ b/src/update-check.ts @@ -6,8 +6,10 @@ export const UPDATE_CHECK_SCRIPT_PATH = "scripts/pluginpack-update-check.sh"; /** Output-relative path of a plugin's hooks registration file. */ export const HOOKS_FILE_PATH = "hooks/hooks.json"; -// The marker every generated command contains; merge is idempotent on it, so a -// re-run (or a source plugin that already wired the script) never appends twice. +/** + * The marker every generated command contains; merge is idempotent on it, so a + * re-run (or a source plugin that already wired the script) never appends twice. + */ const SCRIPT_MARKER = "pluginpack-update-check"; export type UpdateCheckFormat = "claude" | "cursor"; @@ -33,19 +35,23 @@ export function pluginAllowsUpdateCheck( return pluginConfig.updateCheck !== false; } -// POSIX single-quote: close, escaped quote, reopen. +/** POSIX single-quote: close, escaped quote, reopen. */ function shellQuote(value: string): string { return `'${value.replaceAll("'", "'\\''")}'`; } -// Escape a value for interpolation into a printf format string ("%" is the -// only metacharacter) that is then JSON-string-escaped as a whole. +/** + * Escape a value for interpolation into a printf format string ("%" is the + * only metacharacter) that is then JSON-string-escaped as a whole. + */ function printfEscape(value: string): string { return value.replaceAll("%", "%%"); } -// Everything that varies by target format, in one place — adding a third -// format means adding one entry here, not hunting down every `=== "claude"`. +/** + * Everything that varies by target format, in one place — adding a third + * format means adding one entry here, not hunting down every `=== "claude"`. + */ type FormatConfig = { mergeHooks: (existing: string | undefined) => string; // Shell expression supplying printf's args, in the order this format's @@ -74,10 +80,12 @@ const FORMATS: Record = { }, }; -// The nudge is a printf format string: JSON payload with two %s slots, plus a -// trailing newline. printf interprets backslash escapes in its format string, -// so JSON's own escapes (e.g. \") must be doubled to survive, while the -// appended \n is left single so printf renders the newline. +/** + * The nudge is a printf format string: JSON payload with two %s slots, plus a + * trailing newline. printf interprets backslash escapes in its format string, + * so JSON's own escapes (e.g. \") must be doubled to survive, while the + * appended \n is left single so printf renders the newline. + */ function nudgeFormat(format: UpdateCheckFormat, pluginName: string): string { const payload = FORMATS[format].payload(printfEscape(pluginName)); const literal = JSON.stringify(payload).replaceAll("\\", "\\\\"); @@ -144,9 +152,11 @@ function isPlainObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -// Parse an existing hooks.json and hand back the object plus the event array to -// append to, enforcing the shapes both hosts expect. Throws bare reasons; the -// caller wraps them with target/plugin context. +/** + * Parse an existing hooks.json and hand back the object plus the event array to + * append to, enforcing the shapes both hosts expect. Throws bare reasons; the + * caller wraps them with target/plugin context. + */ function parseHooksFile( existing: string, eventName: string,