From 543e51e71f5e5a79d58e6e4522a957445323e7b1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 7 Jun 2026 21:43:30 -0400 Subject: [PATCH] fix(#844): sync runtime manifest versions on npm version bump (#845) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#844): sync runtime manifest versions on npm version bump The release workflow bumps package.json via `npm version` but never stamped the runtime-integration manifests that must track it (.claude-plugin/plugin.json #766, gemini-extension.json #775), so the first RC/finalize whose version diverged from the -dev stream failed the test suite before tagging/publishing. Add scripts/sync-manifest-versions.cjs (single VERSIONED_MANIFESTS registry) wired to a `version` npm lifecycle hook that stamps + stages the manifests on every `npm version` — covering all four release bump sites and local bumps with no workflow edits. A regression guard test fails if any repo JSON whose version matches package.json is not registered, forcing future version-bearing manifests into the sync. Co-Authored-By: Claude Opus 4.8 * docs(#844): add changeset for manifest version sync fix Co-Authored-By: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- .changeset/curious-seals-howl.md | 5 + VERSIONING.md | 18 ++ package.json | 1 + scripts/sync-manifest-versions.cjs | 119 +++++++++ .../issue-844-manifest-version-sync.test.cjs | 236 ++++++++++++++++++ 5 files changed, 379 insertions(+) create mode 100644 .changeset/curious-seals-howl.md create mode 100644 scripts/sync-manifest-versions.cjs create mode 100644 tests/issue-844-manifest-version-sync.test.cjs diff --git a/.changeset/curious-seals-howl.md b/.changeset/curious-seals-howl.md new file mode 100644 index 000000000..aee2587f9 --- /dev/null +++ b/.changeset/curious-seals-howl.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 845 +--- +**Release version bumps now keep runtime manifest versions in sync** — `.claude-plugin/plugin.json` and `gemini-extension.json` are stamped to match `package.json` on every `npm version`, unblocking RC/finalize releases. New version-bearing manifests must be registered in `scripts/sync-manifest-versions.cjs` (enforced by a regression test). diff --git a/VERSIONING.md b/VERSIONING.md index c568db28a..7c3a53493 100644 --- a/VERSIONING.md +++ b/VERSIONING.md @@ -124,6 +124,24 @@ Branch names map to commit types: | `docs/` | `docs:` | none | | `refactor/` | `refactor:` | none | +## Manifest Version Sync + +Certain runtime-integration manifests carry a `version` field that must always +match `package.json`: + +- `.claude-plugin/plugin.json` — Claude Code plugin manifest (issue #766) +- `gemini-extension.json` — Gemini CLI extension manifest (issue #775) + +The `version` npm lifecycle script (`scripts/sync-manifest-versions.cjs --stage`) +stamps these files automatically on every `npm version` call, and stages them so +they are included in the release commit alongside `package.json`. + +To add a new manifest that must track the package version, register its path in +the `VERSIONED_MANIFESTS` array in `scripts/sync-manifest-versions.cjs`. A +regression test (`tests/issue-844-manifest-version-sync.test.cjs`) enforces this: +it scans all committed JSON files for a matching `version` field and fails if any +are missing from the registry. + ## Publishing Commands (Reference) ```bash diff --git a/package.json b/package.json index 8eac5d16e..64e82d54d 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "generate:identity": "node scripts/generate-package-identity.cjs", "prepack": "npm run build:lib", "prepare": "npm run build:lib", + "version": "node scripts/sync-manifest-versions.cjs --stage", "prepublishOnly": "npm run build:lib && npm run build:hooks", "pretest": "npm run build:lib && npm run lint:skill-deps", "pretest:coverage": "npm run build:lib && npm run lint:skill-deps", diff --git a/scripts/sync-manifest-versions.cjs b/scripts/sync-manifest-versions.cjs new file mode 100644 index 000000000..f110b492e --- /dev/null +++ b/scripts/sync-manifest-versions.cjs @@ -0,0 +1,119 @@ +#!/usr/bin/env node +'use strict'; + +/** + * sync-manifest-versions.cjs + * + * Stamps the package.json version into every runtime-integration manifest whose + * top-level `version` field MUST track the package version. Called automatically + * by the `version` npm lifecycle script so that `npm version X.Y.Z` keeps all + * registered manifests in sync. + * + * Usage: + * node scripts/sync-manifest-versions.cjs # stamp + report + * node scripts/sync-manifest-versions.cjs --stage # stamp + git-stage manifests + * node scripts/sync-manifest-versions.cjs --check # report drift, exit 1 if any + */ + +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const ROOT = path.resolve(__dirname, '..'); + +// Single source of truth: runtime-integration manifests whose top-level `version` +// MUST track package.json. Add a new manifest here so `npm version` keeps it in +// sync — the regression guard test (issue 844) fails if you forget. +const VERSIONED_MANIFESTS = [ + '.claude-plugin/plugin.json', + 'gemini-extension.json', +]; + +function readJson(p) { + return JSON.parse(fs.readFileSync(p, 'utf8')); +} + +function getPackageVersion(root) { + const r = root || ROOT; + return readJson(path.join(r, 'package.json')).version; +} + +// Stamp `version` into each registered manifest, preserving field order and +// 2-space + trailing-newline formatting. Returns the list of changed rel paths. +function syncManifestVersions(opts) { + const root = (opts && opts.root) || ROOT; + const v = (opts && opts.version) != null ? opts.version : getPackageVersion(root); + const changed = []; + for (const rel of VERSIONED_MANIFESTS) { + const abs = path.join(root, rel); + const manifest = readJson(abs); + if (manifest.version !== v) { + manifest.version = v; + fs.writeFileSync(abs, JSON.stringify(manifest, null, 2) + '\n'); + changed.push(rel); + } + } + return changed; +} + +// Registered manifests whose version != package version. +function findDrift(opts) { + const root = (opts && opts.root) || ROOT; + const v = (opts && opts.version) != null ? opts.version : getPackageVersion(root); + const drift = []; + for (const rel of VERSIONED_MANIFESTS) { + const found = readJson(path.join(root, rel)).version; + if (found !== v) drift.push({ manifest: rel, found, expected: v }); + } + return drift; +} + +// Best-effort outside git; fail-closed inside a work tree so a release never +// ships a stale manifest that the working-tree test already accepted. +function stageManifests(opts) { + const root = (opts && opts.root) || ROOT; + let insideWorkTree = false; + try { + insideWorkTree = execFileSync('git', ['rev-parse', '--is-inside-work-tree'], { + cwd: root, + stdio: ['ignore', 'pipe', 'ignore'], + }).toString().trim() === 'true'; + } catch {} + if (!insideWorkTree) { + console.warn('sync-manifest-versions: not a git work tree; skipping staging.'); + return; + } + try { + execFileSync('git', ['add', '--', ...VERSIONED_MANIFESTS], { cwd: root, stdio: ['ignore', 'ignore', 'pipe'] }); + } catch (err) { + const detail = err && err.stderr ? err.stderr.toString().trim() : (err && err.message) || 'unknown error'; + throw new Error(`sync-manifest-versions: failed to git-add manifests inside a work tree: ${detail}`); + } +} + +module.exports = { VERSIONED_MANIFESTS, syncManifestVersions, findDrift, getPackageVersion, stageManifests }; + +if (require.main === module) { + const args = process.argv.slice(2); + const version = getPackageVersion(); + if (args.includes('--check')) { + const drift = findDrift({ version }); + if (drift.length) { + for (const d of drift) { + console.error('Manifest ' + d.manifest + ' version ' + d.found + ' != package.json ' + d.expected); + } + console.error('Run `node scripts/sync-manifest-versions.cjs` to fix.'); + process.exitCode = 1; + } else { + console.log('All ' + VERSIONED_MANIFESTS.length + ' versioned manifests in sync at ' + version + '.'); + } + } else { + const changed = syncManifestVersions({ version }); + if (changed.length) { + console.log('Stamped ' + version + ' into: ' + changed.join(', ')); + } else { + console.log('Versioned manifests already at ' + version + '.'); + } + if (args.includes('--stage')) stageManifests(); + } +} diff --git a/tests/issue-844-manifest-version-sync.test.cjs b/tests/issue-844-manifest-version-sync.test.cjs new file mode 100644 index 000000000..109e8756b --- /dev/null +++ b/tests/issue-844-manifest-version-sync.test.cjs @@ -0,0 +1,236 @@ +'use strict'; + +/** + * Regression tests for issue #844: manifest version sync. + * + * Verifies that `scripts/sync-manifest-versions.cjs` correctly stamps the + * package.json version into every registered runtime-integration manifest, + * and that all currently-tracked manifests are in sync. + * + * Key deliverable: the regression guard (test d) asserts that any committed + * JSON file with a top-level `version` field matching package.json is + * registered in VERSIONED_MANIFESTS — forcing explicit opt-in for future + * manifests. + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +const ROOT = path.resolve(__dirname, '..'); +const helpers = require(path.join(__dirname, 'helpers.cjs')); +const { + VERSIONED_MANIFESTS, + syncManifestVersions, + getPackageVersion, + stageManifests, +} = require(path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs')); + +// ─── A: RED→GREEN repro via temp fixture ───────────────────────────────────── +describe('A: syncManifestVersions — temp fixture', () => { + + let tmpRoot; + + test('setup: create temp fixture with stale manifests', () => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-')); + + // Write tmp package.json + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ name: 'x', version: '9.9.9-test.0' }, null, 2) + '\n' + ); + + // Copy real manifests into tmp, stamped at OLD version + for (const rel of VERSIONED_MANIFESTS) { + const realAbs = path.join(ROOT, rel); + const manifest = JSON.parse(fs.readFileSync(realAbs, 'utf8')); + manifest.version = '0.0.0'; + + const destAbs = path.join(tmpRoot, rel); + const destDir = path.dirname(destAbs); + if (!fs.existsSync(destDir)) fs.mkdirSync(destDir, { recursive: true }); + fs.writeFileSync(destAbs, JSON.stringify(manifest, null, 2) + '\n'); + } + }); + + test('pre-sync: at least one manifest has stale version', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const pkgVersion = getPackageVersion(tmpRoot); + let anyStale = false; + for (const rel of VERSIONED_MANIFESTS) { + const m = JSON.parse(fs.readFileSync(path.join(tmpRoot, rel), 'utf8')); + if (m.version !== pkgVersion) { anyStale = true; break; } + } + assert.ok(anyStale, 'At least one manifest should be stale before sync (version 0.0.0 != 9.9.9-test.0)'); + }); + + test('syncManifestVersions stamps all manifests to package.json version', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const changed = syncManifestVersions({ root: tmpRoot }); + assert.ok(changed.length > 0, 'syncManifestVersions should report at least one changed file'); + + const pkgVersion = getPackageVersion(tmpRoot); + assert.equal(pkgVersion, '9.9.9-test.0'); + + for (const rel of VERSIONED_MANIFESTS) { + const abs = path.join(tmpRoot, rel); + const m = JSON.parse(fs.readFileSync(abs, 'utf8')); + assert.equal( + m.version, + '9.9.9-test.0', + `${rel} version should be 9.9.9-test.0 after sync` + ); + } + }); + + test('non-version fields are preserved after sync', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + // Read from the real manifests to know what non-version fields should exist + for (const rel of VERSIONED_MANIFESTS) { + const real = JSON.parse(fs.readFileSync(path.join(ROOT, rel), 'utf8')); + const tmp = JSON.parse(fs.readFileSync(path.join(tmpRoot, rel), 'utf8')); + // Check that every non-version key from the real manifest exists in tmp + for (const key of Object.keys(real)) { + if (key === 'version') continue; + assert.ok( + Object.prototype.hasOwnProperty.call(tmp, key), + `${rel}: field "${key}" should be preserved after sync` + ); + } + } + }); + + test('each synced file ends with a single trailing newline', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + for (const rel of VERSIONED_MANIFESTS) { + const raw = fs.readFileSync(path.join(tmpRoot, rel), 'utf8'); + assert.ok(raw.endsWith('\n'), `${rel} must end with a trailing newline`); + assert.ok(!raw.endsWith('\n\n'), `${rel} must not end with a double newline`); + } + }); + + test('second syncManifestVersions call is idempotent (returns [])', () => { + assert.ok(tmpRoot, 'tmpRoot must be set by setup test'); + const changed = syncManifestVersions({ root: tmpRoot }); + assert.deepEqual(changed, [], 'Second sync call should return [] (already in sync)'); + }); + + test('cleanup: remove temp fixture', () => { + if (tmpRoot) { + helpers.cleanup(tmpRoot); + tmpRoot = null; + } + }); +}); + +// ─── B: Registry-in-sync: real manifests match package.json ────────────────── +describe('B: real manifests match package.json version', () => { + + const pkgVersion = getPackageVersion(ROOT); + + for (const rel of VERSIONED_MANIFESTS) { + test(`${rel} version === ${pkgVersion}`, () => { + const abs = path.join(ROOT, rel); + assert.ok(fs.existsSync(abs), `${rel} must exist at ${abs}`); + const m = JSON.parse(fs.readFileSync(abs, 'utf8')); + assert.equal( + m.version, + pkgVersion, + `${rel} version (${m.version}) must match package.json version (${pkgVersion}). ` + + 'Run `node scripts/sync-manifest-versions.cjs` to fix.' + ); + }); + } +}); + +// ─── C: Regression guard — all version-bearing JSON files are registered ────── +describe('C: regression guard — version-bearing JSON files must be registered', () => { + + // package.json is the version source; package-lock.json is npm-managed. + // Both inherently track the version without the sync script. + const ALLOWED = new Set([...VERSIONED_MANIFESTS, 'package.json', 'package-lock.json']); + + // Semver-ish: matches X.Y.Z with optional pre-release/build metadata. + const SEMVER = /^\d+\.\d+\.\d+(?:[-+].+)?$/; + + // Paths to exclude from the guard + const EXCLUDED_PREFIXES = ['tests/', 'node_modules/', '.changeset/', 'docs/']; + + test('every committed JSON with a semver top-level version is registered or explicitly allowed', (t) => { + // Enumerate committed JSON files via git (no pathspec to avoid recursion quirks; + // filter to .json in JS instead). + let lines; + try { + const out = execFileSync('git', ['ls-files'], { cwd: ROOT }); + lines = out.toString().split('\n').filter((f) => f.endsWith('.json')); + } catch (err) { + t.skip('git unavailable: ' + err.message); + return; + } + + for (const rel of lines) { + if (ALLOWED.has(rel)) continue; + if (EXCLUDED_PREFIXES.some(prefix => rel.startsWith(prefix))) continue; + + const abs = path.join(ROOT, rel); + let parsed; + try { + parsed = JSON.parse(fs.readFileSync(abs, 'utf8')); + } catch (_) { + continue; // skip invalid JSON (shouldn't exist, but be safe) + } + + if ( + parsed && + typeof parsed === 'object' && + !Array.isArray(parsed) && + typeof parsed.version === 'string' && + SEMVER.test(parsed.version) + ) { + assert.ok( + ALLOWED.has(rel), + `${rel} has a semver top-level "version" but is not registered in ` + + 'scripts/sync-manifest-versions.cjs VERSIONED_MANIFESTS (nor an npm-managed file). ' + + "Register it so 'npm version' keeps it in sync (issue #844)." + ); + } + } + }); +}); + +// ─── E: stageManifests in a non-git dir must not throw ─────────────────────── +describe('E: stageManifests — non-git dir is a no-op, not a throw', () => { + + test('stageManifests({root}) with a non-git tempdir warns and returns without throwing', () => { + const tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-844-nogit-')); + try { + // Write a minimal package.json so getPackageVersion doesn't error if called + fs.writeFileSync( + path.join(tmpRoot, 'package.json'), + JSON.stringify({ name: 'x', version: '0.0.0' }, null, 2) + '\n' + ); + // Must not throw even though tmpRoot is not a git repo + assert.doesNotThrow(() => { + stageManifests({ root: tmpRoot }); + }, 'stageManifests must not throw outside a git work tree'); + } finally { + helpers.cleanup(tmpRoot); + } + }); +}); + +// ─── D: CLI --check exits 0 when in sync ───────────────────────────────────── +describe('D: CLI --check exits 0 when manifests are in sync', () => { + + test('node scripts/sync-manifest-versions.cjs --check exits 0', () => { + // Will throw if exit code != 0 + execFileSync( + process.execPath, + [path.join(ROOT, 'scripts', 'sync-manifest-versions.cjs'), '--check'], + { cwd: ROOT } + ); + }); +});