diff --git a/CLAUDE.md b/CLAUDE.md index 59368668e..c52972136 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -222,9 +222,10 @@ There are three independently versioned components. Only bump the one(s) that ac - Bump when: CLI code changes (`cli/bin/`, `cli/engine/detect-antipatterns.mjs`, etc.) **Skills** (Claude Code plugin / skill definitions): -- `.claude-plugin/plugin.json` → `version` +- `.claude-plugin/plugin.json` → `version` (source of truth) - `.claude-plugin/marketplace.json` → `plugins[0].version` - Bump when: skill content changes (`skill/`, reference files, command metadata, etc.) +- After bumping, run `bun run build:release` so the committed `./plugin` subtree (`plugin/.claude-plugin/plugin.json` + `plugin/skills/impeccable/SKILL.md`) is regenerated to the new version. The build validator (`validatePluginVersions` in `scripts/build.js`) fails if `marketplace.json`, the `./plugin` manifest, or the bundled `SKILL.md` frontmatter disagree with `plugin.json` — this guards the marketplace install path against version drift (issue #274). **Chrome extension**: - `extension/manifest.json` → `version` diff --git a/scripts/build.js b/scripts/build.js index e27a00c53..a405cc939 100644 --- a/scripts/build.js +++ b/scripts/build.js @@ -23,6 +23,7 @@ import { generateApiData } from './lib/api-data.js'; import { createTransformer, PROVIDERS } from './lib/transformers/index.js'; import { hooksJsonFor, buildClaudePluginHooksManifest } from './lib/transformers/hooks.js'; import { createAllZips } from './lib/zip.js'; +import { collectPluginVersions } from './lib/validate-plugin-versions.js'; import { ANTIPATTERNS } from '../cli/engine/registry/antipatterns.mjs'; // Sub-page generation is now handled by Astro content collections. @@ -112,6 +113,37 @@ function generateCounts(rootDir, skills, buildDir) { return errors; } +/** + * Guard against plugin/skill version drift (issue #274). The pure comparison + * lives in ./lib/validate-plugin-versions.js (so it's unit-tested directly); + * this wrapper owns the console output and the error count the build gates on. + */ +function validatePluginVersions(rootDir) { + const { source, mismatches, errors } = collectPluginVersions(rootDir); + // No root manifest at all → nothing to check (source null with no errors). + if (source == null && errors.length === 0) return 0; + + for (const { relPath, reason } of errors) { + console.error(` ❌ ${relPath}: ${reason}`); + } + for (const { relPath, found, expected } of mismatches) { + console.error( + ` ❌ ${relPath}: version "${found}" disagrees with .claude-plugin/plugin.json "${expected}"`, + ); + } + + const total = errors.length + mismatches.length; + if (total > 0) { + console.error( + `\n❌ ${total} plugin/skill version problem(s). Bump every version together and run ` + + `\`bun run build:release\` to regenerate the ./plugin subtree (issue #274).`, + ); + } else { + console.log(`✓ Plugin/skill versions agree: ${source}`); + } + return total; +} + function validateSkillFrontmatter(skills) { let errors = 0; @@ -766,6 +798,10 @@ async function build() { // Generate authoritative counts and validate references const countErrors = generateCounts(ROOT_DIR, skills, buildDir); + // Guard plugin/skill version drift: marketplace + ./plugin subtree must + // match root plugin.json so marketplace installs never ship a stale version. + const versionErrors = validatePluginVersions(ROOT_DIR); + // Verify every hand-authored HTML page carries the shared site header const headerErrors = validateSiteHeader(ROOT_DIR); @@ -779,7 +815,7 @@ async function build() { // that has no technical reading. Hardening repetition is intentionally allowed. const skillProseErrors = validateSkillProse(ROOT_DIR); - if (countErrors > 0 || headerErrors > 0 || themeErrors > 0 || proseErrors > 0 || skillProseErrors > 0) { + if (countErrors > 0 || versionErrors > 0 || headerErrors > 0 || themeErrors > 0 || proseErrors > 0 || skillProseErrors > 0) { process.exit(1); } diff --git a/scripts/lib/validate-plugin-versions.js b/scripts/lib/validate-plugin-versions.js new file mode 100644 index 000000000..0681d1d45 --- /dev/null +++ b/scripts/lib/validate-plugin-versions.js @@ -0,0 +1,122 @@ +/** + * Plugin/skill version-drift detection (issue #274). + * + * The Claude Code marketplace installs from the committed `./plugin` subtree, + * so any version disagreement between the hand-edited manifests and the + * generated subtree ships stale content reporting a wrong version. Root + * `.claude-plugin/plugin.json` is the single source of truth (build() reads + * skillsVersion from it). Every other version-bearing file must match it: + * + * - `.claude-plugin/marketplace.json` plugins[0].version — hand-edited + * alongside plugin.json; the post-merge sync workflow can't repair a + * mismatch here because it never bumps versions. + * - `plugin/.claude-plugin/plugin.json` version — generated, derived from + * root at build:release; checked so a bump that forgets to regenerate the + * subtree fails loudly instead of merging a drift window onto main. + * - `plugin/skills/impeccable/SKILL.md` frontmatter version — generated; + * same rationale. + * + * The collector is pure (filesystem-in, data-out) so it can be unit-tested + * against fixtures; build.js owns the logging and the non-zero exit. + */ +import fs from 'fs'; +import path from 'path'; + +/** + * Pull the `version:` value out of a SKILL.md leading frontmatter block. + * CRLF-tolerant (`\r?\n`) to match the shared parseFrontmatter in + * scripts/lib/utils.js — a bundle saved with CRLF line endings must not read + * as a null version and trip a false mismatch. `(.+)` stops at the line + * terminator (so a trailing `\r` is excluded), and `.trim()` mops up the rest. + */ +export function readSkillFrontmatterVersion(content) { + const fm = content.match(/^---\r?\n([\s\S]*?)\r?\n---/); + if (!fm) return null; + const line = fm[1].match(/^version:\s*(.+)/m); + return line ? line[1].trim().replace(/^['"]|['"]$/g, '') : null; +} + +/** + * Read a file and extract a value, turning read/parse failures into a clean + * sentinel instead of a raw throw. A version bump is exactly the moment a + * manifest is half-edited, so a malformed file must produce an actionable + * diagnostic naming the file, not a stack trace out of build(). + * + * @returns {{ value: any } | { error: string }} + */ +function extractFromFile(absPath, extract) { + let raw; + try { + raw = fs.readFileSync(absPath, 'utf-8'); + } catch (err) { + return { error: `could not read file (${err.code || err.message})` }; + } + try { + return { value: extract(raw) }; + } catch (err) { + return { error: `could not parse (${err.message})` }; + } +} + +/** + * Compare every version-bearing plugin/skill file against root plugin.json. + * + * @param {string} rootDir repository root + * @returns {{ + * source: string|null, + * checked: Array<{relPath:string, found:any}>, + * mismatches: Array<{relPath:string, found:any, expected:string}>, + * errors: Array<{relPath:string, reason:string}>, + * }} + * `source` is null only when root plugin.json is absent (nothing to check). + * A present-but-malformed root, or one missing its `version` field, instead + * reports an entry in `errors` so the build fails loudly rather than passing. + */ +export function collectPluginVersions(rootDir) { + const rootRel = '.claude-plugin/plugin.json'; + const rootManifestPath = path.join(rootDir, rootRel); + const empty = { source: null, checked: [], mismatches: [], errors: [] }; + if (!fs.existsSync(rootManifestPath)) return empty; + + const rootResult = extractFromFile(rootManifestPath, (raw) => JSON.parse(raw).version); + if (rootResult.error) { + return { ...empty, errors: [{ relPath: rootRel, reason: rootResult.error }] }; + } + const source = rootResult.value; + if (source == null) { + return { ...empty, errors: [{ relPath: rootRel, reason: 'missing "version" field' }] }; + } + + const checks = [ + { + relPath: '.claude-plugin/marketplace.json', + read: (raw) => JSON.parse(raw).plugins?.[0]?.version, + }, + { + relPath: 'plugin/.claude-plugin/plugin.json', + read: (raw) => JSON.parse(raw).version, + }, + { + relPath: 'plugin/skills/impeccable/SKILL.md', + read: (raw) => readSkillFrontmatterVersion(raw), + }, + ]; + + const checked = []; + const mismatches = []; + const errors = []; + for (const { relPath, read } of checks) { + const absPath = path.join(rootDir, relPath); + if (!fs.existsSync(absPath)) continue; + const result = extractFromFile(absPath, read); + if (result.error) { + errors.push({ relPath, reason: result.error }); + continue; + } + const found = result.value; + checked.push({ relPath, found }); + if (found !== source) mismatches.push({ relPath, found, expected: source }); + } + + return { source, checked, mismatches, errors }; +} diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index c911626f4..afe89cdb1 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -47,6 +47,7 @@ export const SUITES = { 'tests/lib/transformers/providers.test.js', 'tests/docs-integrity.test.js', 'tests/skills-cli.test.js', + 'tests/validate-plugin-versions.test.js', ], }, { diff --git a/tests/validate-plugin-versions.test.js b/tests/validate-plugin-versions.test.js new file mode 100644 index 000000000..c7d2dbf2c --- /dev/null +++ b/tests/validate-plugin-versions.test.js @@ -0,0 +1,177 @@ +/** + * Unit coverage for the plugin/skill version-drift guard (issue #274). + * + * The Claude Code marketplace installs from the committed `./plugin` subtree, + * so a version disagreement between the hand-edited manifests and the + * generated subtree ships stale content under a wrong version number. The + * guard treats root `.claude-plugin/plugin.json` as the source of truth and + * flags any other version-bearing file that disagrees. + */ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { + collectPluginVersions, + readSkillFrontmatterVersion, +} from '../scripts/lib/validate-plugin-versions.js'; + +function skillMd(version) { + return `---\nname: impeccable\nversion: ${version}\nuser-invocable: true\n---\n\nBody.\n`; +} + +function writeFixture(root, { plugin, marketplace, subtreePlugin, skill } = {}) { + const write = (rel, contents) => { + const abs = path.join(root, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, contents); + }; + if (plugin !== undefined) { + write('.claude-plugin/plugin.json', JSON.stringify({ name: 'impeccable', version: plugin }, null, 2)); + } + if (marketplace !== undefined) { + write('.claude-plugin/marketplace.json', JSON.stringify({ plugins: [{ name: 'impeccable', version: marketplace }] }, null, 2)); + } + if (subtreePlugin !== undefined) { + write('plugin/.claude-plugin/plugin.json', JSON.stringify({ name: 'impeccable', version: subtreePlugin, skills: './skills/' }, null, 2)); + } + if (skill !== undefined) { + write('plugin/skills/impeccable/SKILL.md', skillMd(skill)); + } +} + +describe('collectPluginVersions', () => { + let root; + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-ver-')); + }); + afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + test('no mismatches when every version agrees', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.7.1', subtreePlugin: '3.7.1', skill: '3.7.1' }); + const { source, mismatches } = collectPluginVersions(root); + expect(source).toBe('3.7.1'); + expect(mismatches).toEqual([]); + }); + + test('flags a lagging marketplace.json (the half the sync workflow cannot repair)', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.1.1', subtreePlugin: '3.7.1', skill: '3.7.1' }); + const { mismatches } = collectPluginVersions(root); + expect(mismatches).toEqual([ + { relPath: '.claude-plugin/marketplace.json', found: '3.1.1', expected: '3.7.1' }, + ]); + }); + + test('flags a stale ./plugin subtree manifest', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.7.1', subtreePlugin: '3.6.0', skill: '3.7.1' }); + const { mismatches } = collectPluginVersions(root); + expect(mismatches).toEqual([ + { relPath: 'plugin/.claude-plugin/plugin.json', found: '3.6.0', expected: '3.7.1' }, + ]); + }); + + test('flags a stale bundled SKILL.md frontmatter version', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.7.1', subtreePlugin: '3.7.1', skill: '3.1.1' }); + const { mismatches } = collectPluginVersions(root); + expect(mismatches).toEqual([ + { relPath: 'plugin/skills/impeccable/SKILL.md', found: '3.1.1', expected: '3.7.1' }, + ]); + }); + + test('reports every drifted file at once', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.1.1', subtreePlugin: '3.5.0', skill: '3.1.1' }); + const { mismatches } = collectPluginVersions(root); + expect(mismatches.map((m) => m.relPath)).toEqual([ + '.claude-plugin/marketplace.json', + 'plugin/.claude-plugin/plugin.json', + 'plugin/skills/impeccable/SKILL.md', + ]); + }); + + test('skips files that do not exist instead of throwing', () => { + writeFixture(root, { plugin: '3.7.1' }); // only root manifest present + const { source, checked, mismatches, errors } = collectPluginVersions(root); + expect(source).toBe('3.7.1'); + expect(checked).toEqual([]); + expect(mismatches).toEqual([]); + expect(errors).toEqual([]); + }); + + test('returns a null source with no errors when root plugin.json is absent', () => { + const { source, mismatches, errors } = collectPluginVersions(root); + expect(source).toBeNull(); + expect(mismatches).toEqual([]); + expect(errors).toEqual([]); + }); + + test('reports a malformed checked manifest as an error instead of throwing', () => { + writeFixture(root, { plugin: '3.7.1', subtreePlugin: '3.7.1', skill: '3.7.1' }); + // marketplace.json half-edited mid-bump: invalid JSON. + fs.writeFileSync(path.join(root, '.claude-plugin/marketplace.json'), '{ "plugins": [ { "version": '); + const { mismatches, errors } = collectPluginVersions(root); + expect(mismatches).toEqual([]); + expect(errors).toHaveLength(1); + expect(errors[0].relPath).toBe('.claude-plugin/marketplace.json'); + expect(errors[0].reason).toMatch(/parse/i); + }); + + test('reports a malformed root plugin.json as an error, not a thrown stack', () => { + fs.mkdirSync(path.join(root, '.claude-plugin'), { recursive: true }); + fs.writeFileSync(path.join(root, '.claude-plugin/plugin.json'), '{ not json'); + const { source, errors } = collectPluginVersions(root); + expect(source).toBeNull(); + expect(errors).toHaveLength(1); + expect(errors[0].relPath).toBe('.claude-plugin/plugin.json'); + expect(errors[0].reason).toMatch(/parse/i); + }); + + test('flags a root plugin.json that exists but has no version field', () => { + fs.mkdirSync(path.join(root, '.claude-plugin'), { recursive: true }); + fs.writeFileSync(path.join(root, '.claude-plugin/plugin.json'), JSON.stringify({ name: 'impeccable' })); + const { source, errors } = collectPluginVersions(root); + // source stays null, but it is reported as an error rather than silently passing. + expect(source).toBeNull(); + expect(errors).toEqual([{ relPath: '.claude-plugin/plugin.json', reason: 'missing "version" field' }]); + }); +}); + +describe('readSkillFrontmatterVersion', () => { + test('reads an unquoted version', () => { + expect(readSkillFrontmatterVersion(skillMd('3.7.1'))).toBe('3.7.1'); + }); + + test('strips surrounding quotes', () => { + expect(readSkillFrontmatterVersion('---\nversion: "3.7.1"\n---\n')).toBe('3.7.1'); + }); + + test('returns null when there is no frontmatter block', () => { + expect(readSkillFrontmatterVersion('no frontmatter here')).toBeNull(); + }); + + test('reads a version from CRLF-encoded frontmatter', () => { + const crlf = '---\r\nname: impeccable\r\nversion: 3.7.1\r\nuser-invocable: true\r\n---\r\n\r\nBody.\r\n'; + expect(readSkillFrontmatterVersion(crlf)).toBe('3.7.1'); + }); +}); + +describe('collectPluginVersions with CRLF line endings', () => { + let root; + beforeEach(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-ver-crlf-')); + }); + afterEach(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + test('a CRLF-saved SKILL.md at the right version is not a false mismatch', () => { + writeFixture(root, { plugin: '3.7.1', marketplace: '3.7.1', subtreePlugin: '3.7.1', skill: '3.7.1' }); + // Re-save the bundled SKILL.md with CRLF line endings. + const skillPath = path.join(root, 'plugin/skills/impeccable/SKILL.md'); + fs.writeFileSync(skillPath, fs.readFileSync(skillPath, 'utf-8').replace(/\n/g, '\r\n')); + const { mismatches, errors } = collectPluginVersions(root); + expect(mismatches).toEqual([]); + expect(errors).toEqual([]); + }); +});