From 7883a331822c07adaaee34c958f011b7e94043b1 Mon Sep 17 00:00:00 2001 From: igor-ctrl Date: Sun, 17 May 2026 18:06:12 -0500 Subject: [PATCH] =?UTF-8?q?feat(skill):=20bcli=20skill=20install=20?= =?UTF-8?q?=E2=80=94=20slash=20command=20projection=20(AIP=20=C2=A7Phase?= =?UTF-8?q?=207)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generate Claude Code slash commands from saved queries + batch templates. New ``bcli skill install [--target PATH] [--dry-run]``: - Reads the active profile's saved queries from ``~/.config/bcli/queries/.yaml`` and batch templates from ``~/.config/bcli/batches//*.yaml``. - Emits one ``.claude/commands/bcli-.md`` per item with frontmatter (description, argument-hint), a usage section, and a body that invokes ``bcli q arg=$1 …`` or ``bcli batch run …``. - Emits a top-level ``.claude/skills/bcli/SKILL.md`` index grouped by ``categories:``. - Idempotent via SHA-256 content hash embedded in the provenance comment; re-runs on unchanged sources are no-ops (mtime preserved). No ``generated_at`` timestamp — would invalidate the hash on every run and cause infinite churn. - ``manual: true`` in a file's YAML frontmatter protects it from regeneration even if a saved query by the same name exists. - Target resolution: explicit ``--target`` wins; else CWD when it has a ``.claude/`` dir; else ``$HOME``. Matches Claude Code's per-project vs global skill convention. - Atomic writes (.tmp + os.replace) for files that change. ``bcli skill`` is registered as a Typer GROUP so Worker B's Phase 7 ``bcli skill init`` wizard can land alongside without restructuring. Saved-query YAML schema extension (additive, backward-compatible): - ``description``: already supported. - ``categories: [str]``: new; falls back to ``["unsorted"]``. Used to group commands in the SKILL.md index. - ``args: [{name, type, required, example}]``: new; explicit positional ordering for the generated slash command. **If omitted, derived from ``params:`` keys** (required first, optional with ``default:`` second, both in YAML insertion order). Most existing queries need zero changes — their slash command works on the inferred ordering. Batch template discovery is keyed to ``~/.config/bcli/batches/ /*.yaml`` per the file conventions established elsewhere. CWD-relative discovery deferred to v0.2 (open issue if needed). Stdlib only — string templates, no jinja2. Tests: +12 in tests/test_skill_install/test_skill_install.py covering per-query and per-batch generation, args inference, argument-hint rendering, skill index grouping/sorting, idempotency on unchanged sources, regeneration on source changes, manual-frontmatter preservation, dry-run, content-hash integrity, target-dir resolution (CWD detection), and empty-input no-op. docs/saved-queries.md updated with the schema extension and a new "Slash-command projection" section. Suite: 799 → 811 passed. Ruff clean. Hermetic (HOME=/tmp/empty): clean across tests/test_skill_install/ tests/test_describe/ tests/test_idempotency/. --- docs/saved-queries.md | 75 ++- src/bcli_cli/app.py | 5 + src/bcli_cli/commands/skill_cmd.py | 599 ++++++++++++++++++ tests/test_skill_install/__init__.py | 1 + .../test_skill_install/test_skill_install.py | 484 ++++++++++++++ 5 files changed, 1163 insertions(+), 1 deletion(-) create mode 100644 src/bcli_cli/commands/skill_cmd.py create mode 100644 tests/test_skill_install/__init__.py create mode 100644 tests/test_skill_install/test_skill_install.py diff --git a/docs/saved-queries.md b/docs/saved-queries.md index 0b79980..09ab02d 100644 --- a/docs/saved-queries.md +++ b/docs/saved-queries.md @@ -59,7 +59,9 @@ queries: | Field | Type | Notes | |----------------|----------|----------------------------------------------------------| -| `description` | string | Shown in `bcli q` listing. | +| `description` | string | Shown in `bcli q` listing **and** the generated slash command's frontmatter. | +| `categories` | list[str] | Optional. Used by `bcli skill install` to group commands in the generated `SKILL.md` index. Falls back to `["unsorted"]`. | +| `args` | list[obj] | Optional. Explicit positional ordering for the generated slash command. If omitted, inferred from `params:` keys (required first, optional second). See *Slash-command projection* below. | | `endpoint` | string | Required. Entity-set name (resolved through the registry).| | `params` | mapping | Optional. Each key declares a parameter; see *Param declarations* below. | | `filter` | string | OData `$filter`. Supports `${{ params.X }}` substitution. | @@ -162,3 +164,74 @@ bcli --profile ops q items-low-stock min=10 * `--format` — override the active profile's output format (`json`, `markdown`, `csv`, `ndjson`, `table`). * `--dry-run` (global) — skips execution after resolving. + +## Slash-command projection (`bcli skill install`) + +`bcli skill install` reads the saved queries for the active profile and +generates one Claude Code slash command per query at +`/.claude/commands/bcli-.md`, plus a top-level skill index +at `/.claude/skills/bcli/SKILL.md` grouped by `categories:`. + +Three optional fields on each query feed the generator: + +```yaml +queries: + utilization-by-esn: + description: Engine utilization (cycles, hours, FSN) for an ESN + categories: [aviation, daily-ops] + args: + - name: esn + type: string + example: "424322" + required: true + # existing fields below — params/filter/select/etc. + endpoint: util_history + params: + esn: {required: true} + filter: "engine_serial eq '${{ params.esn }}'" +``` + +* `description` — used as the slash command's frontmatter `description:` + and listed under its category in `SKILL.md`. +* `categories` — list of strings. Each category becomes a section in + `SKILL.md`. Queries with no categories land under `unsorted`. +* `args` — explicit positional ordering for the generated slash command + body. Each entry: `{name, type, required, example}`. **If omitted, the + generator derives `args:` from `params:` keys** (required first, + optional with `default:` second, both in YAML insertion order). For + most queries you can leave `args:` out and the projected command will + still work; declare it explicitly when you want a different ordering + than the params dict gives you. + +The generated command body invokes `bcli q arg1=$1 arg2=$2 … +--format json`, so the positional → key mapping in the slash command +matches your `args:` order. + +### Idempotency and manual overrides + +* Each generated file embeds a `content_hash: sha256:…` line in its + provenance comment. Re-running `bcli skill install` on unchanged + sources is a no-op (hash matches → file isn't rewritten). +* To protect a hand-edited slash command file from regeneration, add + `manual: true` to its YAML frontmatter: + ```markdown + --- + manual: true + description: My customised command + --- + ``` + The installer skips any file whose frontmatter declares `manual: true`, + even if a saved query by the same name exists. +* Add `--dry-run` to preview without writing; add `--target PATH` to + point at a specific project root (defaults to CWD when it contains a + `.claude/` directory, else `$HOME`). + +### Batch templates + +Batch workflow YAMLs under `~/.config/bcli/batches//*.yaml` are +projected the same way as saved queries — one +`.claude/commands/bcli-batch-.md` per file, body invoking +`bcli batch run --set arg=$1 --format json --result-out …`. + +CWD-relative batch discovery (a `batches/` directory checked into a +project repo) is a follow-up — open an issue if you need it. diff --git a/src/bcli_cli/app.py b/src/bcli_cli/app.py index eaf9f43..8cc34a8 100644 --- a/src/bcli_cli/app.py +++ b/src/bcli_cli/app.py @@ -175,6 +175,7 @@ def _emit_command_summary() -> None: query_cmd, registry_cmd, skill_init_cmd, + skill_cmd, # noqa: F401 — import-time side effect registers `skill install` test_cmd, ) @@ -187,6 +188,10 @@ def _emit_command_summary() -> None: app.add_typer(test_cmd.app, name="test", help="Connection and endpoint testing") app.add_typer(batch_cmd.app, name="batch", help="Batch operations from YAML files") app.add_typer(attach_cmd.app, name="attach", help="Document-attachment workflows (two-phase /attachments upload)") +# `skill` is a single Typer group shared by ``skill_init_cmd`` (init / update) +# and ``skill_cmd`` (install). PR #19 owns the group registration; the +# ``skill_cmd`` module aliases ``skill_init_cmd.app`` so ``@app.command("install")`` +# attaches to the same group without a duplicate ``add_typer`` call. app.add_typer(skill_init_cmd.app, name="skill", help="Generate a per-user bcli skill bundle (AIP Phase 7)") app.command(name="get")(get_cmd.get_command) app.command(name="post")(post_cmd.post_command) diff --git a/src/bcli_cli/commands/skill_cmd.py b/src/bcli_cli/commands/skill_cmd.py new file mode 100644 index 0000000..5b918a4 --- /dev/null +++ b/src/bcli_cli/commands/skill_cmd.py @@ -0,0 +1,599 @@ +"""``bcli skill`` — Typer group for skill projection (Phase 6 / Phase 7). + +``bcli skill install`` + Generates a slash-command-per-saved-query and slash-command-per-batch + Markdown bundle suitable for Claude Code at + ``/.claude/commands/bcli-.md``, plus a top-level + ``/.claude/skills/bcli/SKILL.md`` index grouped by + ``categories:``. + + Idempotent — each generated file embeds a ``content_hash:`` over the + rest of its body; re-runs that produce the same hash are a no-op. + Hand-edited files with ``manual: true`` in their YAML frontmatter are + NEVER overwritten. + +Worker B (Phase 7) will add ``bcli skill init`` to this group; that's +why we register ``skill`` as a Typer group from the start rather than a +flat command. +""" + +from __future__ import annotations + +import hashlib +import os +import re +import tempfile +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any + +import typer +import yaml +from rich.console import Console + +from bcli.config._defaults import CONFIG_DIR +from bcli_cli._state import state + +# Per-profile saved queries (mirror ``query_cmd.QUERIES_DIR``) and +# batch-template directory. Imported via module attrs so tests can +# monkeypatch with ``setattr(... raising=False)``. +QUERIES_DIR = CONFIG_DIR / "queries" +BATCHES_DIR = CONFIG_DIR / "batches" + + +# `bcli skill` is a single Typer group shared by ``skill_init_cmd`` +# (``init`` / ``update`` wizards — PR #19) and this module +# (``install``). PR #19 was approved first and owns the group's +# top-level registration in ``app.py``; we alias its Typer instance +# so ``@app.command("install")`` below attaches to the *same* group. +# This avoids a duplicate ``add_typer(..., name="skill", ...)`` (which +# Typer raises on) and keeps ``bcli skill --help`` listing init, +# update, and install side by side. +from bcli_cli.commands import skill_init_cmd # noqa: E402 + +app = skill_init_cmd.app + +console = Console() +err = Console(stderr=True) + + +# ─── Data types ────────────────────────────────────────────────────── + + +@dataclass(frozen=True) +class _Arg: + """One CLI argument on a generated slash command.""" + name: str + type: str = "string" + required: bool = False + example: str = "" + + +@dataclass +class _SlashItem: + """A single Markdown file to write under .claude/commands/.""" + kind: str # "query" | "batch" + name: str # base name (e.g. "vendor-by-no") + slug: str # filename slug (e.g. "bcli-vendor-by-no") + description: str + categories: list[str] = field(default_factory=list) + args: list[_Arg] = field(default_factory=list) + # Source-specific metadata used in the body template. + source_path: str = "" # absolute path of the originating yaml + batch_relpath: str = "" # relative path passed to ``bcli batch run`` + + +# ─── YAML parsing helpers (additive — no breaking changes) ───────── + + +def _coerce_args(raw: Any, params: dict[str, Any] | None = None) -> list[_Arg]: + """Resolve a query/batch's ``args:`` list, deriving from ``params:`` if absent. + + Order: explicit ``args:`` (manual override) → derive from ``params:`` + keys with required first, optional second, both in YAML insertion + order. Returns an empty list when neither is present. + """ + if isinstance(raw, list) and raw: + out: list[_Arg] = [] + for entry in raw: + if not isinstance(entry, dict) or "name" not in entry: + continue + out.append(_Arg( + name=str(entry["name"]), + type=str(entry.get("type") or "string"), + required=bool(entry.get("required", False)), + example=str(entry.get("example") or ""), + )) + if out: + return out + + if not isinstance(params, dict): + return [] + + required_first: list[_Arg] = [] + optional_after: list[_Arg] = [] + for key, value in params.items(): + spec = value if isinstance(value, dict) else {} + arg = _Arg( + name=str(key), + type=str(spec.get("type") or "string"), + required=bool(spec.get("required", False)), + example=str(spec.get("example") or ""), + ) + if arg.required: + required_first.append(arg) + else: + optional_after.append(arg) + return required_first + optional_after + + +def _coerce_categories(raw: Any) -> list[str]: + if isinstance(raw, list) and raw: + return [str(c) for c in raw if str(c).strip()] + return ["unsorted"] + + +def _load_queries(profile_name: str) -> dict[str, dict[str, Any]]: + """Load the active profile's saved-queries YAML; tolerate missing file.""" + f = QUERIES_DIR / f"{profile_name}.yaml" + if not f.is_file(): + return {} + try: + raw = yaml.safe_load(f.read_text(encoding="utf-8")) + except yaml.YAMLError: + return {} + if not isinstance(raw, dict): + return {} + queries = raw.get("queries") + if not isinstance(queries, dict): + return {} + return queries + + +def _discover_batches(profile_name: str) -> list[tuple[str, Path, dict[str, Any]]]: + """Find batch templates in ``~/.config/bcli/batches//*.yaml``. + + CWD-relative discovery is deliberately *not* implemented in v0.4 — + documented as a v0.5 follow-up so the surface stays small. + """ + out: list[tuple[str, Path, dict[str, Any]]] = [] + profile_dir = BATCHES_DIR / profile_name + if not profile_dir.is_dir(): + return out + for path in sorted(profile_dir.glob("*.yaml")): + try: + raw = yaml.safe_load(path.read_text(encoding="utf-8")) + except yaml.YAMLError: + continue + if not isinstance(raw, dict): + continue + # Use ``name:`` from the YAML if present, else the file stem. + name = str(raw.get("name") or path.stem) + out.append((name, path, raw)) + return out + + +# ─── Build the SlashItem list ─────────────────────────────────────── + + +def _items_from_queries(queries: dict[str, dict[str, Any]]) -> list[_SlashItem]: + out: list[_SlashItem] = [] + for name, body in queries.items(): + if not isinstance(body, dict): + continue + description = str(body.get("description") or name) + categories = _coerce_categories(body.get("categories")) + args = _coerce_args(body.get("args"), body.get("params")) + out.append(_SlashItem( + kind="query", + name=name, + slug=f"bcli-{name}", + description=description, + categories=categories, + args=args, + )) + out.sort(key=lambda x: x.name) + return out + + +def _items_from_batches(batches: list[tuple[str, Path, dict[str, Any]]]) -> list[_SlashItem]: + out: list[_SlashItem] = [] + for name, path, raw in batches: + description = str(raw.get("description") or name) + categories = _coerce_categories(raw.get("categories")) + args = _coerce_args(raw.get("args"), raw.get("params")) + out.append(_SlashItem( + kind="batch", + name=name, + slug=f"bcli-batch-{name}", + description=description, + categories=categories, + args=args, + source_path=str(path), + batch_relpath=str(path), + )) + out.sort(key=lambda x: x.name) + return out + + +# ─── Render templates ─────────────────────────────────────────────── + + +def _argument_hint(args: list[_Arg]) -> str: + """One-line hint that Claude Code surfaces in slash command pickers.""" + pieces: list[str] = [] + for a in args: + pieces.append(f"<{a.name}>" if a.required else f"[{a.name}]") + return " ".join(pieces) + + +def _command_string_query(item: _SlashItem) -> str: + """Build the ``bcli q ...`` line for a query slash command.""" + positional = " ".join( + f"{a.name}=${i + 1}" for i, a in enumerate(item.args) + ) + if positional: + return f"bcli q {item.name} {positional} --format json" + return f"bcli q {item.name} --format json" + + +def _command_string_batch(item: _SlashItem) -> str: + """Build the ``bcli batch run ...`` line for a batch slash command.""" + set_clauses = " ".join( + f"--set {a.name}=${i + 1}" for i, a in enumerate(item.args) + ) + flag_block = " ".join(filter(None, [ + set_clauses, + "--format json", + f"--result-out /tmp/{item.name}-$$.json", + ])) + return f"bcli batch run {item.batch_relpath} {flag_block}" + + +def _render_command_md(item: _SlashItem, *, profile_name: str) -> str: + """Render the Markdown file body for a single slash command. + + Two-phase: render with NO content_hash line at all, hash that body, + then splice the ``content_hash: sha256:`` line into the + provenance block. The hash is over the body **excluding** the hash + line — so a consumer can verify integrity by stripping the line + and rehashing, and the file is stable across re-runs (no timestamp + churn, no chicken-and-egg). + """ + body = _render_command_md_body( + item, profile_name=profile_name, content_hash=None, + ) + digest = hashlib.sha256(body.encode("utf-8")).hexdigest() + return _render_command_md_body( + item, profile_name=profile_name, content_hash=digest, + ) + + +def _render_command_md_body( + item: _SlashItem, *, profile_name: str, content_hash: str | None, +) -> str: + hint = _argument_hint(item.args) + if item.kind == "query": + cmd_string = _command_string_query(item) + body_intro = ( + "Run the saved query and emit JSON for an agent to consume.\n" + ) + usage_args = " ".join(f"<{a.name}>" for a in item.args) + usage_line = f"/bcli-{item.name}" + (f" {usage_args}" if usage_args else "") + else: + cmd_string = _command_string_batch(item) + body_intro = ( + "Run the batch workflow. ``--result-out`` writes the AIP §Phase 2 " + "result envelope to a tmp file for the agent to read; ``--format " + "json`` emits per-step JSON to stdout.\n" + ) + usage_args = " ".join(f"<{a.name}>" for a in item.args) + usage_line = f"/{item.slug}" + (f" {usage_args}" if usage_args else "") + + frontmatter = ( + "---\n" + f"description: {item.description}\n" + + (f"argument-hint: {hint}\n" if hint else "") + + "---\n" + ) + hash_line = ( + f" content_hash: sha256:{content_hash}\n" + if content_hash is not None + else "" + ) + provenance = ( + "\n" + ) + args_table = "" + if item.args: + rows = "\n".join( + f"| `{a.name}` | {a.type} | " + f"{'required' if a.required else 'optional'} | " + f"{a.example or '—'} |" + for a in item.args + ) + args_table = ( + "\n## Arguments\n\n" + "| name | type | required | example |\n" + "|------|------|----------|---------|\n" + f"{rows}\n" + ) + body = ( + f"{frontmatter}\n" + f"{provenance}\n" + f"# {item.slug}\n\n" + f"{item.description}\n\n" + f"{body_intro}\n" + "## Usage\n\n" + f"```\n{usage_line}\n```\n" + f"{args_table}\n" + "## Implementation\n\n" + "```bash\n" + f"{cmd_string}\n" + "```\n" + ) + return body + + +def _render_index_md(items: list[_SlashItem], *, profile_name: str) -> str: + """Render the top-level SKILL.md, grouped by category.""" + by_cat: dict[str, list[_SlashItem]] = {} + for item in items: + for cat in item.categories or ["unsorted"]: + by_cat.setdefault(cat, []).append(item) + # Stable order: alphabetical categories, but ``unsorted`` last. + ordered_cats = sorted( + by_cat.keys(), + key=lambda c: (c == "unsorted", c), + ) + + lines: list[str] = [] + lines.append("---") + lines.append("name: bcli") + lines.append("description: bcli slash commands generated from saved queries and batch workflows.") + lines.append("---") + lines.append("") + lines.append("") + lines.append("") + lines.append(f"# bcli — profile `{profile_name}`") + lines.append("") + lines.append( + "Slash commands and workflow runners projected from your saved " + "queries (`bcli q`) and batch templates (`bcli batch run`)." + ) + lines.append("") + + for cat in ordered_cats: + lines.append(f"## {cat}") + lines.append("") + for it in sorted(by_cat[cat], key=lambda x: x.slug): + lines.append(f"- `/{it.slug}` — {it.description}") + lines.append("") + + body = "\n".join(lines).rstrip() + "\n" + digest = hashlib.sha256(body.encode("utf-8")).hexdigest() + return body + f"\n\n" + + +# ─── manual: true frontmatter detection ───────────────────────────── + + +_MANUAL_RE = re.compile( + r"\A---\s*\n(?P.*?)\n---\s*\n", + flags=re.DOTALL, +) + + +def _is_manual(path: Path) -> bool: + """True iff the existing file declares ``manual: true`` in frontmatter.""" + if not path.is_file(): + return False + try: + text = path.read_text(encoding="utf-8") + except OSError: + return False + m = _MANUAL_RE.search(text) + if not m: + return False + fm_body = m.group("fm") + try: + loaded = yaml.safe_load(fm_body) or {} + except yaml.YAMLError: + return False + return bool(loaded.get("manual", False)) + + +# ─── Atomic write helper ──────────────────────────────────────────── + + +def _atomic_write(path: Path, content: str) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(prefix=path.name + ".", dir=str(path.parent)) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(content) + os.replace(tmp, path) + except Exception: + try: + os.unlink(tmp) + except OSError: + pass + raise + + +# ─── Hash-check idempotency ───────────────────────────────────────── + + +def _existing_hash(path: Path) -> str | None: + if not path.is_file(): + return None + try: + text = path.read_text(encoding="utf-8") + except OSError: + return None + m = re.search(r"content_hash:\s*sha256:([0-9a-f]+)", text) + return m.group(1) if m else None + + +def _content_hash_of(rendered: str) -> str: + m = re.search(r"content_hash:\s*sha256:([0-9a-f]+)", rendered) + return m.group(1) if m else "" + + +# ─── Target resolution ────────────────────────────────────────────── + + +def _resolve_target_dir(target: Path | None) -> Path: + """Resolve where to drop ``.claude/commands/``. + + 1. Explicit ``--target`` always wins. + 2. CWD has ``.claude/`` → use CWD (project-local convention). + 3. Otherwise ``~/.claude/`` (user-global; Claude Code default). + """ + if target is not None: + return target + cwd = Path.cwd() + if (cwd / ".claude").is_dir(): + return cwd + return Path.home() + + +# ─── Main command ─────────────────────────────────────────────────── + + +@app.command("install") +def install_command( + target: Path | None = typer.Option( + None, + "--target", "-t", + help="Project root to write `.claude/commands/` and `.claude/skills/bcli/SKILL.md` into. " + "Defaults to CWD if it has a `.claude/` dir, else $HOME.", + ), + dry_run: bool = typer.Option( + False, "--dry-run", + help="Print what would be generated without writing anything.", + ), +) -> None: + """Project saved queries + batch templates as Claude Code slash commands. + + Reads the active profile's saved queries (`~/.config/bcli/queries/ + .yaml`) and batch templates (`~/.config/bcli/batches/ + /*.yaml`), generates one `.claude/commands/bcli-.md` + per item, plus a `.claude/skills/bcli/SKILL.md` index grouped by + `categories:`. + + Idempotent — re-runs are no-ops when the source hasn't changed. + Files with `manual: true` in their frontmatter are preserved. + + \b + Examples: + bcli skill install # default target (CWD or $HOME) + bcli skill install --target ./proj # write into a specific project + bcli skill install --dry-run # preview without writing + """ + profile_name = state.active_profile_name + target_dir = _resolve_target_dir(target) + cmds_dir = target_dir / ".claude" / "commands" + index_path = target_dir / ".claude" / "skills" / "bcli" / "SKILL.md" + + queries = _load_queries(profile_name) + batches = _discover_batches(profile_name) + + items = _items_from_queries(queries) + _items_from_batches(batches) + + if not items: + console.print( + "[yellow]No saved queries or batch templates found for profile " + f"'{profile_name}'.[/yellow]" + ) + console.print( + f" Add YAML files under [bold]{QUERIES_DIR}/{profile_name}.yaml[/bold]\n" + f" or [bold]{BATCHES_DIR}/{profile_name}/[/bold] and re-run." + ) + return + + written: list[str] = [] + skipped_manual: list[str] = [] + skipped_unchanged: list[str] = [] + + for item in items: + out_path = cmds_dir / f"{item.slug}.md" + rendered = _render_command_md(item, profile_name=profile_name) + + if _is_manual(out_path): + skipped_manual.append(item.slug) + continue + + existing = _existing_hash(out_path) + target_hash = _content_hash_of(rendered) + if existing == target_hash and existing: + skipped_unchanged.append(item.slug) + continue + + if dry_run: + written.append(item.slug) + continue + + _atomic_write(out_path, rendered) + written.append(item.slug) + + # Index: written last so consumers don't see it before the commands. + index_body = _render_index_md(items, profile_name=profile_name) + index_existing = _existing_hash(index_path) + index_target_hash = _content_hash_of(index_body) + index_changed = index_existing != index_target_hash + if index_changed: + if not dry_run: + _atomic_write(index_path, index_body) + + _summarise( + dry_run=dry_run, + written=written, + skipped_manual=skipped_manual, + skipped_unchanged=skipped_unchanged, + index_changed=index_changed, + cmds_dir=cmds_dir, + index_path=index_path, + ) + + +def _summarise( + *, + dry_run: bool, + written: list[str], + skipped_manual: list[str], + skipped_unchanged: list[str], + index_changed: bool, + cmds_dir: Path, + index_path: Path, +) -> None: + verb = "Would write" if dry_run else "Wrote" + if written: + console.print( + f"[green]{verb}[/green] {len(written)} slash command(s) → " + f"[bold]{cmds_dir}[/bold]" + ) + for slug in sorted(written): + console.print(f" /{slug}") + if index_changed: + console.print( + f"[green]{verb}[/green] skill index → [bold]{index_path}[/bold]" + ) + if skipped_unchanged: + console.print( + f"[dim]Unchanged ({len(skipped_unchanged)}): " + f"{', '.join(sorted(skipped_unchanged))}[/dim]" + ) + if skipped_manual: + console.print( + f"[dim]Preserved manual files ({len(skipped_manual)}): " + f"{', '.join(sorted(skipped_manual))}[/dim]" + ) + + +__all__ = ["app", "install_command"] diff --git a/tests/test_skill_install/__init__.py b/tests/test_skill_install/__init__.py new file mode 100644 index 0000000..a3dcc3c --- /dev/null +++ b/tests/test_skill_install/__init__.py @@ -0,0 +1 @@ +"""Tests for ``bcli skill install`` — AIP §Phase 7 / tasks/todo.md Phase 6.""" diff --git a/tests/test_skill_install/test_skill_install.py b/tests/test_skill_install/test_skill_install.py new file mode 100644 index 0000000..268dfa6 --- /dev/null +++ b/tests/test_skill_install/test_skill_install.py @@ -0,0 +1,484 @@ +"""``bcli skill install`` generates per-query + per-batch slash commands. + +Decisions baked into these tests (see PR body and docs/saved-queries.md): + +* ``args:`` is OPTIONAL on a saved query — if missing, the installer + derives it from ``params:`` (required first, then with-default, both + in YAML insertion order). Existing saved-query bundles work + unchanged. +* ``bcli skill`` is a Typer **group** so Worker B can land + ``bcli skill init`` (Phase 7 wizard) alongside without restructuring. +* ``manual: true`` opt-out is YAML **frontmatter** only, not a comment. +* Skill index lives at ``/.claude/skills/bcli/SKILL.md`` — + matches Claude Code's documented skill-loading convention. +* Idempotency: each generated file embeds a ``content_hash:`` of the + rest of the body; re-runs that produce the same hash are a no-op. + No ``generated_at`` timestamp (would invalidate the hash on every run). +""" + +from __future__ import annotations + +import hashlib +import re +from pathlib import Path +from textwrap import dedent + +import pytest +from typer.testing import CliRunner + +from bcli.config._model import BCConfig, BCDefaults, BCProfile +from bcli_cli._state import state +from bcli_cli.app import app + +runner = CliRunner() + + +# ─── Fixtures ──────────────────────────────────────────────────────── + + +@pytest.fixture(autouse=True) +def isolated_home(tmp_path, monkeypatch): + """Scope HOME / Path.home to tmp_path so the installer's source-of-truth + paths (queries dir, batches dir, default target) all live under + ``tmp_path``. The CONFIG_DIR module constants are patched so + ``query_cmd.QUERIES_DIR`` and our new ``BATCHES_DIR`` resolve here. + """ + config_dir = tmp_path / ".config" / "bcli" + config_dir.mkdir(parents=True) + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setattr(Path, "home", lambda: tmp_path) + monkeypatch.setattr("bcli.config._defaults.CONFIG_DIR", config_dir) + # query_cmd binds QUERIES_DIR at module load — patch the attribute + # directly so the installer reads from our tmp tree. + monkeypatch.setattr( + "bcli_cli.commands.query_cmd.QUERIES_DIR", config_dir / "queries", + ) + monkeypatch.setattr( + "bcli_cli.commands.skill_cmd.QUERIES_DIR", config_dir / "queries", + raising=False, + ) + monkeypatch.setattr( + "bcli_cli.commands.skill_cmd.BATCHES_DIR", config_dir / "batches", + raising=False, + ) + yield tmp_path + + +@pytest.fixture +def cli_state(): + """Active profile required so the installer can resolve its sources.""" + cfg = BCConfig( + defaults=BCDefaults(profile="finance"), + profiles={ + "finance": BCProfile( + tenant_id="t1", + environment="Sandbox", + company_id="c-1", + disable_writes=False, + ), + }, + ) + state._config = cfg + state._registry = None + state._telemetry = None + state.profile_name = None + state.env_override = None + state.company_override = None + state.format = "table" + state.dry_run = False + state.quiet = True + yield state + state._config = None + state._registry = None + state._telemetry = None + state.profile_name = None + + +def _write_queries(home: Path, body: str) -> Path: + f = home / ".config" / "bcli" / "queries" / "finance.yaml" + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(dedent(body).strip() + "\n", encoding="utf-8") + return f + + +def _write_batch(home: Path, name: str, body: str) -> Path: + f = home / ".config" / "bcli" / "batches" / "finance" / f"{name}.yaml" + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(dedent(body).strip() + "\n", encoding="utf-8") + return f + + +def _invoke(*args: str): + return runner.invoke(app, ["skill", "install", *args]) + + +# ─── Command per saved query ──────────────────────────────────────── + + +def test_skill_install_creates_command_per_saved_query(isolated_home, cli_state): + _write_queries(isolated_home, """ + queries: + utilization-by-esn: + description: Engine utilization for an ESN + categories: [aviation, daily-ops] + args: + - name: esn + type: string + example: "424322" + required: true + endpoint: util_history + filter: "engine_serial eq '${{ args.esn }}'" + + customer-by-name: + description: Look up a customer by display name + categories: [finance] + endpoint: customers + params: + name: + required: true + type: string + filter: "displayName eq '${{ params.name }}'" + """) + target = isolated_home / "proj" + target.mkdir() + result = _invoke("--target", str(target)) + assert result.exit_code == 0, result.stdout + result.stderr + + cmds_dir = target / ".claude" / "commands" + assert cmds_dir.is_dir() + util = cmds_dir / "bcli-utilization-by-esn.md" + cust = cmds_dir / "bcli-customer-by-name.md" + assert util.is_file() + assert cust.is_file() + + util_text = util.read_text(encoding="utf-8") + # Frontmatter has a description. + assert "description: Engine utilization for an ESN" in util_text + # The body invokes ``bcli q ...`` with the args threaded positionally. + assert "bcli q utilization-by-esn esn=$1" in util_text + # The body references --format json (agent-friendly). + assert "--format json" in util_text + + # customer-by-name has no explicit args list — installer derives one + # from the params keys. + cust_text = cust.read_text(encoding="utf-8") + assert "bcli q customer-by-name name=$1" in cust_text + + +def test_skill_install_args_inferred_from_params_when_omitted(isolated_home, cli_state): + """No ``args:`` on a query → installer derives it from ``params:`` order. + + Required params come first; optional (with ``default``) follow. Both + in YAML insertion order so the slash command is stable. + """ + _write_queries(isolated_home, """ + queries: + open-invoices: + description: Outstanding invoices + endpoint: customerSalesInvoices + params: + customer-id: + required: true + limit: + default: 50 + order: + default: dueDate + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = (target / ".claude" / "commands" / "bcli-open-invoices.md").read_text() + # customer-id (required) first, then limit + order (optional) — all + # threaded as positional → key form so a /bcli-open-invoices call + # works ergonomically. + assert "bcli q open-invoices customer-id=$1 limit=$2 order=$3" in md + + +def test_skill_install_renders_argument_hint_in_frontmatter(isolated_home, cli_state): + _write_queries(isolated_home, """ + queries: + vendor-by-no: + description: Vendor by number + args: + - {name: vendor-no, required: true, example: "V00010"} + endpoint: vendors + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = (target / ".claude" / "commands" / "bcli-vendor-by-no.md").read_text() + # Required arg shown without brackets; optional args (none here) would + # be ``[name]``. Format: "". + assert re.search(r"argument-hint:\s*", md) + + +# ─── Command per batch YAML ───────────────────────────────────────── + + +def test_skill_install_creates_command_per_batch_yaml(isolated_home, cli_state): + _write_batch(isolated_home, "engine-360", """ + name: engine-360 + description: Full engine 360 for a given ESN + categories: [aviation] + params: + esn: + required: true + steps: + - action: get + endpoint: engines + params: + filter: "serial eq '${{ params.esn }}'" + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = (target / ".claude" / "commands" / "bcli-batch-engine-360.md") + assert md.is_file() + text = md.read_text(encoding="utf-8") + # Body invokes bcli batch run with --format json + --result-out. + assert "bcli batch run" in text + assert "engine-360" in text + assert "--format json" in text + assert "--result-out" in text + + +# ─── Skill index ──────────────────────────────────────────────────── + + +def test_skill_install_generates_index_grouped_by_categories( + isolated_home, cli_state, +): + _write_queries(isolated_home, """ + queries: + util-a: + description: Aviation util A + categories: [aviation] + endpoint: a + util-b: + description: Aviation util B + categories: [aviation] + endpoint: b + customer-by-name: + description: Look up a customer + categories: [finance] + endpoint: customers + params: + name: {required: true} + orphan: + description: Uncategorized + endpoint: orphans + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + + index = target / ".claude" / "skills" / "bcli" / "SKILL.md" + assert index.is_file() + body = index.read_text(encoding="utf-8") + # Each category is a section header. Headings are sorted + # alphabetically for deterministic output; ``unsorted`` (the + # fallback bucket for the ``orphan`` query) sorts to the end. + assert "## aviation" in body + assert "## finance" in body + assert "## unsorted" in body + # Each command listed under its category. + assert "bcli-util-a" in body + assert "bcli-util-b" in body + assert "bcli-customer-by-name" in body + assert "bcli-orphan" in body + + +# ─── Idempotency ───────────────────────────────────────────────────── + + +def test_skill_install_idempotent_no_changes(isolated_home, cli_state): + """Second run on unchanged sources rewrites nothing. + + Verified by mtime: the file's mtime after the second run equals + the first run's mtime. + """ + _write_queries(isolated_home, """ + queries: + q1: + description: One + categories: [a] + endpoint: ones + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = target / ".claude" / "commands" / "bcli-q1.md" + mtime_1 = md.stat().st_mtime_ns + + import time as _t + _t.sleep(0.05) # ensure mtime granularity differs if we did write + + _invoke("--target", str(target)) + mtime_2 = md.stat().st_mtime_ns + assert mtime_2 == mtime_1, ( + "second run must NOT rewrite an unchanged generated file" + ) + + +def test_skill_install_rewrites_when_source_changes(isolated_home, cli_state): + """When the saved-query YAML changes, the generated file regenerates.""" + _write_queries(isolated_home, """ + queries: + q1: + description: One + endpoint: ones + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = target / ".claude" / "commands" / "bcli-q1.md" + text_1 = md.read_text(encoding="utf-8") + + # Change the description. + _write_queries(isolated_home, """ + queries: + q1: + description: Now Two + endpoint: ones + """) + _invoke("--target", str(target)) + text_2 = md.read_text(encoding="utf-8") + assert text_1 != text_2 + assert "Now Two" in text_2 + + +# ─── Manual file preservation ─────────────────────────────────────── + + +def test_skill_install_preserves_manual_files(isolated_home, cli_state): + """A manually-edited slash command file is NEVER overwritten. + + Convention: YAML frontmatter ``manual: true``. Comment-style + ``# manual: true`` is intentionally NOT honored. + """ + _write_queries(isolated_home, """ + queries: + custom-tool: + description: Auto-generated description + endpoint: x + """) + target = isolated_home / "proj" + target.mkdir() + cmds_dir = target / ".claude" / "commands" + cmds_dir.mkdir(parents=True) + manual = cmds_dir / "bcli-custom-tool.md" + manual_body = dedent(""" + --- + manual: true + description: HAND CRAFTED + --- + + Custom body, do not touch. + """).strip() + manual.write_text(manual_body, encoding="utf-8") + + _invoke("--target", str(target)) + + assert manual.read_text(encoding="utf-8") == manual_body, ( + "frontmatter `manual: true` must protect the file from regeneration" + ) + + +# ─── Dry-run ───────────────────────────────────────────────────────── + + +def test_skill_install_dry_run_writes_nothing(isolated_home, cli_state): + _write_queries(isolated_home, """ + queries: + q1: + description: One + endpoint: ones + """) + target = isolated_home / "proj" + target.mkdir() + result = _invoke("--target", str(target), "--dry-run") + assert result.exit_code == 0 + assert not (target / ".claude" / "commands" / "bcli-q1.md").exists() + # Some announcement that this would have been generated. + stdout = result.stdout + assert "bcli-q1" in stdout + assert "dry-run" in stdout.lower() or "would" in stdout.lower() + + +# ─── Hash idempotency primitive ───────────────────────────────────── + + +def test_skill_install_embedded_content_hash_is_stable(isolated_home, cli_state): + """The provenance ``content_hash`` is computed over the file body + *excluding* the hash line itself, so it stays stable across runs. + A consumer can recompute it locally and verify integrity. + """ + _write_queries(isolated_home, """ + queries: + q1: + description: Stable + endpoint: ones + """) + target = isolated_home / "proj" + target.mkdir() + _invoke("--target", str(target)) + md = (target / ".claude" / "commands" / "bcli-q1.md").read_text() + m = re.search(r"content_hash:\s*sha256:([0-9a-f]{16,64})", md) + assert m, f"expected content_hash in generated file:\n{md}" + declared = m.group(1) + + # Recompute over the body with the entire hash *line* removed (the + # documented verification protocol — strip the whole line, not just + # the value, so the digest matches the producer's pre-injection body). + stripped = re.sub( + r"^[ \t]*content_hash:\s*sha256:[0-9a-f]+\s*\n", + "", + md, + flags=re.MULTILINE, + ) + recomputed = hashlib.sha256(stripped.encode("utf-8")).hexdigest() + assert recomputed == declared, ( + f"content_hash mismatch: declared={declared} recomputed={recomputed}" + ) + + +# ─── Target directory ─────────────────────────────────────────────── + + +def test_skill_install_target_dir_defaults_to_cwd_when_dot_claude_present( + isolated_home, cli_state, monkeypatch, +): + """If CWD has a ``.claude/`` dir, default target is CWD; otherwise + fall back to ``~/.claude/``. Tested by running once without + ``--target`` from a dir with .claude/ present. + """ + _write_queries(isolated_home, """ + queries: + q1: + description: One + endpoint: ones + """) + project = isolated_home / "proj-cwd" + project.mkdir() + (project / ".claude").mkdir() + monkeypatch.chdir(project) + + result = _invoke() + assert result.exit_code == 0, result.stdout + result.stderr + assert (project / ".claude" / "commands" / "bcli-q1.md").is_file() + + +# ─── Empty / missing inputs ───────────────────────────────────────── + + +def test_skill_install_no_queries_no_batches_is_a_noop(isolated_home, cli_state): + """No saved queries + no batch templates → command succeeds with a + helpful message; no files written under .claude/commands/.""" + target = isolated_home / "proj" + target.mkdir() + result = _invoke("--target", str(target)) + assert result.exit_code == 0 + cmds_dir = target / ".claude" / "commands" + # Either directory is missing entirely OR it exists empty — both fine. + if cmds_dir.exists(): + assert not any(cmds_dir.glob("bcli-*.md"))