From dcfff3f91793ad2a0fb5e7a7c1bdecfbde737117 Mon Sep 17 00:00:00 2001 From: sysradium Date: Sun, 1 Feb 2026 23:51:00 +0100 Subject: [PATCH] feat: add delete command for removing skills --- README.md | 25 +++++++++++++++++ src/upskill/cli.py | 50 +++++++++++++++++++++++++++++++++ tests/test_delete_command.py | 54 ++++++++++++++++++++++++++++++++++++ uv.lock | 4 +-- 4 files changed, 131 insertions(+), 2 deletions(-) create mode 100644 tests/test_delete_command.py diff --git a/README.md b/README.md index 7066642..2f429a2 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,31 @@ upskill list -v └── scripts/validate.py ``` +### `upskill delete` + +Delete a generated skill safely from your skills directory. + +```bash +upskill delete SKILL_NAME [OPTIONS] +``` + +**Arguments:** +- `SKILL_NAME` - Name of the skill directory to remove (not a path) + +**Options:** +- `-d, --dir PATH` - Skills directory (default: configured `skills_dir`) +- `--force` - Skip confirmation prompt + +**Examples:** + +```bash +# Confirm before deleting +upskill delete git-commit-messages + +# Delete without confirmation +upskill delete my-skill --force +``` + ### `upskill runs` View run results as a plot, or export to CSV. By default, shows a visual comparison of baseline vs with-skill performance. diff --git a/src/upskill/cli.py b/src/upskill/cli.py index 6e8c680..4cf5f5a 100644 --- a/src/upskill/cli.py +++ b/src/upskill/cli.py @@ -3,6 +3,7 @@ import asyncio import json +import shutil import sys from collections.abc import AsyncIterator from contextlib import asynccontextmanager @@ -1117,6 +1118,55 @@ def list_cmd(skills_dir: str | None, verbose: bool): console.print(f"[dim]{len(skills)} skills, ~{total_tokens:,} total tokens[/dim]") +@main.command("delete") +@click.argument("skill_name") +@click.option("-d", "--dir", "skills_dir", type=click.Path(), help="Skills directory") +@click.option("--force", is_flag=True, help="Delete without confirmation") +def delete_cmd(skill_name: str, skills_dir: str | None, force: bool): + """Delete a generated skill directory safely. + + Examples: + + upskill delete git-commit-messages + + upskill delete my-skill --force + """ + config = Config.load() + base_dir = Path(skills_dir) if skills_dir else config.skills_dir + + # Safety guard: only allow deleting a single skill directory name + if Path(skill_name).name != skill_name or skill_name in {".", ".."}: + console.print("[red]Skill name must be a directory name, not a path.[/red]") + sys.exit(1) + + skill_path = (base_dir / skill_name).resolve() + if not skill_path.is_relative_to(base_dir.resolve()): + console.print("[red]Refusing to delete outside the skills directory.[/red]") + sys.exit(1) + + if not skill_path.exists(): + console.print(f"[red]Skill not found: {skill_path}[/red]") + sys.exit(1) + if not skill_path.is_dir(): + console.print(f"[red]Not a directory: {skill_path}[/red]") + sys.exit(1) + if not (skill_path / "SKILL.md").exists(): + console.print( + f"[red]{skill_path} does not look like a skill directory (missing SKILL.md).[/red]" + ) + sys.exit(1) + + if not force: + click.confirm( + f"Delete skill '{skill_name}' at {skill_path}?", + default=False, + abort=True, + ) + + shutil.rmtree(skill_path) + console.print(f"[green]Deleted skill:[/green] {skill_path}") + + @main.command("benchmark") @click.argument("skill_path", type=click.Path(exists=True)) @click.option("-m", "--model", "models", multiple=True, required=True, help="Model to benchmark") diff --git a/tests/test_delete_command.py b/tests/test_delete_command.py new file mode 100644 index 0000000..199fc53 --- /dev/null +++ b/tests/test_delete_command.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from pathlib import Path + +from click.testing import CliRunner + +from upskill.cli import main + + +def test_delete_command_removes_skill_with_force() -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + skill_dir = Path("skills/git-commit-messages") + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("---\nname: git-commit-messages\n---\n") + + result = runner.invoke(main, ["delete", "git-commit-messages", "--force"]) + + assert result.exit_code == 0 + assert not skill_dir.exists() + + +def test_delete_command_requires_confirmation_by_default() -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + skill_dir = Path("skills/my-skill") + skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("---\nname: my-skill\n---\n") + + result = runner.invoke(main, ["delete", "my-skill"], input="y\n") + + assert result.exit_code == 0 + assert not skill_dir.exists() + + +def test_delete_command_rejects_path_like_name() -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + result = runner.invoke(main, ["delete", "../oops", "--force"]) + + assert result.exit_code == 1 + assert "must be a directory name, not a path" in result.output + + +def test_delete_command_rejects_non_skill_dir() -> None: + runner = CliRunner() + with runner.isolated_filesystem(): + not_skill = Path("skills/random-folder") + not_skill.mkdir(parents=True) + + result = runner.invoke(main, ["delete", "random-folder", "--force"]) + + assert result.exit_code == 1 + assert "does not look like a skill directory" in result.output diff --git a/uv.lock b/uv.lock index dd46d91..f06bb73 100644 --- a/uv.lock +++ b/uv.lock @@ -1,5 +1,5 @@ version = 1 -revision = 3 +revision = 2 requires-python = ">=3.13.5, <3.14" [[package]] @@ -1727,7 +1727,7 @@ wheels = [ [[package]] name = "upskill" -version = "0.2.0" +version = "0.2.1" source = { editable = "." } dependencies = [ { name = "click" },