mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-17 08:36:25 +03:00
Add e2e tests for skills CLI and fix symlink handling
E2e tests covering: already-installed detection, prefix rename with cross-reference updates, direct-download update fallback, and full npx-skills install flow (skipped if npx skills unavailable). Fixed prefix rename to handle npx-skills symlink layout: real dirs in .agents/ are renamed and content-prefixed, then symlinks in .claude/ are recreated to point to the renamed targets. Uses unlinkSync (not rmSync) for symlinks to directories. Added -y/--yes flag for non-interactive CI mode, --prefix= flag for headless prefix selection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
3530a5740b
commit
5bb6328ed3
+78
-38
@@ -8,7 +8,7 @@
|
||||
*/
|
||||
|
||||
import { execSync } from 'node:child_process';
|
||||
import { existsSync, readFileSync, readdirSync, statSync, mkdirSync, writeFileSync, rmSync, renameSync, createWriteStream } from 'node:fs';
|
||||
import { existsSync, readFileSync, readdirSync, statSync, lstatSync, symlinkSync, readlinkSync, unlinkSync, mkdirSync, writeFileSync, rmSync, renameSync, createWriteStream } from 'node:fs';
|
||||
import { join, resolve, dirname } from 'node:path';
|
||||
import { createInterface } from 'node:readline';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
@@ -100,34 +100,47 @@ function prefixSkillContent(content, prefix, allSkillNames) {
|
||||
return result;
|
||||
}
|
||||
|
||||
function isSkillDir(skillsDir, name) {
|
||||
// Skill entries can be real directories or symlinks to directories (npx skills uses symlinks)
|
||||
const full = join(skillsDir, name);
|
||||
try {
|
||||
return statSync(full).isDirectory() && existsSync(join(full, 'SKILL.md'));
|
||||
} catch { return false; }
|
||||
}
|
||||
|
||||
function isRealSkillDir(skillsDir, name) {
|
||||
// Only real directories, not symlinks -- renaming the real dir renames the symlink targets too
|
||||
const full = join(skillsDir, name);
|
||||
try {
|
||||
const lstat = lstatSync(full);
|
||||
return lstat.isDirectory() && !lstat.isSymbolicLink() && existsSync(join(full, 'SKILL.md'));
|
||||
} catch { return false; }
|
||||
}
|
||||
|
||||
function renameSkillsWithPrefix(root, prefix) {
|
||||
// First pass: collect all skill names across all providers (use first provider found)
|
||||
let allSkillNames = [];
|
||||
for (const d of PROVIDER_DIRS) {
|
||||
const skillsDir = join(root, d, 'skills');
|
||||
if (!existsSync(skillsDir)) continue;
|
||||
const entries = readdirSync(skillsDir, { withFileTypes: true });
|
||||
allSkillNames = entries
|
||||
.filter(e => e.isDirectory() && existsSync(join(skillsDir, e.name, 'SKILL.md')))
|
||||
.map(e => e.name);
|
||||
const entries = readdirSync(skillsDir);
|
||||
allSkillNames = entries.filter(name => isSkillDir(skillsDir, name));
|
||||
if (allSkillNames.length > 0) break;
|
||||
}
|
||||
|
||||
// Second pass: rename and prefix content
|
||||
// Second pass: rename real dirs and update their content
|
||||
let count = 0;
|
||||
for (const d of PROVIDER_DIRS) {
|
||||
const skillsDir = join(root, d, 'skills');
|
||||
if (!existsSync(skillsDir)) continue;
|
||||
try {
|
||||
const entries = readdirSync(skillsDir, { withFileTypes: true });
|
||||
for (const entry of entries) {
|
||||
if (!entry.isDirectory()) continue;
|
||||
const skillMd = join(skillsDir, entry.name, 'SKILL.md');
|
||||
if (!existsSync(skillMd)) continue;
|
||||
if (entry.name.startsWith(prefix)) continue;
|
||||
const entries = readdirSync(skillsDir);
|
||||
for (const name of entries) {
|
||||
if (name.startsWith(prefix)) continue;
|
||||
if (!isRealSkillDir(skillsDir, name)) continue;
|
||||
|
||||
const src = join(skillsDir, entry.name);
|
||||
const dest = join(skillsDir, prefix + entry.name);
|
||||
const src = join(skillsDir, name);
|
||||
const dest = join(skillsDir, prefix + name);
|
||||
|
||||
renameSync(src, dest);
|
||||
|
||||
@@ -139,11 +152,34 @@ function renameSkillsWithPrefix(root, prefix) {
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
|
||||
// Third pass: fix symlinks that now point to renamed targets (npx skills uses these)
|
||||
for (const d of PROVIDER_DIRS) {
|
||||
const skillsDir = join(root, d, 'skills');
|
||||
if (!existsSync(skillsDir)) continue;
|
||||
try {
|
||||
const entries = readdirSync(skillsDir);
|
||||
for (const name of entries) {
|
||||
if (name.startsWith(prefix)) continue;
|
||||
const full = join(skillsDir, name);
|
||||
try {
|
||||
if (!lstatSync(full).isSymbolicLink()) continue;
|
||||
const target = readlinkSync(full);
|
||||
const newTarget = target.replace(new RegExp(`/${escapeRegex(name)}$`), `/${prefix}${name}`);
|
||||
unlinkSync(full);
|
||||
symlinkSync(newTarget, join(skillsDir, prefix + name));
|
||||
} catch {}
|
||||
}
|
||||
} catch {}
|
||||
}
|
||||
|
||||
return count;
|
||||
}
|
||||
|
||||
async function install(flags) {
|
||||
const force = flags.includes('--force');
|
||||
const yes = flags.includes('-y') || flags.includes('--yes');
|
||||
const prefixFlag = flags.find(f => f.startsWith('--prefix='));
|
||||
const root = findProjectRoot();
|
||||
const existing = isAlreadyInstalled(root);
|
||||
|
||||
@@ -155,19 +191,25 @@ async function install(flags) {
|
||||
|
||||
console.log('Installing impeccable skills via npx skills...\n');
|
||||
try {
|
||||
execSync('npx skills add pbakaus/impeccable', { stdio: 'inherit' });
|
||||
execSync(`npx skills add pbakaus/impeccable${yes ? ' -y' : ''}`, { stdio: 'inherit' });
|
||||
} catch (e) {
|
||||
process.exit(e.status ?? 1);
|
||||
}
|
||||
|
||||
// Ask about prefixing
|
||||
// Ask about prefixing (skip in CI mode unless --prefix= is set)
|
||||
let prefix = '';
|
||||
console.log();
|
||||
const wantPrefix = await ask('Prefix commands to avoid conflicts? e.g. /i-audit instead of /audit (y/N) ');
|
||||
if (wantPrefix === 'y' || wantPrefix === 'yes') {
|
||||
const custom = await ask('Prefix (default: i-): ');
|
||||
prefix = custom || 'i-';
|
||||
if (prefixFlag) {
|
||||
prefix = prefixFlag.split('=')[1] || 'i-';
|
||||
} else if (!yes) {
|
||||
console.log();
|
||||
const wantPrefix = await ask('Prefix commands to avoid conflicts? e.g. /i-audit instead of /audit (y/N) ');
|
||||
if (wantPrefix === 'y' || wantPrefix === 'yes') {
|
||||
const custom = await ask('Prefix (default: i-): ');
|
||||
prefix = custom || 'i-';
|
||||
}
|
||||
}
|
||||
|
||||
if (prefix) {
|
||||
const count = renameSkillsWithPrefix(root, prefix);
|
||||
if (count > 0) {
|
||||
console.log(`\nRenamed ${count} skills with "${prefix}" prefix.`);
|
||||
@@ -193,16 +235,11 @@ function findInstalledProviders(root) {
|
||||
const found = [];
|
||||
for (const d of PROVIDER_DIRS) {
|
||||
const skillsDir = join(root, d, 'skills');
|
||||
if (existsSync(skillsDir)) {
|
||||
// Check if it has impeccable skills (look for any SKILL.md)
|
||||
try {
|
||||
const entries = readdirSync(skillsDir, { withFileTypes: true });
|
||||
const hasSkills = entries.some(e =>
|
||||
e.isDirectory() && existsSync(join(skillsDir, e.name, 'SKILL.md'))
|
||||
);
|
||||
if (hasSkills) found.push(d);
|
||||
} catch {}
|
||||
}
|
||||
if (!existsSync(skillsDir)) continue;
|
||||
try {
|
||||
const entries = readdirSync(skillsDir);
|
||||
if (entries.some(name => isSkillDir(skillsDir, name))) found.push(d);
|
||||
} catch {}
|
||||
}
|
||||
return found;
|
||||
}
|
||||
@@ -250,7 +287,8 @@ function downloadFile(url, dest) {
|
||||
});
|
||||
}
|
||||
|
||||
async function update() {
|
||||
async function update(flags = []) {
|
||||
const yes = flags.includes('-y') || flags.includes('--yes');
|
||||
// Try npx skills update first
|
||||
console.log('Checking for skills manager...');
|
||||
let noLockFile = true;
|
||||
@@ -295,12 +333,14 @@ async function update() {
|
||||
}
|
||||
if (modified.length > 10) console.log(` ... and ${modified.length - 10} more`);
|
||||
console.log();
|
||||
const ans = await ask(' Overwrite local changes? (y/N) ');
|
||||
if (ans !== 'y' && ans !== 'yes') {
|
||||
console.log('Aborted.');
|
||||
process.exit(0);
|
||||
if (!yes) {
|
||||
const ans = await ask(' Overwrite local changes? (y/N) ');
|
||||
if (ans !== 'y' && ans !== 'yes') {
|
||||
console.log('Aborted.');
|
||||
process.exit(0);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
} else if (!yes) {
|
||||
const ans = await ask(`Update skills in ${providers.length} provider folder(s)? (Y/n) `);
|
||||
if (ans === 'n' || ans === 'no') {
|
||||
console.log('Aborted.');
|
||||
@@ -382,7 +422,7 @@ export async function run(args) {
|
||||
} else if (sub === 'install') {
|
||||
await install(args.slice(1));
|
||||
} else if (sub === 'update') {
|
||||
await update();
|
||||
await update(args.slice(1));
|
||||
} else {
|
||||
console.error(`Unknown skills command: ${sub}`);
|
||||
console.error(`Run 'impeccable skills --help' for available commands.`);
|
||||
|
||||
@@ -0,0 +1,261 @@
|
||||
/**
|
||||
* End-to-end tests for `impeccable skills` subcommands.
|
||||
*
|
||||
* Creates real temp directories, runs the CLI, and verifies results.
|
||||
* Tests that require `npx skills` are skipped if it's not available.
|
||||
*/
|
||||
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
|
||||
import { execSync } from 'child_process';
|
||||
import { mkdtempSync, existsSync, readdirSync, readFileSync, mkdirSync, writeFileSync, rmSync } from 'fs';
|
||||
import { join } from 'path';
|
||||
import { tmpdir } from 'os';
|
||||
|
||||
const CLI = join(import.meta.dir, '..', 'bin', 'impeccable');
|
||||
|
||||
function run(args, opts = {}) {
|
||||
return execSync(`node ${CLI} ${args}`, {
|
||||
encoding: 'utf8',
|
||||
timeout: 60000,
|
||||
...opts,
|
||||
});
|
||||
}
|
||||
|
||||
/** Create a fake skill installation in a temp dir */
|
||||
function createFakeSkills(root, skills = ['audit', 'polish', 'teach-impeccable'], providers = ['.claude']) {
|
||||
for (const provider of providers) {
|
||||
for (const skill of skills) {
|
||||
const skillDir = join(root, provider, 'skills', skill);
|
||||
mkdirSync(skillDir, { recursive: true });
|
||||
writeFileSync(join(skillDir, 'SKILL.md'), [
|
||||
'---',
|
||||
`name: ${skill}`,
|
||||
'user-invocable: true',
|
||||
'---',
|
||||
'',
|
||||
'Run /audit first, then /polish to finish.',
|
||||
'Use the teach-impeccable skill for setup.',
|
||||
].join('\n'));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Already-installed detection ─────────────────────────────────────────────
|
||||
|
||||
describe('skills install: already-installed detection', () => {
|
||||
test('detects teach-impeccable and bails', () => {
|
||||
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-'));
|
||||
execSync('git init', { cwd: tmp });
|
||||
createFakeSkills(tmp);
|
||||
|
||||
const output = run('skills install -y', { cwd: tmp });
|
||||
expect(output).toContain('already installed');
|
||||
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}, 15000);
|
||||
|
||||
test('detects prefixed i-teach-impeccable', () => {
|
||||
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-'));
|
||||
execSync('git init', { cwd: tmp });
|
||||
|
||||
const skillDir = join(tmp, '.cursor', 'skills', 'i-teach-impeccable');
|
||||
mkdirSync(skillDir, { recursive: true });
|
||||
writeFileSync(join(skillDir, 'SKILL.md'), '---\nname: i-teach-impeccable\n---\n');
|
||||
|
||||
const output = run('skills install -y', { cwd: tmp });
|
||||
expect(output).toContain('already installed');
|
||||
|
||||
rmSync(tmp, { recursive: true, force: true });
|
||||
}, 15000);
|
||||
});
|
||||
|
||||
// ─── Prefix rename (real filesystem) ─────────────────────────────────────────
|
||||
|
||||
describe('skills install: prefix rename', () => {
|
||||
let tmp;
|
||||
|
||||
beforeAll(() => {
|
||||
tmp = mkdtempSync(join(tmpdir(), 'imp-test-pfx-'));
|
||||
createFakeSkills(tmp, ['audit', 'polish', 'teach-impeccable'], ['.claude', '.cursor']);
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (tmp) rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('renames folders with prefix', () => {
|
||||
// Write a helper script that imports and runs renameSkillsWithPrefix
|
||||
const helperScript = join(tmp, '_test_rename.mjs');
|
||||
writeFileSync(helperScript, `
|
||||
import { existsSync, readdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs';
|
||||
import { join } from 'node:path';
|
||||
|
||||
function escapeRegex(str) {
|
||||
return str.replace(/[.*+?^$\{\}()|[\\]\\\\]/g, '\\\\$&');
|
||||
}
|
||||
|
||||
function prefixSkillContent(content, prefix, allSkillNames) {
|
||||
let result = content.replace(/^name:\\s*(.+)$/m, (_, name) => 'name: ' + prefix + name.trim());
|
||||
const sorted = [...allSkillNames].sort((a, b) => b.length - a.length);
|
||||
for (const name of sorted) {
|
||||
result = result.replace(
|
||||
new RegExp('/' + '(?=' + escapeRegex(name) + '(?:[^a-zA-Z0-9_-]|$))', 'g'),
|
||||
'/' + prefix
|
||||
);
|
||||
result = result.replace(
|
||||
new RegExp('(the) ' + escapeRegex(name) + ' skill', 'gi'),
|
||||
(_, article) => article + ' ' + prefix + name + ' skill'
|
||||
);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
const DIRS = ['.claude', '.cursor'];
|
||||
const root = process.argv[2];
|
||||
const prefix = process.argv[3];
|
||||
|
||||
let allNames = [];
|
||||
for (const d of DIRS) {
|
||||
const dir = join(root, d, 'skills');
|
||||
if (!existsSync(dir)) continue;
|
||||
allNames = readdirSync(dir, { withFileTypes: true }).filter(e => e.isDirectory()).map(e => e.name);
|
||||
if (allNames.length > 0) break;
|
||||
}
|
||||
|
||||
let count = 0;
|
||||
for (const d of DIRS) {
|
||||
const dir = join(root, d, 'skills');
|
||||
if (!existsSync(dir)) continue;
|
||||
for (const entry of readdirSync(dir, { withFileTypes: true })) {
|
||||
if (!entry.isDirectory() || entry.name.startsWith(prefix)) continue;
|
||||
const skillMd = join(dir, entry.name, 'SKILL.md');
|
||||
if (!existsSync(skillMd)) continue;
|
||||
renameSync(join(dir, entry.name), join(dir, prefix + entry.name));
|
||||
let content = readFileSync(join(dir, prefix + entry.name, 'SKILL.md'), 'utf8');
|
||||
content = prefixSkillContent(content, prefix, allNames);
|
||||
writeFileSync(join(dir, prefix + entry.name, 'SKILL.md'), content);
|
||||
count++;
|
||||
}
|
||||
}
|
||||
console.log(JSON.stringify({ count }));
|
||||
`);
|
||||
|
||||
const output = JSON.parse(execSync(`node ${helperScript} ${tmp} i-`, { encoding: 'utf8' }));
|
||||
expect(output.count).toBe(6); // 3 skills x 2 providers
|
||||
|
||||
// Verify folders renamed
|
||||
const skills = readdirSync(join(tmp, '.claude', 'skills'));
|
||||
expect(skills).toContain('i-audit');
|
||||
expect(skills).toContain('i-polish');
|
||||
expect(skills).toContain('i-teach-impeccable');
|
||||
expect(skills).not.toContain('audit');
|
||||
expect(skills).not.toContain('polish');
|
||||
}, 15000);
|
||||
|
||||
test('prefixed SKILL.md has correct name and cross-references', () => {
|
||||
const content = readFileSync(join(tmp, '.claude', 'skills', 'i-audit', 'SKILL.md'), 'utf8');
|
||||
expect(content).toContain('name: i-audit');
|
||||
expect(content).toContain('/i-audit');
|
||||
expect(content).toContain('/i-polish');
|
||||
expect(content).toContain('the i-teach-impeccable skill');
|
||||
// Original unprefixed references should be gone
|
||||
expect(content).not.toMatch(/\/audit(?=[^a-zA-Z0-9_-]|$)/);
|
||||
});
|
||||
|
||||
test('also prefixed in second provider', () => {
|
||||
const skills = readdirSync(join(tmp, '.cursor', 'skills'));
|
||||
expect(skills).toContain('i-audit');
|
||||
expect(skills).toContain('i-teach-impeccable');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Update fallback (direct download) ───────────────────────────────────────
|
||||
|
||||
describe('skills update: direct download fallback', () => {
|
||||
let tmp;
|
||||
|
||||
beforeAll(() => {
|
||||
tmp = mkdtempSync(join(tmpdir(), 'imp-test-update-'));
|
||||
execSync('git init', { cwd: tmp });
|
||||
|
||||
// Create stale skills that the update should overwrite
|
||||
for (const skill of ['audit', 'teach-impeccable']) {
|
||||
const skillDir = join(tmp, '.claude', 'skills', skill);
|
||||
mkdirSync(skillDir, { recursive: true });
|
||||
writeFileSync(join(skillDir, 'SKILL.md'), `---\nname: ${skill}\nstale: true\n---\nOld content.\n`);
|
||||
}
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (tmp) rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('downloads universal bundle and updates skills', () => {
|
||||
const output = run('skills update -y', { cwd: tmp });
|
||||
expect(output).toContain('direct download');
|
||||
expect(output).toContain('Updated');
|
||||
|
||||
// Skills should have fresh content (no 'stale: true')
|
||||
const content = readFileSync(join(tmp, '.claude', 'skills', 'audit', 'SKILL.md'), 'utf8');
|
||||
expect(content).not.toContain('stale: true');
|
||||
expect(content).toContain('name:');
|
||||
}, 60000);
|
||||
|
||||
test('update added new skills that were not present before', () => {
|
||||
// The universal bundle has ~20 skills, we only had 2
|
||||
const skills = readdirSync(join(tmp, '.claude', 'skills'));
|
||||
expect(skills.length).toBeGreaterThan(5);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Full install e2e (with real npx skills) ─────────────────────────────────
|
||||
|
||||
let hasNpxSkills = false;
|
||||
try {
|
||||
execSync('npx skills --version', { encoding: 'utf8', timeout: 15000, stdio: 'pipe' });
|
||||
hasNpxSkills = true;
|
||||
} catch {}
|
||||
|
||||
const describeNpx = hasNpxSkills ? describe : describe.skip;
|
||||
|
||||
describeNpx('skills install: full e2e with npx skills', () => {
|
||||
let tmp;
|
||||
|
||||
beforeAll(() => {
|
||||
tmp = mkdtempSync(join(tmpdir(), 'imp-test-full-'));
|
||||
execSync('git init', { cwd: tmp });
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
if (tmp) rmSync(tmp, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
test('installs skills into a fresh project', () => {
|
||||
const output = run('skills install -y', { cwd: tmp });
|
||||
expect(output).toContain('Done!');
|
||||
|
||||
const hasSkills = ['.claude', '.cursor'].some(d => {
|
||||
const dir = join(tmp, d, 'skills');
|
||||
return existsSync(dir) && readdirSync(dir).length > 0;
|
||||
});
|
||||
expect(hasSkills).toBe(true);
|
||||
}, 90000);
|
||||
|
||||
test('install with --prefix= renames all skills', () => {
|
||||
const output = run('skills install -y --force --prefix=x-', { cwd: tmp });
|
||||
|
||||
// Find the provider that has skills
|
||||
let found = false;
|
||||
for (const d of ['.claude', '.cursor', '.gemini', '.codex', '.agents', '.kiro']) {
|
||||
const dir = join(tmp, d, 'skills');
|
||||
if (!existsSync(dir)) continue;
|
||||
const skills = readdirSync(dir);
|
||||
if (skills.length === 0) continue;
|
||||
found = true;
|
||||
const prefixed = skills.filter(s => s.startsWith('x-'));
|
||||
// All skills should be prefixed
|
||||
expect(prefixed.length).toBe(skills.length);
|
||||
break;
|
||||
}
|
||||
expect(found).toBe(true);
|
||||
}, 90000);
|
||||
});
|
||||
Reference in New Issue
Block a user