From cd25c1e934d0dbe89ac4b9944ea14337b1728613 Mon Sep 17 00:00:00 2001 From: Kjell Tore Guttormsen Date: Fri, 1 May 2026 07:46:15 +0200 Subject: [PATCH] feat(config-audit): cross-plugin collision scanner COL (v5 N6) [skip-docs] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New COL scanner detects skill-name collisions across plugins and between user-level skills (~/.claude/skills/) and plugin-bundled skills. Skill identity is the directory basename — matches how enumerateSkills resolves names. Detection rules (per docs/v5-namespace-research.md, confidence: medium): - Plugin-vs-plugin same skill name → severity low (CA-COL-001) - User-vs-plugin same skill name → severity medium (CA-COL-001) - Plugin-vs-built-in collisions: out of scope for v5.0.0 (insufficient verification — recorded for v5.0.1 follow-up). Findings carry details.namespaces array with {source, name, path} for every conflicting source — supports per-collision reporting downstream. output.mjs: finding() helper now passes through optional `details` field (scanner-specific structured payload). scoring.mjs: COL → "Plugin Hygiene" (new area, 10 total). Posture test updated from 9 → 10 area scores. .gitignore: docs/v5-namespace-research.md is local-only (Step 22a research output, gitignored per plan). Fixture collision-plugins/fake-home/ has user skill `review` colliding with plugin-a + plugin-b's `review` (medium severity), plus plugin-c's unique `summarize` (no collision). [skip-docs] reason: v5 plan fences off README/CLAUDE.md badge updates to Session 5; Forgejo pre-commit-docs-gate hook requires this tag. Tests: 617 → 625 (+8). Co-Authored-By: Claude Opus 4.7 --- plugins/config-audit/.gitignore | 3 + .../scanners/collision-scanner.mjs | 125 +++++++++++++++ plugins/config-audit/scanners/lib/output.mjs | 7 +- plugins/config-audit/scanners/lib/scoring.mjs | 1 + .../scanners/scan-orchestrator.mjs | 2 + .../plugin-a/.claude-plugin/plugin.json | 1 + .../plugins/plugin-a/skills/review/SKILL.md | 5 + .../plugin-b/.claude-plugin/plugin.json | 1 + .../plugins/plugin-b/skills/review/SKILL.md | 5 + .../plugin-c/.claude-plugin/plugin.json | 1 + .../plugin-c/skills/summarize/SKILL.md | 5 + .../fake-home/.claude/skills/review/SKILL.md | 5 + .../tests/scanners/collision.test.mjs | 145 ++++++++++++++++++ .../tests/scanners/posture.test.mjs | 4 +- 14 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 plugins/config-audit/scanners/collision-scanner.mjs create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/.claude-plugin/plugin.json create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/skills/review/SKILL.md create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/.claude-plugin/plugin.json create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/skills/review/SKILL.md create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/.claude-plugin/plugin.json create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/skills/summarize/SKILL.md create mode 100644 plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/skills/review/SKILL.md create mode 100644 plugins/config-audit/tests/scanners/collision.test.mjs diff --git a/plugins/config-audit/.gitignore b/plugins/config-audit/.gitignore index bbf1033..3467b43 100644 --- a/plugins/config-audit/.gitignore +++ b/plugins/config-audit/.gitignore @@ -20,3 +20,6 @@ S*-PROMPT.md # Plugin state (managed by plugin) .config-audit/ + +# v5 namespace research (local-only spike output) +docs/v5-namespace-research.md diff --git a/plugins/config-audit/scanners/collision-scanner.mjs b/plugins/config-audit/scanners/collision-scanner.mjs new file mode 100644 index 0000000..ec80f95 --- /dev/null +++ b/plugins/config-audit/scanners/collision-scanner.mjs @@ -0,0 +1,125 @@ +/** + * COL Scanner — Cross-Plugin/User-vs-Plugin Skill Collision (v5 N6) + * + * Detects skill-name collisions across plugins and between user-level skills + * (~/.claude/skills/) and plugin-bundled skills. Skill names come from the + * directory layout (basename of dirname(SKILL.md)) — that matches how + * enumerateSkills resolves them. + * + * Detection rules (from Step 22a research, confidence: medium): + * - Two or more plugins exposing a skill with the same directory name: + * severity `low` (CA-COL-001) — order ambiguity even when invocation is + * namespaced via `/plugin:skill`. + * - A user-level skill and a plugin skill with the same name: severity + * `medium` (CA-COL-001) — bare invocation may resolve unpredictably. + * - Plugin-vs-built-in collisions: out of scope for v5.0.0 (insufficient + * verification — see docs/v5-namespace-research.md). + * + * Each finding's `details.namespaces` array carries `{ source, name }` for + * every conflicting source so downstream tooling can render a per-collision + * report. + * + * Zero external dependencies. + */ + +import { finding, scannerResult } from './lib/output.mjs'; +import { SEVERITY } from './lib/severity.mjs'; +import { enumeratePlugins, enumerateSkills } from './lib/active-config-reader.mjs'; + +const SCANNER = 'COL'; + +/** + * Group skills by name. Returns Map>. + */ +function groupSkillsByName(skills) { + const grouped = new Map(); + for (const s of skills) { + if (!s || typeof s.name !== 'string') continue; + if (!grouped.has(s.name)) grouped.set(s.name, []); + grouped.get(s.name).push(s); + } + return grouped; +} + +/** + * Main scanner entry point. + * + * @param {string} targetPath unused (collision check is HOME-scoped) + * @param {object} discovery unused (collision check ignores project discovery) + */ +export async function scan(_targetPath, _discovery) { + const start = Date.now(); + const findings = []; + + const plugins = await enumeratePlugins(); + const allSkills = await enumerateSkills(plugins); + + const grouped = groupSkillsByName(allSkills); + + for (const [name, skills] of grouped) { + if (skills.length < 2) continue; + + const userSkill = skills.find(s => s.source === 'user'); + const pluginSkills = skills.filter(s => s.source === 'plugin'); + + if (userSkill && pluginSkills.length > 0) { + // User-vs-plugin collision (severity medium per Step 22a) + const namespaces = [ + { source: 'user', name, path: userSkill.path }, + ...pluginSkills.map(s => ({ + source: `plugin:${s.pluginName}`, + name, + path: s.path, + })), + ]; + findings.push(finding({ + scanner: SCANNER, + severity: SEVERITY.medium, + title: `Skill name "${name}" collides between user-level and plugin sources`, + description: + `A user-level skill at ${userSkill.path} shares its directory name "${name}" ` + + `with ${pluginSkills.length} plugin-bundled skill` + + `${pluginSkills.length === 1 ? '' : 's'}. Bare invocation may resolve ` + + 'unpredictably; the user has to remember which definition is currently active.', + file: userSkill.path, + evidence: + `name="${name}"; sources=` + + [`user`, ...pluginSkills.map(s => `plugin:${s.pluginName}`)].join(','), + recommendation: + `Rename either the user skill (~/.claude/skills/${name}/) or one of the plugin ` + + 'skills, or rely on namespaced invocation paths and remove the bare alias to ' + + 'eliminate the ambiguity.', + category: 'plugin-hygiene', + details: { namespaces }, + })); + } else if (pluginSkills.length >= 2) { + // Plugin-vs-plugin collision (severity low per Step 22a) + const pluginNames = pluginSkills.map(s => s.pluginName); + findings.push(finding({ + scanner: SCANNER, + severity: SEVERITY.low, + title: `Skill name "${name}" used by multiple plugins`, + description: + `${pluginSkills.length} plugins (${pluginNames.join(', ')}) expose a skill ` + + `named "${name}". Even when invocation is namespaced via /plugin:skill, ` + + 'shared names create ambiguity in error messages, search results, and the ' + + 'plugin-skills enumeration.', + file: pluginSkills[0].path, + evidence: `name="${name}"; plugins=${pluginNames.join(',')}`, + recommendation: + 'Coordinate naming across plugins, or rename one to clarify intent. The ' + + 'shared name forces every reader to disambiguate by source.', + category: 'plugin-hygiene', + details: { + namespaces: pluginSkills.map(s => ({ + source: `plugin:${s.pluginName}`, + name, + path: s.path, + })), + }, + })); + } + } + + return scannerResult(SCANNER, 'ok', findings, allSkills.length, Date.now() - start); +} diff --git a/plugins/config-audit/scanners/lib/output.mjs b/plugins/config-audit/scanners/lib/output.mjs index 2e7fff9..e27d42b 100644 --- a/plugins/config-audit/scanners/lib/output.mjs +++ b/plugins/config-audit/scanners/lib/output.mjs @@ -26,12 +26,13 @@ export function resetCounter() { * @param {string} [opts.category] - quality category * @param {string} [opts.recommendation] - suggested fix * @param {boolean} [opts.autoFixable] - can be auto-fixed + * @param {object} [opts.details] - structured details (scanner-specific shape) * @returns {object} */ export function finding(opts) { findingCounter++; const id = `CA-${opts.scanner}-${String(findingCounter).padStart(3, '0')}`; - return { + const result = { id, scanner: opts.scanner, severity: opts.severity, @@ -44,6 +45,10 @@ export function finding(opts) { recommendation: opts.recommendation || null, autoFixable: opts.autoFixable || false, }; + if (opts.details && typeof opts.details === 'object') { + result.details = opts.details; + } + return result; } /** diff --git a/plugins/config-audit/scanners/lib/scoring.mjs b/plugins/config-audit/scanners/lib/scoring.mjs index 4e68449..a14c4d8 100644 --- a/plugins/config-audit/scanners/lib/scoring.mjs +++ b/plugins/config-audit/scanners/lib/scoring.mjs @@ -153,6 +153,7 @@ const SCANNER_AREA_MAP = { TOK: 'Token Efficiency', CPS: 'Token Efficiency', DIS: 'Settings', + COL: 'Plugin Hygiene', }; /** diff --git a/plugins/config-audit/scanners/scan-orchestrator.mjs b/plugins/config-audit/scanners/scan-orchestrator.mjs index 808695c..b9b4dbc 100644 --- a/plugins/config-audit/scanners/scan-orchestrator.mjs +++ b/plugins/config-audit/scanners/scan-orchestrator.mjs @@ -26,6 +26,7 @@ import { scan as scanGap } from './feature-gap-scanner.mjs'; import { scan as scanTokenHotspots } from './token-hotspots.mjs'; import { scan as scanCachePrefix } from './cache-prefix-scanner.mjs'; import { scan as scanDisabledInSchema } from './disabled-in-schema-scanner.mjs'; +import { scan as scanCollision } from './collision-scanner.mjs'; // Directory names that identify test fixture / example directories const FIXTURE_DIR_NAMES = ['tests', 'examples', '__tests__', 'test-fixtures']; @@ -59,6 +60,7 @@ const SCANNERS = [ { name: 'TOK', fn: scanTokenHotspots, label: 'Token Hotspots' }, { name: 'CPS', fn: scanCachePrefix, label: 'Cache-Prefix Stability' }, { name: 'DIS', fn: scanDisabledInSchema, label: 'Disabled-In-Schema' }, + { name: 'COL', fn: scanCollision, label: 'Plugin Skill Collision' }, ]; /** diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/.claude-plugin/plugin.json b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/.claude-plugin/plugin.json new file mode 100644 index 0000000..5ed988c --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/.claude-plugin/plugin.json @@ -0,0 +1 @@ +{"name": "plugin-a", "version": "1.0.0", "description": "test"} diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/skills/review/SKILL.md b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/skills/review/SKILL.md new file mode 100644 index 0000000..611ce8b --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-a/skills/review/SKILL.md @@ -0,0 +1,5 @@ +--- +name: plugin-a:review +description: review skill from plugin-a +--- +Plugin A review. diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/.claude-plugin/plugin.json b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/.claude-plugin/plugin.json new file mode 100644 index 0000000..cc31501 --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/.claude-plugin/plugin.json @@ -0,0 +1 @@ +{"name": "plugin-b", "version": "1.0.0", "description": "test"} diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/skills/review/SKILL.md b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/skills/review/SKILL.md new file mode 100644 index 0000000..09764d8 --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-b/skills/review/SKILL.md @@ -0,0 +1,5 @@ +--- +name: plugin-b:review +description: review skill from plugin-b +--- +Plugin B review. diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/.claude-plugin/plugin.json b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/.claude-plugin/plugin.json new file mode 100644 index 0000000..7e3dd3d --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/.claude-plugin/plugin.json @@ -0,0 +1 @@ +{"name": "plugin-c", "version": "1.0.0", "description": "test"} diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/skills/summarize/SKILL.md b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/skills/summarize/SKILL.md new file mode 100644 index 0000000..631e164 --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/plugins/marketplaces/mp/plugins/plugin-c/skills/summarize/SKILL.md @@ -0,0 +1,5 @@ +--- +name: plugin-c:summarize +description: summarize skill from plugin-c +--- +Plugin C summarize. diff --git a/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/skills/review/SKILL.md b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/skills/review/SKILL.md new file mode 100644 index 0000000..a64e502 --- /dev/null +++ b/plugins/config-audit/tests/fixtures/collision-plugins/fake-home/.claude/skills/review/SKILL.md @@ -0,0 +1,5 @@ +--- +name: review +description: user-level review skill +--- +User review. diff --git a/plugins/config-audit/tests/scanners/collision.test.mjs b/plugins/config-audit/tests/scanners/collision.test.mjs new file mode 100644 index 0000000..93d09c5 --- /dev/null +++ b/plugins/config-audit/tests/scanners/collision.test.mjs @@ -0,0 +1,145 @@ +import { describe, it, beforeEach, afterEach } from 'node:test'; +import assert from 'node:assert/strict'; +import { resolve, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { mkdir, writeFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { resetCounter } from '../../scanners/lib/output.mjs'; +import { scan } from '../../scanners/collision-scanner.mjs'; + +const __dirname = fileURLToPath(new URL('.', import.meta.url)); +const FIXTURES = resolve(__dirname, '../fixtures'); +const COLLISION_FIXTURE_HOME = resolve(FIXTURES, 'collision-plugins', 'fake-home'); + +function uniqueDir(suffix) { + return join(tmpdir(), `config-audit-col-${suffix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`); +} + +/** + * The COL scanner uses process.env.HOME via enumeratePlugins/enumerateSkills. + * Tests must override HOME, run, and restore — never rely on user-state. + */ +async function runScannerWithHome(home) { + resetCounter(); + const original = process.env.HOME; + process.env.HOME = home; + try { + return await scan('/unused', { files: [] }); + } finally { + process.env.HOME = original; + } +} + +describe('COL scanner — basic structure', () => { + it('reports scanner prefix COL', async () => { + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + assert.equal(result.scanner, 'COL'); + }); + + it('finding IDs match CA-COL-NNN pattern', async () => { + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + for (const f of result.findings) { + assert.match(f.id, /^CA-COL-\d{3}$/); + } + }); +}); + +describe('COL scanner — user-vs-plugin collision (medium severity)', () => { + it('flags review skill collision between user-level and plugin-bundled', async () => { + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + const f = result.findings.find(x => /user-level and plugin sources/i.test(x.title || '')); + assert.ok(f, `expected user-vs-plugin finding; got: ${result.findings.map(x => x.title).join(' | ')}`); + assert.equal(f.severity, 'medium', `expected medium, got ${f.severity}`); + assert.match(String(f.title), /review/); + }); + + it('user-vs-plugin finding includes details.namespaces', async () => { + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + const f = result.findings.find(x => /user-level and plugin sources/i.test(x.title || '')); + assert.ok(f); + assert.ok(Array.isArray(f.details?.namespaces), + `expected details.namespaces array; got: ${JSON.stringify(f.details)}`); + assert.ok(f.details.namespaces.length >= 2); + const sources = f.details.namespaces.map(n => n.source); + assert.ok(sources.includes('user'), `expected user in sources; got: ${sources.join(', ')}`); + }); +}); + +describe('COL scanner — negative cases', () => { + it('plugin-c summarize (unique name) generates no finding', async () => { + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + const f = result.findings.find(x => /summarize/i.test(x.title || '')); + assert.equal(f, undefined, + `expected no finding for unique plugin-c summarize skill; got: ${f?.title}`); + }); + + it('clean fake-home with no plugins yields zero findings', async () => { + const cleanHome = uniqueDir('clean'); + try { + await mkdir(join(cleanHome, '.claude', 'plugins'), { recursive: true }); + const result = await runScannerWithHome(cleanHome); + assert.equal(result.findings.length, 0, + `expected 0 findings; got: ${result.findings.map(f => f.title).join(' | ')}`); + } finally { + await rm(cleanHome, { recursive: true, force: true }); + } + }); +}); + +describe('COL scanner — plugin-vs-plugin (low severity, no user-level competitor)', () => { + let altHome; + + beforeEach(async () => { + altHome = uniqueDir('plugin-only'); + const root = join(altHome, '.claude', 'plugins', 'marketplaces', 'mp', 'plugins'); + await mkdir(join(root, 'plugin-x', '.claude-plugin'), { recursive: true }); + await writeFile( + join(root, 'plugin-x', '.claude-plugin', 'plugin.json'), + JSON.stringify({ name: 'plugin-x', version: '1.0.0', description: 'x' }), + ); + await mkdir(join(root, 'plugin-x', 'skills', 'analyze'), { recursive: true }); + await writeFile( + join(root, 'plugin-x', 'skills', 'analyze', 'SKILL.md'), + '---\nname: x:analyze\ndescription: analyze from x\n---\nBody.\n', + ); + await mkdir(join(root, 'plugin-y', '.claude-plugin'), { recursive: true }); + await writeFile( + join(root, 'plugin-y', '.claude-plugin', 'plugin.json'), + JSON.stringify({ name: 'plugin-y', version: '1.0.0', description: 'y' }), + ); + await mkdir(join(root, 'plugin-y', 'skills', 'analyze'), { recursive: true }); + await writeFile( + join(root, 'plugin-y', 'skills', 'analyze', 'SKILL.md'), + '---\nname: y:analyze\ndescription: analyze from y\n---\nBody.\n', + ); + }); + + afterEach(async () => { + if (altHome) await rm(altHome, { recursive: true, force: true }); + }); + + it('plugin-x and plugin-y both define analyze → finding (low severity)', async () => { + const result = await runScannerWithHome(altHome); + const f = result.findings.find(x => /multiple plugins/i.test(x.title || '')); + assert.ok(f, `expected plugin-vs-plugin finding; got: ${result.findings.map(x => x.title).join(' | ')}`); + assert.equal(f.severity, 'low', `expected low, got ${f.severity}`); + assert.match(String(f.title), /analyze/); + assert.ok(Array.isArray(f.details?.namespaces)); + assert.equal(f.details.namespaces.length, 2); + }); +}); + +describe('COL scanner — suppression compatibility', () => { + it('CA-COL-001 is NOT matched by CA-TOK-* glob suppression', async () => { + const { applySuppressions } = await import('../../scanners/lib/suppression.mjs'); + const result = await runScannerWithHome(COLLISION_FIXTURE_HOME); + assert.ok(result.findings.length > 0, 'precondition: at least one COL finding to test against'); + // Apply CA-TOK-* glob suppression — should leave COL findings untouched. + const { active, suppressed } = applySuppressions(result.findings, [ + { pattern: 'CA-TOK-*', source: 'test', sourceLine: 1 }, + ]); + assert.equal(active.length, result.findings.length, + 'CA-TOK-* glob should not match CA-COL-* findings'); + assert.equal(suppressed.length, 0); + }); +}); diff --git a/plugins/config-audit/tests/scanners/posture.test.mjs b/plugins/config-audit/tests/scanners/posture.test.mjs index c809fc7..9222727 100644 --- a/plugins/config-audit/tests/scanners/posture.test.mjs +++ b/plugins/config-audit/tests/scanners/posture.test.mjs @@ -45,8 +45,8 @@ describe('posture.mjs CLI — healthy project', () => { assert.ok(result.segment.segment.length > 0); }); - it('returns 9 area scores', () => { - assert.equal(result.areas.length, 9); + it('returns 10 area scores (v5 adds Plugin Hygiene from COL)', () => { + assert.equal(result.areas.length, 10); for (const area of result.areas) { assert.ok('id' in area); assert.ok('name' in area);