From d1ddc9ce61f4d3d619bcd81eff694e96f2675fb9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 17:52:44 +0000 Subject: [PATCH 1/5] v2.125.14: work-review never shows an unmeasured figure as 0; setup audit flags a skill installed twice; plugin changes need a version bump - The uploader sends a figure the analyzer could not measure as null, not 0, and the hub stores and shows it as a dash. A week with no commits no longer reads as $0.00 per commit. Team medians over nobody are null, not 0. - The upload now carries the tokens from models with no known price, and the hub marks the cost as incomplete rather than understating it. - audit-setup reads installed plugin skills and adds skill-installed-twice: the same skill at two scopes, for example a personal copy beside a plugin's. Both load and compete, and a plugin update moves only one copy. A symlink to the same file counts as one install. - plugins/releases.json records each marketplace plugin's version and file hash. test/plugin-releases.test.js fails when a plugin's files change without a version bump, and npm run gen:plugin-releases refuses to record such a change. work-review is now 1.1.0, so existing installs receive these changes. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X2X5cmFm94J9Gz2esCZBEG --- CLAUDE.md | 6 ++ examples/work-review-hub/digest.mjs | 3 +- examples/work-review-hub/index.html | 36 +++++----- examples/work-review-hub/server.mjs | 17 +++-- package-lock.json | 4 +- package.json | 5 +- plugins/releases.json | 6 ++ .../work-review/.claude-plugin/plugin.json | 2 +- .../work-review/references/best-practices.md | 1 + .../work-review/scripts/audit-setup.mjs | 28 ++++++-- .../skills/work-review/scripts/upload.mjs | 8 ++- release-notes.json | 5 ++ scripts/gen-plugin-releases.mjs | 70 +++++++++++++++++++ test/plugin-releases.test.js | 33 +++++++++ test/work-review-setup-audit.test.js | 22 ++++++ test/work-review-upload.test.js | 13 ++++ 16 files changed, 224 insertions(+), 35 deletions(-) create mode 100644 plugins/releases.json create mode 100644 scripts/gen-plugin-releases.mjs create mode 100644 test/plugin-releases.test.js diff --git a/CLAUDE.md b/CLAUDE.md index 3f420301..26db5802 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,6 +36,12 @@ run `npm run sync:changelog` there, build with `npm test`, and publish the site the release. Patch releases fold into their minor's entry. The page fell 95 minor releases behind (v2.21 shown at v2.116) before this step was written down. +## Releasing: bump a plugin's version when its files change +The `plugins/` marketplace sets an explicit `version` in each `plugin.json`, and Claude +Code ships an update only when that version changes. After changing a plugin, bump its +version, then run `npm run gen:plugin-releases`. `test/plugin-releases.test.js` fails +until you do, and the generator refuses to record a change that kept the old version. + ## Data Persistence If settings or configuration appear wiped, always check `/data` first. AppCrane stores all persistent state (database, env vars, app configs) under the `DATA_DIR` path (default: `./data`). diff --git a/examples/work-review-hub/digest.mjs b/examples/work-review-hub/digest.mjs index 707331ad..94c08579 100644 --- a/examples/work-review-hub/digest.mjs +++ b/examples/work-review-hub/digest.mjs @@ -23,10 +23,11 @@ const SETUP_FIXES = { 'mcp-unused': 'An MCP server you never call: disable it with /mcp', 'allow-all-bash': 'Bash(*) is allowed: narrow it, or use auto mode', 'skill-side-effect-auto': 'A deploy-style skill Claude can run on its own: set disable-model-invocation', + 'skill-installed-twice': 'A skill installed twice, as your own copy and from a plugin: delete your copy', }; const esc = s => String(s).replace(/[&<>"']/g, c => ({ '&': '&', '<': '<', '>': '>', '"': '"', "'": ''' }[c])); -const usd = n => (n >= 100 ? `$${n.toFixed(0)}` : `$${(n || 0).toFixed(2)}`); +const usd = n => (n == null ? 'not measured' : n >= 100 ? `$${n.toFixed(0)}` : `$${n.toFixed(2)}`); /** ISO week label, e.g. 2026-W44, for "once per week" markers and idempotency. */ export function isoWeek(d) { diff --git a/examples/work-review-hub/index.html b/examples/work-review-hub/index.html index ec5903c6..8d89ef6e 100644 --- a/examples/work-review-hub/index.html +++ b/examples/work-review-hub/index.html @@ -69,9 +69,11 @@

Team

// only, never as HTML. const $ = id => document.getElementById(id); const el = (tag, text, cls) => { const e = document.createElement(tag); if (text != null) e.textContent = text; if (cls) e.className = cls; return e; }; -const k = n => n >= 1e6 ? (n / 1e6).toFixed(1) + 'M' : n >= 1e3 ? (n / 1e3).toFixed(1) + 'k' : String(Math.round(n)); -const pct = x => (x * 100).toFixed(0) + '%'; -const usd = n => n >= 100 ? `$${n.toFixed(0)}` : `$${(n || 0).toFixed(2)}`; +// null is "not measured": shown as a dash, never as 0. +const k = n => n == null ? '—' : n >= 1e6 ? (n / 1e6).toFixed(1) + 'M' : n >= 1e3 ? (n / 1e3).toFixed(1) + 'k' : String(Math.round(n)); +const n0 = v => v == null ? '—' : String(v); +const pct = x => x == null ? '—' : (x * 100).toFixed(0) + '%'; +const usd = n => n == null ? '—' : n >= 100 ? `$${n.toFixed(0)}` : `$${n.toFixed(2)}`; const api = p => fetch(p, { credentials: 'same-origin' }).then(r => { if (!r.ok) throw new Error(r.status); return r.json(); }); function kpi(label, value, cls) { const d = el('div', null, 'kpi'); d.append(el('div', value, 'v' + (cls ? ' ' + cls : '')), el('div', label, 'l')); return d; } @@ -83,22 +85,23 @@

Team

const r = reports[reports.length - 1], T = r.totals, Q = r.instructionQuality, O = r.overcomplication; $('asof').textContent = `· snapshot ${r.day}`; $('kpis').replaceChildren( - kpi('Cost at API list prices (not your seat bill)', usd(T.usd)), + kpi(T.unpricedTokens ? `Cost at API list prices: incomplete, ${k(T.unpricedTokens)} tokens from unpriced models not counted` : 'Cost at API list prices (not your seat bill)', usd(T.usd), T.unpricedTokens ? 'warn' : ''), kpi('Spent on prompts that needed correcting', `${pct(Q.reworkShare)} · ${usd(Q.reworkUsd)}`, Q.reworkShare > 0.15 ? 'warn' : ''), - kpi('Commits landed · cost per commit', r.outcomes ? `${r.outcomes.commits} · ${r.outcomes.commits ? usd(r.outcomes.usdPerCommit) : '—'}` : '—'), - kpi('Vague prompts', `${Q.vague.prompts} / ${Q.vague.prompts + Q.anchored.prompts}`), + kpi('Commits landed · cost per commit', r.outcomes ? `${n0(r.outcomes.commits)} · ${r.outcomes.commits ? usd(r.outcomes.usdPerCommit) : '—'}` : '—'), + kpi('Vague prompts', Q.vague.prompts == null || Q.anchored.prompts == null ? '—' : `${Q.vague.prompts} / ${Q.vague.prompts + Q.anchored.prompts}`), kpi('Cache hit rate', pct(T.cacheHitRate), T.cacheHitRate > 0.85 ? 'good' : ''), kpi('Tool error rate', pct(T.errorRate), T.errorRate > 0.1 ? 'warn' : ''), - kpi('Sessions · prompts', `${T.sessions} · ${T.userPrompts}`)); + kpi('Sessions · prompts', `${n0(T.sessions)} · ${n0(T.userPrompts)}`)); // Trend: totalCost per daily snapshot, as a simple line. - const pts = reports.map(x => x.totals.usd || 0), max = Math.max(1, ...pts), W = 1000, H = 120, pad = 8; + // A snapshot whose cost was not measured is left off the line, not drawn at $0. + const pts = reports.map(x => x.totals.usd).filter(v => v != null), max = Math.max(1, ...pts), W = 1000, H = 120, pad = 8; const svg = $('trend'); svg.setAttribute('viewBox', `0 0 ${W} ${H}`); svg.replaceChildren(); const ns = 'http://www.w3.org/2000/svg', line = document.createElementNS(ns, 'polyline'); line.setAttribute('points', pts.map((v, i) => `${pad + i * (W - 2 * pad) / Math.max(1, pts.length - 1)},${H - pad - v / max * (H - 2 * pad)}`).join(' ')); line.setAttribute('fill', 'none'); line.setAttribute('stroke', 'currentColor'); line.setAttribute('stroke-width', '2.5'); line.style.color = 'var(--accent)'; svg.append(line); - $('trendNote').textContent = `${reports.length} daily snapshots (each a 7-day window), ${reports[0].day} → ${r.day}. Peak ${usd(max)} at API list prices.`; + $('trendNote').textContent = `${pts.length} daily snapshots (each a 7-day window), ${reports[0].day} → ${r.day}. Peak ${usd(max)} at API list prices.`; // Since about two weeks ago: did the fixes work? The newest snapshot at least // 10 days older than today's, so the two 7-day windows do not overlap much. @@ -106,13 +109,14 @@

Team

const then = [...reports].reverse().find(x => x.day <= cutoff); if (then) { const per = x => Math.max(1, x.totals.sessions); - const vagueShare = x => { const v = x.instructionQuality.vague.prompts, a = x.instructionQuality.anchored.prompts; return v + a ? v / (v + a) : 0; }; + const vagueShare = x => { const v = x.instructionQuality.vague.prompts, a = x.instructionQuality.anchored.prompts; return v == null || a == null || !(v + a) ? null : v / (v + a); }; + const perSession = n => x => (n(x) == null ? null : n(x) / per(x)); const signals = [ - ['Cost per session', x => x.totals.usd / per(x), 'lower', usd], - ['Cost per landed commit', x => x.outcomes?.usdPerCommit || null, 'lower', usd], + ['Cost per session', perSession(x => x.totals.usd), 'lower', usd], + ['Cost per landed commit', x => (x.outcomes?.commits ? x.outcomes.usdPerCommit : null), 'lower', usd], ['Spend on prompts that needed correcting', x => x.instructionQuality.reworkShare, 'lower', pct], ['Vague prompts', vagueShare, 'lower', pct], - ['Re-reads of unchanged files per session', x => x.totals.redundantReads / per(x), 'lower', n => n.toFixed(1)], + ['Re-reads of unchanged files per session', perSession(x => x.totals.redundantReads), 'lower', n => n.toFixed(1)], ['Setup findings', x => x.setup ? Object.values(x.setup.findings || {}).reduce((a, n) => a + n, 0) : null, 'lower', n => String(n)], ['Cache hit rate', x => x.totals.cacheHitRate, 'higher', pct], ]; @@ -152,8 +156,8 @@

Team

oc.append(sf); } const SA = r.subagents; - if (SA && SA.runs) oc.append(el('div', `Subagents: ${SA.runs} runs, ${usd(SA.usd)} (${usd(SA.startupUsd)} start-up), ${Math.round(SA.parallelShare * 100)}% parallel · no benefit from delegating: ${usd(SA.flaggedUsd)}${SA.repeatedDelegations ? ` · ${SA.repeatedDelegations} procedures delegated 3+ times (skill candidates)` : ''}`, 'mute')); - oc.append(el('div', `Always-on context ~${k(O.medianBaselineContextTokens)} tokens/session · ${O.toolCallsPerPrompt} tool calls per prompt · ${O.subagentRuns} subagents · ${O.nudgePrompts} "continue/yes" prompts`, 'mute')); + if (SA && SA.runs) oc.append(el('div', `Subagents: ${SA.runs} runs, ${usd(SA.usd)} (${usd(SA.startupUsd)} start-up), ${pct(SA.parallelShare)} parallel · no benefit from delegating: ${usd(SA.flaggedUsd)}${SA.repeatedDelegations ? ` · ${SA.repeatedDelegations} procedures delegated 3+ times (skill candidates)` : ''}`, 'mute')); + oc.append(el('div', `Always-on context ${O.medianBaselineContextTokens == null ? "—" : "~" + k(O.medianBaselineContextTokens)} tokens/session · ${n0(O.toolCallsPerPrompt)} tool calls per prompt · ${n0(O.subagentRuns)} subagents · ${n0(O.nudgePrompts)} "continue/yes" prompts`, 'mute')); const fl = el('div'); fl.style.margin = '8px 0'; flags.length ? flags.forEach(([f, n]) => fl.append(el('span', `${f} ×${n}`, 'tag'))) : fl.append(el('span', 'No flags', 'tag')); oc.append(fl); @@ -173,7 +177,7 @@

Team

b.append(el('p', `${T.people} people active in the last ${T.activeDays} days · ${usd(T.usdTotal)} at API list prices (7-day windows).`, 'mute')); const m = T.median, t = el('table'); t.append(row(['Median per person', ''], true)); - for (const [l, v] of [['Cost at API list prices', usd(m.usd)], ['Cost per landed commit', m.usdPerCommit ? usd(m.usdPerCommit) : '—'], ['Spend on prompts that needed correcting', pct(m.reworkShare)], ['Share of prompts that were vague', pct(m.vagueShare)], + for (const [l, v] of [['Cost at API list prices', usd(m.usd)], ['Cost per landed commit', usd(m.usdPerCommit)], ['Spend on prompts that needed correcting', pct(m.reworkShare)], ['Share of prompts that were vague', pct(m.vagueShare)], ['Cache hit rate', pct(m.cacheHitRate)], ['Tool error rate', pct(m.errorRate)], ['CLAUDE.md tokens on every request', k(m.alwaysOnTokens)]]) t.append(row([l, v])); b.append(t); const list = (title, obj) => { diff --git a/examples/work-review-hub/server.mjs b/examples/work-review-hub/server.mjs index a190462b..627b3cf9 100644 --- a/examples/work-review-hub/server.mjs +++ b/examples/work-review-hub/server.mjs @@ -44,7 +44,8 @@ const isNum = v => typeof v === 'number' && Number.isFinite(v) && v >= 0 && v < function nums(o, keys, path) { if (!o || typeof o !== 'object') throw new Error(`${path} missing`); const out = {}; - for (const k of keys) { if (!isNum(o[k] ?? 0)) throw new Error(`${path}.${k} not a number`); out[k] = o[k] ?? 0; } + // null (or absent) means "not measured" and is kept as null, never stored as 0. + for (const k of keys) { if (o[k] != null && !isNum(o[k])) throw new Error(`${path}.${k} not a number`); out[k] = o[k] ?? null; } return out; } const SECRETISH = /(sk-[A-Za-z0-9_-]{16,}|gh[pousr]_[A-Za-z0-9]{20,}|dhk_[A-Za-z0-9_-]{8,}|AKIA[0-9A-Z]{16}|-----BEGIN|eyJ[A-Za-z0-9_-]{10,}\.)/; @@ -69,7 +70,7 @@ export function validate(p) { 'searchBursts', 'parallelToolTurnShare'], 'totals'), tokens: nums(T.tokens, ['input', 'output', 'cacheRead', 'cacheWrite'], 'totals.tokens'), wastedTokensEstimate: nums(T.wastedTokensEstimate, ['redundantReads', 'idleReruns', 'errorOutputs'], 'totals.wastedTokensEstimate'), - usd: nums(T, ['usd'], 'totals').usd, + ...nums(T, ['usd', 'unpricedTokens'], 'totals'), }, instructionQuality: { ...nums(Q, ['totalCost', 'reworkCost', 'reworkShare', 'totalUsd', 'reworkUsd'], 'instructionQuality'), @@ -126,7 +127,9 @@ function history(dir, limit = 90) { // nothing is shown but the count. export const TEAM_MIN_PEOPLE = Math.max(3, Number(process.env.TEAM_MIN_PEOPLE || 5)); const TEAM_ACTIVE_DAYS = 14; -const median = xs => { const s = xs.filter(Number.isFinite).sort((a, b) => a - b); return s.length ? s[Math.floor(s.length / 2)] : 0; }; +// null when nobody measured it: an empty median is not a zero. +const median = xs => { const s = xs.filter(Number.isFinite).sort((a, b) => a - b); return s.length ? s[Math.floor(s.length / 2)] : null; }; +const share = (v, a) => (v == null || a == null || !(v + a) ? null : v / (v + a)); function team() { if (!existsSync(DATA)) return { people: 0, minPeople: TEAM_MIN_PEOPLE, enough: false }; const since = new Date(Date.now() - TEAM_ACTIVE_DAYS * 86400_000).toISOString().slice(0, 10); @@ -139,13 +142,13 @@ function team() { ...base, enough: true, usdTotal: +latest.reduce((a, s) => a + (s.totals.usd || 0), 0).toFixed(2), median: { - usd: median(latest.map(s => s.totals.usd || 0)), - usdPerCommit: median(latest.map(s => s.outcomes?.usdPerCommit).filter(x => x > 0)), + usd: median(latest.map(s => s.totals.usd)), + usdPerCommit: median(latest.map(s => s.outcomes?.commits ? s.outcomes.usdPerCommit : null)), reworkShare: median(latest.map(s => s.instructionQuality.reworkShare)), - vagueShare: median(latest.map(s => { const v = s.instructionQuality.vague.prompts, a = s.instructionQuality.anchored.prompts; return v + a ? v / (v + a) : 0; })), + vagueShare: median(latest.map(s => share(s.instructionQuality.vague.prompts, s.instructionQuality.anchored.prompts))), cacheHitRate: median(latest.map(s => s.totals.cacheHitRate)), errorRate: median(latest.map(s => s.totals.errorRate)), - alwaysOnTokens: median(latest.map(s => s.setup?.alwaysOnTokens).filter(x => x != null)), + alwaysOnTokens: median(latest.map(s => s.setup?.alwaysOnTokens)), }, subagentUsdNoBenefit: +latest.reduce((a, s) => a + (s.subagents?.flaggedUsd || 0), 0).toFixed(2), peopleWithSetupFinding: peopleWith(s => s.setup?.findings), diff --git a/package-lock.json b/package-lock.json index 4fd32bcc..8a1b39b7 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "appcrane", - "version": "2.125.13", + "version": "2.125.14", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "appcrane", - "version": "2.125.13", + "version": "2.125.14", "license": "AGPL-3.0-only", "dependencies": { "@node-saml/node-saml": "^5.1.0", diff --git a/package.json b/package.json index 93afe2d1..79337a68 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "appcrane", - "version": "2.125.13", + "version": "2.125.14", "engines": { "node": ">=22" }, @@ -25,7 +25,8 @@ "dev": "node --watch server/index.js", "test": "node --test --test-timeout=120000 test/*.test.js packages/tenant/*.test.js", "test:serial": "node --test --test-concurrency=1 --test-timeout=300000 test/serial/*.test.js", - "gen:mcp-snapshot": "node scripts/gen-mcp-snapshot.mjs" + "gen:mcp-snapshot": "node scripts/gen-mcp-snapshot.mjs", + "gen:plugin-releases": "node scripts/gen-plugin-releases.mjs" }, "overrides": { "follow-redirects": "^1.16.0" diff --git a/plugins/releases.json b/plugins/releases.json new file mode 100644 index 00000000..2bb10505 --- /dev/null +++ b/plugins/releases.json @@ -0,0 +1,6 @@ +{ + "work-review": { + "version": "1.1.0", + "hash": "ea0af9291b9114433511a0b5ae819f1a42eaf91d2c1d15b6a8169dc40e217f7f" + } +} diff --git a/plugins/work-review/.claude-plugin/plugin.json b/plugins/work-review/.claude-plugin/plugin.json index 59216a96..87755ede 100644 --- a/plugins/work-review/.claude-plugin/plugin.json +++ b/plugins/work-review/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "work-review", - "version": "1.0.0", + "version": "1.1.0", "description": "Reviews how you work with Claude Code: what each prompt cost and why, waste and overcomplicated setups, with concrete fixes. Optional daily upload of a redacted summary to a Work Review hub.", "author": { "name": "AppCrane" }, "repository": "https://github.com/gitayg/appCrane", diff --git a/plugins/work-review/skills/work-review/references/best-practices.md b/plugins/work-review/skills/work-review/references/best-practices.md index bf3f7378..26b47bcf 100644 --- a/plugins/work-review/skills/work-review/references/best-practices.md +++ b/plugins/work-review/skills/work-review/references/best-practices.md @@ -41,6 +41,7 @@ Sources: - **Commands are merged into skills:** "Prefer a skill for new work, since skills also support supporting files" (skills). - **Description:** put the key use case first. Description and `when_to_use` are "truncated at 1,536 characters" in the listing (skills). - **Side effects:** use `disable-model-invocation: true`, because "You don't want Claude deciding to deploy because your code looks ready" (skills). +- **One copy per skill (ours):** the docs do not cover a skill installed twice. Plugin skills are namespaced, so a personal copy beside a plugin's does not replace it: both load and compete for the same requests, and a plugin update moves only one. `skill-installed-twice` flags it; a symlink to the same file is one install. - **Size:** "Keep `SKILL.md` under 500 lines. Move detailed reference material to separate files." (skills) - **Scripts are "executed, not loaded".** Reference them via `${CLAUDE_SKILL_DIR}` (skills). diff --git a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs index d29f527f..60738126 100644 --- a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs +++ b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs @@ -11,7 +11,7 @@ // // Read-only. Prints a markdown summary; --json writes the findings. -import { readdirSync, readFileSync, statSync, writeFileSync, existsSync } from 'fs'; +import { readdirSync, readFileSync, statSync, writeFileSync, existsSync, realpathSync } from 'fs'; import { join, relative, basename, dirname } from 'path'; import { homedir } from 'os'; @@ -166,13 +166,17 @@ for (let i = 0; i < agents.length; i++) for (let j = i + 1; j < agents.length; j } // ── skills ─────────────────────────────────────────────────────── -const skillFiles = [join(PROJECT, '.claude', 'skills'), join(USER_DIR, 'skills')].filter(isDir) - .flatMap(d => walk(d, (p, n) => n === 'SKILL.md', 4)); -const skills = skillFiles.map(f => { +// Installed plugins are read too (inventoried, never judged), so a personal +// copy of a skill a plugin also ships is caught below. +const skillFiles = [[join(PROJECT, '.claude', 'skills'), 'project', 4], [join(USER_DIR, 'skills'), 'user', 4], [join(USER_DIR, 'plugins', 'cache'), 'plugin', 7]] + .filter(([d]) => isDir(d)) + .flatMap(([d, scope, depth]) => walk(d, (p, n) => n === 'SKILL.md', depth).map(f => ({ f, scope: scope === 'user' && /[\\/]synced[\\/]/.test(f) ? 'synced' : scope }))); +const real = f => { try { return realpathSync(f); } catch { return f; } }; +const skills = skillFiles.map(({ f, scope }) => { const text = read(f) || ''; const { fm } = frontmatter(text); const name = fm?.name || basename(dirname(f)); - const s = { name, file: rel(f), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; + const s = { name, scope, file: rel(f), real: real(f), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; // Skills synced from an organisation or plugin are inventoried, not judged: // the person running the review cannot edit them. s.managed = /[\\/](synced|plugins)[\\/]/.test(f); @@ -188,7 +192,21 @@ const skills = skillFiles.map(f => { if (s.used === 0 && enoughUsage) add('skill-unused', 'Skills', 'low', name, `not invoked in ${usage.sessions} recent sessions`, 'Check the description triggers on real requests, or remove it', s); return s; }); +// The same skill installed twice — a personal or project copy beside a plugin's, +// say — loads twice and competes for the same requests, and a plugin update +// moves only one of them: the copies drift with nothing saying so. A symlink to +// the same file is one install, not two. +const twice = new Set(); for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { + const a = skills[i], b = skills[j]; + if (a.real === b.real || a.scope === b.scope || a.name !== b.name) continue; + twice.add(i + ':' + j); + const copy = a.scope === 'plugin' ? b : a, other = copy === a ? b : a; + add('skill-installed-twice', 'Skills', 'medium', a.name, `installed at ${copy.scope} scope (${copy.file}) and ${other.scope} scope (${other.file}); both load and compete, and updating one leaves the other behind`, + other.scope === 'plugin' ? `Delete the ${copy.scope} copy and keep the plugin's, so updates reach it` : 'Keep one copy: delete the other, or link it to the first', { files: [a.file, b.file] }); +} +for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { + if (twice.has(i + ':' + j) || skills[i].real === skills[j].real) continue; const sim = jaccard(shingles(skills[i].description), shingles(skills[j].description)); if (sim >= LIMITS.similarDescriptions) add('skill-overlap', 'Skills', 'medium', `${skills[i].name} / ${skills[j].name}`, `descriptions overlap ${Math.round(sim * 100)}%`, 'Merge them or sharpen when each applies', {}); } diff --git a/plugins/work-review/skills/work-review/scripts/upload.mjs b/plugins/work-review/skills/work-review/scripts/upload.mjs index af7ea633..eb77243d 100644 --- a/plugins/work-review/skills/work-review/scripts/upload.mjs +++ b/plugins/work-review/skills/work-review/scripts/upload.mjs @@ -73,7 +73,10 @@ export function scrub(s) { return t.length > EXCERPT_CHARS ? t.slice(0, EXCERPT_CHARS) + '…' : t; } -const num = v => (typeof v === 'number' && Number.isFinite(v) ? v : 0); +// A figure the analyzer could not measure travels as null, never as 0: a week +// with no commits has no cost per commit, and a stored 0 would read a month +// later exactly like a perfect week. +const num = v => (typeof v === 'number' && Number.isFinite(v) ? v : null); const pickNums = (o, keys) => Object.fromEntries(keys.map(k => [k, num(o?.[k])])); const group = g => pickNums(g, ['prompts', 'medianCost', 'medianUsd', 'medianExploreBeforeFirstEdit', 'reworkRate']); @@ -99,6 +102,9 @@ export function buildPayload(report, { now = new Date(), setup = null, allowExce wastedTokensEstimate: pickNums(T.wastedTokensEstimate, ['redundantReads', 'idleReruns', 'errorOutputs']), // API-list-price dollars (see analyze.mjs): the API-equivalent value, not a bill. usd: num(T.usd), + // Tokens from models with no known price: their cost is missing from usd, + // so the hub says the total is incomplete instead of showing it as whole. + unpricedTokens: Object.values(T.unpricedModels || {}).reduce((a, n) => a + (num(n) || 0), 0), }, instructionQuality: { totalCost: num(Q.totalCost), reworkCost: num(Q.reworkCost), reworkShare: num(Q.reworkShare), diff --git a/release-notes.json b/release-notes.json index d05b6403..727e34d4 100644 --- a/release-notes.json +++ b/release-notes.json @@ -1,4 +1,9 @@ { + "2.125.14": [ + "Work Review shows a figure it could not measure as a dash, never as zero: a week with no commits no longer shows $0.00 per commit, and when some usage comes from a model without a known price, the cost says it is incomplete.", + "The work-review setup audit flags a skill installed twice, such as your own copy beside the same skill from a plugin: both load and compete, and a plugin update only moves one of them.", + "The work-review plugin is now version 1.1.0, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." + ], "2.125.13": [ "The Dashboard's visitor and CPU charts label each day correctly for people west of UTC (in the Americas every day showed as the day before)." ], diff --git a/scripts/gen-plugin-releases.mjs b/scripts/gen-plugin-releases.mjs new file mode 100644 index 00000000..8b5c84a7 --- /dev/null +++ b/scripts/gen-plugin-releases.mjs @@ -0,0 +1,70 @@ +#!/usr/bin/env node +// Regenerates plugins/releases.json: for each plugin in this repo's marketplace, +// the version it ships and a hash of every file in it. +// +// Why: plugin.json sets an explicit `version`, and Claude Code uses that version +// to decide whether an update exists. Change a plugin's files without bumping it +// and nobody who already installed the plugin ever receives the change — +// `claude plugin update` reports it is already at the latest version, and +// nothing goes red. test/plugin-releases.test.js fails the moment the files and +// the recorded hash disagree; this script refuses to re-record a changed plugin +// whose version was not bumped, so regenerating cannot paper over it. +// +// npm run gen:plugin-releases # after changing a plugin and bumping its version +import { readFileSync, writeFileSync, readdirSync, lstatSync, existsSync } from 'node:fs'; +import { createHash } from 'node:crypto'; +import { fileURLToPath } from 'node:url'; +import { dirname, join, relative } from 'node:path'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); +export const RELEASES = join(ROOT, 'plugins', 'releases.json'); + +function files(dir, out = []) { + for (const name of readdirSync(dir).sort()) { + const p = join(dir, name), st = lstatSync(p); + if (st.isDirectory()) files(p, out); + else out.push(p); + } + return out; +} + +/** Hash of a plugin's files: paths and contents, line endings normalised. */ +export function pluginHash(dir) { + const h = createHash('sha256'); + for (const f of files(dir)) { + const st = lstatSync(f); + const body = st.isSymbolicLink() ? 'symlink' : readFileSync(f).toString('latin1').replace(/\r\n/g, '\n'); + h.update(relative(dir, f).replace(/\\/g, '/') + '\0' + body + '\0'); + } + return h.digest('hex'); +} + +/** The marketplace's local plugins as { name: { version, hash } }. */ +export function currentReleases() { + const mkt = JSON.parse(readFileSync(join(ROOT, '.claude-plugin', 'marketplace.json'), 'utf8')); + const out = {}; + for (const p of mkt.plugins) { + if (typeof p.source !== 'string' || !p.source.startsWith('./')) continue; + const dir = join(ROOT, p.source); + const { version } = JSON.parse(readFileSync(join(dir, '.claude-plugin', 'plugin.json'), 'utf8')); + out[p.name] = { version, hash: pluginHash(dir) }; + } + return out; +} + +/** Plugins whose files changed while their version stayed the same. */ +export function unbumped(recorded, current) { + return Object.entries(current).filter(([n, c]) => recorded[n] && recorded[n].hash !== c.hash && recorded[n].version === c.version).map(([n]) => n); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) { + const recorded = existsSync(RELEASES) ? JSON.parse(readFileSync(RELEASES, 'utf8')) : {}; + const current = currentReleases(); + const stuck = unbumped(recorded, current); + if (stuck.length) { + console.error(`refusing: ${stuck.join(', ')} changed but kept version ${stuck.map(n => current[n].version).join(', ')}. Bump "version" in its .claude-plugin/plugin.json first, or installed copies never receive the change.`); + process.exit(1); + } + writeFileSync(RELEASES, JSON.stringify(current, null, 2) + '\n'); + console.log(`wrote ${Object.keys(current).length} plugin(s) to ${relative(ROOT, RELEASES)}`); +} diff --git a/test/plugin-releases.test.js b/test/plugin-releases.test.js new file mode 100644 index 00000000..91e07ad0 --- /dev/null +++ b/test/plugin-releases.test.js @@ -0,0 +1,33 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { readFileSync } from 'fs'; + +// Pins each plugin in this repo's marketplace against plugins/releases.json. +// +// plugin.json sets an explicit `version`, and that version is what Claude Code +// compares to decide whether an update exists. So a change to a plugin's files +// that leaves the version alone reaches nobody who already installed it, and +// nothing anywhere goes red: `claude plugin update` just says "latest". This +// test is that red light. It fails when a plugin's files no longer match the +// recorded hash; the generator refuses to re-record a change without a bump. + +const { currentReleases, unbumped, RELEASES } = await import('../scripts/gen-plugin-releases.mjs'); + +test('every plugin change ships with a version bump', () => { + const recorded = JSON.parse(readFileSync(RELEASES, 'utf8')); + const current = currentReleases(); + const stuck = unbumped(recorded, current); + assert.deepEqual(stuck, [], `${stuck.join(', ')} changed without a version bump: bump "version" in its .claude-plugin/plugin.json, then run npm run gen:plugin-releases`); + for (const [name, c] of Object.entries(current)) { + assert.ok(recorded[name], `${name} is not recorded: run npm run gen:plugin-releases`); + assert.equal(recorded[name].hash, c.hash, `${name} was bumped to ${c.version} but not re-recorded: run npm run gen:plugin-releases`); + assert.equal(recorded[name].version, c.version, `${name}: run npm run gen:plugin-releases`); + } +}); + +test('the guard refuses a changed plugin that kept its version, and accepts a bumped one', () => { + const before = { p: { version: '1.0.0', hash: 'a' } }; + assert.deepEqual(unbumped(before, { p: { version: '1.0.0', hash: 'b' } }), ['p']); + assert.deepEqual(unbumped(before, { p: { version: '1.0.1', hash: 'b' } }), []); + assert.deepEqual(unbumped(before, { p: { version: '1.0.0', hash: 'a' } }), []); +}); diff --git a/test/work-review-setup-audit.test.js b/test/work-review-setup-audit.test.js index 3090941e..89324164 100644 --- a/test/work-review-setup-audit.test.js +++ b/test/work-review-setup-audit.test.js @@ -108,3 +108,25 @@ test('a skill installed as a symlink is inventoried', async () => { const r = run(proj, user, null); assert.ok(r.inventory.skills.some(s => s.name === 'linked')); }); + +test('a skill installed both as a personal copy and from a plugin is caught; a symlink is not', async () => { + const { symlinkSync } = await import('fs'); + const root = mkdtempSync(join(tmpdir(), 'audit-')); + const proj = join(root, 'proj'), user = join(root, 'user'); + mkdirSync(join(root, 'home'), { recursive: true }); + const skill = '---\nname: spec\ndescription: Runs the spec lifecycle from intent to release; use when starting a feature\n---\nsteps'; + w(join(user, 'skills', 'spec', 'SKILL.md'), skill); + w(join(user, 'plugins', 'cache', 'mkt', 'productizer', '4.73.0', 'skills', 'spec', 'SKILL.md'), skill); + // The project links to the plugin's own copy: one install, not two. + w(join(root, 'checkout', 'review', 'SKILL.md'), '---\nname: review\ndescription: Reviews pull requests for missing tests; use when asked to review a PR\n---\nsteps'); + w(join(user, 'plugins', 'cache', 'mkt', 'tools', '1.0.0', 'skills', 'review', '.keep'), ''); + symlinkSync(join(root, 'checkout', 'review', 'SKILL.md'), join(user, 'plugins', 'cache', 'mkt', 'tools', '1.0.0', 'skills', 'review', 'SKILL.md')); + mkdirSync(join(proj, '.claude', 'skills'), { recursive: true }); + symlinkSync(join(root, 'checkout', 'review'), join(proj, '.claude', 'skills', 'review')); + + const r = run(proj, user, null); + const twice = r.findings.filter(f => f.kind === 'skill-installed-twice'); + assert.deepEqual(twice.map(f => f.item), ['spec'], `got ${kinds(r).join(', ')}`); + assert.match(twice[0].fix, /Delete the user copy and keep the plugin's/); + assert.ok(!kinds(r).includes('skill-overlap'), 'a duplicate install is reported once, not also as an overlap'); +}); diff --git a/test/work-review-upload.test.js b/test/work-review-upload.test.js index 0e636546..91454d1c 100644 --- a/test/work-review-upload.test.js +++ b/test/work-review-upload.test.js @@ -63,6 +63,19 @@ test('hub accepts exactly what the uploader builds', () => { assert.equal(v.instructionQuality.costliestBadPrompts.length, 1); }); +test('what was not measured reaches the hub as null, never as 0', () => { + // No commits, so no cost per commit; one model nobody priced. + const r = { ...report, totals: { ...report.totals, unpricedModels: { 'mystery-model': 1200 } }, outcomes: { commits: 0, usdPerCommit: null } }; + const p = buildPayload(r, OK); + assert.equal(p.outcomes.usdPerCommit, null, 'the uploader does not invent a $0 cost per commit'); + assert.equal(p.outcomes.linesAdded, null, 'a field the analyzer did not send is not measured, not zero'); + assert.equal(p.totals.unpricedTokens, 1200, 'the hub is told the dollar total is incomplete'); + const v = validate(p); + assert.equal(v.outcomes.usdPerCommit, null, 'the hub stores it as not measured'); + assert.equal(v.totals.unpricedTokens, 1200); + assert.throws(() => validate({ ...p, outcomes: { ...p.outcomes, usdPerCommit: 'free' } }), /not a number/); +}); + // ── hub over HTTP ──────────────────────────────────────────────── let server, base; before(async () => { server = createServer(handler); await new Promise(r => server.listen(0, '127.0.0.1', r)); base = `http://127.0.0.1:${server.address().port}`; }); From 095466fabbb53d5743dd4077bfe69deb5fdbe8bc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 18:03:10 +0000 Subject: [PATCH 2/5] v2.125.14: fix Bugbot findings: setup counts and digest never show unmeasured as null or $0.00; cached plugin copies are not an overlap - Hub page: unmeasured setup counts show as dashes, not the word "null". - Digest: an unmeasured week's cost is "not measured" in the body and left out of the subject, never $0.00; a finding with no share omits the percentage. - audit-setup: skills nobody here can edit (synced, or two cached versions of one plugin) are not reported as overlapping; skill-installed-twice fires once per skill name. - work-review plugin 1.1.1 (the version guard caught this change). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X2X5cmFm94J9Gz2esCZBEG --- examples/work-review-hub/digest.mjs | 8 ++++---- examples/work-review-hub/index.html | 2 +- plugins/releases.json | 4 ++-- plugins/work-review/.claude-plugin/plugin.json | 2 +- .../skills/work-review/scripts/audit-setup.mjs | 9 +++++++-- release-notes.json | 2 +- test/work-review-digest.test.js | 9 +++++++++ test/work-review-setup-audit.test.js | 4 +++- 8 files changed, 28 insertions(+), 12 deletions(-) diff --git a/examples/work-review-hub/digest.mjs b/examples/work-review-hub/digest.mjs index 94c08579..73791a47 100644 --- a/examples/work-review-hub/digest.mjs +++ b/examples/work-review-hub/digest.mjs @@ -48,16 +48,16 @@ export function digestFor(name, history, { now = new Date(), hubUrl = '' } = {}) const weekAgo = new Date(now.getTime() - 7 * 864e5).toISOString().slice(0, 10); if (!latest || latest.day < weekAgo) return null; const prev = [...history].reverse().find(s => s.day <= new Date(Date.parse(latest.day) - 6 * 864e5).toISOString().slice(0, 10)); - const cost = latest.totals.usd || 0; + const cost = latest.totals.usd ?? null; const lines = []; - lines.push(`Cost at API list prices, last 7 days: ${usd(cost)}${prev ? ` (the week before: ${usd(prev.totals.usd || 0)})` : ''}. On a seat plan this is the API-equivalent value of the work, not a bill.`); + lines.push(`Cost at API list prices, last 7 days: ${usd(cost)}${prev ? ` (the week before: ${usd(prev.totals.usd ?? null)})` : ''}. On a seat plan this is the API-equivalent value of the work, not a bill.`); if (latest.outcomes?.commits) lines.push(`Commits landed: ${latest.outcomes.commits}, ${usd(latest.outcomes.usdPerCommit)} each.`); const fixes = [ - ...(latest.findings || []).filter(f => FIXES[f.id]).map(f => `${FIXES[f.id][0]} (${usd(f.usd)}, ${Math.round(f.share * 100)}% of spend): ${FIXES[f.id][1]}.`), + ...(latest.findings || []).filter(f => FIXES[f.id]).map(f => `${FIXES[f.id][0]} (${usd(f.usd)}${f.share == null ? '' : `, ${Math.round(f.share * 100)}% of spend`}): ${FIXES[f.id][1]}.`), ...Object.keys(latest.setup?.findings || {}).filter(k => SETUP_FIXES[k]).map(k => `${SETUP_FIXES[k]}.`), ].slice(0, 3); const first = (name || '').split(/[\s@]/)[0] || 'there'; - const subject = fixes.length ? `Your week with Claude: ${usd(cost)}, ${fixes.length} thing${fixes.length > 1 ? 's' : ''} to try` : `Your week with Claude: ${usd(cost)}`; + const subject = fixes.length ? `Your week with Claude: ${cost == null ? '' : `${usd(cost)}, `}${fixes.length} thing${fixes.length > 1 ? 's' : ''} to try` : `Your week with Claude${cost == null ? '' : `: ${usd(cost)}`}`; const footer = 'Only you get this email. Turn it off on your Work Review page.'; const text = [`Hi ${first},`, '', ...lines, '', fixes.length ? 'Worth trying this week:' : 'Nothing stood out this week. Nice.', ...fixes.map((f, i) => `${i + 1}. ${f}`), '', hubUrl ? `Your page: ${hubUrl}` : 'Open Work Review in AppCrane for the details.', '', footer].join('\n'); diff --git a/examples/work-review-hub/index.html b/examples/work-review-hub/index.html index 8d89ef6e..3df878e1 100644 --- a/examples/work-review-hub/index.html +++ b/examples/work-review-hub/index.html @@ -150,7 +150,7 @@

Team

const SU = r.setup; if (SU) { const fk = Object.entries(SU.findings || {}); - oc.append(el('div', `Setup: ~${k(SU.alwaysOnTokens)} tokens of CLAUDE.md on every request · ${SU.agents} subagents · ${SU.skills} skills · ${SU.mcpServers} MCP servers · ${SU.hooks} hooks${SU.multiAgentFrameworks ? ` · multi-agent code in this project` : ''}`, 'mute')); + oc.append(el('div', `Setup: ${SU.alwaysOnTokens == null ? '—' : '~' + k(SU.alwaysOnTokens)} tokens of CLAUDE.md on every request · ${n0(SU.agents)} subagents · ${n0(SU.skills)} skills · ${n0(SU.mcpServers)} MCP servers · ${n0(SU.hooks)} hooks${SU.multiAgentFrameworks ? ` · multi-agent code in this project` : ''}`, 'mute')); const sf = el('div'); sf.style.margin = '6px 0'; fk.length ? fk.forEach(([f, n]) => sf.append(el('span', `${f} ×${n}`, 'tag'))) : sf.append(el('span', 'setup: no findings', 'tag')); oc.append(sf); diff --git a/plugins/releases.json b/plugins/releases.json index 2bb10505..ca08b2c8 100644 --- a/plugins/releases.json +++ b/plugins/releases.json @@ -1,6 +1,6 @@ { "work-review": { - "version": "1.1.0", - "hash": "ea0af9291b9114433511a0b5ae819f1a42eaf91d2c1d15b6a8169dc40e217f7f" + "version": "1.1.1", + "hash": "d97615955fbb4468ae98d50f32b6d3f70a244cae8eb2b49aadecd2f0a916ad5d" } } diff --git a/plugins/work-review/.claude-plugin/plugin.json b/plugins/work-review/.claude-plugin/plugin.json index 87755ede..93f1e0cf 100644 --- a/plugins/work-review/.claude-plugin/plugin.json +++ b/plugins/work-review/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "work-review", - "version": "1.1.0", + "version": "1.1.1", "description": "Reviews how you work with Claude Code: what each prompt cost and why, waste and overcomplicated setups, with concrete fixes. Optional daily upload of a redacted summary to a Work Review hub.", "author": { "name": "AppCrane" }, "repository": "https://github.com/gitayg/appCrane", diff --git a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs index 60738126..e58603ce 100644 --- a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs +++ b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs @@ -196,17 +196,22 @@ const skills = skillFiles.map(({ f, scope }) => { // say — loads twice and competes for the same requests, and a plugin update // moves only one of them: the copies drift with nothing saying so. A symlink to // the same file is one install, not two. -const twice = new Set(); +const twice = new Set(), reported = new Set(); for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { const a = skills[i], b = skills[j]; if (a.real === b.real || a.scope === b.scope || a.name !== b.name) continue; twice.add(i + ':' + j); + // One finding per skill name, however many cached plugin versions it has. + if (reported.has(a.name)) continue; + reported.add(a.name); const copy = a.scope === 'plugin' ? b : a, other = copy === a ? b : a; add('skill-installed-twice', 'Skills', 'medium', a.name, `installed at ${copy.scope} scope (${copy.file}) and ${other.scope} scope (${other.file}); both load and compete, and updating one leaves the other behind`, other.scope === 'plugin' ? `Delete the ${copy.scope} copy and keep the plugin's, so updates reach it` : 'Keep one copy: delete the other, or link it to the first', { files: [a.file, b.file] }); } for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { - if (twice.has(i + ':' + j) || skills[i].real === skills[j].real) continue; + // Two skills nobody here can edit (synced, or plugin copies such as two cached + // versions of one plugin) are not the user's overlap to fix. + if (twice.has(i + ':' + j) || skills[i].real === skills[j].real || (skills[i].managed && skills[j].managed)) continue; const sim = jaccard(shingles(skills[i].description), shingles(skills[j].description)); if (sim >= LIMITS.similarDescriptions) add('skill-overlap', 'Skills', 'medium', `${skills[i].name} / ${skills[j].name}`, `descriptions overlap ${Math.round(sim * 100)}%`, 'Merge them or sharpen when each applies', {}); } diff --git a/release-notes.json b/release-notes.json index 727e34d4..53b5d0f0 100644 --- a/release-notes.json +++ b/release-notes.json @@ -2,7 +2,7 @@ "2.125.14": [ "Work Review shows a figure it could not measure as a dash, never as zero: a week with no commits no longer shows $0.00 per commit, and when some usage comes from a model without a known price, the cost says it is incomplete.", "The work-review setup audit flags a skill installed twice, such as your own copy beside the same skill from a plugin: both load and compete, and a plugin update only moves one of them.", - "The work-review plugin is now version 1.1.0, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." + "The work-review plugin is now version 1.1.1, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." ], "2.125.13": [ "The Dashboard's visitor and CPU charts label each day correctly for people west of UTC (in the Americas every day showed as the day before)." diff --git a/test/work-review-digest.test.js b/test/work-review-digest.test.js index dbe3fa22..d4c401c5 100644 --- a/test/work-review-digest.test.js +++ b/test/work-review-digest.test.js @@ -50,6 +50,15 @@ test('the digest says the week\'s cost, the change, and the top fixes', () => { assert.equal(isoWeek(MONDAY), '2026-W44'); }); +test('a week whose cost was not measured is never mailed as $0.00', () => { + const history = [{ day: '2026-10-18', totals: { usd: null } }, + { day: '2026-10-25', totals: { usd: null }, findings: [{ id: 'rework', usd: null, share: null }] }]; + const m = digestFor('Kim', history, { now: MONDAY }); + assert.doesNotMatch(m.subject + m.text, /\$0\.00|0% of spend|null/); + assert.match(m.text, /not measured/); + assert.match(m.subject, /^Your week with Claude: 1 thing to try$/); +}); + test('sent on Monday only, once a week, and never to someone who switched it off', async () => { const f = [{ id: 'rework', usd: 3, share: 0.2 }]; assert.equal((await upload('kim@opswat.com', '2026-10-25', report(20, f))).status, 201); diff --git a/test/work-review-setup-audit.test.js b/test/work-review-setup-audit.test.js index 89324164..c2308d00 100644 --- a/test/work-review-setup-audit.test.js +++ b/test/work-review-setup-audit.test.js @@ -117,6 +117,8 @@ test('a skill installed both as a personal copy and from a plugin is caught; a s const skill = '---\nname: spec\ndescription: Runs the spec lifecycle from intent to release; use when starting a feature\n---\nsteps'; w(join(user, 'skills', 'spec', 'SKILL.md'), skill); w(join(user, 'plugins', 'cache', 'mkt', 'productizer', '4.73.0', 'skills', 'spec', 'SKILL.md'), skill); + // An older cached version of the same plugin: still one plugin skill, not an overlap. + w(join(user, 'plugins', 'cache', 'mkt', 'productizer', '4.72.0', 'skills', 'spec', 'SKILL.md'), skill); // The project links to the plugin's own copy: one install, not two. w(join(root, 'checkout', 'review', 'SKILL.md'), '---\nname: review\ndescription: Reviews pull requests for missing tests; use when asked to review a PR\n---\nsteps'); w(join(user, 'plugins', 'cache', 'mkt', 'tools', '1.0.0', 'skills', 'review', '.keep'), ''); @@ -126,7 +128,7 @@ test('a skill installed both as a personal copy and from a plugin is caught; a s const r = run(proj, user, null); const twice = r.findings.filter(f => f.kind === 'skill-installed-twice'); - assert.deepEqual(twice.map(f => f.item), ['spec'], `got ${kinds(r).join(', ')}`); + assert.deepEqual(twice.map(f => f.item), ['spec'], `one finding per skill, however many cached versions; got ${kinds(r).join(', ')}`); assert.match(twice[0].fix, /Delete the user copy and keep the plugin's/); assert.ok(!kinds(r).includes('skill-overlap'), 'a duplicate install is reported once, not also as an overlap'); }); From 09744aeedba1ae8f8be96999ff6450923455fd05 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 00:27:59 +0000 Subject: [PATCH 3/5] v2.125.15: skill-installed-twice groups copies by name: one finding naming every copy, plugin warning kept; managed-only pairs skipped - A skill at project, user and plugin scope was reported as a local pair only, losing the plugin update-drift warning; copies are now grouped by name and the finding names each scope. - A synced copy beside a plugin's (neither editable here) is no longer flagged. - work-review plugin 1.1.2. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X2X5cmFm94J9Gz2esCZBEG --- plugins/releases.json | 4 +-- .../work-review/.claude-plugin/plugin.json | 2 +- .../work-review/scripts/audit-setup.mjs | 27 +++++++++++-------- release-notes.json | 2 +- test/work-review-setup-audit.test.js | 15 +++++++++++ 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/plugins/releases.json b/plugins/releases.json index ca08b2c8..7167937b 100644 --- a/plugins/releases.json +++ b/plugins/releases.json @@ -1,6 +1,6 @@ { "work-review": { - "version": "1.1.1", - "hash": "d97615955fbb4468ae98d50f32b6d3f70a244cae8eb2b49aadecd2f0a916ad5d" + "version": "1.1.2", + "hash": "e385f0cd074ee9df10d70132e0b5827b5826f1b99a4440cbaa469cf42aadef50" } } diff --git a/plugins/work-review/.claude-plugin/plugin.json b/plugins/work-review/.claude-plugin/plugin.json index 93f1e0cf..81c3ccbd 100644 --- a/plugins/work-review/.claude-plugin/plugin.json +++ b/plugins/work-review/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "work-review", - "version": "1.1.1", + "version": "1.1.2", "description": "Reviews how you work with Claude Code: what each prompt cost and why, waste and overcomplicated setups, with concrete fixes. Optional daily upload of a redacted summary to a Work Review hub.", "author": { "name": "AppCrane" }, "repository": "https://github.com/gitayg/appCrane", diff --git a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs index e58603ce..73adbc61 100644 --- a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs +++ b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs @@ -196,17 +196,22 @@ const skills = skillFiles.map(({ f, scope }) => { // say — loads twice and competes for the same requests, and a plugin update // moves only one of them: the copies drift with nothing saying so. A symlink to // the same file is one install, not two. -const twice = new Set(), reported = new Set(); -for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { - const a = skills[i], b = skills[j]; - if (a.real === b.real || a.scope === b.scope || a.name !== b.name) continue; - twice.add(i + ':' + j); - // One finding per skill name, however many cached plugin versions it has. - if (reported.has(a.name)) continue; - reported.add(a.name); - const copy = a.scope === 'plugin' ? b : a, other = copy === a ? b : a; - add('skill-installed-twice', 'Skills', 'medium', a.name, `installed at ${copy.scope} scope (${copy.file}) and ${other.scope} scope (${other.file}); both load and compete, and updating one leaves the other behind`, - other.scope === 'plugin' ? `Delete the ${copy.scope} copy and keep the plugin's, so updates reach it` : 'Keep one copy: delete the other, or link it to the first', { files: [a.file, b.file] }); +// Grouped by name, so a skill at project, user and plugin scope is one finding +// that names every copy, and the plugin's drift warning is never lost. +const twice = new Set(), byName = new Map(); +skills.forEach((s, i) => { if (!byName.has(s.name)) byName.set(s.name, []); byName.get(s.name).push(i); }); +for (const [name, idx] of byName) { + const copies = [...new Map(idx.map(i => [skills[i].real, skills[i]])).values()]; + if (new Set(copies.map(c => c.scope)).size < 2) continue; + for (const i of idx) for (const j of idx) if (i < j) twice.add(i + ':' + j); + // Only copies this person can edit can be removed; two managed copies (synced + // beside a plugin's) are nobody's to fix here. + const editable = copies.filter(c => !c.managed); + if (!editable.length) continue; + const plugin = copies.find(c => c.scope === 'plugin'); + const where = copies.filter(c => c.scope !== 'plugin' || c === plugin).map(c => `${c.scope} scope (${c.file})`).join(', '); + add('skill-installed-twice', 'Skills', 'medium', name, `installed at ${where}; every copy loads and competes${plugin ? ', and a plugin update moves only the plugin\'s copy' : ''}`, + plugin ? `Delete the ${[...new Set(editable.map(c => c.scope))].join(' and ')} cop${editable.length > 1 ? 'ies' : 'y'} and keep the plugin's, so updates reach it` : 'Keep one copy: delete the others, or link them to one', { files: copies.map(c => c.file) }); } for (let i = 0; i < skills.length; i++) for (let j = i + 1; j < skills.length; j++) { // Two skills nobody here can edit (synced, or plugin copies such as two cached diff --git a/release-notes.json b/release-notes.json index cf80c088..14a06020 100644 --- a/release-notes.json +++ b/release-notes.json @@ -2,7 +2,7 @@ "2.125.15": [ "Work Review shows a figure it could not measure as a dash, never as zero: a week with no commits no longer shows $0.00 per commit, and when some usage comes from a model without a known price, the cost says it is incomplete.", "The work-review setup audit flags a skill installed twice, such as your own copy beside the same skill from a plugin: both load and compete, and a plugin update only moves one of them.", - "The work-review plugin is now version 1.1.1, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." + "The work-review plugin is now version 1.1.2, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." ], "2.125.14": [ "Hosted apps can trust X-AppCrane-Auth-Via (which says a request came from a script using a personal app sign-in key). The identity signature did not cover it, so on a route that skips the proxy it could be removed to make a script look like a person in a browser. A second signature, X-AppCrane-Identity-Sig-V2, now covers it; appcrane-tenant 1.3.0 checks it and returns authVia. Apps on older appcrane-tenant keep working unchanged.", diff --git a/test/work-review-setup-audit.test.js b/test/work-review-setup-audit.test.js index c2308d00..20bde92d 100644 --- a/test/work-review-setup-audit.test.js +++ b/test/work-review-setup-audit.test.js @@ -132,3 +132,18 @@ test('a skill installed both as a personal copy and from a plugin is caught; a s assert.match(twice[0].fix, /Delete the user copy and keep the plugin's/); assert.ok(!kinds(r).includes('skill-overlap'), 'a duplicate install is reported once, not also as an overlap'); }); + +test('a skill at project, user and plugin scope is one finding that keeps the plugin warning; two managed copies are not flagged', () => { + const root = mkdtempSync(join(tmpdir(), 'audit-')); + const proj = join(root, 'proj'), user = join(root, 'user'); + mkdirSync(join(root, 'home'), { recursive: true }); + const md = n => `---\nname: ${n}\ndescription: Runs the ${n} lifecycle from intent to release; use when starting a feature\n---\nsteps`; + for (const d of [join(proj, '.claude', 'skills', 'spec'), join(user, 'skills', 'spec'), join(user, 'plugins', 'cache', 'mkt', 'p', '1.0.0', 'skills', 'spec')]) w(join(d, 'SKILL.md'), md('spec')); + // An org-synced copy beside a plugin's: neither is editable here. + for (const d of [join(user, 'skills', 'synced', 'org', 'sdk'), join(user, 'plugins', 'cache', 'mkt', 'p', '1.0.0', 'skills', 'sdk')]) w(join(d, 'SKILL.md'), md('sdk')); + const r = run(proj, user, null); + const twice = r.findings.filter(f => f.kind === 'skill-installed-twice'); + assert.deepEqual(twice.map(f => f.item), ['spec'], `got ${kinds(r).join(', ')}`); + assert.match(twice[0].finding, /project scope.*user scope.*plugin scope.*plugin update moves only/); + assert.match(twice[0].fix, /Delete the project and user copies and keep the plugin's/); +}); From 68669eacf1e75bcd6f65ea9f5e2c62698c8c6142 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 00:35:31 +0000 Subject: [PATCH 4/5] v2.125.15: skill-installed-twice names the real file, not a symlink to it When a project skill was also linked from user scope, the copies collapsed onto the link, and the advice said to delete the user copy, which would remove only the link. The real location now wins. Only the skill's own folder or file counts as a link, so a symlinked parent such as /tmp on macOS doesn't. work-review plugin 1.1.3. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X2X5cmFm94J9Gz2esCZBEG --- plugins/releases.json | 4 ++-- plugins/work-review/.claude-plugin/plugin.json | 2 +- .../skills/work-review/scripts/audit-setup.mjs | 8 ++++++-- release-notes.json | 2 +- test/work-review-setup-audit.test.js | 18 ++++++++++++++++++ 5 files changed, 28 insertions(+), 6 deletions(-) diff --git a/plugins/releases.json b/plugins/releases.json index 7167937b..420bdfa1 100644 --- a/plugins/releases.json +++ b/plugins/releases.json @@ -1,6 +1,6 @@ { "work-review": { - "version": "1.1.2", - "hash": "e385f0cd074ee9df10d70132e0b5827b5826f1b99a4440cbaa469cf42aadef50" + "version": "1.1.3", + "hash": "a8c03135e513374b8815c5b5e7570905fa6214ab56ae797e8f8263bc167bfb22" } } diff --git a/plugins/work-review/.claude-plugin/plugin.json b/plugins/work-review/.claude-plugin/plugin.json index 81c3ccbd..95a762e1 100644 --- a/plugins/work-review/.claude-plugin/plugin.json +++ b/plugins/work-review/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "work-review", - "version": "1.1.2", + "version": "1.1.3", "description": "Reviews how you work with Claude Code: what each prompt cost and why, waste and overcomplicated setups, with concrete fixes. Optional daily upload of a redacted summary to a Work Review hub.", "author": { "name": "AppCrane" }, "repository": "https://github.com/gitayg/appCrane", diff --git a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs index 73adbc61..2266f673 100644 --- a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs +++ b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs @@ -176,7 +176,7 @@ const skills = skillFiles.map(({ f, scope }) => { const text = read(f) || ''; const { fm } = frontmatter(text); const name = fm?.name || basename(dirname(f)); - const s = { name, scope, file: rel(f), real: real(f), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; + const s = { name, scope, file: rel(f), real: real(f), linked: real(f) !== join(real(dirname(dirname(f))), basename(dirname(f)), basename(f)), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; // Skills synced from an organisation or plugin are inventoried, not judged: // the person running the review cannot edit them. s.managed = /[\\/](synced|plugins)[\\/]/.test(f); @@ -201,7 +201,11 @@ const skills = skillFiles.map(({ f, scope }) => { const twice = new Set(), byName = new Map(); skills.forEach((s, i) => { if (!byName.has(s.name)) byName.set(s.name, []); byName.get(s.name).push(i); }); for (const [name, idx] of byName) { - const copies = [...new Map(idx.map(i => [skills[i].real, skills[i]])).values()]; + // One entry per real file. Where a file is also reached through a symlink, + // name the real location: deleting the link would leave the file installed. + const byReal = new Map(); + for (const i of idx) { const c = skills[i], had = byReal.get(c.real); if (!had || (had.linked && !c.linked)) byReal.set(c.real, c); } + const copies = [...byReal.values()]; if (new Set(copies.map(c => c.scope)).size < 2) continue; for (const i of idx) for (const j of idx) if (i < j) twice.add(i + ':' + j); // Only copies this person can edit can be removed; two managed copies (synced diff --git a/release-notes.json b/release-notes.json index 14a06020..d9207ddd 100644 --- a/release-notes.json +++ b/release-notes.json @@ -2,7 +2,7 @@ "2.125.15": [ "Work Review shows a figure it could not measure as a dash, never as zero: a week with no commits no longer shows $0.00 per commit, and when some usage comes from a model without a known price, the cost says it is incomplete.", "The work-review setup audit flags a skill installed twice, such as your own copy beside the same skill from a plugin: both load and compete, and a plugin update only moves one of them.", - "The work-review plugin is now version 1.1.2, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." + "The work-review plugin is now version 1.1.3, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." ], "2.125.14": [ "Hosted apps can trust X-AppCrane-Auth-Via (which says a request came from a script using a personal app sign-in key). The identity signature did not cover it, so on a route that skips the proxy it could be removed to make a script look like a person in a browser. A second signature, X-AppCrane-Identity-Sig-V2, now covers it; appcrane-tenant 1.3.0 checks it and returns authVia. Apps on older appcrane-tenant keep working unchanged.", diff --git a/test/work-review-setup-audit.test.js b/test/work-review-setup-audit.test.js index 20bde92d..e4db6293 100644 --- a/test/work-review-setup-audit.test.js +++ b/test/work-review-setup-audit.test.js @@ -147,3 +147,21 @@ test('a skill at project, user and plugin scope is one finding that keeps the pl assert.match(twice[0].finding, /project scope.*user scope.*plugin scope.*plugin update moves only/); assert.match(twice[0].fix, /Delete the project and user copies and keep the plugin's/); }); + +test('when a project skill is also linked from user scope, the finding names the real file, not the link', async () => { + const { symlinkSync } = await import('fs'); + const root = mkdtempSync(join(tmpdir(), 'audit-')); + const proj = join(root, 'proj'), user = join(root, 'user'); + mkdirSync(join(root, 'home'), { recursive: true }); + const md = '---\nname: spec\ndescription: Runs the spec lifecycle from intent to release; use when starting a feature\n---\nsteps'; + w(join(proj, '.claude', 'skills', 'spec', 'SKILL.md'), md); + mkdirSync(join(user, 'skills'), { recursive: true }); + symlinkSync(join(proj, '.claude', 'skills', 'spec'), join(user, 'skills', 'spec')); + w(join(user, 'plugins', 'cache', 'mkt', 'p', '1.0.0', 'skills', 'spec', 'SKILL.md'), md); + const r = run(proj, user, null); + const f = r.findings.find(x => x.kind === 'skill-installed-twice'); + assert.ok(f, `got ${kinds(r).join(', ')}`); + assert.match(f.finding, /project scope/); + assert.doesNotMatch(f.finding, /user scope/, 'the symlink is not reported as a separate copy'); + assert.match(f.fix, /Delete the project copy/); +}); From b5ddad631eb33b1c81db73875539fbe35c057955 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 30 Oct 2026 00:36:53 +0000 Subject: [PATCH 5/5] v2.125.15: detect a linked skill with lstat on its own folder or file, not a path join (Semgrep path-join alerts); work-review plugin 1.1.4 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01X2X5cmFm94J9Gz2esCZBEG --- plugins/releases.json | 4 ++-- plugins/work-review/.claude-plugin/plugin.json | 2 +- .../work-review/skills/work-review/scripts/audit-setup.mjs | 6 ++++-- release-notes.json | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/plugins/releases.json b/plugins/releases.json index 420bdfa1..6da15b5e 100644 --- a/plugins/releases.json +++ b/plugins/releases.json @@ -1,6 +1,6 @@ { "work-review": { - "version": "1.1.3", - "hash": "a8c03135e513374b8815c5b5e7570905fa6214ab56ae797e8f8263bc167bfb22" + "version": "1.1.4", + "hash": "22061fa102d238ee478654105b4b3c75349a296c84a42f48c8226d283d8a1783" } } diff --git a/plugins/work-review/.claude-plugin/plugin.json b/plugins/work-review/.claude-plugin/plugin.json index 95a762e1..cf865ae8 100644 --- a/plugins/work-review/.claude-plugin/plugin.json +++ b/plugins/work-review/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "work-review", - "version": "1.1.3", + "version": "1.1.4", "description": "Reviews how you work with Claude Code: what each prompt cost and why, waste and overcomplicated setups, with concrete fixes. Optional daily upload of a redacted summary to a Work Review hub.", "author": { "name": "AppCrane" }, "repository": "https://github.com/gitayg/appCrane", diff --git a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs index 2266f673..c7fc74e0 100644 --- a/plugins/work-review/skills/work-review/scripts/audit-setup.mjs +++ b/plugins/work-review/skills/work-review/scripts/audit-setup.mjs @@ -11,7 +11,7 @@ // // Read-only. Prints a markdown summary; --json writes the findings. -import { readdirSync, readFileSync, statSync, writeFileSync, existsSync, realpathSync } from 'fs'; +import { readdirSync, readFileSync, statSync, writeFileSync, existsSync, realpathSync, lstatSync } from 'fs'; import { join, relative, basename, dirname } from 'path'; import { homedir } from 'os'; @@ -172,11 +172,13 @@ const skillFiles = [[join(PROJECT, '.claude', 'skills'), 'project', 4], [join(US .filter(([d]) => isDir(d)) .flatMap(([d, scope, depth]) => walk(d, (p, n) => n === 'SKILL.md', depth).map(f => ({ f, scope: scope === 'user' && /[\\/]synced[\\/]/.test(f) ? 'synced' : scope }))); const real = f => { try { return realpathSync(f); } catch { return f; } }; +// The skill's own folder or file is a link (a symlinked parent such as /tmp is not). +const isLink = f => { try { return lstatSync(f).isSymbolicLink(); } catch { return false; } }; const skills = skillFiles.map(({ f, scope }) => { const text = read(f) || ''; const { fm } = frontmatter(text); const name = fm?.name || basename(dirname(f)); - const s = { name, scope, file: rel(f), real: real(f), linked: real(f) !== join(real(dirname(dirname(f))), basename(dirname(f)), basename(f)), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; + const s = { name, scope, file: rel(f), real: real(f), linked: isLink(f) || isLink(dirname(f)), description: fm?.description || '', lines: lines(text).length, used: usedCount(usage?.skills, name) }; // Skills synced from an organisation or plugin are inventoried, not judged: // the person running the review cannot edit them. s.managed = /[\\/](synced|plugins)[\\/]/.test(f); diff --git a/release-notes.json b/release-notes.json index d9207ddd..9eded4f9 100644 --- a/release-notes.json +++ b/release-notes.json @@ -2,7 +2,7 @@ "2.125.15": [ "Work Review shows a figure it could not measure as a dash, never as zero: a week with no commits no longer shows $0.00 per commit, and when some usage comes from a model without a known price, the cost says it is incomplete.", "The work-review setup audit flags a skill installed twice, such as your own copy beside the same skill from a plugin: both load and compete, and a plugin update only moves one of them.", - "The work-review plugin is now version 1.1.3, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." + "The work-review plugin is now version 1.1.4, so people who installed it receive these changes; a test now fails whenever the plugin changes without a version bump." ], "2.125.14": [ "Hosted apps can trust X-AppCrane-Auth-Via (which says a request came from a script using a personal app sign-in key). The identity signature did not cover it, so on a route that skips the proxy it could be removed to make a script look like a person in a browser. A second signature, X-AppCrane-Identity-Sig-V2, now covers it; appcrane-tenant 1.3.0 checks it and returns authVia. Apps on older appcrane-tenant keep working unchanged.",