From 904f7b4303cff138821c26ac6eb2db3baf155715 Mon Sep 17 00:00:00 2001 From: claude_code Date: Sat, 27 Jun 2026 05:18:39 +0300 Subject: [PATCH] fix(agent-roles): bump proofreader v3 + guard against content edits without a version bump The proofreader role content was changed (STYLE SHEET block removed) without bumping its catalog version, so clients never saw an update. Bump proofreader 2 -> 3, and add a content-hash guard so this can't happen silently again. - index.json: proofreader version 2 -> 3 - scripts/check.mjs: new content-hash guard. A scripts/content-hashes.json lock maps slug -> { version, hash } (sha256 over emoji/autoStart/name/description/ instructions/launchMessage across all languages). check.mjs now fails when a role's content changed without bumping its version; the new --update-hashes (alias --fix) refreshes the lock but refuses to write when a bump is missing. - check.mjs: also require every index.json role to carry a finite numeric version (matches the server's catalog validation), with defense-in-depth so a missing version can't bypass the bump guard. - scripts/content-hashes.json: new lock artifact (not part of the served catalog). - README.md: document the guard, the lockfile, --update-hashes, and the prune-then-readd limitation. Co-Authored-By: Claude Opus 4.8 --- agent-roles-catalog/README.md | 46 +++- agent-roles-catalog/index.json | 2 +- agent-roles-catalog/scripts/check.mjs | 225 +++++++++++++++++- .../scripts/content-hashes.json | 26 ++ 4 files changed, 296 insertions(+), 3 deletions(-) create mode 100644 agent-roles-catalog/scripts/content-hashes.json diff --git a/agent-roles-catalog/README.md b/agent-roles-catalog/README.md index bbf67b15..8036b5a9 100644 --- a/agent-roles-catalog/README.md +++ b/agent-roles-catalog/README.md @@ -16,6 +16,7 @@ agent-roles-catalog/ .json # one file per declared language (e.g. ru.json, en.json) scripts/ check.mjs # validates the catalog (no dependencies) + content-hashes.json # check artifact: per-role content-hash lock (NOT served) package.json # defines the `check` script README.md ``` @@ -133,7 +134,10 @@ bundle. A slug appears once per language file of its bundle (same slug in ### Change a role's content Edit the role in the relevant `.json` file(s) and **bump that role's -`version`** in `index.json`. +`version`** in `index.json`. Then run `node scripts/check.mjs --update-hashes` +to refresh the content-hash lock (`scripts/content-hashes.json`). `check.mjs` +now **fails if a role's content changed but its `version` was not bumped**, so +this step is mandatory — the lock can only be refreshed after the bump. ## Validating @@ -147,3 +151,43 @@ It fails (exit code 1) if any slug is duplicated across the catalog, if a bundle's index `roles[]` don't match the slugs present in each language file, if a declared language file is missing, or if any role is missing a required field (`slug`, `name`, `instructions`). It prints `OK` on success. + +### Content-hash guard + +`check.mjs` also guards against changing a role's content without bumping its +`version`. It keeps a lockfile, `scripts/content-hashes.json`, mapping each role +`slug` to `{ version, hash }`, where `hash` is a SHA-256 over the role's +content fields (`emoji`, `autoStart`, `name`, `description`, `instructions`, +`launchMessage`) across all of its language files, in a deterministic canonical +form. This lockfile is a **check artifact only** — the server fetches only +`index.json` and the bundle `.json` files, never this file, so it has no +effect on the served catalog or its schema. + +On a normal run, for every role the check recomputes the hash and compares it +against the lock: + +- content unchanged and versions agree → OK; +- content changed but `version` not bumped above the lock → **error** asking you + to bump and refresh; +- content changed and `version` bumped → **error** asking you to record it by + refreshing the lock; +- role missing from the lock, or a lock entry for a role that no longer exists → + **error** asking you to refresh. + +Refresh the lock with: + +```sh +node scripts/check.mjs --update-hashes # alias: --fix +``` + +This recomputes the lock from the current catalog, prunes entries for removed +roles, and prints what changed — but it **refuses to write** (exit 1) if any +role's content changed while its `index.json` version was not bumped, so the +version bump is always enforced first. The check also requires every +`index.json` role to carry a finite numeric `version` (the server requires the +same). + +Known, accepted limitation: a deliberate prune-then-readd of a slug (remove the +role and run `--update-hashes`, then re-add it with changed content at the same +version) is **not** caught, because a brand-new slug has no lock baseline to +enforce a bump against. diff --git a/agent-roles-catalog/index.json b/agent-roles-catalog/index.json index d215abf4..49f37afe 100644 --- a/agent-roles-catalog/index.json +++ b/agent-roles-catalog/index.json @@ -13,7 +13,7 @@ { "slug": "structural-editor", "version": 2 }, { "slug": "line-editor", "version": 2 }, { "slug": "fact-checker", "version": 2 }, - { "slug": "proofreader", "version": 2 }, + { "slug": "proofreader", "version": 3 }, { "slug": "narrator", "version": 1 } ] }, diff --git a/agent-roles-catalog/scripts/check.mjs b/agent-roles-catalog/scripts/check.mjs index 32c4b698..85de8109 100644 --- a/agent-roles-catalog/scripts/check.mjs +++ b/agent-roles-catalog/scripts/check.mjs @@ -4,13 +4,23 @@ // between a bundle's index roles[] and the slugs present in each language // file, a missing declared language file, or a role missing required fields. -import { readFileSync, existsSync } from "node:fs"; +import { readFileSync, writeFileSync, existsSync } from "node:fs"; +import { createHash } from "node:crypto"; import { fileURLToPath } from "node:url"; import { dirname, join } from "node:path"; const __dirname = dirname(fileURLToPath(import.meta.url)); const catalogDir = join(__dirname, ".."); +// `--update-hashes` (alias `--fix`) recomputes the content-hash lockfile from +// the current catalog instead of just validating against it. +const updateHashes = + process.argv.includes("--update-hashes") || process.argv.includes("--fix"); + +// The content-hash lockfile lives under scripts/ and is a CHECK ARTIFACT only: +// the server never fetches it, so it has zero impact on the served schema. +const lockPath = join(__dirname, "content-hashes.json"); + const errors = []; function readJson(path) { @@ -56,6 +66,17 @@ for (const bundle of bundles) { errors.push(`Bundle "${bundleId}" index.json roles[] contains duplicate slugs`); } + // Each index role must carry a finite numeric "version". The server requires + // this (see ai-agent-roles-catalog.provider.ts), and the content-hash guard + // below relies on it for the bump comparison, so enforce it here too. + for (const r of bundle.roles || []) { + if (typeof r.version !== "number" || !Number.isFinite(r.version)) { + errors.push( + `Bundle "${bundleId}" index.json role "${r.slug}" is missing a numeric "version"` + ); + } + } + const languages = Array.isArray(bundle.languages) ? bundle.languages : []; if (languages.length === 0) { errors.push(`Bundle "${bundleId}" declares no languages`); @@ -121,6 +142,208 @@ for (const bundle of bundles) { } } +// --------------------------------------------------------------------------- +// Content-hash guard: detect "content changed without a version bump". +// +// check.mjs cannot use git history, so we maintain a lockfile +// (scripts/content-hashes.json) mapping each role slug to its recorded +// { version, hash }. On every run we recompute each role's content hash and +// compare it against the lock; a content change is only allowed once the role's +// version in index.json has been bumped and the lock refreshed. +// +// Known, accepted limitation: a deliberate prune-then-readd of a slug (remove +// the role and run --update-hashes, then re-add it with changed content at the +// same version) is NOT caught, because a brand-new slug has no lock baseline to +// enforce a bump against. We document this rather than building tombstones. +// --------------------------------------------------------------------------- + +// Content fields hashed for each role, in a fixed canonical order. `slug` is +// identity (not content) and `version` lives in index.json, so neither is here. +// `modelConfig` (an OPTIONAL role field the server also serves) is intentionally +// EXCLUDED: no shipped role uses it today, and being an object it would need a +// deterministic deep canonicalization (recursive key sort) before hashing — +// otherwise JSON.stringify key-order would make the hash non-deterministic. If a +// role ever gains a `modelConfig`, add it here WITH such canonicalization so a +// change to it is still caught by the bump guard. +const CONTENT_FIELDS = [ + "emoji", + "autoStart", + "name", + "description", + "instructions", + "launchMessage", +]; + +// Build a map of slug -> { version, langRoles: { lang: roleObject } } from the +// current catalog so we can compute hashes and read index versions. +function collectCatalogRoles() { + const out = new Map(); // slug -> { version, langRoles: Map } + for (const bundle of bundles) { + const bundleId = bundle.id; + if (!bundleId) continue; + const languages = Array.isArray(bundle.languages) ? bundle.languages : []; + for (const r of bundle.roles || []) { + if (!r || !r.slug) continue; + if (!out.has(r.slug)) { + out.set(r.slug, { version: r.version, langRoles: new Map() }); + } else { + // Same slug declared twice in index.json roles[]; already flagged above. + out.get(r.slug).version = r.version; + } + } + for (const lang of languages) { + const langPath = join(catalogDir, "bundles", bundleId, `${lang}.json`); + if (!existsSync(langPath)) continue; + const langFile = readJson(langPath); + if (!langFile) continue; + const roles = Array.isArray(langFile.roles) ? langFile.roles : []; + for (const role of roles) { + if (!role || !role.slug) continue; + const entry = out.get(role.slug); + if (!entry) continue; // role not declared in index.json; flagged above. + entry.langRoles.set(lang, role); + } + } + } + return out; +} + +// Deterministic content hash for a role: languages sorted ascending, each +// language's content fields taken in CONTENT_FIELDS order (null when absent). +function contentHash(langRoles) { + const langs = [...langRoles.keys()].sort(); + const canonical = langs.map((lang) => { + const role = langRoles.get(lang); + const fields = {}; + for (const field of CONTENT_FIELDS) { + fields[field] = role && role[field] != null ? role[field] : null; + } + return [lang, fields]; + }); + return createHash("sha256").update(JSON.stringify(canonical)).digest("hex"); +} + +// Compute current { version, hash } for every catalog role. +const catalogRoles = collectCatalogRoles(); +const current = new Map(); // slug -> { version, hash } +for (const [slug, entry] of catalogRoles) { + current.set(slug, { + version: entry.version, + hash: contentHash(entry.langRoles), + }); +} + +// Load the existing lock (may be absent on first run). +let lock = {}; +if (existsSync(lockPath)) { + const parsed = readJson(lockPath); + if (parsed && typeof parsed === "object") lock = parsed; +} + +if (updateHashes) { + // Refresh the lock from the current catalog, but refuse to write if any role's + // content changed without its version being bumped above the existing lock. + const blockers = []; + for (const [slug, cur] of current) { + const prev = lock[slug]; + if (!prev) continue; // new role; nothing to enforce a bump against. + if (cur.hash === prev.hash) continue; // content unchanged. + // Defense-in-depth: a non-numeric version must never pass the bump check via + // `undefined <= N` (which is false). The standard checks already flag a + // missing numeric version, but guard here too before comparing. + if (typeof cur.version !== "number" || !Number.isFinite(cur.version)) { + blockers.push( + `role "${slug}" content changed but its index.json "version" is missing or not numeric; set a numeric "version" before refreshing the lock` + ); + } else if (cur.version <= prev.version) { + blockers.push( + `role "${slug}" content changed but its version was not bumped (still ${prev.version}); bump "version" in index.json before refreshing the lock` + ); + } + } + // Still honor the standard checks before allowing a write. + if (errors.length > 0) { + console.error("Catalog check FAILED:"); + for (const e of errors) console.error(` - ${e}`); + process.exit(1); + } + if (blockers.length > 0) { + console.error("Refusing to update content-hash lock:"); + for (const b of blockers) console.error(` - ${b}`); + process.exit(1); + } + + // Compute the change summary relative to the old lock, pruning removed slugs. + const newLock = {}; + const added = []; + const changed = []; + const removed = []; + for (const [slug, cur] of [...current].sort((a, b) => a[0].localeCompare(b[0]))) { + newLock[slug] = { version: cur.version, hash: cur.hash }; + const prev = lock[slug]; + if (!prev) added.push(slug); + else if (prev.hash !== cur.hash || prev.version !== cur.version) changed.push(slug); + } + for (const slug of Object.keys(lock)) { + if (!current.has(slug)) removed.push(slug); + } + writeFileSync(lockPath, JSON.stringify(newLock, null, 2) + "\n"); + console.log(`Wrote ${lockPath}`); + if (added.length) console.log(` added: ${added.join(", ")}`); + if (changed.length) console.log(` updated: ${changed.join(", ")}`); + if (removed.length) console.log(` pruned: ${removed.join(", ")}`); + if (!added.length && !changed.length && !removed.length) { + console.log(" (no changes; lock already up to date)"); + } + console.log("OK"); + process.exit(0); +} + +// Normal run: validate current content against the lock. +for (const [slug, cur] of current) { + const prev = lock[slug]; + if (!prev) { + errors.push( + `role "${slug}" is not recorded in the content-hash lock; run: node scripts/check.mjs --update-hashes` + ); + continue; + } + if (cur.hash === prev.hash) { + // Content unchanged; the lock version must still agree with index.json. + if (cur.version !== prev.version) { + errors.push( + `role "${slug}" content is unchanged but its index.json version (${cur.version}) differs from the lock (${prev.version}); run: node scripts/check.mjs --update-hashes` + ); + } + continue; + } + // Content changed. + // Defense-in-depth: treat a non-numeric version as an error before the `<=` + // comparison, so a missing version can never silently pass the bump check + // (and we avoid a misleading "version bumped to undefined" message). + if (typeof cur.version !== "number" || !Number.isFinite(cur.version)) { + errors.push( + `role "${slug}" content changed but its index.json "version" is missing or not numeric; set a numeric "version", then run: node scripts/check.mjs --update-hashes` + ); + } else if (cur.version <= prev.version) { + errors.push( + `role "${slug}" content changed but its version was not bumped (still ${prev.version}); bump "version" in index.json, then run: node scripts/check.mjs --update-hashes` + ); + } else { + errors.push( + `role "${slug}" content changed and version bumped to ${cur.version}; record it by running: node scripts/check.mjs --update-hashes` + ); + } +} +// Lock entries for slugs that no longer exist in the catalog. +for (const slug of Object.keys(lock)) { + if (!current.has(slug)) { + errors.push( + `content-hash lock has entry for unknown role "${slug}" (no longer in the catalog); run: node scripts/check.mjs --update-hashes` + ); + } +} + if (errors.length > 0) { console.error("Catalog check FAILED:"); for (const e of errors) console.error(` - ${e}`); diff --git a/agent-roles-catalog/scripts/content-hashes.json b/agent-roles-catalog/scripts/content-hashes.json new file mode 100644 index 00000000..830771e4 --- /dev/null +++ b/agent-roles-catalog/scripts/content-hashes.json @@ -0,0 +1,26 @@ +{ + "fact-checker": { + "version": 2, + "hash": "d7ad1dae07d6f4321e7d40c5b36259dbf930264d748834809c4fb77294bf72e3" + }, + "line-editor": { + "version": 2, + "hash": "cca324110dc6f96d2a8a239a2fb95b0ba09fad5806c9b6090a3c210ea7883ceb" + }, + "narrator": { + "version": 1, + "hash": "36b38785fea6ae1c70bf6fb6b29ae5278bb86e389e61f7b9736675a589fa434c" + }, + "proofreader": { + "version": 3, + "hash": "a36047c5cab837b2a727f63d4ddafc269b1fc44b90b365e770ecdb8f77e13952" + }, + "researcher": { + "version": 1, + "hash": "853658fda43ddbe0a4d08f2c6e50b5116d29a2e9ccd7f46e173e65920d8f6ace" + }, + "structural-editor": { + "version": 2, + "hash": "83093baa7262aef8193871a1afcf2b43b11a56fe2d00cade41355cf66d972b74" + } +}