From aaba039710ad3b0caacf059b89c93bd250eefef3 Mon Sep 17 00:00:00 2001 From: vchamarthi <123653452+vchamarthi@users.noreply.github.com> Date: Thu, 22 Oct 2026 22:07:08 -0500 Subject: [PATCH 1/9] feat(cli): installer options, help, and error-message fixes --- .github/workflows/validate.yml | 86 +++++++++ README.md | 5 + bin/intel-skills.mjs | 217 +++++++++++++++++++--- tools/tests/test_cli_flag_order.py | 44 +++++ tools/tests/test_cli_help.py | 44 +++++ tools/tests/test_cli_install_many.py | 54 ++++++ tools/tests/test_cli_list_description.py | 33 ++++ tools/tests/test_cli_show_usage.py | 42 +++++ tools/tests/test_cli_skill_suggestions.py | 43 +++++ tools/tests/test_cli_unknown_command.py | 30 +++ tools/tests/test_cli_unknown_flags.py | 51 +++++ tools/tests/test_cli_verify_all.py | 55 ++++++ tools/tests/test_cli_verify_path.py | 38 ++++ tools/tests/test_cli_verify_path_file.py | 45 +++++ tools/tests/test_cli_version.py | 31 ++++ 15 files changed, 790 insertions(+), 28 deletions(-) create mode 100644 tools/tests/test_cli_flag_order.py create mode 100644 tools/tests/test_cli_help.py create mode 100644 tools/tests/test_cli_install_many.py create mode 100644 tools/tests/test_cli_list_description.py create mode 100644 tools/tests/test_cli_show_usage.py create mode 100644 tools/tests/test_cli_skill_suggestions.py create mode 100644 tools/tests/test_cli_unknown_command.py create mode 100644 tools/tests/test_cli_unknown_flags.py create mode 100644 tools/tests/test_cli_verify_all.py create mode 100644 tools/tests/test_cli_verify_path.py create mode 100644 tools/tests/test_cli_verify_path_file.py create mode 100644 tools/tests/test_cli_version.py diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 367d77c..1a23fa8 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -146,6 +146,33 @@ jobs: - name: Catalog is listable run: node bin/intel-skills.mjs list + - name: Catalog lists as JSON + run: | + set -euo pipefail + node bin/intel-skills.mjs list --json > "${RUNNER_TEMP}/catalog.json" + test "$(node -p "JSON.parse(require('fs').readFileSync(process.argv[1])).length" "${RUNNER_TEMP}/catalog.json")" = \ + "$(node bin/intel-skills.mjs list | wc -l | tr -d ' ')" + + # `show --files` must name exactly what install writes: SKILL.md in, evals/ out. + - name: show --files matches what install writes + run: | + set -euo pipefail + node bin/intel-skills.mjs install linux-perf --dir "${RUNNER_TEMP}/showfiles" + node bin/intel-skills.mjs show linux-perf --files > "${RUNNER_TEMP}/shown.txt" + (cd "${RUNNER_TEMP}/showfiles/linux-perf" && find . -type f | sed 's|^\./||' | LC_ALL=C sort) | + diff - "${RUNNER_TEMP}/shown.txt" + + - name: Catalog list filters partition the catalog + run: | + set -euo pipefail + all=$(node bin/intel-skills.mjs list | wc -l) + imported=$(node bin/intel-skills.mjs list --imported | wc -l) + own=$(node bin/intel-skills.mjs list --own | wc -l) + test "$((imported + own))" -eq "${all}" + node bin/intel-skills.mjs list --imported | cut -f2 | grep -x imported > /dev/null + if node bin/intel-skills.mjs list --imported --own; then exit 1; fi + if node bin/intel-skills.mjs list --bogus; then exit 1; fi + # An imported skill, end to end: installed, then verified offline from disk. An # import is where the file list is longest and where a copy that dropped # something would be least obvious, so it is the single-skill case worth pinning. @@ -154,6 +181,8 @@ jobs: set -euo pipefail node bin/intel-skills.mjs install linux-perf --dir "${RUNNER_TEMP}/skills" node bin/intel-skills.mjs verify linux-perf --path "${RUNNER_TEMP}/skills/linux-perf" + # `--target` resolves the same directory `install` wrote, here through --dir. + node bin/intel-skills.mjs verify linux-perf --dir "${RUNNER_TEMP}/skills" test -s "${RUNNER_TEMP}/skills/linux-perf/SKILL.md" test -s "${RUNNER_TEMP}/skills/linux-perf/.source.json" @@ -168,6 +197,39 @@ jobs: while read -r name; do node bin/intel-skills.mjs verify "${name}" --path "${RUNNER_TEMP}/all/${name}" done + # The same check in one call, and it must fail when one skill is altered. + node bin/intel-skills.mjs verify --all --path "${RUNNER_TEMP}/all" + echo "altered" >> "${RUNNER_TEMP}/all/linux-perf/SKILL.md" + if node bin/intel-skills.mjs verify --all --path "${RUNNER_TEMP}/all"; then + echo "::error::verify --all accepted an installed skill that differs from the catalog" + exit 1 + fi + + # `--dry-run` promises nothing is written, so a regression that ignores the flag + # would install silently. Assert the target directory is still absent. + - name: Install --dry-run writes nothing + run: | + set -euo pipefail + node bin/intel-skills.mjs install linux-perf --dir "${RUNNER_TEMP}/dry" --dry-run + test ! -e "${RUNNER_TEMP}/dry" + + # Several names in one call: every one is written, and a bad name fails the run + # without stopping the others. + - name: Install several named skills + run: | + set -euo pipefail + node bin/intel-skills.mjs install linux-perf dpnp-quickstart --dir "${RUNNER_TEMP}/many" + test -s "${RUNNER_TEMP}/many/linux-perf/SKILL.md" + test -s "${RUNNER_TEMP}/many/dpnp-quickstart/SKILL.md" + if node bin/intel-skills.mjs install no-such-skill linux-perf --dir "${RUNNER_TEMP}/partial"; then + echo "::error::install exited 0 although one named skill does not exist" + exit 1 + fi + test -s "${RUNNER_TEMP}/partial/linux-perf/SKILL.md" + # A repeated name is one install, so it must not trip "destination already exists". + node bin/intel-skills.mjs install linux-perf linux-perf --dir "${RUNNER_TEMP}/twice" | + grep -q '^installed linux-perf ' + test "$(find "${RUNNER_TEMP}/twice" -maxdepth 1 -mindepth 1 | wc -l)" -eq 1 # `verify` noticing is the property the rest rests on, so it is asserted rather # than assumed: an installed skill edited after the fact must fail. A regression @@ -186,3 +248,27 @@ jobs: echo "::error::verify accepted a file no skill in the catalog ships" exit 1 fi + + # Argument handling the steps above never reach, such as `show` with no skill name or `verify --path` on a file. + # Stdlib unittest, so nothing is installed; the tests skip themselves without node. + - name: CLI argument tests + run: python3 -m unittest discover -s tools/tests -p 'test_cli_*.py' + + # The inverse of install: removes exactly the named skill, and refuses a directory + # that is not an installed skill or a name that would leave the target root. + - name: Uninstall removes only an installed skill + run: | + set -euo pipefail + node bin/intel-skills.mjs install linux-perf --dir "${RUNNER_TEMP}/un" + mkdir "${RUNNER_TEMP}/un/not-a-skill" + node bin/intel-skills.mjs uninstall linux-perf --dir "${RUNNER_TEMP}/un" + test ! -e "${RUNNER_TEMP}/un/linux-perf" + if node bin/intel-skills.mjs uninstall not-a-skill --dir "${RUNNER_TEMP}/un"; then + echo "::error::uninstall removed a directory that is not an installed skill" + exit 1 + fi + test -d "${RUNNER_TEMP}/un/not-a-skill" + if node bin/intel-skills.mjs uninstall ../un --dir "${RUNNER_TEMP}/un"; then + echo "::error::uninstall accepted a path-like name" + exit 1 + fi diff --git a/README.md b/README.md index 2767ef1..c2f66a5 100644 --- a/README.md +++ b/README.md @@ -104,10 +104,15 @@ for the case where what matters is that the bytes are exactly the ones a reviewe ```bash npx github:intel/skills list # the catalog, and which kind +npx github:intel/skills list --json # the same, whole, for a script +npx github:intel/skills list --imported # only copies of another repository's skills npx github:intel/skills install linux-perf --target claude-code # -> ~/.claude/skills npx github:intel/skills install --all --target agents # -> ~/.agents/skills npx github:intel/skills install xpu-port --dir .agents/skills # into a project, to commit +npx github:intel/skills install linux-perf xpu-port --target agents # several named skills at once npx github:intel/skills verify linux-perf --path ~/.claude/skills/linux-perf +npx github:intel/skills verify linux-perf --target claude-code # same check, by target +npx github:intel/skills verify --all --path ~/.agents/skills # every catalog skill, after install --all ``` Node 20 or newer, no dependencies, nothing from npm, and no network call after the diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index c11e20e..9652b2e 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -49,15 +49,35 @@ const INSTALL_TARGETS = { */ const NOT_INSTALLED = new Set(["evals", "perf"]); +const LIST_FLAGS = ["--json", "--imported", "--own"]; +const VALUE_FLAGS = new Set(["--target", "--dir", "--path"]); + const NAME = /^[a-z0-9]+(?:-[a-z0-9]+)*$/; function skillDir(name) { if (!NAME.test(String(name))) throw new Error(`invalid skill name: ${name}`); const src = path.join(SKILLS_DIR, name); - if (!fs.existsSync(path.join(src, "SKILL.md"))) throw new Error(`skill not found: ${name}`); + if (!fs.existsSync(path.join(src, "SKILL.md"))) { + // A half-remembered name is the usual cause, so point at the catalog entries that + // contain it rather than leave the user to run `list` and scan thirty-odd rows. + const similar = catalog().filter((candidate) => candidate.includes(name)); + const hint = similar.length > 0 ? `; did you mean: ${similar.join(", ")}?` : ""; + throw new Error(`skill not found: ${name}${hint}`); + } return src; } +/** A one-line YAML scalar without its surrounding quotes, so `list` shows the text alone. */ +function unquote(value) { + if (value.length >= 2 && value.startsWith('"') && value.endsWith('"')) { + return value.slice(1, -1).replace(/\\(["\\])/g, "$1"); + } + if (value.length >= 2 && value.startsWith("'") && value.endsWith("'")) { + return value.slice(1, -1).replace(/''/g, "'"); + } + return value; +} + /** The description an agent routes on, read from the frontmatter of SKILL.md. */ function description(name) { const skillMd = path.join(SKILLS_DIR, name, "SKILL.md"); @@ -69,7 +89,7 @@ function description(name) { const index = lines.findIndex((line) => /^description:/.test(line)); if (index === -1) return ""; const inline = lines[index].replace(/^description:\s*/, "").trim(); - if (inline && !/^[>|]-?$/.test(inline)) return inline; + if (inline && !/^[>|]-?$/.test(inline)) return unquote(inline); const block = []; for (let i = index + 1; i < lines.length && /^\s{2,}\S|^\s*$/.test(lines[i]); i += 1) { block.push(lines[i].trim()); @@ -136,12 +156,17 @@ function destinationFor(root, name, force) { * `scripts/check_runtime_preflight.sh` ships it executable, and a copy that dropped * the bit would fail on the first command the skill documents. */ -function install(name, root, force) { +function install(name, root, force, dryRun = false) { const src = skillDir(name); const dest = destinationFor(root, name, force); + const listed = files(src); + if (dryRun) { + console.log(`would install ${name} -> ${dest} (${listed.length} file(s))`); + for (const rel of listed) console.log(` ${rel}`); + return; + } fs.mkdirSync(root, { recursive: true }); fs.rmSync(dest, { recursive: true, force: true }); - const listed = files(src); for (const rel of listed) { const from = path.join(src, rel); const to = path.join(dest, rel); @@ -169,6 +194,8 @@ function verify(name, dir) { const src = skillDir(name); const installed = path.resolve(dir); if (!fs.existsSync(installed)) throw new Error(`no such directory: ${installed}`); + // A file here used to surface as a raw "ENOTDIR: not a directory, scandir" from node. + if (!fs.statSync(installed).isDirectory()) throw new Error(`not a directory: ${installed}`); const digest = (file) => crypto.createHash("sha256").update(fs.readFileSync(file)).digest("hex"); const wanted = files(src); @@ -194,6 +221,26 @@ function verify(name, dir) { console.log(`ok ${name} (${wanted.length} file(s) match skills/${name}/)`); } +/** + * Remove an installed skill from a target root. + * + * Deliberately narrow: the name must be a catalog-shaped name, the directory must hold a + * SKILL.md (so a typo cannot delete an unrelated folder), and it must stay inside the + * root. It never touches anything but `/`. + */ +function uninstall(name, root) { + if (!NAME.test(String(name))) throw new Error(`invalid skill name: ${name}`); + const dest = path.join(root, name); + if (!path.resolve(dest).startsWith(`${path.resolve(root)}${path.sep}`)) { + throw new Error("uninstall path escapes target directory"); + } + if (!fs.existsSync(path.join(dest, "SKILL.md"))) { + throw new Error(`not an installed skill: ${dest}`); + } + fs.rmSync(dest, { recursive: true, force: true }); + console.log(`removed ${name} from ${dest}`); +} + function argValue(args, flag) { const index = args.indexOf(flag); if (index === -1) return null; @@ -202,12 +249,43 @@ function argValue(args, flag) { return value; } +/** + * Refuse an option the command does not take. A misspelt `--forse` used to be dropped + * silently, so the command ran with its default and the user never learned the flag + * had not applied. + */ +function rejectUnknownOptions(args, valueFlags, switches) { + for (let i = 0; i < args.length; i += 1) { + const arg = args[i]; + if (valueFlags.includes(arg)) i += 1; + else if (arg.startsWith("-") && !switches.includes(arg)) throw new Error(`unknown option: ${arg}`); + } +} + +/** The first argument that is neither a flag nor the value of one, so flags may precede it. */ +function positionals(args) { + const found = []; + for (let i = 0; i < args.length; i += 1) { + if (VALUE_FLAGS.has(args[i])) i += 1; + else if (!args[i].startsWith("--") && !found.includes(args[i])) found.push(args[i]); + } + return found; +} + +function positional(args) { + return positionals(args)[0] ?? null; +} + function usage(code = 0) { console.log(`Usage: - intel-skills list - intel-skills show - intel-skills install |--all --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] [--force] + intel-skills list [--json] [--imported|--own] + intel-skills show [--files] + intel-skills install ...|--all --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] [--force] [--dry-run] + intel-skills uninstall --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] intel-skills verify --path + intel-skills verify --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] + intel-skills --version + intel-skills verify --all --path Every command is a local operation on this checkout: no network, and nothing but node is required. \`list\` marks which skills are copies of another repository's work, and @@ -219,18 +297,63 @@ const [, , command, ...args] = process.argv; try { if (!command || command === "-h" || command === "--help") usage(0); + // `install --help` is what a user types to learn the flags; without this it fell through + // to "invalid skill name: --help" or an exit-1 usage, and `list --help` printed the catalog. + // A flag's value is never a help request, so `--dir -h` still means a directory named -h. + if (args.some((arg, i) => (arg === "-h" || arg === "--help") && !VALUE_FLAGS.has(args[i - 1]))) { + usage(0); + } + + if (command === "--version" || command === "-v") { + console.log(JSON.parse(fs.readFileSync(path.join(ROOT, "package.json"), "utf8")).version); + process.exit(0); + } if (command === "list") { + const unknown = args.find((arg) => !LIST_FLAGS.includes(arg)); + if (unknown) throw new Error(`unknown option for list: ${unknown}`); + const json = args.includes("--json"); + const imported = args.includes("--imported"); + const own = args.includes("--own"); + if (imported && own) throw new Error("--imported and --own cannot be combined"); + const rows = []; for (const name of catalog()) { const origin = source(name); - const kind = origin - ? `imported\t${origin.repo.split("/").slice(-1)[0]}@${String(origin.commit).slice(0, 12)}` - : "own\t-"; - console.log(`${name}\t${kind}\t${description(name).slice(0, 100)}`); + if ((imported && !origin) || (own && origin)) continue; + rows.push({ + name, + kind: origin ? "imported" : "own", + repo: origin ? origin.repo : null, + commit: origin ? String(origin.commit) : null, + license: origin ? origin.license ?? null : null, + description: description(name), + }); + } + if (json) { + // The tab-separated form truncates the description to 100 characters and is built + // for a person; this is the same catalog, whole, for a script that would otherwise + // have to split on tabs. + console.log(JSON.stringify(rows, null, 2)); + } else { + for (const row of rows) { + const kind = row.repo + ? `imported\t${row.repo.split("/").slice(-1)[0]}@${row.commit.slice(0, 12)}` + : "own\t-"; + console.log(`${row.name}\t${kind}\t${row.description.slice(0, 100)}`); + } } } else if (command === "show") { const name = args[0]; + // Without this, `show` alone reported "invalid skill name: undefined", an error + // about a value the user never typed. Every other command prints usage instead. + if (!name || name.startsWith("--")) usage(1); const src = skillDir(name); + if (args.includes("--files")) { + // The exact set `install` would write, so a reviewer can see what lands on disk + // without installing it. + for (const rel of files(src)) console.log(rel); + process.exit(0); + } const origin = source(name); if (origin) { console.log(`upstream: ${origin.repo}/tree/${origin.commit}/${origin.path}`); @@ -243,44 +366,82 @@ try { } console.log(fs.readFileSync(path.join(src, "SKILL.md"), "utf8")); } else if (command === "install") { + rejectUnknownOptions(args, ["--target", "--dir"], ["--force", "--all", "--dry-run"]); + if (args.includes("--all") && args.some((arg, i) => !arg.startsWith("-") && !["--target", "--dir"].includes(args[i - 1]))) { + throw new Error("--all installs every skill; do not also name one"); + } const target = argValue(args, "--target"); const dirOverride = argValue(args, "--dir"); if (!target && !dirOverride) usage(1); const root = targetRoot(target, dirOverride); const force = args.includes("--force"); - if (args.includes("--all")) { - // One skill's failure must not decide the other thirty-two. Aborting on the - // first error left an arbitrary prefix of the catalog installed and said nothing - // about the rest, which is the worst of both: partial state, and no list of what - // is missing. So each is attempted, every failure is reported, and the exit code - // still says the run did not fully succeed. - const names = catalog(); + const dryRun = args.includes("--dry-run"); + // One skill's failure must not decide the others. Aborting on the first error left an + // arbitrary prefix of the catalog installed and said nothing about the rest, which is + // the worst of both: partial state, and no list of what is missing. So each is + // attempted, every failure is reported, and the exit code still says the run did not + // fully succeed. + const names = args.includes("--all") ? catalog() : positionals(args); + if (names.length === 0) usage(1); + if (names.length === 1 && !args.includes("--all")) { + install(names[0], root, force, dryRun); + } else { const failures = []; for (const name of names) { try { - install(name, root, force); + install(name, root, force, dryRun); } catch (error) { failures.push(`${name}: ${error.message}`); console.error(`error: ${name}: ${error.message}`); } } - console.log(`${names.length - failures.length}/${names.length} installed to ${root}`); + console.log(`${names.length - failures.length}/${names.length} ${dryRun ? "would be installed to" : "installed to"} ${root}`); if (failures.length > 0) { console.error(`${failures.length} skill(s) not installed:`); for (const failure of failures) console.error(` ${failure}`); process.exit(1); } - } else { - const name = args[0]; - if (!name || name.startsWith("--")) usage(1); - install(name, root, force); } + } else if (command === "uninstall") { + const target = argValue(args, "--target"); + const dirOverride = argValue(args, "--dir"); + if (!target && !dirOverride) usage(1); + const name = positional(args); + if (!name) usage(1); + uninstall(name, targetRoot(target, dirOverride)); } else if (command === "verify") { - const name = args[0]; - const dir = argValue(args, "--path"); - if (!name || !dir) usage(1); - verify(name, dir); + rejectUnknownOptions(args, ["--path", "--target", "--dir"], ["--all"]); + const target = argValue(args, "--target"); + const dirOverride = argValue(args, "--dir"); + const explicit = argValue(args, "--path"); + // `--target` names the place `install` wrote to, so verify can check it without the + // caller spelling out the per-agent directory. + if (!explicit && !target && !dirOverride) usage(1); + if (args.includes("--all")) { + // The counterpart of `install --all`: the directory it wrote into, with every + // catalog skill checked under it. Each is attempted and every failure is listed, + // for the same reason `install --all` does not stop at the first one. + const root = path.resolve(explicit ?? targetRoot(target, dirOverride)); + if (!fs.existsSync(root)) throw new Error(`no such directory: ${root}`); + const names = catalog(); + const failures = []; + for (const name of names) { + try { + verify(name, path.join(root, name)); + } catch (error) { + failures.push(`${name}: ${error.message}`); + console.error(`error: ${name}: ${error.message}`); + } + } + console.log(`${names.length - failures.length}/${names.length} verified under ${root}`); + if (failures.length > 0) process.exit(1); + } else { + const name = positional(args); + if (!name) usage(1); + verify(name, explicit ?? path.join(targetRoot(target, dirOverride), name)); + } } else { + console.error(`error: unknown command: ${command}`); usage(1); } } catch (error) { diff --git a/tools/tests/test_cli_flag_order.py b/tools/tests/test_cli_flag_order.py new file mode 100644 index 0000000..9aefb28 --- /dev/null +++ b/tools/tests/test_cli_flag_order.py @@ -0,0 +1,44 @@ +"""`install` and `verify` take their flags before or after the skill name.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class FlagOrderTests(unittest.TestCase): + def test_install_then_verify_accepts_either_flag_order(self): + for flags_first in (False, True): + with self.subTest(flags_first=flags_first), tempfile.TemporaryDirectory() as tmp: + target = Path(tmp) / "skills" + if flags_first: + install = run("install", "--force", "--dir", str(target), SKILL) + else: + install = run("install", SKILL, "--dir", str(target), "--force") + self.assertEqual(install.returncode, 0, install.stderr) + self.assertTrue((target / SKILL / "SKILL.md").is_file()) + + path = str(target / SKILL) + verify = run("verify", "--path", path, SKILL) if flags_first else run("verify", SKILL, "--path", path) + self.assertEqual(verify.returncode, 0, verify.stderr) + self.assertTrue(verify.stdout.startswith(f"ok {SKILL}")) + + def test_install_with_only_flags_prints_usage(self): + with tempfile.TemporaryDirectory() as tmp: + result = run("install", "--dir", tmp) + self.assertEqual(result.returncode, 1) + self.assertIn("Usage:", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_help.py b/tools/tests/test_cli_help.py new file mode 100644 index 0000000..0421de4 --- /dev/null +++ b/tools/tests/test_cli_help.py @@ -0,0 +1,44 @@ +"""`--help` after any installer subcommand prints usage and exits 0.""" + +import subprocess +import unittest +from pathlib import Path + +CLI = Path(__file__).resolve().parents[2] / "bin" / "intel-skills.mjs" + + +def run(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["node", str(CLI), *args], capture_output=True, text=True, check=False + ) + + +class SubcommandHelpTest(unittest.TestCase): + def test_subcommand_help_prints_usage(self) -> None: + for args in ( + ("install", "--help"), + ("install", "-h"), + ("verify", "--help"), + ("show", "--help"), + ("list", "--help"), + ): + with self.subTest(args=args): + result = run(*args) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue(result.stdout.startswith("Usage:")) + self.assertNotIn("invalid skill name", result.stderr) + + def test_flag_value_is_not_a_help_request(self) -> None: + result = run("verify", "linux-perf", "--path", "-h") + self.assertEqual(result.returncode, 1) + self.assertNotIn("Usage:", result.stdout) + self.assertIn("no such directory", result.stderr) + + def test_list_still_lists_without_help(self) -> None: + result = run("list") + self.assertEqual(result.returncode, 0) + self.assertNotIn("Usage:", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_install_many.py b/tools/tests/test_cli_install_many.py new file mode 100644 index 0000000..6b74d31 --- /dev/null +++ b/tools/tests/test_cli_install_many.py @@ -0,0 +1,54 @@ +"""`intel-skills install a b c --dir ` installs every named skill.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +NAMES = sorted(p.name for p in (ROOT / "skills").iterdir() if (p / "SKILL.md").is_file()) + + +def run(*args): + return subprocess.run( + ["node", str(CLI), *args], capture_output=True, text=True, check=False + ) + + +class InstallManyTest(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.dir, ignore_errors=True) + + def test_installs_every_named_skill(self): + result = run("install", NAMES[0], NAMES[1], "--dir", str(self.dir)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue((self.dir / NAMES[0] / "SKILL.md").is_file()) + self.assertTrue((self.dir / NAMES[1] / "SKILL.md").is_file()) + self.assertIn("2/2 installed", result.stdout) + + def test_a_bad_name_does_not_stop_the_others(self): + result = run("install", "no-such-skill", NAMES[0], "--dir", str(self.dir)) + self.assertEqual(result.returncode, 1) + self.assertIn("error: no-such-skill:", result.stderr) + self.assertTrue((self.dir / NAMES[0] / "SKILL.md").is_file()) + self.assertIn("1/2 installed", result.stdout) + + def test_a_repeated_name_is_installed_once(self): + result = run("install", NAMES[0], NAMES[0], "--force", "--dir", str(self.dir)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.count(f"installed {NAMES[0]} ->"), 1) + + def test_a_single_name_still_works(self): + result = run("install", NAMES[0], "--dir", str(self.dir)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue((self.dir / NAMES[0] / "SKILL.md").is_file()) + + def test_needs_a_skill_name(self): + self.assertEqual(run("install", "--dir", str(self.dir)).returncode, 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_list_description.py b/tools/tests/test_cli_list_description.py new file mode 100644 index 0000000..c4af432 --- /dev/null +++ b/tools/tests/test_cli_list_description.py @@ -0,0 +1,33 @@ +"""`list` prints a skill's description without the YAML quotes around it.""" + +import shutil +import subprocess +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" + + +def quoted_skills(): + names = [] + for skill_md in sorted((ROOT / "skills").glob("*/SKILL.md")): + for line in skill_md.read_text(encoding="utf8").splitlines()[:20]: + if line.startswith("description:") and line.split(":", 1)[1].strip()[:1] in "\"'": + names.append(skill_md.parent.name) + return names + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class ListDescriptionTests(unittest.TestCase): + def test_quoted_descriptions_are_listed_without_quotes(self): + names = quoted_skills() + self.assertTrue(names, "catalog has no quoted description to exercise") + out = subprocess.run(["node", str(CLI), "list"], capture_output=True, text=True, check=True).stdout + rows = {line.split("\t")[0]: line.split("\t")[3] for line in out.splitlines()} + for name in names: + self.assertNotIn(rows[name][:1], "\"'", f"{name}: {rows[name]!r}") + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_show_usage.py b/tools/tests/test_cli_show_usage.py new file mode 100644 index 0000000..264762c --- /dev/null +++ b/tools/tests/test_cli_show_usage.py @@ -0,0 +1,42 @@ +"""`show` with no skill name prints usage instead of "invalid skill name: undefined".""" + +import shutil +import subprocess +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class ShowUsageTests(unittest.TestCase): + def test_show_without_a_name_prints_usage(self): + result = run("show") + self.assertEqual(result.returncode, 1) + self.assertIn("Usage:", result.stdout) + self.assertNotIn("undefined", result.stdout + result.stderr) + + def test_show_with_only_a_flag_prints_usage(self): + result = run("show", "--files") + self.assertEqual(result.returncode, 1) + self.assertIn("Usage:", result.stdout) + + def test_show_with_a_name_still_prints_the_skill(self): + result = run("show", SKILL) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(f"name: {SKILL}", result.stdout) + + def test_show_unknown_skill_still_names_it(self): + result = run("show", "no-such-skill") + self.assertEqual(result.returncode, 1) + self.assertIn("skill not found: no-such-skill", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_skill_suggestions.py b/tools/tests/test_cli_skill_suggestions.py new file mode 100644 index 0000000..22cb070 --- /dev/null +++ b/tools/tests/test_cli_skill_suggestions.py @@ -0,0 +1,43 @@ +"""An unknown skill name in `show`/`install` names the catalog skills it resembles.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class SkillSuggestionTests(unittest.TestCase): + def test_show_suggests_skills_containing_the_typed_name(self): + result = run("show", "vllm-xpu") + self.assertEqual(result.returncode, 1) + self.assertIn("skill not found: vllm-xpu", result.stderr) + self.assertIn("did you mean", result.stderr) + for name in ("vllm-xpu-bench", "vllm-xpu-profile", "vllm-xpu-run"): + self.assertIn(name, result.stderr) + + def test_install_suggests_and_writes_nothing(self): + with tempfile.TemporaryDirectory() as tmp: + target = Path(tmp) / "skills" + result = run("install", "vllm-xpu", "--dir", str(target)) + self.assertEqual(result.returncode, 1) + self.assertIn("vllm-xpu-run", result.stderr) + self.assertFalse(target.exists()) + + def test_no_suggestion_when_nothing_resembles_the_name(self): + result = run("show", "zzz-nothing-like-it") + self.assertEqual(result.returncode, 1) + self.assertIn("skill not found: zzz-nothing-like-it", result.stderr) + self.assertNotIn("did you mean", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_unknown_command.py b/tools/tests/test_cli_unknown_command.py new file mode 100644 index 0000000..6af3de5 --- /dev/null +++ b/tools/tests/test_cli_unknown_command.py @@ -0,0 +1,30 @@ +"""An unrecognised installer command names itself instead of only printing usage.""" + +import subprocess +import sys +import unittest +from pathlib import Path + +CLI = Path(__file__).resolve().parents[2] / "bin" / "intel-skills.mjs" + + +def run(*args): + return subprocess.run( + ["node", str(CLI), *args], capture_output=True, text=True, check=False + ) + + +class UnknownCommandTest(unittest.TestCase): + def test_unknown_command_is_named_on_stderr(self): + result = run("instal", "linux-perf") + self.assertEqual(result.returncode, 1) + self.assertIn("error: unknown command: instal", result.stderr) + + def test_known_command_still_works(self): + result = run("list") + self.assertEqual(result.returncode, 0) + self.assertNotIn("unknown command", result.stderr) + + +if __name__ == "__main__": + sys.exit(unittest.main()) diff --git a/tools/tests/test_cli_unknown_flags.py b/tools/tests/test_cli_unknown_flags.py new file mode 100644 index 0000000..28b2ca1 --- /dev/null +++ b/tools/tests/test_cli_unknown_flags.py @@ -0,0 +1,51 @@ +"""`install` and `verify` refuse an option they do not know instead of ignoring it.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class UnknownOptionTests(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.target = Path(tmp.name) / "skills" + + def test_install_rejects_misspelled_flag_and_writes_nothing(self): + result = run("install", SKILL, "--dir", str(self.target), "--forse") + self.assertEqual(result.returncode, 1) + self.assertIn("unknown option: --forse", result.stderr) + self.assertFalse(self.target.exists()) + + def test_install_rejects_all_combined_with_a_skill_name(self): + result = run("install", SKILL, "--all", "--dir", str(self.target)) + self.assertEqual(result.returncode, 1) + self.assertIn("--all", result.stderr) + self.assertFalse(self.target.exists()) + + def test_verify_rejects_unknown_flag(self): + self.assertEqual(run("install", SKILL, "--dir", str(self.target)).returncode, 0) + result = run("verify", SKILL, "--path", str(self.target / SKILL), "--bogus") + self.assertEqual(result.returncode, 1) + self.assertIn("unknown option: --bogus", result.stderr) + + def test_known_flags_still_work(self): + self.assertEqual(run("install", SKILL, "--dir", str(self.target), "--force").returncode, 0) + self.assertEqual(run("install", SKILL, "--dir", str(self.target), "--force").returncode, 0) + verified = run("verify", SKILL, "--path", str(self.target / SKILL)) + self.assertEqual(verified.returncode, 0, verified.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_verify_all.py b/tools/tests/test_cli_verify_all.py new file mode 100644 index 0000000..a1e55c6 --- /dev/null +++ b/tools/tests/test_cli_verify_all.py @@ -0,0 +1,55 @@ +"""`intel-skills verify --all --path ` checks every catalog skill under one directory.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +NAMES = sorted(p.name for p in (ROOT / "skills").iterdir() if (p / "SKILL.md").is_file()) + + +def run(*args): + return subprocess.run( + ["node", str(CLI), *args], capture_output=True, text=True, check=False + ) + + +class VerifyAllTest(unittest.TestCase): + def setUp(self): + self.dir = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.dir, ignore_errors=True) + + def install_all(self): + self.assertEqual(run("install", "--all", "--dir", str(self.dir)).returncode, 0) + + def test_accepts_a_full_install(self): + self.install_all() + result = run("verify", "--all", "--path", str(self.dir)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("verified under", result.stdout) + + def test_names_the_altered_and_the_missing_skill(self): + self.install_all() + (self.dir / NAMES[0] / "SKILL.md").write_text("altered\n", encoding="utf-8") + shutil.rmtree(self.dir / NAMES[1]) + + result = run("verify", "--all", "--path", str(self.dir)) + self.assertEqual(result.returncode, 1) + self.assertIn(f"error: {NAMES[0]}:", result.stderr) + self.assertIn(f"error: {NAMES[1]}:", result.stderr) + self.assertIn(f"{len(NAMES) - 2}/{len(NAMES)} verified", result.stdout) + + def test_needs_a_path(self): + self.assertEqual(run("verify", "--all").returncode, 1) + + def test_rejects_a_missing_directory(self): + result = run("verify", "--all", "--path", str(self.dir / "absent")) + self.assertEqual(result.returncode, 1) + self.assertIn("no such directory", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_verify_path.py b/tools/tests/test_cli_verify_path.py new file mode 100644 index 0000000..ed138c5 --- /dev/null +++ b/tools/tests/test_cli_verify_path.py @@ -0,0 +1,38 @@ +"""`verify --path` accepts an installed copy and names a path that is not there.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class VerifyPathTests(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.tmp = Path(tmp.name) + + def test_missing_path_reports_no_such_directory(self): + result = run("verify", SKILL, "--path", str(self.tmp / "absent")) + self.assertEqual(result.returncode, 1) + self.assertIn("no such directory", result.stderr) + + def test_installed_copy_verifies(self): + root = self.tmp / "skills" + self.assertEqual(run("install", SKILL, "--dir", str(root)).returncode, 0) + result = run("verify", SKILL, "--path", str(root / SKILL)) + self.assertEqual(result.returncode, 0, result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_verify_path_file.py b/tools/tests/test_cli_verify_path_file.py new file mode 100644 index 0000000..e549217 --- /dev/null +++ b/tools/tests/test_cli_verify_path_file.py @@ -0,0 +1,45 @@ +"""`verify --path` pointing at a file says "not a directory", not a raw ENOTDIR.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class VerifyPathFileTests(unittest.TestCase): + def test_file_path_is_reported_as_not_a_directory(self): + with tempfile.TemporaryDirectory() as tmp: + target = Path(tmp) / "SKILL.md" + target.write_text("x", encoding="utf8") + result = run("verify", SKILL, "--path", str(target)) + self.assertEqual(result.returncode, 1) + self.assertIn("not a directory", result.stderr) + self.assertNotIn("ENOTDIR", result.stderr) + + def test_missing_path_still_says_no_such_directory(self): + with tempfile.TemporaryDirectory() as tmp: + result = run("verify", SKILL, "--path", str(Path(tmp) / "absent")) + self.assertEqual(result.returncode, 1) + self.assertIn("no such directory", result.stderr) + + def test_installed_copy_still_verifies(self): + with tempfile.TemporaryDirectory() as tmp: + installed = run("install", SKILL, "--dir", tmp) + self.assertEqual(installed.returncode, 0, installed.stderr) + result = run("verify", SKILL, "--path", str(Path(tmp) / SKILL)) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(f"ok {SKILL}", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_version.py b/tools/tests/test_cli_version.py new file mode 100644 index 0000000..7f48555 --- /dev/null +++ b/tools/tests/test_cli_version.py @@ -0,0 +1,31 @@ +"""`intel-skills --version` prints the version package.json declares.""" + +import json +import shutil +import subprocess +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class VersionTests(unittest.TestCase): + def test_long_and_short_flag_print_the_package_version(self): + declared = json.loads((ROOT / "package.json").read_text(encoding="utf-8"))["version"] + for flag in ("--version", "-v"): + result = run(flag) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(result.stdout.strip(), declared) + + def test_usage_mentions_the_flag(self): + self.assertIn("--version", run("--help").stdout) + + +if __name__ == "__main__": + unittest.main() From 331efa7c0cccfd899aab687a5dd806cf74da7f0e Mon Sep 17 00:00:00 2001 From: vchamarthi <123653452+vchamarthi@users.noreply.github.com> Date: Fri, 23 Oct 2026 08:57:50 -0500 Subject: [PATCH 2/9] feat(cli): show --json, verify --all-problems, reject a misspelt --target with --dir show now refuses unknown options and extra skill names, and prints name, description, source and files as JSON with --json. verify --all-problems lists every drifted file instead of the first four. install and verify check --target even when --dir overrides the location. --- README.md | 1 + bin/intel-skills.mjs | 36 ++++++++----- tools/tests/test_cli_show_json.py | 59 +++++++++++++++++++++ tools/tests/test_cli_target_with_dir.py | 45 ++++++++++++++++ tools/tests/test_cli_verify_all_problems.py | 59 +++++++++++++++++++++ 5 files changed, 188 insertions(+), 12 deletions(-) create mode 100644 tools/tests/test_cli_show_json.py create mode 100644 tools/tests/test_cli_target_with_dir.py create mode 100644 tools/tests/test_cli_verify_all_problems.py diff --git a/README.md b/README.md index c2f66a5..e1a560c 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,7 @@ npx github:intel/skills install linux-perf xpu-port --target agents # several n npx github:intel/skills verify linux-perf --path ~/.claude/skills/linux-perf npx github:intel/skills verify linux-perf --target claude-code # same check, by target npx github:intel/skills verify --all --path ~/.agents/skills # every catalog skill, after install --all +npx github:intel/skills show linux-perf --json # name, description, source and files, for a script ``` Node 20 or newer, no dependencies, nothing from npm, and no network call after the diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index 9652b2e..c6afd1b 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -130,11 +130,12 @@ function files(dir) { } function targetRoot(target, dirOverride) { - if (dirOverride) return path.resolve(dirOverride); const segments = Object.hasOwn(INSTALL_TARGETS, target) ? INSTALL_TARGETS[target] : null; - if (!segments) { + // A misspelt --target is an error even when --dir wins, so a typo is not silently ignored. + if (!segments && (target || !dirOverride)) { throw new Error(`unknown target: ${target} (expected one of ${Object.keys(INSTALL_TARGETS).join(", ")})`); } + if (dirOverride) return path.resolve(dirOverride); return path.join(os.homedir(), ...segments); } @@ -190,7 +191,7 @@ function install(name, root, force, dryRun = false) { * matches the upstream an imported skill was copied from — that needs the network and * is `python3 tools/sync_external.py --check`. */ -function verify(name, dir) { +function verify(name, dir, allProblems = false) { const src = skillDir(name); const installed = path.resolve(dir); if (!fs.existsSync(installed)) throw new Error(`no such directory: ${installed}`); @@ -213,9 +214,10 @@ function verify(name, dir) { problems.push(`${rel} is not part of this skill`); } if (problems.length > 0) { + const shown = allProblems ? problems : problems.slice(0, 4); throw new Error( - `${name} at ${installed} does not match skills/${name}/: ${problems.slice(0, 4).join("; ")}` + - (problems.length > 4 ? ` (+${problems.length - 4} more)` : ""), + `${name} at ${installed} does not match skills/${name}/: ${shown.join("; ")}` + + (problems.length > shown.length ? ` (+${problems.length - shown.length} more; --all-problems lists them)` : ""), ); } console.log(`ok ${name} (${wanted.length} file(s) match skills/${name}/)`); @@ -279,10 +281,10 @@ function positional(args) { function usage(code = 0) { console.log(`Usage: intel-skills list [--json] [--imported|--own] - intel-skills show [--files] + intel-skills show [--files|--json] intel-skills install ...|--all --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] [--force] [--dry-run] intel-skills uninstall --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] - intel-skills verify --path + intel-skills verify --path [--all-problems] intel-skills verify --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] intel-skills --version intel-skills verify --all --path @@ -343,10 +345,14 @@ try { } } } else if (command === "show") { - const name = args[0]; + rejectUnknownOptions(args, [], ["--files", "--json"]); + const names = positionals(args); // Without this, `show` alone reported "invalid skill name: undefined", an error // about a value the user never typed. Every other command prints usage instead. - if (!name || name.startsWith("--")) usage(1); + if (names.length === 0) usage(1); + if (names.length > 1) throw new Error(`show takes one skill name (got extra argument ${names[1]})`); + const name = names[0]; + if (args.includes("--files") && args.includes("--json")) throw new Error("--files and --json cannot be combined"); const src = skillDir(name); if (args.includes("--files")) { // The exact set `install` would write, so a reviewer can see what lands on disk @@ -355,6 +361,11 @@ try { process.exit(0); } const origin = source(name); + if (args.includes("--json")) { + // What a script reads instead of scraping the SKILL.md dump below. + console.log(JSON.stringify({ name, description: description(name), source: origin, files: files(src) }, null, 2)); + process.exit(0); + } if (origin) { console.log(`upstream: ${origin.repo}/tree/${origin.commit}/${origin.path}`); console.log(`pinned: ${origin.commit}`); @@ -410,10 +421,11 @@ try { if (!name) usage(1); uninstall(name, targetRoot(target, dirOverride)); } else if (command === "verify") { - rejectUnknownOptions(args, ["--path", "--target", "--dir"], ["--all"]); + rejectUnknownOptions(args, ["--path", "--target", "--dir"], ["--all", "--all-problems"]); const target = argValue(args, "--target"); const dirOverride = argValue(args, "--dir"); const explicit = argValue(args, "--path"); + const allProblems = args.includes("--all-problems"); // `--target` names the place `install` wrote to, so verify can check it without the // caller spelling out the per-agent directory. if (!explicit && !target && !dirOverride) usage(1); @@ -427,7 +439,7 @@ try { const failures = []; for (const name of names) { try { - verify(name, path.join(root, name)); + verify(name, path.join(root, name), allProblems); } catch (error) { failures.push(`${name}: ${error.message}`); console.error(`error: ${name}: ${error.message}`); @@ -438,7 +450,7 @@ try { } else { const name = positional(args); if (!name) usage(1); - verify(name, explicit ?? path.join(targetRoot(target, dirOverride), name)); + verify(name, explicit ?? path.join(targetRoot(target, dirOverride), name), allProblems); } } else { console.error(`error: unknown command: ${command}`); diff --git a/tools/tests/test_cli_show_json.py b/tools/tests/test_cli_show_json.py new file mode 100644 index 0000000..ba3ce8b --- /dev/null +++ b/tools/tests/test_cli_show_json.py @@ -0,0 +1,59 @@ +"""`intel-skills show --json` prints one machine-readable object; show is strict about its arguments.""" + +import json +import shutil +import subprocess +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] +OTHER = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[1] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class ShowJsonTests(unittest.TestCase): + def test_describes_the_skill(self): + result = run("show", SKILL, "--json") + self.assertEqual(result.returncode, 0, result.stderr) + data = json.loads(result.stdout) + self.assertEqual(data["name"], SKILL) + self.assertTrue(data["description"]) + self.assertIn("SKILL.md", data["files"]) + self.assertEqual(data["files"], sorted(data["files"])) + + def test_flag_may_come_first(self): + result = run("show", "--json", SKILL) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(json.loads(result.stdout)["name"], SKILL) + + def test_rejects_unknown_options_and_extra_names(self): + for args in ( + (SKILL, "--jsno"), + (SKILL, OTHER), + ("--json",), + (SKILL, "--files", "--json"), + ): + with self.subTest(args=args): + result = run("show", *args) + self.assertEqual(result.returncode, 1) + self.assertEqual(result.stdout.lstrip()[:1] == "{", False) + + def test_extra_name_is_named_in_the_error(self): + result = run("show", SKILL, OTHER) + self.assertIn(f"extra argument {OTHER}", result.stderr) + + def test_plain_show_still_prints_the_skill_body(self): + result = run("show", SKILL) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn(f"name: {SKILL}", result.stdout) + self.assertFalse(result.stdout.lstrip().startswith("{")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_target_with_dir.py b/tools/tests/test_cli_target_with_dir.py new file mode 100644 index 0000000..ddd9cd1 --- /dev/null +++ b/tools/tests/test_cli_target_with_dir.py @@ -0,0 +1,45 @@ +"""`install --target` is validated even when `--dir` overrides the location.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" + + +def run(*args: str) -> subprocess.CompletedProcess[str]: + return subprocess.run( + ["node", str(CLI), *args], capture_output=True, text=True, check=False + ) + + +def first_skill() -> str: + return run("list").stdout.splitlines()[0].split("\t")[0] + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class InstallTargetTest(unittest.TestCase): + def test_unknown_target_with_dir_is_rejected(self): + with tempfile.TemporaryDirectory() as tmp: + result = run("install", first_skill(), "--target", "claud-code", "--dir", tmp) + self.assertEqual(result.returncode, 1) + self.assertIn("unknown target: claud-code", result.stderr) + self.assertEqual(list(Path(tmp).iterdir()), []) + + def test_known_target_with_dir_installs_into_dir(self): + with tempfile.TemporaryDirectory() as tmp: + name = first_skill() + result = run("install", name, "--target", "agents", "--dir", tmp) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertTrue((Path(tmp) / name / "SKILL.md").is_file()) + + def test_dir_alone_still_installs(self): + with tempfile.TemporaryDirectory() as tmp: + self.assertEqual(run("install", first_skill(), "--dir", tmp).returncode, 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/tests/test_cli_verify_all_problems.py b/tools/tests/test_cli_verify_all_problems.py new file mode 100644 index 0000000..9c2796d --- /dev/null +++ b/tools/tests/test_cli_verify_all_problems.py @@ -0,0 +1,59 @@ +"""`verify --all-problems` lists every drifted file instead of the first four.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[2] +CLI = ROOT / "bin" / "intel-skills.mjs" +SKILL = sorted(p.parent.name for p in (ROOT / "skills").glob("*/SKILL.md"))[0] + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class VerifyAllProblemsTests(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = Path(tmp.name) + self.assertEqual(run("install", SKILL, "--dir", str(self.root)).returncode, 0) + self.skill = self.root / SKILL + + def add_strays(self, count): + for i in range(count): + (self.skill / f"stray-{i}.txt").write_text("x") + + def test_default_truncates_after_four_problems(self): + self.add_strays(6) + result = run("verify", SKILL, "--path", str(self.skill)) + self.assertEqual(result.returncode, 1) + self.assertIn("(+2 more; --all-problems lists them)", result.stderr) + self.assertNotIn("stray-5.txt", result.stderr) + + def test_all_problems_lists_every_file(self): + self.add_strays(6) + result = run("verify", SKILL, "--path", str(self.skill), "--all-problems") + self.assertEqual(result.returncode, 1) + for i in range(6): + self.assertIn(f"stray-{i}.txt is not part of this skill", result.stderr) + self.assertNotIn("(+", result.stderr) + + def test_clean_install_still_verifies_with_flag(self): + result = run("verify", SKILL, "--path", str(self.skill), "--all-problems") + self.assertEqual(result.returncode, 0) + self.assertTrue(result.stdout.startswith(f"ok {SKILL}")) + + def test_flag_is_accepted_by_verify_all(self): + install = run("install", "--all", "--dir", str(self.root / "all")) + self.assertEqual(install.returncode, 0, install.stderr) + result = run("verify", "--all", "--path", str(self.root / "all"), "--all-problems") + self.assertEqual(result.returncode, 0, result.stderr) + + +if __name__ == "__main__": + unittest.main() From 9928308b83cb7306e3c12ed6881f32b2e504eee8 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:26:39 -0700 Subject: [PATCH 3/9] test(cli): skip the remaining CLI tests when node is absent Four new test files called node unconditionally, so without node the suite raised 18 FileNotFoundError errors instead of skipping, contrary to the workflow comment. They now carry the same skipIf as the others. Signed-off-by: Alexander Rybkin --- tools/tests/test_cli_help.py | 2 ++ tools/tests/test_cli_install_many.py | 1 + tools/tests/test_cli_unknown_command.py | 2 ++ tools/tests/test_cli_verify_all.py | 1 + 4 files changed, 6 insertions(+) diff --git a/tools/tests/test_cli_help.py b/tools/tests/test_cli_help.py index 0421de4..e18b14c 100644 --- a/tools/tests/test_cli_help.py +++ b/tools/tests/test_cli_help.py @@ -1,5 +1,6 @@ """`--help` after any installer subcommand prints usage and exits 0.""" +import shutil import subprocess import unittest from pathlib import Path @@ -13,6 +14,7 @@ def run(*args: str) -> subprocess.CompletedProcess[str]: ) +@unittest.skipIf(shutil.which("node") is None, "node not installed") class SubcommandHelpTest(unittest.TestCase): def test_subcommand_help_prints_usage(self) -> None: for args in ( diff --git a/tools/tests/test_cli_install_many.py b/tools/tests/test_cli_install_many.py index 6b74d31..2c5a733 100644 --- a/tools/tests/test_cli_install_many.py +++ b/tools/tests/test_cli_install_many.py @@ -17,6 +17,7 @@ def run(*args): ) +@unittest.skipIf(shutil.which("node") is None, "node not installed") class InstallManyTest(unittest.TestCase): def setUp(self): self.dir = Path(tempfile.mkdtemp()) diff --git a/tools/tests/test_cli_unknown_command.py b/tools/tests/test_cli_unknown_command.py index 6af3de5..afbb64a 100644 --- a/tools/tests/test_cli_unknown_command.py +++ b/tools/tests/test_cli_unknown_command.py @@ -1,5 +1,6 @@ """An unrecognised installer command names itself instead of only printing usage.""" +import shutil import subprocess import sys import unittest @@ -14,6 +15,7 @@ def run(*args): ) +@unittest.skipIf(shutil.which("node") is None, "node not installed") class UnknownCommandTest(unittest.TestCase): def test_unknown_command_is_named_on_stderr(self): result = run("instal", "linux-perf") diff --git a/tools/tests/test_cli_verify_all.py b/tools/tests/test_cli_verify_all.py index a1e55c6..4adeb04 100644 --- a/tools/tests/test_cli_verify_all.py +++ b/tools/tests/test_cli_verify_all.py @@ -17,6 +17,7 @@ def run(*args): ) +@unittest.skipIf(shutil.which("node") is None, "node not installed") class VerifyAllTest(unittest.TestCase): def setUp(self): self.dir = Path(tempfile.mkdtemp()) From 5eea0b3c5a4c6492f3aefba543403fe4356d23df Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:27:41 -0700 Subject: [PATCH 4/9] ci: assert install --all --dry-run writes nothing The only dry-run check used one name, so dropping dryRun from the multi-name loop survived every test and step while writing the whole catalog. Signed-off-by: Alexander Rybkin --- .github/workflows/validate.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1a23fa8..bcb089e 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -212,6 +212,8 @@ jobs: set -euo pipefail node bin/intel-skills.mjs install linux-perf --dir "${RUNNER_TEMP}/dry" --dry-run test ! -e "${RUNNER_TEMP}/dry" + node bin/intel-skills.mjs install --all --dir "${RUNNER_TEMP}/dryall" --dry-run + test ! -e "${RUNNER_TEMP}/dryall" # Several names in one call: every one is written, and a bad name fails the run # without stopping the others. From 514e562bc3c5d0ba907ed2e91b8108de2d101409 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:28:57 -0700 Subject: [PATCH 5/9] fix(cli): uninstall rejects unknown options and extra names uninstall was the one command without rejectUnknownOptions and read only its first name, so 'uninstall X --dry-run' deleted X and 'uninstall a b' removed a alone, both with exit 0. Signed-off-by: Alexander Rybkin --- .github/workflows/validate.yml | 12 ++++++++++++ bin/intel-skills.mjs | 7 +++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index bcb089e..1fe3ead 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -274,3 +274,15 @@ jobs: echo "::error::uninstall accepted a path-like name" exit 1 fi + node bin/intel-skills.mjs install linux-perf dpnp-quickstart --dir "${RUNNER_TEMP}/un" + if node bin/intel-skills.mjs uninstall linux-perf --dir "${RUNNER_TEMP}/un" --dry-run; then + echo "::error::uninstall accepted an option it does not take" + exit 1 + fi + test -d "${RUNNER_TEMP}/un/linux-perf" + if node bin/intel-skills.mjs uninstall linux-perf dpnp-quickstart --dir "${RUNNER_TEMP}/un"; then + echo "::error::uninstall accepted a second skill name" + exit 1 + fi + test -d "${RUNNER_TEMP}/un/linux-perf" + test -d "${RUNNER_TEMP}/un/dpnp-quickstart" diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index c6afd1b..7ed2531 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -414,11 +414,14 @@ try { } } } else if (command === "uninstall") { + rejectUnknownOptions(args, ["--target", "--dir"], []); const target = argValue(args, "--target"); const dirOverride = argValue(args, "--dir"); if (!target && !dirOverride) usage(1); - const name = positional(args); - if (!name) usage(1); + const names = positionals(args); + if (names.length === 0) usage(1); + if (names.length > 1) throw new Error(`uninstall takes one skill name (got extra argument ${names[1]})`); + const name = names[0]; uninstall(name, targetRoot(target, dirOverride)); } else if (command === "verify") { rejectUnknownOptions(args, ["--path", "--target", "--dir"], ["--all", "--all-problems"]); From 9942b2fa44e49ef3041abc072838b216913bf13c Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:31:12 -0700 Subject: [PATCH 6/9] fix(cli): uninstall only removes skills in the catalog uninstall checked only for //SKILL.md, so a user's own skill or another hub's (my-notes) was deleted with exit 0, despite the JSDoc's promise. It now resolves the name through skillDir first. Signed-off-by: Alexander Rybkin --- .github/workflows/validate.yml | 7 +++++++ bin/intel-skills.mjs | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1fe3ead..1e57f86 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -286,3 +286,10 @@ jobs: fi test -d "${RUNNER_TEMP}/un/linux-perf" test -d "${RUNNER_TEMP}/un/dpnp-quickstart" + mkdir -p "${RUNNER_TEMP}/un/my-notes" + echo x > "${RUNNER_TEMP}/un/my-notes/SKILL.md" + if node bin/intel-skills.mjs uninstall my-notes --dir "${RUNNER_TEMP}/un"; then + echo "::error::uninstall removed a skill that is not in the catalog" + exit 1 + fi + test -f "${RUNNER_TEMP}/un/my-notes/SKILL.md" diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index 7ed2531..43fc6bd 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -226,12 +226,12 @@ function verify(name, dir, allProblems = false) { /** * Remove an installed skill from a target root. * - * Deliberately narrow: the name must be a catalog-shaped name, the directory must hold a - * SKILL.md (so a typo cannot delete an unrelated folder), and it must stay inside the - * root. It never touches anything but `/`. + * Deliberately narrow: the name must be in the catalog (so a typo or another hub's skill + * cannot delete an unrelated folder), the directory must hold a SKILL.md, and it must stay + * inside the root. It never touches anything but `/`. */ function uninstall(name, root) { - if (!NAME.test(String(name))) throw new Error(`invalid skill name: ${name}`); + skillDir(name); const dest = path.join(root, name); if (!path.resolve(dest).startsWith(`${path.resolve(root)}${path.sep}`)) { throw new Error("uninstall path escapes target directory"); From f772d1c277ba6c34272dc45db181af33b97cca62 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:32:48 -0700 Subject: [PATCH 7/9] fix(cli): verify refuses extra names and a name with --all verify read only its first name, so 'verify linux-perf xpu-discover' checked one skill and exited 0 even if the other had drifted, and a name next to --all was dropped without a word. Signed-off-by: Alexander Rybkin --- bin/intel-skills.mjs | 15 ++++---- tools/tests/test_cli_verify_extra_names.py | 41 ++++++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) create mode 100644 tools/tests/test_cli_verify_extra_names.py diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index 43fc6bd..0a6688f 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -274,10 +274,6 @@ function positionals(args) { return found; } -function positional(args) { - return positionals(args)[0] ?? null; -} - function usage(code = 0) { console.log(`Usage: intel-skills list [--json] [--imported|--own] @@ -432,15 +428,18 @@ try { // `--target` names the place `install` wrote to, so verify can check it without the // caller spelling out the per-agent directory. if (!explicit && !target && !dirOverride) usage(1); + const names = positionals(args); + if (names.length > 1) throw new Error(`verify takes one skill name (got extra argument ${names[1]})`); if (args.includes("--all")) { + if (names.length > 0) throw new Error("--all verifies every skill; do not also name one"); // The counterpart of `install --all`: the directory it wrote into, with every // catalog skill checked under it. Each is attempted and every failure is listed, // for the same reason `install --all` does not stop at the first one. const root = path.resolve(explicit ?? targetRoot(target, dirOverride)); if (!fs.existsSync(root)) throw new Error(`no such directory: ${root}`); - const names = catalog(); + const all = catalog(); const failures = []; - for (const name of names) { + for (const name of all) { try { verify(name, path.join(root, name), allProblems); } catch (error) { @@ -448,10 +447,10 @@ try { console.error(`error: ${name}: ${error.message}`); } } - console.log(`${names.length - failures.length}/${names.length} verified under ${root}`); + console.log(`${all.length - failures.length}/${all.length} verified under ${root}`); if (failures.length > 0) process.exit(1); } else { - const name = positional(args); + const name = names[0]; if (!name) usage(1); verify(name, explicit ?? path.join(targetRoot(target, dirOverride), name), allProblems); } diff --git a/tools/tests/test_cli_verify_extra_names.py b/tools/tests/test_cli_verify_extra_names.py new file mode 100644 index 0000000..3584704 --- /dev/null +++ b/tools/tests/test_cli_verify_extra_names.py @@ -0,0 +1,41 @@ +"""`verify` refuses a second skill name, or a name next to `--all`, instead of ignoring it.""" + +import shutil +import subprocess +import tempfile +import unittest +from pathlib import Path + +CLI = Path(__file__).resolve().parents[2] / "bin" / "intel-skills.mjs" + + +def run(*args): + return subprocess.run(["node", str(CLI), *args], capture_output=True, text=True, check=False) + + +@unittest.skipIf(shutil.which("node") is None, "node not installed") +class VerifyExtraNamesTests(unittest.TestCase): + def setUp(self): + tmp = tempfile.TemporaryDirectory() + self.addCleanup(tmp.cleanup) + self.root = Path(tmp.name) + self.assertEqual(run("install", "--all", "--dir", str(self.root)).returncode, 0) + + def test_second_name_is_refused(self): + result = run("verify", "linux-perf", "xpu-discover", "--path", str(self.root / "linux-perf")) + self.assertNotEqual(result.returncode, 0) + self.assertIn("verify takes one skill name", result.stderr) + + def test_name_with_all_is_refused(self): + result = run("verify", "linux-perf", "--all", "--path", str(self.root)) + self.assertNotEqual(result.returncode, 0) + self.assertIn("do not also name one", result.stderr) + + def test_one_name_and_all_alone_still_verify(self): + self.assertEqual(run("verify", "linux-perf", "--path", str(self.root / "linux-perf")).returncode, 0) + self.assertEqual(run("verify", "linux-perf", "--dir", str(self.root)).returncode, 0) + self.assertEqual(run("verify", "--all", "--path", str(self.root)).returncode, 0) + + +if __name__ == "__main__": + unittest.main() From ab7ea42c4f0d7779ddad877fdba2a99f2e60b8d5 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Mon, 26 Oct 2026 15:42:51 -0700 Subject: [PATCH 8/9] ci: keep the SKILL.md guard of uninstall covered The catalog check now refuses the old not-a-skill case first, so no CI case reached the SKILL.md guard: deleting it left the step green. A catalog name with no SKILL.md under it covers that branch again. Signed-off-by: Alexander Rybkin --- .github/workflows/validate.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 1e57f86..fce88d8 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -293,3 +293,9 @@ jobs: exit 1 fi test -f "${RUNNER_TEMP}/un/my-notes/SKILL.md" + mkdir "${RUNNER_TEMP}/un/xpu-discover" + if node bin/intel-skills.mjs uninstall xpu-discover --dir "${RUNNER_TEMP}/un"; then + echo "::error::uninstall removed a catalog-named directory with no SKILL.md" + exit 1 + fi + test -d "${RUNNER_TEMP}/un/xpu-discover" From cc5bd8fa310511173668f518d028db5400f2e910 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Tue, 27 Oct 2026 09:38:13 -0700 Subject: [PATCH 9/9] feat(cli): uninstall --force removes a skill no longer in the catalog uninstall takes catalog names only, so a skill dropped from the catalog after it was installed could not be removed. --force skips only that check; the SKILL.md guard, the name check and the root check still apply. Without it the error names --force. Signed-off-by: Alexander Rybkin --- .github/workflows/validate.yml | 12 ++++++++++++ bin/intel-skills.mjs | 20 +++++++++++++------- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index fce88d8..4cd3e53 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -299,3 +299,15 @@ jobs: exit 1 fi test -d "${RUNNER_TEMP}/un/xpu-discover" + node bin/intel-skills.mjs uninstall my-notes --dir "${RUNNER_TEMP}/un" --force + test ! -e "${RUNNER_TEMP}/un/my-notes" + mkdir "${RUNNER_TEMP}/un/not-mine" + if node bin/intel-skills.mjs uninstall not-mine --dir "${RUNNER_TEMP}/un" --force; then + echo "::error::uninstall --force removed a directory with no SKILL.md" + exit 1 + fi + test -d "${RUNNER_TEMP}/un/not-mine" + if node bin/intel-skills.mjs uninstall ../un --dir "${RUNNER_TEMP}/un" --force; then + echo "::error::uninstall --force accepted a path-like name" + exit 1 + fi diff --git a/bin/intel-skills.mjs b/bin/intel-skills.mjs index 0a6688f..60672e8 100755 --- a/bin/intel-skills.mjs +++ b/bin/intel-skills.mjs @@ -227,18 +227,24 @@ function verify(name, dir, allProblems = false) { * Remove an installed skill from a target root. * * Deliberately narrow: the name must be in the catalog (so a typo or another hub's skill - * cannot delete an unrelated folder), the directory must hold a SKILL.md, and it must stay - * inside the root. It never touches anything but `/`. + * cannot delete an unrelated folder) unless `force` is set, for a skill since dropped from + * it; the directory must hold a SKILL.md, and it must stay inside the root. It never + * touches anything but `/`. */ -function uninstall(name, root) { - skillDir(name); +function uninstall(name, root, force = false) { + if (!NAME.test(String(name))) throw new Error(`invalid skill name: ${name}`); + const inCatalog = fs.existsSync(path.join(SKILLS_DIR, name, "SKILL.md")); const dest = path.join(root, name); if (!path.resolve(dest).startsWith(`${path.resolve(root)}${path.sep}`)) { throw new Error("uninstall path escapes target directory"); } if (!fs.existsSync(path.join(dest, "SKILL.md"))) { + if (!inCatalog) skillDir(name); throw new Error(`not an installed skill: ${dest}`); } + if (!inCatalog && !force) { + throw new Error(`skill not found: ${name} is not in the catalog; pass --force to remove ${dest} anyway`); + } fs.rmSync(dest, { recursive: true, force: true }); console.log(`removed ${name} from ${dest}`); } @@ -279,7 +285,7 @@ function usage(code = 0) { intel-skills list [--json] [--imported|--own] intel-skills show [--files|--json] intel-skills install ...|--all --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] [--force] [--dry-run] - intel-skills uninstall --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] + intel-skills uninstall --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] [--force] intel-skills verify --path [--all-problems] intel-skills verify --target ${Object.keys(INSTALL_TARGETS).join("|")} [--dir ] intel-skills --version @@ -410,7 +416,7 @@ try { } } } else if (command === "uninstall") { - rejectUnknownOptions(args, ["--target", "--dir"], []); + rejectUnknownOptions(args, ["--target", "--dir"], ["--force"]); const target = argValue(args, "--target"); const dirOverride = argValue(args, "--dir"); if (!target && !dirOverride) usage(1); @@ -418,7 +424,7 @@ try { if (names.length === 0) usage(1); if (names.length > 1) throw new Error(`uninstall takes one skill name (got extra argument ${names[1]})`); const name = names[0]; - uninstall(name, targetRoot(target, dirOverride)); + uninstall(name, targetRoot(target, dirOverride), args.includes("--force")); } else if (command === "verify") { rejectUnknownOptions(args, ["--path", "--target", "--dir"], ["--all", "--all-problems"]); const target = argValue(args, "--target");