From 1f2259d41386a4a6269a0e64b5f1621189204a0e Mon Sep 17 00:00:00 2001 From: Tyler Date: Sun, 21 Jun 2026 12:45:23 -0600 Subject: [PATCH] feat(sp-skill-catalog): add cyclone-cli skill (CLI conventions) --- .superpowers/skills/cyclone-cli/SKILL.md | 187 +++++++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 .superpowers/skills/cyclone-cli/SKILL.md diff --git a/.superpowers/skills/cyclone-cli/SKILL.md b/.superpowers/skills/cyclone-cli/SKILL.md new file mode 100644 index 0000000..fd8c37c --- /dev/null +++ b/.superpowers/skills/cyclone-cli/SKILL.md @@ -0,0 +1,187 @@ +--- +name: cyclone-cli +description: "Cyclone CLI subcommand conventions (cli.py — Click group + subcommands parse-837/parse-835/validate-npi/validate-tax-id/backup, --yes + click.confirm for destructive ops, exit codes 0/1/2, CliRunner smoke tests in backend/tests/test_cli_*.py). Use when: adding a CLI subcommand, changing an exit code, adding a smoke test, or wiring a security-sensitive command (backup, key rotation, anything touching secrets.py)." +--- + +# cyclone-cli + +The operator-facing CLI is a **Click** group at `cli.py:45` (`@click.group()` for `main`), mounted as the `cyclone` console script in `pyproject.toml:54` (`cyclone = "cyclone.cli:main"`). Seven subcommands ship today: `parse-837`, `parse-835`, `validate-npi`, `validate-tax-id`, plus the `backup` group (`init-passphrase`, `create`, `list`, `verify`, `restore`, `prune`, `status`). `serve` lives separately in `__main__.py:19` (dispatches `uvicorn cyclone.api:app`). + +## When to use + +- **Adding a subcommand.** You need a new operator command (e.g. `cyclone rotate-key`) and want to match the existing Click decorator + smoke-test rhythm. +- **Changing an exit code.** You're tweaking which `sys.exit(N)` a subcommand raises and need the 0/1/2 contract used by `parse_837`, `parse_835`, `validate_npi_cmd`, `validate_tax_id_cmd`, and the `backup` group. +- **Adding a smoke test.** You need `backend/tests/test_cli_.py` using `click.testing.CliRunner` (NOT `subprocess.run`) and want the canonical fixture + monkeypatch layout (Keychain stub, fresh SQLite, `CYCLONE_BACKUP_DIR`). +- **Wiring a security-sensitive command.** Anything touching `cyclone/secrets.py` (Keychain writes), DB key rotation, or destructive restores needs the two-step confirm dance used by `backup restore` / `backup prune` (`cli.py:462,509`). + +## Conventions + +1. **Click decorator pattern, not argparse.** Each subcommand is a + top-level function decorated with `@main.command("")` and + one `@click.option` / `@click.argument` per parameter. Group + dispatch is implicit — no `set_defaults(func=...)` and no + `cmd_(args) -> int` signature. Top-level commands at + `cli.py:77,151,241,261,303`; the `backup` sub-group nests a + second `@main.group()` (`cli.py:303`) with its own + `@backup.command("")` children. +2. **Long-form flags for safety.** Prefer `--rotate-key`, + `--backup-dir`, `--from-stdin` over positional args for anything + that mutates state or takes a secret. `init-passphrase` + (`cli.py:308-311`) demonstrates the canonical "flag OR stdin" + pattern: `--passphrase` for automation, `--from-stdin` for + interactive `getpass()` prompting. +3. **Exit codes: 0 / 1 / 2.** `0` = success. `1` = user / input + error (invalid NPI/EIN at `cli.py:258,277,285`; Keychain write + failure at `cli.py:341,351`; tampered-backup verify at + `cli.py:459`). `2` = operator / parse error + (`CycloneParseError` at `cli.py:106,178`; passphrase + empty/mismatch/short at `cli.py:331,337`). Document the codes + in the docstring (see `validate_npi_cmd` at `cli.py:244-250`). + Use `click.UsageError(...)` for usage mistakes; reserve + `sys.exit(2)` for "the file failed to parse" semantics. +4. **Smoke test with `click.testing.CliRunner`.** Every new + subcommand gets `backend/tests/test_cli_.py` that + imports `from cyclone.cli import main` and invokes via + `CliRunner().invoke(main, [...], catch_exceptions=False)`. The + test must stub Keychain (`monkeypatch.setattr(secrets_mod, + "get_secret"/"set_secret", ...)`) and pin a temp SQLite DB via + `CYCLONE_DB_URL` + `db._reset_for_tests()` — see + `backend/tests/test_cli_backup.py:13-69` for the canonical + `_cli_env` fixture. CliRunner captures output and exit codes + in-process; do NOT shell out to `subprocess.run`. +5. **Destructive ops need `--yes` + `click.confirm(abort=True)`.** + `backup restore` (`cli.py:462-506`) and `backup prune` + (`cli.py:509-537`) both gate the destructive action behind a + `--yes` is_flag and an interactive `click.confirm(..., abort=True)` + prompt. CliRunner auto-aborts confirm prompts, so smoke tests + assert `exit_code != 0` when `--yes` is omitted + (`test_cli_backup.py:122-137`). A `--dry-run` flag is NOT yet + implemented anywhere; if needed, mirror the `--yes` pattern. + +## Patterns + +### A Click subcommand — `@main.command("")` + options + +From `cli.py:241-258` (smallest standalone subcommand): + +```python +@main.command("validate-npi") +@click.argument("npi") +@click.option("--log-level", default="WARNING", show_default=True, + type=click.Choice(["DEBUG", "INFO", "WARNING", "ERROR"])) +def validate_npi_cmd(npi: str, log_level: str) -> None: + """Validate a 10-digit NPI's Luhn checksum locally (SP20). Exit 0 valid, 1 invalid. PHI — don't log the value.""" + setup_logging(level=log_level) + from cyclone.npi import is_valid_npi + if is_valid_npi(npi): + click.echo(f"OK: {len(npi)}-digit NPI passes Luhn checksum") + return + click.echo(f"INVALID: {npi!r} fails NPI Luhn checksum", err=True) + sys.exit(1) +``` + +### A smoke test — `CliRunner` + Keychain stub + temp SQLite + +From `test_cli_backup.py:13-69`. Stable hex salt keeps multiple `CliRunner` invocations consistent within one test. The `Batch` seed at `cli_backup.py:27-35` is omitted — only the Keychain + DB plumbing is the convention: + +```python +@pytest.fixture +def _cli_env(tmp_path, monkeypatch): + from cyclone import db, secrets as secrets_mod + from cyclone import backup_service as svc_mod + + monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db") + db._reset_for_tests() + db.init_db() + # ... seed any DB rows the subcommand needs (see cli_backup.py:27-35) ... + + store = {svc_mod.KEYCHAIN_BACKUP_PASSPHRASE_ACCOUNT: "cli-test-passphrase", + svc_mod.KEYCHAIN_BACKUP_SALT_ACCOUNT: + "0123456789abcdef0123456789abcdef"} + monkeypatch.setattr(secrets_mod, "get_secret", lambda n: store.get(n)) + monkeypatch.setattr(secrets_mod, "set_secret", + lambda n, v: store.__setitem__(n, v) or True) + monkeypatch.setenv("CYCLONE_BACKUP_DIR", str(tmp_path / "backups")) + yield tmp_path / "backups" + db._reset_for_tests() + +def test_backup_create_list_verify_status(_cli_env): + from cyclone.cli import main + runner = CliRunner() + r = runner.invoke(main, ["backup", "create"], catch_exceptions=False) + assert r.exit_code == 0, r.output + assert "created backup id=" in r.output +``` + +### A destructive subcommand — `--yes` + `click.confirm(abort=True)` + +From `cli.py:462-506` (`backup restore`). Two-step: announce, prompt unless `--yes`, then execute. Restore uses an explicit init/confirm round-trip so the operator can back out between phases. Matching smoke test asserts the guard fires: + +```python +@backup.command("restore") +@click.argument("backup_id", type=int) +@click.option("--yes", is_flag=True, help="Skip the interactive confirm prompt") +@click.option("--actor", default="operator-cli", show_default=True) +def backup_restore(backup_id: int, yes: bool, actor: str) -> None: + """Restore the live DB from a backup (two-step, requires --yes).""" + # ... db.init_db() + service config omitted ... + click.echo(f"Initiating restore from backup {backup_id}...") + init = svc.restore_initiate(backup_id) + # ... echo init summary (filename, fp, table_count, ttl) ... + if not yes: + click.confirm( + "Replace the live DB with this backup? " + "This will dispose the engine and rebuild it.", + abort=True, + ) + click.echo("Confirming restore...") + result = svc.restore_confirm(backup_id, init.restore_token, actor=actor) + +def test_backup_restore_requires_yes_flag(_cli_env): + runner = CliRunner() + runner.invoke(main, ["backup", "create"], catch_exceptions=False) + r = runner.invoke(main, ["backup", "restore", "1"], catch_exceptions=False) + # CliRunner auto-aborts confirm prompts → exit_code != 0. + assert r.exit_code != 0 +``` + +## Anti-patterns + +- **Don't `sys.exit(2)` for usage errors.** Reserve code 2 for + parse/operator errors (`CycloneParseError`, Keychain not + initialized, passphrase policy violation). For "you passed the + wrong flag" use `raise click.UsageError(...)` — Click formats + it as a clean help message and exits 2 on its own. +- **Don't print errors to stdout.** Use `click.echo(msg, err=True)` + for all error output. `print(..., file=sys.stderr)` and bare + `logging.error(...)` bypass Click's stdout/stderr split and leak + into test `result.output` — breaking `assert "FAIL" in r.output` + style assertions. +- **Don't add a subcommand without a smoke test.** Every new + `@main.command(...)` ships a sibling + `backend/tests/test_cli_.py` exercising the happy path + AND at least one error path (missing input, invalid arg, + tampered ciphertext — see `test_cli_backup.py:102-119`). +- **Don't reuse the `parse` command name.** Existing subcommands + are type-specific (`parse-837`, `parse-835`); a generic `parse` + would shadow them or force an `--type` flag — neither is the + codebase pattern. + +## Related skills + +- **`cyclone-store`** — `backup` subcommands and the parse + subcommands both round-trip through `CycloneStore` and the DB + session; load when the increment changes write paths or the + `_written` event contract. +- **`cyclone-api-router`** — `cyclone serve` (via `__main__.py:19`) + launches the FastAPI app; the CLI parse subcommands share the + same `CycloneParseError` exception and Pydantic result models + as the matching HTTP endpoints. +- **`cyclone-edi`** — `parse-837` / `parse-835` are the CLI smoke + entry points for the parser/validator surface; load when adding + a parser or R-code rule. +- **`cyclone-tests`** — every CLI subcommand gets a pytest smoke + case under `backend/tests/test_cli_.py`; the `_cli_env` + fixture pattern (fresh SQLite + Keychain stub) is documented + there. +- **`cyclone-spec`** — load when the SP-N spec introduces a new operator command or reserves a new exit-code category. \ No newline at end of file