mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
* test: guard the plugin loader contract that PR #494 exposed The agents manifest key shipped for months and silently loaded zero of the four subagents; no validator looked at the generated plugin manifest's shape and claude plugin validate never checks it. Three layers now do: - scripts/lib/validate-plugin-manifest.js pins the verified loader contract (KNOWN_LOADER_KEYS allowlist, no agents key, trailing-slash skills path from issue #86, every skill/agents/*.md shipped in plugin/agents/), unit-tested in tests/validate-plugin-manifest.test.js including a check of the real committed subtree. - The same check gates bun run build next to the version-drift guard. - tests/plugin-e2e.test.mjs installs the committed ./plugin subtree into a real Claude Code (sandboxed via CLAUDE_CONFIG_DIR in a temp dir) and asserts the component inventory: skill parses, all agents visible, hooks discovered. In the default suite; runs in about a second and skips cleanly when the claude CLI is absent, so CI is unaffected. All three failed against the pre-#494 tree for the shipped reason (Agents 0 of 4) and pass against current main. AI-assisted via Claude Code under maintainer direction. Co-Authored-By: Claude Code <noreply@anthropic.com> * fix: address PR review bot findings - Copilot: guard collectPluginManifestFindings against valid JSON that is not an object (null, string, number, array) so a broken manifest is a finding instead of a build crash; unit test added - Copilot: update the plugin-e2e header comment, the suite is in the default lineup rather than opt-in AI-assisted via Claude Code under maintainer direction. Co-Authored-By: Claude Code <noreply@anthropic.com> * fix: harden plugin E2E sandbox isolation Bugbot: create the sandbox CLAUDE_CONFIG_DIR up front and redirect HOME and USERPROFILE into the temp workDir too, so a CLI code path that derives config or cache locations from the home directory instead of CLAUDE_CONFIG_DIR still cannot touch the developer's real Claude config when the default suite runs. AI-assisted via Claude Code under maintainer direction. Co-Authored-By: Claude Code <noreply@anthropic.com> * fix: agent parity check mirrors the build's emit rules Bugbot: the shipped filename is `${claude-name || name}.md` and a providers: list may exclude claude-code, so comparing raw source basenames could fail the build on a renamed or provider-scoped agent with a build:release hint that cannot fix it. The validator now derives expected filenames the same way the transformer factory does (shared parseFrontmatter, same providers gate) with unit coverage for renames, name overrides, and provider-scoped agents. AI-assisted via Claude Code under maintainer direction. Co-Authored-By: Claude Code <noreply@anthropic.com> * fix: run the plugin E2E through a shell on Windows Bugbot: the claude CLI is a .cmd shim on Windows and Node refuses to spawn those via execFile without a shell, so the availability probe always failed and the suite silently skipped there. Windows now invokes through a shell with every argument double-quoted (temp paths routinely contain spaces); the POSIX path is unchanged. AI-assisted via Claude Code under maintainer direction. Co-Authored-By: Claude Code <noreply@anthropic.com> --------- Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
co-authored by
Claude Code
Claude Code
parent
69b63d36b6
commit
6d2af3f800
@@ -136,13 +136,16 @@ This contribution was prepared by an AI agent that tried to ship unchecked vibes
|
||||
## Testing
|
||||
|
||||
```bash
|
||||
bun run test # Default suite: unit + static framework fixtures
|
||||
bun run test # Default suite: unit + static framework fixtures + plugin loader E2E
|
||||
bun run test:live-e2e # Opt-in: full-cycle live-mode E2E across framework fixtures
|
||||
bun run test:skill-behavior # Opt-in: LLM-backed checks that the skill text actually drives the agent's setup flow
|
||||
bun run test:plugin-e2e # Just the plugin loader E2E (also part of the default suite)
|
||||
```
|
||||
|
||||
Unit tests (build orchestration, detector logic) run via `bun test`. Fixture tests (jsdom-based HTML detection) run via `node --test` because bun is too slow with jsdom. The `test` script handles this split automatically.
|
||||
|
||||
**Plugin loader E2E** (`tests/plugin-e2e.test.mjs`, in the default suite): installs the committed `./plugin` subtree into a real Claude Code, sandboxed via `CLAUDE_CONFIG_DIR` in a temp dir, and asserts the component inventory from `claude plugin details`: the skill parses, every `plugin/agents/*.md` is visible, hooks are discovered. This is the only check that catches loader-contract surprises the unit guards can't know about (PR #494 shipped an `agents` manifest key that silently loaded zero agents; `claude plugin validate` never flags plugin-manifest problems). Runs in about a second; skips cleanly when the `claude` CLI is not on PATH. The known contract itself (allowed manifest keys, no `agents` key, trailing-slash `skills` path, source agents shipped) is pinned deterministically by `scripts/lib/validate-plugin-manifest.js`, unit-tested in `tests/validate-plugin-manifest.test.js` and enforced as a `bun run build` gate. Never add a key to the generated plugin manifest without verifying it against a real install and extending `KNOWN_LOADER_KEYS`.
|
||||
|
||||
**Important:** `tests/build.test.js` uses `spyOn(transformers, 'transformCursor')` with the named exports from `scripts/lib/transformers/index.js`. Those named exports (`transformCursor`, `transformClaudeCode`, etc.) are kept specifically for test spying, even though `build.js` itself uses `createTransformer + PROVIDERS` directly. **Do not delete them as "dead code"** — I made that mistake once and broke 8 tests.
|
||||
|
||||
### Live-mode E2E
|
||||
|
||||
@@ -56,6 +56,7 @@
|
||||
"test:live": "node scripts/run-tests.mjs live",
|
||||
"test:cli-e2e": "node scripts/run-tests.mjs cli-e2e",
|
||||
"test:cli-remote-e2e": "node scripts/run-tests.mjs cli-remote-e2e",
|
||||
"test:plugin-e2e": "node scripts/run-tests.mjs plugin-e2e",
|
||||
"test:live-e2e": "node scripts/run-tests.mjs live-e2e",
|
||||
"test:live-e2e-accept-cleanup": "node scripts/run-tests.mjs live-e2e-accept-cleanup",
|
||||
"test:new-work-e2e": "node scripts/run-tests.mjs new-work-e2e",
|
||||
|
||||
+27
-1
@@ -23,6 +23,7 @@ import { createTransformer, PROVIDERS } from './lib/transformers/index.js';
|
||||
import { hooksJsonFor, buildClaudePluginHooksManifest } from './lib/transformers/hooks.js';
|
||||
import { createAllZips, createProviderZip } from './lib/zip.js';
|
||||
import { collectPluginVersions } from './lib/validate-plugin-versions.js';
|
||||
import { collectPluginManifestFindings } from './lib/validate-plugin-manifest.js';
|
||||
import { stageOpenAIPlugin } from './lib/openai-plugin.js';
|
||||
import { ANTIPATTERNS } from '../cli/engine/registry/antipatterns.mjs';
|
||||
// Sub-page generation is now handled by Astro content collections.
|
||||
@@ -140,6 +141,27 @@ function validatePluginVersions(rootDir) {
|
||||
return total;
|
||||
}
|
||||
|
||||
/**
|
||||
* Guard against unverified plugin manifest shapes (PR #494). The pure check
|
||||
* lives in ./lib/validate-plugin-manifest.js (so it's unit-tested directly);
|
||||
* this wrapper owns the console output and the error count the build gates on.
|
||||
*/
|
||||
function validatePluginManifestShape(rootDir) {
|
||||
const findings = collectPluginManifestFindings(rootDir);
|
||||
for (const { relPath, reason } of findings) {
|
||||
console.error(` ❌ ${relPath}: ${reason}`);
|
||||
}
|
||||
if (findings.length > 0) {
|
||||
console.error(
|
||||
`\n❌ ${findings.length} plugin manifest shape problem(s). Every manifest key is a ` +
|
||||
'claim about the Claude Code loader; see scripts/lib/validate-plugin-manifest.js (PR #494).',
|
||||
);
|
||||
} else {
|
||||
console.log('✓ Plugin manifest shape matches the verified loader contract');
|
||||
}
|
||||
return findings.length;
|
||||
}
|
||||
|
||||
function validateSkillFrontmatter(skills) {
|
||||
let errors = 0;
|
||||
|
||||
@@ -704,6 +726,10 @@ async function build() {
|
||||
// match root plugin.json so marketplace installs never ship a stale version.
|
||||
const versionErrors = validatePluginVersions(ROOT_DIR);
|
||||
|
||||
// Guard the generated plugin manifest's shape: a key the Claude Code loader
|
||||
// does not honor (like the agents array, PR #494) ships silently broken.
|
||||
const manifestShapeErrors = validatePluginManifestShape(ROOT_DIR);
|
||||
|
||||
// Scan user-facing copy for AI tells (em dashes, marketing fluff, denylisted phrases)
|
||||
const proseErrors = validateProse(ROOT_DIR);
|
||||
|
||||
@@ -711,7 +737,7 @@ async function build() {
|
||||
// that has no technical reading. Hardening repetition is intentionally allowed.
|
||||
const skillProseErrors = validateSkillProse(ROOT_DIR);
|
||||
|
||||
if (countErrors > 0 || versionErrors > 0 || proseErrors > 0 || skillProseErrors > 0) {
|
||||
if (countErrors > 0 || versionErrors > 0 || manifestShapeErrors > 0 || proseErrors > 0 || skillProseErrors > 0) {
|
||||
process.exit(1);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,162 @@
|
||||
/**
|
||||
* Plugin manifest shape validation (PR #494).
|
||||
*
|
||||
* The Claude Code marketplace installs from the committed `./plugin` subtree,
|
||||
* and every key in its `.claude-plugin/plugin.json` is a claim about how
|
||||
* Claude Code's plugin loader behaves. Those claims are not checked by
|
||||
* `claude plugin validate` (it validates the marketplace manifest, not the
|
||||
* plugin manifest), and a wrong one fails silently: shipping an `agents` key
|
||||
* as an array of file paths made Claude Code load ZERO agents, while the
|
||||
* loader auto-discovers `agents/*.md` on its own when the key is absent. The
|
||||
* four shipped subagents were unreachable for every marketplace install and
|
||||
* nothing flagged it.
|
||||
*
|
||||
* This guard pins the verified contract:
|
||||
*
|
||||
* - `KNOWN_LOADER_KEYS` is the set of keys confirmed to load correctly in a
|
||||
* real Claude Code install (checked via the `claude plugin details`
|
||||
* component inventory). A key outside the set fails the build until
|
||||
* someone verifies it end to end and adds it here with a note.
|
||||
* - The `agents` key must never appear. Omission is the only shape that
|
||||
* works: an array of file paths loads zero agents, and a string or a
|
||||
* directory entry fails the whole plugin.
|
||||
* - Because omission means the files themselves are the only carrier, every
|
||||
* agent in `skill/agents/` that ships to claude-code must have its emitted
|
||||
* copy in `plugin/agents/`. The expected filename follows the build's emit
|
||||
* rules (`claude-name` / `name` frontmatter override the source basename,
|
||||
* and a `providers:` list may exclude claude-code entirely).
|
||||
* - `skills` must keep the trailing-slash `./skills/` form (issue #86: the
|
||||
* bare form fails to register slash commands).
|
||||
*
|
||||
* The collector is pure (filesystem-in, data-out) so it can be unit-tested
|
||||
* against fixtures; build.js owns the logging and the non-zero exit.
|
||||
*/
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { parseFrontmatter } from './utils.js';
|
||||
|
||||
/**
|
||||
* Keys verified against a live Claude Code install (2026-08, Claude Code
|
||||
* 2.1.220). Descriptive metadata keys pass through harmlessly; `skills` is
|
||||
* the one component path the loader reads from this manifest. Do not add a
|
||||
* component key (`agents`, `hooks`, `commands`, `mcpServers`, ...) without
|
||||
* installing the built subtree and confirming the component inventory in
|
||||
* `claude plugin details` counts every shipped piece.
|
||||
*/
|
||||
export const KNOWN_LOADER_KEYS = [
|
||||
'name',
|
||||
'description',
|
||||
'version',
|
||||
'author',
|
||||
'homepage',
|
||||
'repository',
|
||||
'skills',
|
||||
];
|
||||
|
||||
function listMarkdown(dir) {
|
||||
if (!fs.existsSync(dir)) return [];
|
||||
return fs.readdirSync(dir).filter((file) => file.endsWith('.md')).sort();
|
||||
}
|
||||
|
||||
/**
|
||||
* Which agent files the claude-code build actually emits, mirroring
|
||||
* readSourceFiles (name and providers parsing) and the transformer factory
|
||||
* (`${claudeName || name}.md`, providers gate). A drift between this and the
|
||||
* build shows up as a false finding, so change them together.
|
||||
*/
|
||||
function expectedClaudeAgentFiles(rootDir) {
|
||||
const agentsDir = path.join(rootDir, 'skill', 'agents');
|
||||
const expected = [];
|
||||
for (const sourceFile of listMarkdown(agentsDir)) {
|
||||
const { frontmatter } = parseFrontmatter(
|
||||
fs.readFileSync(path.join(agentsDir, sourceFile), 'utf-8'),
|
||||
);
|
||||
const providersRaw = frontmatter.providers;
|
||||
const providers = Array.isArray(providersRaw)
|
||||
? providersRaw.map((p) => String(p).trim()).filter(Boolean)
|
||||
: typeof providersRaw === 'string' && providersRaw.trim()
|
||||
? providersRaw.split(',').map((p) => p.trim()).filter(Boolean)
|
||||
: null;
|
||||
if (providers && !providers.includes('claude-code')) continue;
|
||||
const name = frontmatter.name || path.basename(sourceFile, '.md');
|
||||
expected.push({ sourceFile, shippedFile: `${frontmatter['claude-name'] || name}.md` });
|
||||
}
|
||||
return expected;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate the generated plugin manifest's shape against the verified loader
|
||||
* contract.
|
||||
*
|
||||
* @param {string} rootDir repository root
|
||||
* @returns {Array<{relPath:string, reason:string}>}
|
||||
* Empty when the subtree is absent (nothing generated yet) or fully valid.
|
||||
*/
|
||||
export function collectPluginManifestFindings(rootDir) {
|
||||
const manifestRel = 'plugin/.claude-plugin/plugin.json';
|
||||
const manifestPath = path.join(rootDir, manifestRel);
|
||||
if (!fs.existsSync(manifestPath)) return [];
|
||||
|
||||
let manifest;
|
||||
try {
|
||||
manifest = JSON.parse(fs.readFileSync(manifestPath, 'utf-8'));
|
||||
} catch (err) {
|
||||
return [{ relPath: manifestRel, reason: `could not parse (${err.message})` }];
|
||||
}
|
||||
// JSON.parse accepts null, strings, numbers, and arrays; the key checks
|
||||
// below need a plain object, so anything else is a finding, not a crash.
|
||||
if (manifest === null || typeof manifest !== 'object' || Array.isArray(manifest)) {
|
||||
return [{
|
||||
relPath: manifestRel,
|
||||
reason: `manifest is ${Array.isArray(manifest) ? 'an array' : manifest === null ? 'null' : `a ${typeof manifest}`}, not a JSON object`,
|
||||
}];
|
||||
}
|
||||
|
||||
const findings = [];
|
||||
|
||||
if ('agents' in manifest) {
|
||||
findings.push({
|
||||
relPath: manifestRel,
|
||||
reason:
|
||||
'declares an "agents" key; Claude Code loads zero agents when it is present ' +
|
||||
'as file paths and fails the whole plugin on other shapes. Omit it and let ' +
|
||||
'the loader auto-discover plugin/agents/*.md (PR #494)',
|
||||
});
|
||||
}
|
||||
|
||||
for (const key of Object.keys(manifest)) {
|
||||
if (key === 'agents') continue; // already reported with the specific fix
|
||||
if (!KNOWN_LOADER_KEYS.includes(key)) {
|
||||
findings.push({
|
||||
relPath: manifestRel,
|
||||
reason:
|
||||
`unverified manifest key "${key}"; confirm it loads in a real Claude Code ` +
|
||||
'install, then add it to KNOWN_LOADER_KEYS in scripts/lib/validate-plugin-manifest.js',
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
if (manifest.skills !== './skills/') {
|
||||
findings.push({
|
||||
relPath: manifestRel,
|
||||
reason:
|
||||
`"skills" is ${JSON.stringify(manifest.skills)}; must be "./skills/" ` +
|
||||
'(trailing-slash form, issue #86)',
|
||||
});
|
||||
}
|
||||
|
||||
// With no `agents` key, shipped files are the only thing the loader sees.
|
||||
const shippedAgents = listMarkdown(path.join(rootDir, 'plugin', 'agents'));
|
||||
for (const { sourceFile, shippedFile } of expectedClaudeAgentFiles(rootDir)) {
|
||||
if (!shippedAgents.includes(shippedFile)) {
|
||||
findings.push({
|
||||
relPath: `plugin/agents/${shippedFile}`,
|
||||
reason:
|
||||
'missing from the plugin subtree; auto-discovery relies on the shipped ' +
|
||||
`file, so skill/agents/${sourceFile} would never load. Run \`bun run build:release\``,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return findings;
|
||||
}
|
||||
+21
-1
@@ -1,7 +1,7 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
export const DEFAULT_SUITES = ['core', 'detector', 'live', 'framework'];
|
||||
export const DEFAULT_SUITES = ['core', 'detector', 'live', 'framework', 'plugin-e2e'];
|
||||
export const OPT_IN_SUITES = [
|
||||
'cli-remote-e2e',
|
||||
'live-e2e',
|
||||
@@ -47,6 +47,7 @@ export const SUITES = {
|
||||
'tests/lib/transformers/providers.test.js',
|
||||
'tests/skills-cli.test.js',
|
||||
'tests/validate-plugin-versions.test.js',
|
||||
'tests/validate-plugin-manifest.test.js',
|
||||
],
|
||||
},
|
||||
{
|
||||
@@ -212,6 +213,25 @@ export const SUITES = {
|
||||
},
|
||||
],
|
||||
},
|
||||
'plugin-e2e': {
|
||||
description: 'Install the committed ./plugin subtree into a real (sandboxed) Claude Code and assert skills, agents, and hooks all load. Skips when the claude CLI is not on PATH.',
|
||||
triggers: [
|
||||
...COMMON_INFRA_PATTERNS,
|
||||
/^plugin\//,
|
||||
/^skill\/agents\//,
|
||||
/^scripts\/build\.js$/,
|
||||
/^scripts\/lib\/validate-plugin-manifest\.js$/,
|
||||
/^tests\/plugin-e2e\.test\.mjs$/,
|
||||
],
|
||||
commands: [
|
||||
{
|
||||
runner: 'node',
|
||||
timeoutMs: 300000,
|
||||
forceExit: true,
|
||||
files: ['tests/plugin-e2e.test.mjs'],
|
||||
},
|
||||
],
|
||||
},
|
||||
'live-e2e': {
|
||||
description: 'Full Playwright live-mode click-to-accept sweep across runtime framework fixtures.',
|
||||
optIn: true,
|
||||
|
||||
@@ -0,0 +1,143 @@
|
||||
/**
|
||||
* Plugin loader E2E: install the committed ./plugin subtree into a real
|
||||
* Claude Code and assert every shipped component actually loads. Part of the
|
||||
* default suite; the only external requirement is the claude CLI, and the
|
||||
* suite skips cleanly when it is not on PATH.
|
||||
*
|
||||
* The unit-level shape guard (tests/validate-plugin-manifest.test.js) pins the
|
||||
* loader contract we KNOW about. This suite is the only thing that catches the
|
||||
* contract being wrong: the `agents` manifest key shipped for months and loaded
|
||||
* zero of the four subagents (PR #494), `claude plugin validate` never flagged
|
||||
* it, and earlier releases had SKILL.md frontmatter that failed to parse. All
|
||||
* of those are invisible until the real loader reports its component
|
||||
* inventory, which is exactly what this suite asserts on.
|
||||
*
|
||||
* Isolation: every claude invocation runs with CLAUDE_CONFIG_DIR, HOME, and
|
||||
* USERPROFILE all pointed into a fresh temp dir, so the developer's real
|
||||
* config, marketplaces, and installed plugins are never touched even if the
|
||||
* CLI derives a path from the home directory rather than CLAUDE_CONFIG_DIR.
|
||||
* Requires the `claude` CLI on PATH; skips cleanly otherwise.
|
||||
*
|
||||
* Run with: bun run test:plugin-e2e
|
||||
*/
|
||||
import { describe, it, before, after } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { execFileSync, execSync } from 'node:child_process';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..');
|
||||
const PLUGIN_DIR = path.join(REPO_ROOT, 'plugin');
|
||||
const MARKETPLACE_NAME = 'impeccable-e2e';
|
||||
const PLUGIN_REF = `impeccable@${MARKETPLACE_NAME}`;
|
||||
|
||||
// On Windows the claude CLI is a .cmd shim, which Node refuses to spawn
|
||||
// without a shell, so commands there go through one with every argument
|
||||
// double-quoted (paths under %TEMP% routinely contain spaces). Elsewhere
|
||||
// execFileSync runs the binary directly with no quoting concerns.
|
||||
const IS_WINDOWS = process.platform === 'win32';
|
||||
const quoteForCmd = (arg) => `"${String(arg).replace(/"/g, '""')}"`;
|
||||
const runClaude = (args, opts) =>
|
||||
IS_WINDOWS
|
||||
? execSync(['claude', ...args.map(quoteForCmd)].join(' '), opts)
|
||||
: execFileSync('claude', args, opts);
|
||||
|
||||
const claudeAvailable = (() => {
|
||||
try {
|
||||
runClaude(['--version'], { stdio: 'ignore', timeout: 30000 });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
})();
|
||||
|
||||
const skip = claudeAvailable ? false : 'claude CLI not on PATH; install Claude Code to run the plugin E2E suite';
|
||||
|
||||
describe('committed plugin subtree loads in a real Claude Code', { skip }, () => {
|
||||
let workDir;
|
||||
let configDir;
|
||||
let detailsOutput;
|
||||
|
||||
const claude = (...args) =>
|
||||
runClaude(args, {
|
||||
encoding: 'utf-8',
|
||||
timeout: 120000,
|
||||
// Neutral cwd so the repo's own .claude/ project config cannot leak in.
|
||||
cwd: workDir,
|
||||
// Belt and suspenders: CLAUDE_CONFIG_DIR is the documented isolation
|
||||
// switch, but any path the CLI derives from the home directory instead
|
||||
// must also land in the sandbox, so HOME/USERPROFILE point there too.
|
||||
env: {
|
||||
...process.env,
|
||||
CLAUDE_CONFIG_DIR: configDir,
|
||||
HOME: workDir,
|
||||
USERPROFILE: workDir,
|
||||
},
|
||||
});
|
||||
|
||||
before(() => {
|
||||
workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-plugin-e2e-'));
|
||||
configDir = path.join(workDir, 'claude-config');
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
const marketplaceDir = path.join(workDir, 'marketplace');
|
||||
fs.mkdirSync(path.join(marketplaceDir, '.claude-plugin'), { recursive: true });
|
||||
fs.cpSync(PLUGIN_DIR, path.join(marketplaceDir, 'impeccable'), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(marketplaceDir, '.claude-plugin', 'marketplace.json'),
|
||||
JSON.stringify({
|
||||
name: MARKETPLACE_NAME,
|
||||
owner: { name: 'impeccable plugin E2E' },
|
||||
plugins: [
|
||||
{ name: 'impeccable', source: './impeccable', description: 'committed ./plugin subtree' },
|
||||
],
|
||||
}, null, 2),
|
||||
);
|
||||
|
||||
claude('plugin', 'marketplace', 'add', marketplaceDir);
|
||||
claude('plugin', 'install', PLUGIN_REF);
|
||||
detailsOutput = claude('plugin', 'details', PLUGIN_REF);
|
||||
});
|
||||
|
||||
after(() => {
|
||||
if (workDir) fs.rmSync(workDir, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
// `claude plugin details` prints "Skills (N) name, name" lines; long name
|
||||
// lists wrap, so counts come from the header and names are matched anywhere.
|
||||
const componentCount = (component) => {
|
||||
const match = detailsOutput.match(new RegExp(`${component}\\s+\\((\\d+)\\)`));
|
||||
assert.ok(match, `component inventory has no "${component} (N)" line:\n${detailsOutput}`);
|
||||
return Number(match[1]);
|
||||
};
|
||||
|
||||
it('reports the plugin as installed rather than silently absent', () => {
|
||||
// A manifest the loader rejects surfaces as "Plugin not found" here, with
|
||||
// no validation error anywhere else. Reaching this assertion at all means
|
||||
// the details call above did not throw.
|
||||
assert.match(detailsOutput, /Component inventory/);
|
||||
});
|
||||
|
||||
it('parses and loads the impeccable skill', () => {
|
||||
assert.equal(componentCount('Skills'), 1);
|
||||
assert.match(detailsOutput, /Skills\s+\(1\)\s+impeccable/);
|
||||
});
|
||||
|
||||
it('loads every shipped agent via auto-discovery (PR #494 regression)', () => {
|
||||
const shipped = fs.readdirSync(path.join(PLUGIN_DIR, 'agents'))
|
||||
.filter((file) => file.endsWith('.md'))
|
||||
.map((file) => file.replace(/\.md$/, ''));
|
||||
assert.ok(shipped.length > 0, 'plugin/agents/ ships no agent files');
|
||||
assert.equal(componentCount('Agents'), shipped.length);
|
||||
for (const name of shipped) {
|
||||
assert.ok(detailsOutput.includes(name), `agent "${name}" missing from inventory`);
|
||||
}
|
||||
});
|
||||
|
||||
it('discovers the packaged hooks', () => {
|
||||
assert.equal(componentCount('Hooks'), 2);
|
||||
assert.match(detailsOutput, /PostToolUse/);
|
||||
assert.match(detailsOutput, /Stop/);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,194 @@
|
||||
/**
|
||||
* Unit coverage for the plugin manifest shape guard (PR #494).
|
||||
*
|
||||
* The bug this exists to catch: the build emitted an `agents` array into the
|
||||
* generated plugin manifest, and that key made Claude Code load zero of the
|
||||
* four shipped subagents. No validator looked at the manifest's shape, so the
|
||||
* defect shipped in every release since the subagents were added. The guard
|
||||
* pins the verified loader contract (KNOWN_LOADER_KEYS) so any new key fails
|
||||
* the build until it has been confirmed against a real Claude Code install.
|
||||
*/
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
import fs from 'fs';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import {
|
||||
collectPluginManifestFindings,
|
||||
KNOWN_LOADER_KEYS,
|
||||
} from '../scripts/lib/validate-plugin-manifest.js';
|
||||
|
||||
const REPO_ROOT = path.resolve(import.meta.dir, '..');
|
||||
|
||||
const GOOD_MANIFEST = {
|
||||
name: 'impeccable',
|
||||
description: 'Test plugin',
|
||||
version: '4.0.4',
|
||||
author: { name: 'Paul Bakaus' },
|
||||
homepage: 'https://impeccable.style',
|
||||
repository: 'https://github.com/pbakaus/impeccable',
|
||||
skills: './skills/',
|
||||
};
|
||||
|
||||
function writeFixture(root, { manifest = GOOD_MANIFEST, sourceAgents = [], shippedAgents } = {}) {
|
||||
const write = (rel, contents) => {
|
||||
const abs = path.join(root, rel);
|
||||
fs.mkdirSync(path.dirname(abs), { recursive: true });
|
||||
fs.writeFileSync(abs, contents);
|
||||
};
|
||||
if (manifest !== undefined) {
|
||||
write(
|
||||
'plugin/.claude-plugin/plugin.json',
|
||||
typeof manifest === 'string' ? manifest : JSON.stringify(manifest, null, 2),
|
||||
);
|
||||
}
|
||||
// Source agents may be plain filenames or { file, frontmatter } for tests
|
||||
// that exercise the build's emit rules (claude-name, name, providers).
|
||||
for (const agent of sourceAgents) {
|
||||
const { file, frontmatter = '' } = typeof agent === 'string' ? { file: agent } : agent;
|
||||
write(`skill/agents/${file}`, `---\n${frontmatter}${frontmatter ? '\n' : ''}description: t\n---\nBody.\n`);
|
||||
}
|
||||
const shipped = shippedAgents ?? sourceAgents.map((a) => (typeof a === 'string' ? a : a.file));
|
||||
for (const file of shipped) write(`plugin/agents/${file}`, '---\ndescription: t\n---\nBody.\n');
|
||||
}
|
||||
|
||||
describe('collectPluginManifestFindings', () => {
|
||||
let root;
|
||||
beforeEach(() => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-manifest-'));
|
||||
});
|
||||
afterEach(() => {
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('a clean manifest with shipped agents produces no findings', () => {
|
||||
writeFixture(root, { sourceAgents: ['reviewer.md', 'producer.md'] });
|
||||
expect(collectPluginManifestFindings(root)).toEqual([]);
|
||||
});
|
||||
|
||||
test('flags an agents key given as an array of file paths (the shipped bug)', () => {
|
||||
writeFixture(root, {
|
||||
manifest: { ...GOOD_MANIFEST, agents: ['./agents/reviewer.md', './agents/producer.md'] },
|
||||
sourceAgents: ['reviewer.md', 'producer.md'],
|
||||
});
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].relPath).toBe('plugin/.claude-plugin/plugin.json');
|
||||
expect(findings[0].reason).toMatch(/"agents" key/);
|
||||
expect(findings[0].reason).toMatch(/zero agents/);
|
||||
});
|
||||
|
||||
test('flags an agents key of any other shape too', () => {
|
||||
writeFixture(root, { manifest: { ...GOOD_MANIFEST, agents: './agents' } });
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings.some((f) => f.reason.match(/"agents" key/))).toBe(true);
|
||||
});
|
||||
|
||||
test('flags a manifest key outside the verified loader contract', () => {
|
||||
writeFixture(root, { manifest: { ...GOOD_MANIFEST, mcpServers: './mcp.json' } });
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].reason).toMatch(/unverified manifest key "mcpServers"/);
|
||||
expect(findings[0].reason).toMatch(/KNOWN_LOADER_KEYS/);
|
||||
});
|
||||
|
||||
test('flags a skills path without the trailing slash (issue #86)', () => {
|
||||
writeFixture(root, { manifest: { ...GOOD_MANIFEST, skills: './skills' } });
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].reason).toMatch(/trailing-slash/);
|
||||
});
|
||||
|
||||
test('flags a source agent missing from the shipped subtree', () => {
|
||||
writeFixture(root, {
|
||||
sourceAgents: ['reviewer.md', 'producer.md'],
|
||||
shippedAgents: ['reviewer.md'],
|
||||
});
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].relPath).toBe('plugin/agents/producer.md');
|
||||
expect(findings[0].reason).toMatch(/never load/);
|
||||
expect(findings[0].reason).toMatch(/skill\/agents\/producer\.md/);
|
||||
});
|
||||
|
||||
test('a claude-name rename expects the emitted filename, not the source basename', () => {
|
||||
writeFixture(root, {
|
||||
sourceAgents: [{ file: 'reviewer.md', frontmatter: 'claude-name: impeccable-reviewer' }],
|
||||
shippedAgents: ['impeccable-reviewer.md'],
|
||||
});
|
||||
expect(collectPluginManifestFindings(root)).toEqual([]);
|
||||
});
|
||||
|
||||
test('a claude-name rename that is not shipped is reported under the emitted filename', () => {
|
||||
writeFixture(root, {
|
||||
sourceAgents: [{ file: 'reviewer.md', frontmatter: 'claude-name: impeccable-reviewer' }],
|
||||
shippedAgents: [],
|
||||
});
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].relPath).toBe('plugin/agents/impeccable-reviewer.md');
|
||||
expect(findings[0].reason).toMatch(/skill\/agents\/reviewer\.md/);
|
||||
});
|
||||
|
||||
test('a frontmatter name overrides the source basename like the build does', () => {
|
||||
writeFixture(root, {
|
||||
sourceAgents: [{ file: 'reviewer.md', frontmatter: 'name: custom-reviewer' }],
|
||||
shippedAgents: ['custom-reviewer.md'],
|
||||
});
|
||||
expect(collectPluginManifestFindings(root)).toEqual([]);
|
||||
});
|
||||
|
||||
test('an agent whose providers list excludes claude-code owes no shipped copy', () => {
|
||||
writeFixture(root, {
|
||||
sourceAgents: [
|
||||
{ file: 'codex-only.md', frontmatter: 'providers: codex' },
|
||||
'reviewer.md',
|
||||
],
|
||||
shippedAgents: ['reviewer.md'],
|
||||
});
|
||||
expect(collectPluginManifestFindings(root)).toEqual([]);
|
||||
});
|
||||
|
||||
test('reports every problem at once', () => {
|
||||
writeFixture(root, {
|
||||
manifest: { ...GOOD_MANIFEST, agents: ['./agents/reviewer.md'], skills: './skills', commands: './commands/' },
|
||||
sourceAgents: ['reviewer.md'],
|
||||
shippedAgents: [],
|
||||
});
|
||||
const reasons = collectPluginManifestFindings(root).map((f) => f.reason);
|
||||
expect(reasons).toHaveLength(4);
|
||||
});
|
||||
|
||||
test('an absent plugin subtree produces no findings', () => {
|
||||
expect(collectPluginManifestFindings(root)).toEqual([]);
|
||||
});
|
||||
|
||||
test('a malformed manifest is a finding, not a thrown stack', () => {
|
||||
writeFixture(root, { manifest: '{ not json' });
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].reason).toMatch(/parse/);
|
||||
});
|
||||
|
||||
test('valid JSON that is not an object is a finding, not a thrown stack', () => {
|
||||
for (const raw of ['null', '"impeccable"', '42', '["./agents/reviewer.md"]']) {
|
||||
writeFixture(root, { manifest: raw });
|
||||
const findings = collectPluginManifestFindings(root);
|
||||
expect(findings).toHaveLength(1);
|
||||
expect(findings[0].reason).toMatch(/not a JSON object/);
|
||||
}
|
||||
});
|
||||
|
||||
test('KNOWN_LOADER_KEYS never re-admits agents', () => {
|
||||
expect(KNOWN_LOADER_KEYS).not.toContain('agents');
|
||||
});
|
||||
});
|
||||
|
||||
describe('committed plugin subtree', () => {
|
||||
// The test that was missing when the agents key shipped: validate the real
|
||||
// artifact the marketplace installs, not a fixture. If this fails, the
|
||||
// committed ./plugin subtree carries a manifest shape Claude Code will not
|
||||
// load; regenerate it with `bun run build:release` after fixing build.js.
|
||||
test('the shipped manifest honors the verified loader contract', () => {
|
||||
expect(collectPluginManifestFindings(REPO_ROOT)).toEqual([]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user